Re: [Xen-devel] [PATCH v3 11/17] libxl/arm: Construct ACPI MADT table

[Julien Grall <julien.grall@xxxxxxx>]

Hi Wei,

On 07/07/16 17:11, Wei Liu wrote:
> On Tue, Jul 05, 2016 at 11:12:41AM +0800, Shannon Zhao wrote:
> > From: Shannon Zhao <shannon.zhao@xxxxxxxxxx>
> >
> > According to the GIC version, construct the MADT table.
> >
> > Signed-off-by: Shannon Zhao <shannon.zhao@xxxxxxxxxx>
> > ---
> >   tools/libxl/libxl_arm_acpi.c | 83 ++++++++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 83 insertions(+)
> >
> > diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
> > index c2599b7..96ce605 100644
> > --- a/tools/libxl/libxl_arm_acpi.c
> > +++ b/tools/libxl/libxl_arm_acpi.c
> > @@ -199,6 +199,86 @@ static void make_acpi_gtdt(libxl__gc *gc, struct 
> > xc_dom_image *dom,
> >                          acpitables[GTDT].size);
> >   }
> >
> > +static void make_acpi_madt_gicc(void *table, int nr_cpus, uint64_t gicc_base)
> > +{
> > +    uint32_t i;
> > +    struct acpi_madt_generic_interrupt *gicc = table;
> > +
> > +    for (i = 0; i < nr_cpus; i++) {
> > +        gicc->header.type = ACPI_MADT_TYPE_GENERIC_INTERRUPT;
> > +        gicc->header.length = sizeof(*gicc);
> > +        gicc->base_address = gicc_base;
> > +        gicc->cpu_interface_number = i;
> > +        gicc->arm_mpidr = libxl__compute_mpdir(i);
> > +        gicc->uid = i;
> > +        gicc->flags = ACPI_MADT_ENABLED;
> > +        gicc++;
> > +    }
> > +}
> > +
> > +static void make_acpi_madt_gicd(void *table, uint64_t gicd_base,
> > +                                uint8_t gic_version)
> > +{
> > +    struct acpi_madt_generic_distributor *gicd = table;
> > +
> > +    gicd->header.type = ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR;
> > +    gicd->header.length = sizeof(*gicd);
> > +    gicd->base_address = gicd_base;
> > +    /* This version field has no meaning before ACPI 5.1 errata. */
> > +    gicd->version = gic_version;
> > +}
> > +
> > +static void make_acpi_madt_gicr(void *table, uint64_t gicr_base,
> > +                                uint64_t gicr_size)
> > +{
> > +    struct acpi_madt_generic_redistributor *gicr = table;
> > +
> > +    gicr->header.type = ACPI_MADT_TYPE_GENERIC_REDISTRIBUTOR;
> > +    gicr->header.length = sizeof(*gicr);
> > +    gicr->base_address = gicr_base;
> > +    gicr->length = gicr_size;
> > +}
> > +
> > +static int make_acpi_madt(libxl__gc *gc, struct xc_dom_image *dom, int nr_cpus,
> > +                          xc_domain_configuration_t *xc_config,
> > +                          struct acpitable acpitables[])
> > +{
> > +    uint64_t offset = acpitables[MADT].addr - GUEST_ACPI_BASE;
> > +    struct acpi_table_madt *madt = dom->acpitable_blob + offset;
> > +    void *table = dom->acpitable_blob + offset;
> > +
> > +    switch (xc_config->gic_version) {
> > +    case XEN_DOMCTL_CONFIG_GIC_V2:
> > +        table += sizeof(struct acpi_table_madt);
> > +        make_acpi_madt_gicc(table, nr_cpus, GUEST_GICC_BASE);
> > +
> > +        table += sizeof(struct acpi_madt_generic_interrupt) * nr_cpus;
> > +        make_acpi_madt_gicd(table, GUEST_GICD_BASE, ACPI_MADT_GIC_VERSION_V2);
> > +        break;
> > +    case XEN_DOMCTL_CONFIG_GIC_V3:
> > +        table += sizeof(struct acpi_table_madt);
> > +        make_acpi_madt_gicc(table, nr_cpus, 0);
> > +
> > +        table += sizeof(struct acpi_madt_generic_interrupt) * nr_cpus;
> > +        make_acpi_madt_gicd(table, GUEST_GICV3_GICD_BASE,
> > +                            ACPI_MADT_GIC_VERSION_V3);
> > +
> > +        table += sizeof(struct acpi_madt_generic_distributor);
> > +        make_acpi_madt_gicr(table, GUEST_GICV3_GICR0_BASE,
> > +                            GUEST_GICV3_GICR0_SIZE);
> > +        break;
> > +    default:
> > +        LOG(ERROR, "Unknown GIC version");
> > +        return ERROR_FAIL;
> > +    }
> > +
> Why is this code snippet referencing libxc structure? I would think all
> relevant information is already available in libxl, right?
The hypervisor may choose the version of the GIC emulated if the user 
did not specified one. Those values are not replicated in libxl because 
this will be encoded in the hvm records during the migration (see the 
thread on the patch which introduced xc_config [1]).
FWIW, this is similar to what is done for the device tree case.

> > +    make_acpi_header(&madt->header, "APIC", acpitables[MADT].size, 3);
> > +    calculate_checksum(madt, offsetof(struct acpi_table_header, checksum),
> > +                       acpitables[MADT].size);
> > +
> > +    return 0;
> > +}
> > +
> >   int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
> >                           libxl__domain_build_state *state,
> >                           struct xc_dom_image *dom)
> > @@ -227,6 +307,9 @@ int libxl__prepare_acpi(libxl__gc *gc, 
> > libxl_domain_build_info *info,
> >       make_acpi_rsdp(gc, dom, acpitables);
> >       make_acpi_xsdt(gc, dom, acpitables);
> >       make_acpi_gtdt(gc, dom, acpitables);
> > +    rc = make_acpi_madt(gc, dom, info->max_vcpus, xc_config, acpitables);
> > +    if (rc)
> > +       return rc;
> >
> >       return 0;
> >   }
> > --
> > 2.0.4
[1] https://lists.xen.org/archives/html/xen-devel/2015-02/msg02538.html

--
Julien Grall

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