Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 5 Oct 2011 17:46:05 +0300
From:      Kostik Belousov <kostikbel@gmail.com>
To:        "Jayachandran C." <c.jayachandran@gmail.com>
Cc:        mips@freebsd.org
Subject:   Re: Mips syscall entry point
Message-ID:  <20111005144605.GC1511@deviant.kiev.zoral.com.ua>
In-Reply-To: <CA%2B7sy7CKYLL7%2BMjB242=XkFP=S728m1=77%2BBqEra9KcmJ0tBKg@mail.gmail.com>
References:  <20111004211144.GW1511@deviant.kiev.zoral.com.ua> <20111004215218.GY1511@deviant.kiev.zoral.com.ua> <CA%2B7sy7BfMgyw5E%2BP6QzcH02Fn4eMNiD%2B__d0Ji8Fjq9rXBg5Lg@mail.gmail.com> <CA%2B7sy7CKYLL7%2BMjB242=XkFP=S728m1=77%2BBqEra9KcmJ0tBKg@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help

--fmrJ4y+ZXhodCOcq
Content-Type: text/plain; charset=koi8-r
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Wed, Oct 05, 2011 at 05:56:10PM +0530, Jayachandran C. wrote:
> On Wed, Oct 5, 2011 at 5:05 PM, Jayachandran C.
> <c.jayachandran@gmail.com> wrote:
> > On Wed, Oct 5, 2011 at 3:22 AM, Kostik Belousov <kostikbel@gmail.com> w=
rote:
> >> On Wed, Oct 05, 2011 at 12:11:44AM +0300, Kostik Belousov wrote:
> >>> Hi,
> >>> below is the patch, test-compiled for XLP64 only, which converts the
> >>> only remaining architecture MIPS to the new syscall entry sequence.
> >>> The advantage of the conversion is sharing most of the code with all
> >>> other architectures and avoiding duplication. Also, the implementation
> >>> automatically feels the missed features for the MIPS, see the BUGS
> >> s/feels/fills/, sorry
> >>> section in the ptrace(2).
> >> For the same reason, capsicum shall not work on MIPS.
> >>
> >>>
> >>> I am asking for you help to debug and test the patch. Please keep me
> >>> on Cc:, I am not on the list.
> >>>
> >>> Thank you.
> >>>
> >>> diff --git a/sys/mips/include/proc.h b/sys/mips/include/proc.h
> >>> index 11a1f8e..4c0b0b6 100644
> [...]
> >
> > This gives me a crash when I test it on XLR (32bit compile). =9AThe
> > crash does not look obvious - I am looking at it, hope to resolve this
> > soon.
>=20
> Actually it is fairly obvious :)  the elf*_machdep.c has to be updated
> for using the cpu_fetch_syscall_args. With that change it comes up on
> 32 bit - will do a few more tests on 64 bit to see how that goes.
>=20
> The other minor issue I saw was the locr0 usage in trap(), in call to
> trapdebug_enter,  it is fine now since  TRAP_DEBUG is not defined.

Thank you very much. Your fix is applied, and I tried to cover both
locr0 and code usage.

Will wait for your testing.

diff --git a/lib/libc/sys/ptrace.2 b/lib/libc/sys/ptrace.2
index e7eb7d6..4359228 100644
--- a/lib/libc/sys/ptrace.2
+++ b/lib/libc/sys/ptrace.2
@@ -2,7 +2,7 @@
 .\"	$NetBSD: ptrace.2,v 1.2 1995/02/27 12:35:37 cgd Exp $
 .\"
 .\" This file is in the public domain.
-.Dd October 3, 2011
+.Dd October 5, 2011
 .Dt PTRACE 2
 .Os
 .Sh NAME
@@ -599,11 +599,3 @@ The
 .Fn ptrace
 function appeared in
 .At v7 .
-.Sh BUGS
-The
-.Dv PL_FLAG_FORKED ,
-.Dv PL_FLAG_SCE ,
-.Dv PL_FLAG_SCX
-and
-.Dv PL_FLAG_EXEC
-are not implemented for MIPS architecture.
diff --git a/sys/mips/include/proc.h b/sys/mips/include/proc.h
index 11a1f8e..4c0b0b6 100644
--- a/sys/mips/include/proc.h
+++ b/sys/mips/include/proc.h
@@ -67,11 +67,22 @@ struct mdproc {
 	/* empty */
 };
=20
+#ifdef _KERNEL
 struct thread;
=20
 void	mips_cpu_switch(struct thread *, struct thread *, struct mtx *);
 void	mips_cpu_throw(struct thread *, struct thread *);
=20
+struct syscall_args {
+	u_int code;
+	struct sysent *callp;
+	register_t args[8];
+	int narg;
+	struct trapframe *trapframe;
+};
+#define	HAVE_SYSCALL_ARGS_DEF 1
+#endif
+
 #ifdef __mips_n64
 #define	KINFO_PROC_SIZE 1088
 #else
diff --git a/sys/mips/mips/elf64_machdep.c b/sys/mips/mips/elf64_machdep.c
index 9fa31fa..ee25ef4 100644
--- a/sys/mips/mips/elf64_machdep.c
+++ b/sys/mips/mips/elf64_machdep.c
@@ -80,8 +80,8 @@ struct sysentvec elf64_freebsd_sysvec =3D {
 	.sv_maxssiz	=3D NULL,
 	.sv_flags	=3D SV_ABI_FREEBSD | SV_LP64,
 	.sv_set_syscall_retval =3D cpu_set_syscall_retval,
-	.sv_fetch_syscall_args =3D NULL, /* XXXKIB */
-	.sv_syscallnames =3D NULL,
+	.sv_fetch_syscall_args =3D cpu_fetch_syscall_args,
+	.sv_syscallnames =3D syscallnames,
 	.sv_schedtail	=3D NULL,
 };
=20
diff --git a/sys/mips/mips/elf_machdep.c b/sys/mips/mips/elf_machdep.c
index 41611e3..85ada0b 100644
--- a/sys/mips/mips/elf_machdep.c
+++ b/sys/mips/mips/elf_machdep.c
@@ -80,7 +80,7 @@ struct sysentvec elf64_freebsd_sysvec =3D {
 	.sv_maxssiz	=3D NULL,
 	.sv_flags	=3D SV_ABI_FREEBSD | SV_LP64,
 	.sv_set_syscall_retval =3D cpu_set_syscall_retval,
-	.sv_fetch_syscall_args =3D NULL, /* XXXKIB */
+	.sv_fetch_syscall_args =3D cpu_fetch_syscall_args,
 	.sv_syscallnames =3D syscallnames,
 	.sv_schedtail	=3D NULL,
 };
@@ -136,7 +136,7 @@ struct sysentvec elf32_freebsd_sysvec =3D {
 	.sv_maxssiz	=3D NULL,
 	.sv_flags	=3D SV_ABI_FREEBSD | SV_ILP32,
 	.sv_set_syscall_retval =3D cpu_set_syscall_retval,
-	.sv_fetch_syscall_args =3D NULL, /* XXXKIB */
+	.sv_fetch_syscall_args =3D cpu_fetch_syscall_args,
 	.sv_syscallnames =3D syscallnames,
 	.sv_schedtail	=3D NULL,
 };
diff --git a/sys/mips/mips/trap.c b/sys/mips/mips/trap.c
index c800e71..97374a7 100644
--- a/sys/mips/mips/trap.c
+++ b/sys/mips/mips/trap.c
@@ -261,6 +261,133 @@ static int emulate_unaligned_access(struct trapframe =
*frame, int mode);
=20
 extern void fswintrberr(void); /* XXX */
=20
+int
+cpu_fetch_syscall_args(struct thread *td, struct syscall_args *sa)
+{
+	struct trapframe *locr0 =3D td->td_frame;
+	struct sysentvec *se;
+	int error, nsaved;
+
+	bzero(sa->args, sizeof(sa->args));
+
+	/* compute next PC after syscall instruction */
+	td->td_pcb->pcb_tpc =3D sa->trapframe->pc; /* Remember if restart */
+	if (DELAYBRANCH(sa->trapframe->cause))	 /* Check BD bit */
+		locr0->pc =3D MipsEmulateBranch(locr0, sa->trapframe->pc, 0, 0);
+	else
+		locr0->pc +=3D sizeof(int);
+	sa->code =3D locr0->v0;
+
+	switch (sa->code) {
+#if defined(__mips_n32) || defined(__mips_n64)
+	case SYS___syscall:
+		/*
+		 * Quads fit in a single register in
+		 * new ABIs.
+		 *
+		 * XXX o64?
+		 */
+#endif
+	case SYS_syscall:
+		/*
+		 * Code is first argument, followed by
+		 * actual args.
+		 */
+		sa->code =3D locr0->a0;
+		sa->args[0] =3D locr0->a1;
+		sa->args[1] =3D locr0->a2;
+		sa->args[2] =3D locr0->a3;
+		nsaved =3D 3;
+#if defined(__mips_n32) || defined(__mips_n64)
+		sa->args[3] =3D locr0->t4;
+		sa->args[4] =3D locr0->t5;
+		sa->args[5] =3D locr0->t6;
+		sa->args[6] =3D locr0->t7;
+		nsaved +=3D 4;
+#endif
+		break;
+
+#if defined(__mips_o32)
+	case SYS___syscall:
+		/*
+		 * Like syscall, but code is a quad, so as
+		 * to maintain quad alignment for the rest
+		 * of the arguments.
+		 */
+		if (_QUAD_LOWWORD =3D=3D 0)
+			sa->code =3D locr0->a0;
+		else
+			sa->code =3D locr0->a1;
+		sa->args[0] =3D locr0->a2;
+		sa->args[1] =3D locr0->a3;
+		nsaved =3D 2;
+		break;
+#endif
+
+	default:
+		sa->args[0] =3D locr0->a0;
+		sa->args[1] =3D locr0->a1;
+		sa->args[2] =3D locr0->a2;
+		sa->args[3] =3D locr0->a3;
+		nsaved =3D 4;
+#if defined (__mips_n32) || defined(__mips_n64)
+		sa->args[4] =3D locr0->t4;
+		sa->args[5] =3D locr0->t5;
+		sa->args[6] =3D locr0->t6;
+		sa->args[7] =3D locr0->t7;
+		nsaved +=3D 4;
+#endif
+		break;
+	}
+#ifdef TRAP_DEBUG
+	if (trap_debug)
+		printf("SYSCALL #%d pid:%u\n", code, p->p_pid);
+#endif
+
+	se =3D td->td_proc->p_sysent;
+	if (se->sv_mask)
+		sa->code &=3D se->sv_mask;
+
+	if (sa->code >=3D se->sv_size)
+		sa->callp =3D &se->sv_table[0];
+	else
+		sa->callp =3D &se->sv_table[sa->code];
+
+	sa->narg =3D sa->callp->sy_narg;
+
+	if (sa->narg > nsaved) {
+#if defined(__mips_n32) || defined(__mips_n64)
+		/*
+		 * XXX
+		 * Is this right for new ABIs?  I think the 4 there
+		 * should be 8, size there are 8 registers to skip,
+		 * not 4, but I'm not certain.
+		 */
+		printf("SYSCALL #%u pid:%u, nargs > nsaved.\n", sa->code,
+		    td->td_proc->p_pid);
+#endif
+		error =3D copyin((caddr_t)(intptr_t)(locr0->sp +
+		    4 * sizeof(register_t)), (caddr_t)&sa->args[nsaved],
+		   (u_int)(sa->narg - nsaved) * sizeof(register_t));
+		if (error !=3D 0) {
+			locr0->v0 =3D error;
+			locr0->a3 =3D 1;
+		}
+	} else
+		error =3D 0;
+
+	if (error =3D=3D 0) {
+		td->td_retval[0] =3D 0;
+		td->td_retval[1] =3D locr0->v1;
+	}
+
+	return (error);
+}
+
+#undef __FBSDID
+#define __FBSDID(x)
+#include "../../kern/subr_syscall.c"
+
 /*
  * Handle an exception.
  * Called from MipsKernGenException() or MipsUserGenException()
@@ -527,177 +654,19 @@ dofault:
=20
 	case T_SYSCALL + T_USER:
 		{
-			struct trapframe *locr0 =3D td->td_frame;
-			struct sysent *callp;
-			unsigned int code;
-			int nargs, nsaved;
-			register_t args[8];
-
-			bzero(args, sizeof args);
-
-			/*
-			 * note: PCPU_LAZY_INC() can only be used if we can
-			 * afford occassional inaccuracy in the count.
-			 */
-			PCPU_LAZY_INC(cnt.v_syscall);
-			if (td->td_ucred !=3D p->p_ucred)
-				cred_update_thread(td);
-#ifdef KSE
-			if (p->p_flag & P_SA)
-				thread_user_enter(td);
-#endif
-			/* compute next PC after syscall instruction */
-			td->td_pcb->pcb_tpc =3D trapframe->pc;	/* Remember if restart */
-			if (DELAYBRANCH(trapframe->cause)) {	/* Check BD bit */
-				locr0->pc =3D MipsEmulateBranch(locr0, trapframe->pc, 0,
-				    0);
-			} else {
-				locr0->pc +=3D sizeof(int);
-			}
-			code =3D locr0->v0;
+			struct syscall_args sa;
+			int error;
=20
-			switch (code) {
-#if defined(__mips_n32) || defined(__mips_n64)
-			case SYS___syscall:
-				/*
-				 * Quads fit in a single register in
-				 * new ABIs.
-				 *
-				 * XXX o64?
-				 */
-#endif
-			case SYS_syscall:
-				/*
-				 * Code is first argument, followed by
-				 * actual args.
-				 */
-				code =3D locr0->a0;
-				args[0] =3D locr0->a1;
-				args[1] =3D locr0->a2;
-				args[2] =3D locr0->a3;
-				nsaved =3D 3;
-#if defined(__mips_n32) || defined(__mips_n64)
-				args[3] =3D locr0->t4;
-				args[4] =3D locr0->t5;
-				args[5] =3D locr0->t6;
-				args[6] =3D locr0->t7;
-				nsaved +=3D 4;
-#endif
-				break;
-
-#if defined(__mips_o32)
-			case SYS___syscall:
-				/*
-				 * Like syscall, but code is a quad, so as
-				 * to maintain quad alignment for the rest
-				 * of the arguments.
-				 */
-				if (_QUAD_LOWWORD =3D=3D 0) {
-					code =3D locr0->a0;
-				} else {
-					code =3D locr0->a1;
-				}
-				args[0] =3D locr0->a2;
-				args[1] =3D locr0->a3;
-				nsaved =3D 2;
-				break;
-#endif
-
-			default:
-				args[0] =3D locr0->a0;
-				args[1] =3D locr0->a1;
-				args[2] =3D locr0->a2;
-				args[3] =3D locr0->a3;
-				nsaved =3D 4;
-#if defined (__mips_n32) || defined(__mips_n64)
-				args[4] =3D locr0->t4;
-				args[5] =3D locr0->t5;
-				args[6] =3D locr0->t6;
-				args[7] =3D locr0->t7;
-				nsaved +=3D 4;
-#endif
-			}
-#ifdef TRAP_DEBUG
-			if (trap_debug) {
-				printf("SYSCALL #%d pid:%u\n", code, p->p_pid);
-			}
-#endif
-
-			if (p->p_sysent->sv_mask)
-				code &=3D p->p_sysent->sv_mask;
-
-			if (code >=3D p->p_sysent->sv_size)
-				callp =3D &p->p_sysent->sv_table[0];
-			else
-				callp =3D &p->p_sysent->sv_table[code];
-
-			nargs =3D callp->sy_narg;
-
-			if (nargs > nsaved) {
-#if defined(__mips_n32) || defined(__mips_n64)
-				/*
-				 * XXX
-				 * Is this right for new ABIs?  I think the 4 there
-				 * should be 8, size there are 8 registers to skip,
-				 * not 4, but I'm not certain.
-				 */
-				printf("SYSCALL #%u pid:%u, nargs > nsaved.\n", code, p->p_pid);
-#endif
-				i =3D copyin((caddr_t)(intptr_t)(locr0->sp +
-				    4 * sizeof(register_t)), (caddr_t)&args[nsaved],
-				    (u_int)(nargs - nsaved) * sizeof(register_t));
-				if (i) {
-					locr0->v0 =3D i;
-					locr0->a3 =3D 1;
-#ifdef KTRACE
-					if (KTRPOINT(td, KTR_SYSCALL))
-						ktrsyscall(code, nargs, args);
-#endif
-					goto done;
-				}
-			}
-#ifdef TRAP_DEBUG
-			if (trap_debug) {
-				for (i =3D 0; i < nargs; i++) {
-					printf("args[%d] =3D %#jx\n", i, (intmax_t)args[i]);
-				}
-			}
-#endif
-#ifdef SYSCALL_TRACING
-			printf("%s(", syscallnames[code]);
-			for (i =3D 0; i < nargs; i++) {
-				printf("%s%#jx", i =3D=3D 0 ? "" : ", ", (intmax_t)args[i]);
-			}
-			printf(")\n");
-#endif
-#ifdef KTRACE
-			if (KTRPOINT(td, KTR_SYSCALL))
-				ktrsyscall(code, nargs, args);
-#endif
-			td->td_retval[0] =3D 0;
-			td->td_retval[1] =3D locr0->v1;
+			sa.trapframe =3D trapframe;
+			error =3D syscallenter(td, &sa);
=20
 #if !defined(SMP) && (defined(DDB) || defined(DEBUG))
 			if (trp =3D=3D trapdebug)
-				trapdebug[TRAPSIZE - 1].code =3D code;
+				trapdebug[TRAPSIZE - 1].code =3D sa.code;
 			else
-				trp[-1].code =3D code;
+				trp[-1].code =3D sa.code;
 #endif
-			STOPEVENT(p, S_SCE, nargs);
-
-			PTRACESTOP_SC(p, td, S_PT_SCE);
-			i =3D (*callp->sy_call) (td, args);
-#if 0
-			/*
-			 * Reinitialize proc pointer `p' as it may be
-			 * different if this is a child returning from fork
-			 * syscall.
-			 */
-			td =3D curthread;
-			locr0 =3D td->td_frame;
-#endif
-			trapdebug_enter(locr0, -code);
-			cpu_set_syscall_retval(td, i);
+			trapdebug_enter(td->td_frame, -sa.code);
=20
 			/*
 			 * The sync'ing of I & D caches for SYS_ptrace() is
@@ -705,38 +674,7 @@ dofault:
 			 * instead of being done here under a special check
 			 * for SYS_ptrace().
 			 */
-	done:
-			/*
-			 * Check for misbehavior.
-			 */
-			WITNESS_WARN(WARN_PANIC, NULL, "System call %s returning",
-			    (code >=3D 0 && code < SYS_MAXSYSCALL) ?
-			    syscallnames[code] : "???");
-			KASSERT(td->td_critnest =3D=3D 0,
-			    ("System call %s returning in a critical section",
-			    (code >=3D 0 && code < SYS_MAXSYSCALL) ?
-			    syscallnames[code] : "???"));
-			KASSERT(td->td_locks =3D=3D 0,
-			    ("System call %s returning with %d locks held",
-			    (code >=3D 0 && code < SYS_MAXSYSCALL) ?
-			    syscallnames[code] : "???",
-			    td->td_locks));
-			userret(td, trapframe);
-#ifdef KTRACE
-			if (KTRPOINT(td, KTR_SYSRET))
-				ktrsysret(code, i, td->td_retval[0]);
-#endif
-			/*
-			 * This works because errno is findable through the
-			 * register set.  If we ever support an emulation
-			 * where this is not the case, this code will need
-			 * to be revisited.
-			 */
-			STOPEVENT(p, S_SCX, code);
-
-			PTRACESTOP_SC(p, td, S_PT_SCX);
-
-			mtx_assert(&Giant, MA_NOTOWNED);
+			syscallret(td, error, &sa);
 			return (trapframe->pc);
 		}
=20

--fmrJ4y+ZXhodCOcq
Content-Type: application/pgp-signature
Content-Disposition: inline

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.18 (FreeBSD)

iEYEARECAAYFAk6Mba0ACgkQC3+MBN1Mb4hNYACeLumBGWiQk5/k7pJnWJCLgigF
59kAoIsKXNJXc9l7l2zDzjv9EwInF6ip
=LX+r
-----END PGP SIGNATURE-----

--fmrJ4y+ZXhodCOcq--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20111005144605.GC1511>