diff mbox series

[2/3] x86/iommu: move xen_in_range() so it can be made static

Message ID 20231117094749.81091-3-roger.pau@citrix.com (mailing list archive)
State Superseded
Headers show
Series x86/iommu: improve setup time of hwdom IOMMU | expand

Commit Message

Roger Pau Monné Nov. 17, 2023, 9:47 a.m. UTC 
No functional change intended.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
 xen/arch/x86/include/asm/setup.h    |  2 --
 xen/arch/x86/setup.c                | 49 ----------------------------
 xen/drivers/passthrough/x86/iommu.c | 50 +++++++++++++++++++++++++++++
 3 files changed, 50 insertions(+), 51 deletions(-)

Comments

Andrew Cooper Nov. 17, 2023, 12:03 p.m. UTC | #1 
On 17/11/2023 9:47 am, Roger Pau Monne wrote:
> No functional change intended.
>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

There may only be one caller (after dropping some bogus tboot logic
recently IIRC), but this isn't an IOMMU-specific helper.

See the comment in the middle which shows the other opencoded things
this needs to be kept up to date.  (And I'd hoped to make this common
because every architecture seems to have different bugs opencoding this
calculation.)

Switching to rangesets is fine, but the result still wants to be
something generic, rather than IOMMU-specific.

~Andrew
Roger Pau Monné Nov. 17, 2023, 12:50 p.m. UTC | #2 
On Fri, Nov 17, 2023 at 12:03:19PM +0000, Andrew Cooper wrote:
> On 17/11/2023 9:47 am, Roger Pau Monne wrote:
> > No functional change intended.
> >
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> 
> There may only be one caller (after dropping some bogus tboot logic
> recently IIRC), but this isn't an IOMMU-specific helper.
> 
> See the comment in the middle which shows the other opencoded things
> this needs to be kept up to date.  (And I'd hoped to make this common
> because every architecture seems to have different bugs opencoding this
> calculation.)

But those symbols ultimately come from the linker script, and hence
didn't see them that tied to the setup code.  The setup code uses them,
just as the IOMMU will do now, and likely other parts of the code like
livepatch.

> Switching to rangesets is fine, but the result still wants to be
> something generic, rather than IOMMU-specific.

I'm fine with leaving the function in setup.c, this was mostly a
cleanup attempt.

Thanks, Roger.
diff mbox series

Patch

diff --git a/xen/arch/x86/include/asm/setup.h b/xen/arch/x86/include/asm/setup.h
index 9a460e4db8f4..4a1600decf6a 100644
--- a/xen/arch/x86/include/asm/setup.h
+++ b/xen/arch/x86/include/asm/setup.h
@@ -36,8 +36,6 @@  unsigned long initial_images_nrpages(nodeid_t node);
 void discard_initial_images(void);
 void *bootstrap_map(const module_t *mod);
 
-int xen_in_range(unsigned long mfn);
-
 extern uint8_t kbd_shift_flags;
 
 #ifdef NDEBUG
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index a3d3f797bb1e..54daff3d4942 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -2080,55 +2080,6 @@  void arch_get_xen_caps(xen_capabilities_info_t *info)
     }
 }
 
-int __hwdom_init xen_in_range(unsigned long mfn)
-{
-    paddr_t start, end;
-    int i;
-
-    enum { region_s3, region_ro, region_rw, region_bss, nr_regions };
-    static struct {
-        paddr_t s, e;
-    } xen_regions[nr_regions] __hwdom_initdata;
-
-    /* initialize first time */
-    if ( !xen_regions[0].s )
-    {
-        /* S3 resume code (and other real mode trampoline code) */
-        xen_regions[region_s3].s = bootsym_phys(trampoline_start);
-        xen_regions[region_s3].e = bootsym_phys(trampoline_end);
-
-        /*
-         * This needs to remain in sync with the uses of the same symbols in
-         * - __start_xen() (above)
-         * - is_xen_fixed_mfn()
-         * - tboot_shutdown()
-         */
-
-        /* hypervisor .text + .rodata */
-        xen_regions[region_ro].s = __pa(&_stext);
-        xen_regions[region_ro].e = __pa(&__2M_rodata_end);
-        /* hypervisor .data + .bss */
-        xen_regions[region_rw].s = __pa(&__2M_rwdata_start);
-        xen_regions[region_rw].e = __pa(&__2M_rwdata_end);
-        if ( efi_boot_mem_unused(&start, &end) )
-        {
-            ASSERT(__pa(start) >= xen_regions[region_rw].s);
-            ASSERT(__pa(end) <= xen_regions[region_rw].e);
-            xen_regions[region_rw].e = __pa(start);
-            xen_regions[region_bss].s = __pa(end);
-            xen_regions[region_bss].e = __pa(&__2M_rwdata_end);
-        }
-    }
-
-    start = (paddr_t)mfn << PAGE_SHIFT;
-    end = start + PAGE_SIZE;
-    for ( i = 0; i < nr_regions; i++ )
-        if ( (start < xen_regions[i].e) && (end > xen_regions[i].s) )
-            return 1;
-
-    return 0;
-}
-
 static int __hwdom_init cf_check io_bitmap_cb(
     unsigned long s, unsigned long e, void *ctx)
 {
diff --git a/xen/drivers/passthrough/x86/iommu.c b/xen/drivers/passthrough/x86/iommu.c
index 857dccb6a465..d70cee9fea77 100644
--- a/xen/drivers/passthrough/x86/iommu.c
+++ b/xen/drivers/passthrough/x86/iommu.c
@@ -13,6 +13,7 @@ 
  */
 
 #include <xen/cpu.h>
+#include <xen/efi.h>
 #include <xen/sched.h>
 #include <xen/iocap.h>
 #include <xen/iommu.h>
@@ -300,6 +301,55 @@  void iommu_identity_map_teardown(struct domain *d)
     }
 }
 
+static int __hwdom_init xen_in_range(unsigned long mfn)
+{
+    paddr_t start, end;
+    int i;
+
+    enum { region_s3, region_ro, region_rw, region_bss, nr_regions };
+    static struct {
+        paddr_t s, e;
+    } xen_regions[nr_regions] __hwdom_initdata;
+
+    /* initialize first time */
+    if ( !xen_regions[0].s )
+    {
+        /* S3 resume code (and other real mode trampoline code) */
+        xen_regions[region_s3].s = bootsym_phys(trampoline_start);
+        xen_regions[region_s3].e = bootsym_phys(trampoline_end);
+
+        /*
+         * This needs to remain in sync with the uses of the same symbols in
+         * - __start_xen()
+         * - is_xen_fixed_mfn()
+         * - tboot_shutdown()
+         */
+
+        /* hypervisor .text + .rodata */
+        xen_regions[region_ro].s = __pa(&_stext);
+        xen_regions[region_ro].e = __pa(&__2M_rodata_end);
+        /* hypervisor .data + .bss */
+        xen_regions[region_rw].s = __pa(&__2M_rwdata_start);
+        xen_regions[region_rw].e = __pa(&__2M_rwdata_end);
+        if ( efi_boot_mem_unused(&start, &end) )
+        {
+            ASSERT(__pa(start) >= xen_regions[region_rw].s);
+            ASSERT(__pa(end) <= xen_regions[region_rw].e);
+            xen_regions[region_rw].e = __pa(start);
+            xen_regions[region_bss].s = __pa(end);
+            xen_regions[region_bss].e = __pa(&__2M_rwdata_end);
+        }
+    }
+
+    start = (paddr_t)mfn << PAGE_SHIFT;
+    end = start + PAGE_SIZE;
+    for ( i = 0; i < nr_regions; i++ )
+        if ( (start < xen_regions[i].e) && (end > xen_regions[i].s) )
+            return 1;
+
+    return 0;
+}
+
 static unsigned int __hwdom_init hwdom_iommu_map(const struct domain *d,
                                                  unsigned long pfn,
                                                  unsigned long max_pfn)