Date: Mon, 21 Jul 2014 11:16:37 -0600 From: Warner Losh <imp@bsdimp.com> To: John-Mark Gurney <jmg@funkthat.com> Cc: Tim Kientzle <tim@kientzle.com>, freebsd-arm <freebsd-arm@freebsd.org>, Fabien Thomas <fabient@freebsd.org>, Ian Lepore <ian@freebsd.org>, arch@freebsd.org Subject: Re: [CFR] mge driver / elf reloc Message-ID: <B23A4513-FB0E-4972-91BA-976DCDF346E5@bsdimp.com> In-Reply-To: <20140721165619.GT45513@funkthat.com> References: <14D22EA6-B73C-47BA-9A86-A957D24F23B8@freebsd.org> <1405810447.85788.41.camel@revolution.hippie.lan> <20140720220514.GP45513@funkthat.com> <F6D53A17-FED0-4F08-BB5B-9F66C5AF5EF6@kientzle.com> <20140720231056.GQ45513@funkthat.com> <9464C309-B390-4A27-981A-E854921B1C98@bsdimp.com> <20140721162559.GS45513@funkthat.com> <467619B1-F530-49AF-91BF-14CA3A31908B@bsdimp.com> <20140721165619.GT45513@funkthat.com>
next in thread | previous in thread | raw e-mail | index | archive | help
--Apple-Mail=_0BBD206C-C6E0-434F-B718-ABB5CFD259E3 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=windows-1252 On Jul 21, 2014, at 10:56 AM, John-Mark Gurney <jmg@funkthat.com> wrote: > Warner Losh wrote this message on Mon, Jul 21, 2014 at 10:31 -0600: >>=20 >> On Jul 21, 2014, at 10:25 AM, John-Mark Gurney <jmg@funkthat.com> = wrote: >>=20 >>> Warner Losh wrote this message on Mon, Jul 21, 2014 at 08:46 -0600: >>>>=20 >>>> On Jul 20, 2014, at 5:10 PM, John-Mark Gurney <jmg@funkthat.com> = wrote: >>>>=20 >>>>> Tim Kientzle wrote this message on Sun, Jul 20, 2014 at 15:25 = -0700: >>>>>>=20 >>>>>> On Jul 20, 2014, at 3:05 PM, John-Mark Gurney <jmg@funkthat.com> = wrote: >>>>>>=20 >>>>>>> Ian Lepore wrote this message on Sat, Jul 19, 2014 at 16:54 = -0600: >>>>>>>> Sorry to take so long to reply to this, I'm trying to get = caught up. I >>>>>>>> see you've already committed the mge fixes. I think the ELF = alignment >>>>>>>> fix looks good and should also be committed. >>>>>>>=20 >>>>>>> So, re the elf alignment... >>>>>>>=20 >>>>>>> I think we should get a set of macros that handle load/stores = to/from >>>>>>> unaligned addresses that are transparent to the caller.... I = need >>>>>>> these for some other code I'm writing...=20 >>>>>>>=20 >>>>>>> I thought Open/Net had these available, but I can't seem to find = them >>>>>>> right now... >>>>>>=20 >>>>>> $ man 9 byteorder >>>>>>=20 >>>>>> is most of what you want, lacking only some aliases to pick >>>>>> the correct macro for native byte order. >>>>>=20 >>>>> Um, those doesn't help if you want native endian order? >>>>=20 >>>> Ummm, yes they do. enc converts from native order. dec decodes to = native byte >>>=20 >>> No they don't.. If you want to read a value in memory that is native >>> endian order to native endian order (no conversion), they cannot be >>> used w/o using something like below? >>=20 >> Missed the native to native. bcopy works, but is ugly, as you note = below. >>=20 >>>> order. They are more general cases than the ntoh* functions that = are more traditional >>>> since they also work on byte streams that may not be completely = aligned when >>>> sitting in memory. Which is what you are asking for. >>>=20 >>> So, you're saying that I now need to write code like: >>> #if LITTLE_ENDIAN /* or how ever this is spelled*/ >>> var =3D le32enc(foo); >>> #else >>> var =3D be32enc(foo); >>> #endif >>>=20 >>> If I want to read a arch native endian value? No thank you? >>=20 >> I?m not saying that at all. >>=20 >>>>> Also, only the enc/dec functions are documented to work on = non-aligned >>>>> address, so that doesn't help in most cases? >>>>=20 >>>> They work on all addresses. They are even documented to work on any = address: >>>>=20 >>>> The be16enc(), be16dec(), be32enc(), be32dec(), be64enc(), = be64dec(), >>>> le16enc(), le16dec(), le32enc(), le32dec(), le64enc(), and = le64dec() >>>> functions encode and decode integers to/from byte strings on any = align- >>>> ment in big/little endian format. >>>>=20 >>>> So they are quite useful in general. Peeking under the covers at = the implementation >>>> also shows they will work for any alignment, so I?m having trouble = understanding >>>> where this objection is really coming from. >>>=20 >>> There are places where you write code such as: >>> int i; >>> memcpy(&i, inp, sizeof i); >>> /* use i */ >>>=20 >>> In order to avoid alignment faults... None of the functions in = byteorder >>> do NO conversion of endian, or you have to know which endian you are = but >>> that doesn't work on MI code... >>>=20 >>> Did you read what the commited code did? >>=20 >> No, I missed that bit beaded on your reply (which seemed to imply you = needed >> endian conversion) which implied the enc/dec are only documented to = work on non-aligned >> which is what I was correcting. >=20 > Hmm... It appears that byteorder(9) leaks to userland though isn't > documented to be available in userland... Should we fix this and make > it an offical userland API? How to document it? In my case, the > enc/dec version would have been enough for what I need, but not "part = of > the userland API"... There is an xref from byteorder(3), but no > comment on either that they will work.. Yes. We should fix this now that it isn=92t confined to the kernel. >> But maybe the more basic question is why do you even have packed >> structures that are native endian that you want to access as = naturally >> aligned structures? How did they become native endian and why weren?t >> they converted to a more natural layout at that time? >=20 > The original message said why=85 I personally think the original code should unconditionally call = load_ptr() and store_ptr() and if the optimization for aligned access is actually worth doing, the = test for it should be in those functions rather than inline throughout the code. The code will = be clearer, and it would be easier to optimize those cases that actually matter. I=92m frankly surprised that these relocations are being generated = unaligned. Perhaps that=92s the real bug here that should be fixed. While I=92m OK with the original = patch (subject to the above), I=92d be curious what other cases there are for this = functionality. You had said that you had additional use cases in the network stack, but I=92m having = trouble groking the use cases. If this is a huge deal, then defining functions to do this is trivial. = I=92m just not sure it is common enough to need a special macro/function call in the base. Warner --Apple-Mail=_0BBD206C-C6E0-434F-B718-ABB5CFD259E3 Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename=signature.asc Content-Type: application/pgp-signature; name=signature.asc Content-Description: Message signed with OpenPGP using GPGMail -----BEGIN PGP SIGNATURE----- Comment: GPGTools - https://gpgtools.org iQIcBAEBCgAGBQJTzUr1AAoJEGwc0Sh9sBEAl6MP+QGfnLxyKTIOrOuCgJ8ps7dW SewJMNsTNebtICWMUBe+SWZ7NDnKEr6M7T61o9wpdckc3n7K2XtrPrppiUHmDF79 Y+nXzJWAMAsDxyJ9Y8axXupV1WsA0xqQ4+VnHWx5wLCAjVkfejw3O5CqF9ljMVr2 jnyge07Kdrfd31Jz/WguuHuBp9E32wv4UCvYWUweRDdwKUmMW2oLMtdbo1beD3pH RLV4lKDMrjpoaiTc9VkaRBT97e4AJFHZQYvUqfb6T0PGQQb3I3cCAir96WbOwpBP NQR/kTg4zRdwsjSos99bwyEkLu1k9JHduEdCMpfB+Y7itff1aybnqFMZzRlNT9X1 ILYpOvvQBlI9QtjbtiEQV/ABGQtLr+R1yMHYqMk5SafAvrR3yQ48FpYFIimnGd2d IIoDZuJxt/yTiM/sXsHhxSDx3qWyfSspcYwuGwdRWiMeG+XDrXBoeoiC48eGMuwG 9jbb3FLNa8cZ3eMJlQn2GEIBBK9qttVBbZxLWK09V/e+fqL0htUzA8XadLrCZUSR uUYoZsgGefPDfty/H51PPSCiOSYABdBkYuUuUZzAsWTp8SXbD7apbyK1TztgkJ6I 7mLv7R0s7I1gWH2qqmn9dwDuEW8Auhku85xTc0XMm06NyQ7RqRcx4f4mX0fxRmOI jpMqP2vyWDtX4xNOsfUD =QDdl -----END PGP SIGNATURE----- --Apple-Mail=_0BBD206C-C6E0-434F-B718-ABB5CFD259E3--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?B23A4513-FB0E-4972-91BA-976DCDF346E5>