Re: [RFC PATCH 31/36] arm64: nmi: Add handling of superpriority interrupts as NMIs

[Vladimir Murzin <vladimir.murzin@arm.com>]

On 7/21/26 09:33, Jinjie Ruan wrote:
> 
> On 7/9/2026 8:13 PM, Vladimir Murzin wrote:
>> From: Mark Brown <broonie@kernel.org>
>>
>> Our goal with superpriority interrupts is to use them as NMIs, taking
>> advantage of the much smaller regions where they are masked to allow
>> prompt handling of the most time-critical interrupts.
>>
>> When an interrupt is configured with superpriority, we enter EL1 as we
>> do for any other interrupt. The presence of a superpriority interrupt
>> is indicated by a status bit in ISR_EL1. We check this bit before
>> unmasking interrupts in elX_interrupt(), and if a superpriority
>> interrupt is pending, we handle it as an NMI. Otherwise, normal
>> interrupts are handled as usual.
>>
>> Since superpriority interrupts are always handled as NMIs, the
>> interrupt controller can rely on in_nmi() to distinguish them from
>> ordinary interrupts.
>>
>> Enable IPIs to use superpriority interrupts as NMIs, matching the
>> existing pseudo-NMI behaviour.
>> 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/entry-common.h |  7 ++++
>>  arch/arm64/kernel/entry-common.c      | 59 ++++++++++++++++++---------
>>  arch/arm64/kernel/smp.c               |  2 +-
>>  3 files changed, 48 insertions(+), 20 deletions(-)
>>
>> diff --git a/arch/arm64/include/asm/entry-common.h b/arch/arm64/include/asm/entry-common.h
>> index 73d82a8d8e95..0681ba91ac3b 100644
>> --- a/arch/arm64/include/asm/entry-common.h
>> +++ b/arch/arm64/include/asm/entry-common.h
>> @@ -37,6 +37,13 @@ static inline bool arch_irqentry_exit_need_resched(void)
>>  	if (system_uses_irq_prio_masking() && read_sysreg(daif))
>>  		return false;
>>  
>> +	/*
>> +	 * If AllInt is set then we must have handled an NMI, so skip
>> +	 * preemption
>> +	 */
>> +	if (system_uses_nmi() && read_sysreg_s(SYS_ALLINT))
>> +		return false;
>> +
>>  	/*
>>  	 * Preempting a task from an IRQ means we leave copies of PSTATE
>>  	 * on the stack. cpufeature's enable calls may modify PSTATE, but
>> diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
>> index a13653b228b7..de71d5a3a6a1 100644
>> --- a/arch/arm64/kernel/entry-common.c
>> +++ b/arch/arm64/kernel/entry-common.c
>> @@ -525,8 +525,8 @@ asmlinkage void noinstr el1h_64_sync_handler(struct pt_regs *regs)
>>  	arm64_debug_exc_context(CRITICAL_CONTEXT);
>>  }
>>  
>> -static __always_inline void __el1_pnmi(struct pt_regs *regs,
>> -				       void (*handler)(struct pt_regs *))
>> +static __always_inline void __el1_nmi(struct pt_regs *regs,
>> +				      void (*handler)(struct pt_regs *))
>>  {
>>  	arm64_exc_hwstate_t hwstate;
>>  	irqentry_state_t state;
>> @@ -545,7 +545,10 @@ static __always_inline void __el1_irq(struct pt_regs *regs,
>>  
>>  	state = arm64_enter_from_kernel_mode(regs);
>>  
>> -	arm64_unmask_exc_context(NONMI_CONTEXT);
>> +	if (system_uses_nmi())
>> +		arm64_unmask_exc_context(NOIRQ_CONTEXT);
>> +	else
>> +		arm64_unmask_exc_context(NONMI_CONTEXT);
> if (gic_supports_pseudo_nmis())
> 	arm64_unmask_exc_context(NONMI_CONTEXT);
> else
> 	arm64_unmask_exc_context(NOIRQ_CONTEXT);
> 
>>  
>>  	irq_enter_rcu();
>>  	do_interrupt_handler(regs, handler);
>> @@ -565,8 +568,11 @@ static __always_inline void __el1_irq(struct pt_regs *regs,
>>  static void noinstr el1_interrupt(struct pt_regs *regs,
>>  				  void (*handler)(struct pt_regs *))
>>  {
>> -	if (IS_ENABLED(CONFIG_ARM64_PSEUDO_NMI) && regs_irqs_disabled(regs))
>> -		__el1_pnmi(regs, handler);
>> +	/* Is there a NMI to handle? */
>> +	if (regs_irqs_disabled(regs))
>> +		__el1_nmi(regs, handler);
>> +	else if (system_uses_nmi() && (read_sysreg(isr_el1) & (ISR_EL1_IS | ISR_EL1_FS)))
>> +		__el1_nmi(regs, handler);
> I think it would be more readable to add a helper below.
> 
> static inline bool is_nmi(regs)
> {
> 	if (regs_irqs_disabled(regs))
> 		return true;
> 
> 	if (system_uses_nmi())
> 		return read_sysreg(isr_el1) & (ISR_EL1_IS | ISR_EL1_FS);
> 
> 	return false;
> }
> 

Makes sense, yet I would keep regs_irqs_disabled() limited to EL1,
since it is very tempting to reuse the helper for EL0 interrupts,
where the regs_irqs_disabled() check is redundant.

>>  static void noinstr el1_interrupt(struct pt_regs *regs,
>>  				  void (*handler)(struct pt_regs *))
>>  {
>> -	if (IS_ENABLED(CONFIG_ARM64_PSEUDO_NMI) && regs_irqs_disabled(regs))
>> -		__el1_pnmi(regs, handler);
>> +	/* Is there a NMI to handle? */
>> +	if (is_nmi(regs))
>> +		__el1_nmi(regs, handler);
>>  	else
>>  		__el1_irq(regs, handler);
>>  
>> @@ -906,24 +912,39 @@ asmlinkage void noinstr el0t_64_sync_handler(struct pt_regs *regs)
>>  static void noinstr el0_interrupt(struct pt_regs *regs,
>>  				  void (*handler)(struct pt_regs *))
>>  {
>> -	arm64_enter_from_user_mode(regs);
>> -
>> -	arm64_unmask_exc_context(NONMI_CONTEXT);
>> -
>>  	if (regs->pc & BIT(55))
>>  		arm64_apply_bp_hardening();
>>  
>> -	irq_enter_rcu();
>> -	do_interrupt_handler(regs, handler);
>> -	irq_exit_rcu();
>> +	/* Is there a NMI to handle? */
>> +	if (system_uses_nmi() && (read_sysreg(isr_el1) & (ISR_EL1_IS | ISR_EL1_FS))) {
>> +		irqentry_state_t state;
>> +		arm64_exc_hwstate_t hwstate;
>> +
>> +		state = irqentry_nmi_enter(regs);
>> +		hwstate = arm64_unmask_exc_context(NONMI_CONTEXT);
>> +		do_interrupt_handler(regs, handler);
>> +		arm64_mask_exc_context(hwstate);
>> +		irqentry_nmi_exit(regs, state);
>> +	} else {
>> +		arm64_enter_from_user_mode(regs);
>> +
>> +		if (system_uses_nmi())
>> +			arm64_unmask_exc_context(NOIRQ_CONTEXT);
>> +		else
>> +			arm64_unmask_exc_context(NONMI_CONTEXT);
> if (gic_supports_pseudo_nmis())
> 	arm64_unmask_exc_context(NONMI_CONTEXT);
> else
> 	arm64_unmask_exc_context(NOIRQ_CONTEXT);
> 
>> +
>> +		irq_enter_rcu();
>> +		do_interrupt_handler(regs, handler);
>> +		irq_exit_rcu();
>> +		/*
>> +		 * For the same reason as in el1_irq() we effectivly
>> +		 * have NOIRQ_CONTEXT on return from handler - keep
>> +		 * track of it
>> +		 */
>> +		arm64_debug_exc_context(NOIRQ_CONTEXT);
>> +		arm64_exit_to_user_mode(regs, arm64_exc_hwstate_of_context(NOIRQ_CONTEXT));
> Here we assert that we are in NOIRQ_CONTEXT, and then we switch to
> NOIRQ_CONTEXT again. Is this redundant?
> 

Well, it is redundant prior to the whole rework, but after the rework
it becomes a no-op because both the previous and next exception states
are the same, so no real work is done. :)

>> +	}
> Can you split them into two sub-functions, such as el0_nmi and el0_irq
> as shown below, so that it looks clearer?
> 

Works for me, I'll update in the next iteration.

> static inline void el0_nmi(struct pt_regs *regs, void (*handler)(struct
> pt_regs *))
> {
> 	arm64_exc_hwstate_t hwstate;
> 	irqentry_state_t state;
> 	
> 	state = irqentry_nmi_enter(regs);
> 	hwstate = arm64_unmask_exc_context(NONMI_CONTEXT);
> 	do_interrupt_handler(regs, handler);
> 	arm64_mask_exc_context(hwstate);
> 	irqentry_nmi_exit(regs, state);
> }
> 
> static inline void el0_irq(struct pt_regs *regs, void (*handler)(struct
> pt_regs *))
> {
> 	arm64_enter_from_user_mode(regs);
> 
> 	if (gic_supports_pseudo_nmis())
> 		arm64_unmask_exc_context(NONMI_CONTEXT);
> 	else
> 		arm64_unmask_exc_context(NOIRQ_CONTEXT);
> 
> 	irq_enter_rcu();
> 	do_interrupt_handler(regs, handler);
> 	irq_exit_rcu();
> 	/*
> 	 * For the same reason as in el1_irq() we effectivly
>     	 * have NOIRQ_CONTEXT on return from handler - keep
>          * track of it
>       	 */
> 	arm64_debug_exc_context(NOIRQ_CONTEXT);			
> 	arm64_exit_to_user_mode(regs, 	
> arm64_exc_hwstate_of_context(NOIRQ_CONTEXT));
> }
> 
> static void noinstr el0_interrupt(struct pt_regs *regs,
>  				  void (*handler)(struct pt_regs *))
> {
> 	if (is_nmi(regs))
> 		el0_nmi(regs);
> 	else
> 		el0_irq(regs);
> }
> 
> otherwise, LGTM
> Reviewed-by: Jinjie Ruan <ruanjinjie@huawei.com>
> 
> 

Cheers
Vladimir

>>  
>> -	/*
>> -	 * For the same reason as in el1_irq() we effectivly
>> -	 * have NOIRQ_CONTEXT on return from handler - keep
>> -	 * track of it
>> -	 */
>> -	arm64_debug_exc_context(NOIRQ_CONTEXT);
>> -	arm64_exit_to_user_mode(regs, arm64_exc_hwstate_of_context(NOIRQ_CONTEXT));
>>  	arm64_debug_exc_context(CRITICAL_CONTEXT);
>>  }
>>  
>> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
>> index 257d50529d14..80e35a8e5c8f 100644
>> --- a/arch/arm64/kernel/smp.c
>> +++ b/arch/arm64/kernel/smp.c
>> @@ -1035,7 +1035,7 @@ static void smp_cross_call(const struct cpumask *target, unsigned int ipinr)
>>  
>>  static bool ipi_should_be_nmi(enum ipi_msg_type ipi)
>>  {
>> -	if (!system_uses_irq_prio_masking())
>> +	if (!system_uses_nmi() && !system_uses_irq_prio_masking())
>>  		return false;
>>  
>>  	switch (ipi) {
>