Jason Wang
2021-Oct-15 07:37 UTC
[RFC PATCH v4 20/20] vdpa: Add custom IOTLB translations to SVQ
On Fri, Oct 15, 2021 at 3:28 PM Eugenio Perez Martin <eperezma at redhat.com> wrote:> > On Wed, Oct 13, 2021 at 7:34 AM Jason Wang <jasowang at redhat.com> wrote: > > > > > > ? 2021/10/1 ??3:06, Eugenio P?rez ??: > > > Use translations added in VhostIOVATree in SVQ. > > > > > > 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. > > > > > > TODO: iova range should be queried before, and add logic to fail when > > > GPA is outside of its range and memory listener or svq add it. > > > > > > Signed-off-by: Eugenio P?rez<eperezma at redhat.com> > > > --- > > > hw/virtio/vhost-shadow-virtqueue.h | 4 +- > > > hw/virtio/vhost-shadow-virtqueue.c | 130 ++++++++++++++++++++++++----- > > > hw/virtio/vhost-vdpa.c | 40 ++++++++- > > > hw/virtio/trace-events | 1 + > > > 4 files changed, 152 insertions(+), 23 deletions(-) > > > > > > Think hard about the whole logic. This is safe since qemu memory map > > will fail if guest submits a invalidate IOVA. > > > > Can you expand on this? What you mean is that VirtQueue already > protects SVQ code if the guest sets an invalid buffer address (GPA), > isn't it?Yes.> > > Then I wonder if we do something much more simpler: > > > > 1) Using qemu VA as IOVA but only maps the VA that belongs to guest > > 2) Then we don't need any IOVA tree here, what we need is to just map > > vring and use qemu VA without any translation > > > > That would be great, but either qemu's SVQ vring or guest translated > buffers address (in qemu VA form) were already in high addresses, > outside of the device's iova range (in my test).You're right. I miss that and that's why we need e.g iova tree and allocator. What I proposed only makes sense when shared virtual memory (SVA) is implemented. In the case of SVA, the valid iova range should be the full VA range.> > I didn't try remapping tricks to make them fit in the range, but I > think it does complicate the solution relatively fast if there was > already memory in that range owned by qemu before enabling SVQ: > > * Guest memory must be contiguous in VA address space, but it "must" > support hotplug/unplug (although vDPA currently pins it). Hotplug > memory could always overlap with SVQ vring, so we would need to move > it. > * Duplicating mapped memory for writing? (Not sure if guest memory is > actually movable in qemu). > * Indirect descriptors will need to allocate and free memory more or > less frequently, increasing the possibility of overlapping.I'm not sure I get the problem, but overlapping is not an issue since we're using VA.> > If we can move guest memory,I'm not sure we can do this or it looks very tricky.> however, I can see how we can track it in > a tree *but* mark when the tree is 1:1 with qemu's VA, so buffers > forwarding does not take the translation penalty. When guest memory > cannot be map 1:1, we can resort to tree, and come back to 1:1 > translation if the offending tree node(s) get deleted. > > However I think this puts the solution a little bit farther than > "starting simple" :). > > Does it make sense?Yes. So I think I will review the IOVA tree codes and get back to you. THanks> > Thanks! > > > Thanks > > >