[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



On 2023/09/14 17:29, Albert Esteve wrote:


On Thu, Sep 14, 2023 at 9:17 AM Akihiko Odaki <akihiko.odaki@xxxxxxxxxx <mailto:akihiko.odaki@xxxxxxxxxx>> wrote:

    On 2023/09/13 23:18, Albert Esteve wrote:
     >
     >
     > On Wed, Sep 13, 2023 at 3:43 PM Akihiko Odaki
    <akihiko.odaki@xxxxxxxxxx <mailto:akihiko.odaki@xxxxxxxxxx>
     > <mailto:akihiko.odaki@xxxxxxxxxx
    <mailto:akihiko.odaki@xxxxxxxxxx>>> wrote:
     >
     >     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>
    <mailto:akihiko.odaki@xxxxxxxxxx <mailto:akihiko.odaki@xxxxxxxxxx>>
     >      > <mailto:akihiko.odaki@xxxxxxxxxx
    <mailto:akihiko.odaki@xxxxxxxxxx>
     >     <mailto: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>>
     >     <mailto:akihiko.odaki@xxxxxxxxxx
    <mailto:akihiko.odaki@xxxxxxxxxx> <mailto:akihiko.odaki@xxxxxxxxxx
    <mailto:akihiko.odaki@xxxxxxxxxx>>>
     >      >      > <mailto:akihiko.odaki@xxxxxxxxxx
    <mailto:akihiko.odaki@xxxxxxxxxx>
     >     <mailto:akihiko.odaki@xxxxxxxxxx
    <mailto:akihiko.odaki@xxxxxxxxxx>>
     >      >     <mailto: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>>>
     >      >      >
     >      >
>  <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>>>>
     >      >      >      >
     >      >      >
     >      >
>  <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>>>
     >      >      >
     >      >
>  <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/>>
     >      >     <https://lwn.net/Articles/828988/
    <https://lwn.net/Articles/828988/>
     >     <https://lwn.net/Articles/828988/
    <https://lwn.net/Articles/828988/>>>
     >     <https://lwn.net/Articles/828988/
    <https://lwn.net/Articles/828988/> <https://lwn.net/Articles/828988/
    <https://lwn.net/Articles/828988/>>
     >      >     <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.
     >
     >
     > I see, thanks for the clarification.
     > I would assume you could avoid the UUID feature for those
    resources, but
     > I will need to check the driver implementation. It is worth checking
     > though, if
     > that would simplify the implementation.
     >
     >
     >      > 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>
>  <https://lists.gnu.org/archive/html/qemu-devel/2023-09/msg01850.html <https://lists.gnu.org/archive/html/qemu-devel/2023-09/msg01850.html>>
     >      >
>  <https://lists.gnu.org/archive/html/qemu-devel/2023-09/msg01850.html <https://lists.gnu.org/archive/html/qemu-devel/2023-09/msg01850.html> >  <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.
     >
     >
     > Yeah, the file was named as virtio-dmabuf following the kernel
     > implementation. Also, because for the moment it only aims to share
     > dmabufs. However, virtio specs leave the virtio object
    defintion vague [1]
     > (I guess purposely). It is up to the specific devices to define
    what an
     > object
     > means for them. So the implementation tries to follow that, and
     > leave the contents of the table generic. The table can hold any
    kind of
     > object,
     > and the API exposes type-specific functions (for dmabufs, or others).
    In the guest kernel, the name "virtio_dma_buf" represents the interface
    between the *guest* kernel and *guest* user-space. It makes sense since
    the cross-device resource sharing is managed by the userspace and
    DMA-BUF is the only interface between them for this purpose.

    The situation is different for QEMU; QEMU interacts with backends using
    backend-specific interfaces (OpenGL/pixman) and virgl is capable to
    export textures as DMA-BUF. DMA-BUF is not universal in this sense. As
    such, we cannot just borrow the kernel-side naming but invent one.

It is not a gpu-specific feature. It is a generic cross-device sharing
mechanism for virtio objects. In this case, virtio objects happen to be
dmabufs in this first iteration. Hence, the name.

virtio-gpu (and vhost-user-gpu) will use this feature only with virgl, that is
fine, and transversal to the object-sharing mechanism. It allows
to share dmabufs in the host following how they are shared in the guest.
The virtgpu driver may call ASSIGN_UUID for other types of resources (not sure,
but could be), but they will never be shared with other virtio devices.
So they are not too relevant. Also, the shared objects table could potentially
be accessed from any virtio device, not only virtio-gpu or virtio-video.

The virtgpu driver will call ASSIGN_UUID for resources that are backed with pixman buffer. What is used as the backing store for resources is an implementation detail of VMM and the guest cannot know it. For the guest, they are same kind of resources (2D images).



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.