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

Vivek Kasireddy posted 10 patches 1 month, 2 weeks ago
Maintainers: "Alex Bennée" <alex.bennee@linaro.org>, Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>, Dmitry Osipenko <dmitry.osipenko@collabora.com>, "Michael S. Tsirkin" <mst@redhat.com>, Alex Williamson <alex@shazbot.org>, "Cédric Le Goater" <clg@redhat.com>
There is a newer version of this series
[PATCH v7 10/10] virtio-gpu-dmabuf: Create dmabuf for blobs associated with VFIO devices
Posted by Vivek Kasireddy 1 month, 2 weeks 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() 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 <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/meson.build         |  1 +
 hw/display/vfio-dmabuf-stubs.c | 23 +++++++++++++++++++++++
 hw/display/virtio-gpu-dmabuf.c | 32 ++++++++++++++++++++++++--------
 3 files changed, 48 insertions(+), 8 deletions(-)
 create mode 100644 hw/display/vfio-dmabuf-stubs.c

diff --git a/hw/display/meson.build b/hw/display/meson.build
index f5f92b1068..7d2bd839cc 100644
--- a/hw/display/meson.build
+++ b/hw/display/meson.build
@@ -70,6 +70,7 @@ if config_all_devices.has_key('CONFIG_VIRTIO_GPU')
                     if_true: [files('virtio-gpu-base.c', 'virtio-gpu.c'), pixman])
   if host_os == 'linux'
     virtio_gpu_ss.add(files('virtio-gpu-dmabuf.c'))
+    virtio_gpu_ss.add(when: 'CONFIG_VFIO', if_false: files('vfio-dmabuf-stubs.c'))
   else
     virtio_gpu_ss.add(files('virtio-gpu-dmabuf-stubs.c'))
   endif
diff --git a/hw/display/vfio-dmabuf-stubs.c b/hw/display/vfio-dmabuf-stubs.c
new file mode 100644
index 0000000000..12f016f0c5
--- /dev/null
+++ b/hw/display/vfio-dmabuf-stubs.c
@@ -0,0 +1,23 @@
+/*
+ * Copyright (c) 2026 Intel and/or its affiliates.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/error-report.h"
+#include "qemu/iov.h"
+
+bool vfio_device_create_dmabuf(struct iovec *iov, unsigned int iov_cnt,
+                               int *fd, Error **errp)
+{
+    error_setg(errp, "VFIO dmabuf support is not enabled");
+    return false;
+}
+
+bool vfio_device_mmap_dmabuf(struct iovec *iov, unsigned int iov_cnt,
+                             void **addr, size_t total_size, Error **errp)
+{
+    error_setg(errp, "VFIO mmap dmabuf support is not enabled");
+    return false;
+}
diff --git a/hw/display/virtio-gpu-dmabuf.c b/hw/display/virtio-gpu-dmabuf.c
index fdcf8e53a2..3f39371de1 100644
--- a/hw/display/virtio-gpu-dmabuf.c
+++ b/hw/display/virtio-gpu-dmabuf.c
@@ -18,6 +18,7 @@
 #include "ui/console.h"
 #include "hw/virtio/virtio-gpu.h"
 #include "hw/virtio/virtio-gpu-pixman.h"
+#include "hw/vfio/vfio-device.h"
 #include "trace.h"
 #include "system/ramblock.h"
 #include "system/hostmem.h"
@@ -82,13 +83,21 @@ virtio_gpu_create_udmabuf(struct virtio_gpu_simple_resource *res, int *fd,
 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)) {
+            error_report_err(mmap_err);
+            return false;
+        }
+        error_free(mmap_err);
+        mmap_err = NULL;
     }
     res->remapped = map;
     return true;
@@ -145,7 +154,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;
     VirtIOGPUErrorType err;
     void *pdata = NULL;
     int fd;
@@ -158,12 +167,19 @@ void virtio_gpu_init_dmabuf(struct virtio_gpu_simple_resource *res)
         err = virtio_gpu_create_udmabuf(res, &fd, &local_err);
         if (err != VIRTIO_GPU_NO_ERROR) {
             error_prepend(&local_err, "Cannot create dmabuf: ");
-            if (err == VIRTIO_GPU_GUEST_ERROR) {
-                qemu_log_mask(LOG_GUEST_ERROR,
-                              "Cannot create dmabuf: incompatible mem\n");
+
+            if (!vfio_device_create_dmabuf(res->iov, res->iov_cnt, &fd,
+                                           &vfio_err)) {
+                if (err == VIRTIO_GPU_GUEST_ERROR) {
+                    qemu_log_mask(LOG_GUEST_ERROR,
+                                  "Cannot create dmabuf: incompatible mem\n");
+                }
+                error_report_err(local_err);
+                error_report_err(vfio_err);
+                return;
             }
-            error_report_err(local_err);
-            return;
+            error_free(local_err);
+            local_err = NULL;
         }
 
         res->dmabuf_fd = fd;
-- 
2.53.0


Re: [PATCH v7 10/10] virtio-gpu-dmabuf: Create dmabuf for blobs associated with VFIO devices
Posted by Akihiko Odaki 1 month, 2 weeks ago
On 2026/02/23 17:00, 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() 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 <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/meson.build         |  1 +
>   hw/display/vfio-dmabuf-stubs.c | 23 +++++++++++++++++++++++
>   hw/display/virtio-gpu-dmabuf.c | 32 ++++++++++++++++++++++++--------
>   3 files changed, 48 insertions(+), 8 deletions(-)
>   create mode 100644 hw/display/vfio-dmabuf-stubs.c
> 
> diff --git a/hw/display/meson.build b/hw/display/meson.build
> index f5f92b1068..7d2bd839cc 100644
> --- a/hw/display/meson.build
> +++ b/hw/display/meson.build
> @@ -70,6 +70,7 @@ if config_all_devices.has_key('CONFIG_VIRTIO_GPU')
>                       if_true: [files('virtio-gpu-base.c', 'virtio-gpu.c'), pixman])
>     if host_os == 'linux'
>       virtio_gpu_ss.add(files('virtio-gpu-dmabuf.c'))
> +    virtio_gpu_ss.add(when: 'CONFIG_VFIO', if_false: files('vfio-dmabuf-stubs.c'))
>     else
>       virtio_gpu_ss.add(files('virtio-gpu-dmabuf-stubs.c'))
>     endif
> diff --git a/hw/display/vfio-dmabuf-stubs.c b/hw/display/vfio-dmabuf-stubs.c
> new file mode 100644
> index 0000000000..12f016f0c5
> --- /dev/null
> +++ b/hw/display/vfio-dmabuf-stubs.c
> @@ -0,0 +1,23 @@
> +/*
> + * Copyright (c) 2026 Intel and/or its affiliates.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/error-report.h"
> +#include "qemu/iov.h"
> +
> +bool vfio_device_create_dmabuf(struct iovec *iov, unsigned int iov_cnt,
> +                               int *fd, Error **errp)
> +{
> +    error_setg(errp, "VFIO dmabuf support is not enabled");
> +    return false;
> +}
> +
> +bool vfio_device_mmap_dmabuf(struct iovec *iov, unsigned int iov_cnt,
> +                             void **addr, size_t total_size, Error **errp)
> +{
> +    error_setg(errp, "VFIO mmap dmabuf support is not enabled");
> +    return false;
> +}

This is VFIO's implementation and shouldn't be here. Follow what
hw/vfio/iommufd-stubs.c, the previously-mentioned example, does.

> diff --git a/hw/display/virtio-gpu-dmabuf.c b/hw/display/virtio-gpu-dmabuf.c
> index fdcf8e53a2..3f39371de1 100644
> --- a/hw/display/virtio-gpu-dmabuf.c
> +++ b/hw/display/virtio-gpu-dmabuf.c
> @@ -18,6 +18,7 @@
>   #include "ui/console.h"
>   #include "hw/virtio/virtio-gpu.h"
>   #include "hw/virtio/virtio-gpu-pixman.h"
> +#include "hw/vfio/vfio-device.h"
>   #include "trace.h"
>   #include "system/ramblock.h"
>   #include "system/hostmem.h"
> @@ -82,13 +83,21 @@ virtio_gpu_create_udmabuf(struct virtio_gpu_simple_resource *res, int *fd,
>   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)) {
> +            error_report_err(mmap_err);
> +            return false;
> +        }
> +        error_free(mmap_err);
> +        mmap_err = NULL;
>       }
>       res->remapped = map;
>       return true;
> @@ -145,7 +154,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;
>       VirtIOGPUErrorType err;
>       void *pdata = NULL;
>       int fd;
> @@ -158,12 +167,19 @@ void virtio_gpu_init_dmabuf(struct virtio_gpu_simple_resource *res)
>           err = virtio_gpu_create_udmabuf(res, &fd, &local_err);
>           if (err != VIRTIO_GPU_NO_ERROR) {
>               error_prepend(&local_err, "Cannot create dmabuf: ");
> -            if (err == VIRTIO_GPU_GUEST_ERROR) {
> -                qemu_log_mask(LOG_GUEST_ERROR,
> -                              "Cannot create dmabuf: incompatible mem\n");
> +
> +            if (!vfio_device_create_dmabuf(res->iov, res->iov_cnt, &fd,
> +                                           &vfio_err)) {
> +                if (err == VIRTIO_GPU_GUEST_ERROR) {
> +                    qemu_log_mask(LOG_GUEST_ERROR,
> +                                  "Cannot create dmabuf: incompatible mem\n");

This logic is faulty. err is not set by vfio_device_create_dmabuf(), so 
it reports a guest error even when the memory is compatible with 
vfio_device_create_dmabuf() and it raised an error for a different reason.

Regards,
Akihiko Odaki

> +                }
> +                error_report_err(local_err);
> +                error_report_err(vfio_err);
> +                return;
>               }
> -            error_report_err(local_err);
> -            return;
> +            error_free(local_err);
> +            local_err = NULL;
>           }
>   
>           res->dmabuf_fd = fd;


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

> Subject: Re: [PATCH v7 10/10] virtio-gpu-dmabuf: Create dmabuf for
> blobs associated with VFIO devices
> 
> On 2026/02/23 17:00, 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() 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 <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/meson.build         |  1 +
> >   hw/display/vfio-dmabuf-stubs.c | 23 +++++++++++++++++++++++
> >   hw/display/virtio-gpu-dmabuf.c | 32 ++++++++++++++++++++++++-----
> ---
> >   3 files changed, 48 insertions(+), 8 deletions(-)
> >   create mode 100644 hw/display/vfio-dmabuf-stubs.c
> >
> > diff --git a/hw/display/meson.build b/hw/display/meson.build
> > index f5f92b1068..7d2bd839cc 100644
> > --- a/hw/display/meson.build
> > +++ b/hw/display/meson.build
> > @@ -70,6 +70,7 @@ if
> config_all_devices.has_key('CONFIG_VIRTIO_GPU')
> >                       if_true: [files('virtio-gpu-base.c', 'virtio-gpu.c'), pixman])
> >     if host_os == 'linux'
> >       virtio_gpu_ss.add(files('virtio-gpu-dmabuf.c'))
> > +    virtio_gpu_ss.add(when: 'CONFIG_VFIO', if_false: files('vfio-
> dmabuf-stubs.c'))
> >     else
> >       virtio_gpu_ss.add(files('virtio-gpu-dmabuf-stubs.c'))
> >     endif
> > diff --git a/hw/display/vfio-dmabuf-stubs.c b/hw/display/vfio-dmabuf-
> stubs.c
> > new file mode 100644
> > index 0000000000..12f016f0c5
> > --- /dev/null
> > +++ b/hw/display/vfio-dmabuf-stubs.c
> > @@ -0,0 +1,23 @@
> > +/*
> > + * Copyright (c) 2026 Intel and/or its affiliates.
> > + *
> > + * SPDX-License-Identifier: GPL-2.0-or-later
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "qemu/error-report.h"
> > +#include "qemu/iov.h"
> > +
> > +bool vfio_device_create_dmabuf(struct iovec *iov, unsigned int
> iov_cnt,
> > +                               int *fd, Error **errp)
> > +{
> > +    error_setg(errp, "VFIO dmabuf support is not enabled");
> > +    return false;
> > +}
> > +
> > +bool vfio_device_mmap_dmabuf(struct iovec *iov, unsigned int
> iov_cnt,
> > +                             void **addr, size_t total_size, Error **errp)
> > +{
> > +    error_setg(errp, "VFIO mmap dmabuf support is not enabled");
> > +    return false;
> > +}
> 
> This is VFIO's implementation and shouldn't be here. Follow what
> hw/vfio/iommufd-stubs.c, the previously-mentioned example, does.
Ok, I'll move this file to hw/vfio directory.

> 
> > diff --git a/hw/display/virtio-gpu-dmabuf.c b/hw/display/virtio-gpu-
> dmabuf.c
> > index fdcf8e53a2..3f39371de1 100644
> > --- a/hw/display/virtio-gpu-dmabuf.c
> > +++ b/hw/display/virtio-gpu-dmabuf.c
> > @@ -18,6 +18,7 @@
> >   #include "ui/console.h"
> >   #include "hw/virtio/virtio-gpu.h"
> >   #include "hw/virtio/virtio-gpu-pixman.h"
> > +#include "hw/vfio/vfio-device.h"
> >   #include "trace.h"
> >   #include "system/ramblock.h"
> >   #include "system/hostmem.h"
> > @@ -82,13 +83,21 @@ virtio_gpu_create_udmabuf(struct
> virtio_gpu_simple_resource *res, int *fd,
> >   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)) {
> > +            error_report_err(mmap_err);
> > +            return false;
> > +        }
> > +        error_free(mmap_err);
> > +        mmap_err = NULL;
> >       }
> >       res->remapped = map;
> >       return true;
> > @@ -145,7 +154,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;
> >       VirtIOGPUErrorType err;
> >       void *pdata = NULL;
> >       int fd;
> > @@ -158,12 +167,19 @@ void virtio_gpu_init_dmabuf(struct
> virtio_gpu_simple_resource *res)
> >           err = virtio_gpu_create_udmabuf(res, &fd, &local_err);
> >           if (err != VIRTIO_GPU_NO_ERROR) {
> >               error_prepend(&local_err, "Cannot create dmabuf: ");
> > -            if (err == VIRTIO_GPU_GUEST_ERROR) {
> > -                qemu_log_mask(LOG_GUEST_ERROR,
> > -                              "Cannot create dmabuf: incompatible mem\n");
> > +
> > +            if (!vfio_device_create_dmabuf(res->iov, res->iov_cnt, &fd,
> > +                                           &vfio_err)) {
> > +                if (err == VIRTIO_GPU_GUEST_ERROR) {
> > +                    qemu_log_mask(LOG_GUEST_ERROR,
> > +                                  "Cannot create dmabuf: incompatible mem\n");
> 
> This logic is faulty. err is not set by vfio_device_create_dmabuf(), so
> it reports a guest error even when the memory is compatible with
> vfio_device_create_dmabuf() and it raised an error for a different
> reason.
Maybe I misunderstood your previous comment about this but I thought
we are going to assume that if both virtio_gpu_create_udmabuf() and
vfio_device_create_dmabuf() fail (assuming Guest error in case of
virtio_gpu_create_udmabuf ()) then it is reasonable to assume that Guest
is at fault for passing in incompatible memory addresses.

However, in order to fix this properly, I think we would have to have
vfio_device_create_dmabuf() return VIRTIO_GPU_GUEST/HOST_ERROR
(or something equivalent) which I was trying to avoid. Do you see any
other proper way to fix this issue without messing with the return type
of vfio_device_create_dmabuf()?

The reason why I am hesitant to make vfio_device_create_dmabuf() return
Guest/Host error is because a future user of this API may not need this info.

Thanks,
Vivek

> 
> Regards,
> Akihiko Odaki
> 
> > +                }
> > +                error_report_err(local_err);
> > +                error_report_err(vfio_err);
> > +                return;
> >               }
> > -            error_report_err(local_err);
> > -            return;
> > +            error_free(local_err);
> > +            local_err = NULL;
> >           }
> >
> >           res->dmabuf_fd = fd;
Re: [PATCH v7 10/10] virtio-gpu-dmabuf: Create dmabuf for blobs associated with VFIO devices
Posted by Akihiko Odaki 1 month, 2 weeks ago
On 2026/02/25 15:03, Kasireddy, Vivek wrote:
> Hi Akihiko,
> 
>> Subject: Re: [PATCH v7 10/10] virtio-gpu-dmabuf: Create dmabuf for
>> blobs associated with VFIO devices
>>
>> On 2026/02/23 17:00, 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() 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 <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/meson.build         |  1 +
>>>    hw/display/vfio-dmabuf-stubs.c | 23 +++++++++++++++++++++++
>>>    hw/display/virtio-gpu-dmabuf.c | 32 ++++++++++++++++++++++++-----
>> ---
>>>    3 files changed, 48 insertions(+), 8 deletions(-)
>>>    create mode 100644 hw/display/vfio-dmabuf-stubs.c
>>>
>>> diff --git a/hw/display/meson.build b/hw/display/meson.build
>>> index f5f92b1068..7d2bd839cc 100644
>>> --- a/hw/display/meson.build
>>> +++ b/hw/display/meson.build
>>> @@ -70,6 +70,7 @@ if
>> config_all_devices.has_key('CONFIG_VIRTIO_GPU')
>>>                        if_true: [files('virtio-gpu-base.c', 'virtio-gpu.c'), pixman])
>>>      if host_os == 'linux'
>>>        virtio_gpu_ss.add(files('virtio-gpu-dmabuf.c'))
>>> +    virtio_gpu_ss.add(when: 'CONFIG_VFIO', if_false: files('vfio-
>> dmabuf-stubs.c'))
>>>      else
>>>        virtio_gpu_ss.add(files('virtio-gpu-dmabuf-stubs.c'))
>>>      endif
>>> diff --git a/hw/display/vfio-dmabuf-stubs.c b/hw/display/vfio-dmabuf-
>> stubs.c
>>> new file mode 100644
>>> index 0000000000..12f016f0c5
>>> --- /dev/null
>>> +++ b/hw/display/vfio-dmabuf-stubs.c
>>> @@ -0,0 +1,23 @@
>>> +/*
>>> + * Copyright (c) 2026 Intel and/or its affiliates.
>>> + *
>>> + * SPDX-License-Identifier: GPL-2.0-or-later
>>> + */
>>> +
>>> +#include "qemu/osdep.h"
>>> +#include "qemu/error-report.h"
>>> +#include "qemu/iov.h"
>>> +
>>> +bool vfio_device_create_dmabuf(struct iovec *iov, unsigned int
>> iov_cnt,
>>> +                               int *fd, Error **errp)
>>> +{
>>> +    error_setg(errp, "VFIO dmabuf support is not enabled");
>>> +    return false;
>>> +}
>>> +
>>> +bool vfio_device_mmap_dmabuf(struct iovec *iov, unsigned int
>> iov_cnt,
>>> +                             void **addr, size_t total_size, Error **errp)
>>> +{
>>> +    error_setg(errp, "VFIO mmap dmabuf support is not enabled");
>>> +    return false;
>>> +}
>>
>> This is VFIO's implementation and shouldn't be here. Follow what
>> hw/vfio/iommufd-stubs.c, the previously-mentioned example, does.
> Ok, I'll move this file to hw/vfio directory.
> 
>>
>>> diff --git a/hw/display/virtio-gpu-dmabuf.c b/hw/display/virtio-gpu-
>> dmabuf.c
>>> index fdcf8e53a2..3f39371de1 100644
>>> --- a/hw/display/virtio-gpu-dmabuf.c
>>> +++ b/hw/display/virtio-gpu-dmabuf.c
>>> @@ -18,6 +18,7 @@
>>>    #include "ui/console.h"
>>>    #include "hw/virtio/virtio-gpu.h"
>>>    #include "hw/virtio/virtio-gpu-pixman.h"
>>> +#include "hw/vfio/vfio-device.h"
>>>    #include "trace.h"
>>>    #include "system/ramblock.h"
>>>    #include "system/hostmem.h"
>>> @@ -82,13 +83,21 @@ virtio_gpu_create_udmabuf(struct
>> virtio_gpu_simple_resource *res, int *fd,
>>>    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)) {
>>> +            error_report_err(mmap_err);
>>> +            return false;
>>> +        }
>>> +        error_free(mmap_err);
>>> +        mmap_err = NULL;
>>>        }
>>>        res->remapped = map;
>>>        return true;
>>> @@ -145,7 +154,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;
>>>        VirtIOGPUErrorType err;
>>>        void *pdata = NULL;
>>>        int fd;
>>> @@ -158,12 +167,19 @@ void virtio_gpu_init_dmabuf(struct
>> virtio_gpu_simple_resource *res)
>>>            err = virtio_gpu_create_udmabuf(res, &fd, &local_err);
>>>            if (err != VIRTIO_GPU_NO_ERROR) {
>>>                error_prepend(&local_err, "Cannot create dmabuf: ");
>>> -            if (err == VIRTIO_GPU_GUEST_ERROR) {
>>> -                qemu_log_mask(LOG_GUEST_ERROR,
>>> -                              "Cannot create dmabuf: incompatible mem\n");
>>> +
>>> +            if (!vfio_device_create_dmabuf(res->iov, res->iov_cnt, &fd,
>>> +                                           &vfio_err)) {
>>> +                if (err == VIRTIO_GPU_GUEST_ERROR) {
>>> +                    qemu_log_mask(LOG_GUEST_ERROR,
>>> +                                  "Cannot create dmabuf: incompatible mem\n");
>>
>> This logic is faulty. err is not set by vfio_device_create_dmabuf(), so
>> it reports a guest error even when the memory is compatible with
>> vfio_device_create_dmabuf() and it raised an error for a different
>> reason.
> Maybe I misunderstood your previous comment about this but I thought
> we are going to assume that if both virtio_gpu_create_udmabuf() and
> vfio_device_create_dmabuf() fail (assuming Guest error in case of
> virtio_gpu_create_udmabuf ()) then it is reasonable to assume that Guest
> is at fault for passing in incompatible memory addresses.

It's not what I suggested. What I suggested is as follows:
 > virtio_gpu_create_udmabuf(res);
 > if (it turned out the memory is incompatible with udmabuf) {
 >     virtio_gpu_create_vfio_dmabuf(res);
 >     if (it turned out the memory is not backed by VFIO) {
 >         qemu_log_mask(LOG_GUEST_ERROR, "incompatible\n");
 >         return;
 >     }
 > }

https://lore.kernel.org/qemu-devel/c7c03384-4815-4032-89bd-de1ef90bb949@rsg.ci.i.u-tokyo.ac.jp/

It is not correct to assume vfio_device_create_dmabuf() is failed due to 
a guest error when virtio_gpu_create_udmabuf(). For example, when mmap() 
fails in vfio_device_mmap_dmabuf(), it is a host error, and reporting a 
guest error in the case is a bug.

> 
> However, in order to fix this properly, I think we would have to have
> vfio_device_create_dmabuf() return VIRTIO_GPU_GUEST/HOST_ERROR
> (or something equivalent) which I was trying to avoid. Do you see any
> other proper way to fix this issue without messing with the return type
> of vfio_device_create_dmabuf()?
> 
> The reason why I am hesitant to make vfio_device_create_dmabuf() return
> Guest/Host error is because a future user of this API may not need this info.

A future user may not need it, but we need it to make this code bug-free.

Regards,
Akihiko Odaki

> 
> Thanks,
> Vivek
> 
>>
>> Regards,
>> Akihiko Odaki
>>
>>> +                }
>>> +                error_report_err(local_err);
>>> +                error_report_err(vfio_err);
>>> +                return;
>>>                }
>>> -            error_report_err(local_err);
>>> -            return;
>>> +            error_free(local_err);
>>> +            local_err = NULL;
>>>            }
>>>
>>>            res->dmabuf_fd = fd;
> 


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

> Subject: Re: [PATCH v7 10/10] virtio-gpu-dmabuf: Create dmabuf for blobs
> associated with VFIO devices
> 
> >>
> >> On 2026/02/23 17:00, 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() 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 <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/meson.build         |  1 +
> >>>    hw/display/vfio-dmabuf-stubs.c | 23 +++++++++++++++++++++++
> >>>    hw/display/virtio-gpu-dmabuf.c | 32 ++++++++++++++++++++++++--
> ---
> >> ---
> >>>    3 files changed, 48 insertions(+), 8 deletions(-)
> >>>    create mode 100644 hw/display/vfio-dmabuf-stubs.c
> >>>
> >>> diff --git a/hw/display/meson.build b/hw/display/meson.build
> >>> index f5f92b1068..7d2bd839cc 100644
> >>> --- a/hw/display/meson.build
> >>> +++ b/hw/display/meson.build
> >>> @@ -70,6 +70,7 @@ if
> >> config_all_devices.has_key('CONFIG_VIRTIO_GPU')
> >>>                        if_true: [files('virtio-gpu-base.c', 'virtio-gpu.c'), pixman])
> >>>      if host_os == 'linux'
> >>>        virtio_gpu_ss.add(files('virtio-gpu-dmabuf.c'))
> >>> +    virtio_gpu_ss.add(when: 'CONFIG_VFIO', if_false: files('vfio-
> >> dmabuf-stubs.c'))
> >>>      else
> >>>        virtio_gpu_ss.add(files('virtio-gpu-dmabuf-stubs.c'))
> >>>      endif
> >>> diff --git a/hw/display/vfio-dmabuf-stubs.c b/hw/display/vfio-
> dmabuf-
> >> stubs.c
> >>> new file mode 100644
> >>> index 0000000000..12f016f0c5
> >>> --- /dev/null
> >>> +++ b/hw/display/vfio-dmabuf-stubs.c
> >>> @@ -0,0 +1,23 @@
> >>> +/*
> >>> + * Copyright (c) 2026 Intel and/or its affiliates.
> >>> + *
> >>> + * SPDX-License-Identifier: GPL-2.0-or-later
> >>> + */
> >>> +
> >>> +#include "qemu/osdep.h"
> >>> +#include "qemu/error-report.h"
> >>> +#include "qemu/iov.h"
> >>> +
> >>> +bool vfio_device_create_dmabuf(struct iovec *iov, unsigned int
> >> iov_cnt,
> >>> +                               int *fd, Error **errp)
> >>> +{
> >>> +    error_setg(errp, "VFIO dmabuf support is not enabled");
> >>> +    return false;
> >>> +}
> >>> +
> >>> +bool vfio_device_mmap_dmabuf(struct iovec *iov, unsigned int
> >> iov_cnt,
> >>> +                             void **addr, size_t total_size, Error **errp)
> >>> +{
> >>> +    error_setg(errp, "VFIO mmap dmabuf support is not enabled");
> >>> +    return false;
> >>> +}
> >>
> >> This is VFIO's implementation and shouldn't be here. Follow what
> >> hw/vfio/iommufd-stubs.c, the previously-mentioned example, does.
> > Ok, I'll move this file to hw/vfio directory.
> >
> >>
> >>> diff --git a/hw/display/virtio-gpu-dmabuf.c b/hw/display/virtio-gpu-
> >> dmabuf.c
> >>> index fdcf8e53a2..3f39371de1 100644
> >>> --- a/hw/display/virtio-gpu-dmabuf.c
> >>> +++ b/hw/display/virtio-gpu-dmabuf.c
> >>> @@ -18,6 +18,7 @@
> >>>    #include "ui/console.h"
> >>>    #include "hw/virtio/virtio-gpu.h"
> >>>    #include "hw/virtio/virtio-gpu-pixman.h"
> >>> +#include "hw/vfio/vfio-device.h"
> >>>    #include "trace.h"
> >>>    #include "system/ramblock.h"
> >>>    #include "system/hostmem.h"
> >>> @@ -82,13 +83,21 @@ virtio_gpu_create_udmabuf(struct
> >> virtio_gpu_simple_resource *res, int *fd,
> >>>    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)) {
> >>> +            error_report_err(mmap_err);
> >>> +            return false;
> >>> +        }
> >>> +        error_free(mmap_err);
> >>> +        mmap_err = NULL;
> >>>        }
> >>>        res->remapped = map;
> >>>        return true;
> >>> @@ -145,7 +154,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;
> >>>        VirtIOGPUErrorType err;
> >>>        void *pdata = NULL;
> >>>        int fd;
> >>> @@ -158,12 +167,19 @@ void virtio_gpu_init_dmabuf(struct
> >> virtio_gpu_simple_resource *res)
> >>>            err = virtio_gpu_create_udmabuf(res, &fd, &local_err);
> >>>            if (err != VIRTIO_GPU_NO_ERROR) {
> >>>                error_prepend(&local_err, "Cannot create dmabuf: ");
> >>> -            if (err == VIRTIO_GPU_GUEST_ERROR) {
> >>> -                qemu_log_mask(LOG_GUEST_ERROR,
> >>> -                              "Cannot create dmabuf: incompatible mem\n");
> >>> +
> >>> +            if (!vfio_device_create_dmabuf(res->iov, res->iov_cnt, &fd,
> >>> +                                           &vfio_err)) {
> >>> +                if (err == VIRTIO_GPU_GUEST_ERROR) {
> >>> +                    qemu_log_mask(LOG_GUEST_ERROR,
> >>> +                                  "Cannot create dmabuf: incompatible mem\n");
> >>
> >> This logic is faulty. err is not set by vfio_device_create_dmabuf(), so
> >> it reports a guest error even when the memory is compatible with
> >> vfio_device_create_dmabuf() and it raised an error for a different
> >> reason.
> > Maybe I misunderstood your previous comment about this but I
> thought
> > we are going to assume that if both virtio_gpu_create_udmabuf() and
> > vfio_device_create_dmabuf() fail (assuming Guest error in case of
> > virtio_gpu_create_udmabuf ()) then it is reasonable to assume that
> Guest
> > is at fault for passing in incompatible memory addresses.
> 
> It's not what I suggested. What I suggested is as follows:
>  > virtio_gpu_create_udmabuf(res);
>  > if (it turned out the memory is incompatible with udmabuf) {
>  >     virtio_gpu_create_vfio_dmabuf(res);
>  >     if (it turned out the memory is not backed by VFIO) {
>  >         qemu_log_mask(LOG_GUEST_ERROR, "incompatible\n");
>  >         return;
>  >     }
>  > }
> 
> https://lore.kernel.org/qemu-devel/c7c03384-4815-4032-89bd-
> de1ef90bb949@rsg.ci.i.u-tokyo.ac.jp/
> 
> It is not correct to assume vfio_device_create_dmabuf() is failed due to
> a guest error when virtio_gpu_create_udmabuf(). For example, when
> mmap()
> fails in vfio_device_mmap_dmabuf(), it is a host error, and reporting a
> guest error in the case is a bug.
> 
> >
> > However, in order to fix this properly, I think we would have to have
> > vfio_device_create_dmabuf() return VIRTIO_GPU_GUEST/HOST_ERROR
> > (or something equivalent) which I was trying to avoid. Do you see any
> > other proper way to fix this issue without messing with the return type
> > of vfio_device_create_dmabuf()?
> >
> > The reason why I am hesitant to make vfio_device_create_dmabuf()
> return
> > Guest/Host error is because a future user of this API may not need this
> info.
> 
> A future user may not need it, but we need it to make this code bug-free.
Ok, I plan to add the following enum and make it available to VFIO and
virtio-gpu-dmabuf.c:
typedef enum DmabufCreateErrorType {
    DMABUF_CREATE_NO_ERROR = 1,
    /* Guest is responsible for this error */
    DMABUF_CREATE_GUEST_ERROR = -1,
    /* Host is at fault for this error */
    DMABUF_CREATE_HOST_ERROR = -2,
} DmabufCreateErrorType;

Is the naming OK? And, what would be a good location (header) to have this in?

Thanks,
Vivek

> 
> Regards,
> Akihiko Odaki
> 
> >
> > Thanks,
> > Vivek
> >
> >>
> >> Regards,
> >> Akihiko Odaki
> >>
> >>> +                }
> >>> +                error_report_err(local_err);
> >>> +                error_report_err(vfio_err);
> >>> +                return;
> >>>                }
> >>> -            error_report_err(local_err);
> >>> -            return;
> >>> +            error_free(local_err);
> >>> +            local_err = NULL;
> >>>            }
> >>>
> >>>            res->dmabuf_fd = fd;
> >
Re: [PATCH v7 10/10] virtio-gpu-dmabuf: Create dmabuf for blobs associated with VFIO devices
Posted by Akihiko Odaki 1 month, 2 weeks ago
On 2026/02/26 15:22, Kasireddy, Vivek wrote:
> Hi Akihiko,
> 
>> Subject: Re: [PATCH v7 10/10] virtio-gpu-dmabuf: Create dmabuf for blobs
>> associated with VFIO devices
>>
>>>>
>>>> On 2026/02/23 17:00, 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() 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 <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/meson.build         |  1 +
>>>>>     hw/display/vfio-dmabuf-stubs.c | 23 +++++++++++++++++++++++
>>>>>     hw/display/virtio-gpu-dmabuf.c | 32 ++++++++++++++++++++++++--
>> ---
>>>> ---
>>>>>     3 files changed, 48 insertions(+), 8 deletions(-)
>>>>>     create mode 100644 hw/display/vfio-dmabuf-stubs.c
>>>>>
>>>>> diff --git a/hw/display/meson.build b/hw/display/meson.build
>>>>> index f5f92b1068..7d2bd839cc 100644
>>>>> --- a/hw/display/meson.build
>>>>> +++ b/hw/display/meson.build
>>>>> @@ -70,6 +70,7 @@ if
>>>> config_all_devices.has_key('CONFIG_VIRTIO_GPU')
>>>>>                         if_true: [files('virtio-gpu-base.c', 'virtio-gpu.c'), pixman])
>>>>>       if host_os == 'linux'
>>>>>         virtio_gpu_ss.add(files('virtio-gpu-dmabuf.c'))
>>>>> +    virtio_gpu_ss.add(when: 'CONFIG_VFIO', if_false: files('vfio-
>>>> dmabuf-stubs.c'))
>>>>>       else
>>>>>         virtio_gpu_ss.add(files('virtio-gpu-dmabuf-stubs.c'))
>>>>>       endif
>>>>> diff --git a/hw/display/vfio-dmabuf-stubs.c b/hw/display/vfio-
>> dmabuf-
>>>> stubs.c
>>>>> new file mode 100644
>>>>> index 0000000000..12f016f0c5
>>>>> --- /dev/null
>>>>> +++ b/hw/display/vfio-dmabuf-stubs.c
>>>>> @@ -0,0 +1,23 @@
>>>>> +/*
>>>>> + * Copyright (c) 2026 Intel and/or its affiliates.
>>>>> + *
>>>>> + * SPDX-License-Identifier: GPL-2.0-or-later
>>>>> + */
>>>>> +
>>>>> +#include "qemu/osdep.h"
>>>>> +#include "qemu/error-report.h"
>>>>> +#include "qemu/iov.h"
>>>>> +
>>>>> +bool vfio_device_create_dmabuf(struct iovec *iov, unsigned int
>>>> iov_cnt,
>>>>> +                               int *fd, Error **errp)
>>>>> +{
>>>>> +    error_setg(errp, "VFIO dmabuf support is not enabled");
>>>>> +    return false;
>>>>> +}
>>>>> +
>>>>> +bool vfio_device_mmap_dmabuf(struct iovec *iov, unsigned int
>>>> iov_cnt,
>>>>> +                             void **addr, size_t total_size, Error **errp)
>>>>> +{
>>>>> +    error_setg(errp, "VFIO mmap dmabuf support is not enabled");
>>>>> +    return false;
>>>>> +}
>>>>
>>>> This is VFIO's implementation and shouldn't be here. Follow what
>>>> hw/vfio/iommufd-stubs.c, the previously-mentioned example, does.
>>> Ok, I'll move this file to hw/vfio directory.
>>>
>>>>
>>>>> diff --git a/hw/display/virtio-gpu-dmabuf.c b/hw/display/virtio-gpu-
>>>> dmabuf.c
>>>>> index fdcf8e53a2..3f39371de1 100644
>>>>> --- a/hw/display/virtio-gpu-dmabuf.c
>>>>> +++ b/hw/display/virtio-gpu-dmabuf.c
>>>>> @@ -18,6 +18,7 @@
>>>>>     #include "ui/console.h"
>>>>>     #include "hw/virtio/virtio-gpu.h"
>>>>>     #include "hw/virtio/virtio-gpu-pixman.h"
>>>>> +#include "hw/vfio/vfio-device.h"
>>>>>     #include "trace.h"
>>>>>     #include "system/ramblock.h"
>>>>>     #include "system/hostmem.h"
>>>>> @@ -82,13 +83,21 @@ virtio_gpu_create_udmabuf(struct
>>>> virtio_gpu_simple_resource *res, int *fd,
>>>>>     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)) {
>>>>> +            error_report_err(mmap_err);
>>>>> +            return false;
>>>>> +        }
>>>>> +        error_free(mmap_err);
>>>>> +        mmap_err = NULL;
>>>>>         }
>>>>>         res->remapped = map;
>>>>>         return true;
>>>>> @@ -145,7 +154,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;
>>>>>         VirtIOGPUErrorType err;
>>>>>         void *pdata = NULL;
>>>>>         int fd;
>>>>> @@ -158,12 +167,19 @@ void virtio_gpu_init_dmabuf(struct
>>>> virtio_gpu_simple_resource *res)
>>>>>             err = virtio_gpu_create_udmabuf(res, &fd, &local_err);
>>>>>             if (err != VIRTIO_GPU_NO_ERROR) {
>>>>>                 error_prepend(&local_err, "Cannot create dmabuf: ");
>>>>> -            if (err == VIRTIO_GPU_GUEST_ERROR) {
>>>>> -                qemu_log_mask(LOG_GUEST_ERROR,
>>>>> -                              "Cannot create dmabuf: incompatible mem\n");
>>>>> +
>>>>> +            if (!vfio_device_create_dmabuf(res->iov, res->iov_cnt, &fd,
>>>>> +                                           &vfio_err)) {
>>>>> +                if (err == VIRTIO_GPU_GUEST_ERROR) {
>>>>> +                    qemu_log_mask(LOG_GUEST_ERROR,
>>>>> +                                  "Cannot create dmabuf: incompatible mem\n");
>>>>
>>>> This logic is faulty. err is not set by vfio_device_create_dmabuf(), so
>>>> it reports a guest error even when the memory is compatible with
>>>> vfio_device_create_dmabuf() and it raised an error for a different
>>>> reason.
>>> Maybe I misunderstood your previous comment about this but I
>> thought
>>> we are going to assume that if both virtio_gpu_create_udmabuf() and
>>> vfio_device_create_dmabuf() fail (assuming Guest error in case of
>>> virtio_gpu_create_udmabuf ()) then it is reasonable to assume that
>> Guest
>>> is at fault for passing in incompatible memory addresses.
>>
>> It's not what I suggested. What I suggested is as follows:
>>   > virtio_gpu_create_udmabuf(res);
>>   > if (it turned out the memory is incompatible with udmabuf) {
>>   >     virtio_gpu_create_vfio_dmabuf(res);
>>   >     if (it turned out the memory is not backed by VFIO) {
>>   >         qemu_log_mask(LOG_GUEST_ERROR, "incompatible\n");
>>   >         return;
>>   >     }
>>   > }
>>
>> https://lore.kernel.org/qemu-devel/c7c03384-4815-4032-89bd-
>> de1ef90bb949@rsg.ci.i.u-tokyo.ac.jp/
>>
>> It is not correct to assume vfio_device_create_dmabuf() is failed due to
>> a guest error when virtio_gpu_create_udmabuf(). For example, when
>> mmap()
>> fails in vfio_device_mmap_dmabuf(), it is a host error, and reporting a
>> guest error in the case is a bug.
>>
>>>
>>> However, in order to fix this properly, I think we would have to have
>>> vfio_device_create_dmabuf() return VIRTIO_GPU_GUEST/HOST_ERROR
>>> (or something equivalent) which I was trying to avoid. Do you see any
>>> other proper way to fix this issue without messing with the return type
>>> of vfio_device_create_dmabuf()?
>>>
>>> The reason why I am hesitant to make vfio_device_create_dmabuf()
>> return
>>> Guest/Host error is because a future user of this API may not need this
>> info.
>>
>> A future user may not need it, but we need it to make this code bug-free.
> Ok, I plan to add the following enum and make it available to VFIO and
> virtio-gpu-dmabuf.c:
> typedef enum DmabufCreateErrorType {
>      DMABUF_CREATE_NO_ERROR = 1,

When there is no error, vfio_device_create_dmabuf() should return the 
file descriptor instead (see e.g., qemu_open()).

>      /* Guest is responsible for this error */
>      DMABUF_CREATE_GUEST_ERROR = -1,
>      /* Host is at fault for this error */
>      DMABUF_CREATE_HOST_ERROR = -2,
> } DmabufCreateErrorType;
> 
> Is the naming OK? And, what would be a good location (header) to have this in?
I would prefix the names VFIO and put it into
include/hw/vfio/vfio-device.h.

In that case, using it for udmabuf is cheating, but I think it's fine 
since it's locally-contained in virtio-gpu-dmabuf.c, its intent is still 
clear, and it has no adverse effect for users (at least there will be no 
bug).

Regards,
Akihiko Odaki

> 
> Thanks,
> Vivek
> 
>>
>> Regards,
>> Akihiko Odaki
>>
>>>
>>> Thanks,
>>> Vivek
>>>
>>>>
>>>> Regards,
>>>> Akihiko Odaki
>>>>
>>>>> +                }
>>>>> +                error_report_err(local_err);
>>>>> +                error_report_err(vfio_err);
>>>>> +                return;
>>>>>                 }
>>>>> -            error_report_err(local_err);
>>>>> -            return;
>>>>> +            error_free(local_err);
>>>>> +            local_err = NULL;
>>>>>             }
>>>>>
>>>>>             res->dmabuf_fd = fd;
>>>
> 


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

> Subject: Re: [PATCH v7 10/10] virtio-gpu-dmabuf: Create dmabuf for
> blobs associated with VFIO devices
> >>>>
> >>>> On 2026/02/23 17:00, 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() 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 <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/meson.build         |  1 +
> >>>>>     hw/display/vfio-dmabuf-stubs.c | 23
> +++++++++++++++++++++++
> >>>>>     hw/display/virtio-gpu-dmabuf.c | 32
> ++++++++++++++++++++++++--
> >> ---
> >>>> ---
> >>>>>     3 files changed, 48 insertions(+), 8 deletions(-)
> >>>>>     create mode 100644 hw/display/vfio-dmabuf-stubs.c
> >>>>>
> >>>>> diff --git a/hw/display/meson.build b/hw/display/meson.build
> >>>>> index f5f92b1068..7d2bd839cc 100644
> >>>>> --- a/hw/display/meson.build
> >>>>> +++ b/hw/display/meson.build
> >>>>> @@ -70,6 +70,7 @@ if
> >>>> config_all_devices.has_key('CONFIG_VIRTIO_GPU')
> >>>>>                         if_true: [files('virtio-gpu-base.c', 'virtio-gpu.c'),
> pixman])
> >>>>>       if host_os == 'linux'
> >>>>>         virtio_gpu_ss.add(files('virtio-gpu-dmabuf.c'))
> >>>>> +    virtio_gpu_ss.add(when: 'CONFIG_VFIO', if_false: files('vfio-
> >>>> dmabuf-stubs.c'))
> >>>>>       else
> >>>>>         virtio_gpu_ss.add(files('virtio-gpu-dmabuf-stubs.c'))
> >>>>>       endif
> >>>>> diff --git a/hw/display/vfio-dmabuf-stubs.c b/hw/display/vfio-
> >> dmabuf-
> >>>> stubs.c
> >>>>> new file mode 100644
> >>>>> index 0000000000..12f016f0c5
> >>>>> --- /dev/null
> >>>>> +++ b/hw/display/vfio-dmabuf-stubs.c
> >>>>> @@ -0,0 +1,23 @@
> >>>>> +/*
> >>>>> + * Copyright (c) 2026 Intel and/or its affiliates.
> >>>>> + *
> >>>>> + * SPDX-License-Identifier: GPL-2.0-or-later
> >>>>> + */
> >>>>> +
> >>>>> +#include "qemu/osdep.h"
> >>>>> +#include "qemu/error-report.h"
> >>>>> +#include "qemu/iov.h"
> >>>>> +
> >>>>> +bool vfio_device_create_dmabuf(struct iovec *iov, unsigned int
> >>>> iov_cnt,
> >>>>> +                               int *fd, Error **errp)
> >>>>> +{
> >>>>> +    error_setg(errp, "VFIO dmabuf support is not enabled");
> >>>>> +    return false;
> >>>>> +}
> >>>>> +
> >>>>> +bool vfio_device_mmap_dmabuf(struct iovec *iov, unsigned int
> >>>> iov_cnt,
> >>>>> +                             void **addr, size_t total_size, Error **errp)
> >>>>> +{
> >>>>> +    error_setg(errp, "VFIO mmap dmabuf support is not
> enabled");
> >>>>> +    return false;
> >>>>> +}
> >>>>
> >>>> This is VFIO's implementation and shouldn't be here. Follow what
> >>>> hw/vfio/iommufd-stubs.c, the previously-mentioned example,
> does.
> >>> Ok, I'll move this file to hw/vfio directory.
> >>>
> >>>>
> >>>>> diff --git a/hw/display/virtio-gpu-dmabuf.c b/hw/display/virtio-
> gpu-
> >>>> dmabuf.c
> >>>>> index fdcf8e53a2..3f39371de1 100644
> >>>>> --- a/hw/display/virtio-gpu-dmabuf.c
> >>>>> +++ b/hw/display/virtio-gpu-dmabuf.c
> >>>>> @@ -18,6 +18,7 @@
> >>>>>     #include "ui/console.h"
> >>>>>     #include "hw/virtio/virtio-gpu.h"
> >>>>>     #include "hw/virtio/virtio-gpu-pixman.h"
> >>>>> +#include "hw/vfio/vfio-device.h"
> >>>>>     #include "trace.h"
> >>>>>     #include "system/ramblock.h"
> >>>>>     #include "system/hostmem.h"
> >>>>> @@ -82,13 +83,21 @@ virtio_gpu_create_udmabuf(struct
> >>>> virtio_gpu_simple_resource *res, int *fd,
> >>>>>     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)) {
> >>>>> +            error_report_err(mmap_err);
> >>>>> +            return false;
> >>>>> +        }
> >>>>> +        error_free(mmap_err);
> >>>>> +        mmap_err = NULL;
> >>>>>         }
> >>>>>         res->remapped = map;
> >>>>>         return true;
> >>>>> @@ -145,7 +154,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;
> >>>>>         VirtIOGPUErrorType err;
> >>>>>         void *pdata = NULL;
> >>>>>         int fd;
> >>>>> @@ -158,12 +167,19 @@ void virtio_gpu_init_dmabuf(struct
> >>>> virtio_gpu_simple_resource *res)
> >>>>>             err = virtio_gpu_create_udmabuf(res, &fd, &local_err);
> >>>>>             if (err != VIRTIO_GPU_NO_ERROR) {
> >>>>>                 error_prepend(&local_err, "Cannot create dmabuf: ");
> >>>>> -            if (err == VIRTIO_GPU_GUEST_ERROR) {
> >>>>> -                qemu_log_mask(LOG_GUEST_ERROR,
> >>>>> -                              "Cannot create dmabuf: incompatible
> mem\n");
> >>>>> +
> >>>>> +            if (!vfio_device_create_dmabuf(res->iov, res->iov_cnt,
> &fd,
> >>>>> +                                           &vfio_err)) {
> >>>>> +                if (err == VIRTIO_GPU_GUEST_ERROR) {
> >>>>> +                    qemu_log_mask(LOG_GUEST_ERROR,
> >>>>> +                                  "Cannot create dmabuf: incompatible
> mem\n");
> >>>>
> >>>> This logic is faulty. err is not set by vfio_device_create_dmabuf(),
> so
> >>>> it reports a guest error even when the memory is compatible with
> >>>> vfio_device_create_dmabuf() and it raised an error for a different
> >>>> reason.
> >>> Maybe I misunderstood your previous comment about this but I
> >> thought
> >>> we are going to assume that if both virtio_gpu_create_udmabuf()
> and
> >>> vfio_device_create_dmabuf() fail (assuming Guest error in case of
> >>> virtio_gpu_create_udmabuf ()) then it is reasonable to assume that
> >> Guest
> >>> is at fault for passing in incompatible memory addresses.
> >>
> >> It's not what I suggested. What I suggested is as follows:
> >>   > virtio_gpu_create_udmabuf(res);
> >>   > if (it turned out the memory is incompatible with udmabuf) {
> >>   >     virtio_gpu_create_vfio_dmabuf(res);
> >>   >     if (it turned out the memory is not backed by VFIO) {
> >>   >         qemu_log_mask(LOG_GUEST_ERROR, "incompatible\n");
> >>   >         return;
> >>   >     }
> >>   > }
> >>
> >> https://lore.kernel.org/qemu-devel/c7c03384-4815-4032-89bd-
> >> de1ef90bb949@rsg.ci.i.u-tokyo.ac.jp/
> >>
> >> It is not correct to assume vfio_device_create_dmabuf() is failed due
> to
> >> a guest error when virtio_gpu_create_udmabuf(). For example, when
> >> mmap()
> >> fails in vfio_device_mmap_dmabuf(), it is a host error, and reporting
> a
> >> guest error in the case is a bug.
> >>
> >>>
> >>> However, in order to fix this properly, I think we would have to
> have
> >>> vfio_device_create_dmabuf() return
> VIRTIO_GPU_GUEST/HOST_ERROR
> >>> (or something equivalent) which I was trying to avoid. Do you see
> any
> >>> other proper way to fix this issue without messing with the return
> type
> >>> of vfio_device_create_dmabuf()?
> >>>
> >>> The reason why I am hesitant to make vfio_device_create_dmabuf()
> >> return
> >>> Guest/Host error is because a future user of this API may not need
> this
> >> info.
> >>
> >> A future user may not need it, but we need it to make this code bug-
> free.
> > Ok, I plan to add the following enum and make it available to VFIO
> and
> > virtio-gpu-dmabuf.c:
> > typedef enum DmabufCreateErrorType {
> >      DMABUF_CREATE_NO_ERROR = 1,
> 
> When there is no error, vfio_device_create_dmabuf() should return the
> file descriptor instead (see e.g., qemu_open()).
Ok, I'll remove DMABUF_CREATE_NO_ERROR from this enum and follow
your suggestion to return fd in case of no error.

> 
> >      /* Guest is responsible for this error */
> >      DMABUF_CREATE_GUEST_ERROR = -1,
> >      /* Host is at fault for this error */
> >      DMABUF_CREATE_HOST_ERROR = -2,
> > } DmabufCreateErrorType;
> >
> > Is the naming OK? And, what would be a good location (header) to
> have this in?
> I would prefix the names VFIO and put it into
> include/hw/vfio/vfio-device.h.
> 
> In that case, using it for udmabuf is cheating, but I think it's fine
> since it's locally-contained in virtio-gpu-dmabuf.c, its intent is still
> clear, and it has no adverse effect for users (at least there will be no
> bug).
Ok, got it. I'll rename and move the enum to vfio-device.h.

Thanks,
Vivek

> 
> Regards,
> Akihiko Odaki
> 
> >
> > Thanks,
> > Vivek
> >
> >>
> >> Regards,
> >> Akihiko Odaki
> >>
> >>>
> >>> Thanks,
> >>> Vivek
> >>>
> >>>>
> >>>> Regards,
> >>>> Akihiko Odaki
> >>>>
> >>>>> +                }
> >>>>> +                error_report_err(local_err);
> >>>>> +                error_report_err(vfio_err);
> >>>>> +                return;
> >>>>>                 }
> >>>>> -            error_report_err(local_err);
> >>>>> -            return;
> >>>>> +            error_free(local_err);
> >>>>> +            local_err = NULL;
> >>>>>             }
> >>>>>
> >>>>>             res->dmabuf_fd = fd;
> >>>
> >