[Spice-devel] UMS memory management

[Yonit Halperin]

Hi,
On 03/27/2013 06:17 AM, Hans de Goede wrote:
> Hi,
>
> On 03/27/2013 09:07 AM, Søren Sandmann wrote:
>> Hello,
>>
>> The following is some notes on what I am planning to do for UMS memory
>> management. It basically amounts to a rewrite of qxl-surface-ums.c
>
> Looks / sounds good, a few remarks below (note I'm not an expert on this
> piece
> of the spice code).
>
> <snip>
>
>>       "in host memory":
>>          - no associated ID
>>          - has associated pixmap
>>          - pixman image may not exist
>
> Hmm, I assume with host-memory you mean guest memory here, right? iow
> surfaces
> will be moved by the driver from video-memory to X-server allocated
> memory (which
> in a vm is guest memory). I'm assuming this is what you mean in the rest of
> my reply.
>
> <snip>
>
>
>> Details:
>>
>> ** IN_VIDEO => IN_HOST
>>
>> - update_area is issued for the surface in question
>> - data is copied to pixman image
>> - state is set to IN_HOST
>> - destroy command is issued for the ID
>
> I assume the main use-case for this is freeing up video memory,
> unfortunately due to glz compression and the drawing command tree
> kept in the spice-server, it is possible that the server itself
> still holds a reference to the surface, and sending the destroy
> will not end up freeing any memory (atleast not directly), this
> is esp. going to be a problem when doing these kind of evictions
> with the intention of freeing up a larger block of memory
> for a new surface.
>
> I've been discussing this with Alon when we were in Brno together,
> and I think we need a new qxl device io-command for this, where the
> driver can tell it wants to video memory to be releases *now*
> rather then when the server looses it last ref, and then the
> server would need to do an eviction of its own, moving the surface
> from (shared) video memory to some memory inside the qemu process.
>
> Note that we will want to limit the amount of memory which can be
> use spice-server-side by these evicted, to avoid this being used
> as some kind of DOS attack, but this should be a limit which is
> (hardly) ever reached in practice.
>
> Implementing this won't be trivial, but if we all agree it is
> something which we want to have, then I hope we can find someone
> in the spice team to start working on this.

Actually, glz related drawables don't hold references to surfaces. 
Surfaces references are kept for rendering purposes. In addition, 
QXL_IO_DESTROY_SURFACE_WAIT, is aimed for releasing all the references 
of a surface, and eventually releasing the surface. The only thing that 
is missing there, is a call to worker->qxl->st->qif->flush_resources
for updating the release ring (it can be achieved by calling
  QXL_IO_FLUSH_RELEASE from the guest, but as Dave noticed it is buggy, 
since qxl_push_free_res doesn't support being called from different 
threads).

As long as update_area is called before evicting a surface from the 
vram, there is no need to copy and keep such surfaces in the server. 
This will only be relevant if at some point we would like to avoid the 
rendering, and instead store the rendering commands that are related to 
the surface.

p.s. I think that for wddm, and also for Dave's kms paging, we will need 
to add QXL_IO_DESTROY_BITMAP_WAIT or something similar.
>
> Regards,
>
> Hans
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel