[PATCH v6 1/3] arm64: Add raw variants of local_daif_save() and local_daif_restore()

Puranjay Mohan <[email protected]>
Newsgroups org.kernel.vger.linux-perf-users,org.infradead.lists.linux-arm-kernel,org.kernel.vger.bpf,org.kernel.vger.linux-kernel,org.kernel.vger.linux-kselftest
Message-ID <[email protected]>
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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.