[PATCH v12 00/10] vfio: Create dmabuf from multiple VFIO ranges and use it in virtio-gpu

[PATCH v12 00/10] vfio: Create dmabuf from multiple VFIO ranges and use it in virtio-gpu

[Vivek Kasireddy]

The virtio-gpu driver running in the Guest VM can create Guest blob
resources (by importing dmabufs) that are backed by System RAM. This
is made possible by making use of memfd memory backend and udmabuf
driver on the Host side. However, in order to create Guest blobs
that are backed by vfio-pci device regions (which happens when
virtio-gpu imports dmabufs from devices that have local memory such
as dGPU VFs), we have to add support for creating dmabuf from multiple
VFIO device ranges (same region) and leverage it in virtio-gpu.

So, while creating the blobs we first try to create a dmabuf by
calling into the udmabuf driver assuming that the backing memory 
is memfd based. If that fails, and if it is determined that the blob
is backed by a vfio-pci device region, we would then call into the
vfio-pci driver to have a dmabuf fd created.

Changelog:
v11 -> v12:
- Collect R-b tags from Akihiko
- Rebase and test on top of Qemu master
- Have the helper return VFIO region instead of index (Cedric)
- Introduce a helper to retrieve VFIO region from a MR (Cedric)
- Document the helpers introduced in vfio/device.c (Cedric)
- Verify that the total size of IOV entries is equal to the size
  of the dmabuf (Cedric)

v10 -> v11:
- Have virtio_gpu_remap_dmabuf() and vfio_device_mmap_dmabuf()
  return the mapped pointer on success and NULL on error (Akihiko)

v9 -> v10:
- Adopt the logic structure and style recommended by Akihiko in
  virtio_gpu_init_dmabuf() while creating and mmaping dmabuf
- Destroy the dmabuf by calling virtio_gpu_destroy_dmabuf() if
  mmap fails
- Set res->dmabuf_fd to -1 after closing it to improve consistency

v8 -> v9:
- Replace VFIO_DMABUF_CREATE_GUEST_ERROR/VFIO_DMABUF_CREATE_HOST_ERROR
  with VFIO_DMABUF_CREATE_ERR_INVALID_IOV/VFIO_DMABUF_CREATE_ERR_UNSPEC
  (Akihiko)
- Drop the error_prepend() in virtio_gpu_init_dmabuf() (Akihiko)
- Improve the logic structure in virtio_gpu_init_dmabuf() by calling
  the map memory function immediately after creating the dmabuf
  in both memfd/udmabuf and VFIO cases (Akihiko)

v7 -> v8:
- Move the error enum to vfio and include only negative values (Akihiko)
- Reuse the same VFIO err enum to categorize Guest/Host errors in
  virtio_gpu_create_udmabuf() (Akihiko)
- Make the error messages more descriptive and readable (Akihiko)
- Do not print any error messages on Guest triggered errors (Akihiko)
- Move the vfio dmabuf stubs file to hw/vfio directory (Akihiko)
- Make vfio_device_create_dmabuf() return an fd on success (Akihiko)
- Rename vfio_device_create_dmabuf to vfio_device_create_dmabuf_fd
  since it now returns an fd on success

v6 -> v7:
- Introduce an enum to categorize Guest and Host errors (Akihiko)
- Drop the CONFIG_VIRTIO_GPU_VFIO_BLOB Kconfig option and add stubs
  for VFIO dmabuf helpers when CONFIG_VFIO is not enabled (Akihiko)
- Check the correct return value of vfio_device_get_region_info()
  in vfio_device_mmap_dmabuf()

v5 -> v6:
- Drop the additional error_setg() call in virtio_gpu_create_udmabuf()
  and only have LOG_GUEST_ERROR for invalid ramblocks (Akihiko)
- Merge the patch that introduces vfio_device_lookup() helper with the
  one that adds support for vfio_device_create_dmabuf() (Cedric)
- Use only one 'Error **' parameter instead of two while creating
  and mmapping the dmabuf (Cedric)
- Collect R-b tags from Akihiko

v4 -> v5:
- Improve commit message for the patch that replaces dma_memory_map()
  with virtio_gpu_dma_memory_map() (Akihiko)
- Create separate patches to remove rcu_read_lock/unlock and use
  g_autofree for list pointer in virtio_gpu_create_udmabuf()
  (Akihiko, Cedric)
- Create a separate patch to improve error handling (Cedric)
- Make vfio_device_lookup() a common helper local to vfio/device.c
  (Cedric, Akihiko)
- Move the dmabuf mmap helper to vfio/device.c (Akihiko)
- Replace PROT_READ with PROT_NONE and vbasedev->fd with -1 while
  creating the placeholder mapping in dmabuf mmap helper (Akihiko)

v3 -> v4:
- Drop the patch that introduces ram_block_is_memfd_backed()
- Drop the additional check in virtio_gpu_create_udmabuf() that
  checks for memfd backed ram blocks (Akihiko)
- Remove rcu_read_lock/unlock and replace warn_report() with
  qemu_log_mask() in virtio_gpu_create_udmabuf()
- Improve the commit message and logic in the patch that adds
  support for vfio_device_create_dmabuf_fd()
- Rebase on top of vfio-next

v2 -> v3:
- Use memory_region_get_ram_ptr() to obtain hva for both memfd and
  VFIO backed memory regions (Akihiko)
- Drop the patch that introduced qemu_iovec_same_memory_regions()
  helper and move the "same memory region" check into
  virtio_gpu_create_udmabuf(), vfio_device_create_dmabuf_fd() (Akihiko)
- Refactor virtio_gpu_init_dmabuf() to not rely on helpers such as
  memory_region_is_ram_device() to identify memory regions and
  instead call virtio_gpu_create_udmabuf() unconditionally (Akihiko)
- Add a patch to introduce ram_block_is_memfd_backed() helper

v1 -> v2:
- Drop the patch that uses res->blob_size instead of res->blob to
  identify blob resources (Akihiko)
- Remove the res->dmabuf_fd < 0 check while attaching backing to a
  resource (Akihiko)
- Remove cmd->cmd_hdr.type != VIRTIO_GPU_CMD_RESOURCE_CREATE_BLOB
  check while attaching backing (Akihiko)
- Improve vfio_get_region_index_from_mr() and add documentation (Cedric)
- Remove rcu_read_lock/unlock around qemu_ram_block_from_host()
  (Akihiko, Cedric)
- Improve, document and rename vfio_device_create_dmabuf() to
  vfio_device_create_dmabuf_fd() (Cedric)
- Add a new helper to lookup VFIO device from memory region (Cedric)
- Document vfio_device_get_region_info() (Cedric)
- Ensure that info variable (in vfio_dmabuf_mmap()) is initialized
  before use (Cedric)
- Rename udmabuf files and helpers to dmabuf (Akihiko)
- Remove the redundant check for virtio_gpu_have_udmabuf() in
  virtio_gpu_init_dmabuf() (Akihiko)
- Add a helper to check whether all the entries of a dmabuf belong
  to a single memory region or not (Akihiko)

RFC -> v1:
- Create the CPU mapping using vfio device fd if the dmabuf exporter
  (vfio-pci) does not provide mmap() support (Akihiko)
- Log a warning with LOG_GUEST_ERROR instead of warn_report() when
  dmabuf cannot be created using Guest provided addresses (Akihiko)
- Use address_space_translate() instead of gpa2hva() to obtain the
  Host addresses (Akihiko)
- Rearrange the patches and improve the commit messages (Akihiko)
- Fix compilation error when VFIO is not enabled (Alex)
- Add a new helper to obtain VFIO region index from memory region
- Move vfio_device_create_dmabuf() to hw/vfio/device.c

Tested with an SRIOV enabled Intel dGPU (B60) by running Gnome Wayland
(in the VM) and Qemu with the following (relevant) parameters:
-device vfio-pci,host=0000:03:00.1
-device virtio-vga,max_outputs=1,xres=1920,yres=1080,blob=true
-display gtk,gl=on

Associated virtio-gpu kernel driver series (merged):
https://lore.kernel.org/dri-devel/20241126031643.3490496-1-vivek.kasireddy@intel.com/

---
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>
Cc: Dongwon Kim <dongwon.kim@intel.com>

Vivek Kasireddy (10):
  virtio-gpu: Recreate the resource's dmabuf if new backing is attached
  virtio-gpu: Find hva for Guest's DMA addr associated with a ram device
  virtio-gpu: Rename udmabuf files and helpers to dmabuf
  virtio-gpu-dmabuf: Remove rcu_read_lock/unlock from
    virtio_gpu_create_udmabuf()
  virtio-gpu-dmabuf: Use g_autofree for the list pointer
  vfio/region: Add a helper to get VFIO region from memory region
  vfio/device: Add support for creating dmabuf from multiple ranges
  vfio/device: Add a helper to mmap a dmabuf
  virtio-gpu-dmabuf: Improve error handling with 'Error **' and err enum
  virtio-gpu-dmabuf: Create dmabuf for blobs associated with VFIO
    devices

 hw/vfio/vfio-region.h                         |  10 +
 include/hw/vfio/vfio-device.h                 |  39 ++++
 include/hw/virtio/virtio-gpu.h                |   6 +-
 ...abuf-stubs.c => virtio-gpu-dmabuf-stubs.c} |   4 +-
 ...rtio-gpu-udmabuf.c => virtio-gpu-dmabuf.c} |  99 ++++++----
 hw/display/virtio-gpu.c                       |  31 +++-
 hw/vfio/device.c                              | 173 ++++++++++++++++++
 hw/vfio/dmabuf-stubs.c                        |  24 +++
 hw/vfio/region.c                              |  11 ++
 hw/display/meson.build                        |   4 +-
 hw/vfio/meson.build                           |   1 +
 11 files changed, 357 insertions(+), 45 deletions(-)
 rename hw/display/{virtio-gpu-udmabuf-stubs.c => virtio-gpu-dmabuf-stubs.c} (77%)
 rename hw/display/{virtio-gpu-udmabuf.c => virtio-gpu-dmabuf.c} (65%)
 create mode 100644 hw/vfio/dmabuf-stubs.c

-- 
2.53.0

[PATCH v12 01/10] virtio-gpu: Recreate the resource's dmabuf if new backing is attached

[Vivek Kasireddy]

There are cases when a blob resource's backing might get detached
and re-attached again such as when the underlying object is getting
migrated in the Guest. In these situations, we need to obtain a new
dmabuf fd, which can be done by calling virtio_gpu_init_udmabuf().

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>
Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 hw/display/virtio-gpu.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index b998ce8324..ccb229ff3f 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -949,6 +949,10 @@ virtio_gpu_resource_attach_backing(VirtIOGPU *g,
         cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
         return;
     }
+
+    if (res->blob_size) {
+        virtio_gpu_init_udmabuf(res);
+    }
 }
 
 static void
-- 
2.53.0

[PATCH v12 02/10] virtio-gpu: Find hva for Guest's DMA addr associated with a ram device

[Vivek Kasireddy]

If the Guest provides a DMA address that is associated with a ram
device (such as a VFIO PCI device region and not its system memory),
then we can obtain the hva (host virtual address) by invoking
address_space_translate() followed by memory_region_get_ram_ptr().

We cannot use dma_memory_map() because for memory regions that do
not support direct access, it would create bounce buffers instead
of returning the actual hva, which is not desirable here. And, since
VFIO device regions are not considered directly accessible (because
they are mostly MMIO based), virtio-gpu cannot invoke dma_memory_map()
to obtain the hva in this case.

Therefore, in order to handle addresses associated with VFIO devices,
we need to use the address_space_translate() API to first identify
the right memory region and the appropriate offset within that
region and then use memory_region_get_ram_ptr() to get the hva.
This approach also works for addresses associated with the system
memory region.

Note that, although we take an explicit reference on the memory
region, we would still rely on dma_memory_unmap() to drop that
reference when the dma mapping is eventually unmapped.

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>
Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 hw/display/virtio-gpu.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index ccb229ff3f..1c6a8e2995 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -810,6 +810,23 @@ static void virtio_gpu_set_scanout_blob(VirtIOGPU *g,
                               &fb, res, &ss.r, &cmd->error);
 }
 
+static void *virtio_gpu_dma_memory_map(VirtIOGPU *g,
+                                       struct virtio_gpu_ctrl_command *cmd,
+                                       uint64_t a, hwaddr *len)
+{
+    MemoryRegion *mr;
+    hwaddr xlat;
+
+    mr = address_space_translate(VIRTIO_DEVICE(g)->dma_as, a, &xlat, len,
+                                 DMA_DIRECTION_TO_DEVICE,
+                                 MEMTXATTRS_UNSPECIFIED);
+    if (memory_region_is_ram(mr)) {
+        memory_region_ref(mr);
+        return memory_region_get_ram_ptr(mr) + xlat;
+    }
+    return NULL;
+}
+
 int virtio_gpu_create_mapping_iov(VirtIOGPU *g,
                                   uint32_t nr_entries, uint32_t offset,
                                   struct virtio_gpu_ctrl_command *cmd,
@@ -851,9 +868,7 @@ int virtio_gpu_create_mapping_iov(VirtIOGPU *g,
 
         do {
             len = l;
-            map = dma_memory_map(VIRTIO_DEVICE(g)->dma_as, a, &len,
-                                 DMA_DIRECTION_TO_DEVICE,
-                                 MEMTXATTRS_UNSPECIFIED);
+            map = virtio_gpu_dma_memory_map(g, cmd, a, &len);
             if (!map) {
                 qemu_log_mask(LOG_GUEST_ERROR, "%s: failed to map MMIO memory for"
                               " element %d\n", __func__, e);
-- 
2.53.0

[PATCH v12 03/10] virtio-gpu: Rename udmabuf files and helpers to dmabuf

[Vivek Kasireddy]

This is prep-work for adding the ability to create dmabuf fds from
VFIO devices in addition to udmabuf.

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>
Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 include/hw/virtio/virtio-gpu.h                       |  6 +++---
 ...gpu-udmabuf-stubs.c => virtio-gpu-dmabuf-stubs.c} |  4 ++--
 .../{virtio-gpu-udmabuf.c => virtio-gpu-dmabuf.c}    | 12 ++++++------
 hw/display/virtio-gpu.c                              |  8 ++++----
 hw/display/meson.build                               |  4 ++--
 5 files changed, 17 insertions(+), 17 deletions(-)
 rename hw/display/{virtio-gpu-udmabuf-stubs.c => virtio-gpu-dmabuf-stubs.c} (77%)
 rename hw/display/{virtio-gpu-udmabuf.c => virtio-gpu-dmabuf.c} (94%)

diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index f69fc19462..99e0865e38 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -370,10 +370,10 @@ bool virtio_gpu_scanout_blob_to_fb(struct virtio_gpu_framebuffer *fb,
                                    struct virtio_gpu_set_scanout_blob *ss,
                                    uint64_t blob_size);
 
-/* virtio-gpu-udmabuf.c */
+/* virtio-gpu-dmabuf.c */
 bool virtio_gpu_have_udmabuf(void);
-void virtio_gpu_init_udmabuf(struct virtio_gpu_simple_resource *res);
-void virtio_gpu_fini_udmabuf(VirtIOGPU *g,
+void virtio_gpu_init_dmabuf(struct virtio_gpu_simple_resource *res);
+void virtio_gpu_fini_dmabuf(VirtIOGPU *g,
                              struct virtio_gpu_simple_resource *res);
 int virtio_gpu_update_dmabuf(VirtIOGPU *g,
                              uint32_t scanout_id,
diff --git a/hw/display/virtio-gpu-udmabuf-stubs.c b/hw/display/virtio-gpu-dmabuf-stubs.c
similarity index 77%
rename from hw/display/virtio-gpu-udmabuf-stubs.c
rename to hw/display/virtio-gpu-dmabuf-stubs.c
index 85d03935a3..f2c4002800 100644
--- a/hw/display/virtio-gpu-udmabuf-stubs.c
+++ b/hw/display/virtio-gpu-dmabuf-stubs.c
@@ -7,12 +7,12 @@ bool virtio_gpu_have_udmabuf(void)
     return false;
 }
 
-void virtio_gpu_init_udmabuf(struct virtio_gpu_simple_resource *res)
+void virtio_gpu_init_dmabuf(struct virtio_gpu_simple_resource *res)
 {
     /* nothing (stub) */
 }
 
-void virtio_gpu_fini_udmabuf(VirtIOGPU *g, struct virtio_gpu_simple_resource *res)
+void virtio_gpu_fini_dmabuf(VirtIOGPU *g, struct virtio_gpu_simple_resource *res)
 {
     /* nothing (stub) */
 }
diff --git a/hw/display/virtio-gpu-udmabuf.c b/hw/display/virtio-gpu-dmabuf.c
similarity index 94%
rename from hw/display/virtio-gpu-udmabuf.c
rename to hw/display/virtio-gpu-dmabuf.c
index 74b6a7766a..776e5c4c64 100644
--- a/hw/display/virtio-gpu-udmabuf.c
+++ b/hw/display/virtio-gpu-dmabuf.c
@@ -68,7 +68,7 @@ static void virtio_gpu_create_udmabuf(struct virtio_gpu_simple_resource *res)
     g_free(list);
 }
 
-static void virtio_gpu_remap_udmabuf(struct virtio_gpu_simple_resource *res)
+static void virtio_gpu_remap_dmabuf(struct virtio_gpu_simple_resource *res)
 {
     res->remapped = mmap(NULL, res->blob_size, PROT_READ,
                          MAP_SHARED, res->dmabuf_fd, 0);
@@ -79,7 +79,7 @@ static void virtio_gpu_remap_udmabuf(struct virtio_gpu_simple_resource *res)
     }
 }
 
-static void virtio_gpu_destroy_udmabuf(struct virtio_gpu_simple_resource *res)
+static void virtio_gpu_destroy_dmabuf(struct virtio_gpu_simple_resource *res)
 {
     if (res->remapped) {
         munmap(res->remapped, res->blob_size);
@@ -128,7 +128,7 @@ bool virtio_gpu_have_udmabuf(void)
     return memfd_backend;
 }
 
-void virtio_gpu_init_udmabuf(struct virtio_gpu_simple_resource *res)
+void virtio_gpu_init_dmabuf(struct virtio_gpu_simple_resource *res)
 {
     void *pdata = NULL;
 
@@ -141,7 +141,7 @@ void virtio_gpu_init_udmabuf(struct virtio_gpu_simple_resource *res)
         if (res->dmabuf_fd < 0) {
             return;
         }
-        virtio_gpu_remap_udmabuf(res);
+        virtio_gpu_remap_dmabuf(res);
         if (!res->remapped) {
             return;
         }
@@ -162,7 +162,7 @@ static void virtio_gpu_free_dmabuf(VirtIOGPU *g, VGPUDMABuf *dmabuf)
     g_free(dmabuf);
 }
 
-void virtio_gpu_fini_udmabuf(VirtIOGPU *g, struct virtio_gpu_simple_resource *res)
+void virtio_gpu_fini_dmabuf(VirtIOGPU *g, struct virtio_gpu_simple_resource *res)
 {
     int max_outputs = g->parent_obj.conf.max_outputs;
     int i;
@@ -179,7 +179,7 @@ void virtio_gpu_fini_udmabuf(VirtIOGPU *g, struct virtio_gpu_simple_resource *re
         }
     }
 
-    virtio_gpu_destroy_udmabuf(res);
+    virtio_gpu_destroy_dmabuf(res);
 }
 
 static VGPUDMABuf
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 1c6a8e2995..b169772321 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -372,7 +372,7 @@ static void virtio_gpu_resource_create_blob(VirtIOGPU *g,
         return;
     }
 
-    virtio_gpu_init_udmabuf(res);
+    virtio_gpu_init_dmabuf(res);
     QTAILQ_INSERT_HEAD(&g->reslist, res, next);
 }
 
@@ -929,7 +929,7 @@ void virtio_gpu_cleanup_mapping(VirtIOGPU *g,
     res->addrs = NULL;
 
     if (res->blob) {
-        virtio_gpu_fini_udmabuf(g, res);
+        virtio_gpu_fini_dmabuf(g, res);
     }
 }
 
@@ -966,7 +966,7 @@ virtio_gpu_resource_attach_backing(VirtIOGPU *g,
     }
 
     if (res->blob_size) {
-        virtio_gpu_init_udmabuf(res);
+        virtio_gpu_init_dmabuf(res);
     }
 }
 
@@ -1457,7 +1457,7 @@ static int virtio_gpu_blob_load(QEMUFile *f, void *opaque, size_t size,
             return -EINVAL;
         }
 
-        virtio_gpu_init_udmabuf(res);
+        virtio_gpu_init_dmabuf(res);
 
         resource_id = qemu_get_be32(f);
     }
diff --git a/hw/display/meson.build b/hw/display/meson.build
index e730c289b1..6169e47bee 100644
--- a/hw/display/meson.build
+++ b/hw/display/meson.build
@@ -69,9 +69,9 @@ if config_all_devices.has_key('CONFIG_VIRTIO_GPU')
   virtio_gpu_ss.add(when: '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-udmabuf.c'))
+    virtio_gpu_ss.add(files('virtio-gpu-dmabuf.c'))
   else
-    virtio_gpu_ss.add(files('virtio-gpu-udmabuf-stubs.c'))
+    virtio_gpu_ss.add(files('virtio-gpu-dmabuf-stubs.c'))
   endif
   virtio_gpu_ss.add(when: 'CONFIG_VHOST_USER_GPU', if_true: files('vhost-user-gpu.c'))
   hw_display_modules += {'virtio-gpu': virtio_gpu_ss}
-- 
2.53.0

[PATCH v12 04/10] virtio-gpu-dmabuf: Remove rcu_read_lock/unlock from virtio_gpu_create_udmabuf()

[Vivek Kasireddy]

There is no need to wrap the call to qemu_ram_block_from_host() with
rcu_read_lock/unlock as we already hold a reference to the memory
region that owns the ram block. Note that, the reference was obtained
via dma_memory_map() while creating the mapping.

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>
Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 hw/display/virtio-gpu-dmabuf.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/hw/display/virtio-gpu-dmabuf.c b/hw/display/virtio-gpu-dmabuf.c
index 776e5c4c64..64582f973c 100644
--- a/hw/display/virtio-gpu-dmabuf.c
+++ b/hw/display/virtio-gpu-dmabuf.c
@@ -43,10 +43,7 @@ static void virtio_gpu_create_udmabuf(struct virtio_gpu_simple_resource *res)
                      sizeof(struct udmabuf_create_item) * res->iov_cnt);
 
     for (i = 0; i < res->iov_cnt; i++) {
-        rcu_read_lock();
         rb = qemu_ram_block_from_host(res->iov[i].iov_base, false, &offset);
-        rcu_read_unlock();
-
         if (!rb || rb->fd < 0) {
             g_free(list);
             return;
-- 
2.53.0

[PATCH v12 05/10] virtio-gpu-dmabuf: Use g_autofree for the list pointer

[Vivek Kasireddy]

Avoid the manual g_free() calls for the list pointer by using
g_autofree in virtio_gpu_create_udmabuf().

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>
Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 hw/display/virtio-gpu-dmabuf.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/display/virtio-gpu-dmabuf.c b/hw/display/virtio-gpu-dmabuf.c
index 64582f973c..e35f7714a9 100644
--- a/hw/display/virtio-gpu-dmabuf.c
+++ b/hw/display/virtio-gpu-dmabuf.c
@@ -29,7 +29,7 @@
 
 static void virtio_gpu_create_udmabuf(struct virtio_gpu_simple_resource *res)
 {
-    struct udmabuf_create_list *list;
+    g_autofree struct udmabuf_create_list *list = NULL;
     RAMBlock *rb;
     ram_addr_t offset;
     int udmabuf, i;
@@ -45,7 +45,6 @@ static void virtio_gpu_create_udmabuf(struct virtio_gpu_simple_resource *res)
     for (i = 0; i < res->iov_cnt; i++) {
         rb = qemu_ram_block_from_host(res->iov[i].iov_base, false, &offset);
         if (!rb || rb->fd < 0) {
-            g_free(list);
             return;
         }
 
@@ -62,7 +61,6 @@ static void virtio_gpu_create_udmabuf(struct virtio_gpu_simple_resource *res)
         warn_report("%s: UDMABUF_CREATE_LIST: %s", __func__,
                     strerror(errno));
     }
-    g_free(list);
 }
 
 static void virtio_gpu_remap_dmabuf(struct virtio_gpu_simple_resource *res)
-- 
2.53.0

[PATCH v12 06/10] vfio/region: Add a helper to get VFIO region from memory region

[Vivek Kasireddy]

Having a way to get the VFIO region associated with a memory region
is helpful in various scenarios. For example, this capability can
be useful in retrieving the region info such as index and offset
needed for mapping a part of a VFIO region or creating a dmabuf.

Cc: Alex Williamson <alex@shazbot.org>
Cc: Cédric Le Goater <clg@redhat.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 hw/vfio/vfio-region.h | 10 ++++++++++
 hw/vfio/region.c      | 11 +++++++++++
 2 files changed, 21 insertions(+)

diff --git a/hw/vfio/vfio-region.h b/hw/vfio/vfio-region.h
index 9b21d4ee5b..3dd47f77f6 100644
--- a/hw/vfio/vfio-region.h
+++ b/hw/vfio/vfio-region.h
@@ -45,4 +45,14 @@ void vfio_region_unmap(VFIORegion *region);
 void vfio_region_exit(VFIORegion *region);
 void vfio_region_finalize(VFIORegion *region);
 
+/**
+ * Return the VFIO region associated with a given MemoryRegion. This can
+ * be useful in retrieving region info such as index and offset.
+ *
+ * @mr: MemoryRegion to use
+ *
+ * Returns the region or NULL on error.
+ */
+void *vfio_get_region_from_mr(MemoryRegion *mr);
+
 #endif /* HW_VFIO_REGION_H */
diff --git a/hw/vfio/region.c b/hw/vfio/region.c
index 47fdc2df34..9d7ac339c9 100644
--- a/hw/vfio/region.c
+++ b/hw/vfio/region.c
@@ -539,3 +539,14 @@ void vfio_region_mmaps_set_enabled(VFIORegion *region, bool enabled)
     trace_vfio_region_mmaps_set_enabled(memory_region_name(region->mem),
                                         enabled);
 }
+
+void *vfio_get_region_from_mr(MemoryRegion *mr)
+{
+    while (mr->container) {
+        if (mr->ops == &vfio_region_ops) {
+            return mr->opaque;
+        }
+        mr = mr->container;
+    }
+    return NULL;
+}
-- 
2.53.0

[PATCH v12 07/10] vfio/device: Add support for creating dmabuf from multiple ranges

[Vivek Kasireddy]

In order to create a dmabuf associated with a buffer that spans
multiple ranges, we first need to identify the VFIO region and
index the buffer (represented by iovec) belongs to and then
translate its addresses to offsets within that region.

The qemu_ram_block_from_host() API gives us both the region and
the offset info we need to populate the dma ranges so that we can
invoke the VFIO_DEVICE_FEATURE_DMA_BUF feature to create the dmabuf.

Also, instead of iterating over all QOM devices to find the
VFIODevice associated with a memory region, introduce a helper
to just use the vfio_device_list to lookup the VFIODevice.
And, introduce another helper lookup the VFIO region given an
address.

Lastly, introduce an enum to return the type of error encountered
while creating the dmabuf fd.

Cc: Alex Williamson <alex@shazbot.org>
Cc: Cédric Le Goater <clg@redhat.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 include/hw/vfio/vfio-device.h |  23 +++++++
 hw/vfio/device.c              | 114 ++++++++++++++++++++++++++++++++++
 hw/vfio/dmabuf-stubs.c        |  17 +++++
 hw/vfio/meson.build           |   1 +
 4 files changed, 155 insertions(+)
 create mode 100644 hw/vfio/dmabuf-stubs.c

diff --git a/include/hw/vfio/vfio-device.h b/include/hw/vfio/vfio-device.h
index 828a31c006..d46640ff53 100644
--- a/include/hw/vfio/vfio-device.h
+++ b/include/hw/vfio/vfio-device.h
@@ -41,6 +41,13 @@ enum {
     VFIO_DEVICE_TYPE_AP = 3,
 };
 
+enum {
+    /* The Guest passed addresses stored in IOV are invalid */
+    VFIO_DMABUF_CREATE_ERR_INVALID_IOV = -1,
+    /* Guest or Host may be at fault for this type of error */
+    VFIO_DMABUF_CREATE_ERR_UNSPEC = -2,
+};
+
 typedef struct VFIODeviceOps VFIODeviceOps;
 typedef struct VFIODeviceIOOps VFIODeviceIOOps;
 typedef struct VFIOMigration VFIOMigration;
@@ -308,6 +315,22 @@ bool vfio_device_has_region_cap(VFIODevice *vbasedev, int region, uint16_t cap_t
 
 int vfio_device_get_irq_info(VFIODevice *vbasedev, int index,
                                 struct vfio_irq_info *info);
+
+/**
+ * Create a dmabuf fd by first translating the addresses in the
+ * iovec array into VFIO region offsets and then invoking the
+ * VFIO_DEVICE_FEATURE_DMA_BUF feature.
+ *
+ * @iov: array of iovec entries associated with a buffer
+ * @iov_cnt: number of entries in the iovec array
+ * @total_size: total size of the dmabuf
+ * @errp: pointer to Error*, to store an error if it happens.
+ *
+ * Returns the created dmabuf fd or either VFIO_DMABUF_CREATE_ERR_UNSPEC
+ * or VFIO_DMABUF_CREATE_ERR_INVALID_IOV on error.
+ */
+int vfio_device_create_dmabuf_fd(struct iovec *iov, unsigned int iov_cnt,
+                                 size_t total_size, Error **errp);
 #endif
 
 /* Returns 0 on success, or a negative errno. */
diff --git a/hw/vfio/device.c b/hw/vfio/device.c
index 973fc35b59..542c378913 100644
--- a/hw/vfio/device.c
+++ b/hw/vfio/device.c
@@ -21,7 +21,9 @@
 #include "qemu/osdep.h"
 #include <sys/ioctl.h>
 
+#include "system/ramblock.h"
 #include "hw/vfio/vfio-device.h"
+#include "hw/vfio/vfio-region.h"
 #include "hw/vfio/pci.h"
 #include "hw/core/iommu.h"
 #include "hw/core/hw-error.h"
@@ -644,3 +646,115 @@ static VFIODeviceIOOps vfio_device_io_ops_ioctl = {
     .region_read = vfio_device_io_region_read,
     .region_write = vfio_device_io_region_write,
 };
+
+/*
+ * This helper looks up the VFIODevice corresponding to the given iov. It
+ * can be useful to determinine if a buffer (represented by the iov) belongs
+ * to a VFIO device or not. This is mainly invoked when external components
+ * such as virtio-gpu request creation of dmabuf fds for buffers that may
+ * belong to a VFIO device.
+ */
+static bool vfio_device_lookup(struct iovec *iov, VFIODevice **vbasedevp,
+                               RAMBlock **first_rbp, Error **errp)
+{
+    VFIODevice *vbasedev;
+    RAMBlock *first_rb;
+    ram_addr_t offset;
+
+    first_rb = qemu_ram_block_from_host(iov[0].iov_base, false, &offset);
+    if (!first_rb) {
+        error_setg(errp, "Could not find first ramblock\n");
+        return false;
+    }
+
+    *first_rbp = first_rb;
+    QLIST_FOREACH(vbasedev, &vfio_device_list, next) {
+        if (vbasedev->dev == first_rb->mr->dev) {
+            *vbasedevp = vbasedev;
+            return true;
+        }
+    }
+    error_setg(errp, "No VFIO device found to create dmabuf from\n");
+    return false;
+}
+
+/*
+ * This helper looks up the VFIORegion corresponding to the given address.
+ * It also verifies that the RAMBlock associated with the address is the
+ * same as the first_rb passed in. This is to ensure that all addresses
+ * in the iov belong to the same region.
+ */
+static bool vfio_region_lookup(void *addr, VFIORegion **regionp,
+                               RAMBlock *first_rb, ram_addr_t *offsetp,
+                               Error **errp)
+{
+    VFIORegion *region;
+    RAMBlock *rb;
+
+    rb = qemu_ram_block_from_host(addr, false, offsetp);
+    if (!rb || rb != first_rb) {
+        error_setg(errp, "Dmabuf segments must belong to the same region\n");
+        return false;
+    }
+
+    region = vfio_get_region_from_mr(rb->mr);
+    if (region) {
+        *regionp = region;
+        return true;
+    }
+    error_setg(errp, "Could not find valid region for dmabuf segment\n");
+    return false;
+}
+
+int vfio_device_create_dmabuf_fd(struct iovec *iov, unsigned int iov_cnt,
+                                 size_t total_size, Error **errp)
+{
+    g_autofree struct vfio_device_feature *feature = NULL;
+    struct vfio_device_feature_dma_buf *dma_buf;
+    RAMBlock *first_rb = NULL;
+    VFIODevice *vbasedev;
+    VFIORegion *region;
+    ram_addr_t offset;
+    size_t argsz;
+    int i, ret;
+
+    if (iov_size(iov, iov_cnt) != total_size) {
+        error_setg(errp, "Total size of iov does not match dmabuf size\n");
+        return VFIO_DMABUF_CREATE_ERR_INVALID_IOV;
+    }
+
+    if (!vfio_device_lookup(iov, &vbasedev, &first_rb, errp)) {
+        return VFIO_DMABUF_CREATE_ERR_INVALID_IOV;
+    }
+
+    argsz = sizeof(*feature) + sizeof (*dma_buf) +
+            sizeof(struct vfio_region_dma_range) * iov_cnt;
+    feature = g_malloc0(argsz);
+    *feature = (struct vfio_device_feature) {
+        .argsz = argsz,
+        .flags = VFIO_DEVICE_FEATURE_GET | VFIO_DEVICE_FEATURE_DMA_BUF,
+    };
+    dma_buf = (struct vfio_device_feature_dma_buf *)feature->data;
+
+    for (i = 0; i < iov_cnt; i++) {
+        if (!vfio_region_lookup(iov[i].iov_base, &region,
+                                first_rb, &offset, errp)) {
+            return VFIO_DMABUF_CREATE_ERR_INVALID_IOV;
+        }
+
+        dma_buf->region_index = region->nr;
+        dma_buf->dma_ranges[i].offset = offset;
+        dma_buf->dma_ranges[i].length = iov[i].iov_len;
+    }
+
+    dma_buf->nr_ranges = iov_cnt;
+    dma_buf->open_flags = O_RDONLY | O_CLOEXEC;
+
+    ret = vfio_device_get_feature(vbasedev, feature);
+    if (ret < 0) {
+        error_setg_errno(errp, -ret,
+                         "Could not create dmabuf fd via VFIO device");
+        return VFIO_DMABUF_CREATE_ERR_UNSPEC;
+    }
+    return ret;
+}
diff --git a/hw/vfio/dmabuf-stubs.c b/hw/vfio/dmabuf-stubs.c
new file mode 100644
index 0000000000..374bd2a188
--- /dev/null
+++ b/hw/vfio/dmabuf-stubs.c
@@ -0,0 +1,17 @@
+/*
+ * 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"
+#include "hw/vfio/vfio-device.h"
+
+int vfio_device_create_dmabuf_fd(struct iovec *iov, unsigned int iov_cnt,
+                                 size_t total_size, Error **errp)
+{
+    error_setg(errp, "VFIO dmabuf support is not enabled");
+    return VFIO_DMABUF_CREATE_HOST_ERROR;
+}
diff --git a/hw/vfio/meson.build b/hw/vfio/meson.build
index 82f68698fb..ac899d30a8 100644
--- a/hw/vfio/meson.build
+++ b/hw/vfio/meson.build
@@ -34,3 +34,4 @@ system_ss.add(when: 'CONFIG_IOMMUFD', if_false: files('iommufd-stubs.c'))
 system_ss.add(when: 'CONFIG_VFIO_PCI', if_true: files(
   'display.c',
 ))
+system_ss.add(when: 'CONFIG_VFIO', if_false: files('dmabuf-stubs.c'))
-- 
2.53.0

[PATCH v12 08/10] vfio/device: Add a helper to mmap a dmabuf

[Vivek Kasireddy]

In order to mmap a dmabuf, we first need to create a placeholder
mapping that would fit the entire size of the dmabuf. This mapping
would then be replaced with smaller mappings of individual dmabuf
segments.

Cc: Alex Williamson <alex@shazbot.org>
Cc: Cédric Le Goater <clg@redhat.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 include/hw/vfio/vfio-device.h | 16 ++++++++++
 hw/vfio/device.c              | 59 +++++++++++++++++++++++++++++++++++
 hw/vfio/dmabuf-stubs.c        |  7 +++++
 3 files changed, 82 insertions(+)

diff --git a/include/hw/vfio/vfio-device.h b/include/hw/vfio/vfio-device.h
index d46640ff53..14909490ef 100644
--- a/include/hw/vfio/vfio-device.h
+++ b/include/hw/vfio/vfio-device.h
@@ -331,6 +331,22 @@ int vfio_device_get_irq_info(VFIODevice *vbasedev, int index,
  */
 int vfio_device_create_dmabuf_fd(struct iovec *iov, unsigned int iov_cnt,
                                  size_t total_size, Error **errp);
+
+/**
+ * Mmap a dmabuf by first translating the addresses in the iovec
+ * array into VFIO region offsets and then creating a placeholder
+ * mapping that would be replaced later with mappings that
+ * correspond to the dmabuf segments.
+ *
+ * @iov: array of iovec entries associated with the dmabuf
+ * @iov_cnt: number of entries in the iovec array
+ * @total_size: total size of the dmabuf
+ * @errp: pointer to Error*, to store an error if it happens.
+ *
+ * Returns the mapped pointer on success and NULL on error.
+ */
+void *vfio_device_mmap_dmabuf(struct iovec *iov, unsigned int iov_cnt,
+                              size_t total_size, Error **errp);
 #endif
 
 /* Returns 0 on success, or a negative errno. */
diff --git a/hw/vfio/device.c b/hw/vfio/device.c
index 542c378913..43223ceacd 100644
--- a/hw/vfio/device.c
+++ b/hw/vfio/device.c
@@ -758,3 +758,62 @@ int vfio_device_create_dmabuf_fd(struct iovec *iov, unsigned int iov_cnt,
     }
     return ret;
 }
+
+void *vfio_device_mmap_dmabuf(struct iovec *iov, unsigned int iov_cnt,
+                              size_t total_size, Error **errp)
+{
+    struct vfio_region_info *info = NULL;
+    ram_addr_t offset, len = 0;
+    VFIODevice *vbasedev;
+    VFIORegion *region;
+    RAMBlock *first_rb;
+    void *map, *submap;
+    int i;
+
+    if (iov_size(iov, iov_cnt) != total_size) {
+        error_setg(errp, "Total size of iov does not match dmabuf size\n");
+        return NULL;
+    }
+
+    if (!vfio_device_lookup(iov, &vbasedev, &first_rb, errp)) {
+        return NULL;
+    }
+
+    /*
+     * We first reserve a contiguous chunk of address space for the entire
+     * dmabuf, then replace it with smaller mappings that correspond to the
+     * individual segments of the dmabuf.
+     */
+    map = mmap(NULL, total_size, PROT_NONE, MAP_SHARED | MAP_ANONYMOUS, -1, 0);
+    if (map == MAP_FAILED) {
+        error_setg_errno(errp, errno, "Could not reserve placeholder mapping");
+        return NULL;
+    }
+
+    for (i = 0; i < iov_cnt; i++) {
+        if (!vfio_region_lookup(iov[i].iov_base, &region,
+                                first_rb, &offset, errp)) {
+            goto err;
+        }
+
+        if (vfio_device_get_region_info(vbasedev, region->nr, &info)) {
+            error_setg(errp, "Cannot find region info for dmabuf segment\n");
+            goto err;
+        }
+
+        submap = mmap(map + len, iov[i].iov_len, PROT_READ,
+                      MAP_SHARED | MAP_FIXED, vbasedev->fd,
+                      info->offset + offset);
+        if (submap == MAP_FAILED) {
+            error_setg_errno(errp, errno, "Could not mmap dmabuf segment\n");
+            goto err;
+        }
+        len += iov[i].iov_len;
+    }
+    return map;
+err:
+    munmap(map, total_size);
+    error_prepend(errp, "VFIO dmabuf mmap failed: ");
+    return NULL;
+}
+
diff --git a/hw/vfio/dmabuf-stubs.c b/hw/vfio/dmabuf-stubs.c
index 374bd2a188..044d1ed058 100644
--- a/hw/vfio/dmabuf-stubs.c
+++ b/hw/vfio/dmabuf-stubs.c
@@ -15,3 +15,10 @@ int vfio_device_create_dmabuf_fd(struct iovec *iov, unsigned int iov_cnt,
     error_setg(errp, "VFIO dmabuf support is not enabled");
     return VFIO_DMABUF_CREATE_HOST_ERROR;
 }
+
+void *vfio_device_mmap_dmabuf(struct iovec *iov, unsigned int iov_cnt,
+                              size_t total_size, Error **errp)
+{
+    error_setg(errp, "VFIO mmap dmabuf support is not enabled");
+    return NULL;
+}
-- 
2.53.0

[PATCH v12 09/10] virtio-gpu-dmabuf: Improve error handling with 'Error **' and err enum

[Vivek Kasireddy]

Make the error handling more robust in virtio_gpu_init_udmabuf()
by introducing 'Error **' parameter to capture errors and using
an enum from VFIO to categorize different errors. This allows for
better error reporting and handling of errors from
virtio_gpu_create_udmabuf() and virtio_gpu_remap_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>
Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 hw/display/virtio-gpu-dmabuf.c | 67 +++++++++++++++++++++++-----------
 1 file changed, 45 insertions(+), 22 deletions(-)

diff --git a/hw/display/virtio-gpu-dmabuf.c b/hw/display/virtio-gpu-dmabuf.c
index e35f7714a9..89aa487654 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"
@@ -27,16 +28,18 @@
 #include "standard-headers/linux/udmabuf.h"
 #include "standard-headers/drm/drm_fourcc.h"
 
-static void virtio_gpu_create_udmabuf(struct virtio_gpu_simple_resource *res)
+static int virtio_gpu_create_udmabuf(struct virtio_gpu_simple_resource *res,
+                                     Error **errp)
 {
     g_autofree struct udmabuf_create_list *list = NULL;
     RAMBlock *rb;
     ram_addr_t offset;
-    int udmabuf, i;
+    int udmabuf, i, fd;
 
     udmabuf = udmabuf_fd();
     if (udmabuf < 0) {
-        return;
+        error_setg(errp, "udmabuf device not available or enabled");
+        return VFIO_DMABUF_CREATE_ERR_UNSPEC;
     }
 
     list = g_malloc0(sizeof(struct udmabuf_create_list) +
@@ -45,7 +48,8 @@ static void virtio_gpu_create_udmabuf(struct virtio_gpu_simple_resource *res)
     for (i = 0; i < res->iov_cnt; i++) {
         rb = qemu_ram_block_from_host(res->iov[i].iov_base, false, &offset);
         if (!rb || rb->fd < 0) {
-            return;
+            error_setg(errp, "IOV memory address incompatible with udmabuf ");
+            return VFIO_DMABUF_CREATE_ERR_INVALID_IOV;
         }
 
         list->list[i].memfd  = rb->fd;
@@ -56,22 +60,28 @@ static void virtio_gpu_create_udmabuf(struct virtio_gpu_simple_resource *res)
     list->count = res->iov_cnt;
     list->flags = UDMABUF_FLAGS_CLOEXEC;
 
-    res->dmabuf_fd = ioctl(udmabuf, UDMABUF_CREATE_LIST, list);
-    if (res->dmabuf_fd < 0) {
-        warn_report("%s: UDMABUF_CREATE_LIST: %s", __func__,
-                    strerror(errno));
+    fd = ioctl(udmabuf, UDMABUF_CREATE_LIST, list);
+    if (fd < 0) {
+        error_setg_errno(errp, errno, "UDMABUF_CREATE_LIST: ioctl failed");
+        if (errno == EINVAL || errno == EBADFD) {
+            return VFIO_DMABUF_CREATE_ERR_INVALID_IOV;
+        }
+        return VFIO_DMABUF_CREATE_ERR_UNSPEC;
     }
+    return fd;
 }
 
-static void virtio_gpu_remap_dmabuf(struct virtio_gpu_simple_resource *res)
+static void *virtio_gpu_remap_dmabuf(struct virtio_gpu_simple_resource *res,
+                                     Error **errp)
 {
-    res->remapped = mmap(NULL, res->blob_size, PROT_READ,
-                         MAP_SHARED, res->dmabuf_fd, 0);
-    if (res->remapped == MAP_FAILED) {
-        warn_report("%s: dmabuf mmap failed: %s", __func__,
-                    strerror(errno));
-        res->remapped = 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");
+        return NULL;
     }
+    return map;
 }
 
 static void virtio_gpu_destroy_dmabuf(struct virtio_gpu_simple_resource *res)
@@ -125,22 +135,35 @@ bool virtio_gpu_have_udmabuf(void)
 
 void virtio_gpu_init_dmabuf(struct virtio_gpu_simple_resource *res)
 {
+    Error *local_err = NULL;
     void *pdata = NULL;
 
-    res->dmabuf_fd = -1;
     if (res->iov_cnt == 1 &&
         res->iov[0].iov_len < 4096) {
+        res->dmabuf_fd = -1;
         pdata = res->iov[0].iov_base;
     } else {
-        virtio_gpu_create_udmabuf(res);
+        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);
+
+            qemu_log_mask(LOG_GUEST_ERROR,
+                          "Cannot create dmabuf: incompatible memory\n");
+            return;
+        } else if (res->dmabuf_fd >= 0) {
+            pdata = virtio_gpu_remap_dmabuf(res, &local_err);
+            if (!pdata) {
+                virtio_gpu_destroy_dmabuf(res);
+            }
+        } else {
+            res->dmabuf_fd = -1;
+        }
+
         if (res->dmabuf_fd < 0) {
+            error_report_err(local_err);
             return;
         }
-        virtio_gpu_remap_dmabuf(res);
-        if (!res->remapped) {
-            return;
-        }
-        pdata = res->remapped;
+        res->remapped = pdata;
     }
 
     res->blob = pdata;
-- 
2.53.0

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

[Vivek Kasireddy]

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>
Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 hw/display/virtio-gpu-dmabuf.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/hw/display/virtio-gpu-dmabuf.c b/hw/display/virtio-gpu-dmabuf.c
index 89aa487654..f953db0fbe 100644
--- a/hw/display/virtio-gpu-dmabuf.c
+++ b/hw/display/virtio-gpu-dmabuf.c
@@ -147,9 +147,26 @@ void virtio_gpu_init_dmabuf(struct virtio_gpu_simple_resource *res)
         if (res->dmabuf_fd == VFIO_DMABUF_CREATE_ERR_INVALID_IOV) {
             error_free_or_abort(&local_err);
 
-            qemu_log_mask(LOG_GUEST_ERROR,
-                          "Cannot create dmabuf: incompatible memory\n");
-            return;
+            res->dmabuf_fd = vfio_device_create_dmabuf_fd(res->iov,
+                                                          res->iov_cnt,
+                                                          res->blob_size,
+                                                          &local_err);
+            if (res->dmabuf_fd == VFIO_DMABUF_CREATE_ERR_INVALID_IOV) {
+                error_free_or_abort(&local_err);
+                qemu_log_mask(LOG_GUEST_ERROR,
+                              "Cannot create dmabuf: incompatible memory\n");
+                return;
+            }
+
+            if (res->dmabuf_fd >= 0) {
+                pdata = vfio_device_mmap_dmabuf(res->iov, res->iov_cnt,
+                                                res->blob_size, &local_err);
+                if (!pdata) {
+                    virtio_gpu_destroy_dmabuf(res);
+                }
+            } else {
+                res->dmabuf_fd = -1;
+            }
         } else if (res->dmabuf_fd >= 0) {
             pdata = virtio_gpu_remap_dmabuf(res, &local_err);
             if (!pdata) {
-- 
2.53.0

Re: [PATCH v12 06/10] vfio/region: Add a helper to get VFIO region from memory region

[Cédric Le Goater]

On 3/19/26 06:15, Vivek Kasireddy wrote:
> Having a way to get the VFIO region associated with a memory region
> is helpful in various scenarios. For example, this capability can
> be useful in retrieving the region info such as index and offset
> needed for mapping a part of a VFIO region or creating a dmabuf.
> 
> Cc: Alex Williamson <alex@shazbot.org>
> Cc: Cédric Le Goater <clg@redhat.com>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> ---
>   hw/vfio/vfio-region.h | 10 ++++++++++
>   hw/vfio/region.c      | 11 +++++++++++
>   2 files changed, 21 insertions(+)
> 
> diff --git a/hw/vfio/vfio-region.h b/hw/vfio/vfio-region.h
> index 9b21d4ee5b..3dd47f77f6 100644
> --- a/hw/vfio/vfio-region.h
> +++ b/hw/vfio/vfio-region.h
> @@ -45,4 +45,14 @@ void vfio_region_unmap(VFIORegion *region);
>   void vfio_region_exit(VFIORegion *region);
>   void vfio_region_finalize(VFIORegion *region);
>   
> +/**
> + * Return the VFIO region associated with a given MemoryRegion. This can
> + * be useful in retrieving region info such as index and offset.
> + *
> + * @mr: MemoryRegion to use
> + *
> + * Returns the region or NULL on error.
> + */
> +void *vfio_get_region_from_mr(MemoryRegion *mr);
> +
>   #endif /* HW_VFIO_REGION_H */
> diff --git a/hw/vfio/region.c b/hw/vfio/region.c
> index 47fdc2df34..9d7ac339c9 100644
> --- a/hw/vfio/region.c
> +++ b/hw/vfio/region.c
> @@ -539,3 +539,14 @@ void vfio_region_mmaps_set_enabled(VFIORegion *region, bool enabled)
>       trace_vfio_region_mmaps_set_enabled(memory_region_name(region->mem),
>                                           enabled);
>   }
> +
> +void *vfio_get_region_from_mr(MemoryRegion *mr)

can we return a "VFIORegion *" ?

Thanks,

C.

> +{
> +    while (mr->container) {
> +        if (mr->ops == &vfio_region_ops) {
> +            return mr->opaque;
> +        }
> +        mr = mr->container;
> +    }
> +    return NULL;
> +}

Re: [PATCH v12 09/10] virtio-gpu-dmabuf: Improve error handling with 'Error **' and err enum

[Cédric Le Goater]

On 3/19/26 06:15, Vivek Kasireddy wrote:
> Make the error handling more robust in virtio_gpu_init_udmabuf()
> by introducing 'Error **' parameter to capture errors and using
> an enum from VFIO to categorize different errors. This allows for
> better error reporting and handling of errors from
> virtio_gpu_create_udmabuf() and virtio_gpu_remap_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>
> Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> ---
>   hw/display/virtio-gpu-dmabuf.c | 67 +++++++++++++++++++++++-----------
>   1 file changed, 45 insertions(+), 22 deletions(-)
> 
> diff --git a/hw/display/virtio-gpu-dmabuf.c b/hw/display/virtio-gpu-dmabuf.c
> index e35f7714a9..89aa487654 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"
> @@ -27,16 +28,18 @@
>   #include "standard-headers/linux/udmabuf.h"
>   #include "standard-headers/drm/drm_fourcc.h"
>   
> -static void virtio_gpu_create_udmabuf(struct virtio_gpu_simple_resource *res)
> +static int virtio_gpu_create_udmabuf(struct virtio_gpu_simple_resource *res,
> +                                     Error **errp)
>   {
>       g_autofree struct udmabuf_create_list *list = NULL;
>       RAMBlock *rb;
>       ram_addr_t offset;
> -    int udmabuf, i;
> +    int udmabuf, i, fd;
>   
>       udmabuf = udmabuf_fd();
>       if (udmabuf < 0) {
> -        return;
> +        error_setg(errp, "udmabuf device not available or enabled");
> +        return VFIO_DMABUF_CREATE_ERR_UNSPEC;

The function virtio_gpu_create_udmabuf() is returning VFIO_DMABUF_* enum
values, which is problematic because the function creates a udmabuf, not
a VFIO dmabuf.

This creates a layering violation. The virtio-gpu-dmabuf code (which
handles both udmabuf and VFIO dmabuf creation) is using error codes
defined in the VFIO-specific header.

Please find another solution.



>       }
>   
>       list = g_malloc0(sizeof(struct udmabuf_create_list) +
> @@ -45,7 +48,8 @@ static void virtio_gpu_create_udmabuf(struct virtio_gpu_simple_resource *res)
>       for (i = 0; i < res->iov_cnt; i++) {
>           rb = qemu_ram_block_from_host(res->iov[i].iov_base, false, &offset);
>           if (!rb || rb->fd < 0) {
> -            return;
> +            error_setg(errp, "IOV memory address incompatible with udmabuf ");
> +            return VFIO_DMABUF_CREATE_ERR_INVALID_IOV;
>           }
>   
>           list->list[i].memfd  = rb->fd;
> @@ -56,22 +60,28 @@ static void virtio_gpu_create_udmabuf(struct virtio_gpu_simple_resource *res)
>       list->count = res->iov_cnt;
>       list->flags = UDMABUF_FLAGS_CLOEXEC;
>   
> -    res->dmabuf_fd = ioctl(udmabuf, UDMABUF_CREATE_LIST, list);
> -    if (res->dmabuf_fd < 0) {
> -        warn_report("%s: UDMABUF_CREATE_LIST: %s", __func__,
> -                    strerror(errno));
> +    fd = ioctl(udmabuf, UDMABUF_CREATE_LIST, list);
> +    if (fd < 0) {
> +        error_setg_errno(errp, errno, "UDMABUF_CREATE_LIST: ioctl failed");
> +        if (errno == EINVAL || errno == EBADFD) {
> +            return VFIO_DMABUF_CREATE_ERR_INVALID_IOV;
> +        }
> +        return VFIO_DMABUF_CREATE_ERR_UNSPEC;
>       }
> +    return fd;
>   }
>   
> -static void virtio_gpu_remap_dmabuf(struct virtio_gpu_simple_resource *res)
> +static void *virtio_gpu_remap_dmabuf(struct virtio_gpu_simple_resource *res,
> +                                     Error **errp)
>   {
> -    res->remapped = mmap(NULL, res->blob_size, PROT_READ,
> -                         MAP_SHARED, res->dmabuf_fd, 0);
> -    if (res->remapped == MAP_FAILED) {
> -        warn_report("%s: dmabuf mmap failed: %s", __func__,
> -                    strerror(errno));
> -        res->remapped = 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");
> +        return NULL;
>       }
> +    return map;
>   }
>   
>   static void virtio_gpu_destroy_dmabuf(struct virtio_gpu_simple_resource *res)
> @@ -125,22 +135,35 @@ bool virtio_gpu_have_udmabuf(void)
>   
>   void virtio_gpu_init_dmabuf(struct virtio_gpu_simple_resource *res)
>   {
> +    Error *local_err = NULL;
>       void *pdata = NULL;
>   
> -    res->dmabuf_fd = -1;
>       if (res->iov_cnt == 1 &&
>           res->iov[0].iov_len < 4096) {
> +        res->dmabuf_fd = -1;
>           pdata = res->iov[0].iov_base;
>       } else {
> -        virtio_gpu_create_udmabuf(res);
> +        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);

Why not report the error in the QEMU log below ?

> +
> +            qemu_log_mask(LOG_GUEST_ERROR,
> +                          "Cannot create dmabuf: incompatible memory\n");
> +            return;
> +        } else if (res->dmabuf_fd >= 0) {
> +            pdata = virtio_gpu_remap_dmabuf(res, &local_err);
> +            if (!pdata) {
> +                virtio_gpu_destroy_dmabuf(res);
> +            }
> +        } else {
> +            res->dmabuf_fd = -1;
> +        }
> +
>           if (res->dmabuf_fd < 0) {
> +            error_report_err(local_err);
>               return;
>           }
> -        virtio_gpu_remap_dmabuf(res);
> -        if (!res->remapped) {
> -            return;
> -        }
> -        pdata = res->remapped;
> +        res->remapped = pdata;
>       }
>   
>       res->blob = pdata;

Re: [PATCH v12 07/10] vfio/device: Add support for creating dmabuf from multiple ranges

[Cédric Le Goater]

On 3/19/26 06:15, Vivek Kasireddy wrote:
> In order to create a dmabuf associated with a buffer that spans
> multiple ranges, we first need to identify the VFIO region and
> index the buffer (represented by iovec) belongs to and then
> translate its addresses to offsets within that region.
> 
> The qemu_ram_block_from_host() API gives us both the region and
> the offset info we need to populate the dma ranges so that we can
> invoke the VFIO_DEVICE_FEATURE_DMA_BUF feature to create the dmabuf.
> 
> Also, instead of iterating over all QOM devices to find the
> VFIODevice associated with a memory region, introduce a helper
> to just use the vfio_device_list to lookup the VFIODevice.
> And, introduce another helper lookup the VFIO region given an
> address.
> 
> Lastly, introduce an enum to return the type of error encountered
> while creating the dmabuf fd.
> 
> Cc: Alex Williamson <alex@shazbot.org>
> Cc: Cédric Le Goater <clg@redhat.com>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> ---
>   include/hw/vfio/vfio-device.h |  23 +++++++
>   hw/vfio/device.c              | 114 ++++++++++++++++++++++++++++++++++
>   hw/vfio/dmabuf-stubs.c        |  17 +++++
>   hw/vfio/meson.build           |   1 +
>   4 files changed, 155 insertions(+)
>   create mode 100644 hw/vfio/dmabuf-stubs.c
> 
> diff --git a/include/hw/vfio/vfio-device.h b/include/hw/vfio/vfio-device.h
> index 828a31c006..d46640ff53 100644
> --- a/include/hw/vfio/vfio-device.h
> +++ b/include/hw/vfio/vfio-device.h
> @@ -41,6 +41,13 @@ enum {
>       VFIO_DEVICE_TYPE_AP = 3,
>   };
>   
> +enum {
> +    /* The Guest passed addresses stored in IOV are invalid */
> +    VFIO_DMABUF_CREATE_ERR_INVALID_IOV = -1,
> +    /* Guest or Host may be at fault for this type of error */
> +    VFIO_DMABUF_CREATE_ERR_UNSPEC = -2,
> +};

Sorry, I am not convinced this is useful.

> +
>   typedef struct VFIODeviceOps VFIODeviceOps;
>   typedef struct VFIODeviceIOOps VFIODeviceIOOps;
>   typedef struct VFIOMigration VFIOMigration;
> @@ -308,6 +315,22 @@ bool vfio_device_has_region_cap(VFIODevice *vbasedev, int region, uint16_t cap_t
>   
>   int vfio_device_get_irq_info(VFIODevice *vbasedev, int index,
>                                   struct vfio_irq_info *info);
> +
> +/**
> + * Create a dmabuf fd by first translating the addresses in the
> + * iovec array into VFIO region offsets and then invoking the
> + * VFIO_DEVICE_FEATURE_DMA_BUF feature.
> + *
> + * @iov: array of iovec entries associated with a buffer
> + * @iov_cnt: number of entries in the iovec array
> + * @total_size: total size of the dmabuf
> + * @errp: pointer to Error*, to store an error if it happens.
> + *
> + * Returns the created dmabuf fd or either VFIO_DMABUF_CREATE_ERR_UNSPEC
> + * or VFIO_DMABUF_CREATE_ERR_INVALID_IOV on error.
> + */
> +int vfio_device_create_dmabuf_fd(struct iovec *iov, unsigned int iov_cnt,
> +                                 size_t total_size, Error **errp);
>   #endif
>   
>   /* Returns 0 on success, or a negative errno. */
> diff --git a/hw/vfio/device.c b/hw/vfio/device.c
> index 973fc35b59..542c378913 100644
> --- a/hw/vfio/device.c
> +++ b/hw/vfio/device.c
> @@ -21,7 +21,9 @@
>   #include "qemu/osdep.h"
>   #include <sys/ioctl.h>
>   
> +#include "system/ramblock.h"
>   #include "hw/vfio/vfio-device.h"
> +#include "hw/vfio/vfio-region.h"
>   #include "hw/vfio/pci.h"
>   #include "hw/core/iommu.h"
>   #include "hw/core/hw-error.h"
> @@ -644,3 +646,115 @@ static VFIODeviceIOOps vfio_device_io_ops_ioctl = {
>       .region_read = vfio_device_io_region_read,
>       .region_write = vfio_device_io_region_write,
>   };
> +
> +/*
> + * This helper looks up the VFIODevice corresponding to the given iov. It
> + * can be useful to determinine if a buffer (represented by the iov) belongs
> + * to a VFIO device or not. This is mainly invoked when external components
> + * such as virtio-gpu request creation of dmabuf fds for buffers that may
> + * belong to a VFIO device.
> + */
> +static bool vfio_device_lookup(struct iovec *iov, VFIODevice **vbasedevp,
> +                               RAMBlock **first_rbp, Error **errp)
> +{
> +    VFIODevice *vbasedev;
> +    RAMBlock *first_rb;
> +    ram_addr_t offset;
> +
> +    first_rb = qemu_ram_block_from_host(iov[0].iov_base, false, &offset);
> +    if (!first_rb) {
> +        error_setg(errp, "Could not find first ramblock\n");
> +        return false;
> +    }
> +
> +    *first_rbp = first_rb;
> +    QLIST_FOREACH(vbasedev, &vfio_device_list, next) {
> +        if (vbasedev->dev == first_rb->mr->dev) {
> +            *vbasedevp = vbasedev;
> +            return true;
> +        }
> +    }
> +    error_setg(errp, "No VFIO device found to create dmabuf from\n");
> +    return false;
> +}
> +
> +/*
> + * This helper looks up the VFIORegion corresponding to the given address.
> + * It also verifies that the RAMBlock associated with the address is the
> + * same as the first_rb passed in. This is to ensure that all addresses
> + * in the iov belong to the same region.
> + */
> +static bool vfio_region_lookup(void *addr, VFIORegion **regionp,
> +                               RAMBlock *first_rb, ram_addr_t *offsetp,
> +                               Error **errp)
> +{
> +    VFIORegion *region;
> +    RAMBlock *rb;
> +
> +    rb = qemu_ram_block_from_host(addr, false, offsetp);
> +    if (!rb || rb != first_rb) {
> +        error_setg(errp, "Dmabuf segments must belong to the same region\n");
> +        return false;
> +    }
> +
> +    region = vfio_get_region_from_mr(rb->mr);
> +    if (region) {
> +        *regionp = region;
> +        return true;
> +    }
> +    error_setg(errp, "Could not find valid region for dmabuf segment\n");
> +    return false;
> +}
> +
> +int vfio_device_create_dmabuf_fd(struct iovec *iov, unsigned int iov_cnt,
> +                                 size_t total_size, Error **errp)
> +{
> +    g_autofree struct vfio_device_feature *feature = NULL;
> +    struct vfio_device_feature_dma_buf *dma_buf;
> +    RAMBlock *first_rb = NULL;
> +    VFIODevice *vbasedev;
> +    VFIORegion *region;
> +    ram_addr_t offset;
> +    size_t argsz;
> +    int i, ret;
> +
> +    if (iov_size(iov, iov_cnt) != total_size) {
> +        error_setg(errp, "Total size of iov does not match dmabuf size\n");
> +        return VFIO_DMABUF_CREATE_ERR_INVALID_IOV;

This returned value is redundant with errp and error-prone for an API.
I think you should find an alternative implementation. May be these
routines simply do not belong to VFIO.
  

> +    }
> +
> +    if (!vfio_device_lookup(iov, &vbasedev, &first_rb, errp)) {
> +        return VFIO_DMABUF_CREATE_ERR_INVALID_IOV;
> +    }
> +
> +    argsz = sizeof(*feature) + sizeof (*dma_buf) +
> +            sizeof(struct vfio_region_dma_range) * iov_cnt;
> +    feature = g_malloc0(argsz);
> +    *feature = (struct vfio_device_feature) {
> +        .argsz = argsz,
> +        .flags = VFIO_DEVICE_FEATURE_GET | VFIO_DEVICE_FEATURE_DMA_BUF,
> +    };
> +    dma_buf = (struct vfio_device_feature_dma_buf *)feature->data;
> +
> +    for (i = 0; i < iov_cnt; i++) {
> +        if (!vfio_region_lookup(iov[i].iov_base, &region,
> +                                first_rb, &offset, errp)) {
> +            return VFIO_DMABUF_CREATE_ERR_INVALID_IOV;
> +        }
> +
> +        dma_buf->region_index = region->nr;
> +        dma_buf->dma_ranges[i].offset = offset;
> +        dma_buf->dma_ranges[i].length = iov[i].iov_len;
> +    }
> +
> +    dma_buf->nr_ranges = iov_cnt;
> +    dma_buf->open_flags = O_RDONLY | O_CLOEXEC;
> +
> +    ret = vfio_device_get_feature(vbasedev, feature);
> +    if (ret < 0) {
> +        error_setg_errno(errp, -ret,
> +                         "Could not create dmabuf fd via VFIO device");
> +        return VFIO_DMABUF_CREATE_ERR_UNSPEC;
> +    }
> +    return ret;
> +}
> diff --git a/hw/vfio/dmabuf-stubs.c b/hw/vfio/dmabuf-stubs.c
> new file mode 100644
> index 0000000000..374bd2a188
> --- /dev/null
> +++ b/hw/vfio/dmabuf-stubs.c
> @@ -0,0 +1,17 @@
> +/*
> + * 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"
> +#include "hw/vfio/vfio-device.h"
> +
> +int vfio_device_create_dmabuf_fd(struct iovec *iov, unsigned int iov_cnt,
> +                                 size_t total_size, Error **errp)
> +{
> +    error_setg(errp, "VFIO dmabuf support is not enabled");
> +    return VFIO_DMABUF_CREATE_HOST_ERROR;

../hw/vfio/dmabuf-stubs.c:16:12: error: ‘VFIO_DMABUF_CREATE_HOST_ERROR’ undeclared (first use in this function)
    16 |     return VFIO_DMABUF_CREATE_HOST_ERROR;

Thanks,

C.



> +}
> diff --git a/hw/vfio/meson.build b/hw/vfio/meson.build
> index 82f68698fb..ac899d30a8 100644
> --- a/hw/vfio/meson.build
> +++ b/hw/vfio/meson.build
> @@ -34,3 +34,4 @@ system_ss.add(when: 'CONFIG_IOMMUFD', if_false: files('iommufd-stubs.c'))
>   system_ss.add(when: 'CONFIG_VFIO_PCI', if_true: files(
>     'display.c',
>   ))
> +system_ss.add(when: 'CONFIG_VFIO', if_false: files('dmabuf-stubs.c'))

RE: [PATCH v12 07/10] vfio/device: Add support for creating dmabuf from multiple ranges

[Kasireddy, Vivek]

Hi Cedric,

> Subject: Re: [PATCH v12 07/10] vfio/device: Add support for creating
> dmabuf from multiple ranges
> 
> On 3/19/26 06:15, Vivek Kasireddy wrote:
> > In order to create a dmabuf associated with a buffer that spans
> > multiple ranges, we first need to identify the VFIO region and
> > index the buffer (represented by iovec) belongs to and then
> > translate its addresses to offsets within that region.
> >
> > The qemu_ram_block_from_host() API gives us both the region and
> > the offset info we need to populate the dma ranges so that we can
> > invoke the VFIO_DEVICE_FEATURE_DMA_BUF feature to create the
> dmabuf.
> >
> > Also, instead of iterating over all QOM devices to find the
> > VFIODevice associated with a memory region, introduce a helper
> > to just use the vfio_device_list to lookup the VFIODevice.
> > And, introduce another helper lookup the VFIO region given an
> > address.
> >
> > Lastly, introduce an enum to return the type of error encountered
> > while creating the dmabuf fd.
> >
> > Cc: Alex Williamson <alex@shazbot.org>
> > Cc: Cédric Le Goater <clg@redhat.com>
> > Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> > ---
> >   include/hw/vfio/vfio-device.h |  23 +++++++
> >   hw/vfio/device.c              | 114
> ++++++++++++++++++++++++++++++++++
> >   hw/vfio/dmabuf-stubs.c        |  17 +++++
> >   hw/vfio/meson.build           |   1 +
> >   4 files changed, 155 insertions(+)
> >   create mode 100644 hw/vfio/dmabuf-stubs.c
> >
> > diff --git a/include/hw/vfio/vfio-device.h b/include/hw/vfio/vfio-
> device.h
> > index 828a31c006..d46640ff53 100644
> > --- a/include/hw/vfio/vfio-device.h
> > +++ b/include/hw/vfio/vfio-device.h
> > @@ -41,6 +41,13 @@ enum {
> >       VFIO_DEVICE_TYPE_AP = 3,
> >   };
> >
> > +enum {
> > +    /* The Guest passed addresses stored in IOV are invalid */
> > +    VFIO_DMABUF_CREATE_ERR_INVALID_IOV = -1,
> > +    /* Guest or Host may be at fault for this type of error */
> > +    VFIO_DMABUF_CREATE_ERR_UNSPEC = -2,
> > +};
> 
> Sorry, I am not convinced this is useful.
Ok, I'll move these error enums elsewhere or not use them in VFIO.
The goal/idea here is to identify the error case where Guest passed
in invalid addresses that do not belong to VFIO or virtio-gpu and log
them with qemu_log_mask(). In all other cases (of errors), we want
to categorize the errors as Host related and report them using
error_report_err().

We have done it this way because there is no way to know (ahead of
time) if the Guest passed addresses belong to VFIO or virtio-gpu or
invalid. So, we always first try to create a dmabuf via udmabuf and if
that fails, try via VFIO and then bail out with a qemu_log_mask() if we
cannot create a dmabuf via VFIO as well.

> 
> > +
> >   typedef struct VFIODeviceOps VFIODeviceOps;
> >   typedef struct VFIODeviceIOOps VFIODeviceIOOps;
> >   typedef struct VFIOMigration VFIOMigration;
> > @@ -308,6 +315,22 @@ bool vfio_device_has_region_cap(VFIODevice
> *vbasedev, int region, uint16_t cap_t
> >
> >   int vfio_device_get_irq_info(VFIODevice *vbasedev, int index,
> >                                   struct vfio_irq_info *info);
> > +
> > +/**
> > + * Create a dmabuf fd by first translating the addresses in the
> > + * iovec array into VFIO region offsets and then invoking the
> > + * VFIO_DEVICE_FEATURE_DMA_BUF feature.
> > + *
> > + * @iov: array of iovec entries associated with a buffer
> > + * @iov_cnt: number of entries in the iovec array
> > + * @total_size: total size of the dmabuf
> > + * @errp: pointer to Error*, to store an error if it happens.
> > + *
> > + * Returns the created dmabuf fd or either
> VFIO_DMABUF_CREATE_ERR_UNSPEC
> > + * or VFIO_DMABUF_CREATE_ERR_INVALID_IOV on error.
> > + */
> > +int vfio_device_create_dmabuf_fd(struct iovec *iov, unsigned int
> iov_cnt,
> > +                                 size_t total_size, Error **errp);
> >   #endif
> >
> >   /* Returns 0 on success, or a negative errno. */
> > diff --git a/hw/vfio/device.c b/hw/vfio/device.c
> > index 973fc35b59..542c378913 100644
> > --- a/hw/vfio/device.c
> > +++ b/hw/vfio/device.c
> > @@ -21,7 +21,9 @@
> >   #include "qemu/osdep.h"
> >   #include <sys/ioctl.h>
> >
> > +#include "system/ramblock.h"
> >   #include "hw/vfio/vfio-device.h"
> > +#include "hw/vfio/vfio-region.h"
> >   #include "hw/vfio/pci.h"
> >   #include "hw/core/iommu.h"
> >   #include "hw/core/hw-error.h"
> > @@ -644,3 +646,115 @@ static VFIODeviceIOOps
> vfio_device_io_ops_ioctl = {
> >       .region_read = vfio_device_io_region_read,
> >       .region_write = vfio_device_io_region_write,
> >   };
> > +
> > +/*
> > + * This helper looks up the VFIODevice corresponding to the given
> iov. It
> > + * can be useful to determinine if a buffer (represented by the iov)
> belongs
> > + * to a VFIO device or not. This is mainly invoked when external
> components
> > + * such as virtio-gpu request creation of dmabuf fds for buffers that
> may
> > + * belong to a VFIO device.
> > + */
> > +static bool vfio_device_lookup(struct iovec *iov, VFIODevice
> **vbasedevp,
> > +                               RAMBlock **first_rbp, Error **errp)
> > +{
> > +    VFIODevice *vbasedev;
> > +    RAMBlock *first_rb;
> > +    ram_addr_t offset;
> > +
> > +    first_rb = qemu_ram_block_from_host(iov[0].iov_base, false,
> &offset);
> > +    if (!first_rb) {
> > +        error_setg(errp, "Could not find first ramblock\n");
> > +        return false;
> > +    }
> > +
> > +    *first_rbp = first_rb;
> > +    QLIST_FOREACH(vbasedev, &vfio_device_list, next) {
> > +        if (vbasedev->dev == first_rb->mr->dev) {
> > +            *vbasedevp = vbasedev;
> > +            return true;
> > +        }
> > +    }
> > +    error_setg(errp, "No VFIO device found to create dmabuf from\n");
> > +    return false;
> > +}
> > +
> > +/*
> > + * This helper looks up the VFIORegion corresponding to the given
> address.
> > + * It also verifies that the RAMBlock associated with the address is
> the
> > + * same as the first_rb passed in. This is to ensure that all addresses
> > + * in the iov belong to the same region.
> > + */
> > +static bool vfio_region_lookup(void *addr, VFIORegion **regionp,
> > +                               RAMBlock *first_rb, ram_addr_t *offsetp,
> > +                               Error **errp)
> > +{
> > +    VFIORegion *region;
> > +    RAMBlock *rb;
> > +
> > +    rb = qemu_ram_block_from_host(addr, false, offsetp);
> > +    if (!rb || rb != first_rb) {
> > +        error_setg(errp, "Dmabuf segments must belong to the same
> region\n");
> > +        return false;
> > +    }
> > +
> > +    region = vfio_get_region_from_mr(rb->mr);
> > +    if (region) {
> > +        *regionp = region;
> > +        return true;
> > +    }
> > +    error_setg(errp, "Could not find valid region for dmabuf
> segment\n");
> > +    return false;
> > +}
> > +
> > +int vfio_device_create_dmabuf_fd(struct iovec *iov, unsigned int
> iov_cnt,
> > +                                 size_t total_size, Error **errp)
> > +{
> > +    g_autofree struct vfio_device_feature *feature = NULL;
> > +    struct vfio_device_feature_dma_buf *dma_buf;
> > +    RAMBlock *first_rb = NULL;
> > +    VFIODevice *vbasedev;
> > +    VFIORegion *region;
> > +    ram_addr_t offset;
> > +    size_t argsz;
> > +    int i, ret;
> > +
> > +    if (iov_size(iov, iov_cnt) != total_size) {
> > +        error_setg(errp, "Total size of iov does not match dmabuf
> size\n");
> > +        return VFIO_DMABUF_CREATE_ERR_INVALID_IOV;
> 
> This returned value is redundant with errp and error-prone for an API.
> I think you should find an alternative implementation. May be these
> routines simply do not belong to VFIO.
Ok, we'll try to explore an alternative that would probably not have these
error enums in VFIO.

> 
> 
> > +    }
> > +
> > +    if (!vfio_device_lookup(iov, &vbasedev, &first_rb, errp)) {
> > +        return VFIO_DMABUF_CREATE_ERR_INVALID_IOV;
> > +    }
> > +
> > +    argsz = sizeof(*feature) + sizeof (*dma_buf) +
> > +            sizeof(struct vfio_region_dma_range) * iov_cnt;
> > +    feature = g_malloc0(argsz);
> > +    *feature = (struct vfio_device_feature) {
> > +        .argsz = argsz,
> > +        .flags = VFIO_DEVICE_FEATURE_GET |
> VFIO_DEVICE_FEATURE_DMA_BUF,
> > +    };
> > +    dma_buf = (struct vfio_device_feature_dma_buf *)feature->data;
> > +
> > +    for (i = 0; i < iov_cnt; i++) {
> > +        if (!vfio_region_lookup(iov[i].iov_base, &region,
> > +                                first_rb, &offset, errp)) {
> > +            return VFIO_DMABUF_CREATE_ERR_INVALID_IOV;
> > +        }
> > +
> > +        dma_buf->region_index = region->nr;
> > +        dma_buf->dma_ranges[i].offset = offset;
> > +        dma_buf->dma_ranges[i].length = iov[i].iov_len;
> > +    }
> > +
> > +    dma_buf->nr_ranges = iov_cnt;
> > +    dma_buf->open_flags = O_RDONLY | O_CLOEXEC;
> > +
> > +    ret = vfio_device_get_feature(vbasedev, feature);
> > +    if (ret < 0) {
> > +        error_setg_errno(errp, -ret,
> > +                         "Could not create dmabuf fd via VFIO device");
> > +        return VFIO_DMABUF_CREATE_ERR_UNSPEC;
> > +    }
> > +    return ret;
> > +}
> > diff --git a/hw/vfio/dmabuf-stubs.c b/hw/vfio/dmabuf-stubs.c
> > new file mode 100644
> > index 0000000000..374bd2a188
> > --- /dev/null
> > +++ b/hw/vfio/dmabuf-stubs.c
> > @@ -0,0 +1,17 @@
> > +/*
> > + * 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"
> > +#include "hw/vfio/vfio-device.h"
> > +
> > +int vfio_device_create_dmabuf_fd(struct iovec *iov, unsigned int
> iov_cnt,
> > +                                 size_t total_size, Error **errp)
> > +{
> > +    error_setg(errp, "VFIO dmabuf support is not enabled");
> > +    return VFIO_DMABUF_CREATE_HOST_ERROR;
> 
> ../hw/vfio/dmabuf-stubs.c:16:12: error:
> 'VFIO_DMABUF_CREATE_HOST_ERROR' undeclared (first use in this
> function)
>     16 |     return VFIO_DMABUF_CREATE_HOST_ERROR;
Will fix it in the next version.

Thanks,
Vivek

> 
> Thanks,
> 
> C.
> 
> 
> 
> > +}
> > diff --git a/hw/vfio/meson.build b/hw/vfio/meson.build
> > index 82f68698fb..ac899d30a8 100644
> > --- a/hw/vfio/meson.build
> > +++ b/hw/vfio/meson.build
> > @@ -34,3 +34,4 @@ system_ss.add(when: 'CONFIG_IOMMUFD',
> if_false: files('iommufd-stubs.c'))
> >   system_ss.add(when: 'CONFIG_VFIO_PCI', if_true: files(
> >     'display.c',
> >   ))
> > +system_ss.add(when: 'CONFIG_VFIO', if_false: files('dmabuf-stubs.c'))

RE: [PATCH v12 06/10] vfio/region: Add a helper to get VFIO region from memory region

[Kasireddy, Vivek]

> Subject: Re: [PATCH v12 06/10] vfio/region: Add a helper to get VFIO
> region from memory region
> 
> On 3/19/26 06:15, Vivek Kasireddy wrote:
> > Having a way to get the VFIO region associated with a memory region
> > is helpful in various scenarios. For example, this capability can
> > be useful in retrieving the region info such as index and offset
> > needed for mapping a part of a VFIO region or creating a dmabuf.
> >
> > Cc: Alex Williamson <alex@shazbot.org>
> > Cc: Cédric Le Goater <clg@redhat.com>
> > Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> > ---
> >   hw/vfio/vfio-region.h | 10 ++++++++++
> >   hw/vfio/region.c      | 11 +++++++++++
> >   2 files changed, 21 insertions(+)
> >
> > diff --git a/hw/vfio/vfio-region.h b/hw/vfio/vfio-region.h
> > index 9b21d4ee5b..3dd47f77f6 100644
> > --- a/hw/vfio/vfio-region.h
> > +++ b/hw/vfio/vfio-region.h
> > @@ -45,4 +45,14 @@ void vfio_region_unmap(VFIORegion *region);
> >   void vfio_region_exit(VFIORegion *region);
> >   void vfio_region_finalize(VFIORegion *region);
> >
> > +/**
> > + * Return the VFIO region associated with a given MemoryRegion.
> This can
> > + * be useful in retrieving region info such as index and offset.
> > + *
> > + * @mr: MemoryRegion to use
> > + *
> > + * Returns the region or NULL on error.
> > + */
> > +void *vfio_get_region_from_mr(MemoryRegion *mr);
> > +
> >   #endif /* HW_VFIO_REGION_H */
> > diff --git a/hw/vfio/region.c b/hw/vfio/region.c
> > index 47fdc2df34..9d7ac339c9 100644
> > --- a/hw/vfio/region.c
> > +++ b/hw/vfio/region.c
> > @@ -539,3 +539,14 @@ void
> vfio_region_mmaps_set_enabled(VFIORegion *region, bool enabled)
> >
> trace_vfio_region_mmaps_set_enabled(memory_region_name(region-
> >mem),
> >                                           enabled);
> >   }
> > +
> > +void *vfio_get_region_from_mr(MemoryRegion *mr)
> 
> can we return a "VFIORegion *" ?
Sure, I'll make the change.

Thanks,
Vivek

> 
> Thanks,
> 
> C.
> 
> > +{
> > +    while (mr->container) {
> > +        if (mr->ops == &vfio_region_ops) {
> > +            return mr->opaque;
> > +        }
> > +        mr = mr->container;
> > +    }
> > +    return NULL;
> > +}

RE: [PATCH v12 09/10] virtio-gpu-dmabuf: Improve error handling with 'Error **' and err enum

[Kasireddy, Vivek]

Hi Cedric,

> Subject: Re: [PATCH v12 09/10] virtio-gpu-dmabuf: Improve error
> handling with 'Error **' and err enum
> 
> On 3/19/26 06:15, Vivek Kasireddy wrote:
> > Make the error handling more robust in virtio_gpu_init_udmabuf()
> > by introducing 'Error **' parameter to capture errors and using
> > an enum from VFIO to categorize different errors. This allows for
> > better error reporting and handling of errors from
> > virtio_gpu_create_udmabuf() and virtio_gpu_remap_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>
> > Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> > Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> > ---
> >   hw/display/virtio-gpu-dmabuf.c | 67 +++++++++++++++++++++++-------
> ----
> >   1 file changed, 45 insertions(+), 22 deletions(-)
> >
> > diff --git a/hw/display/virtio-gpu-dmabuf.c b/hw/display/virtio-gpu-
> dmabuf.c
> > index e35f7714a9..89aa487654 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"
> > @@ -27,16 +28,18 @@
> >   #include "standard-headers/linux/udmabuf.h"
> >   #include "standard-headers/drm/drm_fourcc.h"
> >
> > -static void virtio_gpu_create_udmabuf(struct
> virtio_gpu_simple_resource *res)
> > +static int virtio_gpu_create_udmabuf(struct
> virtio_gpu_simple_resource *res,
> > +                                     Error **errp)
> >   {
> >       g_autofree struct udmabuf_create_list *list = NULL;
> >       RAMBlock *rb;
> >       ram_addr_t offset;
> > -    int udmabuf, i;
> > +    int udmabuf, i, fd;
> >
> >       udmabuf = udmabuf_fd();
> >       if (udmabuf < 0) {
> > -        return;
> > +        error_setg(errp, "udmabuf device not available or enabled");
> > +        return VFIO_DMABUF_CREATE_ERR_UNSPEC;
> 
> The function virtio_gpu_create_udmabuf() is returning VFIO_DMABUF_*
> enum
> values, which is problematic because the function creates a udmabuf,
> not
> a VFIO dmabuf.
> 
> This creates a layering violation. The virtio-gpu-dmabuf code (which
> handles both udmabuf and VFIO dmabuf creation) is using error codes
> defined in the VFIO-specific header.
> 
> Please find another solution.
Other solutions I can think of are either move these error enums into virtio-gpu
(and disregard the error return type from vfio) or move them to some other header
where they are visible to both virtio-gpu and vfio. I'd like hear Akihiko's thoughts/
comments on how to proceed given that he had reviewed virtio-gpu patches in
this series.

> 
> 
> 
> >       }
> >
> >       list = g_malloc0(sizeof(struct udmabuf_create_list) +
> > @@ -45,7 +48,8 @@ static void virtio_gpu_create_udmabuf(struct
> virtio_gpu_simple_resource *res)
> >       for (i = 0; i < res->iov_cnt; i++) {
> >           rb = qemu_ram_block_from_host(res->iov[i].iov_base, false,
> &offset);
> >           if (!rb || rb->fd < 0) {
> > -            return;
> > +            error_setg(errp, "IOV memory address incompatible with
> udmabuf ");
> > +            return VFIO_DMABUF_CREATE_ERR_INVALID_IOV;
> >           }
> >
> >           list->list[i].memfd  = rb->fd;
> > @@ -56,22 +60,28 @@ static void virtio_gpu_create_udmabuf(struct
> virtio_gpu_simple_resource *res)
> >       list->count = res->iov_cnt;
> >       list->flags = UDMABUF_FLAGS_CLOEXEC;
> >
> > -    res->dmabuf_fd = ioctl(udmabuf, UDMABUF_CREATE_LIST, list);
> > -    if (res->dmabuf_fd < 0) {
> > -        warn_report("%s: UDMABUF_CREATE_LIST: %s", __func__,
> > -                    strerror(errno));
> > +    fd = ioctl(udmabuf, UDMABUF_CREATE_LIST, list);
> > +    if (fd < 0) {
> > +        error_setg_errno(errp, errno, "UDMABUF_CREATE_LIST: ioctl
> failed");
> > +        if (errno == EINVAL || errno == EBADFD) {
> > +            return VFIO_DMABUF_CREATE_ERR_INVALID_IOV;
> > +        }
> > +        return VFIO_DMABUF_CREATE_ERR_UNSPEC;
> >       }
> > +    return fd;
> >   }
> >
> > -static void virtio_gpu_remap_dmabuf(struct
> virtio_gpu_simple_resource *res)
> > +static void *virtio_gpu_remap_dmabuf(struct
> virtio_gpu_simple_resource *res,
> > +                                     Error **errp)
> >   {
> > -    res->remapped = mmap(NULL, res->blob_size, PROT_READ,
> > -                         MAP_SHARED, res->dmabuf_fd, 0);
> > -    if (res->remapped == MAP_FAILED) {
> > -        warn_report("%s: dmabuf mmap failed: %s", __func__,
> > -                    strerror(errno));
> > -        res->remapped = 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");
> > +        return NULL;
> >       }
> > +    return map;
> >   }
> >
> >   static void virtio_gpu_destroy_dmabuf(struct
> virtio_gpu_simple_resource *res)
> > @@ -125,22 +135,35 @@ bool virtio_gpu_have_udmabuf(void)
> >
> >   void virtio_gpu_init_dmabuf(struct virtio_gpu_simple_resource *res)
> >   {
> > +    Error *local_err = NULL;
> >       void *pdata = NULL;
> >
> > -    res->dmabuf_fd = -1;
> >       if (res->iov_cnt == 1 &&
> >           res->iov[0].iov_len < 4096) {
> > +        res->dmabuf_fd = -1;
> >           pdata = res->iov[0].iov_base;
> >       } else {
> > -        virtio_gpu_create_udmabuf(res);
> > +        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);
> 
> Why not report the error in the QEMU log below ?
I think the idea is that in the case of INVALID_IOV error, it is sufficient
to just report that the Guest passed in incompatible memory addresses.
But I guess we could also just do:
qemu_log_mask(LOG_GUEST_ERROR, "%s\n", error_get_pretty(local_err));

Thanks,
Vivek
> 
> > +
> > +            qemu_log_mask(LOG_GUEST_ERROR,
> > +                          "Cannot create dmabuf: incompatible memory\n");
> > +            return;
> > +        } else if (res->dmabuf_fd >= 0) {
> > +            pdata = virtio_gpu_remap_dmabuf(res, &local_err);
> > +            if (!pdata) {
> > +                virtio_gpu_destroy_dmabuf(res);
> > +            }
> > +        } else {
> > +            res->dmabuf_fd = -1;
> > +        }
> > +
> >           if (res->dmabuf_fd < 0) {
> > +            error_report_err(local_err);
> >               return;
> >           }
> > -        virtio_gpu_remap_dmabuf(res);
> > -        if (!res->remapped) {
> > -            return;
> > -        }
> > -        pdata = res->remapped;
> > +        res->remapped = pdata;
> >       }
> >
> >       res->blob = pdata;

Re: [PATCH v12 09/10] virtio-gpu-dmabuf: Improve error handling with 'Error **' and err enum

[Akihiko Odaki]

On 2026/03/24 14:53, Kasireddy, Vivek wrote:
> Hi Cedric,
> 
>> Subject: Re: [PATCH v12 09/10] virtio-gpu-dmabuf: Improve error
>> handling with 'Error **' and err enum
>>
>> On 3/19/26 06:15, Vivek Kasireddy wrote:
>>> Make the error handling more robust in virtio_gpu_init_udmabuf()
>>> by introducing 'Error **' parameter to capture errors and using
>>> an enum from VFIO to categorize different errors. This allows for
>>> better error reporting and handling of errors from
>>> virtio_gpu_create_udmabuf() and virtio_gpu_remap_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>
>>> Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
>>> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
>>> ---
>>>    hw/display/virtio-gpu-dmabuf.c | 67 +++++++++++++++++++++++-------
>> ----
>>>    1 file changed, 45 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/hw/display/virtio-gpu-dmabuf.c b/hw/display/virtio-gpu-
>> dmabuf.c
>>> index e35f7714a9..89aa487654 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"
>>> @@ -27,16 +28,18 @@
>>>    #include "standard-headers/linux/udmabuf.h"
>>>    #include "standard-headers/drm/drm_fourcc.h"
>>>
>>> -static void virtio_gpu_create_udmabuf(struct
>> virtio_gpu_simple_resource *res)
>>> +static int virtio_gpu_create_udmabuf(struct
>> virtio_gpu_simple_resource *res,
>>> +                                     Error **errp)
>>>    {
>>>        g_autofree struct udmabuf_create_list *list = NULL;
>>>        RAMBlock *rb;
>>>        ram_addr_t offset;
>>> -    int udmabuf, i;
>>> +    int udmabuf, i, fd;
>>>
>>>        udmabuf = udmabuf_fd();
>>>        if (udmabuf < 0) {
>>> -        return;
>>> +        error_setg(errp, "udmabuf device not available or enabled");
>>> +        return VFIO_DMABUF_CREATE_ERR_UNSPEC;
>>
>> The function virtio_gpu_create_udmabuf() is returning VFIO_DMABUF_*
>> enum
>> values, which is problematic because the function creates a udmabuf,
>> not
>> a VFIO dmabuf.
>>
>> This creates a layering violation. The virtio-gpu-dmabuf code (which
>> handles both udmabuf and VFIO dmabuf creation) is using error codes
>> defined in the VFIO-specific header.

The rationale of using error codes is as follows:

 > 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).

https://lore.kernel.org/qemu-devel/becde56b-90bd-40ca-9329-0c92b9519a67@rsg.ci.i.u-tokyo.ac.jp/

>>
>> Please find another solution.
> Other solutions I can think of are either move these error enums into virtio-gpu
> (and disregard the error return type from vfio) or move them to some other header
> where they are visible to both virtio-gpu and vfio. I'd like hear Akihiko's thoughts/
> comments on how to proceed given that he had reviewed virtio-gpu patches in
> this series.

Disregarding the error conditions of VFIO is not OK. That can cause a 
guest error to be reported as a host error.

I think the simplest alternative is just to have a duplicate enum for 
virtio-gpu.

> 
>>
>>
>>
>>>        }
>>>
>>>        list = g_malloc0(sizeof(struct udmabuf_create_list) +
>>> @@ -45,7 +48,8 @@ static void virtio_gpu_create_udmabuf(struct
>> virtio_gpu_simple_resource *res)
>>>        for (i = 0; i < res->iov_cnt; i++) {
>>>            rb = qemu_ram_block_from_host(res->iov[i].iov_base, false,
>> &offset);
>>>            if (!rb || rb->fd < 0) {
>>> -            return;
>>> +            error_setg(errp, "IOV memory address incompatible with
>> udmabuf ");
>>> +            return VFIO_DMABUF_CREATE_ERR_INVALID_IOV;
>>>            }
>>>
>>>            list->list[i].memfd  = rb->fd;
>>> @@ -56,22 +60,28 @@ static void virtio_gpu_create_udmabuf(struct
>> virtio_gpu_simple_resource *res)
>>>        list->count = res->iov_cnt;
>>>        list->flags = UDMABUF_FLAGS_CLOEXEC;
>>>
>>> -    res->dmabuf_fd = ioctl(udmabuf, UDMABUF_CREATE_LIST, list);
>>> -    if (res->dmabuf_fd < 0) {
>>> -        warn_report("%s: UDMABUF_CREATE_LIST: %s", __func__,
>>> -                    strerror(errno));
>>> +    fd = ioctl(udmabuf, UDMABUF_CREATE_LIST, list);
>>> +    if (fd < 0) {
>>> +        error_setg_errno(errp, errno, "UDMABUF_CREATE_LIST: ioctl
>> failed");
>>> +        if (errno == EINVAL || errno == EBADFD) {
>>> +            return VFIO_DMABUF_CREATE_ERR_INVALID_IOV;
>>> +        }
>>> +        return VFIO_DMABUF_CREATE_ERR_UNSPEC;
>>>        }
>>> +    return fd;
>>>    }
>>>
>>> -static void virtio_gpu_remap_dmabuf(struct
>> virtio_gpu_simple_resource *res)
>>> +static void *virtio_gpu_remap_dmabuf(struct
>> virtio_gpu_simple_resource *res,
>>> +                                     Error **errp)
>>>    {
>>> -    res->remapped = mmap(NULL, res->blob_size, PROT_READ,
>>> -                         MAP_SHARED, res->dmabuf_fd, 0);
>>> -    if (res->remapped == MAP_FAILED) {
>>> -        warn_report("%s: dmabuf mmap failed: %s", __func__,
>>> -                    strerror(errno));
>>> -        res->remapped = 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");
>>> +        return NULL;
>>>        }
>>> +    return map;
>>>    }
>>>
>>>    static void virtio_gpu_destroy_dmabuf(struct
>> virtio_gpu_simple_resource *res)
>>> @@ -125,22 +135,35 @@ bool virtio_gpu_have_udmabuf(void)
>>>
>>>    void virtio_gpu_init_dmabuf(struct virtio_gpu_simple_resource *res)
>>>    {
>>> +    Error *local_err = NULL;
>>>        void *pdata = NULL;
>>>
>>> -    res->dmabuf_fd = -1;
>>>        if (res->iov_cnt == 1 &&
>>>            res->iov[0].iov_len < 4096) {
>>> +        res->dmabuf_fd = -1;
>>>            pdata = res->iov[0].iov_base;
>>>        } else {
>>> -        virtio_gpu_create_udmabuf(res);
>>> +        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);
>>
>> Why not report the error in the QEMU log below ?
> I think the idea is that in the case of INVALID_IOV error, it is sufficient
> to just report that the Guest passed in incompatible memory addresses.
> But I guess we could also just do:
> qemu_log_mask(LOG_GUEST_ERROR, "%s\n", error_get_pretty(local_err));

The error message may not be useful. e.g., "UDMABUF_CREATE_LIST: ioctl 
failed" does not tell what error the guest made and how to fix it.

Regards,
Akihiko Odaki

> 
> Thanks,
> Vivek
>>
>>> +
>>> +            qemu_log_mask(LOG_GUEST_ERROR,
>>> +                          "Cannot create dmabuf: incompatible memory\n");
>>> +            return;
>>> +        } else if (res->dmabuf_fd >= 0) {
>>> +            pdata = virtio_gpu_remap_dmabuf(res, &local_err);
>>> +            if (!pdata) {
>>> +                virtio_gpu_destroy_dmabuf(res);
>>> +            }
>>> +        } else {
>>> +            res->dmabuf_fd = -1;
>>> +        }
>>> +
>>>            if (res->dmabuf_fd < 0) {
>>> +            error_report_err(local_err);
>>>                return;
>>>            }
>>> -        virtio_gpu_remap_dmabuf(res);
>>> -        if (!res->remapped) {
>>> -            return;
>>> -        }
>>> -        pdata = res->remapped;
>>> +        res->remapped = pdata;
>>>        }
>>>
>>>        res->blob = pdata;
> 

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

[Akihiko Odaki]

On 2026/03/19 14:15, 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>
> Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> ---
>   hw/display/virtio-gpu-dmabuf.c | 23 ++++++++++++++++++++---
>   1 file changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/display/virtio-gpu-dmabuf.c b/hw/display/virtio-gpu-dmabuf.c
> index 89aa487654..f953db0fbe 100644
> --- a/hw/display/virtio-gpu-dmabuf.c
> +++ b/hw/display/virtio-gpu-dmabuf.c
> @@ -147,9 +147,26 @@ void virtio_gpu_init_dmabuf(struct virtio_gpu_simple_resource *res)
>           if (res->dmabuf_fd == VFIO_DMABUF_CREATE_ERR_INVALID_IOV) {
>               error_free_or_abort(&local_err);
>   
> -            qemu_log_mask(LOG_GUEST_ERROR,
> -                          "Cannot create dmabuf: incompatible memory\n");
> -            return;
> +            res->dmabuf_fd = vfio_device_create_dmabuf_fd(res->iov,
> +                                                          res->iov_cnt,
> +                                                          res->blob_size,

The correspondence between (iov, iov_cnt) and blob_size is more of a 
internal concern of virtio-gpu, not of VFIO. This parameter is better 
removed from vfio_device_create_dmabuf_fd() and vfio_device_mmap_dmabuf().

Regards,
Akihiko Odaki

> +                                                          &local_err);
> +            if (res->dmabuf_fd == VFIO_DMABUF_CREATE_ERR_INVALID_IOV) {
> +                error_free_or_abort(&local_err);
> +                qemu_log_mask(LOG_GUEST_ERROR,
> +                              "Cannot create dmabuf: incompatible memory\n");
> +                return;
> +            }
> +
> +            if (res->dmabuf_fd >= 0) {
> +                pdata = vfio_device_mmap_dmabuf(res->iov, res->iov_cnt,
> +                                                res->blob_size, &local_err);
> +                if (!pdata) {
> +                    virtio_gpu_destroy_dmabuf(res);
> +                }
> +            } else {
> +                res->dmabuf_fd = -1;
> +            }
>           } else if (res->dmabuf_fd >= 0) {
>               pdata = virtio_gpu_remap_dmabuf(res, &local_err);
>               if (!pdata) {

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

[Kasireddy, Vivek]

Hi Akihiko,

> Subject: Re: [PATCH v12 10/10] virtio-gpu-dmabuf: Create dmabuf for
> blobs associated with VFIO devices
> 
> On 2026/03/19 14:15, 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>
> > Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> > Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> > ---
> >   hw/display/virtio-gpu-dmabuf.c | 23 ++++++++++++++++++++---
> >   1 file changed, 20 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/display/virtio-gpu-dmabuf.c b/hw/display/virtio-gpu-
> dmabuf.c
> > index 89aa487654..f953db0fbe 100644
> > --- a/hw/display/virtio-gpu-dmabuf.c
> > +++ b/hw/display/virtio-gpu-dmabuf.c
> > @@ -147,9 +147,26 @@ void virtio_gpu_init_dmabuf(struct
> virtio_gpu_simple_resource *res)
> >           if (res->dmabuf_fd ==
> VFIO_DMABUF_CREATE_ERR_INVALID_IOV) {
> >               error_free_or_abort(&local_err);
> >
> > -            qemu_log_mask(LOG_GUEST_ERROR,
> > -                          "Cannot create dmabuf: incompatible memory\n");
> > -            return;
> > +            res->dmabuf_fd = vfio_device_create_dmabuf_fd(res->iov,
> > +                                                          res->iov_cnt,
> > +                                                          res->blob_size,
> 
> The correspondence between (iov, iov_cnt) and blob_size is more of a
> internal concern of virtio-gpu, not of VFIO. This parameter is better
> removed from vfio_device_create_dmabuf_fd() and
> vfio_device_mmap_dmabuf().
I don't disagree. So, should we add the following check in
virtio_gpu_init_dmabuf() or somewhere?
if (iov_size(iov, iov_cnt) != blob_size)

Thanks,
Vivek
> 
> Regards,
> Akihiko Odaki
> 
> > +                                                          &local_err);
> > +            if (res->dmabuf_fd ==
> VFIO_DMABUF_CREATE_ERR_INVALID_IOV) {
> > +                error_free_or_abort(&local_err);
> > +                qemu_log_mask(LOG_GUEST_ERROR,
> > +                              "Cannot create dmabuf: incompatible memory\n");
> > +                return;
> > +            }
> > +
> > +            if (res->dmabuf_fd >= 0) {
> > +                pdata = vfio_device_mmap_dmabuf(res->iov, res->iov_cnt,
> > +                                                res->blob_size, &local_err);
> > +                if (!pdata) {
> > +                    virtio_gpu_destroy_dmabuf(res);
> > +                }
> > +            } else {
> > +                res->dmabuf_fd = -1;
> > +            }
> >           } else if (res->dmabuf_fd >= 0) {
> >               pdata = virtio_gpu_remap_dmabuf(res, &local_err);
> >               if (!pdata) {

RE: [PATCH v12 09/10] virtio-gpu-dmabuf: Improve error handling with 'Error **' and err enum

[Kasireddy, Vivek]

Hi Akihiko,

> Subject: Re: [PATCH v12 09/10] virtio-gpu-dmabuf: Improve error
> handling with 'Error **' and err enum
> >>
> >> On 3/19/26 06:15, Vivek Kasireddy wrote:
> >>> Make the error handling more robust in virtio_gpu_init_udmabuf()
> >>> by introducing 'Error **' parameter to capture errors and using
> >>> an enum from VFIO to categorize different errors. This allows for
> >>> better error reporting and handling of errors from
> >>> virtio_gpu_create_udmabuf() and virtio_gpu_remap_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>
> >>> Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> >>> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> >>> ---
> >>>    hw/display/virtio-gpu-dmabuf.c | 67 +++++++++++++++++++++++--
> -----
> >> ----
> >>>    1 file changed, 45 insertions(+), 22 deletions(-)
> >>>
> >>> diff --git a/hw/display/virtio-gpu-dmabuf.c b/hw/display/virtio-gpu-
> >> dmabuf.c
> >>> index e35f7714a9..89aa487654 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"
> >>> @@ -27,16 +28,18 @@
> >>>    #include "standard-headers/linux/udmabuf.h"
> >>>    #include "standard-headers/drm/drm_fourcc.h"
> >>>
> >>> -static void virtio_gpu_create_udmabuf(struct
> >> virtio_gpu_simple_resource *res)
> >>> +static int virtio_gpu_create_udmabuf(struct
> >> virtio_gpu_simple_resource *res,
> >>> +                                     Error **errp)
> >>>    {
> >>>        g_autofree struct udmabuf_create_list *list = NULL;
> >>>        RAMBlock *rb;
> >>>        ram_addr_t offset;
> >>> -    int udmabuf, i;
> >>> +    int udmabuf, i, fd;
> >>>
> >>>        udmabuf = udmabuf_fd();
> >>>        if (udmabuf < 0) {
> >>> -        return;
> >>> +        error_setg(errp, "udmabuf device not available or enabled");
> >>> +        return VFIO_DMABUF_CREATE_ERR_UNSPEC;
> >>
> >> The function virtio_gpu_create_udmabuf() is returning
> VFIO_DMABUF_*
> >> enum
> >> values, which is problematic because the function creates a
> udmabuf,
> >> not
> >> a VFIO dmabuf.
> >>
> >> This creates a layering violation. The virtio-gpu-dmabuf code (which
> >> handles both udmabuf and VFIO dmabuf creation) is using error
> codes
> >> defined in the VFIO-specific header.
> 
> The rationale of using error codes is as follows:
> 
>  > 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).
> 
> https://lore.kernel.org/qemu-devel/becde56b-90bd-40ca-9329-
> 0c92b9519a67@rsg.ci.i.u-tokyo.ac.jp/
> 
> >>
> >> Please find another solution.
> > Other solutions I can think of are either move these error enums into
> virtio-gpu
> > (and disregard the error return type from vfio) or move them to some
> other header
> > where they are visible to both virtio-gpu and vfio. I'd like hear
> Akihiko's thoughts/
> > comments on how to proceed given that he had reviewed virtio-gpu
> patches in
> > this series.
> 
> Disregarding the error conditions of VFIO is not OK. That can cause a
> guest error to be reported as a host error.
> 
> I think the simplest alternative is just to have a duplicate enum for
> virtio-gpu.
That would address the layering violation but Cedric's other concern is
that he believes having errp is sufficient and using these error enums in
VFIO would be overkill.

Thanks,
Vivek
> 
> >
> >>
> >>
> >>
> >>>        }
> >>>
> >>>        list = g_malloc0(sizeof(struct udmabuf_create_list) +
> >>> @@ -45,7 +48,8 @@ static void virtio_gpu_create_udmabuf(struct
> >> virtio_gpu_simple_resource *res)
> >>>        for (i = 0; i < res->iov_cnt; i++) {
> >>>            rb = qemu_ram_block_from_host(res->iov[i].iov_base, false,
> >> &offset);
> >>>            if (!rb || rb->fd < 0) {
> >>> -            return;
> >>> +            error_setg(errp, "IOV memory address incompatible with
> >> udmabuf ");
> >>> +            return VFIO_DMABUF_CREATE_ERR_INVALID_IOV;
> >>>            }
> >>>
> >>>            list->list[i].memfd  = rb->fd;
> >>> @@ -56,22 +60,28 @@ static void
> virtio_gpu_create_udmabuf(struct
> >> virtio_gpu_simple_resource *res)
> >>>        list->count = res->iov_cnt;
> >>>        list->flags = UDMABUF_FLAGS_CLOEXEC;
> >>>
> >>> -    res->dmabuf_fd = ioctl(udmabuf, UDMABUF_CREATE_LIST, list);
> >>> -    if (res->dmabuf_fd < 0) {
> >>> -        warn_report("%s: UDMABUF_CREATE_LIST: %s", __func__,
> >>> -                    strerror(errno));
> >>> +    fd = ioctl(udmabuf, UDMABUF_CREATE_LIST, list);
> >>> +    if (fd < 0) {
> >>> +        error_setg_errno(errp, errno, "UDMABUF_CREATE_LIST: ioctl
> >> failed");
> >>> +        if (errno == EINVAL || errno == EBADFD) {
> >>> +            return VFIO_DMABUF_CREATE_ERR_INVALID_IOV;
> >>> +        }
> >>> +        return VFIO_DMABUF_CREATE_ERR_UNSPEC;
> >>>        }
> >>> +    return fd;
> >>>    }
> >>>
> >>> -static void virtio_gpu_remap_dmabuf(struct
> >> virtio_gpu_simple_resource *res)
> >>> +static void *virtio_gpu_remap_dmabuf(struct
> >> virtio_gpu_simple_resource *res,
> >>> +                                     Error **errp)
> >>>    {
> >>> -    res->remapped = mmap(NULL, res->blob_size, PROT_READ,
> >>> -                         MAP_SHARED, res->dmabuf_fd, 0);
> >>> -    if (res->remapped == MAP_FAILED) {
> >>> -        warn_report("%s: dmabuf mmap failed: %s", __func__,
> >>> -                    strerror(errno));
> >>> -        res->remapped = 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");
> >>> +        return NULL;
> >>>        }
> >>> +    return map;
> >>>    }
> >>>
> >>>    static void virtio_gpu_destroy_dmabuf(struct
> >> virtio_gpu_simple_resource *res)
> >>> @@ -125,22 +135,35 @@ bool virtio_gpu_have_udmabuf(void)
> >>>
> >>>    void virtio_gpu_init_dmabuf(struct virtio_gpu_simple_resource
> *res)
> >>>    {
> >>> +    Error *local_err = NULL;
> >>>        void *pdata = NULL;
> >>>
> >>> -    res->dmabuf_fd = -1;
> >>>        if (res->iov_cnt == 1 &&
> >>>            res->iov[0].iov_len < 4096) {
> >>> +        res->dmabuf_fd = -1;
> >>>            pdata = res->iov[0].iov_base;
> >>>        } else {
> >>> -        virtio_gpu_create_udmabuf(res);
> >>> +        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);
> >>
> >> Why not report the error in the QEMU log below ?
> > I think the idea is that in the case of INVALID_IOV error, it is sufficient
> > to just report that the Guest passed in incompatible memory
> addresses.
> > But I guess we could also just do:
> > qemu_log_mask(LOG_GUEST_ERROR, "%s\n",
> error_get_pretty(local_err));
> 
> The error message may not be useful. e.g., "UDMABUF_CREATE_LIST:
> ioctl
> failed" does not tell what error the guest made and how to fix it.
> 
> Regards,
> Akihiko Odaki
> 
> >
> > Thanks,
> > Vivek
> >>
> >>> +
> >>> +            qemu_log_mask(LOG_GUEST_ERROR,
> >>> +                          "Cannot create dmabuf: incompatible memory\n");
> >>> +            return;
> >>> +        } else if (res->dmabuf_fd >= 0) {
> >>> +            pdata = virtio_gpu_remap_dmabuf(res, &local_err);
> >>> +            if (!pdata) {
> >>> +                virtio_gpu_destroy_dmabuf(res);
> >>> +            }
> >>> +        } else {
> >>> +            res->dmabuf_fd = -1;
> >>> +        }
> >>> +
> >>>            if (res->dmabuf_fd < 0) {
> >>> +            error_report_err(local_err);
> >>>                return;
> >>>            }
> >>> -        virtio_gpu_remap_dmabuf(res);
> >>> -        if (!res->remapped) {
> >>> -            return;
> >>> -        }
> >>> -        pdata = res->remapped;
> >>> +        res->remapped = pdata;
> >>>        }
> >>>
> >>>        res->blob = pdata;
> >

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

[Akihiko Odaki]

On 2026/03/25 14:29, Kasireddy, Vivek wrote:
> Hi Akihiko,
> 
>> Subject: Re: [PATCH v12 10/10] virtio-gpu-dmabuf: Create dmabuf for
>> blobs associated with VFIO devices
>>
>> On 2026/03/19 14:15, 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>
>>> Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
>>> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
>>> ---
>>>    hw/display/virtio-gpu-dmabuf.c | 23 ++++++++++++++++++++---
>>>    1 file changed, 20 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/hw/display/virtio-gpu-dmabuf.c b/hw/display/virtio-gpu-
>> dmabuf.c
>>> index 89aa487654..f953db0fbe 100644
>>> --- a/hw/display/virtio-gpu-dmabuf.c
>>> +++ b/hw/display/virtio-gpu-dmabuf.c
>>> @@ -147,9 +147,26 @@ void virtio_gpu_init_dmabuf(struct
>> virtio_gpu_simple_resource *res)
>>>            if (res->dmabuf_fd ==
>> VFIO_DMABUF_CREATE_ERR_INVALID_IOV) {
>>>                error_free_or_abort(&local_err);
>>>
>>> -            qemu_log_mask(LOG_GUEST_ERROR,
>>> -                          "Cannot create dmabuf: incompatible memory\n");
>>> -            return;
>>> +            res->dmabuf_fd = vfio_device_create_dmabuf_fd(res->iov,
>>> +                                                          res->iov_cnt,
>>> +                                                          res->blob_size,
>>
>> The correspondence between (iov, iov_cnt) and blob_size is more of a
>> internal concern of virtio-gpu, not of VFIO. This parameter is better
>> removed from vfio_device_create_dmabuf_fd() and
>> vfio_device_mmap_dmabuf().
> I don't disagree. So, should we add the following check in
> virtio_gpu_init_dmabuf() or somewhere?
> if (iov_size(iov, iov_cnt) != blob_size)

I suggest to have a check in virtio_gpu_create_mapping_iov() since it's 
not even specific to DMA-BUF.

And instead let's ensure iov_size(iov, iov_cnt) >= blob_size and reject 
otherwise instead. I cited Codex's reasoning for this, which I totally 
agree. (I applied the Codex for Open Source program for access to Codex. 
And just in case: we are currently not allowed to use LLMs for writing 
patches and its use is restricted for the other purposes.)

It will be also more of a bug fix, so I think it is better to be sent as 
an independent patch instead of including it into this series.

Regards,
Akihiko Odaki

Below is the Codex's output based on commit 8e711856d763 ("Merge tag 
'hppa-fixes-for-v11-pull-request' of 
https://github.com/hdeller/qemu-hppa into staging"):

In current QEMU, omitting the check leaves inconsistent state possible, 
and the effect depends on the direction of the mismatch.

If iov_size < blob_size, this is the bad case. The dmabuf/export backing 
is built from the iov lengths in virtio-gpu-udmabuf.c (line 45) and 
virtio-gpu-udmabuf.c (line 63), but remap and scanout bounds use 
blob_size in virtio-gpu-udmabuf.c (line 73) and virtio-gpu.c (line 761). 
There is also a fast path that directly exposes iov_base as res->blob 
for small single-iov blobs in virtio-gpu-udmabuf.c (line 136), and 
scanout later uses that pointer as image data in virtio-gpu.c (line 662) 
and virtio-gpu.c (line 674). So a too-small backing is not a clean or 
obviously safe case.
If iov_size > blob_size, it is mostly a semantics issue. QEMU still 
bounds scanout using blob_size in virtio-gpu.c (line 761), so the extra 
backing is usually just unused. But the resource state is still 
inconsistent.
QEMU does not currently enforce the relationship elsewhere. blob_size 
and the iov are populated independently at blob creation in virtio-gpu.c 
(line 362) and virtio-gpu.c (line 366), and later backing attach also 
does not compare them in virtio-gpu.c (line 946).
So if the question is “can we omit the check entirely?”, the answer is: 
yes, but then you are knowingly accepting malformed guest state and 
relying on later backend behavior. If you want a defensive check, 
iov_size < blob_size is the one with a concrete justification. iov_size 
!= blob_size is harder to defend from the spec.

> 
> Thanks,
> Vivek
>>
>> Regards,
>> Akihiko Odaki
>>
>>> +                                                          &local_err);
>>> +            if (res->dmabuf_fd ==
>> VFIO_DMABUF_CREATE_ERR_INVALID_IOV) {
>>> +                error_free_or_abort(&local_err);
>>> +                qemu_log_mask(LOG_GUEST_ERROR,
>>> +                              "Cannot create dmabuf: incompatible memory\n");
>>> +                return;
>>> +            }
>>> +
>>> +            if (res->dmabuf_fd >= 0) {
>>> +                pdata = vfio_device_mmap_dmabuf(res->iov, res->iov_cnt,
>>> +                                                res->blob_size, &local_err);
>>> +                if (!pdata) {
>>> +                    virtio_gpu_destroy_dmabuf(res);
>>> +                }
>>> +            } else {
>>> +                res->dmabuf_fd = -1;
>>> +            }
>>>            } else if (res->dmabuf_fd >= 0) {
>>>                pdata = virtio_gpu_remap_dmabuf(res, &local_err);
>>>                if (!pdata) {
> 

Re: [PATCH v12 09/10] virtio-gpu-dmabuf: Improve error handling with 'Error **' and err enum

[Akihiko Odaki]

On 2026/03/25 14:31, Kasireddy, Vivek wrote:
> Hi Akihiko,
> 
>> Subject: Re: [PATCH v12 09/10] virtio-gpu-dmabuf: Improve error
>> handling with 'Error **' and err enum
>>>>
>>>> On 3/19/26 06:15, Vivek Kasireddy wrote:
>>>>> Make the error handling more robust in virtio_gpu_init_udmabuf()
>>>>> by introducing 'Error **' parameter to capture errors and using
>>>>> an enum from VFIO to categorize different errors. This allows for
>>>>> better error reporting and handling of errors from
>>>>> virtio_gpu_create_udmabuf() and virtio_gpu_remap_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>
>>>>> Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
>>>>> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
>>>>> ---
>>>>>     hw/display/virtio-gpu-dmabuf.c | 67 +++++++++++++++++++++++--
>> -----
>>>> ----
>>>>>     1 file changed, 45 insertions(+), 22 deletions(-)
>>>>>
>>>>> diff --git a/hw/display/virtio-gpu-dmabuf.c b/hw/display/virtio-gpu-
>>>> dmabuf.c
>>>>> index e35f7714a9..89aa487654 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"
>>>>> @@ -27,16 +28,18 @@
>>>>>     #include "standard-headers/linux/udmabuf.h"
>>>>>     #include "standard-headers/drm/drm_fourcc.h"
>>>>>
>>>>> -static void virtio_gpu_create_udmabuf(struct
>>>> virtio_gpu_simple_resource *res)
>>>>> +static int virtio_gpu_create_udmabuf(struct
>>>> virtio_gpu_simple_resource *res,
>>>>> +                                     Error **errp)
>>>>>     {
>>>>>         g_autofree struct udmabuf_create_list *list = NULL;
>>>>>         RAMBlock *rb;
>>>>>         ram_addr_t offset;
>>>>> -    int udmabuf, i;
>>>>> +    int udmabuf, i, fd;
>>>>>
>>>>>         udmabuf = udmabuf_fd();
>>>>>         if (udmabuf < 0) {
>>>>> -        return;
>>>>> +        error_setg(errp, "udmabuf device not available or enabled");
>>>>> +        return VFIO_DMABUF_CREATE_ERR_UNSPEC;
>>>>
>>>> The function virtio_gpu_create_udmabuf() is returning
>> VFIO_DMABUF_*
>>>> enum
>>>> values, which is problematic because the function creates a
>> udmabuf,
>>>> not
>>>> a VFIO dmabuf.
>>>>
>>>> This creates a layering violation. The virtio-gpu-dmabuf code (which
>>>> handles both udmabuf and VFIO dmabuf creation) is using error
>> codes
>>>> defined in the VFIO-specific header.
>>
>> The rationale of using error codes is as follows:
>>
>>   > 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).
>>
>> https://lore.kernel.org/qemu-devel/becde56b-90bd-40ca-9329-
>> 0c92b9519a67@rsg.ci.i.u-tokyo.ac.jp/
>>
>>>>
>>>> Please find another solution.
>>> Other solutions I can think of are either move these error enums into
>> virtio-gpu
>>> (and disregard the error return type from vfio) or move them to some
>> other header
>>> where they are visible to both virtio-gpu and vfio. I'd like hear
>> Akihiko's thoughts/
>>> comments on how to proceed given that he had reviewed virtio-gpu
>> patches in
>>> this series.
>>
>> Disregarding the error conditions of VFIO is not OK. That can cause a
>> guest error to be reported as a host error.
>>
>> I think the simplest alternative is just to have a duplicate enum for
>> virtio-gpu.
> That would address the layering violation but Cedric's other concern is
> that he believes having errp is sufficient and using these error enums in
> VFIO would be overkill.

It would be beneficial to describe the rationale behind the enums; the 
patch message only says "better error reporting", which is quite 
insufficient.

The rationale is that we need a special handling for the INVALID_IOV 
case. The INVALID_IOV case can happen in two cases:
- The memory is backed by VFIO. It will result in the INVALID_IOV error
   for the first attempt to use udmabuf, but this error can be properly
   recovered by letting the VFIO code to create a DMA-BUF instead.
- The memory is not backed by memfd nor VFIO. In this case, the error
   is a guest's fault, so we should:
   - Use qemu_log_mask(LOG_GUEST_ERROR, ...) instead of
     error_report_err().
   - Emit a message useful to diagnose the guest error, which we have
     discussed in this thread.

A common pattern is to return -errno to let the caller implement a 
special error handling, but in this case we specifically need to 
distinguish the INVALID_IOV case from the others and these enums are 
more convenient for this.

Regards,
Akihiko Odaki

> 
> Thanks,
> Vivek
>>
>>>
>>>>
>>>>
>>>>
>>>>>         }
>>>>>
>>>>>         list = g_malloc0(sizeof(struct udmabuf_create_list) +
>>>>> @@ -45,7 +48,8 @@ static void virtio_gpu_create_udmabuf(struct
>>>> virtio_gpu_simple_resource *res)
>>>>>         for (i = 0; i < res->iov_cnt; i++) {
>>>>>             rb = qemu_ram_block_from_host(res->iov[i].iov_base, false,
>>>> &offset);
>>>>>             if (!rb || rb->fd < 0) {
>>>>> -            return;
>>>>> +            error_setg(errp, "IOV memory address incompatible with
>>>> udmabuf ");
>>>>> +            return VFIO_DMABUF_CREATE_ERR_INVALID_IOV;
>>>>>             }
>>>>>
>>>>>             list->list[i].memfd  = rb->fd;
>>>>> @@ -56,22 +60,28 @@ static void
>> virtio_gpu_create_udmabuf(struct
>>>> virtio_gpu_simple_resource *res)
>>>>>         list->count = res->iov_cnt;
>>>>>         list->flags = UDMABUF_FLAGS_CLOEXEC;
>>>>>
>>>>> -    res->dmabuf_fd = ioctl(udmabuf, UDMABUF_CREATE_LIST, list);
>>>>> -    if (res->dmabuf_fd < 0) {
>>>>> -        warn_report("%s: UDMABUF_CREATE_LIST: %s", __func__,
>>>>> -                    strerror(errno));
>>>>> +    fd = ioctl(udmabuf, UDMABUF_CREATE_LIST, list);
>>>>> +    if (fd < 0) {
>>>>> +        error_setg_errno(errp, errno, "UDMABUF_CREATE_LIST: ioctl
>>>> failed");
>>>>> +        if (errno == EINVAL || errno == EBADFD) {
>>>>> +            return VFIO_DMABUF_CREATE_ERR_INVALID_IOV;
>>>>> +        }
>>>>> +        return VFIO_DMABUF_CREATE_ERR_UNSPEC;
>>>>>         }
>>>>> +    return fd;
>>>>>     }
>>>>>
>>>>> -static void virtio_gpu_remap_dmabuf(struct
>>>> virtio_gpu_simple_resource *res)
>>>>> +static void *virtio_gpu_remap_dmabuf(struct
>>>> virtio_gpu_simple_resource *res,
>>>>> +                                     Error **errp)
>>>>>     {
>>>>> -    res->remapped = mmap(NULL, res->blob_size, PROT_READ,
>>>>> -                         MAP_SHARED, res->dmabuf_fd, 0);
>>>>> -    if (res->remapped == MAP_FAILED) {
>>>>> -        warn_report("%s: dmabuf mmap failed: %s", __func__,
>>>>> -                    strerror(errno));
>>>>> -        res->remapped = 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");
>>>>> +        return NULL;
>>>>>         }
>>>>> +    return map;
>>>>>     }
>>>>>
>>>>>     static void virtio_gpu_destroy_dmabuf(struct
>>>> virtio_gpu_simple_resource *res)
>>>>> @@ -125,22 +135,35 @@ bool virtio_gpu_have_udmabuf(void)
>>>>>
>>>>>     void virtio_gpu_init_dmabuf(struct virtio_gpu_simple_resource
>> *res)
>>>>>     {
>>>>> +    Error *local_err = NULL;
>>>>>         void *pdata = NULL;
>>>>>
>>>>> -    res->dmabuf_fd = -1;
>>>>>         if (res->iov_cnt == 1 &&
>>>>>             res->iov[0].iov_len < 4096) {
>>>>> +        res->dmabuf_fd = -1;
>>>>>             pdata = res->iov[0].iov_base;
>>>>>         } else {
>>>>> -        virtio_gpu_create_udmabuf(res);
>>>>> +        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);
>>>>
>>>> Why not report the error in the QEMU log below ?
>>> I think the idea is that in the case of INVALID_IOV error, it is sufficient
>>> to just report that the Guest passed in incompatible memory
>> addresses.
>>> But I guess we could also just do:
>>> qemu_log_mask(LOG_GUEST_ERROR, "%s\n",
>> error_get_pretty(local_err));
>>
>> The error message may not be useful. e.g., "UDMABUF_CREATE_LIST:
>> ioctl
>> failed" does not tell what error the guest made and how to fix it.
>>
>> Regards,
>> Akihiko Odaki
>>
>>>
>>> Thanks,
>>> Vivek
>>>>
>>>>> +
>>>>> +            qemu_log_mask(LOG_GUEST_ERROR,
>>>>> +                          "Cannot create dmabuf: incompatible memory\n");
>>>>> +            return;
>>>>> +        } else if (res->dmabuf_fd >= 0) {
>>>>> +            pdata = virtio_gpu_remap_dmabuf(res, &local_err);
>>>>> +            if (!pdata) {
>>>>> +                virtio_gpu_destroy_dmabuf(res);
>>>>> +            }
>>>>> +        } else {
>>>>> +            res->dmabuf_fd = -1;
>>>>> +        }
>>>>> +
>>>>>             if (res->dmabuf_fd < 0) {
>>>>> +            error_report_err(local_err);
>>>>>                 return;
>>>>>             }
>>>>> -        virtio_gpu_remap_dmabuf(res);
>>>>> -        if (!res->remapped) {
>>>>> -            return;
>>>>> -        }
>>>>> -        pdata = res->remapped;
>>>>> +        res->remapped = pdata;
>>>>>         }
>>>>>
>>>>>         res->blob = pdata;
>>>
> 

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

[Kasireddy, Vivek]

Hi Akihiko,

> Subject: Re: [PATCH v12 10/10] virtio-gpu-dmabuf: Create dmabuf for
> blobs associated with VFIO devices
> 
> >>
> >> On 2026/03/19 14:15, 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>
> >>> Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> >>> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> >>> ---
> >>>    hw/display/virtio-gpu-dmabuf.c | 23 ++++++++++++++++++++---
> >>>    1 file changed, 20 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/hw/display/virtio-gpu-dmabuf.c b/hw/display/virtio-gpu-
> >> dmabuf.c
> >>> index 89aa487654..f953db0fbe 100644
> >>> --- a/hw/display/virtio-gpu-dmabuf.c
> >>> +++ b/hw/display/virtio-gpu-dmabuf.c
> >>> @@ -147,9 +147,26 @@ void virtio_gpu_init_dmabuf(struct
> >> virtio_gpu_simple_resource *res)
> >>>            if (res->dmabuf_fd ==
> >> VFIO_DMABUF_CREATE_ERR_INVALID_IOV) {
> >>>                error_free_or_abort(&local_err);
> >>>
> >>> -            qemu_log_mask(LOG_GUEST_ERROR,
> >>> -                          "Cannot create dmabuf: incompatible memory\n");
> >>> -            return;
> >>> +            res->dmabuf_fd = vfio_device_create_dmabuf_fd(res->iov,
> >>> +                                                          res->iov_cnt,
> >>> +                                                          res->blob_size,
> >>
> >> The correspondence between (iov, iov_cnt) and blob_size is more of a
> >> internal concern of virtio-gpu, not of VFIO. This parameter is better
> >> removed from vfio_device_create_dmabuf_fd() and
> >> vfio_device_mmap_dmabuf().
> > I don't disagree. So, should we add the following check in
> > virtio_gpu_init_dmabuf() or somewhere?
> > if (iov_size(iov, iov_cnt) != blob_size)
> 
> I suggest to have a check in virtio_gpu_create_mapping_iov() since it's
> not even specific to DMA-BUF.
I think virtio_gpu_resource_create_blob() might be a better place to put
this check in as blob_size is relevant (or valid) only for Guest based Blob
resources. Otherwise, we would have to pass blob_size as a new parameter
to virtio_gpu_create_mapping_iov() and modify all the call sites.

> 
> And instead let's ensure iov_size(iov, iov_cnt) >= blob_size and reject
> otherwise instead. I cited Codex's reasoning for this, which I totally
> agree. (I applied the Codex for Open Source program for access to Codex.
> And just in case: we are currently not allowed to use LLMs for writing
> patches and its use is restricted for the other purposes.)
> 
> It will be also more of a bug fix, so I think it is better to be sent as
> an independent patch instead of including it into this series.
Ok, will send this fix as a separate patch.

Thanks,
Vivek

> 
> Regards,
> Akihiko Odaki
> 
> Below is the Codex's output based on commit 8e711856d763 ("Merge tag
> 'hppa-fixes-for-v11-pull-request' of
> https://github.com/hdeller/qemu-hppa into staging"):
> 
> In current QEMU, omitting the check leaves inconsistent state possible,
> and the effect depends on the direction of the mismatch.
> 
> If iov_size < blob_size, this is the bad case. The dmabuf/export backing
> is built from the iov lengths in virtio-gpu-udmabuf.c (line 45) and
> virtio-gpu-udmabuf.c (line 63), but remap and scanout bounds use
> blob_size in virtio-gpu-udmabuf.c (line 73) and virtio-gpu.c (line 761).
> There is also a fast path that directly exposes iov_base as res->blob
> for small single-iov blobs in virtio-gpu-udmabuf.c (line 136), and
> scanout later uses that pointer as image data in virtio-gpu.c (line 662)
> and virtio-gpu.c (line 674). So a too-small backing is not a clean or
> obviously safe case.
> If iov_size > blob_size, it is mostly a semantics issue. QEMU still
> bounds scanout using blob_size in virtio-gpu.c (line 761), so the extra
> backing is usually just unused. But the resource state is still
> inconsistent.
> QEMU does not currently enforce the relationship elsewhere. blob_size
> and the iov are populated independently at blob creation in virtio-gpu.c
> (line 362) and virtio-gpu.c (line 366), and later backing attach also
> does not compare them in virtio-gpu.c (line 946).
> So if the question is "can we omit the check entirely?", the answer is:
> yes, but then you are knowingly accepting malformed guest state and
> relying on later backend behavior. If you want a defensive check,
> iov_size < blob_size is the one with a concrete justification. iov_size
> != blob_size is harder to defend from the spec.
> 
> >
> > Thanks,
> > Vivek
> >>
> >> Regards,
> >> Akihiko Odaki
> >>
> >>> +                                                          &local_err);
> >>> +            if (res->dmabuf_fd ==
> >> VFIO_DMABUF_CREATE_ERR_INVALID_IOV) {
> >>> +                error_free_or_abort(&local_err);
> >>> +                qemu_log_mask(LOG_GUEST_ERROR,
> >>> +                              "Cannot create dmabuf: incompatible memory\n");
> >>> +                return;
> >>> +            }
> >>> +
> >>> +            if (res->dmabuf_fd >= 0) {
> >>> +                pdata = vfio_device_mmap_dmabuf(res->iov, res->iov_cnt,
> >>> +                                                res->blob_size, &local_err);
> >>> +                if (!pdata) {
> >>> +                    virtio_gpu_destroy_dmabuf(res);
> >>> +                }
> >>> +            } else {
> >>> +                res->dmabuf_fd = -1;
> >>> +            }
> >>>            } else if (res->dmabuf_fd >= 0) {
> >>>                pdata = virtio_gpu_remap_dmabuf(res, &local_err);
> >>>                if (!pdata) {
> >

RE: [PATCH v12 09/10] virtio-gpu-dmabuf: Improve error handling with 'Error **' and err enum

[Kasireddy, Vivek]

Hi Akihiko,

> Subject: Re: [PATCH v12 09/10] virtio-gpu-dmabuf: Improve error handling
> with 'Error **' and err enum
> 
> >>>>
> >>>> On 3/19/26 06:15, Vivek Kasireddy wrote:
> >>>>> Make the error handling more robust in virtio_gpu_init_udmabuf()
> >>>>> by introducing 'Error **' parameter to capture errors and using
> >>>>> an enum from VFIO to categorize different errors. This allows for
> >>>>> better error reporting and handling of errors from
> >>>>> virtio_gpu_create_udmabuf() and virtio_gpu_remap_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>
> >>>>> Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> >>>>> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> >>>>> ---
> >>>>>     hw/display/virtio-gpu-dmabuf.c | 67
> +++++++++++++++++++++++--
> >> -----
> >>>> ----
> >>>>>     1 file changed, 45 insertions(+), 22 deletions(-)
> >>>>>
> >>>>> diff --git a/hw/display/virtio-gpu-dmabuf.c b/hw/display/virtio-
> gpu-
> >>>> dmabuf.c
> >>>>> index e35f7714a9..89aa487654 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"
> >>>>> @@ -27,16 +28,18 @@
> >>>>>     #include "standard-headers/linux/udmabuf.h"
> >>>>>     #include "standard-headers/drm/drm_fourcc.h"
> >>>>>
> >>>>> -static void virtio_gpu_create_udmabuf(struct
> >>>> virtio_gpu_simple_resource *res)
> >>>>> +static int virtio_gpu_create_udmabuf(struct
> >>>> virtio_gpu_simple_resource *res,
> >>>>> +                                     Error **errp)
> >>>>>     {
> >>>>>         g_autofree struct udmabuf_create_list *list = NULL;
> >>>>>         RAMBlock *rb;
> >>>>>         ram_addr_t offset;
> >>>>> -    int udmabuf, i;
> >>>>> +    int udmabuf, i, fd;
> >>>>>
> >>>>>         udmabuf = udmabuf_fd();
> >>>>>         if (udmabuf < 0) {
> >>>>> -        return;
> >>>>> +        error_setg(errp, "udmabuf device not available or enabled");
> >>>>> +        return VFIO_DMABUF_CREATE_ERR_UNSPEC;
> >>>>
> >>>> The function virtio_gpu_create_udmabuf() is returning
> >> VFIO_DMABUF_*
> >>>> enum
> >>>> values, which is problematic because the function creates a
> >> udmabuf,
> >>>> not
> >>>> a VFIO dmabuf.
> >>>>
> >>>> This creates a layering violation. The virtio-gpu-dmabuf code (which
> >>>> handles both udmabuf and VFIO dmabuf creation) is using error
> >> codes
> >>>> defined in the VFIO-specific header.
> >>
> >> The rationale of using error codes is as follows:
> >>
> >>   > 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).
> >>
> >> https://lore.kernel.org/qemu-devel/becde56b-90bd-40ca-9329-
> >> 0c92b9519a67@rsg.ci.i.u-tokyo.ac.jp/
> >>
> >>>>
> >>>> Please find another solution.
> >>> Other solutions I can think of are either move these error enums into
> >> virtio-gpu
> >>> (and disregard the error return type from vfio) or move them to some
> >> other header
> >>> where they are visible to both virtio-gpu and vfio. I'd like hear
> >> Akihiko's thoughts/
> >>> comments on how to proceed given that he had reviewed virtio-gpu
> >> patches in
> >>> this series.
> >>
> >> Disregarding the error conditions of VFIO is not OK. That can cause a
> >> guest error to be reported as a host error.
> >>
> >> I think the simplest alternative is just to have a duplicate enum for
> >> virtio-gpu.
> > That would address the layering violation but Cedric's other concern is
> > that he believes having errp is sufficient and using these error enums in
> > VFIO would be overkill.
> 
> It would be beneficial to describe the rationale behind the enums; the
> patch message only says "better error reporting", which is quite
> insufficient.
> 
> The rationale is that we need a special handling for the INVALID_IOV
> case. The INVALID_IOV case can happen in two cases:
> - The memory is backed by VFIO. It will result in the INVALID_IOV error
>    for the first attempt to use udmabuf, but this error can be properly
>    recovered by letting the VFIO code to create a DMA-BUF instead.
> - The memory is not backed by memfd nor VFIO. In this case, the error
>    is a guest's fault, so we should:
>    - Use qemu_log_mask(LOG_GUEST_ERROR, ...) instead of
>      error_report_err().
>    - Emit a message useful to diagnose the guest error, which we have
>      discussed in this thread.
> 
> A common pattern is to return -errno to let the caller implement a
> special error handling, but in this case we specifically need to
> distinguish the INVALID_IOV case from the others and these enums are
> more convenient for this.
This definitely helps in understanding the reasoning behind why these error
enums are useful. Before submitting the next version (with the above rationale
added), I'd like to wait and hear Cedric's thoughts/comments on whether
this is acceptable or not.

Thanks,
Vivek
> 
> Regards,
> Akihiko Odaki
> 
> >
> > Thanks,
> > Vivek
> >>
> >>>
> >>>>
> >>>>
> >>>>
> >>>>>         }
> >>>>>
> >>>>>         list = g_malloc0(sizeof(struct udmabuf_create_list) +
> >>>>> @@ -45,7 +48,8 @@ static void virtio_gpu_create_udmabuf(struct
> >>>> virtio_gpu_simple_resource *res)
> >>>>>         for (i = 0; i < res->iov_cnt; i++) {
> >>>>>             rb = qemu_ram_block_from_host(res->iov[i].iov_base, false,
> >>>> &offset);
> >>>>>             if (!rb || rb->fd < 0) {
> >>>>> -            return;
> >>>>> +            error_setg(errp, "IOV memory address incompatible with
> >>>> udmabuf ");
> >>>>> +            return VFIO_DMABUF_CREATE_ERR_INVALID_IOV;
> >>>>>             }
> >>>>>
> >>>>>             list->list[i].memfd  = rb->fd;
> >>>>> @@ -56,22 +60,28 @@ static void
> >> virtio_gpu_create_udmabuf(struct
> >>>> virtio_gpu_simple_resource *res)
> >>>>>         list->count = res->iov_cnt;
> >>>>>         list->flags = UDMABUF_FLAGS_CLOEXEC;
> >>>>>
> >>>>> -    res->dmabuf_fd = ioctl(udmabuf, UDMABUF_CREATE_LIST, list);
> >>>>> -    if (res->dmabuf_fd < 0) {
> >>>>> -        warn_report("%s: UDMABUF_CREATE_LIST: %s", __func__,
> >>>>> -                    strerror(errno));
> >>>>> +    fd = ioctl(udmabuf, UDMABUF_CREATE_LIST, list);
> >>>>> +    if (fd < 0) {
> >>>>> +        error_setg_errno(errp, errno, "UDMABUF_CREATE_LIST: ioctl
> >>>> failed");
> >>>>> +        if (errno == EINVAL || errno == EBADFD) {
> >>>>> +            return VFIO_DMABUF_CREATE_ERR_INVALID_IOV;
> >>>>> +        }
> >>>>> +        return VFIO_DMABUF_CREATE_ERR_UNSPEC;
> >>>>>         }
> >>>>> +    return fd;
> >>>>>     }
> >>>>>
> >>>>> -static void virtio_gpu_remap_dmabuf(struct
> >>>> virtio_gpu_simple_resource *res)
> >>>>> +static void *virtio_gpu_remap_dmabuf(struct
> >>>> virtio_gpu_simple_resource *res,
> >>>>> +                                     Error **errp)
> >>>>>     {
> >>>>> -    res->remapped = mmap(NULL, res->blob_size, PROT_READ,
> >>>>> -                         MAP_SHARED, res->dmabuf_fd, 0);
> >>>>> -    if (res->remapped == MAP_FAILED) {
> >>>>> -        warn_report("%s: dmabuf mmap failed: %s", __func__,
> >>>>> -                    strerror(errno));
> >>>>> -        res->remapped = 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");
> >>>>> +        return NULL;
> >>>>>         }
> >>>>> +    return map;
> >>>>>     }
> >>>>>
> >>>>>     static void virtio_gpu_destroy_dmabuf(struct
> >>>> virtio_gpu_simple_resource *res)
> >>>>> @@ -125,22 +135,35 @@ bool virtio_gpu_have_udmabuf(void)
> >>>>>
> >>>>>     void virtio_gpu_init_dmabuf(struct virtio_gpu_simple_resource
> >> *res)
> >>>>>     {
> >>>>> +    Error *local_err = NULL;
> >>>>>         void *pdata = NULL;
> >>>>>
> >>>>> -    res->dmabuf_fd = -1;
> >>>>>         if (res->iov_cnt == 1 &&
> >>>>>             res->iov[0].iov_len < 4096) {
> >>>>> +        res->dmabuf_fd = -1;
> >>>>>             pdata = res->iov[0].iov_base;
> >>>>>         } else {
> >>>>> -        virtio_gpu_create_udmabuf(res);
> >>>>> +        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);
> >>>>
> >>>> Why not report the error in the QEMU log below ?
> >>> I think the idea is that in the case of INVALID_IOV error, it is sufficient
> >>> to just report that the Guest passed in incompatible memory
> >> addresses.
> >>> But I guess we could also just do:
> >>> qemu_log_mask(LOG_GUEST_ERROR, "%s\n",
> >> error_get_pretty(local_err));
> >>
> >> The error message may not be useful. e.g., "UDMABUF_CREATE_LIST:
> >> ioctl
> >> failed" does not tell what error the guest made and how to fix it.
> >>
> >> Regards,
> >> Akihiko Odaki
> >>
> >>>
> >>> Thanks,
> >>> Vivek
> >>>>
> >>>>> +
> >>>>> +            qemu_log_mask(LOG_GUEST_ERROR,
> >>>>> +                          "Cannot create dmabuf: incompatible memory\n");
> >>>>> +            return;
> >>>>> +        } else if (res->dmabuf_fd >= 0) {
> >>>>> +            pdata = virtio_gpu_remap_dmabuf(res, &local_err);
> >>>>> +            if (!pdata) {
> >>>>> +                virtio_gpu_destroy_dmabuf(res);
> >>>>> +            }
> >>>>> +        } else {
> >>>>> +            res->dmabuf_fd = -1;
> >>>>> +        }
> >>>>> +
> >>>>>             if (res->dmabuf_fd < 0) {
> >>>>> +            error_report_err(local_err);
> >>>>>                 return;
> >>>>>             }
> >>>>> -        virtio_gpu_remap_dmabuf(res);
> >>>>> -        if (!res->remapped) {
> >>>>> -            return;
> >>>>> -        }
> >>>>> -        pdata = res->remapped;
> >>>>> +        res->remapped = pdata;
> >>>>>         }
> >>>>>
> >>>>>         res->blob = pdata;
> >>>
> >

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

[Akihiko Odaki]

On 2026/03/26 14:52, Kasireddy, Vivek wrote:
> Hi Akihiko,
> 
>> Subject: Re: [PATCH v12 10/10] virtio-gpu-dmabuf: Create dmabuf for
>> blobs associated with VFIO devices
>>
>>>>
>>>> On 2026/03/19 14:15, 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>
>>>>> Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
>>>>> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
>>>>> ---
>>>>>     hw/display/virtio-gpu-dmabuf.c | 23 ++++++++++++++++++++---
>>>>>     1 file changed, 20 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/hw/display/virtio-gpu-dmabuf.c b/hw/display/virtio-gpu-
>>>> dmabuf.c
>>>>> index 89aa487654..f953db0fbe 100644
>>>>> --- a/hw/display/virtio-gpu-dmabuf.c
>>>>> +++ b/hw/display/virtio-gpu-dmabuf.c
>>>>> @@ -147,9 +147,26 @@ void virtio_gpu_init_dmabuf(struct
>>>> virtio_gpu_simple_resource *res)
>>>>>             if (res->dmabuf_fd ==
>>>> VFIO_DMABUF_CREATE_ERR_INVALID_IOV) {
>>>>>                 error_free_or_abort(&local_err);
>>>>>
>>>>> -            qemu_log_mask(LOG_GUEST_ERROR,
>>>>> -                          "Cannot create dmabuf: incompatible memory\n");
>>>>> -            return;
>>>>> +            res->dmabuf_fd = vfio_device_create_dmabuf_fd(res->iov,
>>>>> +                                                          res->iov_cnt,
>>>>> +                                                          res->blob_size,
>>>>
>>>> The correspondence between (iov, iov_cnt) and blob_size is more of a
>>>> internal concern of virtio-gpu, not of VFIO. This parameter is better
>>>> removed from vfio_device_create_dmabuf_fd() and
>>>> vfio_device_mmap_dmabuf().
>>> I don't disagree. So, should we add the following check in
>>> virtio_gpu_init_dmabuf() or somewhere?
>>> if (iov_size(iov, iov_cnt) != blob_size)
>>
>> I suggest to have a check in virtio_gpu_create_mapping_iov() since it's
>> not even specific to DMA-BUF.
> I think virtio_gpu_resource_create_blob() might be a better place to put
> this check in as blob_size is relevant (or valid) only for Guest based Blob
> resources. Otherwise, we would have to pass blob_size as a new parameter
> to virtio_gpu_create_mapping_iov() and modify all the call sites.

All the call sites need to be modified since blob_size is present and 
needs to be respected at all of them. It is a chore but necessary to 
fully fix the bug Codex found, unfortunately.

Regards,
Akihiko Odaki

> 
>>
>> And instead let's ensure iov_size(iov, iov_cnt) >= blob_size and reject
>> otherwise instead. I cited Codex's reasoning for this, which I totally
>> agree. (I applied the Codex for Open Source program for access to Codex.
>> And just in case: we are currently not allowed to use LLMs for writing
>> patches and its use is restricted for the other purposes.)
>>
>> It will be also more of a bug fix, so I think it is better to be sent as
>> an independent patch instead of including it into this series.
> Ok, will send this fix as a separate patch.
> 
> Thanks,
> Vivek
> 
>>
>> Regards,
>> Akihiko Odaki
>>
>> Below is the Codex's output based on commit 8e711856d763 ("Merge tag
>> 'hppa-fixes-for-v11-pull-request' of
>> https://github.com/hdeller/qemu-hppa into staging"):
>>
>> In current QEMU, omitting the check leaves inconsistent state possible,
>> and the effect depends on the direction of the mismatch.
>>
>> If iov_size < blob_size, this is the bad case. The dmabuf/export backing
>> is built from the iov lengths in virtio-gpu-udmabuf.c (line 45) and
>> virtio-gpu-udmabuf.c (line 63), but remap and scanout bounds use
>> blob_size in virtio-gpu-udmabuf.c (line 73) and virtio-gpu.c (line 761).
>> There is also a fast path that directly exposes iov_base as res->blob
>> for small single-iov blobs in virtio-gpu-udmabuf.c (line 136), and
>> scanout later uses that pointer as image data in virtio-gpu.c (line 662)
>> and virtio-gpu.c (line 674). So a too-small backing is not a clean or
>> obviously safe case.
>> If iov_size > blob_size, it is mostly a semantics issue. QEMU still
>> bounds scanout using blob_size in virtio-gpu.c (line 761), so the extra
>> backing is usually just unused. But the resource state is still
>> inconsistent.
>> QEMU does not currently enforce the relationship elsewhere. blob_size
>> and the iov are populated independently at blob creation in virtio-gpu.c
>> (line 362) and virtio-gpu.c (line 366), and later backing attach also
>> does not compare them in virtio-gpu.c (line 946).
>> So if the question is "can we omit the check entirely?", the answer is:
>> yes, but then you are knowingly accepting malformed guest state and
>> relying on later backend behavior. If you want a defensive check,
>> iov_size < blob_size is the one with a concrete justification. iov_size
>> != blob_size is harder to defend from the spec.
>>
>>>
>>> Thanks,
>>> Vivek
>>>>
>>>> Regards,
>>>> Akihiko Odaki
>>>>
>>>>> +                                                          &local_err);
>>>>> +            if (res->dmabuf_fd ==
>>>> VFIO_DMABUF_CREATE_ERR_INVALID_IOV) {
>>>>> +                error_free_or_abort(&local_err);
>>>>> +                qemu_log_mask(LOG_GUEST_ERROR,
>>>>> +                              "Cannot create dmabuf: incompatible memory\n");
>>>>> +                return;
>>>>> +            }
>>>>> +
>>>>> +            if (res->dmabuf_fd >= 0) {
>>>>> +                pdata = vfio_device_mmap_dmabuf(res->iov, res->iov_cnt,
>>>>> +                                                res->blob_size, &local_err);
>>>>> +                if (!pdata) {
>>>>> +                    virtio_gpu_destroy_dmabuf(res);
>>>>> +                }
>>>>> +            } else {
>>>>> +                res->dmabuf_fd = -1;
>>>>> +            }
>>>>>             } else if (res->dmabuf_fd >= 0) {
>>>>>                 pdata = virtio_gpu_remap_dmabuf(res, &local_err);
>>>>>                 if (!pdata) {
>>>
>