Re: [PATCH v3 06/17] libxl/arm: Estimate the size of ACPI tables

[Wei Liu <wei.liu2@citrix.com>]

On Tue, Jul 05, 2016 at 11:12:36AM +0800, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> Estimate the size of ACPI tables and reserve a memory map space for ACPI
> tables.
> 
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>

I will leave this patch to Stefano and Julien.

> ---
>  tools/libxl/libxl_arm_acpi.c  | 85 +++++++++++++++++++++++++++++++++++++++++++
>  xen/include/public/arch-arm.h |  4 ++
>  2 files changed, 89 insertions(+)
> 
> diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
> index d1c066d..7a59126 100644
> --- a/tools/libxl/libxl_arm_acpi.c
> +++ b/tools/libxl/libxl_arm_acpi.c
> @@ -33,11 +33,92 @@ extern const unsigned char dsdt_anycpu_arm[];
>  _hidden
>  extern const int dsdt_anycpu_arm_len;
>  
> +enum {
> +    RSDP,
> +    XSDT,
> +    GTDT,
> +    MADT,
> +    FADT,
> +    DSDT,
> +    NUMS,
> +};
> +
> +struct acpitable {
> +    uint64_t addr;
> +    size_t size;
> +};
> +
> +static int libxl__estimate_acpi_size(libxl__gc *gc,
> +                                     libxl_domain_build_info *info,
> +                                     struct xc_dom_image *dom,
> +                                     xc_domain_configuration_t *xc_config,
> +                                     struct acpitable acpitables[])
> +{
> +    uint64_t size;
> +
> +    acpitables[RSDP].addr = GUEST_ACPI_BASE;
> +    acpitables[RSDP].size = sizeof(struct acpi_table_rsdp);
> +    dom->acpitable_size += ROUNDUP(acpitables[RSDP].size, 3);
> +
> +    acpitables[XSDT].addr = GUEST_ACPI_BASE + dom->acpitable_size;
> +    /*
> +     * Currently only 3 tables(GTDT, FADT, MADT) are pointed by XSDT. Alloc
> +     * entries for them.
> +     */
> +    acpitables[XSDT].size = sizeof(struct acpi_table_xsdt) +
> +                            sizeof(uint64_t) * 2;
> +    dom->acpitable_size += ROUNDUP(acpitables[XSDT].size, 3);
> +
> +    acpitables[GTDT].addr = GUEST_ACPI_BASE + dom->acpitable_size;
> +    acpitables[GTDT].size = sizeof(struct acpi_table_gtdt);
> +    dom->acpitable_size += ROUNDUP(acpitables[GTDT].size, 3);
> +
> +    acpitables[MADT].addr = GUEST_ACPI_BASE + dom->acpitable_size;
> +
> +    switch (xc_config->gic_version) {
> +    case XEN_DOMCTL_CONFIG_GIC_V2:
> +        size = sizeof(struct acpi_table_madt) +
> +               sizeof(struct acpi_madt_generic_interrupt) * info->max_vcpus +
> +               sizeof(struct acpi_madt_generic_distributor);
> +        break;
> +    case XEN_DOMCTL_CONFIG_GIC_V3:
> +        size = sizeof(struct acpi_table_madt) +
> +               sizeof(struct acpi_madt_generic_interrupt) * info->max_vcpus +
> +               sizeof(struct acpi_madt_generic_distributor) +
> +               sizeof(struct acpi_madt_generic_redistributor);
> +        break;
> +    default:
> +        LOG(ERROR, "Unknown GIC version");
> +        return ERROR_FAIL;
> +    }
> +
> +    acpitables[MADT].size = size;
> +    dom->acpitable_size += ROUNDUP(acpitables[MADT].size, 3);
> +
> +    acpitables[FADT].addr = GUEST_ACPI_BASE + dom->acpitable_size;
> +    acpitables[FADT].size = sizeof(struct acpi_table_fadt);
> +    dom->acpitable_size += ROUNDUP(acpitables[FADT].size, 3);
> +
> +    acpitables[DSDT].addr = GUEST_ACPI_BASE + dom->acpitable_size;
> +    acpitables[DSDT].size = dsdt_anycpu_arm_len;
> +    dom->acpitable_size += ROUNDUP(acpitables[DSDT].size, 3);
> +
> +    assert(dom->acpitable_size <= GUEST_ACPI_SIZE);
> +    dom->acpitable_blob = libxl__zalloc(gc, dom->acpitable_size);
> +

Goto style error handling.

> +    return 0;
> +}
> +
>  int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
>                          libxl__domain_build_state *state,
>                          struct xc_dom_image *dom)
>  {
>      const libxl_version_info *vers;
> +    int rc;
> +    struct acpitable acpitables[NUMS];
> +
> +    /* convenience aliases */
> +    xc_domain_configuration_t *xc_config = &state->config;
>  

Julien seemed to have suggested this structure shouldn't be used. Did I
misremember?

>      vers = libxl_get_version_info(CTX);
>      if (vers == NULL)
> @@ -49,6 +130,10 @@ int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
>      dom->acpitable_blob = NULL;
>      dom->acpitable_size = 0;
>  
> +    rc = libxl__estimate_acpi_size(gc, info, dom, xc_config, acpitables);
> +    if (rc)
> +        return rc;
> +
>      return 0;
>  }

Goto style error handling please.

>  
> diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
> index 4a49254..008a2a0 100644
> --- a/xen/include/public/arch-arm.h
> +++ b/xen/include/public/arch-arm.h
> @@ -406,6 +406,10 @@ typedef uint64_t xen_callback_t;
>  #define GUEST_GICV3_GICR0_BASE     0x03020000ULL    /* vCPU0 - vCPU127 */
>  #define GUEST_GICV3_GICR0_SIZE     0x01000000ULL
>  
> +/* ACPI tables physical address */
> +#define GUEST_ACPI_BASE 0x20000000ULL
> +#define GUEST_ACPI_SIZE 0x00200000ULL
> +
>  /*
>   * 16MB == 4096 pages reserved for guest to use as a region to map its
>   * grant table in.
> -- 
> 2.0.4
> 
> 

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