svn commit: r184974 - head/sys/dev/mfi
John Baldwin
jhb at freebsd.org
Mon Nov 17 15:04:29 PST 2008
On Monday 17 November 2008 05:40:48 pm Kostik Belousov wrote:
> 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 at 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
32bit
> > >> > > | > are zero for the copyout. Confirmed by LSI.
> > >> > > | >
> > >> > > | > Modified:
> > >> > > | > head/sys/dev/mfi/mfi.c
> > >> > > | >
> > >> > > | > Modified: head/sys/dev/mfi/mfi.c
> > >> > > | >
> > >> >
> > >
==============================================================================
> > >> > > | > --- 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 != MFI_CMD) {
> > >> > > | > + /*
> > >> > > | > + * not 64bit native so zero out
any address
> > >> > > | > + * over 32bit */
> > >> > > | > + sense_ptr.high = 0;
> > >> > > | > + }
> > >> > > | > +#endif
> > >> > > | > error = copyout(cm->cm_sense,
sense_ptr.user_space,
> > >> > > | > ioc->mfi_sense_len);
> > >> > > | > if (error != 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 = 0;
> > >> > > | > +#endif
> > >> > > | > error = copyout(cm->cm_sense,
sense_ptr.user_space,
> > >> > > | > l_ioc.lioc_sense_len);
> > >> > > | > if (error != 0) {
> > >> > > |
> > >> > > | Would it make sense to perform this cut slightly more
generically, 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
sysvec,
> > >> > > | like
> > >> > > | if (td->td_proc->p_sysent == &ia32_freebsd_sysvec)
> > >> > > | or
> > >> > > | if (td->td_proc->p_sysent == &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
rather
> > > 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
potentially
> > > be
> > >> > enabled by a COMPAT_SPARC8 or some such on sparc64 if we ever had
that.
> > >>
> > >> 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
linux.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.
Well, I didn't MFC the .sv_maxssiz fix to 6.x due to ABI concerns.
--
John Baldwin
More information about the svn-src-head
mailing list