Re: [PATCH 13/16] xen/arm32: setup: Move out the code to populate the boot allocator

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi Julien,

> On 20 May 2022, at 13:09, Julien Grall <julien@xxxxxxx> wrote:
> 
> From: Julien Grall <jgrall@xxxxxxxxxx>
> 
> In a follow-up patch, we will want to populate the boot allocator
> separately for arm64. The code will end up to be very similar to the one
> on arm32. So move out the code in a new helper populate_boot_allocator().
> 
> For now the code is still protected by CONFIG_ARM_32 to avoid any build
> failure on arm64.
> 
> Take the opportunity to replace mfn_add(xen_mfn_start, xenheap_pages) with
> xenheap_mfn_end as they are equivalent.
> 
> Signed-off-by: Julien Grall <jgrall@xxxxxxxxxx>
Reviewed-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>

Cheers
Bertrand

> 
> ---
> 
>    Changes in v4:
>        - Patch added
> ---
> xen/arch/arm/setup.c | 90 +++++++++++++++++++++++++-------------------
> 1 file changed, 51 insertions(+), 39 deletions(-)
> 
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index d5d0792ed48a..3d5a2283d4ef 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -637,10 +637,58 @@ static void __init init_staticmem_pages(void)
> }
> 
> #ifdef CONFIG_ARM_32
> +/*
> + * Populate the boot allocator. All the RAM but the following regions
> + * will be added:
> + *  - Modules (e.g., Xen, Kernel)
> + *  - Reserved regions
> + *  - Xenheap
> + */
> +static void __init populate_boot_allocator(void)
> +{
> +    unsigned int i;
> +    const struct meminfo *banks = &bootinfo.mem;
> +
> +    for ( i = 0; i < banks->nr_banks; i++ )
> +    {
> +        const struct membank *bank = &banks->bank[i];
> +        paddr_t bank_end = bank->start + bank->size;
> +        paddr_t s, e;
> +
> +        s = bank->start;
> +        while ( s < bank_end )
> +        {
> +            paddr_t n = bank_end;
> +
> +            e = next_module(s, &n);
> +
> +            if ( e == ~(paddr_t)0 )
> +                e = n = bank_end;
> +
> +            /*
> +             * Module in a RAM bank other than the one which we are
> +             * not dealing with here.
> +             */
> +            if ( e > bank_end )
> +                e = bank_end;
> +
> +            /* Avoid the xenheap */
> +            if ( s < mfn_to_maddr(xenheap_mfn_end) &&
> +                 mfn_to_maddr(xenheap_mfn_start) < e )
> +            {
> +                e = mfn_to_maddr(xenheap_mfn_start);
> +                n = mfn_to_maddr(xenheap_mfn_end);
> +            }
> +
> +            fw_unreserved_regions(s, e, init_boot_pages, 0);
> +            s = n;
> +        }
> +    }
> +}
> +
> static void __init setup_mm(void)
> {
> -    paddr_t ram_start, ram_end, ram_size;
> -    paddr_t s, e;
> +    paddr_t ram_start, ram_end, ram_size, e;
>     unsigned long ram_pages;
>     unsigned long heap_pages, xenheap_pages, domheap_pages;
>     int i;
> @@ -718,43 +766,7 @@ static void __init setup_mm(void)
>     setup_xenheap_mappings((e >> PAGE_SHIFT) - xenheap_pages, xenheap_pages);
> 
>     /* Add non-xenheap memory */
> -    for ( i = 0; i < bootinfo.mem.nr_banks; i++ )
> -    {
> -        paddr_t bank_start = bootinfo.mem.bank[i].start;
> -        paddr_t bank_end = bank_start + bootinfo.mem.bank[i].size;
> -
> -        s = bank_start;
> -        while ( s < bank_end )
> -        {
> -            paddr_t n = bank_end;
> -
> -            e = next_module(s, &n);
> -
> -            if ( e == ~(paddr_t)0 )
> -            {
> -                e = n = ram_end;
> -            }
> -
> -            /*
> -             * Module in a RAM bank other than the one which we are
> -             * not dealing with here.
> -             */
> -            if ( e > bank_end )
> -                e = bank_end;
> -
> -            /* Avoid the xenheap */
> -            if ( s < mfn_to_maddr(mfn_add(xenheap_mfn_start, xenheap_pages))
> -                 && mfn_to_maddr(xenheap_mfn_start) < e )
> -            {
> -                e = mfn_to_maddr(xenheap_mfn_start);
> -                n = mfn_to_maddr(mfn_add(xenheap_mfn_start, xenheap_pages));
> -            }
> -
> -            fw_unreserved_regions(s, e, init_boot_pages, 0);
> -
> -            s = n;
> -        }
> -    }
> +    populate_boot_allocator();
> 
>     /* Frame table covers all of RAM region, including holes */
>     setup_frametable_mappings(ram_start, ram_end);
> -- 
> 2.32.0
>