Hi Akihiko,

> Subject: Re: [PATCH v13 1/9] virtio-gpu: Find hva for Guest's DMA addr
> associated with a ram device
> 
> On 2026/10/08 5:09, Vivek Kasireddy wrote:
> > If the Guest provides a DMA address that is associated with a ram
> > device (such as a VFIO 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().
> >
> > We cannot use dma_memory_map() because for memory regions that
> do
> > not support direct access, it would create bounce buffers instead
> > of returning the actual hva, which is not desirable here. And, since
> > VFIO device regions are not considered directly accessible (because
> > they are mostly MMIO based), virtio-gpu cannot invoke
> dma_memory_map()
> > to obtain the hva in this case.
> >
> > Therefore, in order to handle addresses associated with VFIO devices,
> > we need to use the address_space_translate() API to first identify
> > the right memory region and the appropriate offset within that
> > region and then use memory_region_get_ram_ptr() to get the hva.
> > This approach also works for addresses associated with the system
> > memory region.
> >
> > Note that, although we take an explicit reference on the memory
> > region, we would still rely on dma_memory_unmap() to drop that
> > reference when the dma mapping is eventually unmapped.
> >
> > 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]>
> > Reviewed-by: Akihiko Odaki <[email protected]>
> > Signed-off-by: Vivek Kasireddy <[email protected]>
> > ---
> >   hw/display/virtio-gpu.c | 21 ++++++++++++++++++---
> >   1 file changed, 18 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> > index bb0f41bb1e..386db05e5b 100644
> > --- a/hw/display/virtio-gpu.c
> > +++ b/hw/display/virtio-gpu.c
> > @@ -948,6 +948,23 @@ 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;
> > +    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(mr)) {
> > +        memory_region_ref(mr);
> > +        return memory_region_get_ram_ptr(mr) + xlat;
> 
> On rechecking this, I don't think it is safe with Xen.
> 
> memory_region_get_ram_ptr(mr) + xlat assumes a contiguous host
> mapping
> of the entire region. Xen maps guest RAM on demand through its
> mapcache.
> Here, memory_region_get_ram_ptr() obtains a mapping at offset zero
> without pinning the cache entry, and adding xlat afterward does not
> ensure that the requested range is mapped.
> 
> The MemoryRegion reference keeps the region alive, but does not
> prevent
> the mapcache entry from being reused. Resource backing can therefore
> receive an invalid or unpinned pointer.
> 
> I think we need to extend dma_memory_map() for this use case instead
> of
> replacing it, preserving its mapping and unmapping semantics.
I am not sure if extending dma_memory_map() might be a better/cleaner
solution for this use-case. If obtaining the hva/host ptr via
memory_region_get_ram_ptr(mr) + xlat for guest RAM is not safe with
Xen, then would it not make more sense to go back to using dma_memory_map()
for Guest RAM and memory_region_get_ram_ptr(mr) + xlat for VFIO
backed regions? That is, doing the following: 

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);
}

Thanks,
Vivek

> 
> Regards,
> Akihiko Odaki
> 
> > +    }
> > +    return NULL;
> > +}
> > +
> >   int virtio_gpu_create_mapping_iov(VirtIOGPU *g,
> >                                     uint32_t nr_entries, uint32_t offset,
> >                                     struct virtio_gpu_ctrl_command *cmd,
> > @@ -993,9 +1010,7 @@ int virtio_gpu_create_mapping_iov(VirtIOGPU
> *g,
> >           /* TODO: a common DMA map SG helper */
> >           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);


Reply via email to