From b242e0e0e2969c044a318e56f7988bbd84de1f63 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Sat, 4 Jul 2015 00:24:51 +0200 Subject: exec: skip MMIO regions correctly in cpu_physical_memory_write_rom_internal 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. Reported-by: Alexander Graf Reviewed-by: Laurent Vivier Tested-by: Laurent Vivier Fixes: 965eb2f Signed-off-by: Paolo Bonzini --- exec.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/exec.c b/exec.c index 3457f7e828..251dc79e10 100644 --- a/exec.c +++ b/exec.c @@ -353,6 +353,18 @@ address_space_translate_internal(AddressSpaceDispatch *d, hwaddr addr, hwaddr *x *xlat = addr + section->offset_within_region; mr = section->mr; + + /* MMIO registers can be expected to perform full-width accesses based only + * on their address, without considering adjacent registers that could + * decode to completely different MemoryRegions. When such registers + * exist (e.g. I/O ports 0xcf8 and 0xcf9 on most PC chipsets), MMIO + * regions overlap wildly. For this reason we cannot clamp the accesses + * here. + * + * If the length is small (as is the case for address_space_ldl/stl), + * everything works fine. If the incoming length is large, however, + * the caller really has to do the clamping through memory_access_size. + */ if (memory_region_is_ram(mr)) { diff = int128_sub(section->size, int128_make64(addr)); *plen = int128_get64(int128_min(diff, int128_make64(*plen))); @@ -2491,7 +2503,7 @@ static inline void cpu_physical_memory_write_rom_internal(AddressSpace *as, if (!(memory_region_is_ram(mr) || memory_region_is_romd(mr))) { - /* do nothing */ + l = memory_access_size(mr, l, addr1); } else { addr1 += memory_region_get_ram_addr(mr); /* ROM/RAM case */ -- cgit v1.2.3