Re: [RFC v3 4/5] virtio: add in_xlat_addr & out_xlat_addr VirtQueueElement members
On 1/16/2025 11:02 AM, Eugenio Perez Martin wrote:
On Fri, Jan 10, 2025 at 6:09 PM Jonah Palmer wrote:
Adds the in_xlat_addr & out_xlat_addr hwaddr arrays to the
VirtQueueElement struct and introduces an optional GPA output parameter
to dma_memory_map().
These arrays will store a VirtQueueElement's input/output descriptors'
GPA of the mapped memory region, if it's backed by guest memory, via
dma_memory_map().
The GPA will always correspond 1:1 to the iovec entry when translating
addresses between Qemu VAs and SVQ IOVAs in vhost_svq_translate_addr().
This helps to avoid extra complicated code in SVQ's
vhost_svq_vring_write_descs() function (e.g. splitting up iovec into
multiple buffers, not breaking devices using aliased mapping, etc.).
Since the translation is only done once inside the DMA API alongside
virtqueue_pop(), the cost should be minimal.
I think this is a very strong change as it touches the dma subsystem.
Let me try to avoid it on 5/5 :).
This change was what I suggested to Jonah actually, the intent for the
DMA API change was to lessen the friction of vIOMMU support for Shadow
VQ (for which case elem->in_addr and elem->out_addr is GIOVA rather than
GPA). This will be the most efficient and performant way to get the
needed translation done only once without having to maintain separate
GPA mapping & translation out of the memory subsystem. I wonder would it
work to create an variant of dma_memory_map() API while leaving the
current one as is: the new API variant would return a cookie that is
used to represent the address translation, such that SVQ subsystem can
use it later on whenever needed in order to fetch the GIOVA to GPA
translation in place (the translated GPA address may be cached in the
opaque cookie) from the memory system?
Regards,
-Siwei
Signed-off-by: Jonah Palmer
---
hw/display/virtio-gpu.c | 5 ++--
hw/hyperv/vmbus.c | 8 +++---
hw/ide/ahci.c | 7 +++---
hw/usb/libhw.c | 2 +-
hw/virtio/virtio.c | 50 ++---
include/hw/pci/pci_device.h | 2 +-
include/hw/virtio/virtio.h | 2 ++
include/system/dma.h| 25 ++-
system/dma-helpers.c| 2 +-
9 files changed, 77 insertions(+), 26 deletions(-)
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 11a7a85750..afb9a8b69f 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -839,7 +839,7 @@ int virtio_gpu_create_mapping_iov(VirtIOGPU *g,
len = l;
map = dma_memory_map(VIRTIO_DEVICE(g)->dma_as, a, &len,
DMA_DIRECTION_TO_DEVICE,
- MEMTXATTRS_UNSPECIFIED);
+ MEMTXATTRS_UNSPECIFIED, NULL);
if (!map) {
qemu_log_mask(LOG_GUEST_ERROR, "%s: failed to map MMIO memory
for"
" element %d\n", __func__, e);
@@ -1258,7 +1258,8 @@ static bool virtio_gpu_load_restore_mapping(VirtIOGPU *g,
hwaddr len = res->iov[i].iov_len;
res->iov[i].iov_base =
dma_memory_map(VIRTIO_DEVICE(g)->dma_as, res->addrs[i], &len,
- DMA_DIRECTION_TO_DEVICE, MEMTXATTRS_UNSPECIFIED);
+ DMA_DIRECTION_TO_DEVICE, MEMTXATTRS_UNSPECIFIED,
+ NULL);
if (!res->iov[i].iov_base || len != res->iov[i].iov_len) {
/* Clean up the half-a-mapping we just created... */
diff --git a/hw/hyperv/vmbus.c b/hw/hyperv/vmbus.c
index 12a7dc4312..c3308a1bfd 100644
--- a/hw/hyperv/vmbus.c
+++ b/hw/hyperv/vmbus.c
@@ -374,7 +374,7 @@ static ssize_t gpadl_iter_io(GpadlIter *iter, void *buf,
uint32_t len)
maddr = (iter->gpadl->gfns[idx] << TARGET_PAGE_BITS) |
off_in_page;
iter->map = dma_memory_map(iter->as, maddr, &mlen, iter->dir,
- MEMTXATTRS_UNSPECIFIED);
+ MEMTXATTRS_UNSPECIFIED, NULL);
if (mlen != pgleft) {
dma_memory_unmap(iter->as, iter->map, mlen, iter->dir, 0);
iter->map = NULL;
@@ -492,7 +492,8 @@ int vmbus_map_sgl(VMBusChanReq *req, DMADirection dir,
struct iovec *iov,
}
iov[ret_cnt].iov_base = dma_memory_map(sgl->as, a, &l, dir,
- MEMTXATTRS_UNSPECIFIED);
+ MEMTXATTRS_UNSPECIFIED,
+ NULL);
if (!l) {
ret = -EFAULT;
goto err;
@@ -568,7 +569,8 @@ static vmbus_ring_buffer
*ringbuf_map_hdr(VMBusRingBufCommon *ringbuf)
dma_addr_t mlen = sizeof(*rb);
rb = dma_memory_map(ringbuf->as, ringbuf->rb_addr, &mlen,
-DMA_DIRECTION_FROM_DEVICE, MEMTXATTRS_UNSPECIFIED);
+
Re: [RFC v3 4/5] virtio: add in_xlat_addr & out_xlat_addr VirtQueueElement members
On Fri, Jan 10, 2025 at 6:09 PM Jonah Palmer wrote:
>
> Adds the in_xlat_addr & out_xlat_addr hwaddr arrays to the
> VirtQueueElement struct and introduces an optional GPA output parameter
> to dma_memory_map().
>
> These arrays will store a VirtQueueElement's input/output descriptors'
> GPA of the mapped memory region, if it's backed by guest memory, via
> dma_memory_map().
>
> The GPA will always correspond 1:1 to the iovec entry when translating
> addresses between Qemu VAs and SVQ IOVAs in vhost_svq_translate_addr().
> This helps to avoid extra complicated code in SVQ's
> vhost_svq_vring_write_descs() function (e.g. splitting up iovec into
> multiple buffers, not breaking devices using aliased mapping, etc.).
>
> Since the translation is only done once inside the DMA API alongside
> virtqueue_pop(), the cost should be minimal.
>
I think this is a very strong change as it touches the dma subsystem.
Let me try to avoid it on 5/5 :).
> Signed-off-by: Jonah Palmer
> ---
> hw/display/virtio-gpu.c | 5 ++--
> hw/hyperv/vmbus.c | 8 +++---
> hw/ide/ahci.c | 7 +++---
> hw/usb/libhw.c | 2 +-
> hw/virtio/virtio.c | 50 ++---
> include/hw/pci/pci_device.h | 2 +-
> include/hw/virtio/virtio.h | 2 ++
> include/system/dma.h| 25 ++-
> system/dma-helpers.c| 2 +-
> 9 files changed, 77 insertions(+), 26 deletions(-)
>
> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> index 11a7a85750..afb9a8b69f 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -839,7 +839,7 @@ int virtio_gpu_create_mapping_iov(VirtIOGPU *g,
> len = l;
> map = dma_memory_map(VIRTIO_DEVICE(g)->dma_as, a, &len,
> DMA_DIRECTION_TO_DEVICE,
> - MEMTXATTRS_UNSPECIFIED);
> + MEMTXATTRS_UNSPECIFIED, NULL);
> if (!map) {
> qemu_log_mask(LOG_GUEST_ERROR, "%s: failed to map MMIO
> memory for"
>" element %d\n", __func__, e);
> @@ -1258,7 +1258,8 @@ static bool virtio_gpu_load_restore_mapping(VirtIOGPU
> *g,
> hwaddr len = res->iov[i].iov_len;
> res->iov[i].iov_base =
> dma_memory_map(VIRTIO_DEVICE(g)->dma_as, res->addrs[i], &len,
> - DMA_DIRECTION_TO_DEVICE, MEMTXATTRS_UNSPECIFIED);
> + DMA_DIRECTION_TO_DEVICE, MEMTXATTRS_UNSPECIFIED,
> + NULL);
>
> if (!res->iov[i].iov_base || len != res->iov[i].iov_len) {
> /* Clean up the half-a-mapping we just created... */
> diff --git a/hw/hyperv/vmbus.c b/hw/hyperv/vmbus.c
> index 12a7dc4312..c3308a1bfd 100644
> --- a/hw/hyperv/vmbus.c
> +++ b/hw/hyperv/vmbus.c
> @@ -374,7 +374,7 @@ static ssize_t gpadl_iter_io(GpadlIter *iter, void *buf,
> uint32_t len)
> maddr = (iter->gpadl->gfns[idx] << TARGET_PAGE_BITS) |
> off_in_page;
>
> iter->map = dma_memory_map(iter->as, maddr, &mlen, iter->dir,
> - MEMTXATTRS_UNSPECIFIED);
> + MEMTXATTRS_UNSPECIFIED, NULL);
> if (mlen != pgleft) {
> dma_memory_unmap(iter->as, iter->map, mlen, iter->dir, 0);
> iter->map = NULL;
> @@ -492,7 +492,8 @@ int vmbus_map_sgl(VMBusChanReq *req, DMADirection dir,
> struct iovec *iov,
> }
>
> iov[ret_cnt].iov_base = dma_memory_map(sgl->as, a, &l, dir,
> - MEMTXATTRS_UNSPECIFIED);
> + MEMTXATTRS_UNSPECIFIED,
> + NULL);
> if (!l) {
> ret = -EFAULT;
> goto err;
> @@ -568,7 +569,8 @@ static vmbus_ring_buffer
> *ringbuf_map_hdr(VMBusRingBufCommon *ringbuf)
> dma_addr_t mlen = sizeof(*rb);
>
> rb = dma_memory_map(ringbuf->as, ringbuf->rb_addr, &mlen,
> -DMA_DIRECTION_FROM_DEVICE, MEMTXATTRS_UNSPECIFIED);
> +DMA_DIRECTION_FROM_DEVICE, MEMTXATTRS_UNSPECIFIED,
> +NULL);
> if (mlen != sizeof(*rb)) {
> dma_memory_unmap(ringbuf->as, rb, mlen,
> DMA_DIRECTION_FROM_DEVICE, 0);
> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
> index 1303c21cb7..aeea2dc61d 100644
> --- a/hw/ide/ahci.c
> +++ b/hw/ide/ahci.c
> @@ -221,7 +221,7 @@ static void map_page(AddressSpace *as, uint8_t **ptr,
> uint64_t addr,
> }
>
> *ptr = dma_memory_map(as, addr, &len, DMA_DIRECTION_FROM_DEVICE,
> - MEMTXATTRS_UNSPECIFIED);
> + MEMTXATTRS_UNSPECIFIED, NULL);
> if (len < wanted && *ptr) {
> dma_memory_unmap(as, *ptr, len, DMA_DIRECTION_FROM_DEVICE, len);
>
