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>