diff mbox series

[Xen-devel,5/8] xen/arm: efi: Avoid duplicating the addition of a new bank

Message ID 1486149538-20432-10-git-send-email-julien.grall@arm.com
State Superseded
Headers show
Series xen/arm: Fix and clean-up for ACPI and EFI | expand

Commit Message

Julien Grall Feb. 3, 2017, 7:18 p.m. UTC 
The code to add a new bank is duplicated twice. Add a new helper that
checks if the maximum of bank has not reached and adds the bank.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/efi/efi-boot.h | 34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)

Comments

Stefano Stabellini Feb. 16, 2017, 1:47 a.m. UTC | #1 
On Fri, 3 Feb 2017, Julien Grall wrote:
> The code to add a new bank is duplicated twice. Add a new helper that
> checks if the maximum of bank has not reached and adds the bank.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>  xen/arch/arm/efi/efi-boot.h | 34 ++++++++++++++++++----------------
>  1 file changed, 18 insertions(+), 16 deletions(-)
> 
> diff --git a/xen/arch/arm/efi/efi-boot.h b/xen/arch/arm/efi/efi-boot.h
> index 045d6ce..757d9c6 100644
> --- a/xen/arch/arm/efi/efi-boot.h
> +++ b/xen/arch/arm/efi/efi-boot.h
> @@ -124,15 +124,27 @@ static void __init *lookup_fdt_config_table(EFI_SYSTEM_TABLE *sys_table)
>      return fdt;
>  }
>  
> +static bool meminfo_add_bank(struct meminfo *mem, EFI_MEMORY_DESCRIPTOR *desc)
> +{
> +    struct membank *bank;
> +
> +    if ( mem->nr_banks > NR_MEM_BANKS )
> +        return false;
> +
> +    bank = &mem->bank[mem->nr_banks];
> +    bank->start = desc->PhysicalStart;
> +    bank->size = desc->NumberOfPages * EFI_PAGE_SIZE;
> +
> +    mem->nr_banks++;
> +
> +    return true;
> +}
> +
>  static EFI_STATUS __init efi_process_memory_map_bootinfo(EFI_MEMORY_DESCRIPTOR *map,
>                                                  UINTN mmap_size,
>                                                  UINTN desc_size)
>  {
>      int Index;
> -    int i = 0;
> -#ifdef CONFIG_ACPI
> -    int j = 0;
> -#endif
>      EFI_MEMORY_DESCRIPTOR *desc_ptr = map;
>  
>      for ( Index = 0; Index < (mmap_size / desc_size); Index++ )
> @@ -142,37 +154,27 @@ static EFI_STATUS __init efi_process_memory_map_bootinfo(EFI_MEMORY_DESCRIPTOR *
>                (desc_ptr->Type == EfiBootServicesCode ||
>                 desc_ptr->Type == EfiBootServicesData)) )
>          {
> -            if ( i >= NR_MEM_BANKS )
> +            if ( !meminfo_add_bank(&bootinfo.mem, desc_ptr) )
>              {
>                  PrintStr(L"Warning: All " __stringify(NR_MEM_BANKS)
>                            " bootinfo mem banks exhausted.\r\n");
>                  break;
>              }
> -            bootinfo.mem.bank[i].start = desc_ptr->PhysicalStart;
> -            bootinfo.mem.bank[i].size = desc_ptr->NumberOfPages * EFI_PAGE_SIZE;
> -            ++i;
>          }
>  #ifdef CONFIG_ACPI
>          else if ( desc_ptr->Type == EfiACPIReclaimMemory )
>          {
> -            if ( j >= NR_MEM_BANKS )
> +            if ( !meminfo_add_bank(&acpi_mem, desc_ptr) )
>              {
>                  PrintStr(L"Error: All " __stringify(NR_MEM_BANKS)
>                            " acpi meminfo mem banks exhausted.\r\n");
>                  return EFI_LOAD_ERROR;
>              }
> -            acpi_mem.bank[j].start = desc_ptr->PhysicalStart;
> -            acpi_mem.bank[j].size  = desc_ptr->NumberOfPages * EFI_PAGE_SIZE;
> -            ++j;
>          }
>  #endif
>          desc_ptr = NextMemoryDescriptor(desc_ptr, desc_size);
>      }
>  
> -    bootinfo.mem.nr_banks = i;
> -#ifdef CONFIG_ACPI
> -    acpi_mem.nr_banks = j;
> -#endif
>      return EFI_SUCCESS;
>  }
>  
> -- 
> 1.9.1
>
Stefano Stabellini Feb. 16, 2017, 1:49 a.m. UTC | #2 
On Wed, 15 Feb 2017, Stefano Stabellini wrote:
> On Fri, 3 Feb 2017, Julien Grall wrote:
> > The code to add a new bank is duplicated twice. Add a new helper that
> > checks if the maximum of bank has not reached and adds the bank.
> > 
> > Signed-off-by: Julien Grall <julien.grall@arm.com>
> 
> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
> 
> 
> > ---
> >  xen/arch/arm/efi/efi-boot.h | 34 ++++++++++++++++++----------------
> >  1 file changed, 18 insertions(+), 16 deletions(-)
> > 
> > diff --git a/xen/arch/arm/efi/efi-boot.h b/xen/arch/arm/efi/efi-boot.h
> > index 045d6ce..757d9c6 100644
> > --- a/xen/arch/arm/efi/efi-boot.h
> > +++ b/xen/arch/arm/efi/efi-boot.h
> > @@ -124,15 +124,27 @@ static void __init *lookup_fdt_config_table(EFI_SYSTEM_TABLE *sys_table)
> >      return fdt;
> >  }
> >  
> > +static bool meminfo_add_bank(struct meminfo *mem, EFI_MEMORY_DESCRIPTOR *desc)

actually this function should be __init, right?


> > +{
> > +    struct membank *bank;
> > +
> > +    if ( mem->nr_banks > NR_MEM_BANKS )
> > +        return false;
> > +
> > +    bank = &mem->bank[mem->nr_banks];
> > +    bank->start = desc->PhysicalStart;
> > +    bank->size = desc->NumberOfPages * EFI_PAGE_SIZE;
> > +
> > +    mem->nr_banks++;
> > +
> > +    return true;
> > +}
> > +
> >  static EFI_STATUS __init efi_process_memory_map_bootinfo(EFI_MEMORY_DESCRIPTOR *map,
> >                                                  UINTN mmap_size,
> >                                                  UINTN desc_size)
> >  {
> >      int Index;
> > -    int i = 0;
> > -#ifdef CONFIG_ACPI
> > -    int j = 0;
> > -#endif
> >      EFI_MEMORY_DESCRIPTOR *desc_ptr = map;
> >  
> >      for ( Index = 0; Index < (mmap_size / desc_size); Index++ )
> > @@ -142,37 +154,27 @@ static EFI_STATUS __init efi_process_memory_map_bootinfo(EFI_MEMORY_DESCRIPTOR *
> >                (desc_ptr->Type == EfiBootServicesCode ||
> >                 desc_ptr->Type == EfiBootServicesData)) )
> >          {
> > -            if ( i >= NR_MEM_BANKS )
> > +            if ( !meminfo_add_bank(&bootinfo.mem, desc_ptr) )
> >              {
> >                  PrintStr(L"Warning: All " __stringify(NR_MEM_BANKS)
> >                            " bootinfo mem banks exhausted.\r\n");
> >                  break;
> >              }
> > -            bootinfo.mem.bank[i].start = desc_ptr->PhysicalStart;
> > -            bootinfo.mem.bank[i].size = desc_ptr->NumberOfPages * EFI_PAGE_SIZE;
> > -            ++i;
> >          }
> >  #ifdef CONFIG_ACPI
> >          else if ( desc_ptr->Type == EfiACPIReclaimMemory )
> >          {
> > -            if ( j >= NR_MEM_BANKS )
> > +            if ( !meminfo_add_bank(&acpi_mem, desc_ptr) )
> >              {
> >                  PrintStr(L"Error: All " __stringify(NR_MEM_BANKS)
> >                            " acpi meminfo mem banks exhausted.\r\n");
> >                  return EFI_LOAD_ERROR;
> >              }
> > -            acpi_mem.bank[j].start = desc_ptr->PhysicalStart;
> > -            acpi_mem.bank[j].size  = desc_ptr->NumberOfPages * EFI_PAGE_SIZE;
> > -            ++j;
> >          }
> >  #endif
> >          desc_ptr = NextMemoryDescriptor(desc_ptr, desc_size);
> >      }
> >  
> > -    bootinfo.mem.nr_banks = i;
> > -#ifdef CONFIG_ACPI
> > -    acpi_mem.nr_banks = j;
> > -#endif
> >      return EFI_SUCCESS;
> >  }
> >  
> > -- 
> > 1.9.1
> > 
>
Julien Grall March 3, 2017, 6:31 p.m. UTC | #3 
Hi Stefano,

On 16/02/17 01:49, Stefano Stabellini wrote:
> On Wed, 15 Feb 2017, Stefano Stabellini wrote:
>> On Fri, 3 Feb 2017, Julien Grall wrote:
>>> The code to add a new bank is duplicated twice. Add a new helper that
>>> checks if the maximum of bank has not reached and adds the bank.
>>>
>>> Signed-off-by: Julien Grall <julien.grall@arm.com>
>>
>> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
>>
>>
>>> ---
>>>  xen/arch/arm/efi/efi-boot.h | 34 ++++++++++++++++++----------------
>>>  1 file changed, 18 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/xen/arch/arm/efi/efi-boot.h b/xen/arch/arm/efi/efi-boot.h
>>> index 045d6ce..757d9c6 100644
>>> --- a/xen/arch/arm/efi/efi-boot.h
>>> +++ b/xen/arch/arm/efi/efi-boot.h
>>> @@ -124,15 +124,27 @@ static void __init *lookup_fdt_config_table(EFI_SYSTEM_TABLE *sys_table)
>>>      return fdt;
>>>  }
>>>
>>> +static bool meminfo_add_bank(struct meminfo *mem, EFI_MEMORY_DESCRIPTOR *desc)
>
> actually this function should be __init, right?

Yes it should, I will fix it in the next version.

Cheers,
diff mbox series

Patch

diff --git a/xen/arch/arm/efi/efi-boot.h b/xen/arch/arm/efi/efi-boot.h
index 045d6ce..757d9c6 100644
--- a/xen/arch/arm/efi/efi-boot.h
+++ b/xen/arch/arm/efi/efi-boot.h
@@ -124,15 +124,27 @@  static void __init *lookup_fdt_config_table(EFI_SYSTEM_TABLE *sys_table)
     return fdt;
 }
 
+static bool meminfo_add_bank(struct meminfo *mem, EFI_MEMORY_DESCRIPTOR *desc)
+{
+    struct membank *bank;
+
+    if ( mem->nr_banks > NR_MEM_BANKS )
+        return false;
+
+    bank = &mem->bank[mem->nr_banks];
+    bank->start = desc->PhysicalStart;
+    bank->size = desc->NumberOfPages * EFI_PAGE_SIZE;
+
+    mem->nr_banks++;
+
+    return true;
+}
+
 static EFI_STATUS __init efi_process_memory_map_bootinfo(EFI_MEMORY_DESCRIPTOR *map,
                                                 UINTN mmap_size,
                                                 UINTN desc_size)
 {
     int Index;
-    int i = 0;
-#ifdef CONFIG_ACPI
-    int j = 0;
-#endif
     EFI_MEMORY_DESCRIPTOR *desc_ptr = map;
 
     for ( Index = 0; Index < (mmap_size / desc_size); Index++ )
@@ -142,37 +154,27 @@  static EFI_STATUS __init efi_process_memory_map_bootinfo(EFI_MEMORY_DESCRIPTOR *
               (desc_ptr->Type == EfiBootServicesCode ||
                desc_ptr->Type == EfiBootServicesData)) )
         {
-            if ( i >= NR_MEM_BANKS )
+            if ( !meminfo_add_bank(&bootinfo.mem, desc_ptr) )
             {
                 PrintStr(L"Warning: All " __stringify(NR_MEM_BANKS)
                           " bootinfo mem banks exhausted.\r\n");
                 break;
             }
-            bootinfo.mem.bank[i].start = desc_ptr->PhysicalStart;
-            bootinfo.mem.bank[i].size = desc_ptr->NumberOfPages * EFI_PAGE_SIZE;
-            ++i;
         }
 #ifdef CONFIG_ACPI
         else if ( desc_ptr->Type == EfiACPIReclaimMemory )
         {
-            if ( j >= NR_MEM_BANKS )
+            if ( !meminfo_add_bank(&acpi_mem, desc_ptr) )
             {
                 PrintStr(L"Error: All " __stringify(NR_MEM_BANKS)
                           " acpi meminfo mem banks exhausted.\r\n");
                 return EFI_LOAD_ERROR;
             }
-            acpi_mem.bank[j].start = desc_ptr->PhysicalStart;
-            acpi_mem.bank[j].size  = desc_ptr->NumberOfPages * EFI_PAGE_SIZE;
-            ++j;
         }
 #endif
         desc_ptr = NextMemoryDescriptor(desc_ptr, desc_size);
     }
 
-    bootinfo.mem.nr_banks = i;
-#ifdef CONFIG_ACPI
-    acpi_mem.nr_banks = j;
-#endif
     return EFI_SUCCESS;
 }