From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58447) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZC2BT-0001ID-Hp for qemu-devel@nongnu.org; Mon, 06 Jul 2015 04:55:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZC2BP-0006tz-EC for qemu-devel@nongnu.org; Mon, 06 Jul 2015 04:55:27 -0400 Received: from mx1.redhat.com ([209.132.183.28]:43227) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZC2BP-0006tU-5Z for qemu-devel@nongnu.org; Mon, 06 Jul 2015 04:55:23 -0400 References: <1435963378-22229-1-git-send-email-pbonzini@redhat.com> <559713FF.4010601@redhat.com> <559A4179.1080101@suse.de> From: Paolo Bonzini Message-ID: <559A4276.3010208@redhat.com> Date: Mon, 6 Jul 2015 10:55:18 +0200 MIME-Version: 1.0 In-Reply-To: <559A4179.1080101@suse.de> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] exec: skip MMIO regions correctly in cpu_physical_memory_write_rom_internal List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf , Laurent Vivier , qemu-devel@nongnu.org On 06/07/2015 10:51, Alexander Graf wrote: > On 07/04/15 01:00, Laurent Vivier wrote: >> >> On 04/07/2015 00:42, Paolo Bonzini wrote: >>> Loading the BIOS in the mac99 machine is interesting, because there is a >>> PROM in the middle of the BIOS region (from 16K to 32K). Before memory >>> region accesses were clamped, when QEMU was asked to load a BIOS from >>> 0xfff00000 to 0xffffffff it would put even those 16K from the BIOS file >>> into the region. This is weird because those 16K were not actually >>> visible between 0xfff04000 and 0xfff07fff. However, it worked. >>> >>> After clamping was added, this also worked. In this case, the >>> cpu_physical_memory_write_rom_internal function split the write in >>> three parts: the first 16K were copied, the PROM area (second 16K) were >>> ignored, then the rest was copied. >>> >>> Problems then started with commit 965eb2f (exec: do not clamp accesses >>> to MMIO regions, 2015-06-17). Clamping accesses is not done for MMIO >>> regions because they can overlap wildly, and MMIO registers can be >>> expected to perform full-width accesses based only on their address >>> (with no respect for adjacent registers that could decode to completely >>> different MemoryRegions). However, this lack of clamping also applied >>> to the PROM area! cpu_physical_memory_write_rom_internal thus failed >>> to copy the third range above, i.e. only copied the first 16K of the >>> BIOS. >>> >>> In effect, address_space_translate is expecting _something else_ to do >>> the clamping for MMIO regions if the incoming length is large. This >>> "something else" is memory_access_size in the case of address_space_rw, >>> so use the same logic in cpu_physical_memory_write_rom_internal. >>> >>> The fix is just one line, but also add a comment explaining why there >>> is no clamping for MMIO regions, and what it means for the callers. >>> >>> Reported-by: Alexander Graf >>> Fixes: 965eb2f >>> Signed-off-by: Paolo Bonzini >> Reviewed-by: Laurent Vivier >> Tested-by: Laurent Vivier >> > > Thanks, I've applied this to ppc-next to fix up the mac99 target. But > I'd be happy to see it in the tree before my next pull request ;) Sending pull request later today. Paolo