| 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
| ||
|
Message-ID: <e5b758ed-f279-ecc3-3be4-039c5a19b212@linaro.org>
Date: Sat, 1 Apr 2023 22:58:21 +0300
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Bhupesh Sharma <bhupesh.sharma@...aro.org>,
Konrad Dybcio <konrad.dybcio@...aro.org>
Cc: linux-arm-msm@...r.kernel.org, agross@...nel.org,
linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
andersson@...nel.org, bhupesh.linux@...il.com,
krzysztof.kozlowski@...aro.org, robh+dt@...nel.org
Subject: Re: [RESEND PATCH v2 1/1] arm64: dts: qcom: sm6115: Add CPU
idle-states
On 01/04/2023 21:26, Bhupesh Sharma wrote:
> Hi Konrad,
>
> On Sat, 1 Apr 2023 at 17:51, Konrad Dybcio <konrad.dybcio@...aro.org> wrote:
>>
>>
>>
>> On 30.03.2023 21:33, Bhupesh Sharma wrote:
>>> Add CPU idle-state nodes and power-domains in Qualcomm sm6115 SoC dtsi.
>>>
>>> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@...aro.org>
>>> ---
>>> Changes since v1:
>>> - v1 can be viewed here: https://lore.kernel.org/lkml/e5cda4cf-5c2a-a7ed-9e1d-1fe9f2cbef40@linaro.org
>>> - Addressed Konrad's comments on v1 and added GDHS and Power Collapse
>>> cluster power states.
>>>
>>> arch/arm64/boot/dts/qcom/sm6115.dtsi | 136 +++++++++++++++++++++++++++
>>> 1 file changed, 136 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/sm6115.dtsi b/arch/arm64/boot/dts/qcom/sm6115.dtsi
>>> index 2a51c938bbcb..b63395d476ed 100644
>>> --- a/arch/arm64/boot/dts/qcom/sm6115.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/sm6115.dtsi
>>> @@ -45,6 +45,8 @@ CPU0: cpu@0 {
>>> enable-method = "psci";
>>> next-level-cache = <&L2_0>;
>>> qcom,freq-domain = <&cpufreq_hw 0>;
>>> + power-domains = <&CPU_PD0>;
>>> + power-domain-names = "psci";
>>> L2_0: l2-cache {
>>> compatible = "cache";
>>> cache-level = <2>;
>>> @@ -61,6 +63,8 @@ CPU1: cpu@1 {
>>> enable-method = "psci";
>>> next-level-cache = <&L2_0>;
>>> qcom,freq-domain = <&cpufreq_hw 0>;
>>> + power-domains = <&CPU_PD1>;
>>> + power-domain-names = "psci";
>>> };
>>>
>>> CPU2: cpu@2 {
>>> @@ -73,6 +77,8 @@ CPU2: cpu@2 {
>>> enable-method = "psci";
>>> next-level-cache = <&L2_0>;
>>> qcom,freq-domain = <&cpufreq_hw 0>;
>>> + power-domains = <&CPU_PD2>;
>>> + power-domain-names = "psci";
>>> };
>>>
>>> CPU3: cpu@3 {
>>> @@ -85,6 +91,8 @@ CPU3: cpu@3 {
>>> enable-method = "psci";
>>> next-level-cache = <&L2_0>;
>>> qcom,freq-domain = <&cpufreq_hw 0>;
>>> + power-domains = <&CPU_PD3>;
>>> + power-domain-names = "psci";
>>> };
>>>
>>> CPU4: cpu@100 {
>>> @@ -97,6 +105,8 @@ CPU4: cpu@100 {
>>> dynamic-power-coefficient = <282>;
>>> next-level-cache = <&L2_1>;
>>> qcom,freq-domain = <&cpufreq_hw 1>;
>>> + power-domains = <&CPU_PD4>;
>>> + power-domain-names = "psci";
>>> L2_1: l2-cache {
>>> compatible = "cache";
>>> cache-level = <2>;
>>> @@ -113,6 +123,8 @@ CPU5: cpu@101 {
>>> enable-method = "psci";
>>> next-level-cache = <&L2_1>;
>>> qcom,freq-domain = <&cpufreq_hw 1>;
>>> + power-domains = <&CPU_PD5>;
>>> + power-domain-names = "psci";
>>> };
>>>
>>> CPU6: cpu@102 {
>>> @@ -125,6 +137,8 @@ CPU6: cpu@102 {
>>> enable-method = "psci";
>>> next-level-cache = <&L2_1>;
>>> qcom,freq-domain = <&cpufreq_hw 1>;
>>> + power-domains = <&CPU_PD6>;
>>> + power-domain-names = "psci";
>>> };
>>>
>>> CPU7: cpu@103 {
>>> @@ -137,6 +151,8 @@ CPU7: cpu@103 {
>>> enable-method = "psci";
>>> next-level-cache = <&L2_1>;
>>> qcom,freq-domain = <&cpufreq_hw 1>;
>>> + power-domains = <&CPU_PD7>;
>>> + power-domain-names = "psci";
>>> };
>>>
>>> cpu-map {
>>> @@ -176,6 +192,68 @@ core3 {
>>> };
>>> };
>>> };
>>> +
>>> + idle-states {
>>> + entry-method = "psci";
>>> +
>>> + LITTLE_CPU_SLEEP_0: cpu-sleep-0-0 {
>>> + compatible = "arm,idle-state";
>>> + idle-state-name = "silver-rail-power-collapse";
>>> + arm,psci-suspend-param = <0x40000003>;
>>> + entry-latency-us = <290>;
>>> + exit-latency-us = <376>;
>>> + min-residency-us = <1182>;
>>> + local-timer-stop;
>>> + };
>>> +
>>> + BIG_CPU_SLEEP_0: cpu-sleep-1-0 {
>>> + compatible = "arm,idle-state";
>>> + idle-state-name = "gold-rail-power-collapse";
>>> + arm,psci-suspend-param = <0x40000003>;
>>> + entry-latency-us = <297>;
>>> + exit-latency-us = <324>;
>>> + min-residency-us = <1110>;
>>> + local-timer-stop;
>>> + };
>>> + };
>>> +
>>> + domain-idle-states {
>>> + CLUSTER_0_SLEEP_0: cluster-sleep-0-0 {
>>> + /* GDHS */
>>> + compatible = "domain-idle-state";
>>> + arm,psci-suspend-param = <0x40000022>;
>> This 0x22 ending seems very sus.
>>
>> The last nibble represents the core-level power state and the
>> penultimate one represents the same at cluster level. A value
>> of 2 in that cluster nibble is actually undefined by the PSCI spec,
>> whereas the value of 4 (as you have in all of the other idle
>> states, including D3G for the perf cluster) corresponds to
>> "Retention", so unless there's a very weird nuance in the
>> TZ for this SoC, it should probably end in 0x42.
>>
>> Otherwise I think this LGTM now!
>
> I am also learning by experiment about the exact values to use here,
> as the only ready reckoner of how these values are calculated, seems
> to be available via [1].
>
> Also it seems the downstream code uses the following approach to
> calculate the LPM state suspend-param, which for example for
> CLUSTER_0_SLEEP_1 states turns out to be:
>
> state_id = get_cluster_id(cpu->parent, &affinity_level, from_idle); = 0x40
> power_state = (is-reset << 30) = 0x40000000
> affinity_level = (affinity level & 0x3) << 24 = 0x1000000
> state_id += power_state + affinity_level + psci_id;
>
> = 0x40000000 + 0x1000000 + 0x40 + 0x4 = 0x41000044
>
> For the D3G cases as well, I just used the 'qcom,psci-mode = <2>'
> value as provided in downstream code (see [2]), for the overall
> calculations.
>
> Also, the only usage of D3G state I could find upstream (in qcom dtsi
> files0 is for 'msm8916' (see [3]), which also uses the value with
> ending 0x2 -> 'arm,psci-suspend-param = <0x41000032>'
D3G has min-child-idx = 1, so the end PSCI param should be 0x41000023
D3 is 0x41000043
>
> [1]. https://patchwork.kernel.org/project/linux-arm-msm/patch/20201221002907.2870059-3-danny@kdrag0n.dev/#23857409
> [2]. https://android.googlesource.com/kernel/msm-extra/devicetree/+/refs/tags/android-11.0.0_r0.56/qcom/bengal-pm.dtsi#127
> [3]. https://github.com/torvalds/linux/blob/master/arch/arm64/boot/dts/qcom/msm8916.dtsi#L209
>
> Thanks,
> Bhupesh
>
>>> + entry-latency-us = <360>;
>>> + exit-latency-us = <421>;
>>> + min-residency-us = <782>;
>>> + };
>>> +
>>> + CLUSTER_0_SLEEP_1: cluster-sleep-0-1 {
>>> + /* Power Collapse */
>>> + compatible = "domain-idle-state";
>>> + arm,psci-suspend-param = <0x41000044>;
>>> + entry-latency-us = <800>;
>>> + exit-latency-us = <2118>;
>>> + min-residency-us = <7376>;
>>> + };
>>> +
>>> + CLUSTER_1_SLEEP_0: cluster-sleep-1-0 {
>>> + /* GDHS */
>>> + compatible = "domain-idle-state";
>>> + arm,psci-suspend-param = <0x40000042>;
>>> + entry-latency-us = <314>;
>>> + exit-latency-us = <345>;
>>> + min-residency-us = <660>;
>>> + };
>>> +
>>> + CLUSTER_1_SLEEP_1: cluster-sleep-1-1 {
>>> + /* Power Collapse */
>>> + compatible = "domain-idle-state";
>>> + arm,psci-suspend-param = <0x41000044>;
>>> + entry-latency-us = <640>;
>>> + exit-latency-us = <1654>;
>>> + min-residency-us = <8094>;
>>> + };
>>> + };
>>> };
>>>
>>> firmware {
>>> @@ -199,6 +277,64 @@ pmu {
>>> psci {
>>> compatible = "arm,psci-1.0";
>>> method = "smc";
>>> +
>>> + CPU_PD0: power-domain-cpu0 {
>>> + #power-domain-cells = <0>;
>>> + power-domains = <&CLUSTER_0_PD>;
>>> + domain-idle-states = <&LITTLE_CPU_SLEEP_0>;
>>> + };
>>> +
>>> + CPU_PD1: power-domain-cpu1 {
>>> + #power-domain-cells = <0>;
>>> + power-domains = <&CLUSTER_0_PD>;
>>> + domain-idle-states = <&LITTLE_CPU_SLEEP_0>;
>>> + };
>>> +
>>> + CPU_PD2: power-domain-cpu2 {
>>> + #power-domain-cells = <0>;
>>> + power-domains = <&CLUSTER_0_PD>;
>>> + domain-idle-states = <&LITTLE_CPU_SLEEP_0>;
>>> + };
>>> +
>>> + CPU_PD3: power-domain-cpu3 {
>>> + #power-domain-cells = <0>;
>>> + power-domains = <&CLUSTER_0_PD>;
>>> + domain-idle-states = <&LITTLE_CPU_SLEEP_0>;
>>> + };
>>> +
>>> + CPU_PD4: power-domain-cpu4 {
>>> + #power-domain-cells = <0>;
>>> + power-domains = <&CLUSTER_1_PD>;
>>> + domain-idle-states = <&BIG_CPU_SLEEP_0>;
>>> + };
>>> +
>>> + CPU_PD5: power-domain-cpu5 {
>>> + #power-domain-cells = <0>;
>>> + power-domains = <&CLUSTER_1_PD>;
>>> + domain-idle-states = <&BIG_CPU_SLEEP_0>;
>>> + };
>>> +
>>> + CPU_PD6: power-domain-cpu6 {
>>> + #power-domain-cells = <0>;
>>> + power-domains = <&CLUSTER_1_PD>;
>>> + domain-idle-states = <&BIG_CPU_SLEEP_0>;
>>> + };
>>> +
>>> + CPU_PD7: power-domain-cpu7 {
>>> + #power-domain-cells = <0>;
>>> + power-domains = <&CLUSTER_1_PD>;
>>> + domain-idle-states = <&BIG_CPU_SLEEP_0>;
>>> + };
>>> +
>>> + CLUSTER_0_PD: power-domain-cpu-cluster0 {
>>> + #power-domain-cells = <0>;
>>> + domain-idle-states = <&CLUSTER_0_SLEEP_0>, <&CLUSTER_0_SLEEP_1>;
>>> + };
>>> +
>>> + CLUSTER_1_PD: power-domain-cpu-cluster1 {
>>> + #power-domain-cells = <0>;
>>> + domain-idle-states = <&CLUSTER_1_SLEEP_0>, <&CLUSTER_1_SLEEP_1>;
>>> + };
>>> };
>>>
>>> reserved_memory: reserved-memory {
--
With best wishes
Dmitry
Powered by blists - more mailing lists