Re: [PATCH 1/3] arm64: Add cpuidle context save/restore helpers

On Fri, 11 Jun 2021 17:46:57 +0100,
Lorenzo Pieralisi [off-list ref] wrote:

On Tue, Jun 08, 2021 at 06:27:13PM +0100, Marc Zyngier wrote:

quoted

As we need to start doing some additional work on all idle
paths, let's introduce a set of macros that will perform
the work related to the GICv3 pseudo-NMI idle entry exit.

Stubs are introduced to 32bit ARM for compatibility.
As these helpers are currently unused, the is no functional

s/the/there

quoted

change.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm/include/asm/cpuidle.h   |  5 +++++
 arch/arm64/include/asm/cpuidle.h | 35 ++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/arch/arm/include/asm/cpuidle.h b/arch/arm/include/asm/cpuidle.h
index 0d67ed682e07..1e0b8da12d96 100644
--- a/arch/arm/include/asm/cpuidle.h
+++ b/arch/arm/include/asm/cpuidle.h

@@ -49,4 +49,9 @@ extern int arm_cpuidle_suspend(int index);
 
 extern int arm_cpuidle_init(int cpu);
 
+struct arm_cpuidle_context { };
+
+#define arm_cpuidle_save_context(c)	(void)c
+#define arm_cpuidle_restore_context(c)	(void)c
+
 #endif

diff --git a/arch/arm64/include/asm/cpuidle.h b/arch/arm64/include/asm/cpuidle.h
index 3c5ddb429ea2..53adad0a5c7e 100644
--- a/arch/arm64/include/asm/cpuidle.h
+++ b/arch/arm64/include/asm/cpuidle.h

@@ -18,4 +18,39 @@ static inline int arm_cpuidle_suspend(int index)
 	return -EOPNOTSUPP;
 }
 #endif
+
+#ifdef CONFIG_ARM64_PSEUDO_NMI
+#include <asm/arch_gicv3.h>
+
+struct arm_cpuidle_context {
+	unsigned long pmr;
+	unsigned long daif_bits;
+};
+
+#define arm_cpuidle_save_context(__c)					\
+	do {								\
+		struct arm_cpuidle_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); \
+		}							\
+	} while (0)
+
+#define arm_cpuidle_restore_context(__c)				\
+	do {								\
+		struct arm_cpuidle_context *c = __c;			\
+		if (system_uses_irq_prio_masking()) {			\
+			gic_write_pmr(c->pmr);				\
+			write_sysreg(c->daif_bits, daif);		\
+		}							\
+	} while (0)
+#else
+struct arm_cpuidle_context { };
+
+#define arm_cpuidle_save_context(c)	(void)c
+#define arm_cpuidle_restore_context(c)	(void)c
+#endif
 #endif

It looks good to me - maybe I would define it irq_context for clarity
but that's just a naming convention.

would:

struct arm_cpuidle_irq_context { ...  };
#define arm_cpuidle_save_irq_context(c)		...
#define arm_cpuidle_restore_irq_context(c)	...

be OK for you?

Reviewed-by: Lorenzo Pieralisi <redacted>

Thanks!

	M.

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel