Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 13 Oct 2012 11:26:42 +0100 (BST)
From:      Robert Watson <rwatson@FreeBSD.org>
To:        Carl Delsey <carl.r.delsey@intel.com>
Cc:        freebsd-hackers@freebsd.org
Subject:   Re: No bus_space_read_8 on x86 ?
Message-ID:  <alpine.BSF.2.00.1210131122130.46503@fledge.watson.org>
In-Reply-To: <5078575B.2020808@intel.com>
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> <5078575B.2020808@intel.com>

next in thread | previous in thread | raw e-mail | index | archive | help

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




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?alpine.BSF.2.00.1210131122130.46503>