[PATCH v5 0/1] irqchip/gic-v3: Enable non-coherent GIC designs probing

[PATCH v5 0/1] irqchip/gic-v3: Enable non-coherent GIC designs probing

[Lorenzo Pieralisi]

This series is v5 of previous series:

v4: https://lore.kernel.org/all/20231227110038.55453-1-lpieralisi@kernel.org
v3: https://lore.kernel.org/all/20231006125929.48591-1-lpieralisi@kernel.org
v2: https://lore.kernel.org/all/20230906094139.16032-1-lpieralisi@kernel.org
v1: https://lore.kernel.org/all/20230905104721.52199-1-lpieralisi@kernel.org

v4 -> v5
	- ACPICA patches merged for v6.8
	- Refactored ACPI parsing code according to review
	- Rebased against v6.8-rc1

v3 -> v4:
	- Dropped patches [1-3], already merged
	- Added Linuxized ACPICA changes accepted upstream
	- Rebased against v6.7-rc3

v2 -> v3:
	- Added ACPICA temporary changes and ACPI changes to implement
	  ECR https://bugzilla.tianocore.org/show_bug.cgi?id=4557
	- ACPI changes are for testing purposes - subject to ECR code
	  first approval

v1 -> v2:
	- Updated DT bindings as per feedback
	- Updated patch[2] to use GIC quirks infrastructure

Original cover letter
---
The GICv3 architecture specifications provide a means for the
system programmer to set the shareability and cacheability
attributes the GIC components (redistributors and ITSes) use
to drive memory transactions.

Albeit the architecture give control over shareability/cacheability
memory transactions attributes (and barriers), it is allowed to
connect the GIC interconnect ports to non-coherent memory ports
on the interconnect, basically tying off shareability/cacheability
"wires" and de-facto making the redistributors and ITSes non-coherent
memory observers.

This series aims at starting a discussion over a possible solution
to this problem, by adding to the GIC device tree bindings the
standard dma-noncoherent property. The GIC driver uses the property
to force the redistributors and ITSes shareability attributes to
non-shareable, which consequently forces the driver to use CMOs
on GIC memory tables.

On ARM DT DMA is default non-coherent, so the GIC driver can't rely
on the generic DT dma-coherent/non-coherent property management layer
(of_dma_is_coherent()) which would default all GIC designs in the field
as non-coherent; it has to rely on ad-hoc dma-noncoherent property handling.

When a consistent approach is agreed upon for DT an equivalent binding will
be put forward for ACPI based systems.

Lorenzo Pieralisi (1):
  irqchip/gic-v3: Enable non-coherent redistributors/ITSes ACPI probing

 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(+)

-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v5 1/1] irqchip/gic-v3: Enable non-coherent redistributors/ITSes ACPI probing

[Lorenzo Pieralisi]

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(+)

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v5 1/1] irqchip/gic-v3: Enable non-coherent redistributors/ITSes ACPI probing

[Robin Murphy]

On 23/01/2024 11:03 am, 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).

FWIW,

Reviewed-by: Robin Murphy <robin.murphy@arm.com>

I guess the most contentious parts disappeared in Marc's refactoring, so 
what's left seems entirely straightforward and innocuous to me.

I'd also agree that there seems no real value in going out of our way to 
police the firmware for consistency - if at least one entry says to 
apply the quirk, then applying the quirk is the right and safest thing 
to do, so all we could really do on top of that is add extra complexity 
to be able to say "hey, firmware did a silly thing!", but if we're 
starting from the premise that hardware did an inadvisable thing to 
begin with, maybe we should have similarly realistic expectations of the 
corresponding firmware anyway :)

Cheers,
Robin.

> 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(+)
> 
> 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 */

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v5 1/1] irqchip/gic-v3: Enable non-coherent redistributors/ITSes ACPI probing

[Lorenzo Pieralisi]

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
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v5 1/1] irqchip/gic-v3: Enable non-coherent redistributors/ITSes ACPI probing

[Lorenzo Pieralisi]

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
> > 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v5 1/1] irqchip/gic-v3: Enable non-coherent redistributors/ITSes ACPI probing

[Marc Zyngier]

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.
-- 
Jazz is not dead. It just smells funny...

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v5 0/1] irqchip/gic-v3: Enable non-coherent GIC designs probing

[Lorenzo Pieralisi]

On Tue, Jan 23, 2024 at 12:03:31PM +0100, Lorenzo Pieralisi wrote:
> This series is v5 of previous series:
> 
> v4: https://lore.kernel.org/all/20231227110038.55453-1-lpieralisi@kernel.org
> v3: https://lore.kernel.org/all/20231006125929.48591-1-lpieralisi@kernel.org
> v2: https://lore.kernel.org/all/20230906094139.16032-1-lpieralisi@kernel.org
> v1: https://lore.kernel.org/all/20230905104721.52199-1-lpieralisi@kernel.org
> 
> v4 -> v5
> 	- ACPICA patches merged for v6.8
> 	- Refactored ACPI parsing code according to review
> 	- Rebased against v6.8-rc1

Hi Marc, all,

this is not an urgent fix (I don't think there is any ACPI platform
affected in the field so it is not even a fix), I am just asking please
what should I do with it, I appreciate it is late in the cycle (and I
know some fixes got merged in -rcX leading up to -rc7 that are
pre-requisite for this patch to work).

Thanks,
Lorenzo

> v3 -> v4:
> 	- Dropped patches [1-3], already merged
> 	- Added Linuxized ACPICA changes accepted upstream
> 	- Rebased against v6.7-rc3
> 
> v2 -> v3:
> 	- Added ACPICA temporary changes and ACPI changes to implement
> 	  ECR https://bugzilla.tianocore.org/show_bug.cgi?id=4557
> 	- ACPI changes are for testing purposes - subject to ECR code
> 	  first approval
> 
> v1 -> v2:
> 	- Updated DT bindings as per feedback
> 	- Updated patch[2] to use GIC quirks infrastructure
> 
> Original cover letter
> ---
> The GICv3 architecture specifications provide a means for the
> system programmer to set the shareability and cacheability
> attributes the GIC components (redistributors and ITSes) use
> to drive memory transactions.
> 
> Albeit the architecture give control over shareability/cacheability
> memory transactions attributes (and barriers), it is allowed to
> connect the GIC interconnect ports to non-coherent memory ports
> on the interconnect, basically tying off shareability/cacheability
> "wires" and de-facto making the redistributors and ITSes non-coherent
> memory observers.
> 
> This series aims at starting a discussion over a possible solution
> to this problem, by adding to the GIC device tree bindings the
> standard dma-noncoherent property. The GIC driver uses the property
> to force the redistributors and ITSes shareability attributes to
> non-shareable, which consequently forces the driver to use CMOs
> on GIC memory tables.
> 
> On ARM DT DMA is default non-coherent, so the GIC driver can't rely
> on the generic DT dma-coherent/non-coherent property management layer
> (of_dma_is_coherent()) which would default all GIC designs in the field
> as non-coherent; it has to rely on ad-hoc dma-noncoherent property handling.
> 
> When a consistent approach is agreed upon for DT an equivalent binding will
> be put forward for ACPI based systems.
> 
> Lorenzo Pieralisi (1):
>   irqchip/gic-v3: Enable non-coherent redistributors/ITSes ACPI probing
> 
>  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(+)
> 
> -- 
> 2.34.1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v5 0/1] irqchip/gic-v3: Enable non-coherent GIC designs probing

[Lorenzo Pieralisi]

On Wed, Mar 06, 2024 at 03:43:32PM +0100, Lorenzo Pieralisi wrote:
> On Tue, Jan 23, 2024 at 12:03:31PM +0100, Lorenzo Pieralisi wrote:
> > This series is v5 of previous series:
> > 
> > v4: https://lore.kernel.org/all/20231227110038.55453-1-lpieralisi@kernel.org
> > v3: https://lore.kernel.org/all/20231006125929.48591-1-lpieralisi@kernel.org
> > v2: https://lore.kernel.org/all/20230906094139.16032-1-lpieralisi@kernel.org
> > v1: https://lore.kernel.org/all/20230905104721.52199-1-lpieralisi@kernel.org
> > 
> > v4 -> v5
> > 	- ACPICA patches merged for v6.8
> > 	- Refactored ACPI parsing code according to review
> > 	- Rebased against v6.8-rc1
> 
> Hi Marc, all,
> 
> this is not an urgent fix (I don't think there is any ACPI platform
> affected in the field so it is not even a fix), I am just asking please
> what should I do with it, I appreciate it is late in the cycle (and I
> know some fixes got merged in -rcX leading up to -rc7 that are
> pre-requisite for this patch to work).

Hi,

just a reminder to ask how to proceed with this patch, I know it
is not urgent, just to understand how to handle it please.

The related ACPICA changes are already merged in the mainline.

Thanks,
Lorenzo

> Thanks,
> Lorenzo
> 
> > v3 -> v4:
> > 	- Dropped patches [1-3], already merged
> > 	- Added Linuxized ACPICA changes accepted upstream
> > 	- Rebased against v6.7-rc3
> > 
> > v2 -> v3:
> > 	- Added ACPICA temporary changes and ACPI changes to implement
> > 	  ECR https://bugzilla.tianocore.org/show_bug.cgi?id=4557
> > 	- ACPI changes are for testing purposes - subject to ECR code
> > 	  first approval
> > 
> > v1 -> v2:
> > 	- Updated DT bindings as per feedback
> > 	- Updated patch[2] to use GIC quirks infrastructure
> > 
> > Original cover letter
> > ---
> > The GICv3 architecture specifications provide a means for the
> > system programmer to set the shareability and cacheability
> > attributes the GIC components (redistributors and ITSes) use
> > to drive memory transactions.
> > 
> > Albeit the architecture give control over shareability/cacheability
> > memory transactions attributes (and barriers), it is allowed to
> > connect the GIC interconnect ports to non-coherent memory ports
> > on the interconnect, basically tying off shareability/cacheability
> > "wires" and de-facto making the redistributors and ITSes non-coherent
> > memory observers.
> > 
> > This series aims at starting a discussion over a possible solution
> > to this problem, by adding to the GIC device tree bindings the
> > standard dma-noncoherent property. The GIC driver uses the property
> > to force the redistributors and ITSes shareability attributes to
> > non-shareable, which consequently forces the driver to use CMOs
> > on GIC memory tables.
> > 
> > On ARM DT DMA is default non-coherent, so the GIC driver can't rely
> > on the generic DT dma-coherent/non-coherent property management layer
> > (of_dma_is_coherent()) which would default all GIC designs in the field
> > as non-coherent; it has to rely on ad-hoc dma-noncoherent property handling.
> > 
> > When a consistent approach is agreed upon for DT an equivalent binding will
> > be put forward for ACPI based systems.
> > 
> > Lorenzo Pieralisi (1):
> >   irqchip/gic-v3: Enable non-coherent redistributors/ITSes ACPI probing
> > 
> >  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(+)
> > 
> > -- 
> > 2.34.1
> > 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v6 0/1] irqchip/gic-v3: Enable non-coherent GIC designs probing

[Lorenzo Pieralisi]

This series is v6 of previous series:

v5: https://lore.kernel.org/all/20240123110332.112797-1-lpieralisi@kernel.org
v4: https://lore.kernel.org/all/20231227110038.55453-1-lpieralisi@kernel.org
v3: https://lore.kernel.org/all/20231006125929.48591-1-lpieralisi@kernel.org
v2: https://lore.kernel.org/all/20230906094139.16032-1-lpieralisi@kernel.org
v1: https://lore.kernel.org/all/20230905104721.52199-1-lpieralisi@kernel.org

v5 -> v6
	- Rebased against v6.10-rc1
	- Added Marc Zyngier's ACK

v4 -> v5
	- ACPICA patches merged for v6.8
	- Refactored ACPI parsing code according to review
	- Rebased against v6.8-rc1

v3 -> v4:
	- Dropped patches [1-3], already merged
	- Added Linuxized ACPICA changes accepted upstream
	- Rebased against v6.7-rc3

v2 -> v3:
	- Added ACPICA temporary changes and ACPI changes to implement
	  ECR https://bugzilla.tianocore.org/show_bug.cgi?id=4557
	- ACPI changes are for testing purposes - subject to ECR code
	  first approval

v1 -> v2:
	- Updated DT bindings as per feedback
	- Updated patch[2] to use GIC quirks infrastructure

Original cover letter
---
The GICv3 architecture specifications provide a means for the
system programmer to set the shareability and cacheability
attributes the GIC components (redistributors and ITSes) use
to drive memory transactions.

Albeit the architecture give control over shareability/cacheability
memory transactions attributes (and barriers), it is allowed to
connect the GIC interconnect ports to non-coherent memory ports
on the interconnect, basically tying off shareability/cacheability
"wires" and de-facto making the redistributors and ITSes non-coherent
memory observers.

This series aims at starting a discussion over a possible solution
to this problem, by adding to the GIC device tree bindings the
standard dma-noncoherent property. The GIC driver uses the property
to force the redistributors and ITSes shareability attributes to
non-shareable, which consequently forces the driver to use CMOs
on GIC memory tables.

On ARM DT DMA is default non-coherent, so the GIC driver can't rely
on the generic DT dma-coherent/non-coherent property management layer
(of_dma_is_coherent()) which would default all GIC designs in the field
as non-coherent; it has to rely on ad-hoc dma-noncoherent property handling.

When a consistent approach is agreed upon for DT an equivalent binding will
be put forward for ACPI based systems.

Lorenzo Pieralisi (1):
  irqchip/gic-v3: Enable non-coherent redistributors/ITSes ACPI probing

 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(+)

-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v6 1/1] irqchip/gic-v3: Enable non-coherent redistributors/ITSes ACPI probing

[Lorenzo Pieralisi]

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>
Reviewed-by: Robin Murphy <robin.murphy@arm.com>
Acked-by: Marc Zyngier <maz@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
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(+)

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 40ebf1726393..af5297ef2293 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -5600,6 +5600,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 6fb276504bcc..e4bc5f04226e 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -2349,6 +2349,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);
@@ -2373,6 +2378,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 28c3fb2bef0d..000d339e1596 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -279,6 +279,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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v6 1/1] irqchip/gic-v3: Enable non-coherent redistributors/ITSes ACPI probing

[Amit Singh Tomar]

> ----------------------------------------------------------------------
> 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>
> Reviewed-by: Robin Murphy <robin.murphy@arm.com>
> Acked-by: Marc Zyngier <maz@kernel.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> 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(+)
> 
> 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)

Wondering, if we can have a generic function (acpi_get_tbl_revision) to 
obtain the revision number for any ACPI table, not just specific to MADT?

> +{
> +	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 40ebf1726393..af5297ef2293 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -5600,6 +5600,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 6fb276504bcc..e4bc5f04226e 100644
> --- a/drivers/irqchip/irq-gic-v3.c
> +++ b/drivers/irqchip/irq-gic-v3.c
> @@ -2349,6 +2349,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);
> @@ -2373,6 +2378,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 28c3fb2bef0d..000d339e1596 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -279,6 +279,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 */


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v6 1/1] irqchip/gic-v3: Enable non-coherent redistributors/ITSes ACPI probing

[Lorenzo Pieralisi]

On Fri, Jun 07, 2024 at 12:21:54AM +0530, Amit Singh Tomar wrote:

[...]

> > 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)
> 
> Wondering, if we can have a generic function (acpi_get_tbl_revision) to
> obtain the revision number for any ACPI table, not just specific to MADT?

We could - I don't think there would be users other than code in this
patch though so I thought it would not be necessary.

Lorenzo

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v6 1/1] irqchip/gic-v3: Enable non-coherent redistributors/ITSes ACPI probing

[Amit Singh Tomar]

> On Fri, Jun 07, 2024 at 12:21:54AM +0530, Amit Singh Tomar wrote:
> 
> [...]
> 
>>> 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)
>>
>> Wondering, if we can have a generic function (acpi_get_tbl_revision) to
>> obtain the revision number for any ACPI table, not just specific to MADT?
> 
> We could - I don't think there would be users other than code in this
> patch though so I thought it would not be necessary.
> 

Right, it might not be essential now but I see that MPAM will be another 
user of it once the MPAM patches are out.

https://git.kernel.org/pub/scm/linux/kernel/git/morse/linux.git/tree/drivers/acpi/arm64/mpam.c?h=mpam/snapshot/v6.7-rc2#n299

Thanks
-Amit


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v6 1/1] irqchip/gic-v3: Enable non-coherent redistributors/ITSes ACPI probing

[Robin Murphy]

On 2024-06-07 8:53 am, Amit Singh Tomar wrote:
>> On Fri, Jun 07, 2024 at 12:21:54AM +0530, Amit Singh Tomar wrote:
>>
>> [...]
>>
>>>> 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)
>>>
>>> Wondering, if we can have a generic function (acpi_get_tbl_revision) to
>>> obtain the revision number for any ACPI table, not just specific to 
>>> MADT?
>>
>> We could - I don't think there would be users other than code in this
>> patch though so I thought it would not be necessary.
>>
> 
> Right, it might not be essential now but I see that MPAM will be another 
> user of it once the MPAM patches are out.
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/morse/linux.git/tree/drivers/acpi/arm64/mpam.c?h=mpam/snapshot/v6.7-rc2#n299

Not really; there's already plenty of ACPI code which checks the 
revision of a table *while* also parsing other information from it, and 
that MPAM code is doing the same. Using a standalone function to look up 
the table, check one thing and throw it away, and then immediately have 
to look it up again to do the rest would be needlessly overcomplicated.

The thing in the GIC case is that doing this semi-redundant lookup to 
re-retrieve the top-level MADT header while we're already deep into 
parsing its subtables is still the least-worst option, because the 
alternative would be invasively churning the whole common MADT 
abstraction to pass that information all the way down just for this one 
slightly niche thing.

Thanks,
Robin.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel