From: "Gautham R. Shenoy" <[email protected]>
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 <[email protected]>
---
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
diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
index 0c8421d..ffd4d59 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -68,6 +68,7 @@
#include <asm/isa-bridge.h>
#include <asm/security_features.h>
#include <asm/asm-const.h>
+#include <asm/idle.h>
#include <asm/swiotlb.h>
#include <asm/svm.h>
@@ -319,6 +320,8 @@ static int alloc_dispatch_log_kmem_cache(void)
static void pseries_lpar_idle(void)
{
+ unsigned long in_purr;
+
/*
* Default handler to go into low thread priority and possibly
* low power mode by ceding processor to hypervisor
@@ -328,7 +331,7 @@ static void pseries_lpar_idle(void)
return;
/* Indicate to hypervisor that we are idle. */
- get_lppaca()->idle = 1;
+ idle_loop_prolog(&in_purr);
/*
* Yield the processor to the hypervisor. We return if
@@ -339,7 +342,7 @@ static void pseries_lpar_idle(void)
*/
cede_processor();
- get_lppaca()->idle = 0;
+ idle_loop_epilog(in_purr);
}
/*
diff --git a/drivers/cpuidle/cpuidle-pseries.c b/drivers/cpuidle/cpuidle-pseries.c
index 74c2479..fc9dee9c 100644
--- a/drivers/cpuidle/cpuidle-pseries.c
+++ b/drivers/cpuidle/cpuidle-pseries.c
@@ -19,6 +19,7 @@
#include <asm/machdep.h>
#include <asm/firmware.h>
#include <asm/runlatch.h>
+#include <asm/idle.h>
#include <asm/plpar_wrappers.h>
struct cpuidle_driver pseries_idle_driver = {
@@ -31,29 +32,6 @@ struct cpuidle_driver pseries_idle_driver = {
static u64 snooze_timeout __read_mostly;
static bool snooze_timeout_en __read_mostly;
-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();
-}
-
static int snooze_loop(struct cpuidle_device *dev,
struct cpuidle_driver *drv,
int index)
--
1.9.4
"Gautham R. Shenoy" <[email protected]> writes:
> From: "Gautham R. Shenoy" <[email protected]>
>
> 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 <[email protected]>
> ---
> 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.
Hello Nathan,
On Fri, Feb 21, 2020 at 09:03:16AM -0600, Nathan Lynch wrote:
> "Gautham R. Shenoy" <[email protected]> writes:
>
> > From: "Gautham R. Shenoy" <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> > 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
Gautham R Shenoy <[email protected]> writes:
> On Fri, Feb 21, 2020 at 09:03:16AM -0600, Nathan Lynch wrote:
>> 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() ?
Yes that seems appropriate to me.