[PATCH] irqchip/gic-v3: Add Allwinner sunxi001 erratum workaround

[PATCH] irqchip/gic-v3: Add Allwinner sunxi001 erratum workaround

[harry.yu185]

Allwinner A523 GIC600 integration does not support the
sharability feature. So assigned Erratum ID #sunxi001 for this
issue.

That the 0x0201643b ID is not Allwinner specific and thus
there is an extra of_machine_is_compatible() check.

Note, because more than one soc may have this problem, the 'sunxi'
name is used instead of a fixed soc name like A523.

Signed-off-by: harry.yu185 <harry.yu185@gmail.com>
---
 Documentation/arch/arm64/silicon-errata.rst |  2 ++
 arch/arm64/Kconfig                          | 10 ++++++++++
 drivers/irqchip/irq-gic-v3-its.c            | 21 +++++++++++++++++++++
 3 files changed, 33 insertions(+)

diff --git a/Documentation/arch/arm64/silicon-errata.rst b/Documentation/arch/arm64/silicon-errata.rst
index eb8af8032c31..351dd6094a6c 100644
--- a/Documentation/arch/arm64/silicon-errata.rst
+++ b/Documentation/arch/arm64/silicon-errata.rst
@@ -242,6 +242,8 @@ stable kernels.
 +----------------+-----------------+-----------------+-----------------------------+
 | Rockchip       | RK3588          | #3588001        | ROCKCHIP_ERRATUM_3588001    |
 +----------------+-----------------+-----------------+-----------------------------+
+| Allwinner      | SUN55IW3        | #sunxi001       | ALLWINNER_ERRATUM_SUNXI001  |
++----------------+-----------------+-----------------+-----------------------------+
 +----------------+-----------------+-----------------+-----------------------------+
 | Fujitsu        | A64FX           | E#010001        | FUJITSU_ERRATUM_010001      |
 +----------------+-----------------+-----------------+-----------------------------+
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 5d91259ee7b5..5a71227d119a 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1279,6 +1279,16 @@ config ROCKCHIP_ERRATUM_3588001
 
 	  If unsure, say Y.
 
+config ALLWINNER_ERRATUM_SUNXI001
+	bool "Allwinner sunxi001: GIC600 can not support shareability attributes"
+	default y
+	help
+	  The Allwinner GIC600 SoC integration does not support ACE/ACE-lite.
+	  This means, that its sharability feature may not be used, even though it
+	  is supported by the IP itself.
+
+	  If unsure, say Y.
+
 config SOCIONEXT_SYNQUACER_PREITS
 	bool "Socionext Synquacer: Workaround for GICv3 pre-ITS"
 	default y
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 40ebf1726393..d93348947353 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -4775,6 +4775,19 @@ static bool __maybe_unused its_enable_rk3588001(void *data)
 	return true;
 }
 
+static bool __maybe_unused its_enable_sunxi001(void *data)
+{
+	struct its_node *its = data;
+
+	if (!of_machine_is_compatible("arm,sun55iw3p1"))
+		return false;
+
+	its->flags |= ITS_FLAGS_FORCE_NON_SHAREABLE;
+	gic_rdists->flags |= RDIST_FLAGS_FORCE_NON_SHAREABLE;
+
+	return true;
+}
+
 static bool its_set_non_coherent(void *data)
 {
 	struct its_node *its = data;
@@ -4836,6 +4849,14 @@ static const struct gic_quirk its_quirks[] = {
 		.mask   = 0xffffffff,
 		.init   = its_enable_rk3588001,
 	},
+#endif
+#ifdef CONFIG_ALLWINNER_ERRATUM_SUNXI001
+	{
+		.desc   = "ITS: Allwinner erratum sunxi001",
+		.iidr   = 0x0201643b,
+		.mask   = 0xffffffff,
+		.init   = its_enable_sunxi001,
+	},
 #endif
 	{
 		.desc   = "ITS: non-coherent attribute",
-- 
2.37.2

Re: [PATCH] irqchip/gic-v3: Add Allwinner sunxi001 erratum workaround

[Marc Zyngier]

On Sun, 02 Jun 2024 08:10:58 +0100,
"harry.yu185" <harry.yu185@gmail.com> wrote:
> 
> Allwinner A523 GIC600 integration does not support the
> sharability feature. So assigned Erratum ID #sunxi001 for this
> issue.
> 
> That the 0x0201643b ID is not Allwinner specific and thus
> there is an extra of_machine_is_compatible() check.
> 
> Note, because more than one soc may have this problem, the 'sunxi'
> name is used instead of a fixed soc name like A523.
> 
> Signed-off-by: harry.yu185 <harry.yu185@gmail.com>

No, this is all already handled by the driver already (since 6.6).

Please fix your DT to include the "dma-noncoherent" property in the
GIC and ITS nodes, which should paper over the integration bug.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH] irqchip/gic-v3: Add Allwinner sunxi001 erratum workaround

[yu harry]

On Sun, Jun 2, 2024 at 5:25 PM Marc Zyngier <maz@kernel.org> wrote:
>
> On Sun, 02 Jun 2024 08:10:58 +0100,
> "harry.yu185" <harry.yu185@gmail.com> wrote:
> >
> > Allwinner A523 GIC600 integration does not support the
> > sharability feature. So assigned Erratum ID #sunxi001 for this
> > issue.
> >
> > That the 0x0201643b ID is not Allwinner specific and thus
> > there is an extra of_machine_is_compatible() check.
> >
> > Note, because more than one soc may have this problem, the 'sunxi'
> > name is used instead of a fixed soc name like A523.
> >
> > Signed-off-by: harry.yu185 <harry.yu185@gmail.com>
>
> No, this is all already handled by the driver already (since 6.6).
>
> Please fix your DT to include the "dma-noncoherent" property in the
> GIC and ITS nodes, which should paper over the integration bug.
>
> Thanks,
>
>         M.
>
> --
> Without deviation from the norm, progress is not possible.

Thank you for your reply,
the method you said may not be suitable, because this SOC
also needs RDIST_FLAGS_FORCE_NON_SHAREABLE,
just like RK3588, but it is different from the RK3588 version.

Re: [PATCH] irqchip/gic-v3: Add Allwinner sunxi001 erratum workaround

[Marc Zyngier]

On Sun, 02 Jun 2024 14:23:43 +0100,
yu harry <harry.yu185@gmail.com> wrote:
> 
> On Sun, Jun 2, 2024 at 5:25 PM Marc Zyngier <maz@kernel.org> wrote:
> >
> > On Sun, 02 Jun 2024 08:10:58 +0100,
> > "harry.yu185" <harry.yu185@gmail.com> wrote:
> > >
> > > Allwinner A523 GIC600 integration does not support the
> > > sharability feature. So assigned Erratum ID #sunxi001 for this
> > > issue.
> > >
> > > That the 0x0201643b ID is not Allwinner specific and thus
> > > there is an extra of_machine_is_compatible() check.
> > >
> > > Note, because more than one soc may have this problem, the 'sunxi'
> > > name is used instead of a fixed soc name like A523.
> > >
> > > Signed-off-by: harry.yu185 <harry.yu185@gmail.com>
> >
> > No, this is all already handled by the driver already (since 6.6).
> >
> > Please fix your DT to include the "dma-noncoherent" property in the
> > GIC and ITS nodes, which should paper over the integration bug.
> >
> > Thanks,
> >
> >         M.
> >
> > --
> > Without deviation from the norm, progress is not possible.
> 
> Thank you for your reply,
> the method you said may not be suitable, because this SOC
> also needs RDIST_FLAGS_FORCE_NON_SHAREABLE,
> just like RK3588, but it is different from the RK3588 version.

Who is talking of RK3588? Have you read what I wrote? Have you
actually looked at what these attributes do?

For context, here's what you're proposing:

+static bool __maybe_unused its_enable_sunxi001(void *data)
+{
+	struct its_node *its = data;
+
+	if (!of_machine_is_compatible("arm,sun55iw3p1"))
+		return false;
+
+	its->flags |= ITS_FLAGS_FORCE_NON_SHAREABLE;
+	gic_rdists->flags |= RDIST_FLAGS_FORCE_NON_SHAREABLE;
+
+	return true;
+}
+

"dma-noncoherent" on the GIC node provides:

static bool rd_set_non_coherent(void *data)
{
	struct gic_chip_data *d = data;

	d->rdists.flags |= RDIST_FLAGS_FORCE_NON_SHAREABLE;
	return true;
}

"dma-noncoherent" on the ITS node provides:

static bool its_set_non_coherent(void *data)
{
	struct its_node *its = data;

	its->flags |= ITS_FLAGS_FORCE_NON_SHAREABLE;
	return true;
}

So please do explain how the combination of the two isn't equivalent
to your patch. How does it fail to provide the required workaround?

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH] irqchip/gic-v3: Add Allwinner sunxi001 erratum workaround

[Andre Przywara]

On Sun,  2 Jun 2024 15:10:58 +0800
"harry.yu185" <harry.yu185@gmail.com> wrote:

Hi,

(please make sure to CC: the linux-sunxi mailing list on Allwinner
related patches)

I do hope that this whole patch is unnecessary, as Marc pointed out, but
just for the records some comments for future reference, since this
patch seems either premature or out of place.

> Allwinner A523 GIC600 integration does not support the
> sharability feature. So assigned Erratum ID #sunxi001 for this
> issue.
> 
> That the 0x0201643b ID is not Allwinner specific and thus
> there is an extra of_machine_is_compatible() check.
> 
> Note, because more than one soc may have this problem, the 'sunxi'
> name is used instead of a fixed soc name like A523.
> 
> Signed-off-by: harry.yu185 <harry.yu185@gmail.com>
> ---
>  Documentation/arch/arm64/silicon-errata.rst |  2 ++
>  arch/arm64/Kconfig                          | 10 ++++++++++
>  drivers/irqchip/irq-gic-v3-its.c            | 21 +++++++++++++++++++++
>  3 files changed, 33 insertions(+)
> 
> diff --git a/Documentation/arch/arm64/silicon-errata.rst b/Documentation/arch/arm64/silicon-errata.rst
> index eb8af8032c31..351dd6094a6c 100644
> --- a/Documentation/arch/arm64/silicon-errata.rst
> +++ b/Documentation/arch/arm64/silicon-errata.rst
> @@ -242,6 +242,8 @@ stable kernels.
>  +----------------+-----------------+-----------------+-----------------------------+
>  | Rockchip       | RK3588          | #3588001        | ROCKCHIP_ERRATUM_3588001    |
>  +----------------+-----------------+-----------------+-----------------------------+
> +| Allwinner      | SUN55IW3        | #sunxi001       | ALLWINNER_ERRATUM_SUNXI001  |

"sun55iw3" is a CPU die identifier only used in Allwinner's BSP
kernels, mainline uses a different naming scheme.

Also the erratum name looks odd, at the very least I'd expect it to
read "ALLWINNER_ERRATUM_00001" or something. But there is already some
Allwinner timer erratum, which we boldly gave ID 1 (short of an
official erratum number from Allwinner), so using the same number again
will surely lead to confusion.

> ++----------------+-----------------+-----------------+-----------------------------+
>  +----------------+-----------------+-----------------+-----------------------------+
>  | Fujitsu        | A64FX           | E#010001        | FUJITSU_ERRATUM_010001      |
>  +----------------+-----------------+-----------------+-----------------------------+
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 5d91259ee7b5..5a71227d119a 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1279,6 +1279,16 @@ config ROCKCHIP_ERRATUM_3588001
>  
>  	  If unsure, say Y.
>  
> +config ALLWINNER_ERRATUM_SUNXI001
> +	bool "Allwinner sunxi001: GIC600 can not support shareability attributes"
> +	default y

"default ARCH_SUNXI" would be better suited here, I think.

> +	help
> +	  The Allwinner GIC600 SoC integration does not support ACE/ACE-lite.
> +	  This means, that its sharability feature may not be used, even though it
> +	  is supported by the IP itself.
> +
> +	  If unsure, say Y.
> +
>  config SOCIONEXT_SYNQUACER_PREITS
>  	bool "Socionext Synquacer: Workaround for GICv3 pre-ITS"
>  	default y
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index 40ebf1726393..d93348947353 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -4775,6 +4775,19 @@ static bool __maybe_unused its_enable_rk3588001(void *data)
>  	return true;
>  }
>  
> +static bool __maybe_unused its_enable_sunxi001(void *data)
> +{
> +	struct its_node *its = data;
> +
> +	if (!of_machine_is_compatible("arm,sun55iw3p1"))

You cannot reference a compatible name here that is not documented in
the bindings. Which brings us to the elephant in the room: there is no
upstream support for this SoC (family) yet. I have some
work-in-progress series [1], but it's far from finished, mostly blocked
by the lack of hackable hardware (hopefully fixed soon).

So what kernel is this patch supposed to be applied against? You would
need at least a pinctrl and clock driver for even basic operation, none
of that I have seen posted.

Cheers,
Andre

[1] https://github.com/apritzel/linux/commits/a523-EARLY/

> +		return false;
> +
> +	its->flags |= ITS_FLAGS_FORCE_NON_SHAREABLE;
> +	gic_rdists->flags |= RDIST_FLAGS_FORCE_NON_SHAREABLE;
> +
> +	return true;
> +}
> +
>  static bool its_set_non_coherent(void *data)
>  {
>  	struct its_node *its = data;
> @@ -4836,6 +4849,14 @@ static const struct gic_quirk its_quirks[] = {
>  		.mask   = 0xffffffff,
>  		.init   = its_enable_rk3588001,
>  	},
> +#endif
> +#ifdef CONFIG_ALLWINNER_ERRATUM_SUNXI001
> +	{
> +		.desc   = "ITS: Allwinner erratum sunxi001",
> +		.iidr   = 0x0201643b,
> +		.mask   = 0xffffffff,
> +		.init   = its_enable_sunxi001,
> +	},
>  #endif
>  	{
>  		.desc   = "ITS: non-coherent attribute",