[PATCH] irqchip/gic-v3: Workaround Marvell erratum 38545 when reading IAR

* [PATCH] irqchip/gic-v3: Workaround Marvell erratum 38545 when reading IAR
@ 2022-02-26 12:33 Linu Cherian
  2022-02-26 13:50 ` Marc Zyngier
  0 siblings, 1 reply; 4+ messages in thread
From: Linu Cherian @ 2022-02-26 12:33 UTC (permalink / raw)
  To: maz, tglx, catalin.marinas, will
  Cc: linux-kernel, linux-arm-kernel, linuc.decode, Linu Cherian

When a IAR register read races with a GIC interrupt RELEASE event,
GIC-CPU interface could wrongly return a valid INTID to the CPU
for an interrupt that is already released(non activated) instead of 0x3ff.

As a side effect, an interrupt handler could run twice, once with
interrupt priority and then with idle priority.

As a workaround, gic_read_iar is updated so that it will return a
valid interrupt ID only if there is a change in the active priority list
after the IAR read on all the affected Silicons.

Along with this, Thunderx erratum 23154 is reworked to use GIC IIDR
based quirk management for the sake of consistency and also
because there is workaround overlap on some silicon variants.

Signed-off-by: Linu Cherian <lcherian@marvell.com>
---
 Documentation/arm64/silicon-errata.rst |  4 +-
 arch/arm64/Kconfig                     | 10 -----
 arch/arm64/include/asm/arch_gicv3.h    | 24 +++++++++--
 arch/arm64/kernel/cpu_errata.c         |  8 ----
 arch/arm64/tools/cpucaps               |  1 -
 drivers/irqchip/irq-gic-v3.c           | 56 +++++++++++++++++++++++++-
 6 files changed, 77 insertions(+), 26 deletions(-)

diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst
index ea281dd75517..f602faf4bf82 100644
--- a/Documentation/arm64/silicon-errata.rst
+++ b/Documentation/arm64/silicon-errata.rst
@@ -136,10 +136,12 @@ stable kernels.
 +----------------+-----------------+-----------------+-----------------------------+
 | Cavium         | ThunderX ITS    | #23144          | CAVIUM_ERRATUM_23144        |
 +----------------+-----------------+-----------------+-----------------------------+
-| Cavium         | ThunderX GICv3  | #23154          | CAVIUM_ERRATUM_23154        |
+| Cavium         | ThunderX GICv3  | #23154          | N/A                         |
 +----------------+-----------------+-----------------+-----------------------------+
 | Cavium         | ThunderX GICv3  | #38539          | N/A                         |
 +----------------+-----------------+-----------------+-----------------------------+
+| Cavium         | ThunderX GICv3  | #38545          | N/A                         |
++----------------+-----------------+-----------------+-----------------------------+
 | Cavium         | ThunderX Core   | #27456          | CAVIUM_ERRATUM_27456        |
 +----------------+-----------------+-----------------+-----------------------------+
 | Cavium         | ThunderX Core   | #30115          | CAVIUM_ERRATUM_30115        |
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 09b885cc4db5..889cb56bf5ec 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -890,16 +890,6 @@ config CAVIUM_ERRATUM_23144
 
 	  If unsure, say Y.
 
-config CAVIUM_ERRATUM_23154
-	bool "Cavium erratum 23154: Access to ICC_IAR1_EL1 is not sync'ed"
-	default y
-	help
-	  The gicv3 of ThunderX requires a modified version for
-	  reading the IAR status to ensure data synchronization
-	  (access to icc_iar1_el1 is not sync'ed before and after).
-
-	  If unsure, say Y.
-
 config CAVIUM_ERRATUM_27456
 	bool "Cavium erratum 27456: Broadcast TLBI instructions may cause icache corruption"
 	default y
diff --git a/arch/arm64/include/asm/arch_gicv3.h b/arch/arm64/include/asm/arch_gicv3.h
index 4ad22c3135db..bc98a60a4bcb 100644
--- a/arch/arm64/include/asm/arch_gicv3.h
+++ b/arch/arm64/include/asm/arch_gicv3.h
@@ -47,21 +47,37 @@ static inline u64 gic_read_iar_common(void)
 	return irqstat;
 }
 
-/*
+/* Marvell Erratum 38545
+ *
+ * When a IAR register read races with a GIC interrupt RELEASE event,
+ * GIC-CPU interface could wrongly return a valid INTID to the CPU
+ * for an interrupt that is already released(non activated) instead of 0x3ff.
+ *
+ * To workaround this, return a valid interrupt ID only if there is a change
+ * in the active priority list after the IAR read.
+ *
  * Cavium ThunderX erratum 23154
  *
  * The gicv3 of ThunderX requires a modified version for reading the
  * IAR status to ensure data synchronization (access to icc_iar1_el1
  * is not sync'ed before and after).
+ *
+ * Have merged both the workarounds into a common function since,
+ * 1. On Thunderx 88xx 1.x both erratas are applicable.
+ * 2. Having extra nops doesn't add any side effects for Silicons where
+ *    erratum 23154 is not applicable.
  */
-static inline u64 gic_read_iar_cavium_thunderx(void)
+static inline u64 gic_read_iar_marvell_38545_23154(void)
 {
-	u64 irqstat;
+	u64 irqstat, apr;
 
+	apr = read_sysreg_s(SYS_ICC_AP1R0_EL1);
 	nops(8);
 	irqstat = read_sysreg_s(SYS_ICC_IAR1_EL1);
 	nops(4);
-	mb();
+	dsb(sy);
+	if (unlikely(apr == read_sysreg_s(SYS_ICC_AP1R0_EL1)))
+		return 0x3ff;
 
 	return irqstat;
 }
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index b217941713a8..79beb800ee79 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -423,14 +423,6 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
 		ERRATA_MIDR_RANGE_LIST(erratum_845719_list),
 	},
 #endif
-#ifdef CONFIG_CAVIUM_ERRATUM_23154
-	{
-	/* Cavium ThunderX, pass 1.x */
-		.desc = "Cavium erratum 23154",
-		.capability = ARM64_WORKAROUND_CAVIUM_23154,
-		ERRATA_MIDR_REV_RANGE(MIDR_THUNDERX, 0, 0, 1),
-	},
-#endif
 #ifdef CONFIG_CAVIUM_ERRATUM_27456
 	{
 		.desc = "Cavium erratum 27456",
diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
index 9c65b1e25a96..3f751fe4fec4 100644
--- a/arch/arm64/tools/cpucaps
+++ b/arch/arm64/tools/cpucaps
@@ -62,7 +62,6 @@ WORKAROUND_2077057
 WORKAROUND_TRBE_OVERWRITE_FILL_MODE
 WORKAROUND_TSB_FLUSH_FAILURE
 WORKAROUND_TRBE_WRITE_OUT_OF_RANGE
-WORKAROUND_CAVIUM_23154
 WORKAROUND_CAVIUM_27456
 WORKAROUND_CAVIUM_30115
 WORKAROUND_CAVIUM_TX2_219_PRFM
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index 5e935d97207d..a3b58bf4fce4 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -35,6 +35,8 @@
 
 #define FLAGS_WORKAROUND_GICR_WAKER_MSM8996	(1ULL << 0)
 #define FLAGS_WORKAROUND_CAVIUM_ERRATUM_38539	(1ULL << 1)
+#define FLAGS_WORKAROUND_CAVIUM_ERRATUM_23154	(1ULL << 2)
+#define FLAGS_WORKAROUND_MARVELL_ERRATUM_38545	(1ULL << 3)
 
 #define GIC_IRQ_TYPE_PARTITION	(GIC_IRQ_TYPE_LPI + 1)
 
@@ -60,6 +62,7 @@ struct gic_chip_data {
 
 static struct gic_chip_data gic_data __read_mostly;
 static DEFINE_STATIC_KEY_TRUE(supports_deactivate_key);
+static DEFINE_STATIC_KEY_FALSE(gic_iar_quirk);
 
 #define GIC_ID_NR	(1U << GICD_TYPER_ID_BITS(gic_data.rdists.gicd_typer))
 #define GIC_LINE_NR	min(GICD_TYPER_SPIS(gic_data.rdists.gicd_typer), 1020U)
@@ -235,10 +238,19 @@ static void gic_redist_wait_for_rwp(void)
 
 #ifdef CONFIG_ARM64
 
+static u64 __maybe_unused gic_read_iar_fixup(void)
+{
+	if (gic_data.flags & FLAGS_WORKAROUND_MARVELL_ERRATUM_38545 ||
+		gic_data.flags & FLAGS_WORKAROUND_CAVIUM_ERRATUM_23154)
+		return gic_read_iar_marvell_38545_23154();
+	else /* Not possible */
+		return ICC_IAR1_EL1_SPURIOUS;
+}
+
 static u64 __maybe_unused gic_read_iar(void)
 {
-	if (cpus_have_const_cap(ARM64_WORKAROUND_CAVIUM_23154))
-		return gic_read_iar_cavium_thunderx();
+	if (static_branch_unlikely(&gic_iar_quirk))
+		return gic_read_iar_fixup();
 	else
 		return gic_read_iar_common();
 }
@@ -1614,6 +1626,16 @@ static bool gic_enable_quirk_msm8996(void *data)
 	return true;
 }
 
+static bool gic_enable_quirk_cavium_23154(void *data)
+{
+	struct gic_chip_data *d = data;
+
+	d->flags |= FLAGS_WORKAROUND_CAVIUM_ERRATUM_23154;
+	static_branch_enable(&gic_iar_quirk);
+
+	return true;
+}
+
 static bool gic_enable_quirk_cavium_38539(void *data)
 {
 	struct gic_chip_data *d = data;
@@ -1623,6 +1645,16 @@ static bool gic_enable_quirk_cavium_38539(void *data)
 	return true;
 }
 
+static bool gic_enable_quirk_marvell_38545(void *data)
+{
+	struct gic_chip_data *d = data;
+
+	d->flags |= FLAGS_WORKAROUND_MARVELL_ERRATUM_38545;
+	static_branch_enable(&gic_iar_quirk);
+
+	return true;
+}
+
 static bool gic_enable_quirk_hip06_07(void *data)
 {
 	struct gic_chip_data *d = data;
@@ -1660,6 +1692,13 @@ static const struct gic_quirk gic_quirks[] = {
 		.iidr	= 0x00000000,
 		.mask	= 0xffffffff,
 		.init	= gic_enable_quirk_hip06_07,
+	},
+		/* ThunderX: CN88xx 1.x */
+	{
+		.desc	= "GICv3: Cavium erratum 23154",
+		.iidr	= 0xa101034c,
+		.mask	= 0xffff0fff,
+		.init	= gic_enable_quirk_cavium_23154,
 	},
 	{
 		/*
@@ -1674,6 +1713,19 @@ static const struct gic_quirk gic_quirks[] = {
 		.mask	= 0xe8f00fff,
 		.init	= gic_enable_quirk_cavium_38539,
 	},
+	{
+		/*
+		 * IAR register reads could be unreliable, under certain
+		 * race conditions. This erratum applies to:
+		 * - ThunderX: CN88xx
+		 * - OCTEON TX: CN83xx, CN81xx
+		 * - OCTEON TX2: CN93xx, CN96xx, CN98xx, CNF95xx*
+		 */
+		.desc	= "GICv3: Marvell erratum 38545",
+		.iidr	= 0xa000034c,
+		.mask	= 0xe0f00fff,
+		.init	= gic_enable_quirk_marvell_38545,
+	},
 	{
 	}
 };
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread