2023-04-17 21:42:31

by Sebastian Reichel

[permalink] [raw]
Subject: [PATCH v3 0/2] Add Rockchip RK3588 GIC ITS support

Hi,

This adds GIC ITS support to Rockchip RK3588, which is affected
by an integration issue effectively breaking shareability support.
PCIe2 support will follow in its own series.

Changelog:
* Changes since PATCHv2
- https://lore.kernel.org/lkml/[email protected]/
- apply changes requested by Marc Zyngier
* PATCHv1
- https://lore.kernel.org/lkml/[email protected]/
- uses of_dma_is_coherent() instead of providing errata info from kernel
* RFCv1
- https://lore.kernel.org/lkml/[email protected]/
- uses 0x0201743b IIDR for quirk detection and misses errata #

Greetings,

-- Sebastian

Sebastian Reichel (2):
irqchip/gic-v3: Add Rockchip 3588001 errata workaround
arm64: dts: rockchip: rk3588: add GIC ITS support

Documentation/arm64/silicon-errata.rst | 3 ++
arch/arm64/Kconfig | 10 +++++++
arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 17 +++++++++++
drivers/irqchip/irq-gic-v3-its.c | 35 +++++++++++++++++++++++
4 files changed, 65 insertions(+)

--
2.39.2


2023-04-17 21:42:56

by Sebastian Reichel

[permalink] [raw]
Subject: [PATCH v3 2/2] arm64: dts: rockchip: rk3588: add GIC ITS support

Add the two Interrupt Translation Service (ITS) IPs that are part of the
GIC-600. They are mainly required for PCIe Message Signalled Interrupts
(MSI).

Co-developed-by: Kever Yang <[email protected]>
Signed-off-by: Kever Yang <[email protected]>
Signed-off-by: Sebastian Reichel <[email protected]>
---
arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 17 +++++++++++++++++
1 file changed, 17 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
index 2124c654f665..62204b96b0b4 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
@@ -1741,7 +1741,24 @@ gic: interrupt-controller@fe600000 {
mbi-alias = <0x0 0xfe610000>;
mbi-ranges = <424 56>;
msi-controller;
+ ranges;
+ #address-cells = <2>;
#interrupt-cells = <4>;
+ #size-cells = <2>;
+
+ its0: msi-controller@fe640000 {
+ compatible = "arm,gic-v3-its";
+ msi-controller;
+ #msi-cells = <1>;
+ reg = <0x0 0xfe640000 0x0 0x20000>;
+ };
+
+ its1: msi-controller@fe660000 {
+ compatible = "arm,gic-v3-its";
+ msi-controller;
+ #msi-cells = <1>;
+ reg = <0x0 0xfe660000 0x0 0x20000>;
+ };

ppi-partitions {
ppi_partition0: interrupt-partition-0 {
--
2.39.2

2023-04-17 21:43:46

by Sebastian Reichel

[permalink] [raw]
Subject: [PATCH v3 1/2] irqchip/gic-v3: Add Rockchip 3588001 errata workaround

Rockchip RK3588/RK3588s GIC600 integration does not support the
sharability feature. Rockchip assigned Errata ID #3588001 for this
issue.

Note, that the 0x0201743b ID is not Rockchip specific and thus
there is an extra of_machine_is_compatible() check.

The flag are named BROKEN_SHAREABILITY to be vendor agnostic,
since apparently similar integration design errors exist in other
platforms and they can reuse the same flag.

Co-developed-by: XiaoDong Huang <[email protected]>
Signed-off-by: XiaoDong Huang <[email protected]>
Co-developed-by: Kever Yang <[email protected]>
Signed-off-by: Kever Yang <[email protected]>
Co-developed-by: Lucas Tanure <[email protected]>
Signed-off-by: Lucas Tanure <[email protected]>
Signed-off-by: Sebastian Reichel <[email protected]>
---
Documentation/arm64/silicon-errata.rst | 3 +++
arch/arm64/Kconfig | 10 ++++++++
drivers/irqchip/irq-gic-v3-its.c | 35 ++++++++++++++++++++++++++
3 files changed, 48 insertions(+)

diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst
index ec5f889d7681..46d06ed3e4f4 100644
--- a/Documentation/arm64/silicon-errata.rst
+++ b/Documentation/arm64/silicon-errata.rst
@@ -205,6 +205,9 @@ stable kernels.
+----------------+-----------------+-----------------+-----------------------------+
| Qualcomm Tech. | Kryo4xx Gold | N/A | ARM64_ERRATUM_1286807 |
+----------------+-----------------+-----------------+-----------------------------+
++----------------+-----------------+-----------------+-----------------------------+
+| Rockchip | RK3588 | #3588001 | ROCKCHIP_ERRATUM_3588001 |
++----------------+-----------------+-----------------+-----------------------------+

+----------------+-----------------+-----------------+-----------------------------+
| Fujitsu | A64FX | E#010001 | FUJITSU_ERRATUM_010001 |
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 1023e896d46b..1bc43faef10a 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1150,6 +1150,16 @@ config NVIDIA_CARMEL_CNP_ERRATUM

If unsure, say Y.

+config ROCKCHIP_ERRATUM_3588001
+ bool "Rockchip 3588001: GIC600 can not support shareability attributes"
+ default y
+ help
+ The Rockchip RK3588 GIC600 SoC integration does not support ACE/ACE-lite.
+ This means, that the GIC600 may not use the GIC600 sharability feature
+ 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 586271b8aa39..5b7aa48dde25 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -42,9 +42,11 @@
#define ITS_FLAGS_CMDQ_NEEDS_FLUSHING (1ULL << 0)
#define ITS_FLAGS_WORKAROUND_CAVIUM_22375 (1ULL << 1)
#define ITS_FLAGS_WORKAROUND_CAVIUM_23144 (1ULL << 2)
+#define ITS_FLAGS_BROKEN_SHAREABILITY (1ULL << 3)

#define RDIST_FLAGS_PROPBASE_NEEDS_FLUSHING (1 << 0)
#define RDIST_FLAGS_RD_TABLES_PREALLOCATED (1 << 1)
+#define RDIST_FLAGS_BROKEN_SHAREABILITY (1 << 2)

#define RD_LOCAL_LPI_ENABLED BIT(0)
#define RD_LOCAL_PENDTABLE_PREALLOCATED BIT(1)
@@ -2359,6 +2361,9 @@ static int its_setup_baser(struct its_node *its, struct its_baser *baser,
its_write_baser(its, baser, val);
tmp = baser->val;

+ if (its->flags & ITS_FLAGS_BROKEN_SHAREABILITY)
+ tmp &= ~GITS_BASER_SHAREABILITY_MASK;
+
if ((val ^ tmp) & GITS_BASER_SHAREABILITY_MASK) {
/*
* Shareability didn't stick. Just use
@@ -3096,6 +3101,9 @@ static void its_cpu_init_lpis(void)
gicr_write_propbaser(val, rbase + GICR_PROPBASER);
tmp = gicr_read_propbaser(rbase + GICR_PROPBASER);

+ if (gic_rdists->flags & RDIST_FLAGS_BROKEN_SHAREABILITY)
+ tmp &= ~GICR_PROPBASER_SHAREABILITY_MASK;
+
if ((tmp ^ val) & GICR_PROPBASER_SHAREABILITY_MASK) {
if (!(tmp & GICR_PROPBASER_SHAREABILITY_MASK)) {
/*
@@ -3120,6 +3128,9 @@ static void its_cpu_init_lpis(void)
gicr_write_pendbaser(val, rbase + GICR_PENDBASER);
tmp = gicr_read_pendbaser(rbase + GICR_PENDBASER);

+ if (gic_rdists->flags & RDIST_FLAGS_BROKEN_SHAREABILITY)
+ tmp &= ~GICR_PENDBASER_SHAREABILITY_MASK;
+
if (!(tmp & GICR_PENDBASER_SHAREABILITY_MASK)) {
/*
* The HW reports non-shareable, we must remove the
@@ -4710,6 +4721,19 @@ static bool __maybe_unused its_enable_quirk_hip07_161600802(void *data)
return true;
}

+static bool __maybe_unused its_enable_rk3588001(void *data)
+{
+ struct its_node *its = data;
+
+ if (!of_machine_is_compatible("rockchip,rk3588"))
+ return false;
+
+ its->flags |= ITS_FLAGS_BROKEN_SHAREABILITY;
+ gic_rdists->flags |= RDIST_FLAGS_BROKEN_SHAREABILITY;
+
+ return true;
+}
+
static const struct gic_quirk its_quirks[] = {
#ifdef CONFIG_CAVIUM_ERRATUM_22375
{
@@ -4755,6 +4779,14 @@ static const struct gic_quirk its_quirks[] = {
.mask = 0xffffffff,
.init = its_enable_quirk_hip07_161600802,
},
+#endif
+#ifdef CONFIG_ROCKCHIP_ERRATUM_3588001
+ {
+ .desc = "ITS: Rockchip erratum RK3588001",
+ .iidr = 0x0201743b,
+ .mask = 0xffffffff,
+ .init = its_enable_rk3588001,
+ },
#endif
{
}
@@ -5096,6 +5128,9 @@ static int __init its_probe_one(struct resource *res,
gits_write_cbaser(baser, its->base + GITS_CBASER);
tmp = gits_read_cbaser(its->base + GITS_CBASER);

+ if (its->flags & ITS_FLAGS_BROKEN_SHAREABILITY)
+ tmp &= ~GITS_CBASER_SHAREABILITY_MASK;
+
if ((tmp ^ baser) & GITS_CBASER_SHAREABILITY_MASK) {
if (!(tmp & GITS_CBASER_SHAREABILITY_MASK)) {
/*
--
2.39.2

Subject: Re: [PATCH v3 1/2] irqchip/gic-v3: Add Rockchip 3588001 errata workaround

Il 17/04/23 23:40, Sebastian Reichel ha scritto:
> Rockchip RK3588/RK3588s GIC600 integration does not support the
> sharability feature. Rockchip assigned Errata ID #3588001 for this
> issue.
>
> Note, that the 0x0201743b ID is not Rockchip specific and thus
> there is an extra of_machine_is_compatible() check.
>
> The flag are named BROKEN_SHAREABILITY to be vendor agnostic,
> since apparently similar integration design errors exist in other
> platforms and they can reuse the same flag.
>
> Co-developed-by: XiaoDong Huang <[email protected]>
> Signed-off-by: XiaoDong Huang <[email protected]>
> Co-developed-by: Kever Yang <[email protected]>
> Signed-off-by: Kever Yang <[email protected]>
> Co-developed-by: Lucas Tanure <[email protected]>
> Signed-off-by: Lucas Tanure <[email protected]>
> Signed-off-by: Sebastian Reichel <[email protected]>

Reviewed-by: AngeloGioacchino Del Regno <[email protected]>


2023-04-18 08:55:28

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] irqchip/gic-v3: Add Rockchip 3588001 errata workaround

On Mon, 17 Apr 2023 22:40:34 +0100,
Sebastian Reichel <[email protected]> wrote:
>
> Rockchip RK3588/RK3588s GIC600 integration does not support the
> sharability feature. Rockchip assigned Errata ID #3588001 for this
> issue.

s/Errata/Erratum/ (here and in the subject).

>
> Note, that the 0x0201743b ID is not Rockchip specific and thus
> there is an extra of_machine_is_compatible() check.
>
> The flag are named BROKEN_SHAREABILITY to be vendor agnostic,

Either "the flag is" or "the flags are".

> since apparently similar integration design errors exist in other
> platforms and they can reuse the same flag.
>
> Co-developed-by: XiaoDong Huang <[email protected]>
> Signed-off-by: XiaoDong Huang <[email protected]>
> Co-developed-by: Kever Yang <[email protected]>
> Signed-off-by: Kever Yang <[email protected]>
> Co-developed-by: Lucas Tanure <[email protected]>
> Signed-off-by: Lucas Tanure <[email protected]>
> Signed-off-by: Sebastian Reichel <[email protected]>
> ---
> Documentation/arm64/silicon-errata.rst | 3 +++
> arch/arm64/Kconfig | 10 ++++++++
> drivers/irqchip/irq-gic-v3-its.c | 35 ++++++++++++++++++++++++++
> 3 files changed, 48 insertions(+)
>
> diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst
> index ec5f889d7681..46d06ed3e4f4 100644
> --- a/Documentation/arm64/silicon-errata.rst
> +++ b/Documentation/arm64/silicon-errata.rst
> @@ -205,6 +205,9 @@ stable kernels.
> +----------------+-----------------+-----------------+-----------------------------+
> | Qualcomm Tech. | Kryo4xx Gold | N/A | ARM64_ERRATUM_1286807 |
> +----------------+-----------------+-----------------+-----------------------------+
> ++----------------+-----------------+-----------------+-----------------------------+
> +| Rockchip | RK3588 | #3588001 | ROCKCHIP_ERRATUM_3588001 |
> ++----------------+-----------------+-----------------+-----------------------------+
>
> +----------------+-----------------+-----------------+-----------------------------+
> | Fujitsu | A64FX | E#010001 | FUJITSU_ERRATUM_010001 |
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 1023e896d46b..1bc43faef10a 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1150,6 +1150,16 @@ config NVIDIA_CARMEL_CNP_ERRATUM
>
> If unsure, say Y.
>
> +config ROCKCHIP_ERRATUM_3588001
> + bool "Rockchip 3588001: GIC600 can not support shareability attributes"
> + default y
> + help
> + The Rockchip RK3588 GIC600 SoC integration does not support ACE/ACE-lite.
> + This means, that the GIC600 may not use the GIC600 sharability feature

This sentence makes little sense. It is the GIC driver that cannot
make use of the shareability attributes.

> + 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 586271b8aa39..5b7aa48dde25 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -42,9 +42,11 @@
> #define ITS_FLAGS_CMDQ_NEEDS_FLUSHING (1ULL << 0)
> #define ITS_FLAGS_WORKAROUND_CAVIUM_22375 (1ULL << 1)
> #define ITS_FLAGS_WORKAROUND_CAVIUM_23144 (1ULL << 2)
> +#define ITS_FLAGS_BROKEN_SHAREABILITY (1ULL << 3)

Having slept on it, I'd rather this is renamed as
"FORCE_NON_SHAREABLE", as it better describes what we are doing. The
brokenness is an attribute of the platform, not of the flag.

>
> #define RDIST_FLAGS_PROPBASE_NEEDS_FLUSHING (1 << 0)
> #define RDIST_FLAGS_RD_TABLES_PREALLOCATED (1 << 1)
> +#define RDIST_FLAGS_BROKEN_SHAREABILITY (1 << 2)

Same thing here.

Thanks,

M.

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