[PATCH v1 0/7] vfio: Implement VFIO_DEVICE_FEATURE_DMA_BUF and use it in virtio-gpu

[PATCH v1 0/7] vfio: Implement VFIO_DEVICE_FEATURE_DMA_BUF 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 implement VFIO_DEVICE_FEATURE_DMA_BUF and
leverage it in virtio-gpu.

So, while creating the blobs we use memory_region_is_ram_device() to
figure out if the blob is backed by memfd or a vfio-pci device. If
it is determined that the blob is backed by vfio-pci device region,
instead of calling into udmabuf driver to create dmabuf we would
now call into vfio-pci driver to have a dmabuf created on the Host.

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 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 vfio-pci kernel driver series:
https://lore.kernel.org/dri-devel/cover.1754311439.git.leon@kernel.org/
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.williamson@redhat.com>
Cc: Cédric Le Goater <clg@redhat.com>
Cc: Leon Romanovsky <leon@kernel.org>
Cc: Leon Romanovsky <leonro@nvidia.com>
Cc: Jason Gunthorpe <jgg@nvidia.com>
Cc: Dongwon Kim <dongwon.kim@intel.com>

Vivek Kasireddy (7):
  virtio-gpu: Recreate the resource's dmabuf if new backing is attached
  virtio-gpu: Don't rely on res->blob to identify blob resources
  virtio-gpu: Find hva for Guest's DMA addr associated with a ram device
  vfio/region: Add a helper to get region index from memory region
  linux-headers: Update vfio.h to include VFIO_DEVICE_FEATURE_DMA_BUF
  vfio/device: Add support for VFIO_DEVICE_FEATURE_DMA_BUF
  virtio-gpu-udmabuf: Create dmabuf for blobs associated with VFIO
    devices

 hw/display/Kconfig              |   5 ++
 hw/display/virtio-gpu-udmabuf.c | 143 ++++++++++++++++++++++++++++++--
 hw/display/virtio-gpu.c         |  53 +++++++++---
 hw/vfio/device.c                |  43 ++++++++++
 hw/vfio/region.c                |  14 ++++
 include/hw/vfio/vfio-device.h   |   5 ++
 linux-headers/linux/vfio.h      |  25 ++++++
 7 files changed, 270 insertions(+), 18 deletions(-)

-- 
2.50.1

[PATCH v1 1/7] 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>
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 de35902213..70e8757128 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -937,6 +937,10 @@ virtio_gpu_resource_attach_backing(VirtIOGPU *g,
         cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
         return;
     }
+
+    if (res->blob_size && res->dmabuf_fd < 0) {
+        virtio_gpu_init_udmabuf(res);
+    }
 }
 
 static void
-- 
2.50.1

[PATCH v1 2/7] virtio-gpu: Don't rely on res->blob to identify blob resources

[Vivek Kasireddy]

The res->blob pointer may not be valid (non-NULL) for some blobs
where the backing storage is not memfd based. Therefore, we cannot
use it to determine if a resource is a blob or not. Instead, we
could use res->blob_size to make this determination as it is
non-zero for blob resources regardless of where their backing
storage is located.

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>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 hw/display/virtio-gpu.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 70e8757128..df696611a4 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -57,7 +57,7 @@ void virtio_gpu_update_cursor_data(VirtIOGPU *g,
     }
 
     if (res->blob_size) {
-        if (res->blob_size < (s->current_cursor->width *
+        if (!res->blob || res->blob_size < (s->current_cursor->width *
                               s->current_cursor->height * 4)) {
             return;
         }
@@ -144,7 +144,7 @@ virtio_gpu_find_check_resource(VirtIOGPU *g, uint32_t resource_id,
     }
 
     if (require_backing) {
-        if (!res->iov || (!res->image && !res->blob)) {
+        if (!res->iov || (!res->image && !res->blob_size)) {
             qemu_log_mask(LOG_GUEST_ERROR, "%s: no backing storage %d\n",
                           caller, resource_id);
             if (error) {
@@ -446,7 +446,7 @@ static void virtio_gpu_transfer_to_host_2d(VirtIOGPU *g,
 
     res = virtio_gpu_find_check_resource(g, t2d.resource_id, true,
                                          __func__, &cmd->error);
-    if (!res || res->blob) {
+    if (!res || res->blob_size) {
         return;
     }
 
@@ -509,7 +509,7 @@ static void virtio_gpu_resource_flush(VirtIOGPU *g,
         return;
     }
 
-    if (res->blob) {
+    if (res->blob_size) {
         for (i = 0; i < g->parent_obj.conf.max_outputs; i++) {
             scanout = &g->parent_obj.scanout[i];
             if (scanout->resource_id == res->resource_id &&
@@ -540,7 +540,7 @@ static void virtio_gpu_resource_flush(VirtIOGPU *g,
         }
     }
 
-    if (!res->blob &&
+    if (!res->blob_size &&
         (rf.r.x > res->width ||
         rf.r.y > res->height ||
         rf.r.width > res->width ||
@@ -636,7 +636,7 @@ static bool virtio_gpu_do_set_scanout(VirtIOGPU *g,
 
     g->parent_obj.enable = 1;
 
-    if (res->blob) {
+    if (res->blob_size) {
         if (console_has_gl(scanout->con)) {
             if (!virtio_gpu_update_dmabuf(g, scanout_id, res, fb, r)) {
                 virtio_gpu_update_scanout(g, scanout_id, res, fb, r);
@@ -647,13 +647,16 @@ static bool virtio_gpu_do_set_scanout(VirtIOGPU *g,
             return true;
         }
 
+        if (!res->blob) {
+            return false;
+        }
         data = res->blob;
     } else {
         data = (uint8_t *)pixman_image_get_data(res->image);
     }
 
     /* create a surface for this scanout */
-    if ((res->blob && !console_has_gl(scanout->con)) ||
+    if ((res->blob_size && !console_has_gl(scanout->con)) ||
         !scanout->ds ||
         surface_data(scanout->ds) != data + fb->offset ||
         scanout->width != r->width ||
@@ -901,7 +904,7 @@ void virtio_gpu_cleanup_mapping(VirtIOGPU *g,
     g_free(res->addrs);
     res->addrs = NULL;
 
-    if (res->blob) {
+    if (res->blob_size) {
         virtio_gpu_fini_udmabuf(res);
     }
 }
-- 
2.50.1

[PATCH v1 3/7] 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 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().

This is because the ram device's address space is not accessible
to virtio-gpu directly and hence dma_memory_map() cannot be used.
Therefore, we first need to identify the memory region associated
with the DMA address and figure out if it belongs to a ram device
or not and decide how to obtain the host address accordingly.

Note that we take a reference on the memory region if it belongs
to a ram device but we would still call dma_memory_unmap() later
(to unref mr) regardless of how we obtained the hva.

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>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 hw/display/virtio-gpu.c | 30 +++++++++++++++++++++++++++---
 1 file changed, 27 insertions(+), 3 deletions(-)

diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index df696611a4..22bbe76809 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -801,6 +801,32 @@ 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 = NULL;
+    hwaddr xlat;
+
+    if (cmd->cmd_hdr.type != VIRTIO_GPU_CMD_RESOURCE_CREATE_BLOB) {
+        return dma_memory_map(VIRTIO_DEVICE(g)->dma_as, a, len,
+                              DMA_DIRECTION_TO_DEVICE,
+                              MEMTXATTRS_UNSPECIFIED);
+    }
+
+    mr = address_space_translate(VIRTIO_DEVICE(g)->dma_as, a, &xlat, len,
+                                 DMA_DIRECTION_TO_DEVICE,
+                                 MEMTXATTRS_UNSPECIFIED);
+    if (memory_region_is_ram_device(mr)) {
+        memory_region_ref(mr);
+        return memory_region_get_ram_ptr(mr) + xlat;
+    }
+
+    return dma_memory_map(VIRTIO_DEVICE(g)->dma_as, a, len,
+                          DMA_DIRECTION_TO_DEVICE,
+                          MEMTXATTRS_UNSPECIFIED);
+}
+
 int virtio_gpu_create_mapping_iov(VirtIOGPU *g,
                                   uint32_t nr_entries, uint32_t offset,
                                   struct virtio_gpu_ctrl_command *cmd,
@@ -842,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.50.1

[PATCH v1 4/7] vfio/region: Add a helper to get region index from memory region

[Vivek Kasireddy]

Having a way to figure out the region index (or bar) associated
with a memory region is helpful in various scenarios. For example,
this capability can be useful in retrieving the region info needed
for mapping a part of a VFIO region or creating a dmabuf.

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

diff --git a/hw/vfio/region.c b/hw/vfio/region.c
index b165ab0b93..8837afc97f 100644
--- a/hw/vfio/region.c
+++ b/hw/vfio/region.c
@@ -398,3 +398,17 @@ void vfio_region_mmaps_set_enabled(VFIORegion *region, bool enabled)
     trace_vfio_region_mmaps_set_enabled(memory_region_name(region->mem),
                                         enabled);
 }
+
+int vfio_get_region_index_from_mr(MemoryRegion *mr)
+{
+    VFIORegion *region = mr->opaque;
+
+    if (mr->ops != &vfio_region_ops) {
+        mr = mr->container;
+        if (mr->ops != &vfio_region_ops) {
+            return -1;
+        }
+	region = mr->opaque;
+    }
+    return region->nr;
+}
diff --git a/include/hw/vfio/vfio-device.h b/include/hw/vfio/vfio-device.h
index 7e9aed6d3c..bdb106c937 100644
--- a/include/hw/vfio/vfio-device.h
+++ b/include/hw/vfio/vfio-device.h
@@ -277,6 +277,8 @@ 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);
+
+int vfio_get_region_index_from_mr(MemoryRegion *mr);
 #endif
 
 /* Returns 0 on success, or a negative errno. */
-- 
2.50.1

[PATCH v1 5/7] linux-headers: Update vfio.h to include VFIO_DEVICE_FEATURE_DMA_BUF

[Vivek Kasireddy]

Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 linux-headers/linux/vfio.h | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/linux-headers/linux/vfio.h b/linux-headers/linux/vfio.h
index 79bf8c0cc5..d2177c9cba 100644
--- a/linux-headers/linux/vfio.h
+++ b/linux-headers/linux/vfio.h
@@ -1468,6 +1468,31 @@ struct vfio_device_feature_bus_master {
 };
 #define VFIO_DEVICE_FEATURE_BUS_MASTER 10
 
+/**
+ * Upon VFIO_DEVICE_FEATURE_GET create a dma_buf fd for the
+ * regions selected.
+ *
+ * open_flags are the typical flags passed to open(2), eg O_RDWR, O_CLOEXEC,
+ * etc. offset/length specify a slice of the region to create the dmabuf from.
+ * nr_ranges is the total number of (P2P DMA) ranges that comprise the dmabuf.
+ *
+ * Return: The fd number on success, -1 and errno is set on failure.
+ */
+#define VFIO_DEVICE_FEATURE_DMA_BUF 11
+
+struct vfio_region_dma_range {
+	__u64 offset;
+	__u64 length;
+};
+
+struct vfio_device_feature_dma_buf {
+	__u32	region_index;
+	__u32	open_flags;
+	__u32   flags;
+	__u32   nr_ranges;
+	struct vfio_region_dma_range dma_ranges[];
+};
+
 /* -------- API for Type1 VFIO IOMMU -------- */
 
 /**
-- 
2.50.1

[PATCH v1 6/7] vfio/device: Add support for VFIO_DEVICE_FEATURE_DMA_BUF

[Vivek Kasireddy]

In order to implement VFIO_DEVICE_FEATURE_DMA_BUF, 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 in order to invoke
this feature.

Cc: Alex Williamson <alex.williamson@redhat.com>
Cc: Cédric Le Goater <clg@redhat.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 hw/vfio/device.c              | 43 +++++++++++++++++++++++++++++++++++
 include/hw/vfio/vfio-device.h |  3 +++
 2 files changed, 46 insertions(+)

diff --git a/hw/vfio/device.c b/hw/vfio/device.c
index 64f8750389..49070929ac 100644
--- a/hw/vfio/device.c
+++ b/hw/vfio/device.c
@@ -21,6 +21,7 @@
 #include "qemu/osdep.h"
 #include <sys/ioctl.h>
 
+#include "system/ramblock.h"
 #include "hw/vfio/vfio-device.h"
 #include "hw/vfio/pci.h"
 #include "hw/hw.h"
@@ -592,3 +593,45 @@ static VFIODeviceIOOps vfio_device_io_ops_ioctl = {
     .region_read = vfio_device_io_region_read,
     .region_write = vfio_device_io_region_write,
 };
+
+int vfio_device_create_dmabuf(VFIODevice *vdev,
+                              struct iovec *iov, unsigned int iov_cnt)
+{
+    g_autofree struct vfio_device_feature *feature;
+    struct vfio_device_feature_dma_buf *dma_buf;
+    ram_addr_t offset;
+    RAMBlock *rb;
+    size_t argsz;
+    int i, index;
+
+    argsz = sizeof(*feature) + sizeof (*dma_buf) +
+            sizeof(struct vfio_region_dma_range) * iov_cnt;
+    feature = g_malloc0(argsz);
+    dma_buf = (struct vfio_device_feature_dma_buf *)feature->data;
+
+    for (i = 0; i < iov_cnt; i++) {
+        rcu_read_lock();
+        rb = qemu_ram_block_from_host(iov[i].iov_base, false, &offset);
+        rcu_read_unlock();
+
+        if (!rb) {
+            return -1;
+        }
+
+        index = vfio_get_region_index_from_mr(rb->mr);
+        if (index < 0) {
+            return -1;
+        }
+
+        dma_buf->region_index = index;
+        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;
+    feature->argsz = argsz;
+    feature->flags = VFIO_DEVICE_FEATURE_GET | VFIO_DEVICE_FEATURE_DMA_BUF;
+
+    return ioctl(vdev->fd, VFIO_DEVICE_FEATURE, feature);
+}
diff --git a/include/hw/vfio/vfio-device.h b/include/hw/vfio/vfio-device.h
index bdb106c937..74b3c4eef7 100644
--- a/include/hw/vfio/vfio-device.h
+++ b/include/hw/vfio/vfio-device.h
@@ -279,6 +279,9 @@ int vfio_device_get_irq_info(VFIODevice *vbasedev, int index,
                                 struct vfio_irq_info *info);
 
 int vfio_get_region_index_from_mr(MemoryRegion *mr);
+
+int vfio_device_create_dmabuf(VFIODevice *vbasedev,
+                              struct iovec *iov, unsigned int iov_cnt);
 #endif
 
 /* Returns 0 on success, or a negative errno. */
-- 
2.50.1

[PATCH v1 7/7] virtio-gpu-udmabuf: 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. Therefore, we first need to figure
out if the blob is backed by a VFIO device region or a memfd before
we can call the right API to get a dmabuf fd created.

So, once we have the ramblock and the associated mr, we rely on
memory_region_is_ram_device() to tell us where the backing storage
is located. If the blob resource is VFIO backed, we try to find the
right VFIO device that contains the blob and then invoke the API
vfio_device_create_dmabuf().

Note that in virtio_gpu_remap_udmabuf(), we first try to test if
the VFIO dmabuf exporter supports mmap or not. If it doesn't, we
use the VFIO device fd directly to create the CPU 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>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 hw/display/Kconfig              |   5 ++
 hw/display/virtio-gpu-udmabuf.c | 143 ++++++++++++++++++++++++++++++--
 2 files changed, 141 insertions(+), 7 deletions(-)

diff --git a/hw/display/Kconfig b/hw/display/Kconfig
index 1e95ab28ef..0d090f25f5 100644
--- a/hw/display/Kconfig
+++ b/hw/display/Kconfig
@@ -106,6 +106,11 @@ config VIRTIO_VGA
     depends on VIRTIO_PCI
     select VGA
 
+config VIRTIO_GPU_VFIO_BLOB
+    bool
+    default y
+    depends on VFIO
+
 config VHOST_USER_GPU
     bool
     default y
diff --git a/hw/display/virtio-gpu-udmabuf.c b/hw/display/virtio-gpu-udmabuf.c
index d804f321aa..bd06b4f300 100644
--- a/hw/display/virtio-gpu-udmabuf.c
+++ b/hw/display/virtio-gpu-udmabuf.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,6 +28,33 @@
 #include "standard-headers/linux/udmabuf.h"
 #include "standard-headers/drm/drm_fourcc.h"
 
+static void vfio_create_dmabuf(VFIODevice *vdev,
+                               struct virtio_gpu_simple_resource *res)
+{
+#if defined(VIRTIO_GPU_VFIO_BLOB)
+    res->dmabuf_fd = vfio_device_create_dmabuf(vdev, res->iov, res->iov_cnt);
+    if (res->dmabuf_fd < 0) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: VFIO_DEVICE_FEATURE_DMA_BUF: %s\n",
+                      __func__, strerror(errno));
+    }
+#endif
+}
+
+static VFIODevice *vfio_device_lookup(MemoryRegion *mr)
+{
+#if defined(VIRTIO_GPU_VFIO_BLOB)
+    VFIODevice *vdev;
+
+    QLIST_FOREACH(vdev, &vfio_device_list, next) {
+        if (vdev->dev == mr->dev) {
+            return vdev;
+        }
+    }
+#endif
+    return NULL;
+}
+
 static void virtio_gpu_create_udmabuf(struct virtio_gpu_simple_resource *res)
 {
     struct udmabuf_create_list *list;
@@ -68,11 +96,73 @@ 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 *vfio_dmabuf_mmap(struct virtio_gpu_simple_resource *res,
+                              VFIODevice *vdev)
+{
+    struct vfio_region_info *info;
+    ram_addr_t offset, len = 0;
+    void *map, *submap;
+    int i, ret = -1;
+    RAMBlock *rb;
+
+    /*
+     * 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, res->blob_size, PROT_READ, MAP_SHARED, vdev->fd, 0);
+    if (map == MAP_FAILED) {
+        return map;
+    }
+
+    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) {
+            goto err;
+        }
+
+#if defined(VIRTIO_GPU_VFIO_BLOB)
+        ret = vfio_get_region_index_from_mr(rb->mr);
+        if (ret < 0) {
+            goto err;
+        }
+
+        ret = vfio_device_get_region_info(vdev, ret, &info);
+#endif
+        if (ret < 0) {
+            goto err;
+        }
+
+        submap = mmap(map + len, res->iov[i].iov_len, PROT_READ,
+                      MAP_SHARED | MAP_FIXED, vdev->fd,
+                      info->offset + offset);
+        if (submap == MAP_FAILED) {
+            goto err;
+        }
+
+        len += res->iov[i].iov_len;
+    }
+    return map;
+err:
+    munmap(map, res->blob_size);
+    return MAP_FAILED;
+}
+
+static void virtio_gpu_remap_udmabuf(struct virtio_gpu_simple_resource *res,
+                                     VFIODevice *vdev)
 {
     res->remapped = mmap(NULL, res->blob_size, PROT_READ,
                          MAP_SHARED, res->dmabuf_fd, 0);
     if (res->remapped == MAP_FAILED) {
+        if (vdev) {
+            res->remapped = vfio_dmabuf_mmap(res, vdev);
+            if (res->remapped != MAP_FAILED) {
+                return;
+            }
+        }
         warn_report("%s: dmabuf mmap failed: %s", __func__,
                     strerror(errno));
         res->remapped = NULL;
@@ -130,18 +220,59 @@ bool virtio_gpu_have_udmabuf(void)
 
 void virtio_gpu_init_udmabuf(struct virtio_gpu_simple_resource *res)
 {
+    VFIODevice *vdev = NULL;
     void *pdata = NULL;
+    ram_addr_t offset;
+    RAMBlock *rb;
 
     res->dmabuf_fd = -1;
     if (res->iov_cnt == 1 &&
         res->iov[0].iov_len < 4096) {
         pdata = res->iov[0].iov_base;
     } else {
-        virtio_gpu_create_udmabuf(res);
-        if (res->dmabuf_fd < 0) {
+        rcu_read_lock();
+        rb = qemu_ram_block_from_host(res->iov[0].iov_base, false, &offset);
+        rcu_read_unlock();
+
+        if (!rb) {
+            qemu_log_mask(LOG_GUEST_ERROR,
+                          "%s: Could not find ram block for host address\n",
+                          __func__);
             return;
         }
-        virtio_gpu_remap_udmabuf(res);
+
+        if (memory_region_is_ram_device(rb->mr)) {
+            vdev = vfio_device_lookup(rb->mr);
+            if (!vdev) {
+                qemu_log_mask(LOG_GUEST_ERROR,
+                              "%s: Could not find device to create dmabuf\n",
+                              __func__);
+                return;
+            }
+
+            vfio_create_dmabuf(vdev, res);
+            if (res->dmabuf_fd < 0) {
+                qemu_log_mask(LOG_GUEST_ERROR,
+                              "%s: Could not create dmabuf from vfio device\n",
+                              __func__);
+                return;
+            }
+        } else if (memory_region_is_ram(rb->mr) && virtio_gpu_have_udmabuf()) {
+            virtio_gpu_create_udmabuf(res);
+            if (res->dmabuf_fd < 0) {
+                qemu_log_mask(LOG_GUEST_ERROR,
+                              "%s: Could not create dmabuf from memfd\n",
+                              __func__);
+                return;
+            }
+        } else {
+            qemu_log_mask(LOG_GUEST_ERROR,
+                          "%s: memory region cannot be used to create dmabuf\n",
+                          __func__);
+            return;
+        }
+
+        virtio_gpu_remap_udmabuf(res, vdev);
         if (!res->remapped) {
             return;
         }
@@ -153,9 +284,7 @@ void virtio_gpu_init_udmabuf(struct virtio_gpu_simple_resource *res)
 
 void virtio_gpu_fini_udmabuf(struct virtio_gpu_simple_resource *res)
 {
-    if (res->remapped) {
-        virtio_gpu_destroy_udmabuf(res);
-    }
+    virtio_gpu_destroy_udmabuf(res);
 }
 
 static void virtio_gpu_free_dmabuf(VirtIOGPU *g, VGPUDMABuf *dmabuf)
-- 
2.50.1

Re: [PATCH v1 6/7] vfio/device: Add support for VFIO_DEVICE_FEATURE_DMA_BUF

[Cédric Le Goater]

Hello Vivek,

On 10/4/25 01:35, Vivek Kasireddy wrote:
> In order to implement VFIO_DEVICE_FEATURE_DMA_BUF, 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 in order to invoke
> this feature.
> 
> Cc: Alex Williamson <alex.williamson@redhat.com>
> Cc: Cédric Le Goater <clg@redhat.com>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> ---
>   hw/vfio/device.c              | 43 +++++++++++++++++++++++++++++++++++
>   include/hw/vfio/vfio-device.h |  3 +++
>   2 files changed, 46 insertions(+)
> 
> diff --git a/hw/vfio/device.c b/hw/vfio/device.c
> index 64f8750389..49070929ac 100644
> --- a/hw/vfio/device.c
> +++ b/hw/vfio/device.c
> @@ -21,6 +21,7 @@
>   #include "qemu/osdep.h"
>   #include <sys/ioctl.h>
>   
> +#include "system/ramblock.h"
>   #include "hw/vfio/vfio-device.h"
>   #include "hw/vfio/pci.h"
>   #include "hw/hw.h"
> @@ -592,3 +593,45 @@ static VFIODeviceIOOps vfio_device_io_ops_ioctl = {
>       .region_read = vfio_device_io_region_read,
>       .region_write = vfio_device_io_region_write,
>   };
> +
> +int vfio_device_create_dmabuf(VFIODevice *vdev,

a 'vbasedev' name is preferred for VFIODevice variables/parameters.

> +                              struct iovec *iov, unsigned int iov_cnt)
> +{
> +    g_autofree struct vfio_device_feature *feature;

g_autofree variables should be set: 'feature = NULL'

> +    struct vfio_device_feature_dma_buf *dma_buf;
> +    ram_addr_t offset;
> +    RAMBlock *rb;
> +    size_t argsz;
> +    int i, index;
> +
> +    argsz = sizeof(*feature) + sizeof (*dma_buf) +
> +            sizeof(struct vfio_region_dma_range) * iov_cnt;
> +    feature = g_malloc0(argsz);
> +    dma_buf = (struct vfio_device_feature_dma_buf *)feature->data;
> +
> +    for (i = 0; i < iov_cnt; i++) {
> +        rcu_read_lock();

Is it needed ?

> +        rb = qemu_ram_block_from_host(iov[i].iov_base, false, &offset);
> +        rcu_read_unlock();
> +
> +        if (!rb) {
> +            return -1;

wouldn't an errno be more appropriate ?

> +        }
> +
> +        index = vfio_get_region_index_from_mr(rb->mr);
> +        if (index < 0) {
> +            return -1;
> +        }
> +
> +        dma_buf->region_index = index;
> +        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;
> +    feature->argsz = argsz;
> +    feature->flags = VFIO_DEVICE_FEATURE_GET | VFIO_DEVICE_FEATURE_DMA_BUF;
> +
> +    return ioctl(vdev->fd, VFIO_DEVICE_FEATURE, feature);
Please use :

    return vbasedev->io_ops->device_feature(vbasedev, feature);

This would be returning an errno. Callers should be aware.

> +}
> diff --git a/include/hw/vfio/vfio-device.h b/include/hw/vfio/vfio-device.h
> index bdb106c937..74b3c4eef7 100644
> --- a/include/hw/vfio/vfio-device.h
> +++ b/include/hw/vfio/vfio-device.h
> @@ -279,6 +279,9 @@ int vfio_device_get_irq_info(VFIODevice *vbasedev, int index,
>                                   struct vfio_irq_info *info);
>   
>   int vfio_get_region_index_from_mr(MemoryRegion *mr);
> +
> +int vfio_device_create_dmabuf(VFIODevice *vbasedev,
> +                              struct iovec *iov, unsigned int iov_cnt);


Since this is an external routine, please add documentation.


Thanks,

C.


>   #endif
>   
>   /* Returns 0 on success, or a negative errno. */

Re: [PATCH v1 4/7] vfio/region: Add a helper to get region index from memory region

[Cédric Le Goater]

Hello Vivek,

On 10/4/25 01:35, Vivek Kasireddy wrote:
> Having a way to figure out the region index (or bar) associated
> with a memory region is helpful in various scenarios. For example,
> this capability can be useful in retrieving the region info needed
> for mapping a part of a VFIO region or creating a dmabuf.
> 
> Cc: Alex Williamson <alex.williamson@redhat.com>
> Cc: Cédric Le Goater <clg@redhat.com>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> ---
>   hw/vfio/region.c              | 14 ++++++++++++++
>   include/hw/vfio/vfio-device.h |  2 ++
>   2 files changed, 16 insertions(+)
> 
> diff --git a/hw/vfio/region.c b/hw/vfio/region.c
> index b165ab0b93..8837afc97f 100644
> --- a/hw/vfio/region.c
> +++ b/hw/vfio/region.c
> @@ -398,3 +398,17 @@ void vfio_region_mmaps_set_enabled(VFIORegion *region, bool enabled)
>       trace_vfio_region_mmaps_set_enabled(memory_region_name(region->mem),
>                                           enabled);
>   }
> +
> +int vfio_get_region_index_from_mr(MemoryRegion *mr)
> +{
> +    VFIORegion *region = mr->opaque;
> +
> +    if (mr->ops != &vfio_region_ops) {
> +        mr = mr->container;

May be start the routine with :

     while (mr->container) {
         mr = mr->container;
     }

> +        if (mr->ops != &vfio_region_ops) {
> +            return -1;

I think I would prefer returning a 'VFIORegion *' which should be
NULL in case of error. Looks cleaner.


> +        }
> +	region = mr->opaque;
> +    }
> +    return region->nr;
> +}
> diff --git a/include/hw/vfio/vfio-device.h b/include/hw/vfio/vfio-device.h
> index 7e9aed6d3c..bdb106c937 100644
> --- a/include/hw/vfio/vfio-device.h
> +++ b/include/hw/vfio/vfio-device.h
> @@ -277,6 +277,8 @@ 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);
> +
> +int vfio_get_region_index_from_mr(MemoryRegion *mr);
Please add documentation.

Thanks,

C.


>   #endif
>   
>   /* Returns 0 on success, or a negative errno. */

Re: [PATCH v1 0/7] vfio: Implement VFIO_DEVICE_FEATURE_DMA_BUF and use it in virtio-gpu

[Cédric Le Goater]

Vivek,

On 10/4/25 01:35, Vivek Kasireddy wrote:
> 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 implement VFIO_DEVICE_FEATURE_DMA_BUF and
> leverage it in virtio-gpu.
> 
> So, while creating the blobs we use memory_region_is_ram_device() to
> figure out if the blob is backed by memfd or a vfio-pci device. If
> it is determined that the blob is backed by vfio-pci device region,
> instead of calling into udmabuf driver to create dmabuf we would
> now call into vfio-pci driver to have a dmabuf created on the Host.
> 
> 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 by running Gnome Wayland

Could you please be more precise on the HW ?

> (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 vfio-pci kernel driver series:
> https://lore.kernel.org/dri-devel/cover.1754311439.git.leon@kernel.org/
> 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.williamson@redhat.com>
> Cc: Cédric Le Goater <clg@redhat.com>
> Cc: Leon Romanovsky <leon@kernel.org>
> Cc: Leon Romanovsky <leonro@nvidia.com>
> Cc: Jason Gunthorpe <jgg@nvidia.com>
> Cc: Dongwon Kim <dongwon.kim@intel.com>
> 
> Vivek Kasireddy (7):
>    virtio-gpu: Recreate the resource's dmabuf if new backing is attached
>    virtio-gpu: Don't rely on res->blob to identify blob resources
>    virtio-gpu: Find hva for Guest's DMA addr associated with a ram device
>    vfio/region: Add a helper to get region index from memory region
>    linux-headers: Update vfio.h to include VFIO_DEVICE_FEATURE_DMA_BUF
>    vfio/device: Add support for VFIO_DEVICE_FEATURE_DMA_BUF
>    virtio-gpu-udmabuf: Create dmabuf for blobs associated with VFIO
>      devices
> 
>   hw/display/Kconfig              |   5 ++
>   hw/display/virtio-gpu-udmabuf.c | 143 ++++++++++++++++++++++++++++++--
>   hw/display/virtio-gpu.c         |  53 +++++++++---
>   hw/vfio/device.c                |  43 ++++++++++
>   hw/vfio/region.c                |  14 ++++
>   include/hw/vfio/vfio-device.h   |   5 ++
>   linux-headers/linux/vfio.h      |  25 ++++++
>   7 files changed, 270 insertions(+), 18 deletions(-)
> 

Please cc: Alex and I on all patches.

Thanks,

C.

Re: [PATCH v1 7/7] virtio-gpu-udmabuf: Create dmabuf for blobs associated with VFIO devices

[Cédric Le Goater]

On 10/4/25 01:36, Vivek Kasireddy wrote:
> In addition to memfd, a blob resource can also have its backing
> storage in a VFIO device region. Therefore, we first need to figure
> out if the blob is backed by a VFIO device region or a memfd before
> we can call the right API to get a dmabuf fd created.
> 
> So, once we have the ramblock and the associated mr, we rely on
> memory_region_is_ram_device() to tell us where the backing storage
> is located. If the blob resource is VFIO backed, we try to find the
> right VFIO device that contains the blob and then invoke the API
> vfio_device_create_dmabuf().
> 
> Note that in virtio_gpu_remap_udmabuf(), we first try to test if
> the VFIO dmabuf exporter supports mmap or not. If it doesn't, we
> use the VFIO device fd directly to create the CPU 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>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> ---
>   hw/display/Kconfig              |   5 ++
>   hw/display/virtio-gpu-udmabuf.c | 143 ++++++++++++++++++++++++++++++--
>   2 files changed, 141 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/display/Kconfig b/hw/display/Kconfig
> index 1e95ab28ef..0d090f25f5 100644
> --- a/hw/display/Kconfig
> +++ b/hw/display/Kconfig
> @@ -106,6 +106,11 @@ config VIRTIO_VGA
>       depends on VIRTIO_PCI
>       select VGA
>   
> +config VIRTIO_GPU_VFIO_BLOB
> +    bool
> +    default y
> +    depends on VFIO
> +
>   config VHOST_USER_GPU
>       bool
>       default y
> diff --git a/hw/display/virtio-gpu-udmabuf.c b/hw/display/virtio-gpu-udmabuf.c
> index d804f321aa..bd06b4f300 100644
> --- a/hw/display/virtio-gpu-udmabuf.c
> +++ b/hw/display/virtio-gpu-udmabuf.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,6 +28,33 @@
>   #include "standard-headers/linux/udmabuf.h"
>   #include "standard-headers/drm/drm_fourcc.h"
>   
> +static void vfio_create_dmabuf(VFIODevice *vdev,
> +                               struct virtio_gpu_simple_resource *res)
> +{
> +#if defined(VIRTIO_GPU_VFIO_BLOB)
> +    res->dmabuf_fd = vfio_device_create_dmabuf(vdev, res->iov, res->iov_cnt);

I didn't realize an fd was returned until this patch. I'd suggest
renaming vfio_device_create_dmabuf() to vfio_device_create_dmabuf_fd(),
or something explicit IMO.

> +    if (res->dmabuf_fd < 0) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: VFIO_DEVICE_FEATURE_DMA_BUF: %s\n",
> +                      __func__, strerror(errno));
> +    }
> +#endif
> +}
> +
> +static VFIODevice *vfio_device_lookup(MemoryRegion *mr)
> +{
> +#if defined(VIRTIO_GPU_VFIO_BLOB)
> +    VFIODevice *vdev;
> +
> +    QLIST_FOREACH(vdev, &vfio_device_list, next) {
Hmm, I'm not sure we want to expose the VFIOdevice list to other
subsystems. I understand the need, and it's faster than iterating
over QOM devices, but I’d prefer that an API be provided for this
purpose.

I missed how much vfio_device_list has proliferated. Needs a check.


> +        if (vdev->dev == mr->dev) {
> +            return vdev;
> +        }
> +    }
> +#endif
> +    return NULL;
> +}
> +
>   static void virtio_gpu_create_udmabuf(struct virtio_gpu_simple_resource *res)
>   {
>       struct udmabuf_create_list *list;
> @@ -68,11 +96,73 @@ 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 *vfio_dmabuf_mmap(struct virtio_gpu_simple_resource *res,
> +                              VFIODevice *vdev)
> +{
> +    struct vfio_region_info *info;
> +    ram_addr_t offset, len = 0;
> +    void *map, *submap;
> +    int i, ret = -1;
> +    RAMBlock *rb;
> +
> +    /*
> +     * 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, res->blob_size, PROT_READ, MAP_SHARED, vdev->fd, 0);
> +    if (map == MAP_FAILED) {
> +        return map;
> +    }
> +
> +    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) {
> +            goto err;
> +        }
> +
> +#if defined(VIRTIO_GPU_VFIO_BLOB)
> +        ret = vfio_get_region_index_from_mr(rb->mr);
> +        if (ret < 0) {
> +            goto err;
> +        }
> +
> +        ret = vfio_device_get_region_info(vdev, ret, &info);
> +#endif
> +        if (ret < 0) {
> +            goto err;
> +        }

"hmm" again. Not this patch fault but we lack proper documentation
for the VFIO API. Something to work on. Since this patch is using
vfio_device_get_region_info() could you please add documentation
for it ?


Thanks,

C.



> +        submap = mmap(map + len, res->iov[i].iov_len, PROT_READ,
> +                      MAP_SHARED | MAP_FIXED, vdev->fd,
> +                      info->offset + offset);
> +        if (submap == MAP_FAILED) {
> +            goto err;
> +        }
> +
> +        len += res->iov[i].iov_len;
> +    }
> +    return map;
> +err:
> +    munmap(map, res->blob_size);
> +    return MAP_FAILED;
> +}
> +
> +static void virtio_gpu_remap_udmabuf(struct virtio_gpu_simple_resource *res,
> +                                     VFIODevice *vdev)
>   {
>       res->remapped = mmap(NULL, res->blob_size, PROT_READ,
>                            MAP_SHARED, res->dmabuf_fd, 0);
>       if (res->remapped == MAP_FAILED) {
> +        if (vdev) {
> +            res->remapped = vfio_dmabuf_mmap(res, vdev);
> +            if (res->remapped != MAP_FAILED) {
> +                return;
> +            }
> +        }
>           warn_report("%s: dmabuf mmap failed: %s", __func__,
>                       strerror(errno));
>           res->remapped = NULL;
> @@ -130,18 +220,59 @@ bool virtio_gpu_have_udmabuf(void)
>   
>   void virtio_gpu_init_udmabuf(struct virtio_gpu_simple_resource *res)
>   {
> +    VFIODevice *vdev = NULL;
>       void *pdata = NULL;
> +    ram_addr_t offset;
> +    RAMBlock *rb;
>   
>       res->dmabuf_fd = -1;
>       if (res->iov_cnt == 1 &&
>           res->iov[0].iov_len < 4096) {
>           pdata = res->iov[0].iov_base;
>       } else {
> -        virtio_gpu_create_udmabuf(res);
> -        if (res->dmabuf_fd < 0) {
> +        rcu_read_lock();
> +        rb = qemu_ram_block_from_host(res->iov[0].iov_base, false, &offset);
> +        rcu_read_unlock();
> +
> +        if (!rb) {
> +            qemu_log_mask(LOG_GUEST_ERROR,
> +                          "%s: Could not find ram block for host address\n",
> +                          __func__);
>               return;
>           }
> -        virtio_gpu_remap_udmabuf(res);
> +
> +        if (memory_region_is_ram_device(rb->mr)) {
> +            vdev = vfio_device_lookup(rb->mr);
> +            if (!vdev) {
> +                qemu_log_mask(LOG_GUEST_ERROR,
> +                              "%s: Could not find device to create dmabuf\n",
> +                              __func__);
> +                return;
> +            }
> +
> +            vfio_create_dmabuf(vdev, res);
> +            if (res->dmabuf_fd < 0) {
> +                qemu_log_mask(LOG_GUEST_ERROR,
> +                              "%s: Could not create dmabuf from vfio device\n",
> +                              __func__);
> +                return;
> +            }
> +        } else if (memory_region_is_ram(rb->mr) && virtio_gpu_have_udmabuf()) {
> +            virtio_gpu_create_udmabuf(res);
> +            if (res->dmabuf_fd < 0) {
> +                qemu_log_mask(LOG_GUEST_ERROR,
> +                              "%s: Could not create dmabuf from memfd\n",
> +                              __func__);
> +                return;
> +            }
> +        } else {
> +            qemu_log_mask(LOG_GUEST_ERROR,
> +                          "%s: memory region cannot be used to create dmabuf\n",
> +                          __func__);
> +            return;
> +        }
> +
> +        virtio_gpu_remap_udmabuf(res, vdev);
>           if (!res->remapped) {
>               return;
>           }
> @@ -153,9 +284,7 @@ void virtio_gpu_init_udmabuf(struct virtio_gpu_simple_resource *res)
>   
>   void virtio_gpu_fini_udmabuf(struct virtio_gpu_simple_resource *res)
>   {
> -    if (res->remapped) {
> -        virtio_gpu_destroy_udmabuf(res);
> -    }
> +    virtio_gpu_destroy_udmabuf(res);
>   }
>   
>   static void virtio_gpu_free_dmabuf(VirtIOGPU *g, VGPUDMABuf *dmabuf)

RE: [PATCH v1 6/7] vfio/device: Add support for VFIO_DEVICE_FEATURE_DMA_BUF

[Kasireddy, Vivek]

Hi Cedric,

> Subject: Re: [PATCH v1 6/7] vfio/device: Add support for
> VFIO_DEVICE_FEATURE_DMA_BUF
> 
> Hello Vivek,
> 
> On 10/4/25 01:35, Vivek Kasireddy wrote:
> > In order to implement VFIO_DEVICE_FEATURE_DMA_BUF, 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 in order to invoke
> > this feature.
> >
> > Cc: Alex Williamson <alex.williamson@redhat.com>
> > Cc: Cédric Le Goater <clg@redhat.com>
> > Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> > ---
> >   hw/vfio/device.c              | 43 +++++++++++++++++++++++++++++++++++
> >   include/hw/vfio/vfio-device.h |  3 +++
> >   2 files changed, 46 insertions(+)
> >
> > diff --git a/hw/vfio/device.c b/hw/vfio/device.c
> > index 64f8750389..49070929ac 100644
> > --- a/hw/vfio/device.c
> > +++ b/hw/vfio/device.c
> > @@ -21,6 +21,7 @@
> >   #include "qemu/osdep.h"
> >   #include <sys/ioctl.h>
> >
> > +#include "system/ramblock.h"
> >   #include "hw/vfio/vfio-device.h"
> >   #include "hw/vfio/pci.h"
> >   #include "hw/hw.h"
> > @@ -592,3 +593,45 @@ static VFIODeviceIOOps vfio_device_io_ops_ioctl =
> {
> >       .region_read = vfio_device_io_region_read,
> >       .region_write = vfio_device_io_region_write,
> >   };
> > +
> > +int vfio_device_create_dmabuf(VFIODevice *vdev,
> 
> a 'vbasedev' name is preferred for VFIODevice variables/parameters.
Ok, will make the change.

> 
> > +                              struct iovec *iov, unsigned int iov_cnt)
> > +{
> > +    g_autofree struct vfio_device_feature *feature;
> 
> g_autofree variables should be set: 'feature = NULL'
Ok, got it. 

> 
> > +    struct vfio_device_feature_dma_buf *dma_buf;
> > +    ram_addr_t offset;
> > +    RAMBlock *rb;
> > +    size_t argsz;
> > +    int i, index;
> > +
> > +    argsz = sizeof(*feature) + sizeof (*dma_buf) +
> > +            sizeof(struct vfio_region_dma_range) * iov_cnt;
> > +    feature = g_malloc0(argsz);
> > +    dma_buf = (struct vfio_device_feature_dma_buf *)feature->data;
> > +
> > +    for (i = 0; i < iov_cnt; i++) {
> > +        rcu_read_lock();
> 
> Is it needed ?
Looks like it is indeed not needed. I'll remove it in the next version.

> 
> > +        rb = qemu_ram_block_from_host(iov[i].iov_base, false, &offset);
> > +        rcu_read_unlock();
> > +
> > +        if (!rb) {
> > +            return -1;
> 
> wouldn't an errno be more appropriate ?
It would. I'll add it.

> 
> > +        }
> > +
> > +        index = vfio_get_region_index_from_mr(rb->mr);
> > +        if (index < 0) {
> > +            return -1;
> > +        }
> > +
> > +        dma_buf->region_index = index;
> > +        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;
> > +    feature->argsz = argsz;
> > +    feature->flags = VFIO_DEVICE_FEATURE_GET |
> VFIO_DEVICE_FEATURE_DMA_BUF;
> > +
> > +    return ioctl(vdev->fd, VFIO_DEVICE_FEATURE, feature);
> Please use :
> 
>     return vbasedev->io_ops->device_feature(vbasedev, feature);
> 
> This would be returning an errno. Callers should be aware.
Makes sense and it seems a bit cleaner.

> 
> > +}
> > diff --git a/include/hw/vfio/vfio-device.h b/include/hw/vfio/vfio-device.h
> > index bdb106c937..74b3c4eef7 100644
> > --- a/include/hw/vfio/vfio-device.h
> > +++ b/include/hw/vfio/vfio-device.h
> > @@ -279,6 +279,9 @@ int vfio_device_get_irq_info(VFIODevice *vbasedev,
> int index,
> >                                   struct vfio_irq_info *info);
> >
> >   int vfio_get_region_index_from_mr(MemoryRegion *mr);
> > +
> > +int vfio_device_create_dmabuf(VFIODevice *vbasedev,
> > +                              struct iovec *iov, unsigned int iov_cnt);
> 
> 
> Since this is an external routine, please add documentation.
Will do.

Thanks,
Vivek

> 
> 
> Thanks,
> 
> C.
> 
> 
> >   #endif
> >
> >   /* Returns 0 on success, or a negative errno. */

RE: [PATCH v1 4/7] vfio/region: Add a helper to get region index from memory region

[Kasireddy, Vivek]

Hi Cedric,

> Subject: Re: [PATCH v1 4/7] vfio/region: Add a helper to get region index from
> memory region
> 
> Hello Vivek,
> 
> On 10/4/25 01:35, Vivek Kasireddy wrote:
> > Having a way to figure out the region index (or bar) associated
> > with a memory region is helpful in various scenarios. For example,
> > this capability can be useful in retrieving the region info needed
> > for mapping a part of a VFIO region or creating a dmabuf.
> >
> > Cc: Alex Williamson <alex.williamson@redhat.com>
> > Cc: Cédric Le Goater <clg@redhat.com>
> > Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> > ---
> >   hw/vfio/region.c              | 14 ++++++++++++++
> >   include/hw/vfio/vfio-device.h |  2 ++
> >   2 files changed, 16 insertions(+)
> >
> > diff --git a/hw/vfio/region.c b/hw/vfio/region.c
> > index b165ab0b93..8837afc97f 100644
> > --- a/hw/vfio/region.c
> > +++ b/hw/vfio/region.c
> > @@ -398,3 +398,17 @@ void vfio_region_mmaps_set_enabled(VFIORegion
> *region, bool enabled)
> >       trace_vfio_region_mmaps_set_enabled(memory_region_name(region-
> >mem),
> >                                           enabled);
> >   }
> > +
> > +int vfio_get_region_index_from_mr(MemoryRegion *mr)
> > +{
> > +    VFIORegion *region = mr->opaque;
> > +
> > +    if (mr->ops != &vfio_region_ops) {
> > +        mr = mr->container;
> 
> May be start the routine with :
> 
>      while (mr->container) {
>          mr = mr->container;
>      }
Ok, makes sense.

> 
> > +        if (mr->ops != &vfio_region_ops) {
> > +            return -1;
> 
> I think I would prefer returning a 'VFIORegion *' which should be
> NULL in case of error. Looks cleaner.
Given that VFIORegion type is no longer exposed to other subsystems,
I am guessing you meant adding two helpers: internal helper that would
return VFIORegion * and an external routine that would call the internal
helper and return region->nr?

> 
> 
> > +        }
> > +	region = mr->opaque;
> > +    }
> > +    return region->nr;
> > +}
> > diff --git a/include/hw/vfio/vfio-device.h b/include/hw/vfio/vfio-device.h
> > index 7e9aed6d3c..bdb106c937 100644
> > --- a/include/hw/vfio/vfio-device.h
> > +++ b/include/hw/vfio/vfio-device.h
> > @@ -277,6 +277,8 @@ 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);
> > +
> > +int vfio_get_region_index_from_mr(MemoryRegion *mr);
> Please add documentation.
Sure, will do.

Thanks,
Vivek

> 
> Thanks,
> 
> C.
> 
> 
> >   #endif
> >
> >   /* Returns 0 on success, or a negative errno. */

RE: [PATCH v1 0/7] vfio: Implement VFIO_DEVICE_FEATURE_DMA_BUF and use it in virtio-gpu

[Kasireddy, Vivek]

Hi Cedric,

> Subject: Re: [PATCH v1 0/7] vfio: Implement
> VFIO_DEVICE_FEATURE_DMA_BUF and use it in virtio-gpu
> 
> Vivek,
> 
> On 10/4/25 01:35, Vivek Kasireddy wrote:
> > 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 implement VFIO_DEVICE_FEATURE_DMA_BUF and
> > leverage it in virtio-gpu.
> >
> > So, while creating the blobs we use memory_region_is_ram_device() to
> > figure out if the blob is backed by memfd or a vfio-pci device. If
> > it is determined that the blob is backed by vfio-pci device region,
> > instead of calling into udmabuf driver to create dmabuf we would
> > now call into vfio-pci driver to have a dmabuf created on the Host.
> >
> > 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 by running Gnome Wayland
> 
> Could you please be more precise on the HW ?
Ok, I'll mention the specific model.

> 
> > (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 vfio-pci kernel driver series:
> > https://lore.kernel.org/dri-devel/cover.1754311439.git.leon@kernel.org/
> > 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.williamson@redhat.com>
> > Cc: Cédric Le Goater <clg@redhat.com>
> > Cc: Leon Romanovsky <leon@kernel.org>
> > Cc: Leon Romanovsky <leonro@nvidia.com>
> > Cc: Jason Gunthorpe <jgg@nvidia.com>
> > Cc: Dongwon Kim <dongwon.kim@intel.com>
> >
> > Vivek Kasireddy (7):
> >    virtio-gpu: Recreate the resource's dmabuf if new backing is attached
> >    virtio-gpu: Don't rely on res->blob to identify blob resources
> >    virtio-gpu: Find hva for Guest's DMA addr associated with a ram device
> >    vfio/region: Add a helper to get region index from memory region
> >    linux-headers: Update vfio.h to include VFIO_DEVICE_FEATURE_DMA_BUF
> >    vfio/device: Add support for VFIO_DEVICE_FEATURE_DMA_BUF
> >    virtio-gpu-udmabuf: Create dmabuf for blobs associated with VFIO
> >      devices
> >
> >   hw/display/Kconfig              |   5 ++
> >   hw/display/virtio-gpu-udmabuf.c | 143
> ++++++++++++++++++++++++++++++--
> >   hw/display/virtio-gpu.c         |  53 +++++++++---
> >   hw/vfio/device.c                |  43 ++++++++++
> >   hw/vfio/region.c                |  14 ++++
> >   include/hw/vfio/vfio-device.h   |   5 ++
> >   linux-headers/linux/vfio.h      |  25 ++++++
> >   7 files changed, 270 insertions(+), 18 deletions(-)
> >
> 
> Please cc: Alex and I on all patches.
Sure, will do.

Thanks,
Vivek

> 
> Thanks,
> 
> C.

RE: [PATCH v1 7/7] virtio-gpu-udmabuf: Create dmabuf for blobs associated with VFIO devices

[Kasireddy, Vivek]

Hi Cedric,

> Subject: Re: [PATCH v1 7/7] virtio-gpu-udmabuf: Create dmabuf for blobs
> associated with VFIO devices
> 
> On 10/4/25 01:36, Vivek Kasireddy wrote:
> > In addition to memfd, a blob resource can also have its backing
> > storage in a VFIO device region. Therefore, we first need to figure
> > out if the blob is backed by a VFIO device region or a memfd before
> > we can call the right API to get a dmabuf fd created.
> >
> > So, once we have the ramblock and the associated mr, we rely on
> > memory_region_is_ram_device() to tell us where the backing storage
> > is located. If the blob resource is VFIO backed, we try to find the
> > right VFIO device that contains the blob and then invoke the API
> > vfio_device_create_dmabuf().
> >
> > Note that in virtio_gpu_remap_udmabuf(), we first try to test if
> > the VFIO dmabuf exporter supports mmap or not. If it doesn't, we
> > use the VFIO device fd directly to create the CPU 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>
> > Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> > ---
> >   hw/display/Kconfig              |   5 ++
> >   hw/display/virtio-gpu-udmabuf.c | 143
> ++++++++++++++++++++++++++++++--
> >   2 files changed, 141 insertions(+), 7 deletions(-)
> >
> > diff --git a/hw/display/Kconfig b/hw/display/Kconfig
> > index 1e95ab28ef..0d090f25f5 100644
> > --- a/hw/display/Kconfig
> > +++ b/hw/display/Kconfig
> > @@ -106,6 +106,11 @@ config VIRTIO_VGA
> >       depends on VIRTIO_PCI
> >       select VGA
> >
> > +config VIRTIO_GPU_VFIO_BLOB
> > +    bool
> > +    default y
> > +    depends on VFIO
> > +
> >   config VHOST_USER_GPU
> >       bool
> >       default y
> > diff --git a/hw/display/virtio-gpu-udmabuf.c b/hw/display/virtio-gpu-
> udmabuf.c
> > index d804f321aa..bd06b4f300 100644
> > --- a/hw/display/virtio-gpu-udmabuf.c
> > +++ b/hw/display/virtio-gpu-udmabuf.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,6 +28,33 @@
> >   #include "standard-headers/linux/udmabuf.h"
> >   #include "standard-headers/drm/drm_fourcc.h"
> >
> > +static void vfio_create_dmabuf(VFIODevice *vdev,
> > +                               struct virtio_gpu_simple_resource *res)
> > +{
> > +#if defined(VIRTIO_GPU_VFIO_BLOB)
> > +    res->dmabuf_fd = vfio_device_create_dmabuf(vdev, res->iov, res-
> >iov_cnt);
> 
> I didn't realize an fd was returned until this patch. I'd suggest
> renaming vfio_device_create_dmabuf() to vfio_device_create_dmabuf_fd(),
> or something explicit IMO.
Yeah, makes sense. I'll change the name.

> 
> > +    if (res->dmabuf_fd < 0) {
> > +        qemu_log_mask(LOG_GUEST_ERROR,
> > +                      "%s: VFIO_DEVICE_FEATURE_DMA_BUF: %s\n",
> > +                      __func__, strerror(errno));
> > +    }
> > +#endif
> > +}
> > +
> > +static VFIODevice *vfio_device_lookup(MemoryRegion *mr)
> > +{
> > +#if defined(VIRTIO_GPU_VFIO_BLOB)
> > +    VFIODevice *vdev;
> > +
> > +    QLIST_FOREACH(vdev, &vfio_device_list, next) {
> Hmm, I'm not sure we want to expose the VFIOdevice list to other
> subsystems. I understand the need, and it's faster than iterating
> over QOM devices, but I'd prefer that an API be provided for this
> purpose.
Ok, I'll add a new API for this purpose.

> 
> I missed how much vfio_device_list has proliferated. Needs a check.
> 
> 
> > +        if (vdev->dev == mr->dev) {
> > +            return vdev;
> > +        }
> > +    }
> > +#endif
> > +    return NULL;
> > +}
> > +
> >   static void virtio_gpu_create_udmabuf(struct virtio_gpu_simple_resource
> *res)
> >   {
> >       struct udmabuf_create_list *list;
> > @@ -68,11 +96,73 @@ 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 *vfio_dmabuf_mmap(struct virtio_gpu_simple_resource *res,
> > +                              VFIODevice *vdev)
> > +{
> > +    struct vfio_region_info *info;
> > +    ram_addr_t offset, len = 0;
> > +    void *map, *submap;
> > +    int i, ret = -1;
> > +    RAMBlock *rb;
> > +
> > +    /*
> > +     * 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, res->blob_size, PROT_READ, MAP_SHARED, vdev-
> >fd, 0);
> > +    if (map == MAP_FAILED) {
> > +        return map;
> > +    }
> > +
> > +    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) {
> > +            goto err;
> > +        }
> > +
> > +#if defined(VIRTIO_GPU_VFIO_BLOB)
> > +        ret = vfio_get_region_index_from_mr(rb->mr);
> > +        if (ret < 0) {
> > +            goto err;
> > +        }
> > +
> > +        ret = vfio_device_get_region_info(vdev, ret, &info);
> > +#endif
> > +        if (ret < 0) {
> > +            goto err;
> > +        }
> 
> "hmm" again. Not this patch fault but we lack proper documentation
> for the VFIO API. Something to work on. Since this patch is using
> vfio_device_get_region_info() could you please add documentation
> for it ?
Sure, I'll add documentation for this and other routines you mentioned,
in the next version.

Thanks,
Vivek

> 
> 
> Thanks,
> 
> C.
> 
> 
> 
> > +        submap = mmap(map + len, res->iov[i].iov_len, PROT_READ,
> > +                      MAP_SHARED | MAP_FIXED, vdev->fd,
> > +                      info->offset + offset);
> > +        if (submap == MAP_FAILED) {
> > +            goto err;
> > +        }
> > +
> > +        len += res->iov[i].iov_len;
> > +    }
> > +    return map;
> > +err:
> > +    munmap(map, res->blob_size);
> > +    return MAP_FAILED;
> > +}
> > +
> > +static void virtio_gpu_remap_udmabuf(struct virtio_gpu_simple_resource
> *res,
> > +                                     VFIODevice *vdev)
> >   {
> >       res->remapped = mmap(NULL, res->blob_size, PROT_READ,
> >                            MAP_SHARED, res->dmabuf_fd, 0);
> >       if (res->remapped == MAP_FAILED) {
> > +        if (vdev) {
> > +            res->remapped = vfio_dmabuf_mmap(res, vdev);
> > +            if (res->remapped != MAP_FAILED) {
> > +                return;
> > +            }
> > +        }
> >           warn_report("%s: dmabuf mmap failed: %s", __func__,
> >                       strerror(errno));
> >           res->remapped = NULL;
> > @@ -130,18 +220,59 @@ bool virtio_gpu_have_udmabuf(void)
> >
> >   void virtio_gpu_init_udmabuf(struct virtio_gpu_simple_resource *res)
> >   {
> > +    VFIODevice *vdev = NULL;
> >       void *pdata = NULL;
> > +    ram_addr_t offset;
> > +    RAMBlock *rb;
> >
> >       res->dmabuf_fd = -1;
> >       if (res->iov_cnt == 1 &&
> >           res->iov[0].iov_len < 4096) {
> >           pdata = res->iov[0].iov_base;
> >       } else {
> > -        virtio_gpu_create_udmabuf(res);
> > -        if (res->dmabuf_fd < 0) {
> > +        rcu_read_lock();
> > +        rb = qemu_ram_block_from_host(res->iov[0].iov_base, false, &offset);
> > +        rcu_read_unlock();
> > +
> > +        if (!rb) {
> > +            qemu_log_mask(LOG_GUEST_ERROR,
> > +                          "%s: Could not find ram block for host address\n",
> > +                          __func__);
> >               return;
> >           }
> > -        virtio_gpu_remap_udmabuf(res);
> > +
> > +        if (memory_region_is_ram_device(rb->mr)) {
> > +            vdev = vfio_device_lookup(rb->mr);
> > +            if (!vdev) {
> > +                qemu_log_mask(LOG_GUEST_ERROR,
> > +                              "%s: Could not find device to create dmabuf\n",
> > +                              __func__);
> > +                return;
> > +            }
> > +
> > +            vfio_create_dmabuf(vdev, res);
> > +            if (res->dmabuf_fd < 0) {
> > +                qemu_log_mask(LOG_GUEST_ERROR,
> > +                              "%s: Could not create dmabuf from vfio device\n",
> > +                              __func__);
> > +                return;
> > +            }
> > +        } else if (memory_region_is_ram(rb->mr) &&
> virtio_gpu_have_udmabuf()) {
> > +            virtio_gpu_create_udmabuf(res);
> > +            if (res->dmabuf_fd < 0) {
> > +                qemu_log_mask(LOG_GUEST_ERROR,
> > +                              "%s: Could not create dmabuf from memfd\n",
> > +                              __func__);
> > +                return;
> > +            }
> > +        } else {
> > +            qemu_log_mask(LOG_GUEST_ERROR,
> > +                          "%s: memory region cannot be used to create dmabuf\n",
> > +                          __func__);
> > +            return;
> > +        }
> > +
> > +        virtio_gpu_remap_udmabuf(res, vdev);
> >           if (!res->remapped) {
> >               return;
> >           }
> > @@ -153,9 +284,7 @@ void virtio_gpu_init_udmabuf(struct
> virtio_gpu_simple_resource *res)
> >
> >   void virtio_gpu_fini_udmabuf(struct virtio_gpu_simple_resource *res)
> >   {
> > -    if (res->remapped) {
> > -        virtio_gpu_destroy_udmabuf(res);
> > -    }
> > +    virtio_gpu_destroy_udmabuf(res);
> >   }
> >
> >   static void virtio_gpu_free_dmabuf(VirtIOGPU *g, VGPUDMABuf *dmabuf)

Re: [PATCH v1 7/7] virtio-gpu-udmabuf: Create dmabuf for blobs associated with VFIO devices

[Cédric Le Goater]

On 10/6/25 17:59, Cédric Le Goater wrote:
> On 10/4/25 01:36, Vivek Kasireddy wrote:
>> In addition to memfd, a blob resource can also have its backing
>> storage in a VFIO device region. Therefore, we first need to figure
>> out if the blob is backed by a VFIO device region or a memfd before
>> we can call the right API to get a dmabuf fd created.
>>
>> So, once we have the ramblock and the associated mr, we rely on
>> memory_region_is_ram_device() to tell us where the backing storage
>> is located. If the blob resource is VFIO backed, we try to find the
>> right VFIO device that contains the blob and then invoke the API
>> vfio_device_create_dmabuf().
>>
>> Note that in virtio_gpu_remap_udmabuf(), we first try to test if
>> the VFIO dmabuf exporter supports mmap or not. If it doesn't, we
>> use the VFIO device fd directly to create the CPU 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>
>> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
>> ---
>>   hw/display/Kconfig              |   5 ++
>>   hw/display/virtio-gpu-udmabuf.c | 143 ++++++++++++++++++++++++++++++--
>>   2 files changed, 141 insertions(+), 7 deletions(-)
>>
>> diff --git a/hw/display/Kconfig b/hw/display/Kconfig
>> index 1e95ab28ef..0d090f25f5 100644
>> --- a/hw/display/Kconfig
>> +++ b/hw/display/Kconfig
>> @@ -106,6 +106,11 @@ config VIRTIO_VGA
>>       depends on VIRTIO_PCI
>>       select VGA
>> +config VIRTIO_GPU_VFIO_BLOB
>> +    bool
>> +    default y
>> +    depends on VFIO
>> +
>>   config VHOST_USER_GPU
>>       bool
>>       default y
>> diff --git a/hw/display/virtio-gpu-udmabuf.c b/hw/display/virtio-gpu-udmabuf.c
>> index d804f321aa..bd06b4f300 100644
>> --- a/hw/display/virtio-gpu-udmabuf.c
>> +++ b/hw/display/virtio-gpu-udmabuf.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,6 +28,33 @@
>>   #include "standard-headers/linux/udmabuf.h"
>>   #include "standard-headers/drm/drm_fourcc.h"
>> +static void vfio_create_dmabuf(VFIODevice *vdev,
>> +                               struct virtio_gpu_simple_resource *res)
>> +{
>> +#if defined(VIRTIO_GPU_VFIO_BLOB)
>> +    res->dmabuf_fd = vfio_device_create_dmabuf(vdev, res->iov, res->iov_cnt);
> 
> I didn't realize an fd was returned until this patch. I'd suggest
> renaming vfio_device_create_dmabuf() to vfio_device_create_dmabuf_fd(),
> or something explicit IMO.
> 
>> +    if (res->dmabuf_fd < 0) {
>> +        qemu_log_mask(LOG_GUEST_ERROR,
>> +                      "%s: VFIO_DEVICE_FEATURE_DMA_BUF: %s\n",
>> +                      __func__, strerror(errno));
>> +    }
>> +#endif
>> +}
>> +
>> +static VFIODevice *vfio_device_lookup(MemoryRegion *mr)
>> +{
>> +#if defined(VIRTIO_GPU_VFIO_BLOB)
>> +    VFIODevice *vdev;
>> +
>> +    QLIST_FOREACH(vdev, &vfio_device_list, next) {
> Hmm, I'm not sure we want to expose the VFIOdevice list to other
> subsystems. I understand the need, and it's faster than iterating
> over QOM devices, but I’d prefer that an API be provided for this
> purpose.
> 
> I missed how much vfio_device_list has proliferated. Needs a check.
> 
> 
>> +        if (vdev->dev == mr->dev) {
>> +            return vdev;
>> +        }
>> +    }
>> +#endif
>> +    return NULL;
>> +}
>> +
>>   static void virtio_gpu_create_udmabuf(struct virtio_gpu_simple_resource *res)
>>   {
>>       struct udmabuf_create_list *list;
>> @@ -68,11 +96,73 @@ 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 *vfio_dmabuf_mmap(struct virtio_gpu_simple_resource *res,
>> +                              VFIODevice *vdev)
>> +{
>> +    struct vfio_region_info *info;
>> +    ram_addr_t offset, len = 0;
>> +    void *map, *submap;
>> +    int i, ret = -1;
>> +    RAMBlock *rb;
>> +
>> +    /*
>> +     * 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, res->blob_size, PROT_READ, MAP_SHARED, vdev->fd, 0);
>> +    if (map == MAP_FAILED) {
>> +        return map;
>> +    }
>> +
>> +    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) {
>> +            goto err;
>> +        }
>> +
>> +#if defined(VIRTIO_GPU_VFIO_BLOB)
>> +        ret = vfio_get_region_index_from_mr(rb->mr);
>> +        if (ret < 0) {
>> +            goto err;
>> +        }
>> +
>> +        ret = vfio_device_get_region_info(vdev, ret, &info);
>> +#endif
>> +        if (ret < 0) {
>> +            goto err;
>> +        }
> 
> "hmm" again. Not this patch fault but we lack proper documentation
> for the VFIO API. Something to work on. Since this patch is using
> vfio_device_get_region_info() could you please add documentation
> for it ?
> 
> 
> Thanks,
> 
> C.
> 
> 
> 
>> +        submap = mmap(map + len, res->iov[i].iov_len, PROT_READ,
>> +                      MAP_SHARED | MAP_FIXED, vdev->fd,
>> +                      info->offset + offset);

One more thing which needs a fix :

'info' is uninitialized if !VIRTIO_GPU_VFIO_BLOB.


Thanks,

C.



>> +        if (submap == MAP_FAILED) {
>> +            goto err;
>> +        }
>> +
>> +        len += res->iov[i].iov_len;
>> +    }
>> +    return map;
>> +err:
>> +    munmap(map, res->blob_size);
>> +    return MAP_FAILED;
>> +}
>> +
>> +static void virtio_gpu_remap_udmabuf(struct virtio_gpu_simple_resource *res,
>> +                                     VFIODevice *vdev)
>>   {
>>       res->remapped = mmap(NULL, res->blob_size, PROT_READ,
>>                            MAP_SHARED, res->dmabuf_fd, 0);
>>       if (res->remapped == MAP_FAILED) {
>> +        if (vdev) {
>> +            res->remapped = vfio_dmabuf_mmap(res, vdev);
>> +            if (res->remapped != MAP_FAILED) {
>> +                return;
>> +            }
>> +        }
>>           warn_report("%s: dmabuf mmap failed: %s", __func__,
>>                       strerror(errno));
>>           res->remapped = NULL;
>> @@ -130,18 +220,59 @@ bool virtio_gpu_have_udmabuf(void)
>>   void virtio_gpu_init_udmabuf(struct virtio_gpu_simple_resource *res)
>>   {
>> +    VFIODevice *vdev = NULL;
>>       void *pdata = NULL;
>> +    ram_addr_t offset;
>> +    RAMBlock *rb;
>>       res->dmabuf_fd = -1;
>>       if (res->iov_cnt == 1 &&
>>           res->iov[0].iov_len < 4096) {
>>           pdata = res->iov[0].iov_base;
>>       } else {
>> -        virtio_gpu_create_udmabuf(res);
>> -        if (res->dmabuf_fd < 0) {
>> +        rcu_read_lock();
>> +        rb = qemu_ram_block_from_host(res->iov[0].iov_base, false, &offset);
>> +        rcu_read_unlock();
>> +
>> +        if (!rb) {
>> +            qemu_log_mask(LOG_GUEST_ERROR,
>> +                          "%s: Could not find ram block for host address\n",
>> +                          __func__);
>>               return;
>>           }
>> -        virtio_gpu_remap_udmabuf(res);
>> +
>> +        if (memory_region_is_ram_device(rb->mr)) {
>> +            vdev = vfio_device_lookup(rb->mr);
>> +            if (!vdev) {
>> +                qemu_log_mask(LOG_GUEST_ERROR,
>> +                              "%s: Could not find device to create dmabuf\n",
>> +                              __func__);
>> +                return;
>> +            }
>> +
>> +            vfio_create_dmabuf(vdev, res);
>> +            if (res->dmabuf_fd < 0) {
>> +                qemu_log_mask(LOG_GUEST_ERROR,
>> +                              "%s: Could not create dmabuf from vfio device\n",
>> +                              __func__);
>> +                return;
>> +            }
>> +        } else if (memory_region_is_ram(rb->mr) && virtio_gpu_have_udmabuf()) {
>> +            virtio_gpu_create_udmabuf(res);
>> +            if (res->dmabuf_fd < 0) {
>> +                qemu_log_mask(LOG_GUEST_ERROR,
>> +                              "%s: Could not create dmabuf from memfd\n",
>> +                              __func__);
>> +                return;
>> +            }
>> +        } else {
>> +            qemu_log_mask(LOG_GUEST_ERROR,
>> +                          "%s: memory region cannot be used to create dmabuf\n",
>> +                          __func__);
>> +            return;
>> +        }
>> +
>> +        virtio_gpu_remap_udmabuf(res, vdev);
>>           if (!res->remapped) {
>>               return;
>>           }
>> @@ -153,9 +284,7 @@ void virtio_gpu_init_udmabuf(struct virtio_gpu_simple_resource *res)
>>   void virtio_gpu_fini_udmabuf(struct virtio_gpu_simple_resource *res)
>>   {
>> -    if (res->remapped) {
>> -        virtio_gpu_destroy_udmabuf(res);
>> -    }
>> +    virtio_gpu_destroy_udmabuf(res);
>>   }
>>   static void virtio_gpu_free_dmabuf(VirtIOGPU *g, VGPUDMABuf *dmabuf)
> 

Re: [PATCH v1 4/7] vfio/region: Add a helper to get region index from memory region

[Cédric Le Goater]

On 10/7/25 06:50, Kasireddy, Vivek wrote:
> Hi Cedric,
> 
>> Subject: Re: [PATCH v1 4/7] vfio/region: Add a helper to get region index from
>> memory region
>>
>> Hello Vivek,
>>
>> On 10/4/25 01:35, Vivek Kasireddy wrote:
>>> Having a way to figure out the region index (or bar) associated
>>> with a memory region is helpful in various scenarios. For example,
>>> this capability can be useful in retrieving the region info needed
>>> for mapping a part of a VFIO region or creating a dmabuf.
>>>
>>> Cc: Alex Williamson <alex.williamson@redhat.com>
>>> Cc: Cédric Le Goater <clg@redhat.com>
>>> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
>>> ---
>>>    hw/vfio/region.c              | 14 ++++++++++++++
>>>    include/hw/vfio/vfio-device.h |  2 ++
>>>    2 files changed, 16 insertions(+)
>>>
>>> diff --git a/hw/vfio/region.c b/hw/vfio/region.c
>>> index b165ab0b93..8837afc97f 100644
>>> --- a/hw/vfio/region.c
>>> +++ b/hw/vfio/region.c
>>> @@ -398,3 +398,17 @@ void vfio_region_mmaps_set_enabled(VFIORegion
>> *region, bool enabled)
>>>        trace_vfio_region_mmaps_set_enabled(memory_region_name(region-
>>> mem),
>>>                                            enabled);
>>>    }
>>> +
>>> +int vfio_get_region_index_from_mr(MemoryRegion *mr)
>>> +{
>>> +    VFIORegion *region = mr->opaque;
>>> +
>>> +    if (mr->ops != &vfio_region_ops) {
>>> +        mr = mr->container;
>>
>> May be start the routine with :
>>
>>       while (mr->container) {
>>           mr = mr->container;
>>       }
> Ok, makes sense.
> 
>>
>>> +        if (mr->ops != &vfio_region_ops) {
>>> +            return -1;
>>
>> I think I would prefer returning a 'VFIORegion *' which should be
>> NULL in case of error. Looks cleaner.
> Given that VFIORegion type is no longer exposed to other subsystems,
> I am guessing you meant adding two helpers: internal helper that would
> return VFIORegion * and an external routine that would call the internal
> helper and return region->nr?


After seeing how vfio_get_region_index_from_mr() was used in
vfio_dmabuf_mmap(), I think it is fine to keep your proposal
unchanged. we can refine it later.


Thanks,

C.


> 
>>
>>
>>> +        }
>>> +	region = mr->opaque;
>>> +    }
>>> +    return region->nr;
>>> +}
>>> diff --git a/include/hw/vfio/vfio-device.h b/include/hw/vfio/vfio-device.h
>>> index 7e9aed6d3c..bdb106c937 100644
>>> --- a/include/hw/vfio/vfio-device.h
>>> +++ b/include/hw/vfio/vfio-device.h
>>> @@ -277,6 +277,8 @@ 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);
>>> +
>>> +int vfio_get_region_index_from_mr(MemoryRegion *mr);
>> Please add documentation.
> Sure, will do.
> 
> Thanks,
> Vivek
> 
>>
>> Thanks,
>>
>> C.
>>
>>
>>>    #endif
>>>
>>>    /* Returns 0 on success, or a negative errno. */
> 

Re: [PATCH v1 2/7] virtio-gpu: Don't rely on res->blob to identify blob resources

[Akihiko Odaki]

On 2025/10/04 8:35, Vivek Kasireddy wrote:
> The res->blob pointer may not be valid (non-NULL) for some blobs
> where the backing storage is not memfd based. Therefore, we cannot
> use it to determine if a resource is a blob or not. Instead, we
> could use res->blob_size to make this determination as it is
> non-zero for blob resources regardless of where their backing
> storage is located.

I think this patch is no longer necessary since now you add code to 
mmap() VFIO storage with "[PATCH v1 7/7] virtio-gpu-udmabuf: Create 
dmabuf for blobs associated with VFIO devices".

Re: [PATCH v1 7/7] virtio-gpu-udmabuf: Create dmabuf for blobs associated with VFIO devices

[Akihiko Odaki]

On 2025/10/04 8:36, Vivek Kasireddy wrote:
> In addition to memfd, a blob resource can also have its backing
> storage in a VFIO device region. Therefore, we first need to figure
> out if the blob is backed by a VFIO device region or a memfd before
> we can call the right API to get a dmabuf fd created.
> 
> So, once we have the ramblock and the associated mr, we rely on
> memory_region_is_ram_device() to tell us where the backing storage
> is located. If the blob resource is VFIO backed, we try to find the
> right VFIO device that contains the blob and then invoke the API
> vfio_device_create_dmabuf().
> 
> Note that in virtio_gpu_remap_udmabuf(), we first try to test if
> the VFIO dmabuf exporter supports mmap or not. If it doesn't, we
> use the VFIO device fd directly to create the CPU mapping.

It is odd to handle VFIO DMA-BUF in a function named "udmabuf". The 
function and source file need to be renamed.

> 
> 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>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> ---
>   hw/display/Kconfig              |   5 ++
>   hw/display/virtio-gpu-udmabuf.c | 143 ++++++++++++++++++++++++++++++--
>   2 files changed, 141 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/display/Kconfig b/hw/display/Kconfig
> index 1e95ab28ef..0d090f25f5 100644
> --- a/hw/display/Kconfig
> +++ b/hw/display/Kconfig
> @@ -106,6 +106,11 @@ config VIRTIO_VGA
>       depends on VIRTIO_PCI
>       select VGA
>   
> +config VIRTIO_GPU_VFIO_BLOB
> +    bool
> +    default y
> +    depends on VFIO
> +
>   config VHOST_USER_GPU
>       bool
>       default y
> diff --git a/hw/display/virtio-gpu-udmabuf.c b/hw/display/virtio-gpu-udmabuf.c
> index d804f321aa..bd06b4f300 100644
> --- a/hw/display/virtio-gpu-udmabuf.c
> +++ b/hw/display/virtio-gpu-udmabuf.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,6 +28,33 @@
>   #include "standard-headers/linux/udmabuf.h"
>   #include "standard-headers/drm/drm_fourcc.h"
>   
> +static void vfio_create_dmabuf(VFIODevice *vdev,
> +                               struct virtio_gpu_simple_resource *res)
> +{
> +#if defined(VIRTIO_GPU_VFIO_BLOB)
> +    res->dmabuf_fd = vfio_device_create_dmabuf(vdev, res->iov, res->iov_cnt);
> +    if (res->dmabuf_fd < 0) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: VFIO_DEVICE_FEATURE_DMA_BUF: %s\n",
> +                      __func__, strerror(errno));
> +    }
> +#endif
> +}
> +
> +static VFIODevice *vfio_device_lookup(MemoryRegion *mr)
> +{
> +#if defined(VIRTIO_GPU_VFIO_BLOB)
> +    VFIODevice *vdev;
> +
> +    QLIST_FOREACH(vdev, &vfio_device_list, next) {
> +        if (vdev->dev == mr->dev) {
> +            return vdev;
> +        }
> +    }
> +#endif
> +    return NULL;
> +}
> +
>   static void virtio_gpu_create_udmabuf(struct virtio_gpu_simple_resource *res)
>   {
>       struct udmabuf_create_list *list;
> @@ -68,11 +96,73 @@ 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 *vfio_dmabuf_mmap(struct virtio_gpu_simple_resource *res,
> +                              VFIODevice *vdev)
> +{
> +    struct vfio_region_info *info;
> +    ram_addr_t offset, len = 0;
> +    void *map, *submap;
> +    int i, ret = -1;
> +    RAMBlock *rb;
> +
> +    /*
> +     * 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, res->blob_size, PROT_READ, MAP_SHARED, vdev->fd, 0);
> +    if (map == MAP_FAILED) {
> +        return map;
> +    }
> +
> +    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();

I don't think this RCU lock is necessary. The documentation of 
qemu_ram_block_from_host() says:
 > By the time this function returns, the returned pointer is not
 > protected by RCU anymore.  If the caller is not within an RCU critical
 > section and does not hold the BQL, it must have other means of
 > protecting the pointer, such as a reference to the memory region that
 > owns the RAMBlock.

This function is called with the BQL held, and a reference to the memory 
region is also taken in virtio_gpu_dma_memory_map().

> +
> +        if (!rb) {
> +            goto err;
> +        }
> +
> +#if defined(VIRTIO_GPU_VFIO_BLOB)
> +        ret = vfio_get_region_index_from_mr(rb->mr);
> +        if (ret < 0) {
> +            goto err;
> +        }
> +
> +        ret = vfio_device_get_region_info(vdev, ret, &info);
> +#endif
> +        if (ret < 0) {
> +            goto err;
> +        }
> +
> +        submap = mmap(map + len, res->iov[i].iov_len, PROT_READ,
> +                      MAP_SHARED | MAP_FIXED, vdev->fd,
> +                      info->offset + offset);
> +        if (submap == MAP_FAILED) {
> +            goto err;
> +        }
> +
> +        len += res->iov[i].iov_len;
> +    }
> +    return map;
> +err:
> +    munmap(map, res->blob_size);
> +    return MAP_FAILED;
> +}
> +
> +static void virtio_gpu_remap_udmabuf(struct virtio_gpu_simple_resource *res,
> +                                     VFIODevice *vdev)
>   {
>       res->remapped = mmap(NULL, res->blob_size, PROT_READ,
>                            MAP_SHARED, res->dmabuf_fd, 0);
>       if (res->remapped == MAP_FAILED) {
> +        if (vdev) {
> +            res->remapped = vfio_dmabuf_mmap(res, vdev);
> +            if (res->remapped != MAP_FAILED) {
> +                return;
> +            }
> +        }
>           warn_report("%s: dmabuf mmap failed: %s", __func__,
>                       strerror(errno));
>           res->remapped = NULL;
> @@ -130,18 +220,59 @@ bool virtio_gpu_have_udmabuf(void)
>   
>   void virtio_gpu_init_udmabuf(struct virtio_gpu_simple_resource *res)
>   {
> +    VFIODevice *vdev = NULL;
>       void *pdata = NULL;
> +    ram_addr_t offset;
> +    RAMBlock *rb;
>   
>       res->dmabuf_fd = -1;
>       if (res->iov_cnt == 1 &&
>           res->iov[0].iov_len < 4096) {
>           pdata = res->iov[0].iov_base;
>       } else {
> -        virtio_gpu_create_udmabuf(res);
> -        if (res->dmabuf_fd < 0) {
> +        rcu_read_lock();
> +        rb = qemu_ram_block_from_host(res->iov[0].iov_base, false, &offset);
> +        rcu_read_unlock();
> +
> +        if (!rb) {
> +            qemu_log_mask(LOG_GUEST_ERROR,
> +                          "%s: Could not find ram block for host address\n",
> +                          __func__);
>               return;
>           }
> -        virtio_gpu_remap_udmabuf(res);
> +
> +        if (memory_region_is_ram_device(rb->mr)) {
> +            vdev = vfio_device_lookup(rb->mr);
> +            if (!vdev) {
> +                qemu_log_mask(LOG_GUEST_ERROR,
> +                              "%s: Could not find device to create dmabuf\n",
> +                              __func__);
> +                return;
> +            }
> +
> +            vfio_create_dmabuf(vdev, res);
> +            if (res->dmabuf_fd < 0) {
> +                qemu_log_mask(LOG_GUEST_ERROR,
> +                              "%s: Could not create dmabuf from vfio device\n",
> +                              __func__);
> +                return;
> +            }
> +        } else if (memory_region_is_ram(rb->mr) && virtio_gpu_have_udmabuf()) {

memory_region_is_ram_device() and memory_region_is_ram() should be 
called for all iov elements, not just the first one.

Calling virtio_gpu_have_udmabuf() here is redundant since 
virtio_gpu_device_realize() already calls it.

> +            virtio_gpu_create_udmabuf(res);
> +            if (res->dmabuf_fd < 0) {
> +                qemu_log_mask(LOG_GUEST_ERROR,
> +                              "%s: Could not create dmabuf from memfd\n",
> +                              __func__);
> +                return;
> +            }
> +        } else {
> +            qemu_log_mask(LOG_GUEST_ERROR,
> +                          "%s: memory region cannot be used to create dmabuf\n",
> +                          __func__);
> +            return;
> +        }
> +
> +        virtio_gpu_remap_udmabuf(res, vdev);
>           if (!res->remapped) {
>               return;
>           }
> @@ -153,9 +284,7 @@ void virtio_gpu_init_udmabuf(struct virtio_gpu_simple_resource *res)
>   
>   void virtio_gpu_fini_udmabuf(struct virtio_gpu_simple_resource *res)
>   {
> -    if (res->remapped) {
> -        virtio_gpu_destroy_udmabuf(res);
> -    }
> +    virtio_gpu_destroy_udmabuf(res);
>   }
>   
>   static void virtio_gpu_free_dmabuf(VirtIOGPU *g, VGPUDMABuf *dmabuf)

RE: [PATCH v1 2/7] virtio-gpu: Don't rely on res->blob to identify blob resources

[Kasireddy, Vivek]

Hi Akihiko,

> Subject: Re: [PATCH v1 2/7] virtio-gpu: Don't rely on res->blob to identify blob
> resources
> 
> On 2025/10/04 8:35, Vivek Kasireddy wrote:
> > The res->blob pointer may not be valid (non-NULL) for some blobs
> > where the backing storage is not memfd based. Therefore, we cannot
> > use it to determine if a resource is a blob or not. Instead, we
> > could use res->blob_size to make this determination as it is
> > non-zero for blob resources regardless of where their backing
> > storage is located.
> 
> I think this patch is no longer necessary since now you add code to
> mmap() VFIO storage with "[PATCH v1 7/7] virtio-gpu-udmabuf: Create
> dmabuf for blobs associated with VFIO devices".
Right, but given that mmap() can still fail for various reasons and this
use-case can work as long as dmabuf creation succeeds, I think it makes
sense to not rely on res->blob to determine if a resource is blob or not.

Thanks,
Vivek

RE: [PATCH v1 7/7] virtio-gpu-udmabuf: Create dmabuf for blobs associated with VFIO devices

[Kasireddy, Vivek]

Hi Akihiko,

> Subject: Re: [PATCH v1 7/7] virtio-gpu-udmabuf: Create dmabuf for blobs
> associated with VFIO devices
> 
> On 2025/10/04 8:36, Vivek Kasireddy wrote:
> > In addition to memfd, a blob resource can also have its backing
> > storage in a VFIO device region. Therefore, we first need to figure
> > out if the blob is backed by a VFIO device region or a memfd before
> > we can call the right API to get a dmabuf fd created.
> >
> > So, once we have the ramblock and the associated mr, we rely on
> > memory_region_is_ram_device() to tell us where the backing storage
> > is located. If the blob resource is VFIO backed, we try to find the
> > right VFIO device that contains the blob and then invoke the API
> > vfio_device_create_dmabuf().
> >
> > Note that in virtio_gpu_remap_udmabuf(), we first try to test if
> > the VFIO dmabuf exporter supports mmap or not. If it doesn't, we
> > use the VFIO device fd directly to create the CPU mapping.
> 
> It is odd to handle VFIO DMA-BUF in a function named "udmabuf". The
> function and source file need to be renamed.
Ok, makes sense. I'll rename it accordingly.

> 
> >
> > 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>
> > Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> > ---
> >   hw/display/Kconfig              |   5 ++
> >   hw/display/virtio-gpu-udmabuf.c | 143
> ++++++++++++++++++++++++++++++--
> >   2 files changed, 141 insertions(+), 7 deletions(-)
> >
> > diff --git a/hw/display/Kconfig b/hw/display/Kconfig
> > index 1e95ab28ef..0d090f25f5 100644
> > --- a/hw/display/Kconfig
> > +++ b/hw/display/Kconfig
> > @@ -106,6 +106,11 @@ config VIRTIO_VGA
> >       depends on VIRTIO_PCI
> >       select VGA
> >
> > +config VIRTIO_GPU_VFIO_BLOB
> > +    bool
> > +    default y
> > +    depends on VFIO
> > +
> >   config VHOST_USER_GPU
> >       bool
> >       default y
> > diff --git a/hw/display/virtio-gpu-udmabuf.c b/hw/display/virtio-gpu-
> udmabuf.c
> > index d804f321aa..bd06b4f300 100644
> > --- a/hw/display/virtio-gpu-udmabuf.c
> > +++ b/hw/display/virtio-gpu-udmabuf.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,6 +28,33 @@
> >   #include "standard-headers/linux/udmabuf.h"
> >   #include "standard-headers/drm/drm_fourcc.h"
> >
> > +static void vfio_create_dmabuf(VFIODevice *vdev,
> > +                               struct virtio_gpu_simple_resource *res)
> > +{
> > +#if defined(VIRTIO_GPU_VFIO_BLOB)
> > +    res->dmabuf_fd = vfio_device_create_dmabuf(vdev, res->iov, res-
> >iov_cnt);
> > +    if (res->dmabuf_fd < 0) {
> > +        qemu_log_mask(LOG_GUEST_ERROR,
> > +                      "%s: VFIO_DEVICE_FEATURE_DMA_BUF: %s\n",
> > +                      __func__, strerror(errno));
> > +    }
> > +#endif
> > +}
> > +
> > +static VFIODevice *vfio_device_lookup(MemoryRegion *mr)
> > +{
> > +#if defined(VIRTIO_GPU_VFIO_BLOB)
> > +    VFIODevice *vdev;
> > +
> > +    QLIST_FOREACH(vdev, &vfio_device_list, next) {
> > +        if (vdev->dev == mr->dev) {
> > +            return vdev;
> > +        }
> > +    }
> > +#endif
> > +    return NULL;
> > +}
> > +
> >   static void virtio_gpu_create_udmabuf(struct virtio_gpu_simple_resource
> *res)
> >   {
> >       struct udmabuf_create_list *list;
> > @@ -68,11 +96,73 @@ 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 *vfio_dmabuf_mmap(struct virtio_gpu_simple_resource *res,
> > +                              VFIODevice *vdev)
> > +{
> > +    struct vfio_region_info *info;
> > +    ram_addr_t offset, len = 0;
> > +    void *map, *submap;
> > +    int i, ret = -1;
> > +    RAMBlock *rb;
> > +
> > +    /*
> > +     * 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, res->blob_size, PROT_READ, MAP_SHARED, vdev-
> >fd, 0);
> > +    if (map == MAP_FAILED) {
> > +        return map;
> > +    }
> > +
> > +    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();
> 
> I don't think this RCU lock is necessary. The documentation of
> qemu_ram_block_from_host() says:
>  > By the time this function returns, the returned pointer is not
>  > protected by RCU anymore.  If the caller is not within an RCU critical
>  > section and does not hold the BQL, it must have other means of
>  > protecting the pointer, such as a reference to the memory region that
>  > owns the RAMBlock.
> 
> This function is called with the BQL held, and a reference to the memory
> region is also taken in virtio_gpu_dma_memory_map().
I agree. I'll remove the RCU lock.

> 
> > +
> > +        if (!rb) {
> > +            goto err;
> > +        }
> > +
> > +#if defined(VIRTIO_GPU_VFIO_BLOB)
> > +        ret = vfio_get_region_index_from_mr(rb->mr);
> > +        if (ret < 0) {
> > +            goto err;
> > +        }
> > +
> > +        ret = vfio_device_get_region_info(vdev, ret, &info);
> > +#endif
> > +        if (ret < 0) {
> > +            goto err;
> > +        }
> > +
> > +        submap = mmap(map + len, res->iov[i].iov_len, PROT_READ,
> > +                      MAP_SHARED | MAP_FIXED, vdev->fd,
> > +                      info->offset + offset);
> > +        if (submap == MAP_FAILED) {
> > +            goto err;
> > +        }
> > +
> > +        len += res->iov[i].iov_len;
> > +    }
> > +    return map;
> > +err:
> > +    munmap(map, res->blob_size);
> > +    return MAP_FAILED;
> > +}
> > +
> > +static void virtio_gpu_remap_udmabuf(struct virtio_gpu_simple_resource
> *res,
> > +                                     VFIODevice *vdev)
> >   {
> >       res->remapped = mmap(NULL, res->blob_size, PROT_READ,
> >                            MAP_SHARED, res->dmabuf_fd, 0);
> >       if (res->remapped == MAP_FAILED) {
> > +        if (vdev) {
> > +            res->remapped = vfio_dmabuf_mmap(res, vdev);
> > +            if (res->remapped != MAP_FAILED) {
> > +                return;
> > +            }
> > +        }
> >           warn_report("%s: dmabuf mmap failed: %s", __func__,
> >                       strerror(errno));
> >           res->remapped = NULL;
> > @@ -130,18 +220,59 @@ bool virtio_gpu_have_udmabuf(void)
> >
> >   void virtio_gpu_init_udmabuf(struct virtio_gpu_simple_resource *res)
> >   {
> > +    VFIODevice *vdev = NULL;
> >       void *pdata = NULL;
> > +    ram_addr_t offset;
> > +    RAMBlock *rb;
> >
> >       res->dmabuf_fd = -1;
> >       if (res->iov_cnt == 1 &&
> >           res->iov[0].iov_len < 4096) {
> >           pdata = res->iov[0].iov_base;
> >       } else {
> > -        virtio_gpu_create_udmabuf(res);
> > -        if (res->dmabuf_fd < 0) {
> > +        rcu_read_lock();
> > +        rb = qemu_ram_block_from_host(res->iov[0].iov_base, false, &offset);
> > +        rcu_read_unlock();
> > +
> > +        if (!rb) {
> > +            qemu_log_mask(LOG_GUEST_ERROR,
> > +                          "%s: Could not find ram block for host address\n",
> > +                          __func__);
> >               return;
> >           }
> > -        virtio_gpu_remap_udmabuf(res);
> > +
> > +        if (memory_region_is_ram_device(rb->mr)) {
> > +            vdev = vfio_device_lookup(rb->mr);
> > +            if (!vdev) {
> > +                qemu_log_mask(LOG_GUEST_ERROR,
> > +                              "%s: Could not find device to create dmabuf\n",
> > +                              __func__);
> > +                return;
> > +            }
> > +
> > +            vfio_create_dmabuf(vdev, res);
> > +            if (res->dmabuf_fd < 0) {
> > +                qemu_log_mask(LOG_GUEST_ERROR,
> > +                              "%s: Could not create dmabuf from vfio device\n",
> > +                              __func__);
> > +                return;
> > +            }
> > +        } else if (memory_region_is_ram(rb->mr) &&
> virtio_gpu_have_udmabuf()) {
> 
> memory_region_is_ram_device() and memory_region_is_ram() should be
> called for all iov elements, not just the first one.
I am not sure if it is enforced anywhere but I don't think a dmabuf's
segments (or entries) can refer to multiple memory regions. AFAIK,
the buffer associated with a dmabuf exists entirely within a single
memory region. And, when it needs to be migrated, it is moved 
completely. 

So, given that the goal here is to identify the region the dmabuf is
referring to, I think just using the first iov element to make this
determination is sufficient.

> 
> Calling virtio_gpu_have_udmabuf() here is redundant since
> virtio_gpu_device_realize() already calls it.
Ok, I'll remove it.

Thanks,
Vivek

> 
> > +            virtio_gpu_create_udmabuf(res);
> > +            if (res->dmabuf_fd < 0) {
> > +                qemu_log_mask(LOG_GUEST_ERROR,
> > +                              "%s: Could not create dmabuf from memfd\n",
> > +                              __func__);
> > +                return;
> > +            }
> > +        } else {
> > +            qemu_log_mask(LOG_GUEST_ERROR,
> > +                          "%s: memory region cannot be used to create dmabuf\n",
> > +                          __func__);
> > +            return;
> > +        }
> > +
> > +        virtio_gpu_remap_udmabuf(res, vdev);
> >           if (!res->remapped) {
> >               return;
> >           }
> > @@ -153,9 +284,7 @@ void virtio_gpu_init_udmabuf(struct
> virtio_gpu_simple_resource *res)
> >
> >   void virtio_gpu_fini_udmabuf(struct virtio_gpu_simple_resource *res)
> >   {
> > -    if (res->remapped) {
> > -        virtio_gpu_destroy_udmabuf(res);
> > -    }
> > +    virtio_gpu_destroy_udmabuf(res);
> >   }
> >
> >   static void virtio_gpu_free_dmabuf(VirtIOGPU *g, VGPUDMABuf *dmabuf)