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

[Jinjie Ruan <ruanjinjie@huawei.com>]

在 2026/7/28 0:34, Vladimir Murzin 写道:
> From: Ada Couprie Diaz <ada.coupriediaz@arm.com>
> 
> 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 <ada.coupriediaz@arm.com>
> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
> ---
>  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.

> +	/*
> +	 * 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.

Otherwise, LGTM
Reviewed-by: Jinjie Ruan <ruanjinjie@huawei.com>

> +	states.expected.daif = states.saved.daif | DAIF_PROCCTX_NOIRQ;
> +	states.expected.pmr = GIC_PRIO_IRQON;
> +
> +	arm64_update_exc_hwstate(states.expected);
> +
> +	return states;
> +}
> +
> +/*
> + * Return to masking with the PMR, restoring previously saved DAIF and PMR.
> + *
> + * IRQs or interrupt priority masking should not have been re-enabled in between
> + * the save and restore.
> + */
> +static inline
> +void local_exceptions_force_daif_restore(arm64_exc_hwstates_t states)
> +{
> +	if (!system_uses_irq_prio_masking())
> +		return;
> +
> +	/*
> +	 * Cannot use lockdep_assert here as idle entry enables hardirqs
> +	 * while keeping interrupts masked.
> +	 */
> +	WARN_ON_ONCE(!irqs_disabled());
> +
> +	arm64_debug_exc_hwstate(states.expected);
> +	arm64_update_exc_hwstate(states.saved);
> +}
> +
>  /*
>   * During early boot, we unmask PSR.DA before the GIC has been set up.
>   * If we use IRQ priority masking, the PMR and PSR will be out of sync