[RFC PATCH 10/36] arm64: irqflags: save and use both DAIF and PMR
Vladimir Murzin
vladimir.murzin at arm.com
Fri Jul 17 03:28:59 PDT 2026
On 7/10/26 10:02, Jinjie Ruan wrote:
>
> On 7/9/2026 8:13 PM, Vladimir Murzin wrote:
>> From: Ada Couprie Diaz <ada.coupriediaz at arm.com>
>>
>> With pseudo-NMIs enabled, both DAIF and the PMR affect interrupt masking.
>> Now that we have a type which can track both of them at the same time,
>> update our irqflags implementation to use it.
>>
>> Save DAIF flags in all cases, as they can be manipulated directly by other
>> code, and the PMR if it is in use.
>>
>> When checking if IRQs are disabled, now that we always save DAIF we can
>> check that the I flag is set and bypass checking the PMR if it is.
>> We can also properly check if PMR masks interrupts (PMR < GIC_PRIO_IRQON),
>> now that we don't need to rely on the GIC_PRIO_PSR_I_SET bit being set in
>> the PMR to know if DAIF is already masking interrupts.
>> Update `irqs_priority_unmasked()` to align with this change.
>>
>> This allows us to remove the `__daif_...` and `__pmr_...` versions
>> of the save and check functions, as they are now unified.
>>
>> We can reasonably merge the two `__{daif,pmr}_irq_restore()` functions
>> in the main one, as the DAIF and PMR values are properly split now.
>>
>> Signed-off-by: Ada Couprie Diaz <ada.coupriediaz at arm.com>
>> Signed-off-by: Vladimir Murzin <vladimir.murzin at arm.com>
>> ---
>> arch/arm64/include/asm/irqflags.h | 110 ++++++------------------------
>> arch/arm64/include/asm/ptrace.h | 2 +-
>> 2 files changed, 23 insertions(+), 89 deletions(-)
>>
>> diff --git a/arch/arm64/include/asm/irqflags.h b/arch/arm64/include/asm/irqflags.h
>> index 7775904ba6a9..62f047702493 100644
>> --- a/arch/arm64/include/asm/irqflags.h
>> +++ b/arch/arm64/include/asm/irqflags.h
>> @@ -95,117 +95,48 @@ static __always_inline void arch_local_irq_disable(void)
>> }
>> }
>>
>> -static __always_inline arm64_exc_hwstate_t __daif_local_save_flags(void)
>> -{
>> - return (arm64_exc_hwstate_t){ .daif = read_sysreg(daif) };
>> -}
>> -
>> -static __always_inline arm64_exc_hwstate_t __pmr_local_save_flags(void)
>> -{
>> - return (arm64_exc_hwstate_t){ .pmr = read_sysreg_s(SYS_ICC_PMR_EL1) };
>> -}
>> -
>> /*
>> * Save the current interrupt enable state.
>> */
>> static __always_inline unsigned long arch_local_save_flags(void)
>> {
>> - if (system_uses_irq_prio_masking()) {
>> - return __pmr_local_save_flags().flags;
>> - } else {
>> - return __daif_local_save_flags().flags;
>> - }
>> -}
>> + arm64_exc_hwstate_t hwstate = { .daif = read_sysreg(daif) };
>>
>> -static __always_inline
>> -bool __daif_irqs_disabled_flags(arm64_exc_hwstate_t hwstate)
>> -{
>> - return hwstate.daif & PSR_I_BIT;
>> -}
>> + if (system_uses_irq_prio_masking())
>> + hwstate.pmr = read_sysreg_s(SYS_ICC_PMR_EL1);
>>
>> -static __always_inline
>> -bool __pmr_irqs_disabled_flags(arm64_exc_hwstate_t hwstate)
>> -{
>> - return hwstate.pmr != GIC_PRIO_IRQON;
>> + return hwstate.flags;
>> }
>>
>> static __always_inline bool arch_irqs_disabled_flags(unsigned long flags)
>> {
>> arm64_exc_hwstate_t hwstate = { .flags = flags };
>>
>> - if (system_uses_irq_prio_masking()) {
>> - return __pmr_irqs_disabled_flags(hwstate);
>> - } else {
>> - return __daif_irqs_disabled_flags(hwstate);
>> - }
>> -}
>> + /* If I is set, the PMR doesn't matter: interrupts will not be taken. */
>> + if (hwstate.daif & PSR_I_BIT)
>> + return true;
>>
>> -static __always_inline bool __daif_irqs_disabled(void)
>> -{
>> - return __daif_irqs_disabled_flags(__daif_local_save_flags());
>> -}
>> + if (system_uses_irq_prio_masking() && hwstate.pmr < GIC_PRIO_IRQON)
> I think "hwstate.pmr == GIC_PRIO_IRQOFF" is fine, Besides DAIF.I bit set
> and pmr is GIC_PRIO_IRQOFF, are there any other possible cases?
>
Numerically, PMR can hold values ordered as:
IRQOFF < IRQON < IRQON | GIC_PRIO_PSR_I_SET
So hwstate.pmr < GIC_PRIO_IRQON is indeed IRQOFF.
The only other possibility would be a buggy case with the DAIF.I bit
clear and PMR set to IRQON | GIC_PRIO_PSR_I_SET.
So I think we can temporarily add a debug check for that, which would
disappear together with GIC_PRIO_PSR_I_SET. Something like:
static __always_inline bool arch_irqs_disabled_flags(unsigned long flags)
{
arm64_exc_hwstate_t hwstate = { .flags = flags };
if (hwstate.daif & PSR_I_BIT)
return true;
if (system_uses_irq_prio_masking()) {
WARN_ON_ONCE(IS_ENABLED(CONFIG_DEBUG_IRQFLAGS) &&
hwstate.pmr == GIC_PRIO_IRQON | GIC_PRIO_PSR_I_SET);
return hwstate.pmr != GIC_PRIO_IRQON;
}
return false;
}
>> + return true;
>>
>> -static __always_inline bool __pmr_irqs_disabled(void)
>> -{
>> - return __pmr_irqs_disabled_flags(__pmr_local_save_flags());
>> + return false;
>> }
>>
>> static __always_inline bool arch_irqs_disabled(void)
>> {
>> - if (system_uses_irq_prio_masking()) {
>> - return __pmr_irqs_disabled();
>> - } else {
>> - return __daif_irqs_disabled();
>> - }
>> -}
>> -
>> -static __always_inline arm64_exc_hwstate_t __daif_local_irq_save(void)
>> -{
>> - arm64_exc_hwstate_t hwstate = __daif_local_save_flags();
>> -
>> - __daif_local_irq_disable();
>> -
>> - return hwstate;
>> -}
>> -
>> -static __always_inline arm64_exc_hwstate_t __pmr_local_irq_save(void)
>> -{
>> - arm64_exc_hwstate_t hwstate = __pmr_local_save_flags();
>> -
>> - /*
>> - * There are too many states with IRQs disabled, just keep the current
>> - * state if interrupts are already disabled/masked.
>> - */
>> - if (!__pmr_irqs_disabled_flags(hwstate))
>> - __pmr_local_irq_disable();
>> -
>> - return hwstate;
>> + return arch_irqs_disabled_flags(arch_local_save_flags());
>> }
>>
>> static __always_inline unsigned long arch_local_irq_save(void)
>> {
>> - if (system_uses_irq_prio_masking()) {
>> - return __pmr_local_irq_save().flags;
>> - } else {
>> - return __daif_local_irq_save().flags;
>> - }
>> -}
>> + unsigned long flags = arch_local_save_flags();
>>
>> -static __always_inline
>> -void __daif_local_irq_restore(arm64_exc_hwstate_t hwstate)
>> -{
>> - barrier();
>> - write_sysreg(hwstate.daif, daif);
>> - barrier();
>> -}
>> + if (system_uses_irq_prio_masking())
>> + __pmr_local_irq_disable();
>> + else
>> + __daif_local_irq_disable();
>>
>> -static __always_inline
>> -void __pmr_local_irq_restore(arm64_exc_hwstate_t hwstate)
>> -{
>> - barrier();
>> - write_sysreg_s(hwstate.pmr, SYS_ICC_PMR_EL1);
>> - pmr_sync();
>> - barrier();
>> + return flags;
>> }
>>
>> /*
>> @@ -215,11 +146,14 @@ static __always_inline void arch_local_irq_restore(unsigned long flags)
>> {
>> arm64_exc_hwstate_t hwstate = { .flags = flags };
>>
>> + barrier();
>> if (system_uses_irq_prio_masking()) {
>> - __pmr_local_irq_restore(hwstate);
>> + write_sysreg_s(hwstate.pmr, SYS_ICC_PMR_EL1);
>> + pmr_sync();
>> } else {
>> - __daif_local_irq_restore(hwstate);
>> + write_sysreg(hwstate.daif, daif);
>> }
>> + barrier();
>> }
>>
>> #endif /* __ASM_IRQFLAGS_H */
>> diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
>> index f7dc5fb9427d..192eb97cd50b 100644
>> --- a/arch/arm64/include/asm/ptrace.h
>> +++ b/arch/arm64/include/asm/ptrace.h
>> @@ -205,7 +205,7 @@ static inline void forget_syscall(struct pt_regs *regs)
>>
>> #define irqs_priority_unmasked(regs) \
>> (system_uses_irq_prio_masking() ? \
>> - (regs)->pmr == GIC_PRIO_IRQON : \
>> + (regs)->pmr >= GIC_PRIO_IRQON : \
> I think "=== GIC_PRIO_IRQON" is fine.
>
Agree. This can be aligned to arch_irqs_disabled_flags() (maybe as a separate
patch):
static __always_inline bool regs_irqs_disabled(const struct pt_regs *regs)
{
if (regs->pstate & PSR_I_BIT)
return true;
if (system_uses_irq_prio_masking()) {
WARN_ON_ONCE(IS_ENABLED(CONFIG_DEBUG_IRQFLAGS) &&
regs->pmr == GIC_PRIO_IRQON | GIC_PRIO_PSR_I_SET);
return regs->pmr != GIC_PRIO_IRQON;
}
return false;
}
What do you reckon?
> otherwise, LGTM
> Reviewed-by: Jinjie Ruan <ruanjinjie at huawei.com>
>
>> true)
>>
>> static __always_inline bool regs_irqs_disabled(const struct pt_regs *regs)
>
Cheers
Vladimir
More information about the linux-arm-kernel
mailing list