Re: [Qemu-devel] [PATCH] exec: skip MMIO regions correctly in cpu_physical_memory_write_rom_internal

[Laurent Vivier]

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 <address@hidden>
> Fixes: 965eb2f
> Signed-off-by: Paolo Bonzini <address@hidden>

Reviewed-by: Laurent Vivier <address@hidden>
Tested-by: Laurent Vivier <address@hidden>