Re: [RFC PATCH v2 19/45] arm64: cpuidle: use new helpers to bypass interrupt priority masking

On 8/11/26 09:59, Jinjie Ruan wrote:

在 2026/7/28 0:34, Vladimir Murzin 写道:

quoted

From: Ada Couprie Diaz <redacted>

When the CPU goes into WFI and pseudo-NMIs are in use, we need to make
sure that interrupts are not masked by the PMR otherwise they would not
reach the CPU and wake it up.

Now that we have a proper API that handles both DAIF and PMR, extend it
to provide helpers allowing to temporarily switch to masking interrupts
via DAIF only when interrupt priorities are in use.
This could allow other parts of the code to make use of it and makes it
easier to check for proper use.

Replace the custom cpuidle helper with those new helpers.

Signed-off-by: Ada Couprie Diaz <redacted>
Signed-off-by: Vladimir Murzin <redacted>
---
 arch/arm64/include/asm/cpuidle.h            | 18 ++----
 arch/arm64/include/asm/interrupts/masking.h | 61 +++++++++++++++++++++
 2 files changed, 65 insertions(+), 14 deletions(-)

diff --git a/arch/arm64/include/asm/cpuidle.h b/arch/arm64/include/asm/cpuidle.h
index 2047713e097d..b70ef5fd5e57 100644
--- a/arch/arm64/include/asm/cpuidle.h
+++ b/arch/arm64/include/asm/cpuidle.h

@@ -5,32 +5,22 @@
 #include <asm/proc-fns.h>
 
 #ifdef CONFIG_ARM64_PSEUDO_NMI
-#include <asm/arch_gicv3.h>
+#include <asm/interrupts/masking.h>
 
 struct arm_cpuidle_irq_context {
-	unsigned long pmr;
-	unsigned long daif_bits;
+	arm64_exc_hwstates_t arm64_context;
 };
 
 #define arm_cpuidle_save_irq_context(__c)				\
 	do {								\
 		struct arm_cpuidle_irq_context *c = __c;		\
-		if (system_uses_irq_prio_masking()) {			\
-			c->daif_bits = read_sysreg(daif);		\
-			write_sysreg(c->daif_bits | PSR_I_BIT | PSR_F_BIT, \
-				     daif);				\
-			c->pmr = gic_read_pmr();			\
-			gic_write_pmr(GIC_PRIO_IRQON | GIC_PRIO_PSR_I_SET); \
-		}							\
+		c->arm64_context = local_exceptions_force_daif_save(); 	\
 	} while (0)
 
 #define arm_cpuidle_restore_irq_context(__c)				\
 	do {								\
 		struct arm_cpuidle_irq_context *c = __c;		\
-		if (system_uses_irq_prio_masking()) {			\
-			gic_write_pmr(c->pmr);				\
-			write_sysreg(c->daif_bits, daif);		\
-		}							\
+		local_exceptions_force_daif_restore(c->arm64_context);	\
 	} while (0)
 #else
 struct arm_cpuidle_irq_context { };

diff --git a/arch/arm64/include/asm/interrupts/masking.h b/arch/arm64/include/asm/interrupts/masking.h
index 193089fda252..e13852442062 100644
--- a/arch/arm64/include/asm/interrupts/masking.h
+++ b/arch/arm64/include/asm/interrupts/masking.h

@@ -89,6 +89,67 @@ static inline void local_exceptions_final_mask(void)
 }
 #endif /* CONFIG_DEBUG_IRQFLAGS */
 
+/*
+ * In some cases, WFI or guest entry for example, we always want interrupts
+ * to reach the CPU even if masked. Masking via the PMR prevents them from
+ * reaching the CPU and waking it up.
+ * Force IRQ masking using DAIF by raising the priority mask
+ * and setting the IF flags.
+ *
+ * Should only be called when IRQs are already masked.
+ */
+static inline arm64_exc_hwstates_t local_exceptions_force_daif_save(void)
+{
+	arm64_exc_hwstates_t states = {};
+
+	if (!system_uses_irq_prio_masking())
+		return states;
+
+	/*
+	 * Cannot use lockdep_assert here as idle entry enables hardirqs
+	 * while keeping interrupts masked.
+	 */
+	WARN_ON_ONCE(!irqs_disabled());
+
+	states.saved.daif = read_sysreg(daif);
+	states.saved.pmr = read_sysreg_s(SYS_ICC_PMR_EL1);

I'd suggest using gic_read_pmr() as it aligns better with the original
implementation.

IIUC, the reason gic_read_pmr() exists is to be used in code shared between
arm64 and arm32. Since we are entirely in arm64, IMO, we better use sysreg
directly.

quoted

+	/*
+	 * We might have IF set or unset. In case IF already set orring with IF
+	 * won't change anything, combined with PMR set to IRQON we might become
+	 * one of contexts: CRITICAL, ERROR or NONMI. In case IF is unset (which
+	 * also implies that DA is unset) orring with IF and combining with PMR
+	 * set to IRQON would effectively gives us NONMI context.
+	 */

The comment seems redundant to me, because __arm64_update_exc_hwstate()
only needs the hwstate parameter and has no dependency on the context type.

Ok. I'll drop it in the next version.

Otherwise, LGTM
Reviewed-by: Jinjie Ruan <redacted>

Cheers
Vladimir