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

[Julien Grall <julien@xxxxxxx>]

On 23/05/2022 08:28, Michal Orzel wrote:
> Hi Julien,
Hi Michal,

> On 20.05.2022 14:09, Julien Grall 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>
> >
> > ---
> >
> >      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;
> Shouldn't this be an int (as it was previously) because ...
> > +    const struct meminfo *banks = &bootinfo.mem;
> > +
> > +    for ( i = 0; i < banks->nr_banks; i++ )
> ... nr_banks is int ?
Hmmm... AFAIK banks->nr_banks never hold a negative value, so I am not 
sure why it was introduced as an "int".
Looking through the code, we seem to have a mix of "unsigned int" and 
"int". There seem to be less on the latter, so I have sent a patch to 
switch nr_banks to "unsigned int" [1].
This is based on this series thought and I would like to keep the 
"unsigned int" here.
> Apart from that:
> Reviewed-by: Michal Orzel <michal.orzel@xxxxxxx>
Thanks! Please let me know if this reviewed-by hold.

Cheers,

[1] https://lore.kernel.org/xen-devel/20220523194631.66262-1-julien@xxxxxxx

--
Julien Grall