Re: [RFC] Support stop state version quirk and firmware enabled stop

Gautham R Shenoy
2020-04-08T09:50:52Z

Hi Pratik,

On Wed, Mar 04, 2020 at 09:26:48PM +0530, Pratik Rajesh Sampat wrote:
> A concept patch in Skiboot to illustrate the case wherein handling of
> stop states for different DD versions of a CPU can be achieved by a
> simple modification in the list of cpu_features.
> As an example idle-stop1 is defined which uses P9_CPU_DD1 to define the
> cpu feature.
>
> Along with that, an implementation is being worked upon the LE OPAL
> series which helps OPAL handle the stop state entry and exit.
>
> This patch advertises this capability of the firmware which can be
> availed if the quirk-version-setting is not cognizable.
>
> The firmware-enabled stop is being worked by Abhishek Goel
> <huntbag@linux.vnet.ibm.com> building upon the LE OPAL series.
>
> Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
> ---
> core/cpufeatures.c | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
> diff --git a/core/cpufeatures.c b/core/cpufeatures.c
> index ec30c975..b9875e7b 100644
> --- a/core/cpufeatures.c
> +++ b/core/cpufeatures.c
> @@ -510,6 +510,25 @@ static const struct cpu_feature cpu_features_table[] = {
> -1, -1, -1,
> NULL, },
>
> + /*
> + * QUIRK for ISAv3.0B stop idle instructions and registers
> + * Helps us determine if there are any quirks
> + * XXX: Same of idle-stop
> + */
> + { "idle-stop-v1",
> + CPU_P9_DD1,
> + ISA_V3_0B, USABLE_HV|USABLE_OS,
> + HV_CUSTOM, OS_CUSTOM,
> + -1, -1, -1,
> + NULL, },

So, at this point, we don't need any such quirk for any of the DD
version right ? This is to demonstrate that if say P9_DD1 had a quirk
w.r.t stop-state handling, then this is how we would advertise it to
the kernel.

> +
> + { "firmware-stop-supported",
> + CPU_P9,
> + ISA_V3_0B, USABLE_HV|USABLE_OS,
> + HV_CUSTOM, OS_CUSTOM,
> + -1, -1, -1,
> + NULL, },
> +

I suppose this is for the opal-cpuidle driver support posted here:
https://lists.ozlabs.org/pipermail/skiboot/2020-April/016726.html

> /*
> * ISAv3.0B Hypervisor Virtualization Interrupt
> * Also associated system registers, LPCR EE, HEIC, HVICE,
> @@ -883,6 +902,9 @@ static void add_cpufeatures(struct dt_node *cpus,
> const struct cpu_feature *f = &cpu_features_table[i];
>
> if (f->cpus_supported & cpu_feature_cpu) {
> + if (!strcmp(f->name, "firmware-stop-supported") &&
> + HAVE_BIG_ENDIAN)
> + continue;

In OPAL do we have an macro defining BIG_ENDIAN ? If yes, you could
wrap the "firmware-stop-supported" in cpu_features_table[] within
#ifndef BIG_ENDIAN. That way you won't need a special case here.

> DBG(" '%s'\n", f->name);
> add_cpu_feature_nodeps(features, f);
> }
> --
> 2.24.1
>

--
Thanks and Regards
gautham.