[Qemu-devel] Re: [PATCH RFC V4 10/14] Introduce qemu_ram_ptr_unlock.

[Stefano Stabellini]

On Tue, 28 Sep 2010, Anthony Liguori wrote:
> On 09/28/2010 10:25 AM, Stefano Stabellini wrote:
> > On Tue, 28 Sep 2010, Anthony Liguori wrote:
> >    
> >> On 09/28/2010 10:01 AM, address@hidden wrote:
> >>      
> >>> From: Anthony PERARD<address@hidden>
> >>>
> >>> This function allows to unlock a ram_ptr give by qemu_get_ram_ptr. After
> >>> a call to qemu_ram_ptr_unlock, the pointer may be unmap from QEMU when
> >>> used with Xen.
> >>>
> >>> Signed-off-by: Anthony PERARD<address@hidden>
> >>>
> >>>        
> >> Why isn't hooking cpu_physical_memory_{map,unmap}() not enough?  That's
> >> really the intention of the API.
> >>
> >> You only really care about guest RAM, not device memory, correct?
> >>      
> > Yes, however at the moment all the calls to qemu_get_ram_ptr imply the
> > mapping in qemu address space to remain valid for an unlimited amount of
> > time.
> >    
> 
> Yes, but qemu_get_ram_ptr() is not a general purpose API.  It really 
> should only have one use--within exec.c to implement 
> cpu_physical_memory_* functions.  There are a few uses in hw/* but 
> they're all wrong and should be removed.  Fortunately, for the purposes 
> of the Xen machine, almost none of them actually matter.
> 

If this is the case, it is even better :)
Can we replace the call to qemu_get_ram_ptr with cpu_physical_memory_map
in the vga code?


> What I'm thinking is that RAM in Xen should not be backed at all from a 
> RAMBlock.  Instead, cpu_physical_memory_* functions should call an 
> explicit map/unmap() function that can be implemented as 
> qemu_get_ram_ptr() and a nop in the TCG/KVM case and as explicit map 
> cache operations in the Xen case.
> 

Yes, we basically followed a very similar line of thought: in the current
implementation we have just one ramblock as a placeholder for the
guest's ram, then we have three hooks in qemu_ram_alloc_from_ptr,
qemu_get_ram_ptr and qemu_ram_free for xen specific ways to allocate,
map and free memory but we reuse everything else.

Let's take cpu_physical_memory_map for example: we completely reuse the
generic implementation, that ends up calling either
qemu_get_ram_ptr or cpu_physical_memory_rw.
In case of qemu_get_ram_ptr, we still reuse the generic code but we have
a xen specific hook to call the mapcache.
In case of cpu_physical_memory_rw, we didn't need to change anything to
do the mapping because it is implemented using qemu_get_ram_ptr (see
above), we just added a call to qemu_ram_ptr_unlock to unlock the
mapping at the end of the function (a nop for TCG/KVM).
So qemu_get_ram_ptr and qemu_ram_ptr_unlock are basically the explicit
map/unmap() functions you are referring to.

We could probably remove the single ramblock we added and provide a xen
specific implementation of qemu_ram_alloc_from_ptr and qemu_ram_free
that don't iterate over the ramblock list if you think is better.