From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Lztk8-00048X-93 for qemu-devel@nongnu.org; Fri, 01 May 2009 10:29:36 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1Lztk3-00047N-2a for qemu-devel@nongnu.org; Fri, 01 May 2009 10:29:35 -0400 Received: from [199.232.76.173] (port=58094 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Lztk2-00047G-Ob for qemu-devel@nongnu.org; Fri, 01 May 2009 10:29:30 -0400 Received: from pop-tawny.atl.sa.earthlink.net ([207.69.195.67]:60792) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1Lztk2-0001GW-Di for qemu-devel@nongnu.org; Fri, 01 May 2009 10:29:30 -0400 Message-ID: <49FB0663.8050009@earthlink.net> Date: Fri, 01 May 2009 10:25:39 -0400 From: Robert Reif MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH] 64 bit I/O support v7 References: <49EDB109.5010009@earthlink.net> <200905011303.50885.paul@codesourcery.com> In-Reply-To: <200905011303.50885.paul@codesourcery.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paul Brook Cc: qemu-devel@nongnu.org Paul Brook wrote: >> The old array function call is supported so no existing drivers need >> to be modified. They can continue to do 2 32 bit accesses because 2 >> helper functions have been added to break up the accesses automatically. >> However, the helper functions should only be used until all drivers are >> converted to use the structure and then can be removed along >> with the old array functions api. The replacement of the arrays with >> structures in the drivers is very straightforward for drivers that don't >> do 64 bit I/O and the few that do can be cleaned up to remove the >> work arounds for the lack of true 64 bit I/O by their maintainers. >> > > This is going to be a bit of a pain, and a lot of duplication. My expectation > is that most devices don't know/care about 64-bit accesses and want them to > be automatically split into a pair of 32-bit accesses. I suggest pushing this > down into the lower level dispatch routines. By my reading your mem_writeq > helpers are broken if we happen to have multiple regions with the same opaque > value (this effects at least lsi53c895a.c). > > In the interests of avoiding duplication, I'd also implement > cpu_register_io_memory in terms of cpu_register_io_memory64. > > >> + return ((CPUReadMemoryFunc64*)(*mmio->mem_read[idx][3])) >> (mmio->opaque[idx][0][3], addr + mmio->region_offset[idx][0][3]); >> > > Eww. It would be a good idea to fix this at the same time. > > Paul > > > > The issue this patch is trying to address is not the case where software is trying to access 2 32 bit registers using a 64 bit access (because that should fault on sane hardware) but the case where we have a 64 bit counter and the 64 bit access is split up into 2 32 bit accesses and the counter ends up being read twice. Some drivers work around this by caching half of the 64 bit access so the cache is accessed for the second half rather than the hardware. However some hardware (like sparc) can access the timer as either 32 or 64 bits so this trick doesn't work. Hardware that supports reading 2 32 bit registers with one 64 bit access can have the 64 callback do 2 32 bit accesses.