local_daif_save() unconditionally calls trace_hardirqs_off(), and local_daif_restore() the matching trace_hardirqs_on(). A caller already in a hardirqs-off context, or one that must not run tracing code between masking exceptions and its next step, has no way to opt out.
cpu_suspend() is one such caller and already carries a comment asking for this. Sampling the BRBE branch record buffer is another: the tracing calls generate branches that evict the records about to be read. Split the tracing out into raw_local_daif_save() and raw_local_daif_restore(), and convert cpu_suspend() to the raw save with a lockdep_assert_irqs_disabled() for the precondition it now relies on. Its restore stays traced, to re-arm the irqsoff tracer for the resume path. The PMR unmasking cpu_suspend() needs is in raw_local_daif_mask(), so it is unaffected. Signed-off-by: Puranjay Mohan <[email protected]> --- arch/arm64/include/asm/daifflags.h | 41 +++++++++++++++++++++++++----- arch/arm64/kernel/suspend.c | 7 ++--- 2 files changed, 36 insertions(+), 12 deletions(-) diff --git a/arch/arm64/include/asm/daifflags.h b/arch/arm64/include/asm/daifflags.h index 795b351284673..c3adba0985bda 100644 --- a/arch/arm64/include/asm/daifflags.h +++ b/arch/arm64/include/asm/daifflags.h @@ -18,8 +18,7 @@ #define DAIF_MASK (PSR_D_BIT | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT) -/* mask/save/unmask/restore all exceptions, including interrupts. */ -static __always_inline void local_daif_mask(void) +static __always_inline void raw_local_daif_mask(void) { WARN_ON(system_has_prio_mask_debugging() && (read_sysreg_s(SYS_ICC_PMR_EL1) == (GIC_PRIO_IRQOFF | @@ -34,6 +33,12 @@ static __always_inline void local_daif_mask(void) /* Don't really care for a dsb here, we don't intend to enable IRQs */ if (system_uses_irq_prio_masking()) gic_write_pmr(GIC_PRIO_IRQON | GIC_PRIO_PSR_I_SET); +} + +/* mask/save/unmask/restore all exceptions, including interrupts. */ +static __always_inline void local_daif_mask(void) +{ + raw_local_daif_mask(); trace_hardirqs_off(); } @@ -53,18 +58,29 @@ static __always_inline unsigned long local_daif_save_flags(void) return flags; } -static __always_inline unsigned long local_daif_save(void) +static __always_inline unsigned long raw_local_daif_save(void) { unsigned long flags; flags = local_daif_save_flags(); - local_daif_mask(); + raw_local_daif_mask(); return flags; } -static __always_inline void local_daif_restore(unsigned long flags) +static __always_inline unsigned long local_daif_save(void) +{ + unsigned long flags; + + flags = raw_local_daif_save(); + + trace_hardirqs_off(); + + return flags; +} + +static __always_inline void __local_daif_restore(unsigned long flags, bool trace) { bool irq_disabled = flags & PSR_I_BIT; @@ -72,7 +88,8 @@ static __always_inline void local_daif_restore(unsigned long flags) (read_sysreg(daif) & (PSR_I_BIT | PSR_F_BIT)) != (PSR_I_BIT | PSR_F_BIT)); if (!irq_disabled) { - trace_hardirqs_on(); + if (trace) + trace_hardirqs_on(); if (system_uses_irq_prio_masking()) { gic_write_pmr(GIC_PRIO_IRQON); @@ -116,10 +133,20 @@ static __always_inline void local_daif_restore(unsigned long flags) write_sysreg(flags, daif); - if (irq_disabled) + if (irq_disabled && trace) trace_hardirqs_off(); } +static __always_inline void local_daif_restore(unsigned long flags) +{ + __local_daif_restore(flags, true); +} + +static __always_inline void raw_local_daif_restore(unsigned long flags) +{ + __local_daif_restore(flags, false); +} + /* * Called by synchronous exception handlers to restore the DAIF bits that were * modified by taking an exception. diff --git a/arch/arm64/kernel/suspend.c b/arch/arm64/kernel/suspend.c index c41724a40b756..461948ad15ec8 100644 --- a/arch/arm64/kernel/suspend.c +++ b/arch/arm64/kernel/suspend.c @@ -117,14 +117,11 @@ int cpu_suspend(unsigned long arg, int (*fn)(unsigned long)) * updates to mdscr register (saved and restored along with * general purpose registers) from kernel debuggers. * - * Strictly speaking the trace_hardirqs_off() here is superfluous, - * hardirqs should be firmly off by now. This really ought to use - * something like raw_local_daif_save(). - * * This also unmasks interrupts in PMR in order to reliably * resume if we're using pseudo-NMIs. */ - flags = local_daif_save(); + lockdep_assert_irqs_disabled(); + flags = raw_local_daif_save(); /* * Function graph tracer state gets inconsistent when the kernel -- 2.53.0-Meta
