This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
Home Products Support Community News


[Xen-devel] Re: [Qemu-devel] [PATCH 06/11] exec.c: refactor cpu_physical

To: Alexander Graf <agraf@xxxxxxx>
Subject: [Xen-devel] Re: [Qemu-devel] [PATCH 06/11] exec.c: refactor cpu_physical_memory_map
From: Peter Maydell <peter.maydell@xxxxxxxxxx>
Date: Thu, 23 Jun 2011 18:08:12 +0100
Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx Devel" <xen-devel@xxxxxxxxxxxxxxxxxxx>, "qemu-devel@xxxxxxxxxx Developers" <qemu-devel@xxxxxxxxxx>, Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
Delivery-date: Thu, 23 Jun 2011 13:12:31 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <1308454790-13750-6-git-send-email-agraf@xxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <1308452228-11055-1-git-send-email-agraf@xxxxxxx> <1308454790-13750-6-git-send-email-agraf@xxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
On 19 June 2011 04:39, Alexander Graf <agraf@xxxxxxx> wrote:
> From: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> 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.

This change breaks cpu_physical_memory_map() in the case where
the passed in physical memory address corresponds to a RAM block
which has been mapped in at multiple physical addresses. Specifically,
for Versatile Express this means we abort() when framebuffer_update_display()
calls cpu_physical_memory_map() for an address which is in the second
mapped area.

> +/* Return a host pointer to guest's ram. Similar to qemu_get_ram_ptr
> + * but takes a size argument */
> +void *qemu_ram_ptr_length(target_phys_addr_t addr, target_phys_addr_t *size)

qemu_get_ram_ptr() takes a ramaddr_t, not a target_phys_addr_t;
so should this, because we're just looking through the ram_list
without doing physaddr to ramaddr translation.

Conceptually size should also be a ram_addr_t*, although if you
do that you can't just pass the plen pointer through to it.

The old implementation of cpu_physical_memory_map() worked
because it created the address to pass to qemu_get_ram_ptr()
from the p->phys_offset it got from phys_page_find(). The
new implementation needs to do something similar, not just pass
a target_phys_addr_t to qemu_ram_ptr_length().

> +        fprintf(stderr, "Bad ram offset %" PRIx64 "\n", (uint64_t)addr);
> +        abort();
> +
> +        *size = 0;
> +        return NULL;

There doesn't seem much point in having code following an abort().

-- PMM

Xen-devel mailing list

<Prev in Thread] Current Thread [Next in Thread>