Hi Akihiko,
> Subject: Re: [PATCH v8 10/10] virtio-gpu-dmabuf: Create dmabuf for blobs
> associated with VFIO devices
>
> On 2026/02/28 7:13, Vivek Kasireddy wrote:
> > In addition to memfd, a blob resource can also have its backing
> > storage in a VFIO device region. Since, there is no effective way
> > to determine where the backing storage is located, we first try to
> > create a dmabuf assuming it is in memfd. If that fails, we try to
> > create a dmabuf assuming it is in VFIO device region.
> >
> > So, we first call virtio_gpu_create_udmabuf() to check if the blob's
> > backing storage is located in a memfd or not. If it is not, we invoke
> > the vfio_device_create_dmabuf_fd() API which identifies the right
> > VFIO device and eventually creates a dmabuf fd.
> >
> > Note that in virtio_gpu_remap_dmabuf(), we first try to test if
> > the VFIO dmabuf exporter supports mmap or not. If it doesn't, we
> > fall back to the vfio_device_mmap_dmabuf() API to get a mapping
> > created for the dmabuf.
> >
> > Cc: Marc-André Lureau <[email protected]>
> > Cc: Alex Bennée <[email protected]>
> > Cc: Akihiko Odaki <[email protected]>
> > Cc: Dmitry Osipenko <[email protected]>
> > Cc: Alex Williamson <[email protected]>
> > Cc: Cédric Le Goater <[email protected]>
> > Signed-off-by: Vivek Kasireddy <[email protected]>
> > ---
> > hw/display/virtio-gpu-dmabuf.c | 35 +++++++++++++++++++++++++++-
> ------
> > 1 file changed, 28 insertions(+), 7 deletions(-)
> >
> > diff --git a/hw/display/virtio-gpu-dmabuf.c b/hw/display/virtio-gpu-
> dmabuf.c
> > index b2badfc1bd..533d3d601c 100644
> > --- a/hw/display/virtio-gpu-dmabuf.c
> > +++ b/hw/display/virtio-gpu-dmabuf.c
> > @@ -74,13 +74,21 @@ static int virtio_gpu_create_udmabuf(struct
> virtio_gpu_simple_resource *res,
> > static bool virtio_gpu_remap_dmabuf(struct
> virtio_gpu_simple_resource *res,
> > Error **errp)
> > {
> > + Error *mmap_err = NULL;
> > void *map;
> >
> > map = mmap(NULL, res->blob_size, PROT_READ, MAP_SHARED, res-
> >dmabuf_fd, 0);
> > if (map == MAP_FAILED) {
> > - error_setg_errno(errp, errno, "dmabuf mmap failed");
> > + error_setg_errno(&mmap_err, errno, "dmabuf mmap failed: ");
> > res->remapped = NULL;
> > - return false;
> > +
> > + if (!vfio_device_mmap_dmabuf(res->iov, res->iov_cnt, &map,
> > + res->blob_size, errp)) {
>
> Let's call vfio_device_mmap_dmabuf() only when
> vfio_device_create_dmabuf_fd() is called. Hopefully it will simplify the
> logic structure.
The logic looks OK to me but are you suggesting that it would be simpler to
remove the call to vfio_device_mmap_dmabuf() from virtio_gpu_remap_dmabuf()
and call it where vfio_device_create_dmabuf_fd() gets called?
Or, if we keep the call to vfio_device_mmap_dmabuf() in
virtio_gpu_remap_dmabuf(),
then to implement your suggestion, I think we need a flag to identify dmabufs
that are created via VFIO so that we would call vfio_device_mmap_dmabuf()
only for these ones.
Also, for VFIO based dmabufs, should we directly call vfio_device_mmap_dmabuf()
or is it ok if we first call mmap() and check if it works or not?
Thanks,
Vivek
>
> Regards,
> Akihiko Odaki
>
> > + error_report_err(mmap_err);
> > + return false;
> > + }
> > + error_free(mmap_err);
> > + mmap_err = NULL;
> > }
> > res->remapped = map;
> > return true;
> > @@ -137,7 +145,7 @@ bool virtio_gpu_have_udmabuf(void)
> >
> > void virtio_gpu_init_dmabuf(struct virtio_gpu_simple_resource *res)
> > {
> > - Error *local_err = NULL;
> > + Error *local_err = NULL, *vfio_err = NULL;
> > void *pdata = NULL;
> > int ret;
> >
> > @@ -151,15 +159,28 @@ void virtio_gpu_init_dmabuf(struct
> virtio_gpu_simple_resource *res)
> > error_prepend(&local_err, "Cannot create dmabuf: ");
> >
> > if (ret == VFIO_DMABUF_CREATE_GUEST_ERROR) {
> > - qemu_log_mask(LOG_GUEST_ERROR,
> > - "Cannot create dmabuf: incompatible
> > memory\n");
> > - error_free(local_err);
> > - return;
> > + ret = vfio_device_create_dmabuf_fd(res->iov, res->iov_cnt,
> > + &vfio_err);
> > + if (ret > 0) {
> > + error_free(local_err);
> > + local_err = NULL;
> > + goto out;
> > + }
> > +
> > + if (ret == VFIO_DMABUF_CREATE_GUEST_ERROR) {
> > + qemu_log_mask(LOG_GUEST_ERROR,
> > + "Cannot create dmabuf: incompatible
> > memory\n");
> > + error_free(vfio_err);
> > + error_free(local_err);
> > + return;
> > + }
> > + error_report_err(vfio_err);
> > }
> > error_report_err(local_err);
> > return;
> > }
> >
> > +out:
> > res->dmabuf_fd = ret;
> > if (!virtio_gpu_remap_dmabuf(res, &local_err)) {
> > error_report_err(local_err);
