diff mbox series

[v6,13/15] xen/arm: make consider_modules() available for xen relocation

Message ID 20240129171811.21382-14-carlo.nonato@minervasys.tech (mailing list archive)
State Superseded
Headers show
Series Arm cache coloring | expand

Commit Message

Carlo Nonato Jan. 29, 2024, 5:18 p.m. UTC
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 back to setup.c and make it
non-static so that it can be used outside.

Signed-off-by: Carlo Nonato <carlo.nonato@minervasys.tech>
---
v6:
- new patch
---
 xen/arch/arm/arm32/mmu/mm.c      | 93 +-------------------------------
 xen/arch/arm/include/asm/setup.h |  3 ++
 xen/arch/arm/setup.c             | 92 +++++++++++++++++++++++++++++++
 3 files changed, 96 insertions(+), 92 deletions(-)

Comments

Julien Grall March 8, 2024, 10:30 p.m. UTC | #1
(+ Ayan + Henry)

Hi Carlo,

On 29/01/2024 17:18, 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 back to setup.c and make it
> non-static so that it can be used outside.
> 
> Signed-off-by: Carlo Nonato <carlo.nonato@minervasys.tech>
> ---
> v6:
> - new patch
> ---
>   xen/arch/arm/arm32/mmu/mm.c      | 93 +-------------------------------
>   xen/arch/arm/include/asm/setup.h |  3 ++
>   xen/arch/arm/setup.c             | 92 +++++++++++++++++++++++++++++++

Ayan, Henry, will the function consider_modules() be used for the MPU 
code? If not, then I think the function should live in arm/mmu/setup.c.

Cheers,
Henry Wang March 9, 2024, 6:44 a.m. UTC | #2
Hi Julien,

On 3/9/2024 6:30 AM, Julien Grall wrote:
> (+ Ayan + Henry)

(- my old email address + the new one)

> Hi Carlo,
>
> On 29/01/2024 17:18, 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 back to setup.c and 
>> make it
>> non-static so that it can be used outside.
>>
>> Signed-off-by: Carlo Nonato <carlo.nonato@minervasys.tech>
>> ---
>> v6:
>> - new patch
>> ---
>>   xen/arch/arm/arm32/mmu/mm.c      | 93 +-------------------------------
>>   xen/arch/arm/include/asm/setup.h |  3 ++
>>   xen/arch/arm/setup.c             | 92 +++++++++++++++++++++++++++++++
>
> Ayan, Henry, will the function consider_modules() be used for the MPU 
> code?

I checked all the MPU branches (the recent arm64 implementation and an 
arm32 branch to my best knowledge), no I don't think the function will 
be used for the MPU code. However I am not sure if Ayan has a newer 
arm32 MPU branch on top of the latest staging or so using the function, 
Ayan would you like to add some comments?

> If not, then I think the function should live in arm/mmu/setup.c.

+1, arm/mmu/setup.c is a good place.

Kind regards,
Henry

>
> Cheers,
>
Ayan Kumar Halder March 11, 2024, 12:12 p.m. UTC | #3
On 09/03/2024 06:44, Henry Wang wrote:
> Hi Julien,
Hi Julien/Henry,
>
> On 3/9/2024 6:30 AM, Julien Grall wrote:
>> (+ Ayan + Henry)
>
> (- my old email address + the new one)
>
>> Hi Carlo,
>>
>> On 29/01/2024 17:18, 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 back to setup.c and 
>>> make it
>>> non-static so that it can be used outside.
>>>
>>> Signed-off-by: Carlo Nonato <carlo.nonato@minervasys.tech>
>>> ---
>>> v6:
>>> - new patch
>>> ---
>>>   xen/arch/arm/arm32/mmu/mm.c      | 93 
>>> +-------------------------------
>>>   xen/arch/arm/include/asm/setup.h |  3 ++
>>>   xen/arch/arm/setup.c             | 92 +++++++++++++++++++++++++++++++
>>
>> Ayan, Henry, will the function consider_modules() be used for the MPU 
>> code?
>
> I checked all the MPU branches (the recent arm64 implementation and an 
> arm32 branch to my best knowledge), no I don't think the function will 
> be used for the MPU code. However I am not sure if Ayan has a newer 
> arm32 MPU branch on top of the latest staging or so using the 
> function, Ayan would you like to add some comments?

Yes, Henry is correct.

consider_modules() is mmu specific only.

- Ayan

>
>> If not, then I think the function should live in arm/mmu/setup.c.
>
> +1, arm/mmu/setup.c is a good place.
>
> Kind regards,
> Henry
>
>>
>> Cheers,
>>
>
diff mbox series

Patch

diff --git a/xen/arch/arm/arm32/mmu/mm.c b/xen/arch/arm/arm32/mmu/mm.c
index cb441ca87c..e9e1e48f9f 100644
--- a/xen/arch/arm/arm32/mmu/mm.c
+++ b/xen/arch/arm/arm32/mmu/mm.c
@@ -7,6 +7,7 @@ 
 #include <xen/pfn.h>
 #include <asm/fixmap.h>
 #include <asm/static-memory.h>
+#include <asm/setup.h>
 
 static unsigned long opt_xenheap_megabytes __initdata;
 integer_param("xenheap_megabytes", opt_xenheap_megabytes);
@@ -29,98 +30,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 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);
-        }
-    }
-
-    /* Now check any fdt reserved areas. */
-
-    nr = fdt_num_mem_rsv(device_tree_flattened);
-
-    for ( ; i < mi->nr_mods + nr; i++ )
-    {
-        paddr_t mod_s, mod_e;
-
-        if ( fdt_get_mem_rsv_paddr(device_tree_flattened,
-                                   i - mi->nr_mods,
-                                   &mod_s, &mod_e ) < 0 )
-            /* If we can't read it, pretend it doesn't exist... */
-            continue;
-
-        /* fdt_get_mem_rsv_paddr returns length */
-        mod_e += mod_s;
-
-        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 bootinfo.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 < bootinfo.reserved_mem.nr_banks; i++ )
-    {
-        paddr_t r_s = bootinfo.reserved_mem.bank[i - nr].start;
-        paddr_t r_e = r_s + bootinfo.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);
-        }
-    }
-    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 d15a88d2e0..37c0e345f0 100644
--- a/xen/arch/arm/include/asm/setup.h
+++ b/xen/arch/arm/include/asm/setup.h
@@ -207,6 +207,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/setup.c b/xen/arch/arm/setup.c
index 14cb023783..28f4761705 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -545,6 +545,98 @@  static void * __init relocate_fdt(paddr_t dtb_paddr, size_t dtb_size)
     return fdt;
 }
 
+/*
+ * 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 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);
+        }
+    }
+
+    /* Now check any fdt reserved areas. */
+
+    nr = fdt_num_mem_rsv(device_tree_flattened);
+
+    for ( ; i < mi->nr_mods + nr; i++ )
+    {
+        paddr_t mod_s, mod_e;
+
+        if ( fdt_get_mem_rsv_paddr(device_tree_flattened,
+                                   i - mi->nr_mods,
+                                   &mod_s, &mod_e ) < 0 )
+            /* If we can't read it, pretend it doesn't exist... */
+            continue;
+
+        /* fdt_get_mem_rsv_paddr returns length */
+        mod_e += mod_s;
+
+        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 bootinfo.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 < bootinfo.reserved_mem.nr_banks; i++ )
+    {
+        paddr_t r_s = bootinfo.reserved_mem.bank[i - nr].start;
+        paddr_t r_e = r_s + bootinfo.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);
+        }
+    }
+    return e;
+}
+
 /*
  * Return the end of the non-module region starting at s. In other
  * words return s the start of the next modules after s.