On Wed, Sep 13, 2023 at 3:43 PM Akihiko Odaki <akihiko.odaki@daynix.com
<mailto:akihiko.odaki@daynix.com>> wrote:
On 2023/09/13 21:58, Albert Esteve wrote:
>
>
> On Wed, Sep 13, 2023 at 2:22 PM Akihiko Odaki
<akihiko.odaki@daynix.com <mailto:akihiko.odaki@daynix.com>
> <mailto:akihiko.odaki@daynix.com
<mailto:akihiko.odaki@daynix.com>>> wrote:
>
> On 2023/09/13 20:34, Albert Esteve wrote:
> >
> >
> > On Wed, Sep 13, 2023 at 12:34 PM Akihiko Odaki
> <akihiko.odaki@daynix.com <mailto:akihiko.odaki@daynix.com>
<mailto:akihiko.odaki@daynix.com <mailto:akihiko.odaki@daynix.com>>
> > <mailto:akihiko.odaki@daynix.com
<mailto:akihiko.odaki@daynix.com>
> <mailto:akihiko.odaki@daynix.com
<mailto:akihiko.odaki@daynix.com>>>> 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>>>>
> > > 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/>>>
>
> 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>>
> 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).