| Message ID | 1458207668-12012-2-git-send-email-zhaoshenglong@huawei.com |
|---|---|
| State | Superseded |
| Headers | show |
Hi Shannon, On 17/03/16 09:40, Shannon Zhao wrote: > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > index 83676e4..4e20499 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) > +{ > + size_t efi_size, acpi_size, madt_size; > + u64 addr; > + struct acpi_table_rsdp *rsdp_tbl; > + struct acpi_table_header *table; > + > + 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)); acpi_os_map_memory can fail. If you are sure that in this specific case it will never, add a BUG_ON(rsdp_tbl != NULL). Otherwise, please add proper check. > + table = acpi_os_map_memory(rsdp_tbl->xsdt_physical_address, > + sizeof(struct acpi_table_header)); Ditto. > + /* 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. */ The coding style for multi-lines comment is: /* * For ACPI ... * .... */ > + 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); This is equivalent of BUG() > + 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/arch/arm/efi/efi-dom0.c b/xen/arch/arm/efi/efi-dom0.c > new file mode 100644 > index 0000000..90a7699 > --- /dev/null > +++ b/xen/arch/arm/efi/efi-dom0.c > @@ -0,0 +1,50 @@ > +/* > + * efi-dom0.c - Domain0 EFI Boot Support > + * > + * Copyright (C) 2016 Shannon Zhao <shannon.zhao@linaro.org> > + * > + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program; If not, see <http://www.gnu.org/licenses/>. > + * > + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > + */ > + > +#include "efi.h" > +#include "efi-dom0.h" > +#include <asm/setup.h> > +#include <asm/acpi.h> > + > +struct meminfo __initdata acpi_mem; I'm worry about sharing data between the EFI stub and Xen. On ARM, the EFI stub is completely independent. Once it has finished to execute, it will call xen as if it runs without UEFI. This means that BSS will be zeroed. I would have expect to see acpi_mem residing in BSS. So I'm wondering how this can work. > +/* Constant to indicate "Xen" in unicode u16 format */ > +static const CHAR16 xen_efi_fw_vendor[] = {0x0058, 0x0065, 0x006E, 0x0000}; > + > +size_t __init estimate_efi_size(int mem_nr_banks) > +{ > + size_t size; > + size_t est_size = sizeof(EFI_SYSTEM_TABLE); > + size_t ect_size = sizeof(EFI_CONFIGURATION_TABLE); > + size_t emd_size = sizeof(EFI_MEMORY_DESCRIPTOR); > + size_t fw_vendor_size = sizeof(xen_efi_fw_vendor); > + int acpi_mem_nr_banks = 0; > + > + if ( !acpi_disabled ) > + acpi_mem_nr_banks = acpi_mem.nr_banks; > + > + size = ROUNDUP(est_size + ect_size + fw_vendor_size, 8); > + /* plus 1 for new created tables */ > + size += ROUNDUP(emd_size * (mem_nr_banks + acpi_mem_nr_banks + 1), 8); > + > + return size; > +} Missing the classic emacs comment: /* * Local variables: * mode: C * c-file-style: "BSD" * c-basic-offset: 4 * indent-tabs-mode: nil * End: */ > diff --git a/xen/arch/arm/efi/efi-dom0.h b/xen/arch/arm/efi/efi-dom0.h > new file mode 100644 > index 0000000..a509cdd > --- /dev/null > +++ b/xen/arch/arm/efi/efi-dom0.h Why does this header live in arch/arm/efi instead of include/asm-arm? > @@ -0,0 +1,3 @@ The header should be properly guarded. > +#include <asm/setup.h> > + > +extern struct meminfo acpi_mem; > diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h > index 30ac53b..7f233a1 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); > > +size_t estimate_efi_size(int mem_nr_banks); > + > int construct_dom0(struct domain *d); > > void discard_initial_modules(void); > Regards,
Hi Shannon, On 17/03/2016 09:40, Shannon Zhao wrote: [...] > +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; I forgot to mention it on the last mail. If you re-use the gnttab region, you need to make sure that the region size will be enough to fit the ACPI tables. It will help for debugging if we reach the limit in the future. > + > + return 0; [...] Regards,
On 2016年03月19日 02:44, Julien Grall wrote: >> +++ b/xen/arch/arm/efi/efi-dom0.h > > Why does this header live in arch/arm/efi instead of include/asm-arm? Because this file only externs the acpi_mem and is included in the files under the same directory and there is no such same name file for x86. So I think it doesn't need to add it under include/asm-arm. Thanks,
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..4e20499 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) +{ + size_t efi_size, acpi_size, madt_size; + u64 addr; + struct acpi_table_rsdp *rsdp_tbl; + struct acpi_table_header *table; + + 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/arch/arm/efi/Makefile b/xen/arch/arm/efi/Makefile index 729e53e..d34c916 100644 --- a/xen/arch/arm/efi/Makefile +++ b/xen/arch/arm/efi/Makefile @@ -1,3 +1,4 @@ CFLAGS += -fshort-wchar obj-y += boot.init.o runtime.o +obj-$(CONFIG_ACPI) += efi-dom0.init.o diff --git a/xen/arch/arm/efi/efi-boot.h b/xen/arch/arm/efi/efi-boot.h index c58caca..045d6ce 100644 --- a/xen/arch/arm/efi/efi-boot.h +++ b/xen/arch/arm/efi/efi-boot.h @@ -7,6 +7,7 @@ #include <xen/libfdt/libfdt.h> #include <asm/setup.h> #include <asm/smp.h> +#include "efi-dom0.h" void noreturn efi_xen_start(void *fdt_ptr, uint32_t fdt_size); void __flush_dcache_area(const void *vaddr, unsigned long size); @@ -17,9 +18,6 @@ void __flush_dcache_area(const void *vaddr, unsigned long size); static struct file __initdata dtbfile; static void __initdata *fdt; static void __initdata *memmap; -#ifdef CONFIG_ACPI -static struct meminfo __initdata acpi_mem; -#endif static int __init setup_chosen_node(void *fdt, int *addr_cells, int *size_cells) { diff --git a/xen/arch/arm/efi/efi-dom0.c b/xen/arch/arm/efi/efi-dom0.c new file mode 100644 index 0000000..90a7699 --- /dev/null +++ b/xen/arch/arm/efi/efi-dom0.c @@ -0,0 +1,50 @@ +/* + * efi-dom0.c - Domain0 EFI Boot Support + * + * Copyright (C) 2016 Shannon Zhao <shannon.zhao@linaro.org> + * + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; If not, see <http://www.gnu.org/licenses/>. + * + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + */ + +#include "efi.h" +#include "efi-dom0.h" +#include <asm/setup.h> +#include <asm/acpi.h> + +struct meminfo __initdata acpi_mem; +/* Constant to indicate "Xen" in unicode u16 format */ +static const CHAR16 xen_efi_fw_vendor[] = {0x0058, 0x0065, 0x006E, 0x0000}; + +size_t __init estimate_efi_size(int mem_nr_banks) +{ + size_t size; + size_t est_size = sizeof(EFI_SYSTEM_TABLE); + size_t ect_size = sizeof(EFI_CONFIGURATION_TABLE); + size_t emd_size = sizeof(EFI_MEMORY_DESCRIPTOR); + size_t fw_vendor_size = sizeof(xen_efi_fw_vendor); + int acpi_mem_nr_banks = 0; + + if ( !acpi_disabled ) + acpi_mem_nr_banks = acpi_mem.nr_banks; + + size = ROUNDUP(est_size + ect_size + fw_vendor_size, 8); + /* plus 1 for new created tables */ + size += ROUNDUP(emd_size * (mem_nr_banks + acpi_mem_nr_banks + 1), 8); + + return size; +} diff --git a/xen/arch/arm/efi/efi-dom0.h b/xen/arch/arm/efi/efi-dom0.h new file mode 100644 index 0000000..a509cdd --- /dev/null +++ b/xen/arch/arm/efi/efi-dom0.h @@ -0,0 +1,3 @@ +#include <asm/setup.h> + +extern struct meminfo acpi_mem; diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h index 30ac53b..7f233a1 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); +size_t estimate_efi_size(int mem_nr_banks); + int construct_dom0(struct domain *d); void discard_initial_modules(void);