Re: [RFC PATCH 30/36] arm64: irq: Report FEAT_NMI masking local IRQs

在 2026/7/22 21:55, Vladimir Murzin 写道:

On 7/21/26 10:06, Jinjie Ruan wrote:

quoted

On 7/9/2026 8:13 PM, Vladimir Murzin wrote:

quoted

From: Ada Couprie Diaz <redacted>

As we clear SCTLR_EL1.SPINTMASK when enabling FEAT_NMI, ALLINT masks
IRQs and FIQs regardless of superpriority.
Update irqflags.h and `regs_irqs_disabled()` to take it into account,
so we properly keep track of

We have documentation at the top of irqflags.h which explains the DAIF
masking. Since the additional masking with NMIs is related and also
covers the IF in DAIF, extend the comment to note what's going on
with NMIs.

Co-developed-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Ada Couprie Diaz <redacted>
Signed-off-by: Vladimir Murzin <redacted>
---
 arch/arm64/include/asm/irqflags.h | 17 +++++++++++++++++
 arch/arm64/include/asm/ptrace.h   |  9 ++++++++-
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/irqflags.h b/arch/arm64/include/asm/irqflags.h
index b571d34bf11d..a291a231d644 100644
--- a/arch/arm64/include/asm/irqflags.h
+++ b/arch/arm64/include/asm/irqflags.h

@@ -20,6 +20,16 @@
  * always masked and unmasked together, and have no side effects for other
  * flags. Keeping to this order makes it easier for entry.S to know which
  * exceptions should be unmasked.
+ *
+ * With the addition of the FEAT_NMI extension we gain an additional
+ * class of superpriority IRQ/FIQ which is separately masked with a
+ * choice of modes controlled by SCTLR_ELn.{SPINTMASK,NMI}.
+ * Linux sets SPINTMASK to 0 and NMI to 1 which results in ALLINT.ALLINT
+ * masking both superpriority interrupts and IRQ/FIQ regardless of the
+ * I and F settings. Since these superpriority interrupts are being
+ * used as NMIs we do not include them in the interrupt masking here,
+ * anything that requires that NMIs be masked needs to explicitly do so,
+ * but we do check for ALLINT masking IRQs/FIQs.
  */
 
  /*

@@ -103,6 +113,9 @@ static __always_inline unsigned long arch_local_save_flags(void)
 {
 	arm64_exc_hwstate_t hwstate = { .daif = read_sysreg(daif) };
 
+	if (system_uses_nmi())
+		hwstate.allint = read_sysreg_s(SYS_ALLINT);
+
 	if (system_uses_irq_prio_masking())
 		hwstate.pmr = read_sysreg_s(SYS_ICC_PMR_EL1);
 

@@ -117,6 +130,10 @@ static __always_inline bool arch_irqs_disabled_flags(unsigned long flags)
 	if (hwstate.daif & PSR_I_BIT)
 		return true;
 
+	/* SCTLR_EL1.SPINTMASK is clear, so ALLINT masks *all* IRQs/FIQs. */
+	if (system_uses_nmi() && hwstate.allint > 0)

The ">0" here doesn't seem very rigorous, maybe:

if (system_uses_nmi() && hwstate.allint & ALLINT_ALLINT)

Agree.

quoted

quoted

+		return true;
+
 	if (system_uses_irq_prio_masking() && hwstate.pmr < GIC_PRIO_IRQON)
 		return true;
 

diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
index c630165882e8..096d85d75bd7 100644
--- a/arch/arm64/include/asm/ptrace.h
+++ b/arch/arm64/include/asm/ptrace.h

@@ -206,9 +206,16 @@ static inline void forget_syscall(struct pt_regs *regs)
 		(regs)->pmr >= GIC_PRIO_IRQON :				\
 		true)
 
+#define irqs_allint_clear(regs)				\
+	(system_uses_nmi() ?				\
+		((regs)->pstate & PSR_ALLINT_BIT) == 0:	\
+		true)
+
 static __always_inline bool regs_irqs_disabled(const struct pt_regs *regs)
 {
-	return (regs->pstate & PSR_I_BIT) || !irqs_priority_unmasked(regs);
+	return (regs->pstate & PSR_I_BIT) ||
+		!irqs_priority_unmasked(regs) ||
+		!irqs_allint_clear(regs);

Why not define the logic for disable? The semantics of
regs_irqs_disabled() is "interrupts disabled".

204 #define
irqs_priority_masked(regs)>----->------->------->------->-------\
205 >-------(system_uses_irq_prio_masking()
?>------>------->------->-------\
206 >------->-------(regs)->pmr == GIC_PRIO_IRQOFF
:>------->------->-------\
207 >------->-------false)
208
209 #define irqs_allint_masked(regs)>------->------->------->-------\
210 >-------(system_uses_nmi() ?>--->------->------->-------\
211 >------->-------((regs)->pstate & PSR_ALLINT_BIT) :>----\
212 >------->-------false)
213
214 static __always_inline bool regs_irqs_disabled(const struct pt_regs
*regs)
215 {
216 >-------return (regs->pstate & PSR_I_BIT) ||
217 >------->-------irqs_priority_masked(regs) ||
218 >------->-------irqs_allint_masked(regs);

That's in line with my proposal [1], with that it would look like

static __always_inline bool regs_irqs_disabled(const struct pt_regs *regs)
{
        if (regs->pstate & PSR_I_BIT)
                return true;

        if (system_uses_nmi())
                return !!(regs->pstate & PSR_ALLINT_BIT);

        if (system_uses_irq_prio_masking()) {
                WARN_ON_ONCE(IS_ENABLED(CONFIG_DEBUG_IRQFLAGS) &&
                             regs->pmr != GIC_PRIO_IRQOFF &&
                             regs->pmr != GIC_PRIO_IRQON);

                return regs->pmr != GIC_PRIO_IRQON;
        }

        return false;
}

I like this style.

[1] https://lore.kernel.org/linux-arm-kernel/fedf2b3e-14ed-4483-8cdf-d31f1ac97b84@arm.com/ (local)

quoted

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

quoted

 }
 
 #define interrupts_enabled(regs)	(!regs_irqs_disabled(regs))

Cheers
Vladimir