On 2025/11/13 12:39, 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/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.
AFAICS, virtio_gpu_init_dmabuf() is only called in non-virgl/non-rutabaga code.
And, this patch series and my use-case (GPU SRIOV) only needs to deal with
non-writeable memory because the rendering is already done by the Guest and
the Host only needs to display the Guest's FB.
However, I see that virtio_gpu_create_mapping_iov() is used by virgl/rutabaga
code as well, so I am wondering how do things work right now given that
virtio_gpu_create_mapping_iov() always calls dma_memory_map()?
In other words, is there no problem currently with non-writeable memory
in virgl/rutabaga use-cases?
The current code is problematic, and using memory_region_is_ram() will
fix it.
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.
Yeah, what you suggest is doable but seems a bit convoluted to have to
first call virtio_gpu_create_udmabuf() and if it fails then call VFIO related
functions.
I think using memory_region_is_ram_device() and memory_region_is_ram()
to identify the right memory region and calling either
virtio_gpu_create_udmabuf()
or vfio_create_dmabuf() is much more intuitive and readable.
memory_region_is_ram_device() and memory_region_is_ram() are not
sufficient to identify the right memory region.
memory_region_is_ram_device() returns true for RAM device created by
non-VFIO devices, and memory_region_is_ram() returns true for memory
regions created with memory_region_init_ram_ptr(), which is not backed
with memfd.
In general, checking prerequisite conditions before performing
operations implemented elsewhere is dangerous because the checks and the
operation implementation can be diverged, and these facts demonstrate that.
Regards,
Akihiko Odaki
