Hi Luca,
On 12/03/2024 14:03, Luca Fancellu wrote:
>
>
> The function find_unallocated_memory is using the same code to
> loop through 3 structure of the same type, in order to avoid
> code duplication, rework the code to have only one loop that
> goes through all the structures.
>
> Signed-off-by: Luca Fancellu <luca.fance...@arm.com>
> ---
> xen/arch/arm/domain_build.c | 62 ++++++++++---------------------------
> 1 file changed, 17 insertions(+), 45 deletions(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index b254f252e7cb..d0f2ac6060eb 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -869,12 +869,14 @@ static int __init add_ext_regions(unsigned long s_gfn,
> unsigned long e_gfn,
> static int __init find_unallocated_memory(const struct kernel_info *kinfo,
> struct membanks *ext_regions)
> {
> - const struct membanks *kinfo_mem = kernel_info_get_mem(kinfo);
> - const struct membanks *mem = bootinfo_get_mem();
> - const struct membanks *reserved_mem = bootinfo_get_reserved_mem();
> + const struct membanks *mem_banks[] = {
> + bootinfo_get_mem(),
> + kernel_info_get_mem(kinfo),
> + bootinfo_get_reserved_mem(),
> + };
> struct rangeset *unalloc_mem;
> paddr_t start, end;
> - unsigned int i;
> + unsigned int i, j;
> int res;
>
> dt_dprintk("Find unallocated memory for extended regions\n");
> @@ -883,50 +885,20 @@ static int __init find_unallocated_memory(const struct
> kernel_info *kinfo,
> if ( !unalloc_mem )
> return -ENOMEM;
>
> - /* Start with all available RAM */
> - for ( i = 0; i < mem->nr_banks; i++ )
> - {
> - start = mem->bank[i].start;
> - end = mem->bank[i].start + mem->bank[i].size;
> - res = rangeset_add_range(unalloc_mem, PFN_DOWN(start),
> - PFN_DOWN(end - 1));
> - if ( res )
> - {
> - printk(XENLOG_ERR "Failed to add: %#"PRIpaddr"->%#"PRIpaddr"\n",
> - start, end);
> - goto out;
> - }
> - }
> -
> - /* Remove RAM assigned to Dom0 */
> - for ( i = 0; i < kinfo_mem->nr_banks; i++ )
> - {
> - start = kinfo_mem->bank[i].start;
> - end = kinfo_mem->bank[i].start + kinfo_mem->bank[i].size;
> - res = rangeset_remove_range(unalloc_mem, PFN_DOWN(start),
> - PFN_DOWN(end - 1));
> - if ( res )
> + for ( i = 0; i < ARRAY_SIZE(mem_banks); i++ )
> + for ( j = 0; j < mem_banks[i]->nr_banks; j++ )
It might be a matter of personal opinion, but I would actually prefer the
current code
that looks simpler/neater (the steps are clear) to me. I'd like to know other
maintainers opinion.
~Michal
