On 2025/09/03 7:42, Vivek Kasireddy wrote:
> 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.
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.
While I added comments for each patch, the overall implementation
direction of this series looks good to me.
Regards,
Akihiko Odaki
>
> 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>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> ---
> 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);