|
From: | Jason Wang |
Subject: | Re: [RFC v3 25/29] vhost: Add custom IOTLB translations to SVQ |
Date: | Thu, 3 Jun 2021 11:39:42 +0800 |
User-agent: | Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Thunderbird/78.10.2 |
在 2021/6/3 上午1:51, Eugenio Perez Martin 写道:
On Wed, Jun 2, 2021 at 11:52 AM Jason Wang <jasowang@redhat.com> wrote:在 2021/5/20 上午12:28, Eugenio Pérez 写道:Use translations added in IOVAReverseMaps in SVQ if the vhost device does not support the mapping of the full qemu's virtual address space. In other cases, Shadow Virtqueue still uses the qemu's virtual address of the buffer pointed by the descriptor, which has been translated already by qemu's VirtQueue machinery.I'd say let stick to a single kind of translation (iova allocator) that works for all the cases first and add optimizations on top.Ok, I will start from here for the next revision.Now every element needs to store the previous address also, so VirtQueue can consume the elements properly. This adds a little overhead per VQ element, having to allocate more memory to stash them. As a possible optimization, this allocation could be avoided if the descriptor is not a chain but a single one, but this is left undone. Checking also for vhost_set_iotlb_callback to send used ring remapping. This is only needed for kernel, and would print an error in case of vhost devices with its own mapping (vdpa). This could change for other callback, like checking for vhost_force_iommu, enable_custom_iommu, or another. Another option could be to, at least, extract the check of "is map(used, writable) needed?" in another function. But at the moment just copy the check used in vhost_dev_start here. Signed-off-by: Eugenio Pérez <eperezma@redhat.com> --- hw/virtio/vhost-shadow-virtqueue.c | 134 ++++++++++++++++++++++++++--- hw/virtio/vhost.c | 29 +++++-- 2 files changed, 145 insertions(+), 18 deletions(-) diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c index 934d3bb27b..a92da979d1 100644 --- a/hw/virtio/vhost-shadow-virtqueue.c +++ b/hw/virtio/vhost-shadow-virtqueue.c @@ -10,12 +10,19 @@ #include "hw/virtio/vhost-shadow-virtqueue.h" #include "hw/virtio/vhost.h" #include "hw/virtio/virtio-access.h" +#include "hw/virtio/vhost-iova-tree.h" #include "standard-headers/linux/vhost_types.h" #include "qemu/error-report.h" #include "qemu/main-loop.h" +typedef struct SVQElement { + VirtQueueElement elem; + void **in_sg_stash; + void **out_sg_stash;Any reason for the trick like this? Can we simply use iovec and iov_copy() here?At the moment the device writes the buffer directly to the guest's memory, and SVQ only translates the descriptor. In this scenario, there would be no need for iov_copy, isn't it?
It depends on which kinds of translation you used.If I read the code correctly, stash is used for storing HVAs after the HVA->IOVA translation.
This looks exactly the work of iov (and do we guarantee the there will be a 1:1 translation?)
And if the mapping is 1:1 you can simply use iov_copy(). But this wont' be a option if we will always use iova allocator.
The reason for stash and unstash them was to allow the 1:1 mapping with qemu memory and IOMMU and iova allocator to work with less changes, In particular, the reason for unstash is that virtqueue_fill, expects qemu pointers to set the guest memory page as dirty in virtqueue_unmap_sg->dma_memory_unmap. Now I think that just storing the iova address from the allocator in a separated field and using a wrapper to get the IOVA addresses in SVQ would be a better idea, so I would change to this if everyone agrees.
I agree. Thanks
Thanks!Thanks
[Prev in Thread] | Current Thread | [Next in Thread] |