Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 18 Nov 2008 00:40:48 +0200
From:      Kostik Belousov <kostikbel@gmail.com>
To:        Peter Wemm <peter@wemm.org>
Cc:        Doug Ambrisko <ambrisko@freebsd.org>, src-committers@freebsd.org, Doug Ambrisko <ambrisko@ambrisko.com>, John Baldwin <jhb@freebsd.org>, svn-src-all@freebsd.org, dchagin@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r184974 - head/sys/dev/mfi
Message-ID:  <20081117224048.GN90129@deviant.kiev.zoral.com.ua>
In-Reply-To: <e7db6d980811171321n217adff0gc6213c2779a0f94@mail.gmail.com>
References:  <200811170237.mAH2bjY5088186@ambrisko.com> <200811171211.42740.jhb@freebsd.org> <20081117193541.GG90129@deviant.kiev.zoral.com.ua> <200811171613.36602.jhb@freebsd.org> <e7db6d980811171321n217adff0gc6213c2779a0f94@mail.gmail.com>

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

--rFUhhEVnhEf/dYhU
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Mon, Nov 17, 2008 at 01:21:53PM -0800, Peter Wemm wrote:
> On Mon, Nov 17, 2008 at 1:13 PM, John Baldwin <jhb@freebsd.org> wrote:
> > On Monday 17 November 2008 02:35:41 pm Kostik Belousov wrote:
> >> On Mon, Nov 17, 2008 at 12:11:41PM -0500, John Baldwin wrote:
> >> > On Sunday 16 November 2008 09:37:45 pm Doug Ambrisko wrote:
> >> > > Kostik Belousov writes:
> >> > > | On Fri, Nov 14, 2008 at 09:05:45PM +0000, Doug Ambrisko wrote:
> >> > > | > Author: ambrisko
> >> > > | > Date: Fri Nov 14 21:05:45 2008
> >> > > | > New Revision: 184974
> >> > > | > URL: http://svn.freebsd.org/changeset/base/184974
> >> > > | >
> >> > > | > Log:
> >> > > | >   When running a 32bit app. on amd64, ensure the bits above 32=
bit
> >> > > | >   are zero for the copyout.  Confirmed by LSI.
> >> > > | >
> >> > > | > Modified:
> >> > > | >   head/sys/dev/mfi/mfi.c
> >> > > | >
> >> > > | > Modified: head/sys/dev/mfi/mfi.c
> >> > > | >
> >> >
> > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D
> >> > > | > --- head/sys/dev/mfi/mfi.c    Fri Nov 14 18:09:19 2008        =
(r184973)
> >> > > | > +++ head/sys/dev/mfi/mfi.c    Fri Nov 14 21:05:45 2008        =
(r184974)
> >> > > | > @@ -2130,6 +2130,14 @@ mfi_ioctl(struct cdev *dev, u_long cmd,
> >> > > | >                           ->mfi_frame.raw[ioc->mfi_sense_off],
> >> > > | >                           &sense_ptr.sense_ptr_data[0],
> >> > > | >                           sizeof(sense_ptr.sense_ptr_data));
> >> > > | > +#ifdef __amd64__
> >> > > | > +                     if (cmd !=3D MFI_CMD) {
> >> > > | > +                             /*
> >> > > | > +                              * not 64bit native so zero out =
any address
> >> > > | > +                              * over 32bit */
> >> > > | > +                             sense_ptr.high =3D 0;
> >> > > | > +                     }
> >> > > | > +#endif
> >> > > | >                       error =3D copyout(cm->cm_sense, sense_pt=
r.user_space,
> >> > > | >                           ioc->mfi_sense_len);
> >> > > | >                       if (error !=3D 0) {
> >> > > | > @@ -2353,6 +2361,13 @@ mfi_linux_ioctl_int(struct cdev *dev, u_
> >> > > | >                              ->lioc_frame.raw[l_ioc.lioc_sense=
_off],
> >> > > | >                           &sense_ptr.sense_ptr_data[0],
> >> > > | >                           sizeof(sense_ptr.sense_ptr_data));
> >> > > | > +#ifdef __amd64__
> >> > > | > +                     /*
> >> > > | > +                      * only 32bit Linux support so zero out =
any
> >> > > | > +                      * address over 32bit
> >> > > | > +                      */
> >> > > | > +                     sense_ptr.high =3D 0;
> >> > > | > +#endif
> >> > > | >                       error =3D copyout(cm->cm_sense, sense_pt=
r.user_space,
> >> > > | >                           l_ioc.lioc_sense_len);
> >> > > | >                       if (error !=3D 0) {
> >> > > |
> >> > > | Would it make sense to perform this cut slightly more genericall=
y, by
> >> > > | checking whether the current process is 32bit ?
> >> > > |
> >> > > | We still have not grew the easy to check flag or attribute of the
> > image,
> >> > > | but usual practice is to compare p_sysent with corresponding sys=
vec,
> >> > > | like
> >> > > |         if (td->td_proc->p_sysent =3D=3D &ia32_freebsd_sysvec)
> >> > > | or
> >> > > |         if (td->td_proc->p_sysent =3D=3D &elf_linux_sysvec)
> >> > >
> >> > > So far we do it based on the ioctl since the 32bit or 64bit ioctl
> >> > > value is different.  I put in that comment for Linux since there
> >> > > is talk/work for Linux amd64 emulation.  For 64bit Linux ioctl
> >> > > support we need a 64bit structure defined.  When the ioctl can't
> >> > > be figured out then I've used the p_sysent.  Eventually, something
> >> > > that is more generic then the #ifdef __amd64__ should be done
> >> > > in all the drivers that do this emulation.
> >> >
> >> > I prefer depending on things like ioctl values and the 32-bit sysctl=
 flag
> > when
> >> > possible.  If we do have to directly check for the ABI, I'd much rat=
her
> > have
> >> > a flags field in sysent rather than trying to compare against global
> > symbols,
> >> > as you can't compare against a global symbol unless it is present in=
 the
> >> > kernel.  Something like:
> >> >
> >> >     if (td->td_proc->p_sysent->sy_flags & SY_32)
> >> >
> >> > or some such.  I've wanted to have a COMPAT_FREEBSD32 that gets
> > auto-enabled
> >> > when you turn on COMPAT_IA32 on amd64 and ia64.  It would also poten=
tially
> > be
> >> > enabled by a COMPAT_SPARC8 or some such on sparc64 if we ever had th=
at.
> >>
> >> Ok, what about the following. I only compiled it on i386/amd64. And,
> >> there are more places to convert to such checks, for sure.
> >>
> >> [yummy patch]
> >
> > Commit!  Note that this is not MFC'able due to ABI breakage for older l=
inux.ko
> > modules, but this is the proper solution for 8.0+.
I do not think that sysent KBI shall be preserved on the stable branch,
it is too core functionality. Our KBI guarantee mostly center around
drivers and less so for filesystem modules.

linux or any other ABI emulator probably do not deserve KBI stability
guarantee, IMHO.

Anyway, I do not insist.

> >
> > --
> > John Baldwin
> >
>=20
>=20
> I was thinking of suggesting a macro to replace the verbose test if
> curproc->td_proc->p_sysent->sv_flags, but I couldn't think of
> something off the top of my head.
>=20
> -                      if (curthread->td_proc->p_sysent =3D=3D
> &ia32_freebsd_sysvec) {
> +                       if (curthread->td_proc->p_sysent->sv_flags & SV_I=
LP32) {
>=20
> if (SV_FLAGS(curthread) & SV_ILP32) ... or the like.  I'm not set on
> this, it just seemed like it might be worth mentioning.
>=20
> Also, change curthread->td_proc with curproc, for what its worth.
Did it.

I discussed the change with Dmitry Chagin, and he wants explicit mark
of the image ABI, for linux64/amd64 work.

diff --git a/sys/amd64/amd64/elf_machdep.c b/sys/amd64/amd64/elf_machdep.c
index ec1afc7..4f6d178 100644
--- a/sys/amd64/amd64/elf_machdep.c
+++ b/sys/amd64/amd64/elf_machdep.c
@@ -72,7 +72,8 @@ struct sysentvec elf64_freebsd_sysvec =3D {
 	.sv_copyout_strings	=3D exec_copyout_strings,
 	.sv_setregs	=3D exec_setregs,
 	.sv_fixlimit	=3D NULL,
-	.sv_maxssiz	=3D NULL
+	.sv_maxssiz	=3D NULL,
+	.sv_flags	=3D SV_ABI_FREEBSD | SV_LP64
 };
=20
 static Elf64_Brandinfo freebsd_brand_info =3D {
diff --git a/sys/amd64/linux32/linux32_sysvec.c b/sys/amd64/linux32/linux32=
_sysvec.c
index e233700..3acee30 100644
--- a/sys/amd64/linux32/linux32_sysvec.c
+++ b/sys/amd64/linux32/linux32_sysvec.c
@@ -1026,6 +1026,7 @@ struct sysentvec elf_linux_sysvec =3D {
 	.sv_setregs	=3D exec_linux_setregs,
 	.sv_fixlimit	=3D linux32_fixlimit,
 	.sv_maxssiz	=3D &linux32_maxssiz,
+	.sv_flags	=3D SV_ABI_LINUX | SV_ILP32 | SV_IA32
 };
=20
 static Elf32_Brandinfo linux_brand =3D {
diff --git a/sys/arm/arm/elf_machdep.c b/sys/arm/arm/elf_machdep.c
index f44a622..693eab1 100644
--- a/sys/arm/arm/elf_machdep.c
+++ b/sys/arm/arm/elf_machdep.c
@@ -72,7 +72,8 @@ struct sysentvec elf32_freebsd_sysvec =3D {
 	.sv_copyout_strings =3D exec_copyout_strings,
 	.sv_setregs	=3D exec_setregs,
 	.sv_fixlimit	=3D NULL,
-	.sv_maxssiz	=3D NULL
+	.sv_maxssiz	=3D NULL,
+	.sv_flags	=3D SV_ABI_FREEBSD | SV_ILP32
 };
=20
 static Elf32_Brandinfo freebsd_brand_info =3D {
diff --git a/sys/compat/ia32/ia32_sysvec.c b/sys/compat/ia32/ia32_sysvec.c
index ef74ba0..0b32b9a 100644
--- a/sys/compat/ia32/ia32_sysvec.c
+++ b/sys/compat/ia32/ia32_sysvec.c
@@ -135,7 +135,8 @@ struct sysentvec ia32_freebsd_sysvec =3D {
 	.sv_copyout_strings	=3D ia32_copyout_strings,
 	.sv_setregs	=3D ia32_setregs,
 	.sv_fixlimit	=3D ia32_fixlimit,
-	.sv_maxssiz	=3D &ia32_maxssiz
+	.sv_maxssiz	=3D &ia32_maxssiz,
+	.sv_flags	=3D SV_ABI_FREEBSD | SV_IA32 | SV_ILP32
 };
=20
=20
diff --git a/sys/compat/svr4/svr4_sysvec.c b/sys/compat/svr4/svr4_sysvec.c
index 60cca7b..63e8e54 100644
--- a/sys/compat/svr4/svr4_sysvec.c
+++ b/sys/compat/svr4/svr4_sysvec.c
@@ -190,7 +190,8 @@ struct sysentvec svr4_sysvec =3D {
 	.sv_copyout_strings =3D exec_copyout_strings,
 	.sv_setregs	=3D exec_setregs,
 	.sv_fixlimit	=3D NULL,
-	.sv_maxssiz     =3D NULL
+	.sv_maxssiz     =3D NULL,
+	.sv_flags	=3D SV_ABI_UNDEF | SV_IA32 | SV_ILP32
 };
=20
 const char      svr4_emul_path[] =3D "/compat/svr4";
diff --git a/sys/i386/i386/elf_machdep.c b/sys/i386/i386/elf_machdep.c
index 93f1d45..19eddd0 100644
--- a/sys/i386/i386/elf_machdep.c
+++ b/sys/i386/i386/elf_machdep.c
@@ -72,7 +72,8 @@ struct sysentvec elf32_freebsd_sysvec =3D {
 	.sv_copyout_strings	=3D exec_copyout_strings,
 	.sv_setregs	=3D exec_setregs,
 	.sv_fixlimit	=3D NULL,
-	.sv_maxssiz	=3D NULL
+	.sv_maxssiz	=3D NULL,
+	.sv_flags	=3D SV_ABI_FREEBSD | SV_IA32 | SV_ILP32
 };
=20
 static Elf32_Brandinfo freebsd_brand_info =3D {
diff --git a/sys/i386/ibcs2/ibcs2_sysvec.c b/sys/i386/ibcs2/ibcs2_sysvec.c
index 2c834dd..9112ed7 100644
--- a/sys/i386/ibcs2/ibcs2_sysvec.c
+++ b/sys/i386/ibcs2/ibcs2_sysvec.c
@@ -85,7 +85,8 @@ struct sysentvec ibcs2_svr3_sysvec =3D {
 	.sv_copyout_strings =3D exec_copyout_strings,
 	.sv_setregs	=3D exec_setregs,
 	.sv_fixlimit	=3D NULL,
-	.sv_maxssiz	=3D NULL
+	.sv_maxssiz	=3D NULL,
+	.sv_flags	=3D SV_ABI_UNDEF | SV_IA32 | SV_ILP32
 };
=20
 static int
diff --git a/sys/i386/linux/linux_sysvec.c b/sys/i386/linux/linux_sysvec.c
index a3acfc9..7444901 100644
--- a/sys/i386/linux/linux_sysvec.c
+++ b/sys/i386/linux/linux_sysvec.c
@@ -837,7 +837,8 @@ struct sysentvec linux_sysvec =3D {
 	.sv_copyout_strings =3D exec_copyout_strings,
 	.sv_setregs	=3D exec_linux_setregs,
 	.sv_fixlimit	=3D NULL,
-	.sv_maxssiz	=3D NULL
+	.sv_maxssiz	=3D NULL,
+	.sv_flags	=3D SV_ABI_LINUX | SV_AOUT | SV_IA32 | SV_ILP32
 };
=20
 struct sysentvec elf_linux_sysvec =3D {
@@ -867,7 +868,8 @@ struct sysentvec elf_linux_sysvec =3D {
 	.sv_copyout_strings =3D exec_copyout_strings,
 	.sv_setregs	=3D exec_linux_setregs,
 	.sv_fixlimit	=3D NULL,
-	.sv_maxssiz	=3D NULL
+	.sv_maxssiz	=3D NULL,
+	.sv_flags	=3D SV_ABI_LINUX | SV_IA32 | SV_ILP32
 };
=20
 static Elf32_Brandinfo linux_brand =3D {
diff --git a/sys/ia64/ia64/elf_machdep.c b/sys/ia64/ia64/elf_machdep.c
index 94f4cdc..a3a6e57 100644
--- a/sys/ia64/ia64/elf_machdep.c
+++ b/sys/ia64/ia64/elf_machdep.c
@@ -80,7 +80,8 @@ struct sysentvec elf64_freebsd_sysvec =3D {
 	.sv_copyout_strings =3D exec_copyout_strings,
 	.sv_setregs	=3D exec_setregs,
 	.sv_fixlimit	=3D NULL,
-	.sv_maxssiz	=3D NULL
+	.sv_maxssiz	=3D NULL,
+	.sv_flags	=3D SV_ABI_FREEBSD | SV_LP64
 };
=20
 static Elf64_Brandinfo freebsd_brand_info =3D {
diff --git a/sys/kern/imgact_aout.c b/sys/kern/imgact_aout.c
index f4e4614..6c2f627 100644
--- a/sys/kern/imgact_aout.c
+++ b/sys/kern/imgact_aout.c
@@ -82,7 +82,13 @@ struct sysentvec aout_sysvec =3D {
 	.sv_copyout_strings	=3D exec_copyout_strings,
 	.sv_setregs	=3D exec_setregs,
 	.sv_fixlimit	=3D NULL,
-	.sv_maxssiz	=3D NULL
+	.sv_maxssiz	=3D NULL,
+	.sv_flags	=3D SV_ABI_FREEBSD | SV_AOUT |
+#if defined(__i386__)
+	SV_IA32 | SV_ILP32
+#else
+#error Choose SV_XXX flags for the platform
+#endif
 };
=20
 static int
diff --git a/sys/kern/kern_thr.c b/sys/kern/kern_thr.c
index dade1c2..3802259 100644
--- a/sys/kern/kern_thr.c
+++ b/sys/kern/kern_thr.c
@@ -57,14 +57,12 @@ __FBSDID("$FreeBSD$");
=20
 #ifdef COMPAT_IA32
=20
-extern struct sysentvec ia32_freebsd_sysvec;
-
 static inline int
 suword_lwpid(void *addr, lwpid_t lwpid)
 {
 	int error;
=20
-	if (curproc->p_sysent !=3D &ia32_freebsd_sysvec)
+	if (SV_CURPROC_FLAG(SV_LP64))
 		error =3D suword(addr, lwpid);
 	else
 		error =3D suword32(addr, lwpid);
diff --git a/sys/kern/uipc_socket.c b/sys/kern/uipc_socket.c
index eb587fb..9a7237d 100644
--- a/sys/kern/uipc_socket.c
+++ b/sys/kern/uipc_socket.c
@@ -136,9 +136,8 @@ __FBSDID("$FreeBSD$");
=20
 #ifdef COMPAT_IA32
 #include <sys/mount.h>
+#include <sys/sysent.h>
 #include <compat/freebsd32/freebsd32.h>
-
-extern struct sysentvec ia32_freebsd_sysvec;
 #endif
=20
 static int	soreceive_rcvoob(struct socket *so, struct uio *uio,
@@ -2277,7 +2276,7 @@ sosetopt(struct socket *so, struct sockopt *sopt)
 		case SO_SNDTIMEO:
 		case SO_RCVTIMEO:
 #ifdef COMPAT_IA32
-			if (curthread->td_proc->p_sysent =3D=3D &ia32_freebsd_sysvec) {
+			if (SV_CURPROC_FLAG(SV_ILP32)) {
 				struct timeval32 tv32;
=20
 				error =3D sooptcopyin(sopt, &tv32, sizeof tv32,
@@ -2458,7 +2457,7 @@ integer:
 			tv.tv_sec =3D optval / hz;
 			tv.tv_usec =3D (optval % hz) * tick;
 #ifdef COMPAT_IA32
-			if (curthread->td_proc->p_sysent =3D=3D &ia32_freebsd_sysvec) {
+			if (SV_CURPROC_FLAG(SV_ILP32)) {
 				struct timeval32 tv32;
=20
 				CP(tv, tv32, tv_sec);
diff --git a/sys/mips/mips/elf_machdep.c b/sys/mips/mips/elf_machdep.c
index 0234722..dc08bc2 100644
--- a/sys/mips/mips/elf_machdep.c
+++ b/sys/mips/mips/elf_machdep.c
@@ -74,7 +74,8 @@ struct sysentvec elf32_freebsd_sysvec =3D {
 	.sv_copyout_strings =3D exec_copyout_strings,
 	.sv_setregs	=3D exec_setregs,
 	.sv_fixlimit	=3D NULL,
-	.sv_maxssiz	=3D NULL
+	.sv_maxssiz	=3D NULL,
+	.sv_flags	=3D SV_ABI_FREEBSD | SV_ILP32
 };
=20
 static Elf32_Brandinfo freebsd_brand_info =3D {
diff --git a/sys/powerpc/powerpc/elf_machdep.c b/sys/powerpc/powerpc/elf_ma=
chdep.c
index dadf3ca..69ac55b 100644
--- a/sys/powerpc/powerpc/elf_machdep.c
+++ b/sys/powerpc/powerpc/elf_machdep.c
@@ -75,7 +75,8 @@ struct sysentvec elf32_freebsd_sysvec =3D {
 	.sv_copyout_strings =3D exec_copyout_strings,
 	.sv_setregs	=3D exec_setregs,
 	.sv_fixlimit	=3D NULL,
-	.sv_maxssiz	=3D NULL
+	.sv_maxssiz	=3D NULL,
+	.sv_flags	=3D SV_ABI_FREEBSD | SV_ILP32
 };
=20
 static Elf32_Brandinfo freebsd_brand_info =3D {
diff --git a/sys/sparc64/sparc64/elf_machdep.c b/sys/sparc64/sparc64/elf_ma=
chdep.c
index d1e610a..a956c5c 100644
--- a/sys/sparc64/sparc64/elf_machdep.c
+++ b/sys/sparc64/sparc64/elf_machdep.c
@@ -87,7 +87,8 @@ static struct sysentvec elf64_freebsd_sysvec =3D {
 	.sv_copyout_strings =3D exec_copyout_strings,
 	.sv_setregs	=3D exec_setregs,
 	.sv_fixlimit	=3D NULL,
-	.sv_maxssiz	=3D NULL
+	.sv_maxssiz	=3D NULL,
+	.sv_flags	=3D SV_ABI_FREEBSD | SV_LP64
 };
=20
 static Elf64_Brandinfo freebsd_brand_info =3D {
diff --git a/sys/sys/sysent.h b/sys/sys/sysent.h
index 0ec07a7..c068946 100644
--- a/sys/sys/sysent.h
+++ b/sys/sys/sysent.h
@@ -100,8 +100,22 @@ struct sysentvec {
 	void		(*sv_setregs)(struct thread *, u_long, u_long, u_long);
 	void		(*sv_fixlimit)(struct rlimit *, int);
 	u_long		*sv_maxssiz;
+	u_int		sv_flags;
 };
=20
+#define	SV_ILP32	0x000100
+#define	SV_LP64		0x000200
+#define	SV_IA32		0x004000
+#define	SV_AOUT		0x008000
+
+#define	SV_ABI_MASK	0xff
+#define	SV_CURPROC_FLAG(x) (curproc->p_sysent->sv_flags & (x))
+#define	SV_CURPROC_ABI() (curproc->p_sysent->sv_flags & SV_ABI_MASK)
+/* same as ELFOSABI_XXX, to prevent header pollution */
+#define	SV_ABI_LINUX	3
+#define	SV_ABI_FREEBSD 	9
+#define	SV_ABI_UNDEF	255
+
 #ifdef _KERNEL
 extern struct sysentvec aout_sysvec;
 extern struct sysentvec elf_freebsd_sysvec;

--rFUhhEVnhEf/dYhU
Content-Type: application/pgp-signature
Content-Disposition: inline

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (FreeBSD)

iEYEARECAAYFAkkh8u8ACgkQC3+MBN1Mb4iMDQCgudEVnKVU+JYHmOXCbQPwdHOK
QikAniwxGFAfMbmQjf5iKkVsAfOabta/
=8iLC
-----END PGP SIGNATURE-----

--rFUhhEVnhEf/dYhU--



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