Re: [PATCH 2/2] xen/arm: Skip loops in init_pdx() when no PDX compression is used

[Hari Limaye <hari.limaye@xxxxxxx>]

Hi Michal,

> On Fri, Jul 04, 2025 at 09:54:28AM +0000, Michal Orzel wrote:
> When CONFIG_PDX_COMPRESSION=n, pdx_init_mask(), pdx_region_mask() and
> pfn_pdx_hole_setup() are just stubs doing nothing. It does not make
> sense to keep the two loops iterating over all the memory banks.
> 
> Signed-off-by: Michal Orzel <michal.orzel@xxxxxxx>
> ---
>  xen/arch/arm/setup.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 93b730ffb5fb..12b76a0a9837 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -255,7 +255,9 @@ void __init init_pdx(void)
>  {
>      const struct membanks *mem = bootinfo_get_mem();
>      paddr_t bank_start, bank_size, bank_end, ram_end = 0;
> +    int bank;

Minor/question: Would we potentially prefer to move the declaration of
the loop counter `bank` variables to each for loop? As the variable is
not used outside of the loops and is always initialized to 0, this seems
perhaps better from a variable scope perspective?

>  
> +#ifdef CONFIG_PDX_COMPRESSION
>      /*
>       * Arm does not have any restrictions on the bits to compress. Pass 0 to
>       * let the common code further restrict the mask.
> @@ -264,7 +266,6 @@ void __init init_pdx(void)
>       * update this function too.
>       */
>      uint64_t mask = pdx_init_mask(0x0);
> -    int bank;
>  
>      for ( bank = 0 ; bank < mem->nr_banks; bank++ )
>      {
> @@ -284,6 +285,7 @@ void __init init_pdx(void)
>      }
>  
>      pfn_pdx_hole_setup(mask >> PAGE_SHIFT);
> +#endif
>  
>      for ( bank = 0 ; bank < mem->nr_banks; bank++ )
>      {

Otherwise, LGTM! Tested (compilation) via both Arm AArch32 and AArch64 builds.

Many thanks,
Hari