Message ID | 20250205145813.394915-1-jonah.palmer@oracle.com (mailing list archive) |
---|---|
Headers | show |
Series | Handling aliased guest memory maps in vhost-vDPA SVQs | expand |
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 >
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 > > >