From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55854) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZB9wW-0001NP-4Z for qemu-devel@nongnu.org; Fri, 03 Jul 2015 19:00:25 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZB9wR-0005bi-3O for qemu-devel@nongnu.org; Fri, 03 Jul 2015 19:00:24 -0400 Received: from mx1.redhat.com ([209.132.183.28]:38892) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZB9wQ-0005bF-TS for qemu-devel@nongnu.org; Fri, 03 Jul 2015 19:00:19 -0400 Message-ID: <559713FF.4010601@redhat.com> Date: Sat, 04 Jul 2015 01:00:15 +0200 From: Laurent Vivier MIME-Version: 1.0 References: <1435963378-22229-1-git-send-email-pbonzini@redhat.com> In-Reply-To: <1435963378-22229-1-git-send-email-pbonzini@redhat.com> 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: Paolo Bonzini , qemu-devel@nongnu.org Cc: agraf@suse.de 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