linux-kernel - Re: [PATCH v2] arm64: dts: Hi3660: Fix up psci state id

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5A3CD23C.3080307@hisilicon.com>
Date:   Fri, 22 Dec 2017 09:37:00 +0000
From:   Wei Xu <xuwei5@...ilicon.com>
To:     Leo Yan <leo.yan@...aro.org>, Rob Herring <robh+dt@...nel.org>,
        "Mark Rutland" <mark.rutland@....com>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will.deacon@....com>,
        <linux-arm-kernel@...ts.infradead.org>,
        <devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>
CC:     Vincent Guittot <vincent.guittot@...aro.org>,
        Daniel Lezcano <daniel.lezcano@...aro.org>,
        Sudeep Holla <sudeep.holla@....com>,
        Soby Mathew <Soby.Mathew@....com>
Subject: Re: [PATCH v2] arm64: dts: Hi3660: Fix up psci state id

Hi Leo,

On 2017/12/12 9:12, Leo Yan wrote:
> Thanks a lot for Vincent Guittot careful work to find bug for 'CPU_NAP'
> idle state.  From ftrace log we can observe CA73 CPUs can be easily
> waken up from 'CPU_NAP' state but the 'waken up' CPUs doesn't handle
> anything and sleep again; so there have tons of trace events for CA73
> CPUs entering and exiting idle state.
> 
> On Hi3660 CA73 has retention state 'CPU_NAP' for CPU idle, this state we
> set its psci parameter as '0x0000001' and from this parameter it can
> calculate state id is 1.  Unfortunately ARM trusted firmware (ARM-TF)
> takes 1 as a invalid value for state id, so the CPU cannot enter idle
> state and directly bail out to kernel.
> 
> We want to create good practice for psci parameters platform definition,
> so review the psci specification. The spec "ARM Power State Coordination
> Interface - Platform Design Document (ARM DEN 0022D)" recommends state
> ID in chapter "6.5 Recommended StateID Encoding".  The recommended power
> state IDs can be presented by below listed values; and it divides into
> three fields, every field can use 4 bits to present power states
> corresponding to core level, cluster level and system level:
>   0: Run
>   1: Standby
>   2: Retention
>   3: Powerdown
> 
> This commit changes psci parameter to compliance with the suggested
> state ID in the doc.  Except we change 'CPU_NAP' state psci parameter
> to '0x0000002', this commit also changes 'CPU_SLEEP' and 'CLUSTER_SLEEP'
> state parameters to '0x0010003' and '0x1010033' respectively.
> 
> Credits to Daniel, Sudeep and Soby for suggestion and consolidation.
> 
> Cc: Vincent Guittot <vincent.guittot@...aro.org>
> Cc: Daniel Lezcano <daniel.lezcano@...aro.org>
> Cc: Sudeep Holla <sudeep.holla@....com>
> Cc: Soby Mathew <Soby.Mathew@....com>
> Signed-off-by: Leo Yan <leo.yan@...aro.org>
> ---

Applied into hisilicon dt tree.
Thanks!

Best Regards,
Wei

>  arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> index ab0b95b..99d5a46 100644
> --- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> +++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> @@ -147,7 +147,7 @@
>  
>  			CPU_NAP: cpu-nap {
>  				compatible = "arm,idle-state";
> -				arm,psci-suspend-param = <0x0000001>;
> +				arm,psci-suspend-param = <0x0000002>;
>  				entry-latency-us = <7>;
>  				exit-latency-us = <2>;
>  				min-residency-us = <15>;
> @@ -156,7 +156,7 @@
>  			CPU_SLEEP: cpu-sleep {
>  				compatible = "arm,idle-state";
>  				local-timer-stop;
> -				arm,psci-suspend-param = <0x0010000>;
> +				arm,psci-suspend-param = <0x0010003>;
>  				entry-latency-us = <40>;
>  				exit-latency-us = <70>;
>  				min-residency-us = <3000>;
> @@ -165,7 +165,7 @@
>  			CLUSTER_SLEEP_0: cluster-sleep-0 {
>  				compatible = "arm,idle-state";
>  				local-timer-stop;
> -				arm,psci-suspend-param = <0x1010000>;
> +				arm,psci-suspend-param = <0x1010033>;
>  				entry-latency-us = <500>;
>  				exit-latency-us = <5000>;
>  				min-residency-us = <20000>;
> @@ -174,7 +174,7 @@
>  			CLUSTER_SLEEP_1: cluster-sleep-1 {
>  				compatible = "arm,idle-state";
>  				local-timer-stop;
> -				arm,psci-suspend-param = <0x1010000>;
> +				arm,psci-suspend-param = <0x1010033>;
>  				entry-latency-us = <1000>;
>  				exit-latency-us = <5000>;
>  				min-residency-us = <20000>;
>