[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] exec: skip MMIO regions correctly in cpu_physic
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH] exec: skip MMIO regions correctly in cpu_physical_memory_write_rom_internal |
Date: |
Mon, 6 Jul 2015 10:55:18 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.0.1 |
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 <address@hidden>
>>> Fixes: 965eb2f
>>> Signed-off-by: Paolo Bonzini <address@hidden>
>> Reviewed-by: Laurent Vivier <address@hidden>
>> Tested-by: Laurent Vivier <address@hidden>
>>
>
> 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