| Message ID | 1423058539-26403-27-git-send-email-parth.dixit@linaro.org |
|---|---|
| State | New |
| Headers | show |
Hi Parth, On 04/02/2015 14:02, parth.dixit@linaro.org wrote: > From: Parth Dixit <parth.dixit@linaro.org> > > For ACPI on arm device initialization is done by dom0 after parsing DSDT. For ACPI on ARM, device ... > xen requires mmio region information described in uefi tables > for mapping it to dom0. > > Signed-off-by: Parth Dixit <parth.dixit@linaro.org> > --- > xen/arch/arm/efi/efi-boot.h | 16 ++++++++++++++++ > xen/arch/arm/setup.c | 1 + > xen/include/asm-arm/setup.h | 1 + > 3 files changed, 18 insertions(+) > > diff --git a/xen/arch/arm/efi/efi-boot.h b/xen/arch/arm/efi/efi-boot.h > index 639942d..535f484 100644 > --- a/xen/arch/arm/efi/efi-boot.h > +++ b/xen/arch/arm/efi/efi-boot.h > @@ -127,6 +127,8 @@ static EFI_STATUS __init efi_process_memory_map_bootinfo(EFI_MEMORY_DESCRIPTOR * > { > int Index; > int i = 0; > + int j = 0; > + > EFI_MEMORY_DESCRIPTOR *desc_ptr = map; > > for ( Index = 0; Index < (mmap_size / desc_size); Index++ ) > @@ -145,10 +147,24 @@ static EFI_STATUS __init efi_process_memory_map_bootinfo(EFI_MEMORY_DESCRIPTOR * > break; > } > } > + else if ( desc_ptr->Type == EfiMemoryMappedIO > + || desc_ptr->Type == EfiMemoryMappedIOPortSpace ) I would turn the function into a switch case. if ... else if ... is really difficult to read and ... > + { > + acpi_mmio.bank[j].start = desc_ptr->PhysicalStart; > + acpi_mmio.bank[j].size = desc_ptr->NumberOfPages * EFI_PAGE_SIZE; > + if ( ++j >= NR_MEM_BANKS ) > + { > + PrintStr(L"Warning: All "); > + DisplayUint(NR_MEM_BANKS, -1); > + PrintStr(L" acpi meminfo mem banks exhausted.\r\n"); > + break; ... make this such problem appears. While the number of MMIO banks has expired, we should continue to loop to get the RAM regions. Leaving the loop will make less obvious the further issue (such as no memory bank found). > + } > + } > desc_ptr = NextMemoryDescriptor(desc_ptr, desc_size); > } > > bootinfo.mem.nr_banks = i; > + acpi_mmio.nr_banks = j; > return EFI_SUCCESS; > } > > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c > index 93c8a8a..930746b 100644 > --- a/xen/arch/arm/setup.c > +++ b/xen/arch/arm/setup.c > @@ -50,6 +50,7 @@ > #include <asm-arm/cputype.h> > > struct bootinfo __initdata bootinfo; #ifdef CONFIG_ACPI? > +struct meminfo __initdata acpi_mmio; > > struct cpuinfo_arm __read_mostly boot_cpu_data; > > diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h > index ba5a67d..5ea9ed6 100644 > --- a/xen/include/asm-arm/setup.h > +++ b/xen/include/asm-arm/setup.h > @@ -46,6 +46,7 @@ struct bootinfo { > }; > > extern struct bootinfo bootinfo; > +extern struct meminfo acpi_mmio; I'm not sure that it's the right things to use meminfo. It only has 64 banks... Could not we re-read at runtime the UEFI memory map? Regards,
On 06/02/2015 00:34, Stefano Stabellini wrote: >> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c >> index 93c8a8a..930746b 100644 >> --- a/xen/arch/arm/setup.c >> +++ b/xen/arch/arm/setup.c >> @@ -50,6 +50,7 @@ >> #include <asm-arm/cputype.h> >> >> struct bootinfo __initdata bootinfo; >> +struct meminfo __initdata acpi_mmio; >> >> struct cpuinfo_arm __read_mostly boot_cpu_data; >> >> diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h >> index ba5a67d..5ea9ed6 100644 >> --- a/xen/include/asm-arm/setup.h >> +++ b/xen/include/asm-arm/setup.h >> @@ -46,6 +46,7 @@ struct bootinfo { >> }; >> >> extern struct bootinfo bootinfo; >> +extern struct meminfo acpi_mmio; > > It might make sense to reuse bootinfo.mem. Do you mean by extending the meminfo structure with a flags indicating if it's a RAM or MMIO range? I guess it might be good, but I'm concerned about the static size of the array, 64 may not be suffisant. Regards,
Hi Stefano, On 06/02/2015 22:17, Stefano Stabellini wrote: > On Fri, 6 Feb 2015, Julien Grall wrote: >> On 06/02/2015 00:34, Stefano Stabellini wrote: >>>> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c >>>> index 93c8a8a..930746b 100644 >>>> --- a/xen/arch/arm/setup.c >>>> +++ b/xen/arch/arm/setup.c >>>> @@ -50,6 +50,7 @@ >>>> #include <asm-arm/cputype.h> >>>> >>>> struct bootinfo __initdata bootinfo; >>>> +struct meminfo __initdata acpi_mmio; >>>> >>>> struct cpuinfo_arm __read_mostly boot_cpu_data; >>>> >>>> diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h >>>> index ba5a67d..5ea9ed6 100644 >>>> --- a/xen/include/asm-arm/setup.h >>>> +++ b/xen/include/asm-arm/setup.h >>>> @@ -46,6 +46,7 @@ struct bootinfo { >>>> }; >>>> >>>> extern struct bootinfo bootinfo; >>>> +extern struct meminfo acpi_mmio; >>> >>> It might make sense to reuse bootinfo.mem. >> >> Do you mean by extending the meminfo structure with a flags indicating if it's >> a RAM or MMIO range? > > Right. We could avoid introducing acpi_mmio and instead storing the > banks info in bootinfo.mem, that I am guessing it would be unused on > acpi? It's used by EFI to store the memory bank. So we can't reuse it for another purpose. But we could extend the structure to add a type. Although, let's wait until we know if we can use UEFI to get the MMIO region. > >> I guess it might be good, but I'm concerned about the static size of the >> array, 64 may not be suffisant. > > Yes, you are right about that. Do we have any ideas on how many banks > are available on a few real systems? I have no ideas. > The alternative would be to create a list. At that time, we can't use the boot allocator (memory has not been populate). We would have to use the AllocatePages(). Regards,
diff --git a/xen/arch/arm/efi/efi-boot.h b/xen/arch/arm/efi/efi-boot.h index 639942d..535f484 100644 --- a/xen/arch/arm/efi/efi-boot.h +++ b/xen/arch/arm/efi/efi-boot.h @@ -127,6 +127,8 @@ static EFI_STATUS __init efi_process_memory_map_bootinfo(EFI_MEMORY_DESCRIPTOR * { int Index; int i = 0; + int j = 0; + EFI_MEMORY_DESCRIPTOR *desc_ptr = map; for ( Index = 0; Index < (mmap_size / desc_size); Index++ ) @@ -145,10 +147,24 @@ static EFI_STATUS __init efi_process_memory_map_bootinfo(EFI_MEMORY_DESCRIPTOR * break; } } + else if ( desc_ptr->Type == EfiMemoryMappedIO + || desc_ptr->Type == EfiMemoryMappedIOPortSpace ) + { + acpi_mmio.bank[j].start = desc_ptr->PhysicalStart; + acpi_mmio.bank[j].size = desc_ptr->NumberOfPages * EFI_PAGE_SIZE; + if ( ++j >= NR_MEM_BANKS ) + { + PrintStr(L"Warning: All "); + DisplayUint(NR_MEM_BANKS, -1); + PrintStr(L" acpi meminfo mem banks exhausted.\r\n"); + break; + } + } desc_ptr = NextMemoryDescriptor(desc_ptr, desc_size); } bootinfo.mem.nr_banks = i; + acpi_mmio.nr_banks = j; return EFI_SUCCESS; } diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c index 93c8a8a..930746b 100644 --- a/xen/arch/arm/setup.c +++ b/xen/arch/arm/setup.c @@ -50,6 +50,7 @@ #include <asm-arm/cputype.h> struct bootinfo __initdata bootinfo; +struct meminfo __initdata acpi_mmio; struct cpuinfo_arm __read_mostly boot_cpu_data; diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h index ba5a67d..5ea9ed6 100644 --- a/xen/include/asm-arm/setup.h +++ b/xen/include/asm-arm/setup.h @@ -46,6 +46,7 @@ struct bootinfo { }; extern struct bootinfo bootinfo; +extern struct meminfo acpi_mmio; void arch_init_memory(void);