Message ID | 20201119121347.27139-2-julien@xen.org (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | xen/arm: Allow Xen to boot with ACPI 5.1 | expand |
Hi Julien, > On 19 Nov 2020, at 12:13, Julien Grall <julien@xen.org> wrote: > > From: Julien Grall <julien.grall@arm.com> > > The length of the GICC structure in the MADT ACPI table differs between > version 5.1 and 6.0, although there are no other relevant differences. > > Use the BAD_MADT_GICC_ENTRY macro, which was specifically designed to > overcome this issue. > The serie is braking the build on arm64 if ACPI is not activated: arch/arm/gic.c:456: undefined reference to `acpi_gbl_FADT’ Cheers Bertrand > Signed-off-by: Julien Grall <julien.grall@arm.com> > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > Signed-off-by: Julien Grall <jgrall@amazon.com> > > --- > Changes in v3: > - Update the commit title as we also modify GICv3 code > - Use the correct length in more places > > Changes in v2: > - Patch added > --- > xen/arch/arm/acpi/boot.c | 2 +- > xen/arch/arm/gic-v2.c | 5 +++-- > xen/arch/arm/gic-v3.c | 6 +++--- > xen/arch/arm/gic.c | 2 +- > 4 files changed, 8 insertions(+), 7 deletions(-) > > diff --git a/xen/arch/arm/acpi/boot.c b/xen/arch/arm/acpi/boot.c > index 30e4bd1bc5a7..55c3e5cbc834 100644 > --- a/xen/arch/arm/acpi/boot.c > +++ b/xen/arch/arm/acpi/boot.c > @@ -131,7 +131,7 @@ acpi_parse_gic_cpu_interface(struct acpi_subtable_header *header, > struct acpi_madt_generic_interrupt *processor = > container_of(header, struct acpi_madt_generic_interrupt, header); > > - if ( BAD_MADT_ENTRY(processor, end) ) > + if ( BAD_MADT_GICC_ENTRY(processor, end) ) > return -EINVAL; > > acpi_table_print_madt_entry(header); > diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c > index 0f747538dbcd..0e5f23201974 100644 > --- a/xen/arch/arm/gic-v2.c > +++ b/xen/arch/arm/gic-v2.c > @@ -1136,7 +1136,8 @@ static int gicv2_make_hwdom_madt(const struct domain *d, u32 offset) > > host_gicc = container_of(header, struct acpi_madt_generic_interrupt, > header); > - size = sizeof(struct acpi_madt_generic_interrupt); > + > + size = ACPI_MADT_GICC_LENGTH; > /* Add Generic Interrupt */ > for ( i = 0; i < d->max_vcpus; i++ ) > { > @@ -1165,7 +1166,7 @@ gic_acpi_parse_madt_cpu(struct acpi_subtable_header *header, > struct acpi_madt_generic_interrupt *processor = > container_of(header, struct acpi_madt_generic_interrupt, header); > > - if ( BAD_MADT_ENTRY(processor, end) ) > + if ( BAD_MADT_GICC_ENTRY(processor, end) ) > return -EINVAL; > > /* Read from APIC table and fill up the GIC variables */ > diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c > index 0f6cbf6224e9..483ec1598f37 100644 > --- a/xen/arch/arm/gic-v3.c > +++ b/xen/arch/arm/gic-v3.c > @@ -1499,7 +1499,7 @@ static int gicv3_make_hwdom_madt(const struct domain *d, u32 offset) > > host_gicc = container_of(header, struct acpi_madt_generic_interrupt, > header); > - size = sizeof(struct acpi_madt_generic_interrupt); > + size = ACPI_MADT_GICC_LENGTH; > for ( i = 0; i < d->max_vcpus; i++ ) > { > gicc = (struct acpi_madt_generic_interrupt *)(base_ptr + table_len); > @@ -1558,7 +1558,7 @@ gic_acpi_parse_madt_cpu(struct acpi_subtable_header *header, > struct acpi_madt_generic_interrupt *processor = > container_of(header, struct acpi_madt_generic_interrupt, header); > > - if ( BAD_MADT_ENTRY(processor, end) ) > + if ( BAD_MADT_GICC_ENTRY(processor, end) ) > return -EINVAL; > > /* Read from APIC table and fill up the GIC variables */ > @@ -1628,7 +1628,7 @@ gic_acpi_get_madt_cpu_num(struct acpi_subtable_header *header, > struct acpi_madt_generic_interrupt *cpuif; > > cpuif = (struct acpi_madt_generic_interrupt *)header; > - if ( BAD_MADT_ENTRY(cpuif, end) || !cpuif->gicr_base_address ) > + if ( BAD_MADT_GICC_ENTRY(cpuif, end) || !cpuif->gicr_base_address ) > return -EINVAL; > > return 0; > diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c > index d623c57cb9fa..b100c85ef314 100644 > --- a/xen/arch/arm/gic.c > +++ b/xen/arch/arm/gic.c > @@ -453,7 +453,7 @@ unsigned long gic_get_hwdom_madt_size(const struct domain *d) > unsigned long madt_size; > > madt_size = sizeof(struct acpi_table_madt) > - + sizeof(struct acpi_madt_generic_interrupt) * d->max_vcpus > + + ACPI_MADT_GICC_LENGTH * d->max_vcpus > + sizeof(struct acpi_madt_generic_distributor) > + gic_hw_ops->get_hwdom_extra_madt_size(d); > > -- > 2.17.1 >
On 19/11/2020 15:22, Bertrand Marquis wrote: > Hi Julien, Hi Bertrand, > >> On 19 Nov 2020, at 12:13, Julien Grall <julien@xen.org> wrote: >> >> From: Julien Grall <julien.grall@arm.com> >> >> The length of the GICC structure in the MADT ACPI table differs between >> version 5.1 and 6.0, although there are no other relevant differences. >> >> Use the BAD_MADT_GICC_ENTRY macro, which was specifically designed to >> overcome this issue. >> > > The serie is braking the build on arm64 if ACPI is not activated: > arch/arm/gic.c:456: undefined reference to `acpi_gbl_FADT’ :(. Sorry for that. I usually test with and without ACPI enabled but forgot to do for this series. Cheers,
On Thu, 19 Nov 2020, Julien Grall wrote: > On 19/11/2020 15:22, Bertrand Marquis wrote: > > Hi Julien, > > Hi Bertrand, > > > > > > On 19 Nov 2020, at 12:13, Julien Grall <julien@xen.org> wrote: > > > > > > From: Julien Grall <julien.grall@arm.com> > > > > > > The length of the GICC structure in the MADT ACPI table differs between > > > version 5.1 and 6.0, although there are no other relevant differences. > > > > > > Use the BAD_MADT_GICC_ENTRY macro, which was specifically designed to > > > overcome this issue. > > > > > > > The serie is braking the build on arm64 if ACPI is not activated: > > arch/arm/gic.c:456: undefined reference to `acpi_gbl_FADT’ > > :(. Sorry for that. I usually test with and without ACPI enabled but forgot to > do for this series. I just wanted to add that I checked and it looks like to converted all places now
diff --git a/xen/arch/arm/acpi/boot.c b/xen/arch/arm/acpi/boot.c index 30e4bd1bc5a7..55c3e5cbc834 100644 --- a/xen/arch/arm/acpi/boot.c +++ b/xen/arch/arm/acpi/boot.c @@ -131,7 +131,7 @@ acpi_parse_gic_cpu_interface(struct acpi_subtable_header *header, struct acpi_madt_generic_interrupt *processor = container_of(header, struct acpi_madt_generic_interrupt, header); - if ( BAD_MADT_ENTRY(processor, end) ) + if ( BAD_MADT_GICC_ENTRY(processor, end) ) return -EINVAL; acpi_table_print_madt_entry(header); diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c index 0f747538dbcd..0e5f23201974 100644 --- a/xen/arch/arm/gic-v2.c +++ b/xen/arch/arm/gic-v2.c @@ -1136,7 +1136,8 @@ static int gicv2_make_hwdom_madt(const struct domain *d, u32 offset) host_gicc = container_of(header, struct acpi_madt_generic_interrupt, header); - size = sizeof(struct acpi_madt_generic_interrupt); + + size = ACPI_MADT_GICC_LENGTH; /* Add Generic Interrupt */ for ( i = 0; i < d->max_vcpus; i++ ) { @@ -1165,7 +1166,7 @@ gic_acpi_parse_madt_cpu(struct acpi_subtable_header *header, struct acpi_madt_generic_interrupt *processor = container_of(header, struct acpi_madt_generic_interrupt, header); - if ( BAD_MADT_ENTRY(processor, end) ) + if ( BAD_MADT_GICC_ENTRY(processor, end) ) return -EINVAL; /* Read from APIC table and fill up the GIC variables */ diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c index 0f6cbf6224e9..483ec1598f37 100644 --- a/xen/arch/arm/gic-v3.c +++ b/xen/arch/arm/gic-v3.c @@ -1499,7 +1499,7 @@ static int gicv3_make_hwdom_madt(const struct domain *d, u32 offset) host_gicc = container_of(header, struct acpi_madt_generic_interrupt, header); - size = sizeof(struct acpi_madt_generic_interrupt); + size = ACPI_MADT_GICC_LENGTH; for ( i = 0; i < d->max_vcpus; i++ ) { gicc = (struct acpi_madt_generic_interrupt *)(base_ptr + table_len); @@ -1558,7 +1558,7 @@ gic_acpi_parse_madt_cpu(struct acpi_subtable_header *header, struct acpi_madt_generic_interrupt *processor = container_of(header, struct acpi_madt_generic_interrupt, header); - if ( BAD_MADT_ENTRY(processor, end) ) + if ( BAD_MADT_GICC_ENTRY(processor, end) ) return -EINVAL; /* Read from APIC table and fill up the GIC variables */ @@ -1628,7 +1628,7 @@ gic_acpi_get_madt_cpu_num(struct acpi_subtable_header *header, struct acpi_madt_generic_interrupt *cpuif; cpuif = (struct acpi_madt_generic_interrupt *)header; - if ( BAD_MADT_ENTRY(cpuif, end) || !cpuif->gicr_base_address ) + if ( BAD_MADT_GICC_ENTRY(cpuif, end) || !cpuif->gicr_base_address ) return -EINVAL; return 0; diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c index d623c57cb9fa..b100c85ef314 100644 --- a/xen/arch/arm/gic.c +++ b/xen/arch/arm/gic.c @@ -453,7 +453,7 @@ unsigned long gic_get_hwdom_madt_size(const struct domain *d) unsigned long madt_size; madt_size = sizeof(struct acpi_table_madt) - + sizeof(struct acpi_madt_generic_interrupt) * d->max_vcpus + + ACPI_MADT_GICC_LENGTH * d->max_vcpus + sizeof(struct acpi_madt_generic_distributor) + gic_hw_ops->get_hwdom_extra_madt_size(d);