On 09.01.2026 10:27, Oleksii Kurochko wrote: > Newer ACPI revisions define the MADT GICC entry with Length = 82 bytes [1]. > The current BAD_MADT_GICC_ENTRY() check rejects entries whose length does not > match the known values, which leads to: > GICv3: No valid GICC entries exist. > as observed on the AmpereOne platform. > > To fix this, import the logic from Linux commit 9eb1c92b47c7: > The BAD_MADT_GICC_ENTRY check is a little too strict because > it rejects MADT entries that don't match the currently known > lengths. We should remove this restriction to avoid problems > if the table length changes. Future code which might depend on > additional fields should be written to validate those fields > before using them, rather than trying to globally check > known MADT version lengths. > > Link: > https://lkml.kernel.org/r/[email protected] > Signed-off-by: Jeremy Linton <[email protected]> > [[email protected]: added MADT macro comments] > Signed-off-by: Lorenzo Pieralisi <[email protected]> > Acked-by: Sudeep Holla <[email protected]> > Cc: Will Deacon <[email protected]> > Cc: Catalin Marinas <[email protected]> > Cc: Al Stone <[email protected]> > Cc: "Rafael J. Wysocki" <[email protected]> > Signed-off-by: Will Deacon <[email protected]> > > As ACPI_MADT_GICC_LENGTH is dropped, update the functions where it is > used. As we rewrite the MADT for hwdom, reuse the host GICC header length > instead of ACPI_MADT_GICC_LENGTH. > > Mark gic_get_hwdom_madt_size() as __init since its only caller is also > __init. > > [1] > https://uefi.org/specs/ACPI/6.6/05_ACPI_Software_Programming_Model.html#gic-cpu-interface-gicc-structure > > Reported-By: Yann Dirson <[email protected]> > Co-developed-by: Yann Sionneau <[email protected]> > Signed-off-by: Oleksii Kurochko <[email protected]> > --- > I ran CI tests where it made sense for this patch, as I don’t see any CI job > that builds Xen with CONFIG_ACPI=y: > https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/2252409762 > > I also built Xen manually with CONFIG_ACPI=y enabled and tested it on the > AmpereOne platform. > --- > xen/arch/arm/acpi/domain_build.c | 6 ++++++ > xen/arch/arm/gic-v2.c | 3 ++- > xen/arch/arm/gic-v3.c | 3 ++- > xen/arch/arm/gic.c | 13 +++++++++++-- > xen/arch/arm/include/asm/acpi.h | 21 +++++++++++++++------ > 5 files changed, 36 insertions(+), 10 deletions(-) > > diff --git a/xen/arch/arm/acpi/domain_build.c > b/xen/arch/arm/acpi/domain_build.c > index 1c3555d814cc..959698d13ac3 100644 > --- a/xen/arch/arm/acpi/domain_build.c > +++ b/xen/arch/arm/acpi/domain_build.c > @@ -458,6 +458,12 @@ static int __init estimate_acpi_efi_size(struct domain > *d, > acpi_size += ROUNDUP(sizeof(struct acpi_table_stao), 8); > > madt_size = gic_get_hwdom_madt_size(d); > + if ( !madt_size ) > + { > + printk("Unable to get hwdom MADT size\n"); > + return -EINVAL; > + } > + > acpi_size += ROUNDUP(madt_size, 8); > > addr = acpi_os_get_root_pointer(); > diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c > index b23e72a3d05d..aae6a7bf3076 100644 > --- a/xen/arch/arm/gic-v2.c > +++ b/xen/arch/arm/gic-v2.c > @@ -1121,7 +1121,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 = ACPI_MADT_GICC_LENGTH; > + size = host_gicc->header.length; > + > /* Add Generic Interrupt */ > for ( i = 0; i < d->max_vcpus; i++ ) > { > diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c > index bc07f97c16ab..75b89efad462 100644 > --- a/xen/arch/arm/gic-v3.c > +++ b/xen/arch/arm/gic-v3.c > @@ -1672,7 +1672,8 @@ static int gicv3_make_hwdom_madt(const struct domain > *d, u32 offset) > > host_gicc = container_of(header, struct acpi_madt_generic_interrupt, > header); > - size = ACPI_MADT_GICC_LENGTH; > + size = host_gicc->header.length; > + > for ( i = 0; i < d->max_vcpus; i++ ) > { > gicc = (struct acpi_madt_generic_interrupt *)(base_ptr + table_len); > diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c > index ee75258fc3c3..e4fcfd60205d 100644 > --- a/xen/arch/arm/gic.c > +++ b/xen/arch/arm/gic.c > @@ -414,12 +414,21 @@ int gic_make_hwdom_madt(const struct domain *d, u32 > offset) > return gic_hw_ops->make_hwdom_madt(d, offset); > } > > -unsigned long gic_get_hwdom_madt_size(const struct domain *d) > +unsigned long __init gic_get_hwdom_madt_size(const struct domain *d) > { > unsigned long madt_size; > + const struct acpi_subtable_header *header; > + const struct acpi_madt_generic_interrupt *host_gicc; > + > + header = acpi_table_get_entry_madt(ACPI_MADT_TYPE_GENERIC_INTERRUPT, 0); > + if ( !header ) > + return 0; > + > + host_gicc = container_of(header, const struct > acpi_madt_generic_interrupt, > + header); > > madt_size = sizeof(struct acpi_table_madt) > - + ACPI_MADT_GICC_LENGTH * d->max_vcpus > + + host_gicc->header.length * d->max_vcpus
Just to double-check: All entries are strictly required to be of the same length? (Related question further down.) > --- a/xen/arch/arm/include/asm/acpi.h > +++ b/xen/arch/arm/include/asm/acpi.h > @@ -53,13 +53,22 @@ void acpi_smp_init_cpus(void); > */ > paddr_t acpi_get_table_offset(struct membank tbl_add[], EFI_MEM_RES index); > > -/* Macros for consistency checks of the GICC subtable of MADT */ > -#define ACPI_MADT_GICC_LENGTH \ > - (acpi_gbl_FADT.header.revision < 6 ? 76 : 80) Given this, ... > +/* > + * MADT GICC minimum length refers to the MADT GICC structure table length as > + * defined in the earliest ACPI version supported on arm64, ie ACPI 5.1. > + * > + * The efficiency_class member was added to the > + * struct acpi_madt_generic_interrupt to represent the MADT GICC structure > + * "Processor Power Efficiency Class" field, added in ACPI 6.0 whose offset > + * is therefore used to delimit the MADT GICC structure minimum length > + * appropriately. > + */ > +#define ACPI_MADT_GICC_MIN_LENGTH ACPI_OFFSET( \ > + struct acpi_madt_generic_interrupt, efficiency_class) > > -#define BAD_MADT_GICC_ENTRY(entry, end) > \ > - (!(entry) || (unsigned long)(entry) + sizeof(*(entry)) > (end) || > \ > - (entry)->header.length != ACPI_MADT_GICC_LENGTH) > +#define BAD_MADT_GICC_ENTRY(entry, end) \ > + (!(entry) || (entry)->header.length < ACPI_MADT_GICC_MIN_LENGTH || \ > + (unsigned long)(entry) + (entry)->header.length > (end)) ... is 76 a valid length when the FADT revision is 6 or higher? And 80 is a valid length for 6.5 or higher? Jan
