linux-kernel - Re: [PATCH v3 2/3] cpuidle: psci: Move enabling OSI mode after power domains creation

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] [day] [month] [year] [list]

Date:   Thu, 20 Apr 2023 12:11:22 +0530
From:   "Maulik Shah (mkshah)" <quic_mkshah@...cinc.com>
To:     Ulf Hansson <ulf.hansson@...aro.org>
CC:     <andersson@...nel.org>, <dianders@...omium.org>,
        <swboyd@...omium.org>, <wingers@...gle.com>,
        <linux-arm-msm@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <linux-pm@...r.kernel.org>, <sudeep.holla@....com>,
        <jwerner@...omium.org>, <quic_lsrao@...cinc.com>,
        <quic_rjendra@...cinc.com>
Subject: Re: [PATCH v3 2/3] cpuidle: psci: Move enabling OSI mode after power
 domains creation

Hi Ulf,

On 4/18/2023 1:42 PM, Ulf Hansson wrote:
> O
>
>          /* Bail out if not using the hierarchical CPU topology. */
>          if (!pd_count)
> -               goto no_pd;
> +               goto remove_pd;
> We should return 0 here instead, right?
right. will fix in next revision.
>>          /* Link genpd masters/subdomains to model the CPU topology. */
>>          ret = dt_idle_pd_init_topology(np);
>>          if (ret)
>> -               goto remove_pd;
>> +               goto remove_pd_topology;
> This looks wrong to me. Shouldn't we continue to goto the "remove_pd"
> label for this error path?
We should need to remove already added subdomains via 
of_genpd_add_subdomain() if one of them fails.
So this look ok to me.
>
>> +
>> +       /* let's try to enable OSI. */
>> +       ret = psci_set_osi_mode(use_osi);
>> +       if (ret)
>> +               goto remove_pd_topology;
>>
>>          pr_info("Initialized CPU PM domain topology using %s mode\n",
>>                  use_osi ? "OSI" : "PC");
>>          return 0;
>>
>> -put_node:
>> -       of_node_put(node);
>> +remove_pd_topology:
>> +       dt_idle_pd_remove_topology(np);
>>   remove_pd:
>>          psci_pd_remove();
>> +put_node:
>> +       of_node_put(node);
> This of_node_put() should only be called if we break the
> "for_each_child_of_node" loop above because of an error, I think.
> Perhaps it's cleaner to just move this within the loop?
yes will move inside loop.

Thanks,
Maulik