[PATCH 2/4] iommu/x86: introduce a generic IVMD/RMRR range validity helper

Roger Pau Monne posted 4 patches 2 years ago
[PATCH 2/4] iommu/x86: introduce a generic IVMD/RMRR range validity helper
Posted by Roger Pau Monne 2 years ago
IVMD and RMRR ranges are functionally equivalent, and as so could use the same
validity checker.

Move the IVMD to x86 common IOMMU code and adjust the function to take a pair
of [start, end) mfn parameters.

So far only the AMD-Vi side is adjusted to use the newly introduced helper, the
VT-d side will be adjusted in a further change.

No functional change intended.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
 xen/arch/x86/include/asm/iommu.h         |  3 ++
 xen/drivers/passthrough/amd/iommu_acpi.c | 38 ++------------------
 xen/drivers/passthrough/x86/iommu.c      | 46 ++++++++++++++++++++++++
 3 files changed, 52 insertions(+), 35 deletions(-)

diff --git a/xen/arch/x86/include/asm/iommu.h b/xen/arch/x86/include/asm/iommu.h
index 15a848ddc329..5c7e37331aad 100644
--- a/xen/arch/x86/include/asm/iommu.h
+++ b/xen/arch/x86/include/asm/iommu.h
@@ -135,6 +135,9 @@ struct page_info *__must_check iommu_alloc_pgtable(struct domain_iommu *hd,
                                                    uint64_t contig_mask);
 void iommu_queue_free_pgtable(struct domain_iommu *hd, struct page_info *pg);
 
+/* Check [start, end) unity map range for correctness. */
+bool iommu_unity_region_ok(mfn_t start, mfn_t end);
+
 #endif /* !__ARCH_X86_IOMMU_H__ */
 /*
  * Local variables:
diff --git a/xen/drivers/passthrough/amd/iommu_acpi.c b/xen/drivers/passthrough/amd/iommu_acpi.c
index ca70f4f3ae2c..40468dbbccf3 100644
--- a/xen/drivers/passthrough/amd/iommu_acpi.c
+++ b/xen/drivers/passthrough/amd/iommu_acpi.c
@@ -405,41 +405,9 @@ static int __init parse_ivmd_block(const struct acpi_ivrs_memory *ivmd_block)
         return 0;
     }
 
-    if ( !e820_all_mapped(base, limit + PAGE_SIZE, E820_RESERVED) )
-    {
-        paddr_t addr;
-
-        AMD_IOMMU_WARN("IVMD: [%lx,%lx) is not (entirely) in reserved memory\n",
-                       base, limit + PAGE_SIZE);
-
-        for ( addr = base; addr <= limit; addr += PAGE_SIZE )
-        {
-            unsigned int type = page_get_ram_type(maddr_to_mfn(addr));
-
-            if ( type == RAM_TYPE_UNKNOWN )
-            {
-                if ( e820_add_range(addr, addr + PAGE_SIZE,
-                                    E820_RESERVED) )
-                    continue;
-                AMD_IOMMU_ERROR("IVMD: page at %lx couldn't be reserved\n",
-                                addr);
-                return -EIO;
-            }
-
-            /*
-             * Types which aren't RAM are considered good enough.
-             * Note that a page being partially RESERVED, ACPI or UNUSABLE will
-             * force Xen into assuming the whole page as having that type in
-             * practice.
-             */
-            if ( type & (RAM_TYPE_RESERVED | RAM_TYPE_ACPI |
-                         RAM_TYPE_UNUSABLE) )
-                continue;
-
-            AMD_IOMMU_ERROR("IVMD: page at %lx can't be converted\n", addr);
-            return -EIO;
-        }
-    }
+    if ( !iommu_unity_region_ok(maddr_to_mfn(base),
+                                maddr_to_mfn(limit + PAGE_SIZE)) )
+        return -EIO;
 
     if ( ivmd_block->header.flags & ACPI_IVMD_EXCLUSION_RANGE )
         exclusion = true;
diff --git a/xen/drivers/passthrough/x86/iommu.c b/xen/drivers/passthrough/x86/iommu.c
index c90755ff58fa..63d4cb898218 100644
--- a/xen/drivers/passthrough/x86/iommu.c
+++ b/xen/drivers/passthrough/x86/iommu.c
@@ -792,6 +792,52 @@ static int __init cf_check adjust_irq_affinities(void)
 }
 __initcall(adjust_irq_affinities);
 
+bool __init iommu_unity_region_ok(mfn_t start, mfn_t end)
+{
+    mfn_t addr;
+
+    if ( e820_all_mapped(mfn_to_maddr(start), mfn_to_maddr(end),
+                         E820_RESERVED) )
+        return true;
+
+    printk(XENLOG_WARNING "IOMMU: [%#" PRI_mfn " ,%#" PRI_mfn
+           ") is not (entirely) in reserved memory\n",
+           mfn_x(start), mfn_x(end));
+
+    for ( addr = start; !mfn_eq(addr, end); mfn_add(addr, 1) )
+    {
+        unsigned int type = page_get_ram_type(addr);
+
+        if ( type == RAM_TYPE_UNKNOWN )
+        {
+            if ( e820_add_range(mfn_to_maddr(addr),
+                                mfn_to_maddr(addr) + PAGE_SIZE, E820_RESERVED) )
+                continue;
+            printk(XENLOG_WARNING
+                   "IOMMU: page at %#" PRI_mfn " couldn't be reserved\n",
+                   mfn_x(addr));
+            return false;
+        }
+
+        /*
+         * Types which aren't RAM are considered good enough.
+         * Note that a page being partially RESERVED, ACPI or UNUSABLE will
+         * force Xen into assuming the whole page as having that type in
+         * practice.
+         */
+        if ( type & (RAM_TYPE_RESERVED | RAM_TYPE_ACPI |
+                     RAM_TYPE_UNUSABLE) )
+            continue;
+
+        printk(XENLOG_WARNING
+               "IOMMU: page at %#" PRI_mfn " can't be converted\n",
+               mfn_x(addr));
+        return false;
+    }
+
+    return true;
+}
+
 /*
  * Local variables:
  * mode: C
-- 
2.43.0


Re: [PATCH 2/4] iommu/x86: introduce a generic IVMD/RMRR range validity helper
Posted by Jan Beulich 2 years ago
On 01.02.2024 18:01, Roger Pau Monne wrote:
> IVMD and RMRR ranges are functionally equivalent, and as so could use the same
> validity checker.

May I suggest s/equivalent/similar/?

> Move the IVMD to x86 common IOMMU code and adjust the function to take a pair
> of [start, end) mfn parameters.

[start,end) ranges generally come with the problem of not allowing to
represent the full address space. While that isn't specifically a problem
here, seeing that both VT-d and V-i present inclusive ranges, how about
making the common function match that?

Jan
Re: [PATCH 2/4] iommu/x86: introduce a generic IVMD/RMRR range validity helper
Posted by Roger Pau Monné 2 years ago
On Tue, Feb 06, 2024 at 12:17:29PM +0100, Jan Beulich wrote:
> On 01.02.2024 18:01, Roger Pau Monne wrote:
> > IVMD and RMRR ranges are functionally equivalent, and as so could use the same
> > validity checker.
> 
> May I suggest s/equivalent/similar/?

Sure.

> > Move the IVMD to x86 common IOMMU code and adjust the function to take a pair
> > of [start, end) mfn parameters.
> 
> [start,end) ranges generally come with the problem of not allowing to
> represent the full address space. While that isn't specifically a problem
> here, seeing that both VT-d and V-i present inclusive ranges, how about
> making the common function match that?

OK, I can adjust the code that way.  I originally did it using non
inclusive because it looked like the code would be clearer.

Thanks, Roger.