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

[Jan Kiszka <jan.kiszka@web.de>]

[-- Attachment #1: Type: text/plain, Size: 2697 bytes --]

On 2011-07-12 08:28, Alexander Graf wrote:
> 
> On 12.07.2011, at 00:17, Jan Kiszka wrote:
> 
>> On 2011-05-19 19:35, stefano.stabellini@eu.citrix.com wrote:
>>> From: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>>>
>>> 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 <stefano.stabellini@eu.citrix.com>
> 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 <stefano.stabellini@eu.citrix.com>
>     Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>     Signed-off-by: Alexander Graf <agraf@suse.de>

Maybe subject or changlog can reflect what this all fixes?

> 
> Anthony?

Am I the only one under the impression that too many patches are in
limbo ATM?

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]