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
_______________________________________________
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "freebsd-hackers-unsubscr...@freebsd.org"
