On Wed, Sep 13, 2023 at 4:18 PM Albert Esteve <aest...@redhat.com> wrote:
> > > On Wed, Sep 13, 2023 at 3:43 PM Akihiko Odaki <akihiko.od...@daynix.com> > wrote: > >> On 2023/09/13 21:58, Albert Esteve wrote: >> > >> > >> > On Wed, Sep 13, 2023 at 2:22 PM Akihiko Odaki <akihiko.od...@daynix.com >> > <mailto:akihiko.od...@daynix.com>> wrote: >> > >> > On 2023/09/13 20:34, Albert Esteve wrote: >> > > >> > > >> > > On Wed, Sep 13, 2023 at 12:34 PM Akihiko Odaki >> > <akihiko.od...@daynix.com <mailto:akihiko.od...@daynix.com> >> > > <mailto:akihiko.od...@daynix.com >> > <mailto:akihiko.od...@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>>> >> > > > 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. >> > > 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> >> > lands. >> >> Also, to clarify what I stated here: I am not trying to get your patch blocked until the other one lands. I think both could be integrated in parallel, and then have virtgpu use the cross-device sharing API on top of your patch. Regards, Albert > 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). > > [1] - > https://docs.oasis-open.org/virtio/virtio/v1.2/csd01/virtio-v1.2-csd01.html#x1-10500011 > > >> Regards, >> Akihiko Odaki >> >>
