linux-kernel - Re: [PATCH v9 05/17] ARM: tegra: Propagate error from tegra_idle_lp2

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]

Date:   Fri, 21 Feb 2020 21:42:30 +0300
From:   Dmitry Osipenko <digetx@...il.com>
To:     Daniel Lezcano <daniel.lezcano@...aro.org>
Cc:     Thierry Reding <thierry.reding@...il.com>,
        Jonathan Hunter <jonathanh@...dia.com>,
        Peter De Schrijver <pdeschrijver@...dia.com>,
        "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Michał Mirosław <mirq-linux@...e.qmqm.pl>,
        Jasper Korten <jja2000@...il.com>,
        David Heidelberg <david@...t.cz>,
        Peter Geis <pgwipeout@...il.com>, linux-pm@...r.kernel.org,
        linux-tegra@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v9 05/17] ARM: tegra: Propagate error from
 tegra_idle_lp2_last()

21.02.2020 20:40, Daniel Lezcano пишет:
> On Fri, Feb 21, 2020 at 08:21:41PM +0300, Dmitry Osipenko wrote:
>> 21.02.2020 18:16, Daniel Lezcano пишет:
>>> On Thu, Feb 13, 2020 at 02:51:22AM +0300, Dmitry Osipenko wrote:
>>>> Technically cpu_suspend() may fail and it's never good to lose information
>>>> about failure. For example things like cpuidle core could correctly sample
>>>> idling time in the case of failure.
>>>>
>>>> Acked-by: Peter De Schrijver <pdeschrijver@...dia.com>
>>>> Tested-by: Peter Geis <pgwipeout@...il.com>
>>>> Tested-by: Jasper Korten <jja2000@...il.com>
>>>> Tested-by: David Heidelberg <david@...t.cz>
>>>> Signed-off-by: Dmitry Osipenko <digetx@...il.com>
>>>> ---
>>>
>>> [ ... ]
>>>
>>>>  	cpu_cluster_pm_enter();
>>>>  	suspend_cpu_complex();
>>>>  
>>>> -	cpu_suspend(PHYS_OFFSET - PAGE_OFFSET, &tegra_sleep_cpu);
>>>> +	err = cpu_suspend(PHYS_OFFSET - PAGE_OFFSET, &tegra_sleep_cpu);
>>>>  
>>>>  	/*
>>>>  	 * Resume L2 cache if it wasn't re-enabled early during resume,
>>>> @@ -208,6 +210,8 @@ void tegra_idle_lp2_last(void)
>>>>  
>>>>  	restore_cpu_complex();
>>>
>>> If the cpu_suspend fails, does restore_cpu_complex() need to be called ?
>>
>> Yes, because suspend_cpu_complex() didn't fail. I don't see any reason
>> why restore_cpu_complex() shouldn't be called, please clarify yours thought.
> 
> If the suspend fails, the power down does not happen, thus the logic is not
> lost and then it not necessary to restore something which has not been lost.
> 
> I don't know the hardware details, so that may be partially correct.

At quick glance, the restore_cpu_complex() is only used for restoring
the GIC's state on Tegra.

I don't think it's really worth the effort to handle
restore_cpu_complex() specially in a case of the error condition because
the chance that the error will ever happen is very small and restoring
the cluster's state won't cause any trouble in that case.

Let's keep this patch as-is for simplicity :)