Re: [PATCH v3 17/19] xen/arm64: mm: Add memory to the boot allocator first

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 21 Feb 2022, Julien Grall wrote:
> From: Julien Grall <jgrall@xxxxxxxxxx>
> 
> Currently, memory is added to the boot allocator after the xenheap
> mappings are done. This will break if the first mapping is more than
> 512GB of RAM.
> 
> In addition to that, a follow-up patch will rework setup_xenheap_mappings()
> to use smaller mappings (e.g. 2MB, 4KB). So it will be necessary to have
> memory in the boot allocator earlier.
> 
> Only free memory (e.g. not reserved or modules) can be added to the boot
> allocator. It might be possible that some regions (including the first
> one) will have no free memory.
> 
> So we need to add all the free memory to the boot allocator first
> and then add do the mappings.
> 
> Signed-off-by: Julien Grall <jgrall@xxxxxxxxxx>
> 
> ---
>     Changes in v3:
>         - Patch added
> ---
>  xen/arch/arm/setup.c | 63 +++++++++++++++++++++++++++++---------------
>  1 file changed, 42 insertions(+), 21 deletions(-)
> 
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index d5d0792ed48a..777cf96639f5 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -767,30 +767,18 @@ static void __init setup_mm(void)
>      init_staticmem_pages();
>  }
>  #else /* CONFIG_ARM_64 */
> -static void __init setup_mm(void)
> +static void __init populate_boot_allocator(void)
>  {
> -    paddr_t ram_start = ~0;
> -    paddr_t ram_end = 0;
> -    paddr_t ram_size = 0;
> -    int bank;
> -
> -    init_pdx();
> +    unsigned int i;
> +    const struct meminfo *banks = &bootinfo.mem;
>  
> -    total_pages = 0;
> -    for ( bank = 0 ; bank < bootinfo.mem.nr_banks; bank++ )
> +    for ( i = 0; i < banks->nr_banks; i++ )
>      {
> -        paddr_t bank_start = bootinfo.mem.bank[bank].start;
> -        paddr_t bank_size = bootinfo.mem.bank[bank].size;
> -        paddr_t bank_end = bank_start + bank_size;
> +        const struct membank *bank = &banks->bank[i];
> +        paddr_t bank_end = bank->start + bank->size;
>          paddr_t s, e;
>  
> -        ram_size = ram_size + bank_size;
> -        ram_start = min(ram_start,bank_start);
> -        ram_end = max(ram_end,bank_end);
> -
> -        setup_xenheap_mappings(bank_start>>PAGE_SHIFT, 
> bank_size>>PAGE_SHIFT);
> -
> -        s = bank_start;
> +        s = bank->start;
>          while ( s < bank_end )
>          {
>              paddr_t n = bank_end;
> @@ -798,9 +786,7 @@ static void __init setup_mm(void)
>              e = next_module(s, &n);
>  
>              if ( e == ~(paddr_t)0 )
> -            {
>                  e = n = bank_end;
> -            }
>  
>              if ( e > bank_end )
>                  e = bank_end;
> @@ -809,6 +795,41 @@ static void __init setup_mm(void)
>              s = n;
>          }
>      }
> +}
> +
> +static void __init setup_mm(void)
> +{
> +    const struct meminfo *banks = &bootinfo.mem;
> +    paddr_t ram_start = ~0;
> +    paddr_t ram_end = 0;
> +    paddr_t ram_size = 0;
> +    unsigned int i;
> +
> +    init_pdx();
> +
> +    /*
> +     * We need some memory to allocate the page-tables used for the xenheap
> +     * mappings. But some regions may contain memory already allocated
> +     * for other uses (e.g. modules, reserved-memory...).
> +     *
> +     * For simplify add all the free regions in the boot allocator.
> +     */

We currently have:

BUG_ON(nr_bootmem_regions == (PAGE_SIZE / sizeof(struct bootmem_region)));

Do you think we should check for the limit in populate_boot_allocator?
Or there is no need because it is unrealistic to reach it?


> +    populate_boot_allocator();
> +
> +    total_pages = 0;
> +
> +    for ( i = 0; i < banks->nr_banks; i++ )
> +    {
> +        const struct membank *bank = &banks->bank[i];
> +        paddr_t bank_end = bank->start + bank->size;
> +
> +        ram_size = ram_size + bank->size;
> +        ram_start = min(ram_start, bank->start);
> +        ram_end = max(ram_end, bank_end);
> +
> +        setup_xenheap_mappings(PFN_DOWN(bank->start),
> +                               PFN_DOWN(bank->size));
> +    }
>  
>      total_pages += ram_size >> PAGE_SHIFT;
>  
> -- 
> 2.32.0
>