diff mbox

[v4,15/24] arm/acpi: Prepare EFI memory descriptor for Dom0

Message ID 1456658360-16080-16-git-send-email-zhaoshenglong@huawei.com (mailing list archive)
State New, archived
Headers show

Commit Message

Shannon Zhao Feb. 28, 2016, 11:19 a.m. UTC 
From: Shannon Zhao <shannon.zhao@linaro.org>

Create a few EFI memory descriptors to tell Dom0 the RAM region
information, ACPI table regions and EFI tables reserved resions.

Cc: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Parth Dixit <parth.dixit@linaro.org>
Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
v4: use a single descriptor for new created tables
---
 xen/arch/arm/domain_build.c |  2 ++
 xen/common/efi/boot.c       | 38 ++++++++++++++++++++++++++++++++++++++
 xen/include/asm-arm/setup.h |  5 +++++
 3 files changed, 45 insertions(+)

Comments

Stefano Stabellini Feb. 29, 2016, 2:37 p.m. UTC | #1 
On Sun, 28 Feb 2016, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> Create a few EFI memory descriptors to tell Dom0 the RAM region
> information, ACPI table regions and EFI tables reserved resions.
> 
> Cc: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Parth Dixit <parth.dixit@linaro.org>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
> v4: use a single descriptor for new created tables
> ---
>  xen/arch/arm/domain_build.c |  2 ++
>  xen/common/efi/boot.c       | 38 ++++++++++++++++++++++++++++++++++++++
>  xen/include/asm-arm/setup.h |  5 +++++
>  3 files changed, 45 insertions(+)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 09f9770..1ec6271 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -1688,6 +1688,8 @@ static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
>      acpi_map_other_tables(d);
>      acpi_create_efi_system_table(d->arch.efi_acpi_gpa, d->arch.efi_acpi_table,
>                                   tbl_add);
> +    acpi_create_efi_mmap_table(d->arch.efi_acpi_gpa, d->arch.efi_acpi_len,
> +                               d->arch.efi_acpi_table, &kinfo->mem, tbl_add);
>  
>      return 0;
>  }
> diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
> index 238c5fd..b8d7409 100644
> --- a/xen/common/efi/boot.c
> +++ b/xen/common/efi/boot.c
> @@ -1233,6 +1233,44 @@ void __init acpi_create_efi_system_table(paddr_t paddr, void *efi_acpi_table,
>      tbl_add[TBL_EFIT].start = table_addr;
>      tbl_add[TBL_EFIT].size = table_size;
>  }
> +
> +void __init acpi_create_efi_mmap_table(paddr_t paddr, paddr_t size,
> +                                       void *efi_acpi_table,
> +                                       const struct meminfo *mem,
> +                                       struct membank tbl_add[])

This function probably belongs to xen/arch/arm/domain_build.c or
arch/arm/efi


> +{
> +    EFI_MEMORY_DESCRIPTOR *memory_map;
> +    int i, offset;
> +    u8 *base_ptr;
> +
> +    tbl_add[TBL_MMAP].start = paddr + acpi_get_table_offset(tbl_add, TBL_MMAP);
> +    tbl_add[TBL_MMAP].size = sizeof(EFI_MEMORY_DESCRIPTOR)
> +                             * (mem->nr_banks + acpi_mem.nr_banks + 1);

I don't like the fact that we are getting the number of memory banks
from a parameter and the number of acpi banks from a global variable.
Can we pass in acpi_mem.nr_banks too?


> +    base_ptr = efi_acpi_table + acpi_get_table_offset(tbl_add, TBL_MMAP);
> +    memory_map = (EFI_MEMORY_DESCRIPTOR *)(base_ptr);
> +
> +    offset = 0;
> +    for( i = 0; i < mem->nr_banks; i++, offset++ )
> +    {
> +        memory_map[offset].Type = EfiConventionalMemory;
> +        memory_map[offset].PhysicalStart = mem->bank[i].start;
> +        memory_map[offset].NumberOfPages = PFN_UP(mem->bank[i].size);
> +        memory_map[offset].Attribute = EFI_MEMORY_WB;
> +    }
> +
> +    for( i = 0; i < acpi_mem.nr_banks; i++, offset++ )
> +    {
> +        memory_map[offset].Type = EfiACPIReclaimMemory;
> +        memory_map[offset].PhysicalStart = acpi_mem.bank[i].start;
> +        memory_map[offset].NumberOfPages = PFN_UP(acpi_mem.bank[i].size);
> +        memory_map[offset].Attribute = EFI_MEMORY_WB;
> +    }
> +
> +    memory_map[offset].Type = EfiACPIReclaimMemory;
> +    memory_map[offset].PhysicalStart = paddr;
> +    memory_map[offset].NumberOfPages = PFN_UP(size);
> +    memory_map[offset].Attribute = EFI_MEMORY_WB;
> +}
>  #endif
>  
>  #ifndef CONFIG_ARM /* TODO - runtime service support */
> diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h
> index 2d65796..af5a038 100644
> --- a/xen/include/asm-arm/setup.h
> +++ b/xen/include/asm-arm/setup.h
> @@ -56,6 +56,11 @@ int estimate_efi_size(int mem_nr_banks);
>  void acpi_create_efi_system_table(paddr_t paddr, void *efi_acpi_table,
>                                    struct membank tbl_add[]);
>  
> +void acpi_create_efi_mmap_table(paddr_t paddr, paddr_t size,
> +                                void *efi_acpi_table,
> +                                const struct meminfo *mem,
> +                                struct membank tbl_add[]);
> +
>  int construct_dom0(struct domain *d);
>  
>  void discard_initial_modules(void);
Shannon Zhao March 1, 2016, 3 a.m. UTC | #2 
On 2016/2/29 22:37, Stefano Stabellini wrote:
> On Sun, 28 Feb 2016, Shannon Zhao wrote:
>> > From: Shannon Zhao <shannon.zhao@linaro.org>
>> > 
>> > Create a few EFI memory descriptors to tell Dom0 the RAM region
>> > information, ACPI table regions and EFI tables reserved resions.
>> > 
>> > Cc: Jan Beulich <jbeulich@suse.com>
>> > Signed-off-by: Parth Dixit <parth.dixit@linaro.org>
>> > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>> > ---
>> > v4: use a single descriptor for new created tables
>> > ---
>> >  xen/arch/arm/domain_build.c |  2 ++
>> >  xen/common/efi/boot.c       | 38 ++++++++++++++++++++++++++++++++++++++
>> >  xen/include/asm-arm/setup.h |  5 +++++
>> >  3 files changed, 45 insertions(+)
>> > 
>> > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>> > index 09f9770..1ec6271 100644
>> > --- a/xen/arch/arm/domain_build.c
>> > +++ b/xen/arch/arm/domain_build.c
>> > @@ -1688,6 +1688,8 @@ static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
>> >      acpi_map_other_tables(d);
>> >      acpi_create_efi_system_table(d->arch.efi_acpi_gpa, d->arch.efi_acpi_table,
>> >                                   tbl_add);
>> > +    acpi_create_efi_mmap_table(d->arch.efi_acpi_gpa, d->arch.efi_acpi_len,
>> > +                               d->arch.efi_acpi_table, &kinfo->mem, tbl_add);
>> >  
>> >      return 0;
>> >  }
>> > diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
>> > index 238c5fd..b8d7409 100644
>> > --- a/xen/common/efi/boot.c
>> > +++ b/xen/common/efi/boot.c
>> > @@ -1233,6 +1233,44 @@ void __init acpi_create_efi_system_table(paddr_t paddr, void *efi_acpi_table,
>> >      tbl_add[TBL_EFIT].start = table_addr;
>> >      tbl_add[TBL_EFIT].size = table_size;
>> >  }
>> > +
>> > +void __init acpi_create_efi_mmap_table(paddr_t paddr, paddr_t size,
>> > +                                       void *efi_acpi_table,
>> > +                                       const struct meminfo *mem,
>> > +                                       struct membank tbl_add[])
> This function probably belongs to xen/arch/arm/domain_build.c or
> arch/arm/efi
> 
As said before at previous patch set, placing these functions in
common/efi/boot.c is to use the EFI_MEMORY_DESCRIPTOR etc.

See
http://lists.xenproject.org/archives/html/xen-devel/2016-01/msg00097.html

But I agree that it cloud be moved to a new file(e.g. dom0-efi.c) under
arch/arm/efi.

> 
>> > +{
>> > +    EFI_MEMORY_DESCRIPTOR *memory_map;
>> > +    int i, offset;
>> > +    u8 *base_ptr;
>> > +
>> > +    tbl_add[TBL_MMAP].start = paddr + acpi_get_table_offset(tbl_add, TBL_MMAP);
>> > +    tbl_add[TBL_MMAP].size = sizeof(EFI_MEMORY_DESCRIPTOR)
>> > +                             * (mem->nr_banks + acpi_mem.nr_banks + 1);
> I don't like the fact that we are getting the number of memory banks
> from a parameter and the number of acpi banks from a global variable.
> Can we pass in acpi_mem.nr_banks too?
> 
While acpi_mem is defined in efi-boot.h, the caller in domain_build.c of
acpi_create_efi_mmap_table can't access acpi_mem since if we include
efi-boot.h in domain_build.c there are some compiling errors like above.

Thanks,
Stefano Stabellini March 1, 2016, 2:57 p.m. UTC | #3 
On Tue, 1 Mar 2016, Shannon Zhao wrote:
> On 2016/2/29 22:37, Stefano Stabellini wrote:
> > On Sun, 28 Feb 2016, Shannon Zhao wrote:
> >> > From: Shannon Zhao <shannon.zhao@linaro.org>
> >> > 
> >> > Create a few EFI memory descriptors to tell Dom0 the RAM region
> >> > information, ACPI table regions and EFI tables reserved resions.
> >> > 
> >> > Cc: Jan Beulich <jbeulich@suse.com>
> >> > Signed-off-by: Parth Dixit <parth.dixit@linaro.org>
> >> > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> >> > ---
> >> > v4: use a single descriptor for new created tables
> >> > ---
> >> >  xen/arch/arm/domain_build.c |  2 ++
> >> >  xen/common/efi/boot.c       | 38 ++++++++++++++++++++++++++++++++++++++
> >> >  xen/include/asm-arm/setup.h |  5 +++++
> >> >  3 files changed, 45 insertions(+)
> >> > 
> >> > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> >> > index 09f9770..1ec6271 100644
> >> > --- a/xen/arch/arm/domain_build.c
> >> > +++ b/xen/arch/arm/domain_build.c
> >> > @@ -1688,6 +1688,8 @@ static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
> >> >      acpi_map_other_tables(d);
> >> >      acpi_create_efi_system_table(d->arch.efi_acpi_gpa, d->arch.efi_acpi_table,
> >> >                                   tbl_add);
> >> > +    acpi_create_efi_mmap_table(d->arch.efi_acpi_gpa, d->arch.efi_acpi_len,
> >> > +                               d->arch.efi_acpi_table, &kinfo->mem, tbl_add);
> >> >  
> >> >      return 0;
> >> >  }
> >> > diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
> >> > index 238c5fd..b8d7409 100644
> >> > --- a/xen/common/efi/boot.c
> >> > +++ b/xen/common/efi/boot.c
> >> > @@ -1233,6 +1233,44 @@ void __init acpi_create_efi_system_table(paddr_t paddr, void *efi_acpi_table,
> >> >      tbl_add[TBL_EFIT].start = table_addr;
> >> >      tbl_add[TBL_EFIT].size = table_size;
> >> >  }
> >> > +
> >> > +void __init acpi_create_efi_mmap_table(paddr_t paddr, paddr_t size,
> >> > +                                       void *efi_acpi_table,
> >> > +                                       const struct meminfo *mem,
> >> > +                                       struct membank tbl_add[])
> > This function probably belongs to xen/arch/arm/domain_build.c or
> > arch/arm/efi
> > 
> As said before at previous patch set, placing these functions in
> common/efi/boot.c is to use the EFI_MEMORY_DESCRIPTOR etc.
> 
> See
> http://lists.xenproject.org/archives/html/xen-devel/2016-01/msg00097.html

Sorry Shannon, I forgot about it.


> But I agree that it cloud be moved to a new file(e.g. dom0-efi.c) under
> arch/arm/efi.

Given the limitations, it would be acceptable to have the function here,
but given that this is about acpi and only useful on ARM, adding the
function to a file under arch/arm/efi would be even better.


> >> > +{
> >> > +    EFI_MEMORY_DESCRIPTOR *memory_map;
> >> > +    int i, offset;
> >> > +    u8 *base_ptr;
> >> > +
> >> > +    tbl_add[TBL_MMAP].start = paddr + acpi_get_table_offset(tbl_add, TBL_MMAP);
> >> > +    tbl_add[TBL_MMAP].size = sizeof(EFI_MEMORY_DESCRIPTOR)
> >> > +                             * (mem->nr_banks + acpi_mem.nr_banks + 1);
> > I don't like the fact that we are getting the number of memory banks
> > from a parameter and the number of acpi banks from a global variable.
> > Can we pass in acpi_mem.nr_banks too?
> > 
> While acpi_mem is defined in efi-boot.h, the caller in domain_build.c of
> acpi_create_efi_mmap_table can't access acpi_mem since if we include
> efi-boot.h in domain_build.c there are some compiling errors like above.

All right, it is not very nice, but I cannot think of a better way of
doing it right now.
Jan Beulich March 1, 2016, 3:07 p.m. UTC | #4 
>>> On 01.03.16 at 15:57, <stefano.stabellini@eu.citrix.com> wrote:
> On Tue, 1 Mar 2016, Shannon Zhao wrote:
>> On 2016/2/29 22:37, Stefano Stabellini wrote:
>> > On Sun, 28 Feb 2016, Shannon Zhao wrote:
>> >> > --- a/xen/common/efi/boot.c
>> >> > +++ b/xen/common/efi/boot.c
>> >> > @@ -1233,6 +1233,44 @@ void __init acpi_create_efi_system_table(paddr_t paddr, void *efi_acpi_table,
>> >> >      tbl_add[TBL_EFIT].start = table_addr;
>> >> >      tbl_add[TBL_EFIT].size = table_size;
>> >> >  }
>> >> > +
>> >> > +void __init acpi_create_efi_mmap_table(paddr_t paddr, paddr_t size,
>> >> > +                                       void *efi_acpi_table,
>> >> > +                                       const struct meminfo *mem,
>> >> > +                                       struct membank tbl_add[])
>> > This function probably belongs to xen/arch/arm/domain_build.c or
>> > arch/arm/efi
>> > 
>> As said before at previous patch set, placing these functions in
>> common/efi/boot.c is to use the EFI_MEMORY_DESCRIPTOR etc.
>> 
>> See
>> http://lists.xenproject.org/archives/html/xen-devel/2016-01/msg00097.html 
> 
> Sorry Shannon, I forgot about it.
> 
> 
>> But I agree that it cloud be moved to a new file(e.g. dom0-efi.c) under
>> arch/arm/efi.
> 
> Given the limitations, it would be acceptable to have the function here,

Afaic, having it in xen/common/efi/ is a no-go (we be acceptable
only it we were really just taking of one relatively small function,
but more gets added here by other patches), hence ...

> but given that this is about acpi and only useful on ARM, adding the
> function to a file under arch/arm/efi would be even better.

... this seems the only viable alternative.

Jan
diff mbox

Patch

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 09f9770..1ec6271 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1688,6 +1688,8 @@  static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
     acpi_map_other_tables(d);
     acpi_create_efi_system_table(d->arch.efi_acpi_gpa, d->arch.efi_acpi_table,
                                  tbl_add);
+    acpi_create_efi_mmap_table(d->arch.efi_acpi_gpa, d->arch.efi_acpi_len,
+                               d->arch.efi_acpi_table, &kinfo->mem, tbl_add);
 
     return 0;
 }
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 238c5fd..b8d7409 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -1233,6 +1233,44 @@  void __init acpi_create_efi_system_table(paddr_t paddr, void *efi_acpi_table,
     tbl_add[TBL_EFIT].start = table_addr;
     tbl_add[TBL_EFIT].size = table_size;
 }
+
+void __init acpi_create_efi_mmap_table(paddr_t paddr, paddr_t size,
+                                       void *efi_acpi_table,
+                                       const struct meminfo *mem,
+                                       struct membank tbl_add[])
+{
+    EFI_MEMORY_DESCRIPTOR *memory_map;
+    int i, offset;
+    u8 *base_ptr;
+
+    tbl_add[TBL_MMAP].start = paddr + acpi_get_table_offset(tbl_add, TBL_MMAP);
+    tbl_add[TBL_MMAP].size = sizeof(EFI_MEMORY_DESCRIPTOR)
+                             * (mem->nr_banks + acpi_mem.nr_banks + 1);
+    base_ptr = efi_acpi_table + acpi_get_table_offset(tbl_add, TBL_MMAP);
+    memory_map = (EFI_MEMORY_DESCRIPTOR *)(base_ptr);
+
+    offset = 0;
+    for( i = 0; i < mem->nr_banks; i++, offset++ )
+    {
+        memory_map[offset].Type = EfiConventionalMemory;
+        memory_map[offset].PhysicalStart = mem->bank[i].start;
+        memory_map[offset].NumberOfPages = PFN_UP(mem->bank[i].size);
+        memory_map[offset].Attribute = EFI_MEMORY_WB;
+    }
+
+    for( i = 0; i < acpi_mem.nr_banks; i++, offset++ )
+    {
+        memory_map[offset].Type = EfiACPIReclaimMemory;
+        memory_map[offset].PhysicalStart = acpi_mem.bank[i].start;
+        memory_map[offset].NumberOfPages = PFN_UP(acpi_mem.bank[i].size);
+        memory_map[offset].Attribute = EFI_MEMORY_WB;
+    }
+
+    memory_map[offset].Type = EfiACPIReclaimMemory;
+    memory_map[offset].PhysicalStart = paddr;
+    memory_map[offset].NumberOfPages = PFN_UP(size);
+    memory_map[offset].Attribute = EFI_MEMORY_WB;
+}
 #endif
 
 #ifndef CONFIG_ARM /* TODO - runtime service support */
diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h
index 2d65796..af5a038 100644
--- a/xen/include/asm-arm/setup.h
+++ b/xen/include/asm-arm/setup.h
@@ -56,6 +56,11 @@  int estimate_efi_size(int mem_nr_banks);
 void acpi_create_efi_system_table(paddr_t paddr, void *efi_acpi_table,
                                   struct membank tbl_add[]);
 
+void acpi_create_efi_mmap_table(paddr_t paddr, paddr_t size,
+                                void *efi_acpi_table,
+                                const struct meminfo *mem,
+                                struct membank tbl_add[]);
+
 int construct_dom0(struct domain *d);
 
 void discard_initial_modules(void);