Re: [Xen-devel] [PATCH v4 03/24] arm/acpi: Read acpi memory info from uefi

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Mon, 29 Feb 2016, Jan Beulich wrote:
> >>> On 28.02.16 at 12:18, <zhaoshenglong@xxxxxxxxxx> wrote:
> > From: Parth Dixit <parth.dixit@xxxxxxxxxx>
> > 
> > ACPI memory is seperate from conventional memory and should be marked
> > as reserved while passing to DOM0. Create a new meminfo structure to
> > store all the acpi tables listed in uefi.
> > 
> > Signed-off-by: Parth Dixit <parth.dixit@xxxxxxxxxx>
> > Signed-off-by: Shannon Zhao <shannon.zhao@xxxxxxxxxx>
> > Acked-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> 
> With this I'll commit as is, but I'd like to note that ...
> 
> > @@ -129,6 +132,9 @@ static EFI_STATUS __init 
> > efi_process_memory_map_bootinfo(EFI_MEMORY_DESCRIPTOR *
> >  {
> >      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++ )
> > @@ -148,10 +154,27 @@ static EFI_STATUS __init 
> > efi_process_memory_map_bootinfo(EFI_MEMORY_DESCRIPTOR *
> >              bootinfo.mem.bank[i].size = desc_ptr->NumberOfPages * 
> > EFI_PAGE_SIZE;
> >              ++i;
> >          }
> > +#if defined (CONFIG_ACPI) && defined (CONFIG_ARM)
> > +        else if ( desc_ptr->Type == EfiACPIReclaimMemory )
> > +        {
> > +            if ( j >= NR_MEM_BANKS )
> > +            {
> > +                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;
> > +#if defined (CONFIG_ACPI) && defined (CONFIG_ARM)
> > +    acpi_mem.nr_banks = j;
> > +#endif
> >      return EFI_SUCCESS;
> >  }
> 
> ... the three #ifdef-s here aren't consistent.
 
Well spotted, the CONFIG_ARM is redundant, could you please remove it
while committing?

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel