On 2025/11/12 13:30, Kasireddy, Vivek wrote:
Hi Akihiko,

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

On 2025/11/09 14:33, Vivek Kasireddy wrote:
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 <[email protected]>
Cc: Alex Bennée <[email protected]>
Cc: Akihiko Odaki <[email protected]>
Cc: Dmitry Osipenko <[email protected]>
Cc: Alex Williamson <[email protected]>
Cc: Cédric Le Goater <[email protected]>
Signed-off-by: Vivek Kasireddy <[email protected]>
---
   hw/display/virtio-gpu.c | 24 +++++++++++++++++++++---
   1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c index
199b18c746..d352b5afd6 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -798,6 +798,26 @@ 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;
+
+    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);

This function should:
- call memory_region_get_ram_ptr(mr)
    if memory_region_is_ram(mr)
- return NULL otherwise

There are a few reasons. First, the documentation of dma_memory_map()
tells to use it "only for reads OR writes - not for read-modify-write
operations." It can be used for read-modify-write operations so
dma_memory_map() should be avoided.
This patch series only deals with non-virgl use-cases where AFAICS resources
are not written to on the Host.


Second, it ensures that the mapped pointer is writable.
"[PATCH v2 10/10] virtio-gpu-dmabuf: Create dmabuf for blobs associated
with VFIO devices" adds checks for memory_region_is_ram() and
memory_region_is_ram_device() to virtio_gpu_init_dmabuf(), but the other
callers also use the function to map writable pointers.
Unless I am missing something, I don't see where writable pointers are used
in non-virgl use-cases?

Rutabaga uses too, but you are right about that 2D operations won't use it.

That said, exposing non-writable memory to Virgl and Rutabaga lets the guest corrupt memory so should be avoided. On the other hand, it is unlikely that rejecting non-writable memory will cause any problem. You can also add another code path to use memory_region_supports_direct_access() instead of memory_region_is_ram() for virtio-gpu for 2D and avoid calling memory_region_is_ram() in virtio_gpu_init_dmabuf() if you want to keep non-writable memory working.



It also makes the check of memory_region_is_ram_device() and
memory_region_is_ram() unnecessary for virtio_gpu_init_dmabuf(), reducing
the overall complexity.
Since buffers reside completely in either ram or ram_device regions, using both
memory_region_is_ram_device() and memory_region_is_ram() to check where
they are located seems necessary and unavoidable.

It can unconditionally call virtio_gpu_create_udmabuf(), and if the function finds the memory is incompatible with udmabuf, it can call vfio_device_lookup() to tell if the memory belongs to VFIO or not.

Regards,
Akihiko Odaki

Reply via email to