From owner-freebsd-hackers@FreeBSD.ORG Sat Oct 13 10:26:44 2012 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 60591A7F; Sat, 13 Oct 2012 10:26:44 +0000 (UTC) (envelope-from rwatson@FreeBSD.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id 118148FC12; Sat, 13 Oct 2012 10:26:44 +0000 (UTC) Received: from fledge.watson.org (fledge.watson.org [65.122.17.41]) by cyrus.watson.org (Postfix) with ESMTPS id 10A2546B09; Sat, 13 Oct 2012 06:26:43 -0400 (EDT) Date: Sat, 13 Oct 2012 11:26:42 +0100 (BST) From: Robert Watson X-X-Sender: robert@fledge.watson.org To: Carl Delsey Subject: Re: No bus_space_read_8 on x86 ? In-Reply-To: <5078575B.2020808@intel.com> Message-ID: References: <506DC574.9010300@intel.com> <201210091154.15873.jhb@freebsd.org> <5075EC29.1010907@intel.com> <201210121131.46373.jhb@freebsd.org> <5078575B.2020808@intel.com> User-Agent: Alpine 2.00 (BSF 1167 2008-08-23) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: freebsd-hackers@freebsd.org X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 13 Oct 2012 10:26:44 -0000 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