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

Vivek Kasireddy posted 10 patches 5 days, 16 hours ago
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, "Alex Bennée" <alex.bennee@linaro.org>, Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>, Dmitry Osipenko <dmitry.osipenko@collabora.com>, "Michael S. Tsirkin" <mst@redhat.com>, Alex Williamson <alex@shazbot.org>, "Cédric Le Goater" <clg@redhat.com>, Cornelia Huck <cohuck@redhat.com>
[PATCH v2 02/10] virtio-gpu: Find hva for Guest's DMA addr associated with a ram device
Posted by Vivek Kasireddy 5 days, 16 hours ago
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>
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/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);
+}
+
 int virtio_gpu_create_mapping_iov(VirtIOGPU *g,
                                   uint32_t nr_entries, uint32_t offset,
                                   struct virtio_gpu_ctrl_command *cmd,
@@ -839,9 +859,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


Re: [PATCH v2 02/10] virtio-gpu: Find hva for Guest's DMA addr associated with a ram device
Posted by Akihiko Odaki 3 days, 17 hours ago
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 <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>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> ---
>   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.

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.

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.

Regards,
Akihiko Odaki

> +}
> +
>   int virtio_gpu_create_mapping_iov(VirtIOGPU *g,
>                                     uint32_t nr_entries, uint32_t offset,
>                                     struct virtio_gpu_ctrl_command *cmd,
> @@ -839,9 +859,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);


RE: [PATCH v2 02/10] virtio-gpu: Find hva for Guest's DMA addr associated with a ram device
Posted by Kasireddy, Vivek 2 days, 17 hours ago
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 <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>
> > Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> > ---
> >   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?

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

Thanks,
Vivek

> 
> Regards,
> Akihiko Odaki
> 
> > +}
> > +
> >   int virtio_gpu_create_mapping_iov(VirtIOGPU *g,
> >                                     uint32_t nr_entries, uint32_t offset,
> >                                     struct virtio_gpu_ctrl_command
> > *cmd, @@ -839,9 +859,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);
Re: [PATCH v2 02/10] virtio-gpu: Find hva for Guest's DMA addr associated with a ram device
Posted by Akihiko Odaki 2 days, 14 hours ago
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 <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>
>>> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
>>> ---
>>>    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

Re: [PATCH v2 02/10] virtio-gpu: Find hva for Guest's DMA addr associated with a ram device
Posted by Akihiko Odaki 4 days, 17 hours ago
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 <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>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>

Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>

> ---
>   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);
> +}
> +
>   int virtio_gpu_create_mapping_iov(VirtIOGPU *g,
>                                     uint32_t nr_entries, uint32_t offset,
>                                     struct virtio_gpu_ctrl_command *cmd,
> @@ -839,9 +859,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);