Date: Fri, 12 Oct 2012 10:46:03 -0700 From: Carl Delsey <carl.r.delsey@intel.com> To: Robert Watson <rwatson@FreeBSD.org> Cc: freebsd-hackers@freebsd.org Subject: Re: No bus_space_read_8 on x86 ? Message-ID: <5078575B.2020808@intel.com> In-Reply-To: <alpine.BSF.2.00.1210121700220.62497@fledge.watson.org> References: <506DC574.9010300@intel.com> <201210091154.15873.jhb@freebsd.org> <5075EC29.1010907@intel.com> <201210121131.46373.jhb@freebsd.org> <alpine.BSF.2.00.1210121700220.62497@fledge.watson.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 10/12/2012 9:04 AM, Robert Watson wrote: > > On Fri, 12 Oct 2012, John Baldwin wrote: > >>>>> I believe it was because bus reads weren't guaranteed to be atomic >>>>> on i386. don't know if that's still the case or a concern, but it >>>>> was an intentional omission. >>>> True. If you are on a 32-bit system you can read the two 4 byte >>>> values and then build a 64-bit value. For 64-bit platforms we >>>> should offer bus_read_8() however. >>> >>> I believe there is still no way to perform a 64-bit read on a i386 >>> (or at least without messing with SSE instructions), but if you have >>> to read a 64-bit register, you are stuck with doing two 32-bit reads >>> and concatenating them. I figure we may as well provide an >>> implementation for those who have to do that as well as the >>> implementation for 64-bit. >> >> I think the problem though is that the way you should glue those two >> 32-bit reads together is device dependent. I don't think you can >> provide a completely device-neutral bus_read_8() on i386. We should >> certainly have it on 64-bit platforms, but I think drivers that want >> to work on 32-bit platforms need to explicitly merge the two words >> themselves. > > Indeed -- and on non-x86, where there are uncached direct map > segments, and TLB entries that disable caching, reading 2x 32-bit vs > 1x 64-bit have quite different effects in terms of atomicity. Where > uncached I/Os are being used, those differences may affect semantics > significantly -- e.g., if your device has a 64-bit memory-mapped FIFO > or registers, 2x 32-bit gives you two halves of two different 64-bit > values, rather than two halves of the same value. As device drivers > depend on those atomicity semantics, we should (at the busspace level) > offer only the exactly expected semantics, rather than trying to patch > things up. If a device driver accessing 64-bit fields wants to > support doing it using two 32-bit reads, it can figure out how to > splice it together following bus_space_read_region_4(). I wouldn't make any default behaviour for bus_space_read_8 on i386, just amd64. My assumption (which may be unjustified) is that by far the most common implementations to read a 64-bit register on i386 would be to read the lower 4 bytes first, followed by the upper 4 bytes (or vice versa) and then stitch them together. I think we should provide helper functions for these two cases, otherwise I fear our code base will be littered with multiple independent implementations of this. Some driver writer who wants to take advantage of these helper functions would do something like #ifdef i386 #define bus_space_read_8 bus_space_read_8_lower_first #endif otherwise, using bus_space_read_8 won't compile for i386 builds. If these implementations won't work for their case, they are free to write their own implementation or take whatever action is necessary. I guess my question is, are these cases common enough that it is worth helping developers by providing functions that do the double read and shifts for them, or do we leave them to deal with it on their own at the risk of possibly some duplicated code. Thanks, Carl
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?5078575B.2020808>