Cache coloring must physically relocate Xen in order to color the hypervisor
and consider_modules() is a key function that is needed to find a new
available physical address.

672d67f339c0 ("xen/arm: Split MMU-specific setup_mm() and related code out")
moved consider_modules() under arm32. Move it to mmu/setup.c and make it
non-static so that it can be used outside.

Signed-off-by: Carlo Nonato <carlo.nonato@minervasys.tech>
---
v10:
- no changes
v9:
- no changes
v8:
- patch adapted to new changes to consider_modules()
v7:
- moved consider_modules() to arm/mmu/setup.c
v6:
- new patch
---
 xen/arch/arm/arm32/mmu/mm.c      | 95 +------------------------------
 xen/arch/arm/include/asm/setup.h |  3 +
 xen/arch/arm/mmu/setup.c         | 97 ++++++++++++++++++++++++++++++++
 3 files changed, 101 insertions(+), 94 deletions(-)

diff --git a/xen/arch/arm/arm32/mmu/mm.c b/xen/arch/arm/arm32/mmu/mm.c
index 063611412b..c5fcd19291 100644
--- a/xen/arch/arm/arm32/mmu/mm.c
+++ b/xen/arch/arm/arm32/mmu/mm.c
@@ -9,6 +9,7 @@
 #include <asm/fixmap.h>
 #include <asm/static-memory.h>
 #include <asm/static-shmem.h>
+#include <asm/setup.h>
 
 static unsigned long opt_xenheap_megabytes __initdata;
 integer_param("xenheap_megabytes", opt_xenheap_megabytes);
@@ -31,100 +32,6 @@ static void __init setup_directmap_mappings(unsigned long base_mfn,
     directmap_virt_end = XENHEAP_VIRT_START + nr_mfns * PAGE_SIZE;
 }
 
-/*
- * Returns the end address of the highest region in the range s..e
- * with required size and alignment that does not conflict with the
- * modules from first_mod to nr_modules.
- *
- * For non-recursive callers first_mod should normally be 0 (all
- * modules and Xen itself) or 1 (all modules but not Xen).
- */
-static paddr_t __init consider_modules(paddr_t s, paddr_t e,
-                                       uint32_t size, paddr_t align,
-                                       int first_mod)
-{
-    const struct membanks *reserved_mem = bootinfo_get_reserved_mem();
-#ifdef CONFIG_STATIC_SHM
-    const struct membanks *shmem = bootinfo_get_shmem();
-#endif
-    const struct bootmodules *mi = &bootinfo.modules;
-    int i;
-    int nr;
-
-    s = (s+align-1) & ~(align-1);
-    e = e & ~(align-1);
-
-    if ( s > e ||  e - s < size )
-        return 0;
-
-    /* First check the boot modules */
-    for ( i = first_mod; i < mi->nr_mods; i++ )
-    {
-        paddr_t mod_s = mi->module[i].start;
-        paddr_t mod_e = mod_s + mi->module[i].size;
-
-        if ( s < mod_e && mod_s < e )
-        {
-            mod_e = consider_modules(mod_e, e, size, align, i+1);
-            if ( mod_e )
-                return mod_e;
-
-            return consider_modules(s, mod_s, size, align, i+1);
-        }
-    }
-
-    /*
-     * i is the current bootmodule we are evaluating, across all
-     * possible kinds of bootmodules.
-     *
-     * When retrieving the corresponding reserved-memory addresses, we
-     * need to index the reserved_mem bank starting from 0, and only counting
-     * the reserved-memory modules. Hence, we need to use i - nr.
-     */
-    nr = mi->nr_mods;
-    for ( ; i - nr < reserved_mem->nr_banks; i++ )
-    {
-        paddr_t r_s = reserved_mem->bank[i - nr].start;
-        paddr_t r_e = r_s + reserved_mem->bank[i - nr].size;
-
-        if ( s < r_e && r_s < e )
-        {
-            r_e = consider_modules(r_e, e, size, align, i + 1);
-            if ( r_e )
-                return r_e;
-
-            return consider_modules(s, r_s, size, align, i + 1);
-        }
-    }
-
-#ifdef CONFIG_STATIC_SHM
-    nr += reserved_mem->nr_banks;
-    for ( ; i - nr < shmem->nr_banks; i++ )
-    {
-        paddr_t r_s, r_e;
-
-        r_s = shmem->bank[i - nr].start;
-
-        /* Shared memory banks can contain INVALID_PADDR as start */
-        if ( INVALID_PADDR == r_s )
-            continue;
-
-        r_e = r_s + shmem->bank[i - nr].size;
-
-        if ( s < r_e && r_s < e )
-        {
-            r_e = consider_modules(r_e, e, size, align, i + 1);
-            if ( r_e )
-                return r_e;
-
-            return consider_modules(s, r_s, size, align, i + 1);
-        }
-    }
-#endif
-
-    return e;
-}
-
 /*
  * Find a contiguous region that fits in the static heap region with
  * required size and alignment, and return the end address of the region
diff --git a/xen/arch/arm/include/asm/setup.h b/xen/arch/arm/include/asm/setup.h
index 64c227d171..0c560d141f 100644
--- a/xen/arch/arm/include/asm/setup.h
+++ b/xen/arch/arm/include/asm/setup.h
@@ -89,6 +89,9 @@ struct init_info
     unsigned int cpuid;
 };
 
+paddr_t consider_modules(paddr_t s, paddr_t e, uint32_t size, paddr_t align,
+                         int first_mod);
+
 #endif
 /*
  * Local variables:
diff --git a/xen/arch/arm/mmu/setup.c b/xen/arch/arm/mmu/setup.c
index 9664e85ee6..1cf62390e3 100644
--- a/xen/arch/arm/mmu/setup.c
+++ b/xen/arch/arm/mmu/setup.c
@@ -6,7 +6,10 @@
  */
 
 #include <xen/init.h>
+#include <xen/lib.h>
 #include <xen/libfdt/libfdt.h>
+#include <xen/libfdt/libfdt-xen.h>
+#include <xen/llc-coloring.h>
 #include <xen/sections.h>
 #include <xen/sizes.h>
 #include <xen/vmap.h>
@@ -222,6 +225,100 @@ static void xen_pt_enforce_wnx(void)
     flush_xen_tlb_local();
 }
 
+/*
+ * Returns the end address of the highest region in the range s..e
+ * with required size and alignment that does not conflict with the
+ * modules from first_mod to nr_modules.
+ *
+ * For non-recursive callers first_mod should normally be 0 (all
+ * modules and Xen itself) or 1 (all modules but not Xen).
+ */
+paddr_t __init consider_modules(paddr_t s, paddr_t e,
+                                uint32_t size, paddr_t align,
+                                int first_mod)
+{
+    const struct membanks *reserved_mem = bootinfo_get_reserved_mem();
+#ifdef CONFIG_STATIC_SHM
+    const struct membanks *shmem = bootinfo_get_shmem();
+#endif
+    const struct bootmodules *mi = &bootinfo.modules;
+    int i;
+    int nr;
+
+    s = (s+align-1) & ~(align-1);
+    e = e & ~(align-1);
+
+    if ( s > e ||  e - s < size )
+        return 0;
+
+    /* First check the boot modules */
+    for ( i = first_mod; i < mi->nr_mods; i++ )
+    {
+        paddr_t mod_s = mi->module[i].start;
+        paddr_t mod_e = mod_s + mi->module[i].size;
+
+        if ( s < mod_e && mod_s < e )
+        {
+            mod_e = consider_modules(mod_e, e, size, align, i+1);
+            if ( mod_e )
+                return mod_e;
+
+            return consider_modules(s, mod_s, size, align, i+1);
+        }
+    }
+
+    /*
+     * i is the current bootmodule we are evaluating, across all
+     * possible kinds of bootmodules.
+     *
+     * When retrieving the corresponding reserved-memory addresses, we
+     * need to index the reserved_mem bank starting from 0, and only counting
+     * the reserved-memory modules. Hence, we need to use i - nr.
+     */
+    nr = mi->nr_mods;
+    for ( ; i - nr < reserved_mem->nr_banks; i++ )
+    {
+        paddr_t r_s = reserved_mem->bank[i - nr].start;
+        paddr_t r_e = r_s + reserved_mem->bank[i - nr].size;
+
+        if ( s < r_e && r_s < e )
+        {
+            r_e = consider_modules(r_e, e, size, align, i + 1);
+            if ( r_e )
+                return r_e;
+
+            return consider_modules(s, r_s, size, align, i + 1);
+        }
+    }
+
+#ifdef CONFIG_STATIC_SHM
+    nr += reserved_mem->nr_banks;
+    for ( ; i - nr < shmem->nr_banks; i++ )
+    {
+        paddr_t r_s, r_e;
+
+        r_s = shmem->bank[i - nr].start;
+
+        /* Shared memory banks can contain INVALID_PADDR as start */
+        if ( INVALID_PADDR == r_s )
+            continue;
+
+        r_e = r_s + shmem->bank[i - nr].size;
+
+        if ( s < r_e && r_s < e )
+        {
+            r_e = consider_modules(r_e, e, size, align, i + 1);
+            if ( r_e )
+                return r_e;
+
+            return consider_modules(s, r_s, size, align, i + 1);
+        }
+    }
+#endif
+
+    return e;
+}
+
 /*
  * Boot-time pagetable setup.
  * Changes here may need matching changes in head.S
-- 
2.43.0

On 19/11/2024 15:13, Carlo Nonato wrote:
> 
> 
> Cache coloring must physically relocate Xen in order to color the hypervisor
> and consider_modules() is a key function that is needed to find a new
> available physical address.
> 
> 672d67f339c0 ("xen/arm: Split MMU-specific setup_mm() and related code out")
> moved consider_modules() under arm32. Move it to mmu/setup.c and make it
> non-static so that it can be used outside.
> 
> Signed-off-by: Carlo Nonato <carlo.nonato@minervasys.tech>
> ---
> v10:
> - no changes
> v9:
> - no changes
> v8:
> - patch adapted to new changes to consider_modules()
> v7:
> - moved consider_modules() to arm/mmu/setup.c
> v6:
> - new patch
> ---
>  xen/arch/arm/arm32/mmu/mm.c      | 95 +------------------------------
>  xen/arch/arm/include/asm/setup.h |  3 +
>  xen/arch/arm/mmu/setup.c         | 97 ++++++++++++++++++++++++++++++++
>  3 files changed, 101 insertions(+), 94 deletions(-)
> 
> diff --git a/xen/arch/arm/arm32/mmu/mm.c b/xen/arch/arm/arm32/mmu/mm.c
> index 063611412b..c5fcd19291 100644
> --- a/xen/arch/arm/arm32/mmu/mm.c
> +++ b/xen/arch/arm/arm32/mmu/mm.c
> @@ -9,6 +9,7 @@
>  #include <asm/fixmap.h>
>  #include <asm/static-memory.h>
>  #include <asm/static-shmem.h>
> +#include <asm/setup.h>
Sort alphabetically.

> 
>  static unsigned long opt_xenheap_megabytes __initdata;
>  integer_param("xenheap_megabytes", opt_xenheap_megabytes);
> @@ -31,100 +32,6 @@ static void __init setup_directmap_mappings(unsigned long base_mfn,
>      directmap_virt_end = XENHEAP_VIRT_START + nr_mfns * PAGE_SIZE;
>  }
> 
> -/*
> - * Returns the end address of the highest region in the range s..e
> - * with required size and alignment that does not conflict with the
> - * modules from first_mod to nr_modules.
> - *
> - * For non-recursive callers first_mod should normally be 0 (all
> - * modules and Xen itself) or 1 (all modules but not Xen).
> - */
> -static paddr_t __init consider_modules(paddr_t s, paddr_t e,
> -                                       uint32_t size, paddr_t align,
> -                                       int first_mod)
> -{
> -    const struct membanks *reserved_mem = bootinfo_get_reserved_mem();
> -#ifdef CONFIG_STATIC_SHM
> -    const struct membanks *shmem = bootinfo_get_shmem();
> -#endif
> -    const struct bootmodules *mi = &bootinfo.modules;
> -    int i;
> -    int nr;
> -
> -    s = (s+align-1) & ~(align-1);
> -    e = e & ~(align-1);
> -
> -    if ( s > e ||  e - s < size )
> -        return 0;
> -
> -    /* First check the boot modules */
> -    for ( i = first_mod; i < mi->nr_mods; i++ )
> -    {
> -        paddr_t mod_s = mi->module[i].start;
> -        paddr_t mod_e = mod_s + mi->module[i].size;
> -
> -        if ( s < mod_e && mod_s < e )
> -        {
> -            mod_e = consider_modules(mod_e, e, size, align, i+1);
> -            if ( mod_e )
> -                return mod_e;
> -
> -            return consider_modules(s, mod_s, size, align, i+1);
> -        }
> -    }
> -
> -    /*
> -     * i is the current bootmodule we are evaluating, across all
> -     * possible kinds of bootmodules.
> -     *
> -     * When retrieving the corresponding reserved-memory addresses, we
> -     * need to index the reserved_mem bank starting from 0, and only counting
> -     * the reserved-memory modules. Hence, we need to use i - nr.
> -     */
> -    nr = mi->nr_mods;
> -    for ( ; i - nr < reserved_mem->nr_banks; i++ )
> -    {
> -        paddr_t r_s = reserved_mem->bank[i - nr].start;
> -        paddr_t r_e = r_s + reserved_mem->bank[i - nr].size;
> -
> -        if ( s < r_e && r_s < e )
> -        {
> -            r_e = consider_modules(r_e, e, size, align, i + 1);
> -            if ( r_e )
> -                return r_e;
> -
> -            return consider_modules(s, r_s, size, align, i + 1);
> -        }
> -    }
> -
> -#ifdef CONFIG_STATIC_SHM
> -    nr += reserved_mem->nr_banks;
> -    for ( ; i - nr < shmem->nr_banks; i++ )
> -    {
> -        paddr_t r_s, r_e;
> -
> -        r_s = shmem->bank[i - nr].start;
> -
> -        /* Shared memory banks can contain INVALID_PADDR as start */
> -        if ( INVALID_PADDR == r_s )
> -            continue;
> -
> -        r_e = r_s + shmem->bank[i - nr].size;
> -
> -        if ( s < r_e && r_s < e )
> -        {
> -            r_e = consider_modules(r_e, e, size, align, i + 1);
> -            if ( r_e )
> -                return r_e;
> -
> -            return consider_modules(s, r_s, size, align, i + 1);
> -        }
> -    }
> -#endif
> -
> -    return e;
> -}
> -
>  /*
>   * Find a contiguous region that fits in the static heap region with
>   * required size and alignment, and return the end address of the region
> diff --git a/xen/arch/arm/include/asm/setup.h b/xen/arch/arm/include/asm/setup.h
> index 64c227d171..0c560d141f 100644
> --- a/xen/arch/arm/include/asm/setup.h
> +++ b/xen/arch/arm/include/asm/setup.h
> @@ -89,6 +89,9 @@ struct init_info
>      unsigned int cpuid;
>  };
> 
> +paddr_t consider_modules(paddr_t s, paddr_t e, uint32_t size, paddr_t align,
> +                         int first_mod);
> +
>  #endif
>  /*
>   * Local variables:
> diff --git a/xen/arch/arm/mmu/setup.c b/xen/arch/arm/mmu/setup.c
> index 9664e85ee6..1cf62390e3 100644
> --- a/xen/arch/arm/mmu/setup.c
> +++ b/xen/arch/arm/mmu/setup.c
> @@ -6,7 +6,10 @@
>   */
> 
>  #include <xen/init.h>
> +#include <xen/lib.h>
What's in lib.h that consider_modules() requires?

>  #include <xen/libfdt/libfdt.h>
> +#include <xen/libfdt/libfdt-xen.h>
Why? AFAICS, this header defines fdt_get_mem_rsv_paddr() which is not used in this file.

> +#include <xen/llc-coloring.h>
Why? There's nothing LLC related here at this point.

With the header inclusions sorted out:
Reviewed-by: Michal Orzel <michal.orzel@amd.com>

~Michal