Re: [QEMU PATCH v4 10/13] virtio-gpu: Resource UUID

[Akihiko Odaki <akihiko.odaki@xxxxxxxxxx>]

On 2023/09/13 21:58, Albert Esteve wrote:
> On Wed, Sep 13, 2023 at 2:22 PM Akihiko Odaki <akihiko.odaki@xxxxxxxxxx 
> <mailto:akihiko.odaki@xxxxxxxxxx>> wrote:
>     On 2023/09/13 20:34, Albert Esteve wrote:
>      >
>      >
>      > On Wed, Sep 13, 2023 at 12:34 PM Akihiko Odaki
>     <akihiko.odaki@xxxxxxxxxx <mailto:akihiko.odaki@xxxxxxxxxx>
>      > <mailto:akihiko.odaki@xxxxxxxxxx
>     <mailto:akihiko.odaki@xxxxxxxxxx>>> wrote:
>      >
>      >     On 2023/09/13 16:55, Albert Esteve wrote:
>      >      > Hi Antonio,
>      >      >
>      >      > If I'm not mistaken, this patch is related with:
>      >      >
>      >
>     https://lists.gnu.org/archive/html/qemu-devel/2023-09/msg01853.html
>     <https://lists.gnu.org/archive/html/qemu-devel/2023-09/msg01853.html>
>      >   
>       <https://lists.gnu.org/archive/html/qemu-devel/2023-09/msg01853.html <https://lists.gnu.org/archive/html/qemu-devel/2023-09/msg01853.html>>
>      >      >
>      >   
>       <https://lists.gnu.org/archive/html/qemu-devel/2023-09/msg01853.html <https://lists.gnu.org/archive/html/qemu-devel/2023-09/msg01853.html>
>      >   
>       <https://lists.gnu.org/archive/html/qemu-devel/2023-09/msg01853.html <https://lists.gnu.org/archive/html/qemu-devel/2023-09/msg01853.html>>>
>      >      > IMHO, ideally, virtio-gpu and vhost-user-gpu both, would
>     use the
>      >      > infrastructure from the patch I linked to store the
>      >      > virtio objects, so that they can be later shared with
>     other devices.
>      >
>      >     I don't think such sharing is possible because the resources are
>      >     identified by IDs that are local to the device. That also
>     complicates
>      >     migration.
>      >
>      >     Regards,
>      >     Akihiko Odaki
>      >
>      > Hi Akihiko,
>      >
>      > As far as I understand, the feature to export dma-bufs from the
>      > virtgpu was introduced as part of the virtio cross-device sharing
>      > proposal [1]. Thus, it shall be posible. When virtgpu ASSING_UUID,
>      > it exports and identifies the dmabuf resource, so that when the
>     dmabuf gets
>      > shared inside the guest (e.g., with virtio-video), we can use the
>     assigned
>      > UUID to find the dmabuf in the host (using the patch that I
>     linked above),
>      > and import it.
>      >
>      > [1] - https://lwn.net/Articles/828988/
>     <https://lwn.net/Articles/828988/> <https://lwn.net/Articles/828988/
>     <https://lwn.net/Articles/828988/>>
>
>     The problem is that virtio-gpu can have other kind of resources like
>     pixman and OpenGL textures and manage them and DMA-BUFs with unified
>     resource ID.
>
>
> I see.
>
>
>     So you cannot change:
>     g_hash_table_insert(g->resource_uuids,
>     GUINT_TO_POINTER(assign.resource_id), uuid);
>     by:
>     virtio_add_dmabuf(uuid, assign.resource_id);
>
>     assign.resource_id is not DMA-BUF file descriptor, and the underlying
>     resource my not be DMA-BUF at first place.
>
>
> I didn't really look into the patch in-depth, so the code was intended
> to give an idea of how the implementation would look like with
> the cross-device patch API. Indeed, it is not the resource_id,
> (I just took a brief look at the virtio specificacion 1.2), but the 
> underlying
> resource what we want to use here.
>
>
>     Also, since this lives in the common code that is not used only by
>     virtio-gpu-gl but also virtio-gpu, which supports migration, we also
>     need to take care of that. It is not a problem for DMA-BUF as
>     DMA-BUF is
>     not migratable anyway, but the situation is different in this case.
>
>     Implementing cross-device sharing is certainly a possibility, but that
>     requires more than dealing with DMA-BUFs.
>
>
> So, if I understood correctly, dmabufs are just a subset of the resources
> that the gpu manages, or can assign UUIDs to. I am not sure why
> the virt gpu driver would want to send a ASSIGN_UUID for anything
> that is not a dmabuf (are we sure it does?), but I am not super familiarized
> with virtgpu to begin with.
In my understanding, an resource will be first created as OpenGL or 
Vulkan textures and then exported as a DMA-BUF file descriptor. For 
these resource types exporting/importing code is mandatory.
For pixman buffers (i.e., non-virgl device), I don't see a compelling 
reason to have cross-device sharing. It is possible to omit resource 
UUID feature from non-virgl device to avoid implementing complicated 
migration.
> But I see that internally, the GPU specs relate a UUID with a resource_id,
> so we still need both tables:
> - one to relate UUID with resource_id to be able to locate the 
> underlying resource
> - the table that holds the dmabuf with the UUID for cross-device sharing
>
> With that in mind, sounds to me that the support for cross-device 
> sharing could
> be added on top of this patch, once 
> https://lists.gnu.org/archive/html/qemu-devel/2023-09/msg01850.html 
> <https://lists.gnu.org/archive/html/qemu-devel/2023-09/msg01850.html>
> lands.
That is possible, but I think it's better to implement cross-device 
sharing at the same time introducing virtio-dmabuf.
The current design of virtio-dmabuf looks somewhat inconsistent; it's 
named "dmabuf", but internally the UUIDs are stored into something named 
"resource_uuids" and it has SharedObjectType so it's more like a generic 
resource sharing mechanism. If you actually have an implementation of 
cross-device sharing using virtio-dmabuf, it will be clear what kind of 
feature is truly necessary.
Regards,
Akihiko Odaki