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

[Jinjie Ruan <ruanjinjie@huawei.com>]

在 2026/7/22 21:55, Vladimir Murzin 写道:
> On 7/21/26 10:06, Jinjie Ruan wrote:
>>
>> On 7/9/2026 8:13 PM, Vladimir Murzin wrote:
>>> From: Ada Couprie Diaz <ada.coupriediaz@arm.com>
>>>
>>> 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 <ada.coupriediaz@arm.com>
>>> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
>>> ---
>>>  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.
> 
>>> +		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/
> 
>> otherwise, LGTM
>> Reviewed-by: Jinjie Ruan <ruanjinjie@huawei.com>
>>
>>>  }
>>>  
>>>  #define interrupts_enabled(regs)	(!regs_irqs_disabled(regs))
>>
> 
> Cheers
> Vladimir
>