[Xen-devel] [PATCH v3 40/62] arm/acpi: Estimate memory required for acpi/efi tables

2015-11-17 Thread shannon . zhao
From: Shannon Zhao 

Estimate the memory required for loading acpi/efi tables in Dom0. Alloc
the pages to store the new created EFI and ACPI tables and free these
pages when destroying domain.

Signed-off-by: Parth Dixit 
Signed-off-by: Shannon Zhao 
---
 xen/arch/arm/domain.c   |  4 +++
 xen/arch/arm/domain_build.c | 80 -
 xen/common/efi/boot.c   | 21 
 xen/include/asm-arm/setup.h |  2 ++
 4 files changed, 106 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 880d0a6..10c58c4 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -638,6 +638,10 @@ void arch_domain_destroy(struct domain *d)
 domain_vgic_free(d);
 domain_vuart_free(d);
 free_xenheap_page(d->shared_info);
+#ifdef CONFIG_ACPI
+free_xenheap_pages(d->arch.efi_acpi_table,
+   get_order_from_bytes(d->arch.efi_acpi_len));
+#endif
 }
 
 void arch_domain_shutdown(struct domain *d)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 0c3441a..b5ed44c 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -12,6 +12,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -1354,6 +1356,78 @@ static int prepare_dtb(struct domain *d, struct 
kernel_info *kinfo)
 return -EINVAL;
 }
 
+#ifdef CONFIG_ACPI
+static int estimate_acpi_efi_size(struct domain *d, struct kernel_info *kinfo)
+{
+u64 efi_size, acpi_size = 0, addr;
+u32 madt_size;
+struct acpi_table_rsdp *rsdp_tbl;
+struct acpi_table_header *table = NULL;
+
+efi_size = estimate_efi_size(kinfo->mem.nr_banks);
+
+acpi_size += PAGE_ALIGN(sizeof(struct acpi_table_fadt));
+acpi_size += PAGE_ALIGN(sizeof(struct acpi_table_stao));
+
+madt_size = sizeof(struct acpi_table_madt)
++ sizeof(struct acpi_madt_generic_interrupt) * d->max_vcpus
++ sizeof(struct acpi_madt_generic_distributor);
+if ( d->arch.vgic.version == GIC_V3 )
+madt_size += sizeof(struct acpi_madt_generic_redistributor)
+ * d->arch.vgic.nr_regions;
+acpi_size += PAGE_ALIGN(madt_size);
+
+addr = acpi_os_get_root_pointer();
+if ( !addr )
+{
+printk("Unable to get acpi root pointer\n");
+return -EINVAL;
+}
+rsdp_tbl = acpi_os_map_memory(addr, sizeof(struct acpi_table_rsdp));
+table = acpi_os_map_memory(rsdp_tbl->xsdt_physical_address,
+   sizeof(struct acpi_table_header));
+acpi_size += PAGE_ALIGN(table->length + sizeof(u64));
+acpi_os_unmap_memory(table, sizeof(struct acpi_table_header));
+acpi_os_unmap_memory(rsdp_tbl, sizeof(struct acpi_table_rsdp));
+
+acpi_size += PAGE_ALIGN(sizeof(struct acpi_table_rsdp));
+d->arch.efi_acpi_len = PAGE_ALIGN(efi_size) + PAGE_ALIGN(acpi_size);
+
+return 0;
+}
+
+static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
+{
+int rc = 0;
+int order;
+
+rc = estimate_acpi_efi_size(d, kinfo);
+if ( rc != 0 )
+return rc;
+
+order = get_order_from_bytes(d->arch.efi_acpi_len);
+d->arch.efi_acpi_table = alloc_xenheap_pages(order, 0);
+if ( d->arch.efi_acpi_table == NULL )
+{
+printk("unable to allocate memory!\n");
+return -1;
+}
+memset(d->arch.efi_acpi_table, 0, d->arch.efi_acpi_len);
+
+/* For ACPI, Dom0 doesn't use kinfo->gnttab_start to get the grant table
+ * region. So we use it as the ACPI table mapped address. */
+d->arch.efi_acpi_gpa = kinfo->gnttab_start;
+
+return 0;
+}
+#else
+static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
+{
+/* Only booting with ACPI will hit here */
+BUG_ON(1);
+return -EINVAL;
+}
+#endif
 static void dtb_load(struct kernel_info *kinfo)
 {
 void * __user dtb_virt = (void * __user)(register_t)kinfo->dtb_paddr;
@@ -1540,7 +1614,11 @@ int construct_dom0(struct domain *d)
 allocate_memory(d, &kinfo);
 find_gnttab_region(d, &kinfo);
 
-rc = prepare_dtb(d, &kinfo);
+if ( acpi_disabled )
+rc = prepare_dtb(d, &kinfo);
+else
+rc = prepare_acpi(d, &kinfo);
+
 if ( rc < 0 )
 return rc;
 
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 53c7452..78d8ae9 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -13,6 +13,7 @@
 #include 
 #include 
 #include 
+#include 
 #if EFI_PAGE_SIZE != PAGE_SIZE
 # error Cannot use xen/pfn.h here!
 #endif
@@ -1171,6 +1172,26 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE 
*SystemTable)
 for( ; ; ); /* not reached */
 }
 
+#ifdef CONFIG_ACPI
+/* Constant to indicate "Xen" in unicode u16 format */
+static const u16 XEN_EFI_FW_VENDOR[] ={0x0058,0x0065,0x006E,0x};
+
+int __init estimate_efi_size(int mem_nr_banks)
+{
+int size;
+int est_size = sizeof(EFI_SYSTEM_TABLE);
+int ect_size = sizeof(EFI_CONFIGURATION_TABL

Re: [Xen-devel] [PATCH v3 40/62] arm/acpi: Estimate memory required for acpi/efi tables

2015-12-31 Thread Shannon Zhao


On 2015/11/27 0:39, Stefano Stabellini wrote:
> On Tue, 17 Nov 2015, shannon.z...@linaro.org wrote:
>> From: Shannon Zhao 
>>
>> Estimate the memory required for loading acpi/efi tables in Dom0. Alloc
>> the pages to store the new created EFI and ACPI tables and free these
>> pages when destroying domain.
> 
> Could you please explain what you are page aligning exactly and why?
> 

At least it should be 64bit aligned of the table start address. If not,
guest will throw out an alignment fault.

ACPI: Using GIC for interrupt routing
Unhandled fault: alignment fault (0x9621) at 0xff86c19c
Internal error: : 9621 [#1] PREEMPT SMP
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.3.0+ #551
Hardware name: (null) (DT)
task: ffc00887 ti: ffc00884c000 task.ti: ffc00884c000
PC is at acpi_get_phys_id+0x264/0x290
LR is at acpi_get_phys_id+0x178/0x290

> 
>>
>> Signed-off-by: Parth Dixit 
>> Signed-off-by: Shannon Zhao 
>> ---
>>  xen/arch/arm/domain.c   |  4 +++
>>  xen/arch/arm/domain_build.c | 80 
>> -
>>  xen/common/efi/boot.c   | 21 
>>  xen/include/asm-arm/setup.h |  2 ++
>>  4 files changed, 106 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
>> index 880d0a6..10c58c4 100644
>> --- a/xen/arch/arm/domain.c
>> +++ b/xen/arch/arm/domain.c
>> @@ -638,6 +638,10 @@ void arch_domain_destroy(struct domain *d)
>>  domain_vgic_free(d);
>>  domain_vuart_free(d);
>>  free_xenheap_page(d->shared_info);
>> +#ifdef CONFIG_ACPI
>> +free_xenheap_pages(d->arch.efi_acpi_table,
>> +   get_order_from_bytes(d->arch.efi_acpi_len));
>> +#endif
>>  }
>>  
>>  void arch_domain_shutdown(struct domain *d)
>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>> index 0c3441a..b5ed44c 100644
>> --- a/xen/arch/arm/domain_build.c
>> +++ b/xen/arch/arm/domain_build.c
>> @@ -12,6 +12,8 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>> @@ -1354,6 +1356,78 @@ static int prepare_dtb(struct domain *d, struct 
>> kernel_info *kinfo)
>>  return -EINVAL;
>>  }
>>  
>> +#ifdef CONFIG_ACPI
>> +static int estimate_acpi_efi_size(struct domain *d, struct kernel_info 
>> *kinfo)
>> +{
>> +u64 efi_size, acpi_size = 0, addr;
>> +u32 madt_size;
>> +struct acpi_table_rsdp *rsdp_tbl;
>> +struct acpi_table_header *table = NULL;
>> +
>> +efi_size = estimate_efi_size(kinfo->mem.nr_banks);
>> +
>> +acpi_size += PAGE_ALIGN(sizeof(struct acpi_table_fadt));
>> +acpi_size += PAGE_ALIGN(sizeof(struct acpi_table_stao));
>> +
>> +madt_size = sizeof(struct acpi_table_madt)
>> ++ sizeof(struct acpi_madt_generic_interrupt) * d->max_vcpus
>> ++ sizeof(struct acpi_madt_generic_distributor);
>> +if ( d->arch.vgic.version == GIC_V3 )
>> +madt_size += sizeof(struct acpi_madt_generic_redistributor)
>> + * d->arch.vgic.nr_regions;
>> +acpi_size += PAGE_ALIGN(madt_size);
> 
> are the MADT and FADT tables guaranteed to be on separate pages or is it
> just an estimate?
> 
> 
>> +addr = acpi_os_get_root_pointer();
>> +if ( !addr )
>> +{
>> +printk("Unable to get acpi root pointer\n");
>> +return -EINVAL;
>> +}
>> +rsdp_tbl = acpi_os_map_memory(addr, sizeof(struct acpi_table_rsdp));
>> +table = acpi_os_map_memory(rsdp_tbl->xsdt_physical_address,
>> +   sizeof(struct acpi_table_header));
>> +acpi_size += PAGE_ALIGN(table->length + sizeof(u64));
>> +acpi_os_unmap_memory(table, sizeof(struct acpi_table_header));
>> +acpi_os_unmap_memory(rsdp_tbl, sizeof(struct acpi_table_rsdp));
>> +
>> +acpi_size += PAGE_ALIGN(sizeof(struct acpi_table_rsdp));
>> +d->arch.efi_acpi_len = PAGE_ALIGN(efi_size) + PAGE_ALIGN(acpi_size);
>> +
>> +return 0;
>> +}
>> +
>> +static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
>> +{
>> +int rc = 0;
>> +int order;
>> +
>> +rc = estimate_acpi_efi_size(d, kinfo);
>> +if ( rc != 0 )
>> +return rc;
>> +
>> +order = get_order_from_bytes(d->arch.efi_acpi_len);
>> +d->arch.efi_acpi_table = alloc_xenheap_pages(order, 0);
>> +if ( d->arch.efi_acpi_table == NULL )
>> +{
>> +printk("unable to allocate memory!\n");
>> +return -1;
> 
> ENOMEM
> 
> 
>> +}
>> +memset(d->arch.efi_acpi_table, 0, d->arch.efi_acpi_len);
>> +
>> +/* For ACPI, Dom0 doesn't use kinfo->gnttab_start to get the grant table
>> + * region. So we use it as the ACPI table mapped address. */
>> +d->arch.efi_acpi_gpa = kinfo->gnttab_start;
>> +
>> +return 0;
>> +}
>> +#else
>> +static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
>> +{
>> +/* Only booting with ACPI will hit here */
>> +BUG_ON(1);
>> +return -EINVAL;
>> +}
>> +

Re: [Xen-devel] [PATCH v3 40/62] arm/acpi: Estimate memory required for acpi/efi tables

2016-01-04 Thread Shannon Zhao


On 2015/11/30 23:14, Julien Grall wrote:
> Hi Shannon,
> 
> On 17/11/15 09:40, shannon.z...@linaro.org wrote:
>> diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
>> index 53c7452..78d8ae9 100644
>> --- a/xen/common/efi/boot.c
>> +++ b/xen/common/efi/boot.c
>> @@ -13,6 +13,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  #if EFI_PAGE_SIZE != PAGE_SIZE
>>  # error Cannot use xen/pfn.h here!
>>  #endif
>> @@ -1171,6 +1172,26 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE 
>> *SystemTable)
>>  for( ; ; ); /* not reached */
>>  }
>>  
>> +#ifdef CONFIG_ACPI
>> +/* Constant to indicate "Xen" in unicode u16 format */
>> +static const u16 XEN_EFI_FW_VENDOR[] ={0x0058,0x0065,0x006E,0x};
>> +
>> +int __init estimate_efi_size(int mem_nr_banks)
>> +{
>> +int size;
>> +int est_size = sizeof(EFI_SYSTEM_TABLE);
>> +int ect_size = sizeof(EFI_CONFIGURATION_TABLE);
>> +int emd_size = sizeof(EFI_MEMORY_DESCRIPTOR);
>> +int fw_vendor_size = sizeof(XEN_EFI_FW_VENDOR);
>> +
>> +size = PAGE_ALIGN(est_size + ect_size + fw_vendor_size)
>> +   + PAGE_ALIGN(emd_size *
>> +(mem_nr_banks + acpi_mem.nr_banks + TBL_MMAX));
>> +
>> +return size;
>> +}
>> +#endif
>> +
> 
> efi/boot.c only contains code related to the stub EFI. This is not the
> case of this code.
> 

I add these codes at this file because I want to reuse the definitions
of EFI structures like EFI_SYSTEM_TABLE. Parth re-define those
structures at previous patch set and one comment suggested to reuse the
existing ones. If I add these codes in xen/arch/arm/domain_build.c, it
will have some conflicts. E.g:

In file included from /home/open-source/xen/xen/include/asm/efibind.h:2:0,
 from domain_build.c:29:
/home/open-source/xen/xen/include/asm/arm64/efibind.h:86:20: error:
conflicting types for 'UINT64'
 typedef uint64_t   UINT64;
^
In file included from /home/open-source/xen/xen/include/acpi/acpi.h:57:0,
 from /home/open-source/xen/xen/include/xen/acpi.h:33,
 from domain_build.c:15:
/home/open-source/xen/xen/include/acpi/actypes.h:130:35: note: previous
declaration of 'UINT64' was here
 typedef COMPILER_DEPENDENT_UINT64 UINT64;

> Moreover, this code won't compile on x86 because acpi_mem is only
> defined for ARM.
> 
> Regards,
> 

Thanks,
-- 
Shannon


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


Re: [Xen-devel] [PATCH v3 40/62] arm/acpi: Estimate memory required for acpi/efi tables

2016-01-04 Thread Stefano Stabellini
On Mon, 4 Jan 2016, Shannon Zhao wrote:
> On 2015/11/30 23:14, Julien Grall wrote:
> > Hi Shannon,
> > 
> > On 17/11/15 09:40, shannon.z...@linaro.org wrote:
> >> diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
> >> index 53c7452..78d8ae9 100644
> >> --- a/xen/common/efi/boot.c
> >> +++ b/xen/common/efi/boot.c
> >> @@ -13,6 +13,7 @@
> >>  #include 
> >>  #include 
> >>  #include 
> >> +#include 
> >>  #if EFI_PAGE_SIZE != PAGE_SIZE
> >>  # error Cannot use xen/pfn.h here!
> >>  #endif
> >> @@ -1171,6 +1172,26 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE 
> >> *SystemTable)
> >>  for( ; ; ); /* not reached */
> >>  }
> >>  
> >> +#ifdef CONFIG_ACPI
> >> +/* Constant to indicate "Xen" in unicode u16 format */
> >> +static const u16 XEN_EFI_FW_VENDOR[] ={0x0058,0x0065,0x006E,0x};
> >> +
> >> +int __init estimate_efi_size(int mem_nr_banks)
> >> +{
> >> +int size;
> >> +int est_size = sizeof(EFI_SYSTEM_TABLE);
> >> +int ect_size = sizeof(EFI_CONFIGURATION_TABLE);
> >> +int emd_size = sizeof(EFI_MEMORY_DESCRIPTOR);
> >> +int fw_vendor_size = sizeof(XEN_EFI_FW_VENDOR);
> >> +
> >> +size = PAGE_ALIGN(est_size + ect_size + fw_vendor_size)
> >> +   + PAGE_ALIGN(emd_size *
> >> +(mem_nr_banks + acpi_mem.nr_banks + TBL_MMAX));
> >> +
> >> +return size;
> >> +}
> >> +#endif
> >> +
> > 
> > efi/boot.c only contains code related to the stub EFI. This is not the
> > case of this code.
> > 
> 
> I add these codes at this file because I want to reuse the definitions
> of EFI structures like EFI_SYSTEM_TABLE. Parth re-define those
> structures at previous patch set and one comment suggested to reuse the
> existing ones. If I add these codes in xen/arch/arm/domain_build.c, it
> will have some conflicts. E.g:
> 
> In file included from /home/open-source/xen/xen/include/asm/efibind.h:2:0,
>  from domain_build.c:29:
> /home/open-source/xen/xen/include/asm/arm64/efibind.h:86:20: error:
> conflicting types for 'UINT64'
>  typedef uint64_t   UINT64;
> ^
> In file included from /home/open-source/xen/xen/include/acpi/acpi.h:57:0,
>  from /home/open-source/xen/xen/include/xen/acpi.h:33,
>  from domain_build.c:15:
> /home/open-source/xen/xen/include/acpi/actypes.h:130:35: note: previous
> declaration of 'UINT64' was here
>  typedef COMPILER_DEPENDENT_UINT64 UINT64;

I think that having this code in efi/boot.c might be OK, but let's see
what Jan thinks.



> > Moreover, this code won't compile on x86 because acpi_mem is only
> > defined for ARM.
> > 
> > Regards,
> > 
> 
> Thanks,
> -- 
> Shannon
> 

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


Re: [Xen-devel] [PATCH v3 40/62] arm/acpi: Estimate memory required for acpi/efi tables

2016-01-04 Thread Stefano Stabellini
On Thu, 31 Dec 2015, Shannon Zhao wrote:
> On 2015/11/27 0:39, Stefano Stabellini wrote:
> > On Tue, 17 Nov 2015, shannon.z...@linaro.org wrote:
> >> From: Shannon Zhao 
> >>
> >> Estimate the memory required for loading acpi/efi tables in Dom0. Alloc
> >> the pages to store the new created EFI and ACPI tables and free these
> >> pages when destroying domain.
> > 
> > Could you please explain what you are page aligning exactly and why?
> > 
> 
> At least it should be 64bit aligned of the table start address. If not,
> guest will throw out an alignment fault.
> 
> ACPI: Using GIC for interrupt routing
> Unhandled fault: alignment fault (0x9621) at 0xff86c19c
> Internal error: : 9621 [#1] PREEMPT SMP
> Modules linked in:
> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.3.0+ #551
> Hardware name: (null) (DT)
> task: ffc00887 ti: ffc00884c000 task.ti: ffc00884c000
> PC is at acpi_get_phys_id+0x264/0x290
> LR is at acpi_get_phys_id+0x178/0x290

OK, but between page alignment and 64-bit alignment there is a very big
difference.


> > 
> >>
> >> Signed-off-by: Parth Dixit 
> >> Signed-off-by: Shannon Zhao 
> >> ---
> >>  xen/arch/arm/domain.c   |  4 +++
> >>  xen/arch/arm/domain_build.c | 80 
> >> -
> >>  xen/common/efi/boot.c   | 21 
> >>  xen/include/asm-arm/setup.h |  2 ++
> >>  4 files changed, 106 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> >> index 880d0a6..10c58c4 100644
> >> --- a/xen/arch/arm/domain.c
> >> +++ b/xen/arch/arm/domain.c
> >> @@ -638,6 +638,10 @@ void arch_domain_destroy(struct domain *d)
> >>  domain_vgic_free(d);
> >>  domain_vuart_free(d);
> >>  free_xenheap_page(d->shared_info);
> >> +#ifdef CONFIG_ACPI
> >> +free_xenheap_pages(d->arch.efi_acpi_table,
> >> +   get_order_from_bytes(d->arch.efi_acpi_len));
> >> +#endif
> >>  }
> >>  
> >>  void arch_domain_shutdown(struct domain *d)
> >> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> >> index 0c3441a..b5ed44c 100644
> >> --- a/xen/arch/arm/domain_build.c
> >> +++ b/xen/arch/arm/domain_build.c
> >> @@ -12,6 +12,8 @@
> >>  #include 
> >>  #include 
> >>  #include 
> >> +#include 
> >> +#include 
> >>  #include 
> >>  #include 
> >>  #include 
> >> @@ -1354,6 +1356,78 @@ static int prepare_dtb(struct domain *d, struct 
> >> kernel_info *kinfo)
> >>  return -EINVAL;
> >>  }
> >>  
> >> +#ifdef CONFIG_ACPI
> >> +static int estimate_acpi_efi_size(struct domain *d, struct kernel_info 
> >> *kinfo)
> >> +{
> >> +u64 efi_size, acpi_size = 0, addr;
> >> +u32 madt_size;
> >> +struct acpi_table_rsdp *rsdp_tbl;
> >> +struct acpi_table_header *table = NULL;
> >> +
> >> +efi_size = estimate_efi_size(kinfo->mem.nr_banks);
> >> +
> >> +acpi_size += PAGE_ALIGN(sizeof(struct acpi_table_fadt));
> >> +acpi_size += PAGE_ALIGN(sizeof(struct acpi_table_stao));
> >> +
> >> +madt_size = sizeof(struct acpi_table_madt)
> >> ++ sizeof(struct acpi_madt_generic_interrupt) * 
> >> d->max_vcpus
> >> ++ sizeof(struct acpi_madt_generic_distributor);
> >> +if ( d->arch.vgic.version == GIC_V3 )
> >> +madt_size += sizeof(struct acpi_madt_generic_redistributor)
> >> + * d->arch.vgic.nr_regions;
> >> +acpi_size += PAGE_ALIGN(madt_size);
> > 
> > are the MADT and FADT tables guaranteed to be on separate pages or is it
> > just an estimate?
> > 
> > 
> >> +addr = acpi_os_get_root_pointer();
> >> +if ( !addr )
> >> +{
> >> +printk("Unable to get acpi root pointer\n");
> >> +return -EINVAL;
> >> +}
> >> +rsdp_tbl = acpi_os_map_memory(addr, sizeof(struct acpi_table_rsdp));
> >> +table = acpi_os_map_memory(rsdp_tbl->xsdt_physical_address,
> >> +   sizeof(struct acpi_table_header));
> >> +acpi_size += PAGE_ALIGN(table->length + sizeof(u64));
> >> +acpi_os_unmap_memory(table, sizeof(struct acpi_table_header));
> >> +acpi_os_unmap_memory(rsdp_tbl, sizeof(struct acpi_table_rsdp));
> >> +
> >> +acpi_size += PAGE_ALIGN(sizeof(struct acpi_table_rsdp));
> >> +d->arch.efi_acpi_len = PAGE_ALIGN(efi_size) + PAGE_ALIGN(acpi_size);
> >> +
> >> +return 0;
> >> +}
> >> +
> >> +static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
> >> +{
> >> +int rc = 0;
> >> +int order;
> >> +
> >> +rc = estimate_acpi_efi_size(d, kinfo);
> >> +if ( rc != 0 )
> >> +return rc;
> >> +
> >> +order = get_order_from_bytes(d->arch.efi_acpi_len);
> >> +d->arch.efi_acpi_table = alloc_xenheap_pages(order, 0);
> >> +if ( d->arch.efi_acpi_table == NULL )
> >> +{
> >> +printk("unable to allocate memory!\n");
> >> +return -1;
> > 
> > ENOMEM
> > 
> > 
> >> +}
> >> +memset(d->arch.efi_acpi_table, 0, d->arch.efi_acpi_len);
> >> +
> >> +/* For ACPI, 

Re: [Xen-devel] [PATCH v3 40/62] arm/acpi: Estimate memory required for acpi/efi tables

2015-11-26 Thread Stefano Stabellini
On Tue, 17 Nov 2015, shannon.z...@linaro.org wrote:
> From: Shannon Zhao 
> 
> Estimate the memory required for loading acpi/efi tables in Dom0. Alloc
> the pages to store the new created EFI and ACPI tables and free these
> pages when destroying domain.

Could you please explain what you are page aligning exactly and why?


> 
> Signed-off-by: Parth Dixit 
> Signed-off-by: Shannon Zhao 
> ---
>  xen/arch/arm/domain.c   |  4 +++
>  xen/arch/arm/domain_build.c | 80 
> -
>  xen/common/efi/boot.c   | 21 
>  xen/include/asm-arm/setup.h |  2 ++
>  4 files changed, 106 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 880d0a6..10c58c4 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -638,6 +638,10 @@ void arch_domain_destroy(struct domain *d)
>  domain_vgic_free(d);
>  domain_vuart_free(d);
>  free_xenheap_page(d->shared_info);
> +#ifdef CONFIG_ACPI
> +free_xenheap_pages(d->arch.efi_acpi_table,
> +   get_order_from_bytes(d->arch.efi_acpi_len));
> +#endif
>  }
>  
>  void arch_domain_shutdown(struct domain *d)
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 0c3441a..b5ed44c 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -12,6 +12,8 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -1354,6 +1356,78 @@ static int prepare_dtb(struct domain *d, struct 
> kernel_info *kinfo)
>  return -EINVAL;
>  }
>  
> +#ifdef CONFIG_ACPI
> +static int estimate_acpi_efi_size(struct domain *d, struct kernel_info 
> *kinfo)
> +{
> +u64 efi_size, acpi_size = 0, addr;
> +u32 madt_size;
> +struct acpi_table_rsdp *rsdp_tbl;
> +struct acpi_table_header *table = NULL;
> +
> +efi_size = estimate_efi_size(kinfo->mem.nr_banks);
> +
> +acpi_size += PAGE_ALIGN(sizeof(struct acpi_table_fadt));
> +acpi_size += PAGE_ALIGN(sizeof(struct acpi_table_stao));
> +
> +madt_size = sizeof(struct acpi_table_madt)
> ++ sizeof(struct acpi_madt_generic_interrupt) * d->max_vcpus
> ++ sizeof(struct acpi_madt_generic_distributor);
> +if ( d->arch.vgic.version == GIC_V3 )
> +madt_size += sizeof(struct acpi_madt_generic_redistributor)
> + * d->arch.vgic.nr_regions;
> +acpi_size += PAGE_ALIGN(madt_size);

are the MADT and FADT tables guaranteed to be on separate pages or is it
just an estimate?


> +addr = acpi_os_get_root_pointer();
> +if ( !addr )
> +{
> +printk("Unable to get acpi root pointer\n");
> +return -EINVAL;
> +}
> +rsdp_tbl = acpi_os_map_memory(addr, sizeof(struct acpi_table_rsdp));
> +table = acpi_os_map_memory(rsdp_tbl->xsdt_physical_address,
> +   sizeof(struct acpi_table_header));
> +acpi_size += PAGE_ALIGN(table->length + sizeof(u64));
> +acpi_os_unmap_memory(table, sizeof(struct acpi_table_header));
> +acpi_os_unmap_memory(rsdp_tbl, sizeof(struct acpi_table_rsdp));
> +
> +acpi_size += PAGE_ALIGN(sizeof(struct acpi_table_rsdp));
> +d->arch.efi_acpi_len = PAGE_ALIGN(efi_size) + PAGE_ALIGN(acpi_size);
> +
> +return 0;
> +}
> +
> +static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
> +{
> +int rc = 0;
> +int order;
> +
> +rc = estimate_acpi_efi_size(d, kinfo);
> +if ( rc != 0 )
> +return rc;
> +
> +order = get_order_from_bytes(d->arch.efi_acpi_len);
> +d->arch.efi_acpi_table = alloc_xenheap_pages(order, 0);
> +if ( d->arch.efi_acpi_table == NULL )
> +{
> +printk("unable to allocate memory!\n");
> +return -1;

ENOMEM


> +}
> +memset(d->arch.efi_acpi_table, 0, d->arch.efi_acpi_len);
> +
> +/* For ACPI, Dom0 doesn't use kinfo->gnttab_start to get the grant table
> + * region. So we use it as the ACPI table mapped address. */
> +d->arch.efi_acpi_gpa = kinfo->gnttab_start;
> +
> +return 0;
> +}
> +#else
> +static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
> +{
> +/* Only booting with ACPI will hit here */
> +BUG_ON(1);
> +return -EINVAL;
> +}
> +#endif
>  static void dtb_load(struct kernel_info *kinfo)
>  {
>  void * __user dtb_virt = (void * __user)(register_t)kinfo->dtb_paddr;
> @@ -1540,7 +1614,11 @@ int construct_dom0(struct domain *d)
>  allocate_memory(d, &kinfo);
>  find_gnttab_region(d, &kinfo);
>  
> -rc = prepare_dtb(d, &kinfo);
> +if ( acpi_disabled )
> +rc = prepare_dtb(d, &kinfo);
> +else
> +rc = prepare_acpi(d, &kinfo);
> +
>  if ( rc < 0 )
>  return rc;
>  
> diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
> index 53c7452..78d8ae9 100644
> --- a/xen/common/efi/boot.c
> +++ b/xen/common/efi/boot.c
> @@ -13,6 +13,7 @@
>  #include 
>  #include 
>  #include 
> +#inc

Re: [Xen-devel] [PATCH v3 40/62] arm/acpi: Estimate memory required for acpi/efi tables

2015-11-30 Thread Julien Grall
Hi Shannon,

On 17/11/15 09:40, shannon.z...@linaro.org wrote:
> diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
> index 53c7452..78d8ae9 100644
> --- a/xen/common/efi/boot.c
> +++ b/xen/common/efi/boot.c
> @@ -13,6 +13,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #if EFI_PAGE_SIZE != PAGE_SIZE
>  # error Cannot use xen/pfn.h here!
>  #endif
> @@ -1171,6 +1172,26 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE 
> *SystemTable)
>  for( ; ; ); /* not reached */
>  }
>  
> +#ifdef CONFIG_ACPI
> +/* Constant to indicate "Xen" in unicode u16 format */
> +static const u16 XEN_EFI_FW_VENDOR[] ={0x0058,0x0065,0x006E,0x};
> +
> +int __init estimate_efi_size(int mem_nr_banks)
> +{
> +int size;
> +int est_size = sizeof(EFI_SYSTEM_TABLE);
> +int ect_size = sizeof(EFI_CONFIGURATION_TABLE);
> +int emd_size = sizeof(EFI_MEMORY_DESCRIPTOR);
> +int fw_vendor_size = sizeof(XEN_EFI_FW_VENDOR);
> +
> +size = PAGE_ALIGN(est_size + ect_size + fw_vendor_size)
> +   + PAGE_ALIGN(emd_size *
> +(mem_nr_banks + acpi_mem.nr_banks + TBL_MMAX));
> +
> +return size;
> +}
> +#endif
> +

efi/boot.c only contains code related to the stub EFI. This is not the
case of this code.

Moreover, this code won't compile on x86 because acpi_mem is only
defined for ARM.

Regards,

-- 
Julien Grall

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