Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Wed, 8 Apr 2020 15:20:33 +0530
From:   Gautham R Shenoy <ego@linux.vnet.ibm.com>
To:     Pratik Rajesh Sampat <psampat@linux.ibm.com>
Cc:     skiboot@lists.ozlabs.org, oohall@gmail.com, mikey@neuling.org,
        npiggin@gmail.com, vaidy@linux.ibm.com, ego@linux.vnet.ibm.com,
        linuxppc-dev@ozlabs.org, linux-kernel@vger.kernel.org,
        mpe@ellerman.id.au, pratik.r.sampat@gmail.com
Subject: Re: [RFC] Support stop state version quirk and firmware enabled stop
Message-ID: <20200408095033.GC950@in.ibm.com>
Reply-To: ego@linux.vnet.ibm.com
References: <20200304155648.11501-1-psampat@linux.ibm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200304155648.11501-1-psampat@linux.ibm.com>
User-Agent: Mutt/1.5.23 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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.