Hi Jan
> -----Original Message-----
> From: Jan Beulich <jbeul...@suse.com>
> Sent: Monday, July 19, 2021 4:20 PM
> To: Penny Zheng <penny.zh...@arm.com>
> Cc: Bertrand Marquis <bertrand.marq...@arm.com>; Wei Chen
> <wei.c...@arm.com>; nd <n...@arm.com>; xen-devel@lists.xenproject.org;
> sstabell...@kernel.org; jul...@xen.org
> Subject: Re: [PATCH V3 05/10] xen/arm: static memory initialization
>
> On 15.07.2021 07:18, Penny Zheng wrote:
> > v3 change:
> > - include addition of CONFIG_STATIC_ALLOCATION in this commit, where
> > it is firstly used and also change the name to CONFIG_STATIC_MEMORY
> > - Fix TAB typo in Kconfig
>
> Not sure what this relates to, but ...
>
Before I was wrongly using " set tabstop=4 " for Kconfig file, so...
> > --- a/xen/arch/arm/Kconfig
> > +++ b/xen/arch/arm/Kconfig
> > @@ -106,6 +106,9 @@ config TEE
> >
> > source "arch/arm/tee/Kconfig"
> >
> > +config STATIC_MEMORY
> > + def_bool y
>
> ... this is (wrongly) using spaces for indentation.
>
> I also wonder about the placement: Shouldn't the option live in common code,
> with Arm "select"ing it?
>
Sure, I will place it in xen/common/Kconfig, and add "depends on ARM".
> > --- a/xen/arch/arm/setup.c
> > +++ b/xen/arch/arm/setup.c
> > @@ -622,6 +622,28 @@ static void __init init_pdx(void)
> > }
> > }
> >
> > +/* Static memory initialization */
> > +static void __init init_staticmem_pages(void) {
> > + unsigned int bank;
> > +
> > + /* TODO: Considering NUMA-support scenario. */
> > + for ( bank = 0 ; bank < bootinfo.static_mem.nr_banks; bank++ )
> > + {
> > + paddr_t bank_start = bootinfo.static_mem.bank[bank].start;
> > + paddr_t bank_size = bootinfo.static_mem.bank[bank].size;
> > + paddr_t bank_end = bank_start + bank_size;
> > +
> > + bank_start = round_pgup(bank_start);
> > + bank_end = round_pgdown(bank_end);
> > + if ( bank_end <= bank_start )
> > + return;
> > +
> > + free_staticmem_pages(maddr_to_page(bank_start),
> > + (bank_end - bank_start) >> PAGE_SHIFT,
> > + false);
>
> Indentation (one too few spaces). Perhaps also consider to avoid open-coding
> PFN_DOWN() here; in fact it and PFN_UP() could be used in place of
> round_pg{down,up}() above.
>
Sure. I will replace round_pg{down,up}() with PFN_DOWN()/PFN_UP().
> Jan
Cheers
Penny Zheng
