Message-Id: <a9d8d9aa-f63c-481e-b051-a3da0adb3c66@app.fastmail.com>
Date: Fri, 14 Mar 2025 17:31:44 +0100
From: "Arnd Bergmann" <arnd@...db.de>
To: "Lorenzo Pieralisi" <lpieralisi@...nel.org>,
 "Elliot Berman" <quic_eberman@...cinc.com>
Cc: "Bjorn Andersson" <andersson@...nel.org>,
 "Sebastian Reichel" <sre@...nel.org>, "Rob Herring" <robh@...nel.org>,
 "Conor Dooley" <conor+dt@...nel.org>, "Vinod Koul" <vkoul@...nel.org>,
 "Andy Yan" <andy.yan@...k-chips.com>,
 "Mark Rutland" <mark.rutland@....com>,
 "Bartosz Golaszewski" <bartosz.golaszewski@...aro.org>,
 "Olof Johansson" <olof@...om.net>,
 "Catalin Marinas" <catalin.marinas@....com>,
 "Will Deacon" <will@...nel.org>, cros-qcom-dts-watchers@...omium.org,
 "Krzysztof Kozlowski" <krzk+dt@...nel.org>,
 "Konrad Dybcio" <konradybcio@...nel.org>,
 "Srinivas Kandagatla" <srinivas.kandagatla@...aro.org>,
 "Satya Durga Srinivasu Prabhala" <quic_satyap@...cinc.com>,
 "Melody Olvera" <quic_molvera@...cinc.com>,
 "Shivendra Pratap" <quic_spratap@...cinc.com>,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org,
 "Florian Fainelli" <florian.fainelli@...adcom.com>,
 "Stephen Boyd" <swboyd@...omium.org>, linux-pm@...r.kernel.org,
 linux-arm-msm@...r.kernel.org, "Elliot Berman" <elliotb317@...il.com>,
 "Elliot Berman" <elliot.berman@....qualcomm.com>
Subject: Re: [PATCH v9 2/5] firmware: psci: Read and use vendor reset types

On Fri, Mar 14, 2025, at 12:19, Lorenzo Pieralisi wrote:
> On Mon, Mar 03, 2025 at 01:08:31PM -0800, Elliot Berman wrote:
>> From: Elliot Berman <elliot.berman@....qualcomm.com>
>> 
>> SoC vendors have different types of resets and are controlled through
>> various registers. For instance, Qualcomm chipsets can reboot to a
>> "download mode" that allows a RAM dump to be collected. Another example
>> is they also support writing a cookie that can be read by bootloader
>> during next boot. PSCI offers a mechanism, SYSTEM_RESET2, for these
>> vendor reset types to be implemented without requiring drivers for every
>> register/cookie.
>> 
>> Add support in PSCI to statically map reboot mode commands from
>> userspace to a vendor reset and cookie value using the device tree.
>
> I have managed to discuss a little bit this patchset over the last
> few days and I think we have defined a plan going forward.
>
> A point that was raised is:
>
> https://man7.org/linux/man-pages/man2/reboot.2.html
>
> LINUX_REBOOT_CMD_RESTART2 *arg command, what is it supposed to
> represent ?
>
> Is it the mode the system should reboot into OR it is the
> actual command to be issued (which is what this patchset
> implements) ?
>
> LINUX_REBOOT_CMD_RESTART "..a default restart..."
>
> It is unclear what "default" means. We wonder whether the
> reboot_mode variable was introduced to _define_ that "default".

I think the reboot_mode predates the 'cmd' argument: linux-2.1.30
introduced LINUX_REBOOT_CMD_RESTART2 and it already had
the warm/cold/bios/hard options for i386 reboot. I think the
argument went unused for a while after it got introduced though.

> So, in short, my aim is trying to decouple reboot_mode from the
> LINUX_REBOOT_CMD_RESTART2 *arg command.
>
> I believe that adding a sysfs interface to reboot-mode driver
> infrastructure would be useful, so that the commands would
> be exposed to userspace and userspace can set the *arg command
> specifically to issue a given reset/mode.
>
> I wonder why this is not already in place for eg syscon-reboot-mode
> resets, how does user space issue a command in those systems if the
> available commands aren't exposed to userspace ?
>
> Is there a kernel entity exposing those "modes" to userspace, somehow ?

Don't know one either.

>> A separate initcall is needed to parse the devicetree, instead of using
>> psci_dt_init because mm isn't sufficiently set up to allocate memory.
>> 
>> Reboot mode framework is close but doesn't quite fit with the
>> design and requirements for PSCI SYSTEM_RESET2. Some of these issues can
>> be solved but doesn't seem reasonable in sum:
>>  1. reboot mode registers against the reboot_notifier_list, which is too
>>     early to call SYSTEM_RESET2. PSCI would need to remember the reset
>>     type from the reboot-mode framework callback and use it
>>     psci_sys_reset.
>>  2. reboot mode assumes only one cookie/parameter is described in the
>>     device tree. SYSTEM_RESET2 uses 2: one for the type and one for
>>     cookie.
>
> This can be changed and I think it should, so that the reboot modes
> are exposed to user space and PSCI can use that.

Can we try to call them 'arguments' rather than 'modes' while discussing?
I think it's way too easy to confuse them otherwise.

>> +	psci_np = of_find_matching_node(NULL, psci_of_match);
>> +	if (!psci_np)
>> +		return 0;
>> +
>> +	np = of_find_node_by_name(psci_np, "reset-types");
>> +	if (!np)
>> +		return 0;
>
> Related to my initial question above. If LINUX_REBOOT_CMD_RESTART2 *arg command,
> is the actual reset to be issued, should we add a default mode "cold"
> and, if SYSTEM_RESET2 is supported, a "warm" reset mode too ?
>
> It all boils down to what *arg represents - adding "cold" and "warm"
> modes would remove the dependency on reboot_mode for resets issued
> through LINUX_REBOOT_CMD_RESTART2, the question is whether this
> is the correct thing to do.
>
> Comments very welcome.

It would make some sense to me to treat all psci reboot as "warm" and
not do anything here if reboot="cold" is set: those would have to
be backed by a hardware specific driver.

A related problem is how to determine when to use UEFI reboot: at the
moment arm64 tries the UEFI runtime interface before it even attempts
any of the notifiers, so PSCI reboot won't ever be used if UEFI is
present.

     Arnd

Powered by blists - more mailing lists