Re: [PATCH 14/16] xen/arm64: mm: Add memory to the boot allocator first

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 20 May 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.
> 
> Populating the boot allocator is nearly the same between arm32 and
> arm64. The only difference is on the former we need to exclude the
> xenheap for the boot allocator. Gate the difference with CONFIG_ARM_32
> so the code be re-used on arm64.
> 
> Signed-off-by: Julien Grall <jgrall@xxxxxxxxxx>

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>


> ---
>     Changes in v4:
>         - The implementation of populate_boot_allocator() has been
>           moved in a separate patch.
>         - Fix typo
> 
>     Changes in v3:
>         - Patch added
> ---
>  xen/arch/arm/setup.c | 55 +++++++++++++++++++-------------------------
>  1 file changed, 24 insertions(+), 31 deletions(-)
> 
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 3d5a2283d4ef..db1768c03f03 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -636,13 +636,12 @@ static void __init init_staticmem_pages(void)
>  #endif
>  }
>  
> -#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
> + *  - Xenheap (arm32 only)
>   */
>  static void __init populate_boot_allocator(void)
>  {
> @@ -672,6 +671,7 @@ static void __init populate_boot_allocator(void)
>              if ( e > bank_end )
>                  e = bank_end;
>  
> +#ifdef CONFIG_ARM_32
>              /* Avoid the xenheap */
>              if ( s < mfn_to_maddr(xenheap_mfn_end) &&
>                   mfn_to_maddr(xenheap_mfn_start) < e )
> @@ -679,6 +679,7 @@ static void __init populate_boot_allocator(void)
>                  e = mfn_to_maddr(xenheap_mfn_start);
>                  n = mfn_to_maddr(xenheap_mfn_end);
>              }
> +#endif
>  
>              fw_unreserved_regions(s, e, init_boot_pages, 0);
>              s = n;
> @@ -686,6 +687,7 @@ static void __init populate_boot_allocator(void)
>      }
>  }
>  
> +#ifdef CONFIG_ARM_32
>  static void __init setup_mm(void)
>  {
>      paddr_t ram_start, ram_end, ram_size, e;
> @@ -781,45 +783,36 @@ static void __init setup_mm(void)
>  #else /* CONFIG_ARM_64 */
>  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;
> -    int bank;
> +    unsigned int i;
>  
>      init_pdx();
>  
> -    total_pages = 0;
> -    for ( bank = 0 ; bank < bootinfo.mem.nr_banks; bank++ )
> -    {
> -        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;
> -        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;
> -        while ( s < bank_end )
> -        {
> -            paddr_t n = bank_end;
> +    /*
> +     * 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 simplicity, add all the free regions in the boot allocator.
> +     */
> +    populate_boot_allocator();
>  
> -            e = next_module(s, &n);
> +    total_pages = 0;
>  
> -            if ( e == ~(paddr_t)0 )
> -            {
> -                e = n = bank_end;
> -            }
> +    for ( i = 0; i < banks->nr_banks; i++ )
> +    {
> +        const struct membank *bank = &banks->bank[i];
> +        paddr_t bank_end = bank->start + bank->size;
>  
> -            if ( e > bank_end )
> -                e = bank_end;
> +        ram_size = ram_size + bank->size;
> +        ram_start = min(ram_start, bank->start);
> +        ram_end = max(ram_end, bank_end);
>  
> -            fw_unreserved_regions(s, e, init_boot_pages, 0);
> -            s = n;
> -        }
> +        setup_xenheap_mappings(PFN_DOWN(bank->start),
> +                               PFN_DOWN(bank->size));
>      }
>  
>      total_pages += ram_size >> PAGE_SHIFT;
> -- 
> 2.32.0
>