| Message ID | 1456658360-16080-5-git-send-email-zhaoshenglong@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 28.02.16 at 12:19, <zhaoshenglong@huawei.com> wrote: > --- a/xen/common/efi/boot.c > +++ b/xen/common/efi/boot.c > @@ -13,6 +13,7 @@ > #include <xen/multiboot.h> > #include <xen/pci_regs.h> > #include <xen/pfn.h> > +#include <asm/acpi.h> > #if EFI_PAGE_SIZE != PAGE_SIZE > # error Cannot use xen/pfn.h here! > #endif > @@ -1171,6 +1172,25 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable) > for( ; ; ); /* not reached */ > } > > +#if defined (CONFIG_ACPI) && defined (CONFIG_ARM) Are both parts really necessary? It would seem to me that the latter should suffice. > +/* Constant to indicate "Xen" in unicode u16 format */ > +static const u16 XEN_EFI_FW_VENDOR[] ={0x0058,0x0065,0x006E,0x0000}; This should be a wide string literal, with the variable type being CHAR16. Also variable names shouldn't be all upper case. > +int __init estimate_efi_size(int mem_nr_banks) > +{ > + int size = 0; > + 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 += ROUNDUP((est_size + ect_size + fw_vendor_size), 8); > + size += ROUNDUP(emd_size * (mem_nr_banks + acpi_mem.nr_banks + 1), 8); > + > + return size; > +} It would seem to me that none of the variables involved really holds a signed quantity. This should be reflected by the types chosen - likely you need s/int/size_t/ for the entire function, except for the function parameter, which looks like it wants to be unsigned int. Also the initializer of "size" could be easily got rid of, and there's a pair of pointless parentheses in the first ROUNDUP(). As to the configuration table - the sizeof() covers a single table entry only afaict - is that really intended? Jan
On Sun, 28 Feb 2016, Shannon Zhao wrote: > From: Shannon Zhao <shannon.zhao@linaro.org> > > Estimate the memory required for loading acpi/efi tables in Dom0. Make > the length of each table aligned with 64bit. Alloc the pages to store > the new created EFI and ACPI tables and free these pages when > destroying domain. > > 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: make the length of each table aligned with 64bit > --- > xen/arch/arm/domain.c | 4 +++ > xen/arch/arm/domain_build.c | 81 ++++++++++++++++++++++++++++++++++++++++++++- > xen/common/efi/boot.c | 20 +++++++++++ > 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 3d274ae..1365b4a 100644 > --- a/xen/arch/arm/domain.c > +++ b/xen/arch/arm/domain.c > @@ -640,6 +640,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 83676e4..b10a69d 100644 > --- a/xen/arch/arm/domain_build.c > +++ b/xen/arch/arm/domain_build.c > @@ -12,6 +12,8 @@ > #include <xen/libfdt/libfdt.h> > #include <xen/guest_access.h> > #include <xen/iocap.h> > +#include <xen/acpi.h> > +#include <acpi/actables.h> > #include <asm/device.h> > #include <asm/setup.h> > #include <asm/platform.h> > @@ -1354,6 +1356,79 @@ 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 += ROUNDUP(sizeof(struct acpi_table_fadt), 8); > + acpi_size += ROUNDUP(sizeof(struct acpi_table_stao), 8); > + > + 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 += ROUNDUP(madt_size, 8); > + > + 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)); > + /* Add place for STAO table in XSDT table */ > + acpi_size += ROUNDUP(table->length + sizeof(u64), 8); > + acpi_os_unmap_memory(table, sizeof(struct acpi_table_header)); > + acpi_os_unmap_memory(rsdp_tbl, sizeof(struct acpi_table_rsdp)); > + > + acpi_size += ROUNDUP(sizeof(struct acpi_table_rsdp), 8); > + d->arch.efi_acpi_len = ROUNDUP(efi_size, 8) + ROUNDUP(acpi_size, 8); > + > + 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 -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 +1615,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..535c685 100644 > --- a/xen/common/efi/boot.c > +++ b/xen/common/efi/boot.c > @@ -13,6 +13,7 @@ > #include <xen/multiboot.h> > #include <xen/pci_regs.h> > #include <xen/pfn.h> > +#include <asm/acpi.h> > #if EFI_PAGE_SIZE != PAGE_SIZE > # error Cannot use xen/pfn.h here! > #endif > @@ -1171,6 +1172,25 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable) > for( ; ; ); /* not reached */ > } > > +#if defined (CONFIG_ACPI) && defined (CONFIG_ARM) > +/* Constant to indicate "Xen" in unicode u16 format */ > +static const u16 XEN_EFI_FW_VENDOR[] ={0x0058,0x0065,0x006E,0x0000}; > + > +int __init estimate_efi_size(int mem_nr_banks) > +{ > + int size = 0; > + 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 += ROUNDUP((est_size + ect_size + fw_vendor_size), 8); > + size += ROUNDUP(emd_size * (mem_nr_banks + acpi_mem.nr_banks + 1), 8); > + > + return size; > +} > +#endif Given that EFI and ACPI are separate things, I don't think we should ifdef CONFIG_ACPI this function. estimate_efi_size should work even if no ACPI tables are present, right? Like Jan suggested, we need to be consistent with the integer types in this function and the callers. Given that mem_nr_banks, like acpi_mem.nr_banks, is also available through the bootinfo global variable, I don't think it makes too much sense to get the number of memory banks from a parameter and the number of acpi banks from the global variable. We should be consistent. > #ifndef CONFIG_ARM /* TODO - runtime service support */ > > static bool_t __initdata efi_rs_enable = 1; > diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h > index 30ac53b..b759813 100644 > --- a/xen/include/asm-arm/setup.h > +++ b/xen/include/asm-arm/setup.h > @@ -51,6 +51,8 @@ void arch_init_memory(void); > > void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len); > > +int estimate_efi_size(int mem_nr_banks); > + > int construct_dom0(struct domain *d); > > void discard_initial_modules(void);
On 2016/2/29 20:13, Stefano Stabellini wrote: > On Sun, 28 Feb 2016, Shannon Zhao wrote: >> > From: Shannon Zhao <shannon.zhao@linaro.org> >> > >> > Estimate the memory required for loading acpi/efi tables in Dom0. Make >> > the length of each table aligned with 64bit. Alloc the pages to store >> > the new created EFI and ACPI tables and free these pages when >> > destroying domain. >> > >> > 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: make the length of each table aligned with 64bit >> > --- >> > xen/arch/arm/domain.c | 4 +++ >> > xen/arch/arm/domain_build.c | 81 ++++++++++++++++++++++++++++++++++++++++++++- >> > xen/common/efi/boot.c | 20 +++++++++++ >> > 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 3d274ae..1365b4a 100644 >> > --- a/xen/arch/arm/domain.c >> > +++ b/xen/arch/arm/domain.c >> > @@ -640,6 +640,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 83676e4..b10a69d 100644 >> > --- a/xen/arch/arm/domain_build.c >> > +++ b/xen/arch/arm/domain_build.c >> > @@ -12,6 +12,8 @@ >> > #include <xen/libfdt/libfdt.h> >> > #include <xen/guest_access.h> >> > #include <xen/iocap.h> >> > +#include <xen/acpi.h> >> > +#include <acpi/actables.h> >> > #include <asm/device.h> >> > #include <asm/setup.h> >> > #include <asm/platform.h> >> > @@ -1354,6 +1356,79 @@ 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 += ROUNDUP(sizeof(struct acpi_table_fadt), 8); >> > + acpi_size += ROUNDUP(sizeof(struct acpi_table_stao), 8); >> > + >> > + 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 += ROUNDUP(madt_size, 8); >> > + >> > + 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)); >> > + /* Add place for STAO table in XSDT table */ >> > + acpi_size += ROUNDUP(table->length + sizeof(u64), 8); >> > + acpi_os_unmap_memory(table, sizeof(struct acpi_table_header)); >> > + acpi_os_unmap_memory(rsdp_tbl, sizeof(struct acpi_table_rsdp)); >> > + >> > + acpi_size += ROUNDUP(sizeof(struct acpi_table_rsdp), 8); >> > + d->arch.efi_acpi_len = ROUNDUP(efi_size, 8) + ROUNDUP(acpi_size, 8); >> > + >> > + 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 -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 +1615,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..535c685 100644 >> > --- a/xen/common/efi/boot.c >> > +++ b/xen/common/efi/boot.c >> > @@ -13,6 +13,7 @@ >> > #include <xen/multiboot.h> >> > #include <xen/pci_regs.h> >> > #include <xen/pfn.h> >> > +#include <asm/acpi.h> >> > #if EFI_PAGE_SIZE != PAGE_SIZE >> > # error Cannot use xen/pfn.h here! >> > #endif >> > @@ -1171,6 +1172,25 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable) >> > for( ; ; ); /* not reached */ >> > } >> > >> > +#if defined (CONFIG_ACPI) && defined (CONFIG_ARM) >> > +/* Constant to indicate "Xen" in unicode u16 format */ >> > +static const u16 XEN_EFI_FW_VENDOR[] ={0x0058,0x0065,0x006E,0x0000}; >> > + >> > +int __init estimate_efi_size(int mem_nr_banks) >> > +{ >> > + int size = 0; >> > + 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 += ROUNDUP((est_size + ect_size + fw_vendor_size), 8); >> > + size += ROUNDUP(emd_size * (mem_nr_banks + acpi_mem.nr_banks + 1), 8); >> > + >> > + return size; >> > +} >> > +#endif > Given that EFI and ACPI are separate things, I don't think we should > ifdef CONFIG_ACPI this function. estimate_efi_size should work even if > no ACPI tables are present, right? > Yes, while we could check acpi_disabled to know whether Xen boots with ACPI, is there any way to check if Xen boots with UEFI(especially UEFI + DT)? Thanks,
On Tue, 1 Mar 2016, Shannon Zhao wrote: > On 2016/2/29 20:13, Stefano Stabellini wrote: > > On Sun, 28 Feb 2016, Shannon Zhao wrote: > >> > From: Shannon Zhao <shannon.zhao@linaro.org> > >> > > >> > Estimate the memory required for loading acpi/efi tables in Dom0. Make > >> > the length of each table aligned with 64bit. Alloc the pages to store > >> > the new created EFI and ACPI tables and free these pages when > >> > destroying domain. > >> > > >> > 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: make the length of each table aligned with 64bit > >> > --- > >> > xen/arch/arm/domain.c | 4 +++ > >> > xen/arch/arm/domain_build.c | 81 ++++++++++++++++++++++++++++++++++++++++++++- > >> > xen/common/efi/boot.c | 20 +++++++++++ > >> > 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 3d274ae..1365b4a 100644 > >> > --- a/xen/arch/arm/domain.c > >> > +++ b/xen/arch/arm/domain.c > >> > @@ -640,6 +640,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 83676e4..b10a69d 100644 > >> > --- a/xen/arch/arm/domain_build.c > >> > +++ b/xen/arch/arm/domain_build.c > >> > @@ -12,6 +12,8 @@ > >> > #include <xen/libfdt/libfdt.h> > >> > #include <xen/guest_access.h> > >> > #include <xen/iocap.h> > >> > +#include <xen/acpi.h> > >> > +#include <acpi/actables.h> > >> > #include <asm/device.h> > >> > #include <asm/setup.h> > >> > #include <asm/platform.h> > >> > @@ -1354,6 +1356,79 @@ 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 += ROUNDUP(sizeof(struct acpi_table_fadt), 8); > >> > + acpi_size += ROUNDUP(sizeof(struct acpi_table_stao), 8); > >> > + > >> > + 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 += ROUNDUP(madt_size, 8); > >> > + > >> > + 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)); > >> > + /* Add place for STAO table in XSDT table */ > >> > + acpi_size += ROUNDUP(table->length + sizeof(u64), 8); > >> > + acpi_os_unmap_memory(table, sizeof(struct acpi_table_header)); > >> > + acpi_os_unmap_memory(rsdp_tbl, sizeof(struct acpi_table_rsdp)); > >> > + > >> > + acpi_size += ROUNDUP(sizeof(struct acpi_table_rsdp), 8); > >> > + d->arch.efi_acpi_len = ROUNDUP(efi_size, 8) + ROUNDUP(acpi_size, 8); > >> > + > >> > + 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 -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 +1615,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..535c685 100644 > >> > --- a/xen/common/efi/boot.c > >> > +++ b/xen/common/efi/boot.c > >> > @@ -13,6 +13,7 @@ > >> > #include <xen/multiboot.h> > >> > #include <xen/pci_regs.h> > >> > #include <xen/pfn.h> > >> > +#include <asm/acpi.h> > >> > #if EFI_PAGE_SIZE != PAGE_SIZE > >> > # error Cannot use xen/pfn.h here! > >> > #endif > >> > @@ -1171,6 +1172,25 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable) > >> > for( ; ; ); /* not reached */ > >> > } > >> > > >> > +#if defined (CONFIG_ACPI) && defined (CONFIG_ARM) > >> > +/* Constant to indicate "Xen" in unicode u16 format */ > >> > +static const u16 XEN_EFI_FW_VENDOR[] ={0x0058,0x0065,0x006E,0x0000}; > >> > + > >> > +int __init estimate_efi_size(int mem_nr_banks) > >> > +{ > >> > + int size = 0; > >> > + 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 += ROUNDUP((est_size + ect_size + fw_vendor_size), 8); > >> > + size += ROUNDUP(emd_size * (mem_nr_banks + acpi_mem.nr_banks + 1), 8); > >> > + > >> > + return size; > >> > +} > >> > +#endif > > Given that EFI and ACPI are separate things, I don't think we should > > ifdef CONFIG_ACPI this function. estimate_efi_size should work even if > > no ACPI tables are present, right? > > > Yes, while we could check acpi_disabled to know whether Xen boots with > ACPI, is there any way to check if Xen boots with UEFI(especially UEFI + > DT)? There is an efi_enabled variable, but that is 0 on ARM because runtime services are not implemented yet. But it is OK to assume that on ARM if ACPI is enabled, efi is also enabled. That said, these are runtime checks, while the problem with the code in this patch is a compile-time #ifdef.
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c index 3d274ae..1365b4a 100644 --- a/xen/arch/arm/domain.c +++ b/xen/arch/arm/domain.c @@ -640,6 +640,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 83676e4..b10a69d 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -12,6 +12,8 @@ #include <xen/libfdt/libfdt.h> #include <xen/guest_access.h> #include <xen/iocap.h> +#include <xen/acpi.h> +#include <acpi/actables.h> #include <asm/device.h> #include <asm/setup.h> #include <asm/platform.h> @@ -1354,6 +1356,79 @@ 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 += ROUNDUP(sizeof(struct acpi_table_fadt), 8); + acpi_size += ROUNDUP(sizeof(struct acpi_table_stao), 8); + + 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 += ROUNDUP(madt_size, 8); + + 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)); + /* Add place for STAO table in XSDT table */ + acpi_size += ROUNDUP(table->length + sizeof(u64), 8); + acpi_os_unmap_memory(table, sizeof(struct acpi_table_header)); + acpi_os_unmap_memory(rsdp_tbl, sizeof(struct acpi_table_rsdp)); + + acpi_size += ROUNDUP(sizeof(struct acpi_table_rsdp), 8); + d->arch.efi_acpi_len = ROUNDUP(efi_size, 8) + ROUNDUP(acpi_size, 8); + + 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 -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 +1615,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..535c685 100644 --- a/xen/common/efi/boot.c +++ b/xen/common/efi/boot.c @@ -13,6 +13,7 @@ #include <xen/multiboot.h> #include <xen/pci_regs.h> #include <xen/pfn.h> +#include <asm/acpi.h> #if EFI_PAGE_SIZE != PAGE_SIZE # error Cannot use xen/pfn.h here! #endif @@ -1171,6 +1172,25 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable) for( ; ; ); /* not reached */ } +#if defined (CONFIG_ACPI) && defined (CONFIG_ARM) +/* Constant to indicate "Xen" in unicode u16 format */ +static const u16 XEN_EFI_FW_VENDOR[] ={0x0058,0x0065,0x006E,0x0000}; + +int __init estimate_efi_size(int mem_nr_banks) +{ + int size = 0; + 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 += ROUNDUP((est_size + ect_size + fw_vendor_size), 8); + size += ROUNDUP(emd_size * (mem_nr_banks + acpi_mem.nr_banks + 1), 8); + + return size; +} +#endif + #ifndef CONFIG_ARM /* TODO - runtime service support */ static bool_t __initdata efi_rs_enable = 1; diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h index 30ac53b..b759813 100644 --- a/xen/include/asm-arm/setup.h +++ b/xen/include/asm-arm/setup.h @@ -51,6 +51,8 @@ void arch_init_memory(void); void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len); +int estimate_efi_size(int mem_nr_banks); + int construct_dom0(struct domain *d); void discard_initial_modules(void);