Received-SPF: pass (google.com: domain of linux-kernel+bounces-201849-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Wed, 5 Jun 2024 09:14:09 +0200
From: Lorenzo Pieralisi <lpieralisi@kernel.org>
To: linux-kernel@vger.kernel.org, Marc Zyngier <maz@kernel.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>
Cc: Robin Murphy <robin.murphy@arm.com>,
	Mark Rutland <mark.rutland@arm.com>,
	linux-arm-kernel@lists.infradead.org, linux-acpi@vger.kernel.org,
	acpica-devel@lists.linux.dev, Fang Xiang <fangxiang3@xiaomi.com>,
	Robert Moore <robert.moore@intel.com>
Subject: Re: [PATCH v5 1/1] irqchip/gic-v3: Enable non-coherent
 redistributors/ITSes ACPI probing
Message-ID: <ZmAQQVR+inUJpJ7z@lpieralisi>
References: <20240123110332.112797-1-lpieralisi@kernel.org>
 <20240123110332.112797-2-lpieralisi@kernel.org>
 <ZiYi64/VBWkfM2E2@lpieralisi>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ZiYi64/VBWkfM2E2@lpieralisi>

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