No bus_space_read_8 on x86 ?
Robert Watson
rwatson at FreeBSD.org
Sat Oct 13 10:26:44 UTC 2012
On Fri, 12 Oct 2012, Carl Delsey wrote:
>> 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.
I was thinking we might suggest to developers that they use a KPI that
specifically captures the underlying semantics, so it's clear they understand
them. Untested example:
uint64_t v;
/*
* On 32-bit systems, read the 64-bit statistic using two 32-bit
* reads.
*
* XXX: This will sometimes lead to a race.
*
* XXX: Gosh, I wonder if some word-swapping is needed in the merge?
*/
#ifdef 32-bit
bus_space_read_region_4(space, handle, offset, (uint32_t *)&v, 2;
#else
bus_space_read_8(space, handle, offset, &v);
#endif
The potential need to word swap, however, suggests that you may be right about
the error-prone nature of manual merging.
Robert
More information about the freebsd-hackers
mailing list