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

From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
To: Marc Zyngier <maz@kernel.org>
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: Wed, 16 Jun 2021 13:07:35 +0100	[thread overview]
Message-ID: <20210616120735.GA31915@lpieralisi> (raw)
In-Reply-To: <87czsrthkv.wl-maz@kernel.org>

On Sat, Jun 12, 2021 at 01:04:16PM +0100, Marc Zyngier wrote:
> 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?

Yes absolutely, thanks a lot.

Lorenzo

> > 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