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

[Alexander Graf <agraf@suse.de>]

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 <agraf@redhat.com>
>> Fixes: 965eb2f
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Reviewed-by: Laurent Vivier <lvivier@redhat.com>
> Tested-by: Laurent Vivier <lvivier@redhat.com>
>

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 ;)


Alex