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

From: Marc Zyngier <maz@kernel.org>
To: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: linux-arm-kernel@lists.infradead.org,
	Will Deacon <will@kernel.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Valentin Schneider <valentin.schneider@arm.com>,
	Alexandru Elisei <alexandru.elisei@arm.com>,
	Russell King <linux@armlinux.org.uk>,
	kernel-team@android.com
Subject: Re: [PATCH 1/3] arm64: Add cpuidle context save/restore helpers
Date: Sat, 12 Jun 2021 13:04:16 +0100	[thread overview]
Message-ID: <87czsrthkv.wl-maz@kernel.org> (raw)
In-Reply-To: <20210611164657.GA9252@lpieralisi>

On Fri, 11 Jun 2021 17:46:57 +0100,
Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote:
> 
> On Tue, Jun 08, 2021 at 06:27:13PM +0100, Marc Zyngier wrote:
> > 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
> 
> > 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 <lorenzo.pieralisi@arm.com>

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