[PATCH v9 10/10] virtio-gpu-dmabuf: Create dmabuf for blobs associated with VFIO devices

Vivek Kasireddy posted 10 patches 1 month, 1 week ago
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, "Alex Bennée" <alex.bennee@linaro.org>, Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>, Dmitry Osipenko <dmitry.osipenko@collabora.com>, Alex Williamson <alex@shazbot.org>, "Cédric Le Goater" <clg@redhat.com>
There is a newer version of this series
[PATCH v9 10/10] virtio-gpu-dmabuf: Create dmabuf for blobs associated with VFIO devices
Posted by Vivek Kasireddy 1 month, 1 week ago
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, for mmapping the dmabuf, we directly call mmap() if the
dmabuf fd was created via virtio_gpu_create_udmabuf() since we know
that the udmabuf driver supports mmap(). However, if the dmabuf was
created via vfio_device_create_dmabuf_fd(), we use the
vfio_device_mmap_dmabuf() API to get a mapping for the dmabuf.

Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Cc: Alex Bennée <alex.bennee@linaro.org>
Cc: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
Cc: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Cc: Alex Williamson <alex@shazbot.org>
Cc: Cédric Le Goater <clg@redhat.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 hw/display/virtio-gpu-dmabuf.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/hw/display/virtio-gpu-dmabuf.c b/hw/display/virtio-gpu-dmabuf.c
index dd64fbbc2d..7f4c399416 100644
--- a/hw/display/virtio-gpu-dmabuf.c
+++ b/hw/display/virtio-gpu-dmabuf.c
@@ -136,7 +136,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;
 
@@ -155,10 +155,24 @@ void virtio_gpu_init_dmabuf(struct virtio_gpu_simple_resource *res)
         }
 
         if (ret == VFIO_DMABUF_CREATE_ERR_INVALID_IOV) {
-            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) {
+                if (vfio_device_mmap_dmabuf(res->iov, res->iov_cnt, &pdata,
+                                            res->blob_size, &vfio_err)) {
+                    error_free(local_err);
+                    goto out;
+                }
+            }
+
+            if (ret == VFIO_DMABUF_CREATE_ERR_INVALID_IOV) {
+                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;
-- 
2.53.0


Re: [PATCH v9 10/10] virtio-gpu-dmabuf: Create dmabuf for blobs associated with VFIO devices
Posted by Akihiko Odaki 1 month, 1 week ago
On 2026/03/05 15:54, 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, for mmapping the dmabuf, we directly call mmap() if the
> dmabuf fd was created via virtio_gpu_create_udmabuf() since we know
> that the udmabuf driver supports mmap(). However, if the dmabuf was
> created via vfio_device_create_dmabuf_fd(), we use the
> vfio_device_mmap_dmabuf() API to get a mapping for the dmabuf.
> 
> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> Cc: Alex Bennée <alex.bennee@linaro.org>
> Cc: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> Cc: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> Cc: Alex Williamson <alex@shazbot.org>
> Cc: Cédric Le Goater <clg@redhat.com>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> ---
>   hw/display/virtio-gpu-dmabuf.c | 24 +++++++++++++++++++-----
>   1 file changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/display/virtio-gpu-dmabuf.c b/hw/display/virtio-gpu-dmabuf.c
> index dd64fbbc2d..7f4c399416 100644
> --- a/hw/display/virtio-gpu-dmabuf.c
> +++ b/hw/display/virtio-gpu-dmabuf.c
> @@ -136,7 +136,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;
>   
> @@ -155,10 +155,24 @@ void virtio_gpu_init_dmabuf(struct virtio_gpu_simple_resource *res)
>           }
>   
>           if (ret == VFIO_DMABUF_CREATE_ERR_INVALID_IOV) {
> -            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) {

Zero represents a success by convention.

> +                if (vfio_device_mmap_dmabuf(res->iov, res->iov_cnt, &pdata,
> +                                            res->blob_size, &vfio_err)) {
> +                    error_free(local_err);
> +                    goto out;
> +                }
> +            }
> +
> +            if (ret == VFIO_DMABUF_CREATE_ERR_INVALID_IOV) {
> +                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);

This reports invalid IOV even when it is actually a valid pointer to 
VFIO and the VFIO is failing. Such an error report does not help and we 
should instead report only the VFIO error.

And there is another bug with the previous patch where res->dmabuf_fd is 
invalid when virtio_gpu_remap_dmabuf() uses it.

Below is my attempt to fix these issues:

     if (res->iov_cnt == 1 &&
         res->iov[0].iov_len < 4096) {
         res->dmabuf_fd = -1;
         pdata = res->iov[0].iov_base;
     } else {
         res->dmabuf_fd = virtio_gpu_create_udmabuf(res, &local_err);
         if (res->dmabuf_fd == VFIO_DMABUF_CREATE_ERR_INVALID_IOV) {
             error_free_or_abort(local_err);

             res->dmabuf_fd = vfio_device_create_dmabuf_fd(res->iov,
                                                           res->iov_cnt,
                                                           &local_err);
             if (res->dmabuf_fd == VFIO_DMABUF_CREATE_ERR_INVALID_IOV) {
                 res->dmabuf_fd = -1;
                 error_free_or_abort(local_err);
                 qemu_log_mask(LOG_GUEST_ERROR,
                               "Cannot create dmabuf: incompatible 
memory\n");
                 return;
             }

             if (res->dmabuf_fd < 0 ||
                 !vfio_device_mmap_dmabuf(res->iov, res->iov_cnt, &pdata,
                                          res->blob_size, &local_err)) {
                 res->dmabuf_fd = -1;
             }
         } else if (res->dmabuf_fd < 0 ||
                    !virtio_gpu_remap_dmabuf(res, &pdata, &local_err)) {
             res->dmabuf_fd = -1;
         }

         if (res->dmabuf_fd < 0) {
             error_report_err(local_err);
             return;
         }

         res->remapped = pdata;
     }

Regards,
Akihiko Odaki

>           return;


RE: [PATCH v9 10/10] virtio-gpu-dmabuf: Create dmabuf for blobs associated with VFIO devices
Posted by Kasireddy, Vivek 1 month ago
Hi Akihiko,

> Subject: Re: [PATCH v9 10/10] virtio-gpu-dmabuf: Create dmabuf for
> blobs associated with VFIO devices
> 
> On 2026/03/05 15:54, 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, for mmapping the dmabuf, we directly call mmap() if the
> > dmabuf fd was created via virtio_gpu_create_udmabuf() since we
> know
> > that the udmabuf driver supports mmap(). However, if the dmabuf
> was
> > created via vfio_device_create_dmabuf_fd(), we use the
> > vfio_device_mmap_dmabuf() API to get a mapping for the dmabuf.
> >
> > Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Cc: Alex Bennée <alex.bennee@linaro.org>
> > Cc: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> > Cc: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> > Cc: Alex Williamson <alex@shazbot.org>
> > Cc: Cédric Le Goater <clg@redhat.com>
> > Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> > ---
> >   hw/display/virtio-gpu-dmabuf.c | 24 +++++++++++++++++++-----
> >   1 file changed, 19 insertions(+), 5 deletions(-)
> >
> > diff --git a/hw/display/virtio-gpu-dmabuf.c b/hw/display/virtio-gpu-
> dmabuf.c
> > index dd64fbbc2d..7f4c399416 100644
> > --- a/hw/display/virtio-gpu-dmabuf.c
> > +++ b/hw/display/virtio-gpu-dmabuf.c
> > @@ -136,7 +136,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;
> >
> > @@ -155,10 +155,24 @@ void virtio_gpu_init_dmabuf(struct
> virtio_gpu_simple_resource *res)
> >           }
> >
> >           if (ret == VFIO_DMABUF_CREATE_ERR_INVALID_IOV) {
> > -            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) {
> 
> Zero represents a success by convention.
> 
> > +                if (vfio_device_mmap_dmabuf(res->iov, res->iov_cnt,
> &pdata,
> > +                                            res->blob_size, &vfio_err)) {
> > +                    error_free(local_err);
> > +                    goto out;
> > +                }
> > +            }
> > +
> > +            if (ret == VFIO_DMABUF_CREATE_ERR_INVALID_IOV) {
> > +                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);
> 
> This reports invalid IOV even when it is actually a valid pointer to
> VFIO and the VFIO is failing. Such an error report does not help and we
> should instead report only the VFIO error.
I thought reporting more errors to the user made sense here in this case.

> 
> And there is another bug with the previous patch where res-
> >dmabuf_fd is
> invalid when virtio_gpu_remap_dmabuf() uses it.
Yeah, I see it. Will fix it in the next version. FYI, this was the only version
I did not test as my test environment was not available.

> 
> Below is my attempt to fix these issues:
It is a different style and structure (compared to mine) but I guess I'll adopt
it in the next version to get things going. I do have one comment/question below.

> 
>      if (res->iov_cnt == 1 &&
>          res->iov[0].iov_len < 4096) {
>          res->dmabuf_fd = -1;
>          pdata = res->iov[0].iov_base;
>      } else {
>          res->dmabuf_fd = virtio_gpu_create_udmabuf(res, &local_err);
>          if (res->dmabuf_fd == VFIO_DMABUF_CREATE_ERR_INVALID_IOV) {
>              error_free_or_abort(local_err);
> 
>              res->dmabuf_fd = vfio_device_create_dmabuf_fd(res->iov,
>                                                            res->iov_cnt,
>                                                            &local_err);
>              if (res->dmabuf_fd ==
> VFIO_DMABUF_CREATE_ERR_INVALID_IOV) {
>                  res->dmabuf_fd = -1;
>                  error_free_or_abort(local_err);
>                  qemu_log_mask(LOG_GUEST_ERROR,
>                                "Cannot create dmabuf: incompatible
> memory\n");
>                  return;
>              }
> 
>              if (res->dmabuf_fd < 0 ||
>                  !vfio_device_mmap_dmabuf(res->iov, res->iov_cnt, &pdata,
>                                           res->blob_size, &local_err)) {
>                  res->dmabuf_fd = -1;
I am wondering here and in the below else if what is the benefit of setting
res->dmabuf_fd to -1 if it is already negative? Also, if res->dmabuf_fd is
valid (non-negative) and mmap fails, shouldn't we call virtio_gpu_destroy_dmabuf()
to close the dmabuf fd?

Thanks,
Vivek

>              }
>          } else if (res->dmabuf_fd < 0 ||
>                     !virtio_gpu_remap_dmabuf(res, &pdata, &local_err)) {
>              res->dmabuf_fd = -1;
>          }
> 
>          if (res->dmabuf_fd < 0) {
>              error_report_err(local_err);
>              return;
>          }
> 
>          res->remapped = pdata;
>      }
> 
> Regards,
> Akihiko Odaki
> 
> >           return;
Re: [PATCH v9 10/10] virtio-gpu-dmabuf: Create dmabuf for blobs associated with VFIO devices
Posted by Akihiko Odaki 1 month ago
On 2026/03/06 16:47, Kasireddy, Vivek wrote:
> Hi Akihiko,
> 
>> Subject: Re: [PATCH v9 10/10] virtio-gpu-dmabuf: Create dmabuf for
>> blobs associated with VFIO devices
>>
>> On 2026/03/05 15:54, 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, for mmapping the dmabuf, we directly call mmap() if the
>>> dmabuf fd was created via virtio_gpu_create_udmabuf() since we
>> know
>>> that the udmabuf driver supports mmap(). However, if the dmabuf
>> was
>>> created via vfio_device_create_dmabuf_fd(), we use the
>>> vfio_device_mmap_dmabuf() API to get a mapping for the dmabuf.
>>>
>>> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
>>> Cc: Alex Bennée <alex.bennee@linaro.org>
>>> Cc: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
>>> Cc: Dmitry Osipenko <dmitry.osipenko@collabora.com>
>>> Cc: Alex Williamson <alex@shazbot.org>
>>> Cc: Cédric Le Goater <clg@redhat.com>
>>> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
>>> ---
>>>    hw/display/virtio-gpu-dmabuf.c | 24 +++++++++++++++++++-----
>>>    1 file changed, 19 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/hw/display/virtio-gpu-dmabuf.c b/hw/display/virtio-gpu-
>> dmabuf.c
>>> index dd64fbbc2d..7f4c399416 100644
>>> --- a/hw/display/virtio-gpu-dmabuf.c
>>> +++ b/hw/display/virtio-gpu-dmabuf.c
>>> @@ -136,7 +136,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;
>>>
>>> @@ -155,10 +155,24 @@ void virtio_gpu_init_dmabuf(struct
>> virtio_gpu_simple_resource *res)
>>>            }
>>>
>>>            if (ret == VFIO_DMABUF_CREATE_ERR_INVALID_IOV) {
>>> -            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) {
>>
>> Zero represents a success by convention.
>>
>>> +                if (vfio_device_mmap_dmabuf(res->iov, res->iov_cnt,
>> &pdata,
>>> +                                            res->blob_size, &vfio_err)) {
>>> +                    error_free(local_err);
>>> +                    goto out;
>>> +                }
>>> +            }
>>> +
>>> +            if (ret == VFIO_DMABUF_CREATE_ERR_INVALID_IOV) {
>>> +                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);
>>
>> This reports invalid IOV even when it is actually a valid pointer to
>> VFIO and the VFIO is failing. Such an error report does not help and we
>> should instead report only the VFIO error.
> I thought reporting more errors to the user made sense here in this case.

In that case we know udmabuf is doing fine but it still reports an 
error, which is misleading.

> 
>>
>> And there is another bug with the previous patch where res-
>>> dmabuf_fd is
>> invalid when virtio_gpu_remap_dmabuf() uses it.
> Yeah, I see it. Will fix it in the next version. FYI, this was the only version
> I did not test as my test environment was not available.
> 
>>
>> Below is my attempt to fix these issues:
> It is a different style and structure (compared to mine) but I guess I'll adopt
> it in the next version to get things going. I do have one comment/question below.
> 
>>
>>       if (res->iov_cnt == 1 &&
>>           res->iov[0].iov_len < 4096) {
>>           res->dmabuf_fd = -1;
>>           pdata = res->iov[0].iov_base;
>>       } else {
>>           res->dmabuf_fd = virtio_gpu_create_udmabuf(res, &local_err);
>>           if (res->dmabuf_fd == VFIO_DMABUF_CREATE_ERR_INVALID_IOV) {
>>               error_free_or_abort(local_err);
>>
>>               res->dmabuf_fd = vfio_device_create_dmabuf_fd(res->iov,
>>                                                             res->iov_cnt,
>>                                                             &local_err);
>>               if (res->dmabuf_fd ==
>> VFIO_DMABUF_CREATE_ERR_INVALID_IOV) {
>>                   res->dmabuf_fd = -1;
>>                   error_free_or_abort(local_err);
>>                   qemu_log_mask(LOG_GUEST_ERROR,
>>                                 "Cannot create dmabuf: incompatible
>> memory\n");
>>                   return;
>>               }
>>
>>               if (res->dmabuf_fd < 0 ||
>>                   !vfio_device_mmap_dmabuf(res->iov, res->iov_cnt, &pdata,
>>                                            res->blob_size, &local_err)) {
>>                   res->dmabuf_fd = -1;
> I am wondering here and in the below else if what is the benefit of setting
> res->dmabuf_fd to -1 if it is already negative? Also, if res->dmabuf_fd is
> valid (non-negative) and mmap fails, shouldn't we call virtio_gpu_destroy_dmabuf()
> to close the dmabuf fd?

Setting -1 is not strictly needed but improves consistency. You are 
right about virtio_gpu_destroy_dmabuf().

Regards,
Akihiko Odaki

> 
> Thanks,
> Vivek
> 
>>               }
>>           } else if (res->dmabuf_fd < 0 ||
>>                      !virtio_gpu_remap_dmabuf(res, &pdata, &local_err)) {
>>               res->dmabuf_fd = -1;
>>           }
>>
>>           if (res->dmabuf_fd < 0) {
>>               error_report_err(local_err);
>>               return;
>>           }
>>
>>           res->remapped = pdata;
>>       }
>>
>> Regards,
>> Akihiko Odaki
>>
>>>            return;
>