[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 06/11] exec.c: refactor cpu_physical_memory_map
From: |
Stefano Stabellini |
Subject: |
Re: [Qemu-devel] [PATCH 06/11] exec.c: refactor cpu_physical_memory_map |
Date: |
Thu, 23 Jun 2011 18:56:40 +0100 |
User-agent: |
Alpine 2.00 (DEB 1167 2008-08-23) |
On Thu, 23 Jun 2011, Peter Maydell wrote:
> On 19 June 2011 04:39, Alexander Graf <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.
>
> 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().
>
Thanks for the detailed explanation of the problem, I think I understand
what I have to do to fix.
However I would like to have a repro of the bug before sending any
patches, so that I am sure that the solution works correctly.
However I am not very familiar with ARM emulation in Qemu: could you
please let me know which target I have to compile, which machine I have
to emulate and what other steps do I have to take in order to see this
issue?
Many thanks in advance.
> > + 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().
right, I'll remove it
- [Qemu-devel] [PULL] Xen Patch Queue, Alexander Graf, 2011/06/20
- [Qemu-devel] [PATCH 09/11] xen: only track the linear framebuffer, Alexander Graf, 2011/06/20
- [Qemu-devel] [PATCH 11/11] xen: Add the Xen platform pci device, Alexander Graf, 2011/06/20
- [Qemu-devel] [PATCH 10/11] xen: fix interrupt routing, Alexander Graf, 2011/06/20
- [Qemu-devel] [PATCH 02/11] xen: Introduce VGA sync dirty bitmap support, Alexander Graf, 2011/06/20
- [Qemu-devel] [PATCH 07/11] xen: mapcache performance improvements, Alexander Graf, 2011/06/20
- [Qemu-devel] [PATCH 01/11] xen: Add xc_domain_add_to_physmap to xen_interface., Alexander Graf, 2011/06/20
- [Qemu-devel] [PATCH 08/11] cirrus_vga: reset lfb_addr after a pci config write if the BAR is unmapped, Alexander Graf, 2011/06/20
- [Qemu-devel] [PATCH 06/11] exec.c: refactor cpu_physical_memory_map, Alexander Graf, 2011/06/20
[Qemu-devel] [PATCH 05/11] xen: remove xen_map_block and xen_unmap_block, Alexander Graf, 2011/06/20
[Qemu-devel] [PATCH 04/11] xen: remove qemu_map_cache_unlock, Alexander Graf, 2011/06/20
[Qemu-devel] [PATCH 03/11] xen: fix qemu_map_cache with size != MCACHE_BUCKET_SIZE, Alexander Graf, 2011/06/20
Re: [Qemu-devel] [PULL] Xen Patch Queue, Anthony Liguori, 2011/06/22