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

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Thu, 23 Jun 2011, Peter Maydell wrote:
> 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().
> 

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

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel