linux-kernel - Re: [PATCH v4 1/6] firmware/psci: Add definitions for PSCI v1.3 specification

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: <d23d2cdcab3a2fca5cb36023b42ad9d4a0c7ff22.camel@infradead.org>
Date: Thu, 26 Sep 2024 17:30:52 +0100
From: David Woodhouse <dwmw2@...radead.org>
To: Miguel Luis <miguel.luis@...cle.com>, Souvik Chakravarty
	 <Souvik.Chakravarty@....com>
Cc: Paolo Bonzini <pbonzini@...hat.com>, Jonathan Corbet <corbet@....net>, 
 Marc Zyngier <maz@...nel.org>, Oliver Upton <oliver.upton@...ux.dev>, James
 Morse <james.morse@....com>,  Suzuki K Poulose <suzuki.poulose@....com>,
 Zenghui Yu <yuzenghui@...wei.com>, Catalin Marinas
 <catalin.marinas@....com>, Will Deacon <will@...nel.org>, Mark Rutland
 <mark.rutland@....com>, Lorenzo Pieralisi <lpieralisi@...nel.org>, "Rafael
 J. Wysocki" <rafael@...nel.org>, Pavel Machek <pavel@....cz>, Len Brown
 <len.brown@...el.com>, Shuah Khan <shuah@...nel.org>, "kvm@...r.kernel.org"
 <kvm@...r.kernel.org>,  "linux-doc@...r.kernel.org"
 <linux-doc@...r.kernel.org>, "linux-kernel@...r.kernel.org"
 <linux-kernel@...r.kernel.org>, "linux-arm-kernel@...ts.infradead.org"
 <linux-arm-kernel@...ts.infradead.org>, "kvmarm@...ts.linux.dev"
 <kvmarm@...ts.linux.dev>, "linux-pm@...r.kernel.org"
 <linux-pm@...r.kernel.org>,  "linux-kselftest@...r.kernel.org"
 <linux-kselftest@...r.kernel.org>
Subject: Re: [PATCH v4 1/6] firmware/psci: Add definitions for PSCI v1.3
 specification

On Thu, 2024-09-26 at 09:56 +0000, Miguel Luis wrote:
> 
> > +/* PSCI v1.3 hibernate type for SYSTEM_OFF2 */
> > +#define PSCI_1_3_HIBERNATE_TYPE_OFF 0
> 
> Should it be 1 as hibernate type?

It is in discovery, as BIT(PSCI_1_3_HIBERNATE_TYPE_OFF) == 1<<0 == 1.

But using a bitmask was only supposed to be for the discovery with
PSCI_FEATURES, as that has to advertise all the available hibernation
types.

The actual SYSTEM_OFF2 call was supposed to just take the numeric value
as an argument, since obviously *that* one isn't a bitmask. 

Except... I see that now the spec has finally been updated, it seems to
say that 0x1 is the value to pass to the SYSTEM_OFF2 call for
HIBERNATE_OFF, not 0x0. Which doesn't seem to make much sense, and I
don't recall it being what we discussed. Souvik, what happened there?

My understanding was that for each supported hibernation type #n, for
which HIBERERNATE_OFF is zero), the PSCI_FEATURES query would include
the bit (1<<n) to indicate that it is supported, and then the actual
SYSTEM_OFF2 call parameter would be (n) itself, precisely as
implemented here.

But the spec now seems to say that HIBERNATE_OFF is advertised as
(1<<0) in PSCI_FEATURES, but invoked with the value (1).

Is it too late to fix?

If it isn't just a thinko, what is the intent in the current spec?

If we have new hibernate types such that

 #define PSCI_1_3_HIBERNATE_TYPE_OFF 0
 #define PSCI_1_3_HIBERNATE_TYPE_FOO 1
 #define PSCI_1_3_HIBERNATE_TYPE_BAR 2

It seems obvious that the PSCI_FEATURES response will contain (1<<0),
(1<<1) and (1<<2) for them respectively, but what is supposed to be
passed to the actual SYSTEM_OFF2 call? Is it always just going to be
(PSCI_1_3_HIBERNATE_TYPE_xxx + 1)?

I think we should just fix §5.1.10 to report that 0x0 is HIBERNATE_OFF,
yes?

Download attachment "smime.p7s" of type "application/pkcs7-signature" (5965 bytes)