Hello Nathan, On Fri, Feb 21, 2020 at 09:03:16AM -0600, Nathan Lynch wrote: > "Gautham R. Shenoy" <e...@linux.vnet.ibm.com> writes: > > > From: "Gautham R. Shenoy" <e...@linux.vnet.ibm.com> > > > > Currently prior to entering an idle state on a Linux Guest, the > > pseries cpuidle driver implement an idle_loop_prolog() and > > idle_loop_epilog() functions which ensure that idle_purr is correctly > > computed, and the hypervisor is informed that the CPU cycles have been > > donated. > > > > These prolog and epilog functions are also required in the default > > idle call, i.e pseries_lpar_idle(). Hence move these accessor > > functions to a common header file and call them from > > pseries_lpar_idle(). Since the existing header files such as > > asm/processor.h have enough clutter, create a new header file > > asm/idle.h. > > > > Signed-off-by: Gautham R. Shenoy <e...@linux.vnet.ibm.com> > > --- > > arch/powerpc/include/asm/idle.h | 27 +++++++++++++++++++++++++++ > > arch/powerpc/platforms/pseries/setup.c | 7 +++++-- > > drivers/cpuidle/cpuidle-pseries.c | 24 +----------------------- > > 3 files changed, 33 insertions(+), 25 deletions(-) > > create mode 100644 arch/powerpc/include/asm/idle.h > > > > diff --git a/arch/powerpc/include/asm/idle.h > > b/arch/powerpc/include/asm/idle.h > > new file mode 100644 > > index 0000000..f32a7d8 > > --- /dev/null > > +++ b/arch/powerpc/include/asm/idle.h > > @@ -0,0 +1,27 @@ > > +#ifndef _ASM_POWERPC_IDLE_H > > +#define _ASM_POWERPC_IDLE_H > > +#include <asm/runlatch.h> > > + > > +static inline void idle_loop_prolog(unsigned long *in_purr) > > +{ > > + ppc64_runlatch_off(); > > + *in_purr = mfspr(SPRN_PURR); > > + /* > > + * Indicate to the HV that we are idle. Now would be > > + * a good time to find other work to dispatch. > > + */ > > + get_lppaca()->idle = 1; > > +} > > + > > +static inline void idle_loop_epilog(unsigned long in_purr) > > +{ > > + u64 wait_cycles; > > + > > + wait_cycles = be64_to_cpu(get_lppaca()->wait_state_cycles); > > + wait_cycles += mfspr(SPRN_PURR) - in_purr; > > + get_lppaca()->wait_state_cycles = cpu_to_be64(wait_cycles); > > + get_lppaca()->idle = 0; > > + > > + ppc64_runlatch_on(); > > +} > > +#endif > > Looks fine and correct as a cleanup, but asm/include/idle.h and > idle_loop_prolog, idle_loop_epilog, strike me as too generic for > pseries-specific code.
Should it be prefixed with pseries , i.e pseries_idle_prolog() and pseries_idle_epilog() ? Also, I am planning another round of cleanup to move all the idle-related declaration from asm/include/processor.h to asm/include/idle.h