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.non...@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.or...@amd.com>
~Michal
