Re: [PATCH v4 04/24] arm/acpi: Estimate memory required for acpi/efi tables

[Shannon Zhao <zhaoshenglong@huawei.com>]

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,
-- 
Shannon


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