Re: [RFC 6/6] virtio-gpu: Find the host addr given gpa associated with a ram device

[Akihiko Odaki]

On 2025/09/03 7:42, Vivek Kasireddy wrote:
If the Guest provides a gpa (guest physical address) associated with
a PCI region, then we can obtain the hva (host virtual address) via
gpa2hva() API instead of dma_memory_map(). Note that we would still
call dma_memory_unmap() (to unref mr) regardless of how we obtained
the hva.

I think address_space_translate() should be used instead. The guest 
passes addresses that are valid in the DMA address space, which may not 
be a GPA if an IOMMU is in effect. address_space_translate() allows you 
specifying the DMA address space.

The motivation for this change should also be described in the patch 
message; otherwise a reader may wonder why dma_memory_map() does not 
suffice.

While I added comments for each patch, the overall implementation 
direction of this series looks good to me.

Regards,
Akihiko Odaki


Cc: Marc-André Lureau <marcandre.lur...@redhat.com>
Cc: Alex Bennée <alex.ben...@linaro.org>
Cc: Akihiko Odaki <od...@rsg.ci.i.u-tokyo.ac.jp>
Cc: Dmitry Osipenko <dmitry.osipe...@collabora.com>
Signed-off-by: Vivek Kasireddy <vivek.kasire...@intel.com>
---
  hw/display/virtio-gpu.c | 35 ++++++++++++++++++++++++++++++++---
  1 file changed, 32 insertions(+), 3 deletions(-)

diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 1654a417b8..4af8390bb5 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -32,6 +32,7 @@
  #include "qemu/module.h"
  #include "qapi/error.h"
  #include "qemu/error-report.h"
+#include "monitor/monitor.h"
  
  #define VIRTIO_GPU_VM_VERSION 1
  
@@ -799,6 +800,36 @@ static void virtio_gpu_set_scanout_blob(VirtIOGPU *g,
                                &fb, res, &ss.r, &cmd->error);
  }
  
+static void *map_gpa2hva(VirtIOGPU *g,
+                         struct virtio_gpu_ctrl_command *cmd,
+                         uint64_t gpa, hwaddr *len)
+{
+    MemoryRegion *mr = NULL;
+    Error *errp = NULL;
+    void *map;
+
+    if (cmd->cmd_hdr.type != VIRTIO_GPU_CMD_RESOURCE_CREATE_BLOB) {
+        return dma_memory_map(VIRTIO_DEVICE(g)->dma_as, gpa, len,
+                              DMA_DIRECTION_TO_DEVICE,
+                              MEMTXATTRS_UNSPECIFIED);
+    }
+
+    map = gpa2hva(&mr, gpa, 1, &errp);
+    if (errp) {
+        error_report_err(errp);
+        return NULL;
+    }
+
+    if (!memory_region_is_ram_device(mr)) {
+        memory_region_unref(mr);
+        map = dma_memory_map(VIRTIO_DEVICE(g)->dma_as, gpa, len,
+                             DMA_DIRECTION_TO_DEVICE,
+                             MEMTXATTRS_UNSPECIFIED);
+    }
+
+    return map;
+}
+
  int virtio_gpu_create_mapping_iov(VirtIOGPU *g,
                                    uint32_t nr_entries, uint32_t offset,
                                    struct virtio_gpu_ctrl_command *cmd,
@@ -840,9 +871,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 = map_gpa2hva(g, cmd, a, &len);
              if (!map) {
                  qemu_log_mask(LOG_GUEST_ERROR, "%s: failed to map MMIO memory 
for"
                                " element %d\n", __func__, e);