Re: [PATCH 0/4] Handling aliased guest memory maps in vhost-vDPA SVQs

[Lei Yang]

I tested this series patches with vdpa's regression tests, everything
works fine.

Tested-by: Lei Yang <leiyang@redhat.com>

On Wed, Feb 12, 2025 at 12:20 AM Eugenio Perez Martin
<eperezma@redhat.com> wrote:
>
> On Wed, Feb 5, 2025 at 3:58 PM Jonah Palmer <jonah.palmer@oracle.com> wrote:
> >
> > An issue arises from aliased memory mappings in the guest, where
> > different GPAs map to the same HVA. For example:
> >
> >  - GPA1->HVA1
> >  - GPA2->HVA1
> >
> > When these mappings exist in the IOVA->HVA tree, a lookup by an HVA
> > backed by guest memory results in ambiguities because the same HVA could
> > correspond to multiple GPAs. This duplication causes issues in managing
> > IOVA trees for SVQs in vDPA, leading to incorrect behavior.
> >
> > For example, consider these mapped guest memory regions:
> >
> >               HVA                            GPA                         
> > IOVA
> > -------------------------------  --------------------------- 
> > ----------------------------
> > [0x7f7903e00000, 0x7f7983e00000) [0x0, 0x80000000)           [0x1000, 
> > 0x80000000)
> > [0x7f7983e00000, 0x7f9903e00000) [0x100000000, 0x2080000000) [0x80001000, 
> > 0x2000001000)
> > [0x7f7903ea0000, 0x7f7903ec0000) [0xfeda0000, 0xfedc0000)    [0x2000001000, 
> > 0x2000021000)
> >
> > The last HVA range [0x7f7903ea0000, 0x7f7903ec0000) is contained within
> > the first HVA range [0x7f7903e00000, 0x7f7983e00000). Despite this, the
> > GPA ranges for the first and third mappings do not overlap, so the guest
> > sees them as distinct physical memory regions.
> >
> > Now consider an operation to unmap the mapping associated with HVA
> > 0x7f7903eb0000. This HVA fits into both the first and third HVA ranges.
> >
> > When searching the HVA->IOVA tree, the search stops at the first
> > matching HVA range [0x7f7903e00000, 0x7f7983e00000) due to the behavior
> > of the red-black tree (GTree). However, the correct mapping to remove is
> > the third mappings, as the HVA translate to GPA 0xfedb0000, which only
> > fits in the third mapping's GPA range.
> >
> > To address this, we implement a GPA->IOVA tree and use the GPAs of
> > descriptors backed by guest memory when translating to IOVA.
> >
> > When a descriptor's GPA is used for translation, the GPA->IOVA tree
> > ensures that each GPA maps to exactly one IOVA, regardless of any
> > overlapping HVAs. This guarantees that operations such as unmapping or
> > accessing a descriptor correctly targets the intended memory region in
> > the guest's address space.
> >
> > For descriptors not backed by guest memory, the existing IOVA->HVA tree
> > is still used.
> >
> > GPAs are unique and non-overlapping within the guest's address space. By
> > translating GPAs to IOVAs, the ambiguity caused by multiple GPAs mapping
> > to the same HVA is avoided.
> >
>
> I'd squash patches 2 and 3. While it adds some clarification, I think
> it is not worth adding the code to remove it.
>
> Either way,
>
> Acked-by: Eugenio Pérez <eperezma@redhat.com>
>
> Thanks!
>
> > --------
> > This series is a different approach of [1] and is based off of [2],
> > where this issue was originally discovered.
> >
> > Patch v1:
> > ---------
> >  * Created separate alloc functions for guest-backed and host-only
> >    memory.
> >  * Alloc functions also insert mappings to their respective trees.
> >  * Make patches self-contained and functional to prevent breakage during
> >    bisection.
> >  * Don't move range checks from alloc functions.
> >  * Use existing VirtQueueElement members 'in_addr' & 'out_addr' instead
> >    of creating custom 'in_xlat_addr' & 'out_xlat_addr' members.
> >  * Move documentation changes to separate patch.
> >
> > RFC v3:
> > -------
> >  * Decouple the IOVA allocator to support a SVQ IOVA->HVA tree for
> >    host-only mappings.
> >  * Move range check for IOVA allocator to its own patch.
> >  * Implement a GPA->IOVA tree alongside the SVQ IOVA->HVA & IOVA-only
> >    trees.
> >  * Add in_xlat_addr & out_xlat_addr VirtQueueElement members to hold the
> >    GPAs of an element's input/output descriptors (to be used during
> >    translation).
> >
> > RFC v2:
> > -------
> >  * Don't decouple IOVA allocator.
> >  * Build a IOVA->GPA tree (instead of GPA->IOVA tree).
> >  * Remove IOVA-only tree and keep the full IOVA->HVA tree.
> >  * Only search through RAMBlocks when we know the HVA is backed by
> >    guest memory.
> >  * Slight rewording of function names.
> >
> > RFC v1:
> > -------
> >  * Alternative approach of [1].
> >  * First attempt to address this issue found in [2].
> >
> > [1] 
> > https://lore.kernel.org/qemu-devel/20240410100345.389462-1-eperezma@redhat.com
> > [2] 
> > https://lore.kernel.org/qemu-devel/20240201180924.487579-1-eperezma@redhat.com
> >
> > Jonah Palmer (4):
> >   vhost-iova-tree: Implement an IOVA-only tree
> >   vhost-iova-tree: Implement GPA->IOVA & partial IOVA->HVA trees
> >   svq: Support translations via GPAs in vhost_svq_translate_addr
> >   vhost-iova-tree: Update documentation
> >
> >  hw/virtio/vhost-iova-tree.c        | 115 ++++++++++++++++++++++++-----
> >  hw/virtio/vhost-iova-tree.h        |   8 +-
> >  hw/virtio/vhost-shadow-virtqueue.c |  55 +++++++++-----
> >  hw/virtio/vhost-shadow-virtqueue.h |   5 +-
> >  hw/virtio/vhost-vdpa.c             |  40 ++++++----
> >  include/qemu/iova-tree.h           |  22 ++++++
> >  net/vhost-vdpa.c                   |  12 ++-
> >  util/iova-tree.c                   |  46 ++++++++++++
> >  8 files changed, 248 insertions(+), 55 deletions(-)
> >
> > --
> > 2.43.5
> >
>