linux-kernel - Re: [PATCH 3/3] firmware/psci: Allow specifying an S2RAM state through CPU

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]

Message-ID: <Z1LQ0-3AVkVHgPaY@bogus>
Date: Fri, 6 Dec 2024 10:24:19 +0000
From: Sudeep Holla <sudeep.holla@....com>
To: Lorenzo Pieralisi <lpieralisi@...nel.org>
Cc: Konrad Dybcio <konradybcio@...nel.org>, Rob Herring <robh@...nel.org>,
	Sudeep Holla <sudeep.holla@....com>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...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>
Subject: Re: [PATCH 3/3] firmware/psci: Allow specifying an S2RAM state
 through CPU_SUSPEND

On Wed, Nov 13, 2024 at 01:57:23PM +0100, Lorenzo Pieralisi wrote:
> On Mon, Oct 28, 2024 at 03:22:59PM +0100, Konrad Dybcio wrote:
> > From: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
> > 
> > Certain firmware implementations (such as the ones found on Qualcomm
> > SoCs between roughly 2015 and 2023) expose an S3-like S2RAM state
> > through the CPU_SUSPEND call.
> > 
> > This works exactly like SYSTEM_SUSPEND. The PSCI spec describes that
> > call as optional (and only introduced in PSCIv1.0), so not all
> > platforms expose it.
> > 
> > Marking a DT-described "domain-idle-state" as such isn't currently
> > well accounted for in the PSCI idle topology infrastructure: the
> > cpuidle and genpd framework are deeply intertwined, and trying to
> > separate them would cause more havoc than good.
> 
> I don't understand what you mean here please elaborate.
> 
> The part of the story I understand is that you have a system (well,
> firmware for an extended set of systems) that does not implement
> SYSTEM_SUSPEND but can reach a S2R like system state through the
> CPU_SUSPEND call. Firmware works in OS-initiated mode, idle-states
> should allow you to define idle states that allow the system to
> enter the S2R state through CPUidle.
> 
> Please explain to me what's missing.
> 
> > Instead, allow the specifying of a single CPU_SUSPEND sleep param
> > under the /psci node that shall be treated exactly like SYSTEM_SUSPEND
> > from Linux's POV. As a bonus, this way we also don't have to fight
> > with the genpd idle governor to avoid taking the S3-like state into
> > consideration.
> 
> That's not acceptable. I want to understand what's preventing this
> system to enter that state through suspend2idle and the mainline code.
> 
> > Signed-off-by: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
> > ---
> >  drivers/firmware/psci/psci.c | 36 +++++++++++++++++++++++++++++++-----
> >  1 file changed, 31 insertions(+), 5 deletions(-)
> 
> NACK
> 

+1, will wait for the response here before adding any more questions that
may lead to more confusion or discussion churn.

-- 
Regards,
Sudeep