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 for Android: free password hash cracker in your pocket

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

Message-ID: <ce1d2196-cf09-4d27-8b41-372948a59e89@gmail.com>
Date: Tue, 12 Nov 2024 19:32:36 +0100
From: Konrad Dybcio <konradybcio@...il.com>
To: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
 Konrad Dybcio <konradybcio@...nel.org>
Cc: 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>,
 Konrad Dybcio <konrad.dybcio@....qualcomm.com>,
 Sudeep Holla <sudeep.holla@....com>
Subject: Re: [PATCH 0/3] Allow specifying an S2RAM sleep on pre-SYSTEM_SUSPEND
 PSCI impls



On 11/12/24 19:01, Manivannan Sadhasivam wrote:
> On Mon, Oct 28, 2024 at 03:22:56PM +0100, Konrad Dybcio wrote:
>> Certain firmwares expose exactly what PSCI_SYSTEM_SUSPEND does through
>> CPU_SUSPEND instead. Inform Linux about that.
>> Please see the commit messages for a more detailed explanation.
>>
> 
> It is still not PSCI_SYSTEM_SUSPEND though...

It *literally* does the same thing on devices where it's exposed.

> 
>> This is effectively a more educated follow-up to [1].
>>
>> The ultimate goal is to stop making Linux think that certain states
>> only concern cores/clusters, and consequently setting
>> pm_set_suspend/resume_via_firmware(), so that client drivers (such as
>> NVMe, see related discussion over at [2]) can make informed decisions
>> about assuming the power state of the device they govern.
>>
>> If this series gets green light, I'll push a follow-up one that wires
>> up said sleep state on Qualcomm SoCs across the board.
>>
> 
> Sorry. I don't think PSCI is the right place for this. Qcom SoCs have a common
> firmware across all segments (mostly), 

This ^

> so there is no S2R involved and only S2Idle.

is not at all related to this ^, the "so" makes no sense.

(also you're wrong, this *is* S2RAM)

> If you use PSCI to implement suspend_via_firmware(), then all the SoCs
> making use of the PSCI implementation will have the same behavior. I don't think
> we would want that.

This is an issue with the NVMe framework that is totally unrelated to this
change, see below. Also, the code only sets that on targets where such state
exists and is described.

> For instance, if a Qcom SoC is used in an android tablet with the same firmware,
> then this would allow the NVMe device to be turned off during system suspend all
> the time when user presses the lock button. And this will cause NVMe device to
> wear out faster. The said approach will work fine for non-android usecases
> though.

The NVMe framework doesn't make a distinction between "phone screen off" and
"laptop lid closed & thrown in a bag" on *any* platform. The usecase you're
describing is not supported as of today since nobody *actually* has NVMe on a
phone that also happens to run upstream Linux.
I'm not going to solve imaginary problems.

Besides, userspace already has sysfs to tune device power state knobs. Which
Android uses very extensively on market devices.

Konrad