| Message ID | 20240123110332.112797-2-lpieralisi@kernel.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | irqchip/gic-v3: Enable non-coherent GIC designs probing | expand |
On Tue, Jan 23, 2024 at 12:03:32PM +0100, Lorenzo Pieralisi wrote: > The GIC architecture specification defines a set of registers for > redistributors and ITSes that control the sharebility and cacheability > attributes of redistributors/ITSes initiator ports on the interconnect > (GICR_[V]PROPBASER, GICR_[V]PENDBASER, GITS_BASER<n>). > > Architecturally the GIC provides a means to drive shareability and > cacheability attributes signals but it is not mandatory for designs to > wire up the corresponding interconnect signals that control the > cacheability/shareability of transactions. > > Redistributors and ITSes interconnect ports can be connected to > non-coherent interconnects that are not able to manage the > shareability/cacheability attributes; this implicitly makes the > redistributors and ITSes non-coherent observers. > > To enable non-coherent GIC designs on ACPI based systems, parse the MADT > GICC/GICR/ITS subtables non-coherent flags to determine whether the > respective components are non-coherent observers and force the > shareability attributes to be programmed into the redistributors and > ITSes registers. > > An ACPI global function (acpi_get_madt_revision()) is added to retrieve > the MADT revision, in that it is essential to check the MADT revision > before checking for flags that were added with MADT revision 7 so that > if the kernel is booted with an ACPI MADT table with revision < 7 it > skips parsing the newly added flags (that should be zeroed reserved > values for MADT versions < 7 but they could turn out to be buggy and > should be ignored). > > Signed-off-by: Lorenzo Pieralisi <lpieralisi@kernel.org> > Cc: Robin Murphy <robin.murphy@arm.com> > Cc: Mark Rutland <mark.rutland@arm.com> > Cc: "Rafael J. Wysocki" <rafael@kernel.org> > Cc: Marc Zyngier <maz@kernel.org> > --- > drivers/acpi/processor_core.c | 15 +++++++++++++++ > drivers/irqchip/irq-gic-v3-its.c | 4 ++++ > drivers/irqchip/irq-gic-v3.c | 9 +++++++++ > include/linux/acpi.h | 3 +++ > 4 files changed, 31 insertions(+) Hi Marc, Rafael, I would kindly ask you please what to do with this patch, it still applies to v6.9-rc5 - I can resend it if needed, ACPICA changes are already merged as-per the cover letter. Thanks, Lorenzo > diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c > index b203cfe28550..915713c0e9b7 100644 > --- a/drivers/acpi/processor_core.c > +++ b/drivers/acpi/processor_core.c > @@ -215,6 +215,21 @@ phys_cpuid_t __init acpi_map_madt_entry(u32 acpi_id) > return rv; > } > > +int __init acpi_get_madt_revision(void) > +{ > + struct acpi_table_header *madt = NULL; > + int revision; > + > + if (ACPI_FAILURE(acpi_get_table(ACPI_SIG_MADT, 0, &madt))) > + return -EINVAL; > + > + revision = madt->revision; > + > + acpi_put_table(madt); > + > + return revision; > +} > + > static phys_cpuid_t map_mat_entry(acpi_handle handle, int type, u32 acpi_id) > { > struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL }; > diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c > index fec1b58470df..a60c560ce891 100644 > --- a/drivers/irqchip/irq-gic-v3-its.c > +++ b/drivers/irqchip/irq-gic-v3-its.c > @@ -5591,6 +5591,10 @@ static int __init gic_acpi_parse_madt_its(union acpi_subtable_headers *header, > goto node_err; > } > > + if (acpi_get_madt_revision() >= 7 && > + (its_entry->flags & ACPI_MADT_ITS_NON_COHERENT)) > + its->flags |= ITS_FLAGS_FORCE_NON_SHAREABLE; > + > err = its_probe_one(its); > if (!err) > return 0; > diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c > index 98b0329b7154..8cb8dff86c12 100644 > --- a/drivers/irqchip/irq-gic-v3.c > +++ b/drivers/irqchip/irq-gic-v3.c > @@ -2356,6 +2356,11 @@ gic_acpi_parse_madt_redist(union acpi_subtable_headers *header, > pr_err("Couldn't map GICR region @%llx\n", redist->base_address); > return -ENOMEM; > } > + > + if (acpi_get_madt_revision() >= 7 && > + (redist->flags & ACPI_MADT_GICR_NON_COHERENT)) > + gic_data.rdists.flags |= RDIST_FLAGS_FORCE_NON_SHAREABLE; > + > gic_request_region(redist->base_address, redist->length, "GICR"); > > gic_acpi_register_redist(redist->base_address, redist_base); > @@ -2380,6 +2385,10 @@ gic_acpi_parse_madt_gicc(union acpi_subtable_headers *header, > return -ENOMEM; > gic_request_region(gicc->gicr_base_address, size, "GICR"); > > + if (acpi_get_madt_revision() >= 7 && > + (gicc->flags & ACPI_MADT_GICC_NON_COHERENT)) > + gic_data.rdists.flags |= RDIST_FLAGS_FORCE_NON_SHAREABLE; > + > gic_acpi_register_redist(gicc->gicr_base_address, redist_base); > return 0; > } > diff --git a/include/linux/acpi.h b/include/linux/acpi.h > index b7165e52b3c6..4eedab0e51c3 100644 > --- a/include/linux/acpi.h > +++ b/include/linux/acpi.h > @@ -284,6 +284,9 @@ static inline bool invalid_phys_cpuid(phys_cpuid_t phys_id) > return phys_id == PHYS_CPUID_INVALID; > } > > + > +int __init acpi_get_madt_revision(void); > + > /* Validate the processor object's proc_id */ > bool acpi_duplicate_processor_id(int proc_id); > /* Processor _CTS control */ > -- > 2.34.1 >
On Mon, Apr 22, 2024 at 10:42:19AM +0200, Lorenzo Pieralisi wrote: > On Tue, Jan 23, 2024 at 12:03:32PM +0100, Lorenzo Pieralisi wrote: > > The GIC architecture specification defines a set of registers for > > redistributors and ITSes that control the sharebility and cacheability > > attributes of redistributors/ITSes initiator ports on the interconnect > > (GICR_[V]PROPBASER, GICR_[V]PENDBASER, GITS_BASER<n>). > > > > Architecturally the GIC provides a means to drive shareability and > > cacheability attributes signals but it is not mandatory for designs to > > wire up the corresponding interconnect signals that control the > > cacheability/shareability of transactions. > > > > Redistributors and ITSes interconnect ports can be connected to > > non-coherent interconnects that are not able to manage the > > shareability/cacheability attributes; this implicitly makes the > > redistributors and ITSes non-coherent observers. > > > > To enable non-coherent GIC designs on ACPI based systems, parse the MADT > > GICC/GICR/ITS subtables non-coherent flags to determine whether the > > respective components are non-coherent observers and force the > > shareability attributes to be programmed into the redistributors and > > ITSes registers. > > > > An ACPI global function (acpi_get_madt_revision()) is added to retrieve > > the MADT revision, in that it is essential to check the MADT revision > > before checking for flags that were added with MADT revision 7 so that > > if the kernel is booted with an ACPI MADT table with revision < 7 it > > skips parsing the newly added flags (that should be zeroed reserved > > values for MADT versions < 7 but they could turn out to be buggy and > > should be ignored). > > > > Signed-off-by: Lorenzo Pieralisi <lpieralisi@kernel.org> > > Cc: Robin Murphy <robin.murphy@arm.com> > > Cc: Mark Rutland <mark.rutland@arm.com> > > Cc: "Rafael J. Wysocki" <rafael@kernel.org> > > Cc: Marc Zyngier <maz@kernel.org> > > --- > > drivers/acpi/processor_core.c | 15 +++++++++++++++ > > drivers/irqchip/irq-gic-v3-its.c | 4 ++++ > > drivers/irqchip/irq-gic-v3.c | 9 +++++++++ > > include/linux/acpi.h | 3 +++ > > 4 files changed, 31 insertions(+) > > Hi Marc, Rafael, > > I would kindly ask you please what to do with this patch, it still > applies to v6.9-rc5 - I can resend it if needed, ACPICA changes > are already merged as-per the cover letter. Hi Marc, Rafael, I would kindly ask please what to do with this patch, rebased to v6.10-rc1, I can resend it if that's preferred, please let me know. Thanks, Lorenzo > > diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c > > index b203cfe28550..915713c0e9b7 100644 > > --- a/drivers/acpi/processor_core.c > > +++ b/drivers/acpi/processor_core.c > > @@ -215,6 +215,21 @@ phys_cpuid_t __init acpi_map_madt_entry(u32 acpi_id) > > return rv; > > } > > > > +int __init acpi_get_madt_revision(void) > > +{ > > + struct acpi_table_header *madt = NULL; > > + int revision; > > + > > + if (ACPI_FAILURE(acpi_get_table(ACPI_SIG_MADT, 0, &madt))) > > + return -EINVAL; > > + > > + revision = madt->revision; > > + > > + acpi_put_table(madt); > > + > > + return revision; > > +} > > + > > static phys_cpuid_t map_mat_entry(acpi_handle handle, int type, u32 acpi_id) > > { > > struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL }; > > diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c > > index fec1b58470df..a60c560ce891 100644 > > --- a/drivers/irqchip/irq-gic-v3-its.c > > +++ b/drivers/irqchip/irq-gic-v3-its.c > > @@ -5591,6 +5591,10 @@ static int __init gic_acpi_parse_madt_its(union acpi_subtable_headers *header, > > goto node_err; > > } > > > > + if (acpi_get_madt_revision() >= 7 && > > + (its_entry->flags & ACPI_MADT_ITS_NON_COHERENT)) > > + its->flags |= ITS_FLAGS_FORCE_NON_SHAREABLE; > > + > > err = its_probe_one(its); > > if (!err) > > return 0; > > diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c > > index 98b0329b7154..8cb8dff86c12 100644 > > --- a/drivers/irqchip/irq-gic-v3.c > > +++ b/drivers/irqchip/irq-gic-v3.c > > @@ -2356,6 +2356,11 @@ gic_acpi_parse_madt_redist(union acpi_subtable_headers *header, > > pr_err("Couldn't map GICR region @%llx\n", redist->base_address); > > return -ENOMEM; > > } > > + > > + if (acpi_get_madt_revision() >= 7 && > > + (redist->flags & ACPI_MADT_GICR_NON_COHERENT)) > > + gic_data.rdists.flags |= RDIST_FLAGS_FORCE_NON_SHAREABLE; > > + > > gic_request_region(redist->base_address, redist->length, "GICR"); > > > > gic_acpi_register_redist(redist->base_address, redist_base); > > @@ -2380,6 +2385,10 @@ gic_acpi_parse_madt_gicc(union acpi_subtable_headers *header, > > return -ENOMEM; > > gic_request_region(gicc->gicr_base_address, size, "GICR"); > > > > + if (acpi_get_madt_revision() >= 7 && > > + (gicc->flags & ACPI_MADT_GICC_NON_COHERENT)) > > + gic_data.rdists.flags |= RDIST_FLAGS_FORCE_NON_SHAREABLE; > > + > > gic_acpi_register_redist(gicc->gicr_base_address, redist_base); > > return 0; > > } > > diff --git a/include/linux/acpi.h b/include/linux/acpi.h > > index b7165e52b3c6..4eedab0e51c3 100644 > > --- a/include/linux/acpi.h > > +++ b/include/linux/acpi.h > > @@ -284,6 +284,9 @@ static inline bool invalid_phys_cpuid(phys_cpuid_t phys_id) > > return phys_id == PHYS_CPUID_INVALID; > > } > > > > + > > +int __init acpi_get_madt_revision(void); > > + > > /* Validate the processor object's proc_id */ > > bool acpi_duplicate_processor_id(int proc_id); > > /* Processor _CTS control */ > > -- > > 2.34.1 > >
On 2024-06-05 08:14, Lorenzo Pieralisi wrote: > On Mon, Apr 22, 2024 at 10:42:19AM +0200, Lorenzo Pieralisi wrote: >> On Tue, Jan 23, 2024 at 12:03:32PM +0100, Lorenzo Pieralisi wrote: >> > The GIC architecture specification defines a set of registers for >> > redistributors and ITSes that control the sharebility and cacheability >> > attributes of redistributors/ITSes initiator ports on the interconnect >> > (GICR_[V]PROPBASER, GICR_[V]PENDBASER, GITS_BASER<n>). >> > >> > Architecturally the GIC provides a means to drive shareability and >> > cacheability attributes signals but it is not mandatory for designs to >> > wire up the corresponding interconnect signals that control the >> > cacheability/shareability of transactions. >> > >> > Redistributors and ITSes interconnect ports can be connected to >> > non-coherent interconnects that are not able to manage the >> > shareability/cacheability attributes; this implicitly makes the >> > redistributors and ITSes non-coherent observers. >> > >> > To enable non-coherent GIC designs on ACPI based systems, parse the MADT >> > GICC/GICR/ITS subtables non-coherent flags to determine whether the >> > respective components are non-coherent observers and force the >> > shareability attributes to be programmed into the redistributors and >> > ITSes registers. >> > >> > An ACPI global function (acpi_get_madt_revision()) is added to retrieve >> > the MADT revision, in that it is essential to check the MADT revision >> > before checking for flags that were added with MADT revision 7 so that >> > if the kernel is booted with an ACPI MADT table with revision < 7 it >> > skips parsing the newly added flags (that should be zeroed reserved >> > values for MADT versions < 7 but they could turn out to be buggy and >> > should be ignored). >> > >> > Signed-off-by: Lorenzo Pieralisi <lpieralisi@kernel.org> >> > Cc: Robin Murphy <robin.murphy@arm.com> >> > Cc: Mark Rutland <mark.rutland@arm.com> >> > Cc: "Rafael J. Wysocki" <rafael@kernel.org> >> > Cc: Marc Zyngier <maz@kernel.org> >> > --- >> > drivers/acpi/processor_core.c | 15 +++++++++++++++ >> > drivers/irqchip/irq-gic-v3-its.c | 4 ++++ >> > drivers/irqchip/irq-gic-v3.c | 9 +++++++++ >> > include/linux/acpi.h | 3 +++ >> > 4 files changed, 31 insertions(+) >> >> Hi Marc, Rafael, >> >> I would kindly ask you please what to do with this patch, it still >> applies to v6.9-rc5 - I can resend it if needed, ACPICA changes >> are already merged as-per the cover letter. > > Hi Marc, Rafael, > > I would kindly ask please what to do with this patch, rebased to > v6.10-rc1, > I can resend it if that's preferred, please let me know. Please resend it with my: Acked-by: Marc Zyngier <maz@kernel.org> and Cc'ing Thomas Gleixner. Thanks, M.
diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c index b203cfe28550..915713c0e9b7 100644 --- a/drivers/acpi/processor_core.c +++ b/drivers/acpi/processor_core.c @@ -215,6 +215,21 @@ phys_cpuid_t __init acpi_map_madt_entry(u32 acpi_id) return rv; } +int __init acpi_get_madt_revision(void) +{ + struct acpi_table_header *madt = NULL; + int revision; + + if (ACPI_FAILURE(acpi_get_table(ACPI_SIG_MADT, 0, &madt))) + return -EINVAL; + + revision = madt->revision; + + acpi_put_table(madt); + + return revision; +} + static phys_cpuid_t map_mat_entry(acpi_handle handle, int type, u32 acpi_id) { struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL }; diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c index fec1b58470df..a60c560ce891 100644 --- a/drivers/irqchip/irq-gic-v3-its.c +++ b/drivers/irqchip/irq-gic-v3-its.c @@ -5591,6 +5591,10 @@ static int __init gic_acpi_parse_madt_its(union acpi_subtable_headers *header, goto node_err; } + if (acpi_get_madt_revision() >= 7 && + (its_entry->flags & ACPI_MADT_ITS_NON_COHERENT)) + its->flags |= ITS_FLAGS_FORCE_NON_SHAREABLE; + err = its_probe_one(its); if (!err) return 0; diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c index 98b0329b7154..8cb8dff86c12 100644 --- a/drivers/irqchip/irq-gic-v3.c +++ b/drivers/irqchip/irq-gic-v3.c @@ -2356,6 +2356,11 @@ gic_acpi_parse_madt_redist(union acpi_subtable_headers *header, pr_err("Couldn't map GICR region @%llx\n", redist->base_address); return -ENOMEM; } + + if (acpi_get_madt_revision() >= 7 && + (redist->flags & ACPI_MADT_GICR_NON_COHERENT)) + gic_data.rdists.flags |= RDIST_FLAGS_FORCE_NON_SHAREABLE; + gic_request_region(redist->base_address, redist->length, "GICR"); gic_acpi_register_redist(redist->base_address, redist_base); @@ -2380,6 +2385,10 @@ gic_acpi_parse_madt_gicc(union acpi_subtable_headers *header, return -ENOMEM; gic_request_region(gicc->gicr_base_address, size, "GICR"); + if (acpi_get_madt_revision() >= 7 && + (gicc->flags & ACPI_MADT_GICC_NON_COHERENT)) + gic_data.rdists.flags |= RDIST_FLAGS_FORCE_NON_SHAREABLE; + gic_acpi_register_redist(gicc->gicr_base_address, redist_base); return 0; } diff --git a/include/linux/acpi.h b/include/linux/acpi.h index b7165e52b3c6..4eedab0e51c3 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -284,6 +284,9 @@ static inline bool invalid_phys_cpuid(phys_cpuid_t phys_id) return phys_id == PHYS_CPUID_INVALID; } + +int __init acpi_get_madt_revision(void); + /* Validate the processor object's proc_id */ bool acpi_duplicate_processor_id(int proc_id); /* Processor _CTS control */
The GIC architecture specification defines a set of registers for redistributors and ITSes that control the sharebility and cacheability attributes of redistributors/ITSes initiator ports on the interconnect (GICR_[V]PROPBASER, GICR_[V]PENDBASER, GITS_BASER<n>). Architecturally the GIC provides a means to drive shareability and cacheability attributes signals but it is not mandatory for designs to wire up the corresponding interconnect signals that control the cacheability/shareability of transactions. Redistributors and ITSes interconnect ports can be connected to non-coherent interconnects that are not able to manage the shareability/cacheability attributes; this implicitly makes the redistributors and ITSes non-coherent observers. To enable non-coherent GIC designs on ACPI based systems, parse the MADT GICC/GICR/ITS subtables non-coherent flags to determine whether the respective components are non-coherent observers and force the shareability attributes to be programmed into the redistributors and ITSes registers. An ACPI global function (acpi_get_madt_revision()) is added to retrieve the MADT revision, in that it is essential to check the MADT revision before checking for flags that were added with MADT revision 7 so that if the kernel is booted with an ACPI MADT table with revision < 7 it skips parsing the newly added flags (that should be zeroed reserved values for MADT versions < 7 but they could turn out to be buggy and should be ignored). Signed-off-by: Lorenzo Pieralisi <lpieralisi@kernel.org> Cc: Robin Murphy <robin.murphy@arm.com> Cc: Mark Rutland <mark.rutland@arm.com> Cc: "Rafael J. Wysocki" <rafael@kernel.org> Cc: Marc Zyngier <maz@kernel.org> --- drivers/acpi/processor_core.c | 15 +++++++++++++++ drivers/irqchip/irq-gic-v3-its.c | 4 ++++ drivers/irqchip/irq-gic-v3.c | 9 +++++++++ include/linux/acpi.h | 3 +++ 4 files changed, 31 insertions(+)