Re: [RFC PATCH 10/36] arm64: irqflags: save and use both DAIF and PMR
From: Liao, Chang <hidden>
Date: 2026-07-10 03:53:43
在 2026/7/9 20:13, Vladimir Murzin 写道:
quoted hunk ↗ jump to hunk
From: Ada Couprie Diaz <redacted> 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 <redacted> Signed-off-by: Vladimir Murzin <redacted> --- 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) + 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 : \ true) static __always_inline bool regs_irqs_disabled(const struct pt_regs *regs)
Is there any reason to keep interrupt_enabled()? It appears to be rarely used across interrupt and exception code. So far, i've only found a single use in gic_handle_irq(), corret me if i'm wrong. So using regs_irqs_disable() directly might reduce the number of small helper that people need to remember. -- BR Liao, Chang