linux-kernel - Re: [PATCH 0/3] Allow specifying an S2RAM sleep on pre-SYSTEM

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: <765bb1c8-31de-4aec-b8ef-f141a3e25c56@oss.qualcomm.com>
Date: Fri, 20 Dec 2024 13:42:04 +0100
From: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
To: Sudeep Holla <sudeep.holla@....com>,
        Konrad Dybcio <konrad.dybcio@....qualcomm.com>
Cc: Elliot Berman <quic_eberman@...cinc.com>,
        Konrad Dybcio <konradybcio@...nel.org>, Rob Herring <robh@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Conor Dooley
 <conor+dt@...nel.org>,
        Lorenzo Pieralisi <lpieralisi@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Marijn Suijten <marijn.suijten@...ainline.org>,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org,
        Bjorn Andersson <bjorn.andersson@....qualcomm.com>
Subject: Re: [PATCH 0/3] Allow specifying an S2RAM sleep on pre-SYSTEM_SUSPEND
 PSCI impls

On 20.12.2024 12:39 PM, Sudeep Holla wrote:
> On Thu, Dec 19, 2024 at 08:26:51PM +0100, Konrad Dybcio wrote:
>> On 14.11.2024 2:10 AM, Elliot Berman wrote:
>>
>>> I'm not sure why you'd like to support s2ram. Is it *only* that you'd
>>> like to be able to set pm_set_supend/resume_via_firmware()? I hope this
>>> doesn't sound silly: what if you register a platform_s2idle_ops for the
>>> relevant SoCs which calls pm_set_suspend/resume_via_firwmare()?
>>
>> S2RAM is what you get after entering a certain state, but currently
>> it's presented as just another (s2idle) idle state.
>>
> 
> Just to be clear, I assume you mean CPU_SUSPEND idle state. There is
> no special or different s2idle idle states IIUC.

Yeah, right.

>> That means some hardware that may need to be reinitialized, isn't as
>> Linux has no clue it might have lost power.
>>
> 
> Interesting, so this means firmware doesn't automatically save and restore
> states yet exposes it as CPU_SUSPEND idle state.

Reading the spec, I'm pretty sure PSCI calls should only mess with the
power state of the cores, core-adjacent peripherals and GIC.

Reading section 5.20.1 (SYSTEM_SUSPEND / Intended use) I think it says
mostly what I'm trying to convey:


"In a typical implementation, the semantics are equivalent to a
CPU_SUSPEND to the deepest low-power state. However, it is possible that
an implementation might reserve a deeper state for SYSTEM_SUSPEND than
those used with CPU_SUSPEND."

- this is the situation on QC platforms, with the case of not reserving a
deeper state for SYSTEM_SUSPEND

>> One of such cases is the PCIe block, with storage drivers specifically
>> looking for pm_suspend_via_firmware, but that's unfortunately not the
>> whole list.
>>
> 
> Well I can now imagine and I understand what's wrong here. An idle state
> is exposed to OS with an expectation that OS saves and restores certain
> state. Unless you tie it some other power domains that theses devices
> share, it is hard for OS to know the state is being lost and it needs
> to save and restore them. It is simple wrong to assume that OS needs
> to take care of them even though the power domain hierarchy doesn't
> represent this dependency to enter such a state. cpuidle-psci-domain.c
> takes care of this IIUC. Ulf can provide details if you are interested.

The spec disagrees:

"Note that entering the system into S2 or S3 carries with it several
preconditions. For example, all devices in the system must be in a state
that is compatible with entry into the system state"

- this also happens to be relevant here, given PSCI is not supposed to
power-govern the entire SoC, but only the CPU block. We have specialty
hardware that does power management for non-CPU IPs, but to request
a system power rail disablement, it must be done in conjunction with the
CPU requesting such CPU_SUSPEND state. And only after the required hardware
is de-initialized.

Konrad