[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH v2 3/4] x86/HVM: defer IOTLB flushes across a grant unmap batch



gnttab_unmap_grant_ref() flushes the TLBs of a PV caller once per group
of GNTTAB_UNMAP_BATCH_SIZE unmaps. A translated caller instead gets an
IOTLB flush per page from the p2m code, so netback in a PVH dom0 issues
one for every page a domU transmits.

Open a batch for translated callers before each group, and end it next
to gnttab_flush_tlb(), so before unmap_common_complete() releases the
pages. Ending the batch issues one flush covering the flushes the
unmaps skipped, as iommu_skipped_flush recorded them.

The batch lives in the vCPU, and iommu_dont_flush_iotlb isn't held
across the guest copies between the unmaps, as those can sleep on a
vm_event ring and change the p2m, e.g. by unsharing a page. If the
flush fails, the pages may still be reachable through the IOMMU, so
the group isn't completed and its pages leak.

PV callers, Arm, and other callers of replace_grant_host_mapping() are
unchanged.

Assisted-by: Claude Code:claude-opus-5-5
Signed-off-by: Luca Di Maio <luca@xxxxxxxxx>

---
Changes in v2:
- Flush the range recorded in iommu_skipped_flush, rather than one
  derived from the unmapped gfns (Teddy).
- Leave rounding the range to an aligned power of two to VT-d, in
  patch 1 (Teddy).
---
 xen/arch/arm/include/asm/grant_table.h     |  3 ++
 xen/arch/x86/hvm/grant_table.c             | 49 +++++++++++++++++++++-
 xen/arch/x86/include/asm/grant_table.h     | 13 ++++++
 xen/arch/x86/include/asm/hvm/grant_table.h |  5 +++
 xen/arch/x86/include/asm/hvm/vcpu.h        |  9 ++++
 xen/common/grant_table.c                   |  6 +++
 6 files changed, 84 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/include/asm/grant_table.h 
b/xen/arch/arm/include/asm/grant_table.h
index 7661a95341..66f90083db 100644
--- a/xen/arch/arm/include/asm/grant_table.h
+++ b/xen/arch/arm/include/asm/grant_table.h
@@ -40,6 +40,9 @@ int create_grant_host_mapping(uint64_t gpaddr, mfn_t frame,
 int replace_grant_host_mapping(uint64_t gpaddr, mfn_t frame,
                                uint64_t new_gpaddr, unsigned int flags);
 
+static inline void gnttab_unmap_batch_begin(void) {}
+static inline int gnttab_unmap_batch_end(void) { return 0; }
+
 /*
  * The region used by Xen on the memory will never be mapped in DOM0
  * memory layout. Therefore it can be used for the grant table.
diff --git a/xen/arch/x86/hvm/grant_table.c b/xen/arch/x86/hvm/grant_table.c
index afe449d888..5ecba6db65 100644
--- a/xen/arch/x86/hvm/grant_table.c
+++ b/xen/arch/x86/hvm/grant_table.c
@@ -41,6 +41,8 @@ int replace_grant_p2m_mapping(uint64_t addr, mfn_t frame,
     p2m_type_t type;
     mfn_t old_mfn;
     struct domain *d = current->domain;
+    struct hvm_grant_unmap_batch *batch = &current->arch.hvm.grant_unmap_batch;
+    int rc;
 
     if ( new_addr != 0 || (flags & GNTMAP_contains_pte) )
         return GNTST_general_error;
@@ -54,7 +56,29 @@ int replace_grant_p2m_mapping(uint64_t addr, mfn_t frame,
                  type, mfn_x(old_mfn), mfn_x(frame));
         return GNTST_general_error;
     }
-    if ( p2m_remove_page(d, _gfn(gfn), frame, PAGE_ORDER_4K) )
+
+    /* Not across the guest copies, which can change the p2m or sleep. */
+    if ( batch->open )
+    {
+        this_cpu(iommu_skipped_flush) =
+            (struct iommu_skipped_flush){ .first = INVALID_DFN };
+        this_cpu(iommu_dont_flush_iotlb) = true;
+    }
+
+    rc = p2m_remove_page(d, _gfn(gfn), frame, PAGE_ORDER_4K);
+
+    if ( batch->open )
+    {
+        const struct iommu_skipped_flush *skipped =
+            &this_cpu(iommu_skipped_flush);
+
+        this_cpu(iommu_dont_flush_iotlb) = false;
+        batch->iommu_flags |= skipped->flags;
+        batch->dfn_first = min(batch->dfn_first, dfn_x(skipped->first));
+        batch->dfn_last = max(batch->dfn_last, dfn_x(skipped->last));
+    }
+
+    if ( rc )
     {
         put_gfn(d, gfn);
         return GNTST_general_error;
@@ -64,6 +88,29 @@ int replace_grant_p2m_mapping(uint64_t addr, mfn_t frame,
     return GNTST_okay;
 }
 
+void grant_p2m_unmap_batch_begin(void)
+{
+    struct hvm_grant_unmap_batch *batch = &current->arch.hvm.grant_unmap_batch;
+
+    ASSERT(!batch->open);
+    *batch = (struct hvm_grant_unmap_batch){ .open = true, .dfn_first = ~0UL };
+}
+
+int grant_p2m_unmap_batch_end(void)
+{
+    struct domain *d = current->domain;
+    struct hvm_grant_unmap_batch *batch = &current->arch.hvm.grant_unmap_batch;
+
+    batch->open = false;
+
+    if ( !batch->iommu_flags )
+        return 0;
+
+    return iommu_iotlb_flush(d, _dfn(batch->dfn_first),
+                             batch->dfn_last - batch->dfn_first + 1,
+                             batch->iommu_flags);
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/x86/include/asm/grant_table.h 
b/xen/arch/x86/include/asm/grant_table.h
index e939c9bacb..3aae47092f 100644
--- a/xen/arch/x86/include/asm/grant_table.h
+++ b/xen/arch/x86/include/asm/grant_table.h
@@ -32,6 +32,19 @@ static inline int replace_grant_host_mapping(uint64_t addr, 
mfn_t frame,
     return replace_grant_pv_mapping(addr, frame, new_addr, flags);
 }
 
+static inline void gnttab_unmap_batch_begin(void)
+{
+    if ( paging_mode_external(current->domain) )
+        grant_p2m_unmap_batch_begin();
+}
+
+static inline int __must_check gnttab_unmap_batch_end(void)
+{
+    if ( paging_mode_external(current->domain) )
+        return grant_p2m_unmap_batch_end();
+    return 0;
+}
+
 #define gnttab_get_frame_gfn(gt, st, idx) ({                             \
     mfn_t mfn_ = (st) ? gnttab_status_mfn(gt, idx)                       \
                       : gnttab_shared_mfn(gt, idx);                      \
diff --git a/xen/arch/x86/include/asm/hvm/grant_table.h 
b/xen/arch/x86/include/asm/hvm/grant_table.h
index 01e23f79b8..9877262f51 100644
--- a/xen/arch/x86/include/asm/hvm/grant_table.h
+++ b/xen/arch/x86/include/asm/hvm/grant_table.h
@@ -19,6 +19,8 @@ int create_grant_p2m_mapping(uint64_t addr, mfn_t frame,
                              unsigned int cache_flags);
 int replace_grant_p2m_mapping(uint64_t addr, mfn_t frame,
                               uint64_t new_addr, unsigned int flags);
+void grant_p2m_unmap_batch_begin(void);
+int grant_p2m_unmap_batch_end(void);
 
 #else
 
@@ -37,6 +39,9 @@ static inline int replace_grant_p2m_mapping(uint64_t addr, 
mfn_t frame,
     return GNTST_general_error;
 }
 
+static inline void grant_p2m_unmap_batch_begin(void) {}
+static inline int grant_p2m_unmap_batch_end(void) { return 0; }
+
 #endif
 
 #endif /* __X86_HVM_GRANT_TABLE_H__ */
diff --git a/xen/arch/x86/include/asm/hvm/vcpu.h 
b/xen/arch/x86/include/asm/hvm/vcpu.h
index 876c230784..990411e2c7 100644
--- a/xen/arch/x86/include/asm/hvm/vcpu.h
+++ b/xen/arch/x86/include/asm/hvm/vcpu.h
@@ -108,6 +108,13 @@ struct altp2mvcpu {
 
 #define vcpu_altp2m(v) ((v)->arch.hvm.avcpu)
 
+/* Per vCPU, as the guest copies between the unmaps can sleep. */
+struct hvm_grant_unmap_batch {
+    bool          open;
+    unsigned int  iommu_flags;
+    unsigned long dfn_first, dfn_last;
+};
+
 struct hvm_vcpu {
     /* Guest control-register and EFER values, just as the guest sees them. */
     unsigned long       guest_cr[5];
@@ -172,6 +179,8 @@ struct hvm_vcpu {
     /* Pending hw/sw interrupt (.vector = -1 means nothing pending). */
     struct x86_event     inject_event;
 
+    struct hvm_grant_unmap_batch grant_unmap_batch;
+
 #ifdef CONFIG_VIRIDIAN
     struct viridian_vcpu *viridian;
 #endif
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index b15805f3f9..ade42b4cb4 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -1722,6 +1722,7 @@ gnttab_unmap_grant_ref(
         c = min(count, (unsigned int)GNTTAB_UNMAP_BATCH_SIZE);
         partial_done = 0;
 
+        gnttab_unmap_batch_begin();
         for ( i = 0; i < c; i++ )
         {
             if ( unlikely(__copy_from_guest(&op, uop, 1)) )
@@ -1734,6 +1735,9 @@ gnttab_unmap_grant_ref(
         }
 
         gnttab_flush_tlb(current->domain);
+        /* After a failed flush the pages may still be reachable: leak them. */
+        if ( gnttab_unmap_batch_end() )
+            partial_done = 0;
 
         for ( i = 0; i < partial_done; i++ )
             unmap_common_complete(&common[i]);
@@ -1749,6 +1753,8 @@ gnttab_unmap_grant_ref(
 
 fault:
     gnttab_flush_tlb(current->domain);
+    if ( gnttab_unmap_batch_end() )
+        partial_done = 0;
 
     for ( i = 0; i < partial_done; i++ )
         unmap_common_complete(&common[i]);
-- 
2.47.3




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.