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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 5 Apr 2022, Julien Grall wrote:
> On 05/04/2022 22:50, Stefano Stabellini wrote:
> > > +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)));
> 
> This has enough space for 256 distinct regions on arm64 (512 regions on
> arm32).
> 
> > 
> > Do you think we should check for the limit in populate_boot_allocator?
> 
> This patch doesn't change the number of regions added to the boot allocator.
> So if we need to check the limit then I would rather deal separately (see more
> below).
> 
> > Or there is no need because it is unrealistic to reach it?
> I can't say never because history told us on some UEFI systems, there will be
> a large number of regions exposed. I haven't heard anyone that would hit the
> BUG_ON().
> 
> The problem is what do we do if we hit the limit? We could ignore all the
> regions after. However, there are potentially a risk there would not be enough
> memory to cover the boot memory allocation (regions may be really small).
> 
> So if we ever hit the limit, then I think we should update the boot allocator.

OK, thanks for the explanation.

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>