Hi Akihiko, > Subject: Re: [RFC 6/6] virtio-gpu: Find the host addr given gpa associated > with a ram device > > > 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. Thank you for your suggestion. Looks like address_space_translate() followed by memory_region_get_ram_ptr() also makes it work and is much cleaner. I'll include this change in the next version.
> > 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. Sure, I'll improve the commit message to explain why dma_memory_map() does not work in this case. > > While I added comments for each patch, the overall implementation > direction of this series looks good to me. Thank you for taking a look! Thanks, Vivek > > Regards, > Akihiko Odaki > > > > > Cc: Marc-André Lureau <[email protected]> > > Cc: Alex Bennée <[email protected]> > > Cc: Akihiko Odaki <[email protected]> > > Cc: Dmitry Osipenko <[email protected]> > > Signed-off-by: Vivek Kasireddy <[email protected]> > > --- > > 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);
