[PATCH] gic: increase the arch_timer priority to avoid hardlockup

[PATCH] gic: increase the arch_timer priority to avoid hardlockup

[Yifan Wu]

From: Qinxin Xia <xiaqinxin@huawei.com>

----------------------------------------------------------------------

On HIP12, when GIC receives multiple interrupts of the same priority and
different types, the interrupts are selected in the following sequence:
SPI > LPI > SGI > PPI. This scheduling rule may cause PPI starvation.
To prevent starvation from triggering system watchdog hardlockup, the
interrupt priority is explicitly increased in the arch_timer driver.

Signed-off-by: Qinxin Xia <xiaqinxin@huawei.com>
Signed-off-by: Hongye Lin <linhongye@h-partners.com>
Signed-off-by: Yifan Wu <wuyifan50@huawei.com>
---
 Documentation/arch/arm64/silicon-errata.rst |  2 +
 arch/arm64/Kconfig                          | 11 ++++++
 drivers/clocksource/arm_arch_timer.c        | 44 +++++++++++++++++++++
 drivers/irqchip/irq-gic-v3.c                |  7 +++-
 include/linux/irqchip/arm-gic-v3.h          |  3 ++
 5 files changed, 66 insertions(+), 1 deletion(-)

diff --git a/Documentation/arch/arm64/silicon-errata.rst b/Documentation/arch/arm64/silicon-errata.rst
index a7ec57060f64..6e6d5c953c99 100644
--- a/Documentation/arch/arm64/silicon-errata.rst
+++ b/Documentation/arch/arm64/silicon-errata.rst
@@ -266,6 +266,8 @@ stable kernels.
 +----------------+-----------------+-----------------+-----------------------------+
 | Hisilicon      | Hip09           | #162100801      | HISILICON_ERRATUM_162100801 |
 +----------------+-----------------+-----------------+-----------------------------+
+| Hisilicon      | Hip12           | #165010801      | HISILICON_ERRATUM_165010801 |
++----------------+-----------------+-----------------+-----------------------------+
 +----------------+-----------------+-----------------+-----------------------------+
 | Qualcomm Tech. | Kryo/Falkor v1  | E1003           | QCOM_FALKOR_ERRATUM_1003    |
 +----------------+-----------------+-----------------+-----------------------------+
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 6663ffd23f25..ec75dc0cf14d 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1273,6 +1273,17 @@ config HISILICON_ERRATUM_162100801
 
 	  If unsure, say Y.
 
+config HISILICON_ERRATUM_165010801
+	bool "Hisilicon erratum 165010801"
+	depends on ARCH_HISI
+	default y
+	help
+	  On HIP12, when GIC receives multiple interrupts of the same priority and
+	  different types, the interrupts are selected in the following sequence:
+	  SPI > LPI > SGI > PPI. This scheduling rule may cause PPI starvation.
+	  To prevent starvation from triggering system watchdog hardlockup, the
+	  interrupt priority is explicitly increased in the arch_timer driver.
+
 config QCOM_FALKOR_ERRATUM_1003
 	bool "Falkor E1003: Incorrect translation due to ASID change"
 	default y
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 90aeff44a276..834b904b460c 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -29,6 +29,11 @@
 #include <linux/arm-smccc.h>
 #include <linux/ptp_kvm.h>
 
+#ifdef CONFIG_HISILICON_ERRATUM_165010801
+#include <linux/irqchip/arm-gic-common.h>
+#include <linux/irqchip/arm-gic-v3.h>
+#endif
+
 #include <asm/arch_timer.h>
 #include <asm/virt.h>
 
@@ -61,6 +66,9 @@ static enum vdso_clock_mode vdso_default = VDSO_CLOCKMODE_ARCHTIMER;
 #else
 static enum vdso_clock_mode vdso_default = VDSO_CLOCKMODE_NONE;
 #endif /* CONFIG_GENERIC_GETTIMEOFDAY */
+#ifdef CONFIG_HISILICON_ERRATUM_165010801
+static bool prio_setup;
+#endif
 
 static cpumask_t evtstrm_available = CPU_MASK_NONE;
 static bool evtstrm_enable __ro_after_init = IS_ENABLED(CONFIG_ARM_ARCH_TIMER_EVTSTREAM);
@@ -254,6 +262,17 @@ static const struct ate_acpi_oem_info hisi_161010101_oem_info[] = {
 };
 #endif
 
+#ifdef CONFIG_HISILICON_ERRATUM_165010801
+static struct ate_acpi_oem_info hisi_165010801_oem_info[] = {
+	{
+		.oem_id		= "HISI  ",
+		.oem_table_id	= "HIP12   ",
+		.oem_revision	= 0,
+	},
+	{ /* Sentinel indicating the end of the OEM array */ },
+};
+#endif
+
 #ifdef CONFIG_ARM64_ERRATUM_858921
 static u64 notrace arm64_858921_read_cntpct_el0(void)
 {
@@ -384,6 +403,13 @@ static const struct arch_timer_erratum_workaround ool_workarounds[] = {
 		.set_next_event_virt = erratum_set_next_event_virt,
 	},
 #endif
+#ifdef CONFIG_HISILICON_ERRATUM_165010801
+	{
+		.match_type = ate_match_acpi_oem_info,
+		.id = hisi_165010801_oem_info,
+		.desc = "HiSilicon erratum 165010801",
+	},
+#endif
 #ifdef CONFIG_ARM64_ERRATUM_858921
 	{
 		.match_type = ate_match_local_cap_id,
@@ -491,6 +517,12 @@ void arch_timer_enable_workaround(const struct arch_timer_erratum_workaround *wa
 	if (wa->read_cntvct_el0 || wa->read_cntpct_el0)
 		atomic_set(&timer_unstable_counter_workaround_in_use, 1);
 
+#ifdef CONFIG_HISILICON_ERRATUM_165010801
+	if (!strncmp(wa->desc, "HiSilicon erratum 165010801",
+		     strlen("HiSilicon erratum 165010801")))
+		prio_setup = true;
+#endif
+
 	/*
 	 * Don't use the vdso fastpath if errata require using the
 	 * out-of-line counter accessor. We may change our mind pretty
@@ -830,6 +862,18 @@ static int arch_timer_starting_cpu(unsigned int cpu)
 
 	__arch_timer_setup(clk);
 
+#ifdef CONFIG_HISILICON_ERRATUM_165010801
+	if (prio_setup && is_kernel_in_hyp_mode()) {
+		struct irq_data *d = irq_get_irq_data(arch_timer_ppi[arch_timer_uses_ppi]);
+
+		if (!d)
+			pr_warn("WARNING: Invalid arch_timer ppi irq: %d!\n",
+				arch_timer_ppi[arch_timer_uses_ppi]);
+		else
+			gic_irq_set_prio(d, GICD_INT_DEF_PRI & (GICD_INT_DEF_PRI - 1));
+	}
+#endif
+
 	flags = check_ppi_trigger(arch_timer_ppi[arch_timer_uses_ppi]);
 	enable_percpu_irq(arch_timer_ppi[arch_timer_uses_ppi], flags);
 
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index 3de351e66ee8..11f737328857 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -584,7 +584,12 @@ static int gic_irq_get_irqchip_state(struct irq_data *d,
 	return 0;
 }
 
-static void gic_irq_set_prio(struct irq_data *d, u8 prio)
+#ifndef CONFIG_HISILICON_ERRATUM_165010801
+static void
+#else
+void
+#endif
+gic_irq_set_prio(struct irq_data *d, u8 prio)
 {
 	void __iomem *base = gic_dist_base(d);
 	u32 offset, index;
diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h
index 70c0948f978e..1c5ed36fcaf7 100644
--- a/include/linux/irqchip/arm-gic-v3.h
+++ b/include/linux/irqchip/arm-gic-v3.h
@@ -656,6 +656,9 @@ static inline bool gic_enable_sre(void)
 	return !!(val & ICC_SRE_EL1_SRE);
 }
 
+#ifdef CONFIG_HISILICON_ERRATUM_165010801
+void gic_irq_set_prio(struct irq_data *d, u8 prio);
+#endif
 #endif
 
 #endif
-- 
2.33.0

Re: [PATCH] gic: increase the arch_timer priority to avoid hardlockup

[Marc Zyngier]

On Thu, 16 Oct 2025 04:47:33 +0100,
Yifan Wu <wuyifan50@huawei.com> wrote:
> 
> From: Qinxin Xia <xiaqinxin@huawei.com>
> 
> ----------------------------------------------------------------------
> 
> On HIP12, when GIC receives multiple interrupts of the same priority and
> different types, the interrupts are selected in the following sequence:
> SPI > LPI > SGI > PPI. This scheduling rule may cause PPI starvation.
> To prevent starvation from triggering system watchdog hardlockup, the
> interrupt priority is explicitly increased in the arch_timer driver.

No. This breaks pseudo NMIs, and is way too invasive. Also, how about
the other PPIs? Frankly, if your GIC is not able to do some form of
fair delivery, then it probably isn't fit for purpose.

I don't think this sort of change is acceptable.

	M.

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

Re: [PATCH] gic: increase the arch_timer priority to avoid hardlockup

[Marc Zyngier]

On Thu, 16 Oct 2025 08:12:23 +0100,
Marc Zyngier <maz@kernel.org> wrote:
> 
> On Thu, 16 Oct 2025 04:47:33 +0100,
> Yifan Wu <wuyifan50@huawei.com> wrote:
> > 
> > From: Qinxin Xia <xiaqinxin@huawei.com>
> > 
> > ----------------------------------------------------------------------
> > 
> > On HIP12, when GIC receives multiple interrupts of the same priority and
> > different types, the interrupts are selected in the following sequence:
> > SPI > LPI > SGI > PPI. This scheduling rule may cause PPI starvation.
> > To prevent starvation from triggering system watchdog hardlockup, the
> > interrupt priority is explicitly increased in the arch_timer driver.
> 
> No. This breaks pseudo NMIs, and is way too invasive. Also, how about
> the other PPIs? Frankly, if your GIC is not able to do some form of
> fair delivery, then it probably isn't fit for purpose.

Thinking about this some more, this is even worse than it looks.

Your GIC is, one way or another, implementing the interrupt classes as
a form of extra (sub-)priority bits, which breaks everything. How do
you want to solve this?  By adding some other hardcoded priority
scheme. That's just as bad as the situation you have now.

What happens if, for example, your have a CPU with a screaming timer
interrupt and that another CPU tries to IPI it? Same problem, just
pushed one level further.

You cannot solve this by just moving from one bad priority scheme to
another. There is simply no priority scheme that can solve this,
because priorities are the exact opposite of fairness, and fairness is
the property we are relying on.

Thanks,

	M.

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