Re: [Qemu-devel] [PATCH v2 4/5] exec.c: refactor cpu_physical_memory_map

[Alexander Graf]

On 12.07.2011, at 00:17, Jan Kiszka wrote:

> On 2011-05-19 19:35, address@hidden wrote:
>> From: Stefano Stabellini <address@hidden>
>> 
>> Introduce qemu_ram_ptr_length that takes an address and a size as
>> parameters rather than just an address.
>> 
>> Refactor cpu_physical_memory_map so that we call qemu_ram_ptr_length only
>> once rather than calling qemu_get_ram_ptr one time per page.
>> This is not only more efficient but also tries to simplify the logic of
>> the function.
>> Currently we are relying on the fact that all the pages are mapped
>> contiguously in qemu's address space: we have a check to make sure that
>> the virtual address returned by qemu_get_ram_ptr from the second call on
>> is consecutive. Now we are making this more explicit replacing all the
>> calls to qemu_get_ram_ptr with a single call to qemu_ram_ptr_length
>> passing a size argument.
> 
> This breaks cpu_physical_memory_map for >4G addresses on PC.
> Effectively, it doesn't account for the PCI gap, ie. that the RAM block
> is actually mapped in two chunks into the guest physical memory. One
> outcome is that QEMU aborts when we try to process an address that is
> now "outside" RAM. Simple to reproduce with a virtio NIC and 5G guest
> memory, even without KVM.

Yeah, that's what happens when you read mails too early in the morning :). The 
xen branch didn't get pulled yet, so upstream is missing the following patch:

commit f221e5ac378feea71d9857ddaa40f829c511742f
Author: Stefano Stabellini <address@hidden>
Date:   Mon Jun 27 18:26:06 2011 +0100

    qemu_ram_ptr_length: take ram_addr_t as arguments
    
    qemu_ram_ptr_length should take ram_addr_t as argument rather than
    target_phys_addr_t because is doing comparisons with RAMBlock addresses.
    
    cpu_physical_memory_map should create a ram_addr_t address to pass to
    qemu_ram_ptr_length from PhysPageDesc phys_offset.
    
    Remove code after abort() in qemu_ram_ptr_length.
    
    Changes in v2:
    
    - handle 0 size in qemu_ram_ptr_length;
    
    - rename addr1 to raddr;
    
    - initialize raddr to ULONG_MAX.
    
    Signed-off-by: Stefano Stabellini <address@hidden>
    Reviewed-by: Peter Maydell <address@hidden>
    Signed-off-by: Alexander Graf <address@hidden>


Anthony?

Alex