From owner-svn-src-head@FreeBSD.ORG Fri Mar 27 06:06:50 2009 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 06DC21065672; Fri, 27 Mar 2009 06:06:50 +0000 (UTC) (envelope-from dchagin@dchagin.static.corbina.ru) Received: from contrabass.post.ru (contrabass.post.ru [85.21.78.5]) by mx1.freebsd.org (Postfix) with ESMTP id 8E4B88FC15; Fri, 27 Mar 2009 06:06:48 +0000 (UTC) (envelope-from dchagin@dchagin.static.corbina.ru) Received: from corbina.ru (mail.post.ru [195.14.50.16]) by contrabass.post.ru (Postfix) with ESMTP id 4237777F70; Fri, 27 Mar 2009 09:06:47 +0300 (MSK) X-Virus-Scanned: by cgpav Uf39PSi9pFi9oFi9 Received: from [10.208.17.3] (HELO dchagin.static.corbina.ru) by corbina.ru (CommuniGate Pro SMTP 5.1.14) with ESMTPS id 1703542079; Fri, 27 Mar 2009 09:06:47 +0300 Received: from dchagin.static.corbina.ru (localhost.chd.net [127.0.0.1]) by dchagin.static.corbina.ru (8.14.3/8.14.3) with ESMTP id n2R66kHR002053; Fri, 27 Mar 2009 09:06:46 +0300 (MSK) (envelope-from dchagin@dchagin.static.corbina.ru) Received: (from dchagin@localhost) by dchagin.static.corbina.ru (8.14.3/8.14.3/Submit) id n2R66h3a002052; Fri, 27 Mar 2009 09:06:43 +0300 (MSK) (envelope-from dchagin) Date: Fri, 27 Mar 2009 09:06:43 +0300 From: Chagin Dmitry To: Doug Ambrisko Message-ID: <20090327060643.GA1937@dchagin.static.corbina.ru> References: <200903262209.n2QM9NdZ078655@ambrisko.com> <200903270008.n2R08xBg085980@ambrisko.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="AqsLC8rIMeq19msA" Content-Disposition: inline In-Reply-To: <200903270008.n2R08xBg085980@ambrisko.com> User-Agent: Mutt/1.5.19 (2009-01-05) Cc: Doug Ambrisko , src-committers@freebsd.org, John Baldwin , Roman Divacky , svn-src-head@freebsd.org, svn-src-all@freebsd.org Subject: Re: svn commit: r190445 - in head/sys: amd64/linux32 compat/linprocfs compat/linux conf dev/ipmi modules/ipmi modules/linprocfs X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 27 Mar 2009 06:06:50 -0000 --AqsLC8rIMeq19msA Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Mar 26, 2009 at 05:08:59PM -0700, Doug Ambrisko wrote: > Doug Ambrisko writes: > | John Baldwin writes: > | | On Thursday 26 March 2009 5:29:42 pm Doug Ambrisko wrote: > | [snip] > | | > Maybe you have another suggestion to fix this. The problem showed = up > | | > when doing a mmap of 0xcf79c000 into 0xffffffffcf79c000 also a mmap > | | > of 0xf0000 ended up the same way. This caused it to fail. Note th= is > | | > is only on amd64 with a Linux. It didn't happen with a FreeBSD i386 > | | > version on amd64. Here is a sample test program: > | |=20 > | | I'm sure this can be easily fixed in the Linux mmap() handlers instea= d. Do=20 > | | you know if your Linux binary is using mmap2() or the old mmap()? > |=20 > | I think it uses linux_mmap then bouncing it to linux_mmap_common. > | linux_mmap_common had it right but when it mmap picked it up then=20 > | it was wrong in my intrumentation.=20 > |=20 > | I'll flip the l_off_t type back and then instrument it more to find > | out when things are going bad. I missed the other usage of l_off_t > | so I agree this is a bad change. However, I wonder if the other > | usage of l_off_t actually works right or there is a bug with that > | as well? > |=20 > | I should be able to get something put together pretty quick and > | send it for review. >=20 > Okay, I did some more instrumenting again and found that I was=20 > slightly wrong. The mmap that was failing was 0xcf79c000 and not > 0xf0000. This probably makes since since the sign bit was set > on 0xcf79c000. However, it appear mmap doesn't really do negative > seeks. Looking at the freebsd32_mmap the structure it uses for > args is: > struct freebsd6_freebsd32_mmap_args { > char addr_l_[PADL_(caddr_t)]; caddr_t addr; char addr_r_[PADR_(ca= ddr_t)]; > char len_l_[PADL_(size_t)]; size_t len; char len_r_[PADR_(size_t)= ]; > char prot_l_[PADL_(int)]; int prot; char prot_r_[PADR_(int)]; > char flags_l_[PADL_(int)]; int flags; char flags_r_[PADR_(int)]; > char fd_l_[PADL_(int)]; int fd; char fd_r_[PADR_(int)]; > char pad_l_[PADL_(int)]; int pad; char pad_r_[PADR_(int)]; > char poslo_l_[PADL_(u_int32_t)]; u_int32_t poslo; char poslo_r_[P= ADR_(u_int32_t)]; > char poshi_l_[PADL_(u_int32_t)]; u_int32_t poshi; char poshi_r_[P= ADR_(u_int32_t)]; > }; > with both the high and the lows being u_int32_t. >=20 > So I wonder if in the linux32 the structure that is: > struct l_mmap_argv { > l_uintptr_t addr; > l_size_t len; > l_int prot; > l_int flags; > l_int fd; > l_off_t pgoff; > } __packed; > should be uint32_t for pgoff? >=20 yes, you are right. s/uint32_t/l_ulong/ :) also remove __packed. thnx! > Using this patch things work okay: >=20 > Index: linux.h > =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 > RCS file: /usr/local/cvsroot/freebsd/src/sys/amd64/linux32/linux.h,v > retrieving revision 1.24 > diff -u -p -r1.24 linux.h > --- linux.h 26 Mar 2009 17:14:22 -0000 1.24 > +++ linux.h 27 Mar 2009 00:01:07 -0000 > @@ -79,7 +79,7 @@ typedef l_ulong l_ino_t; > typedef l_int l_key_t; > typedef l_longlong l_loff_t; > typedef l_ushort l_mode_t; > -typedef l_ulong l_off_t; > +typedef l_long l_off_t; > typedef l_int l_pid_t; > typedef l_uint l_size_t; > typedef l_long l_suseconds_t; > Index: linux32_machdep.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 > RCS file: /usr/local/cvsroot/freebsd/src/sys/amd64/linux32/linux32_machde= p.c,v > retrieving revision 1.52 > diff -u -p -r1.52 linux32_machdep.c > --- linux32_machdep.c 18 Feb 2009 16:11:39 -0000 1.52 > +++ linux32_machdep.c 27 Mar 2009 00:01:07 -0000 > @@ -788,6 +788,7 @@ linux_mmap(struct thread *td, struct lin > { > int error; > struct l_mmap_argv linux_args; > + uint32_t pos; > =20 > error =3D copyin(args->ptr, &linux_args, sizeof(linux_args)); > if (error) > @@ -801,7 +802,10 @@ linux_mmap(struct thread *td, struct lin > #endif > if ((linux_args.pgoff % PAGE_SIZE) !=3D 0) > return (EINVAL); > - linux_args.pgoff /=3D PAGE_SIZE; > + pos =3D linux_args.pgoff; > + pos /=3D PAGE_SIZE; > + linux_args.pgoff =3D pos; > +=09 > =20 > return (linux_mmap_common(td, &linux_args)); > } >=20 >=20 > So which should we do? The uint32_t for the /=3D PAGE_SIZE or in > the mmap structure? FWIW, they are mmaping /dev/mem and grabbing > the SMBIOS structure put at 0xcf79c000 and 0xcf7f0000 which are not > negative offsets. linux_mmap2 and linux_common don't really have > this problem in this case since they are using the memory address=20 > / PAGE_SIZE. So they don't run into this sign problem like this. > I've confirmed that that above patch makes the Linux BMC firmware > upgrade tool works. On a real Linux machine it also mmaps these > addresses and it works there otherwise the program goes into the > weeds since it can't find the IPMI controller. This change only > mucks with linux_mmap. >=20 > Thanks, >=20 > Doug A. --=20 Have fun! chd --AqsLC8rIMeq19msA Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.10 (FreeBSD) iEUEARECAAYFAknMbPEACgkQ0t2Tb3OO/O3PcQCdGYWbLBrPymis+DftwtGhoCsi 3C8Al2dkhYt+EPOksKnAjicQY5i+EJw= =ENzP -----END PGP SIGNATURE----- --AqsLC8rIMeq19msA--