[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <34c52c88fd8fcbf68c453c1e94e4cd6294becff1.camel@linaro.org>
Date: Wed, 30 Jul 2025 16:23:02 +0100
From: André Draszik <andre.draszik@...aro.org>
To: Shivendra Pratap <shivendra.pratap@....qualcomm.com>, Bartosz
Golaszewski <bartosz.golaszewski@...aro.org>, Bjorn Andersson
<andersson@...nel.org>, Sebastian Reichel <sre@...nel.org>, Rob Herring
<robh@...nel.org>, Sudeep Holla <sudeep.holla@....com>, Souvik Chakravarty
<Souvik.Chakravarty@....com>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Andy Yan <andy.yan@...k-chips.com>,
Mark Rutland <mark.rutland@....com>, Lorenzo Pieralisi
<lpieralisi@...nel.org>, Arnd Bergmann <arnd@...db.de>, Konrad Dybcio
<konradybcio@...nel.org>, cros-qcom-dts-watchers@...omium.org, Vinod Koul
<vkoul@...nel.org>, Catalin Marinas <catalin.marinas@....com>, Will Deacon
<will@...nel.org>, Florian Fainelli <florian.fainelli@...adcom.com>
Cc: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>, Mukesh Ojha
<mukesh.ojha@....qualcomm.com>, Stephen Boyd <swboyd@...omium.org>,
linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org,
devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-arm-msm@...r.kernel.org, Elliot Berman <quic_eberman@...cinc.com>,
Srinivas Kandagatla
<srini@...nel.org>
Subject: Re: [PATCH v13 07/10] firmware: psci: Implement vendor-specific
resets as reboot-mode
On Wed, 2025-07-30 at 18:33 +0530, Shivendra Pratap wrote:
>
>
> On 7/30/2025 2:14 PM, André Draszik wrote:
> > On Sun, 2025-07-27 at 21:54 +0530, Shivendra Pratap wrote:
> > > SoC vendors have different types of resets which are controlled
> > > through various hardware registers. For instance, Qualcomm SoC
> > > may have a requirement that reboot with “bootloader” command
> > > should reboot the device to bootloader flashing mode and reboot
> > > with “edl” should reboot the device into Emergency flashing mode.
> > > Setting up such reboots on Qualcomm devices can be inconsistent
> > > across SoC platforms and may require setting different HW
> > > registers, where some of these registers may not be accessible to
> > > HLOS. These knobs evolve over product generations and require
> > > more drivers. PSCI spec defines, SYSTEM_RESET2, vendor-specific
> > > reset which can help align this requirement. Add support for PSCI
> > > SYSTEM_RESET2, vendor-specific resets and align the implementation
> > > to allow user-space initiated reboots to trigger these resets.
> > >
> > > Introduce a late_initcall to register PSCI vendor-specific resets
> > > as reboot modes. Implement a reboot-mode write function that sets
> > > reset_type and cookie values during the reboot notifier callback.
> > > Introduce a firmware-based call for SYSTEM_RESET2 vendor-specific
> > > reset in the psci_sys_reset path, using reset_type and cookie if
> > > supported by secure firmware.
> > >
> > > By using the above implementation, userspace will be able to issue
> > > such resets using the reboot() system call with the "*arg"
> > > parameter as a string based command. The commands can be defined
> > > in PSCI device tree node as “reset-types” and are based on the
> > > reboot-mode based commands.
> > >
> > > Signed-off-by: Shivendra Pratap <shivendra.pratap@....qualcomm.com>
> > > ---
> > > drivers/firmware/psci/Kconfig | 2 ++
> > > drivers/firmware/psci/psci.c | 57 ++++++++++++++++++++++++++++++++++++++++++-
> > > 2 files changed, 58 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/firmware/psci/Kconfig b/drivers/firmware/psci/Kconfig
> > > index 97944168b5e66aea1e38a7eb2d4ced8348fce64b..93ff7b071a0c364a376699733e6bc5654d56a17f 100644
> > > --- a/drivers/firmware/psci/Kconfig
> > > +++ b/drivers/firmware/psci/Kconfig
> > > @@ -1,6 +1,8 @@
> > > # SPDX-License-Identifier: GPL-2.0-only
> > > config ARM_PSCI_FW
> > > bool
> > > + select POWER_RESET
> > > + select REBOOT_MODE
> > >
> > > config ARM_PSCI_CHECKER
> > > bool "ARM PSCI checker"
> > > diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
> > > index 38ca190d4a22d6e7e0f06420e8478a2b0ec2fe6f..e14bcdbec1750db8aa9297c8bcdb242f58cc420e 100644
> > > --- a/drivers/firmware/psci/psci.c
> > > +++ b/drivers/firmware/psci/psci.c
> > > @@ -17,6 +17,7 @@
> > > #include <linux/printk.h>
> > > #include <linux/psci.h>
> > > #include <linux/reboot.h>
> > > +#include <linux/reboot-mode.h>
> > > #include <linux/slab.h>
> > > #include <linux/suspend.h>
> > >
> > > @@ -51,6 +52,14 @@ static int resident_cpu = -1;
> > > struct psci_operations psci_ops;
> > > static enum arm_smccc_conduit psci_conduit = SMCCC_CONDUIT_NONE;
> > >
> > > +struct psci_vendor_sysreset2 {
> > > + u32 reset_type;
> > > + u32 cookie;
> > > + bool valid;
> > > +};
> > > +
> > > +static struct psci_vendor_sysreset2 vendor_reset;
> > > +
> > > bool psci_tos_resident_on(int cpu)
> > > {
> > > return cpu == resident_cpu;
> > > @@ -309,7 +318,10 @@ static int get_set_conduit_method(const struct device_node *np)
> > > static int psci_sys_reset(struct notifier_block *nb, unsigned long action,
> > > void *data)
> > > {
> > > - if ((reboot_mode == REBOOT_WARM || reboot_mode == REBOOT_SOFT) &&
> > > + if (vendor_reset.valid && psci_system_reset2_supported) {
> > > + invoke_psci_fn(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2), vendor_reset.reset_type,
> > > + vendor_reset.cookie, 0);
> > > + } else if ((reboot_mode == REBOOT_WARM || reboot_mode == REBOOT_SOFT) &&
> > > psci_system_reset2_supported) {
> > > /*
> > > * reset_type[31] = 0 (architectural)
> >
> > I don't know the PSCI spec, but it looks like with this code it's not
> > possible to set a reboot mode (in DT) and at the same time instruct
> > the firmware whether a warm or a cold reboot was requested.
>
> The code added in this patch is kind of dead, until vendor_reset.valid is set to true.
> It can be true, only when both below conditions are true.
> 1. A SoC DT defines a psci->reboot-mode command say - "bootloader".
> 2. reboot sys call is issued using LINUX_REBOOT_CMD_RESTART2 and the arg* as "bootloader".
> reboot(LINUX_REBOOT_MAGIC1, LINUX_REBOOT_MAGIC2, LINUX_REBOOT_CMD_RESTART2, "bootloader");
>
> With that in place, warm and cold reboot just work same as before until above conditions are true.
> There is no effect on regular reboots or the reboots with a "command" which is not defined in
> psci->reboot-mode DT.
>
> Now lets take a case below, where a SoC vendor wants a combination of psci->reboo-mode command and
> warm/cold to work in combination. For ex. a requirement below:
> - reboot command say - "bootlaoder" should do a cold reboot along with some extra HW reg writes.
> - reboot command say - "edl" should do a warm reboot along with some extra HW reg writes.
>
> 1. For this, both commands will be defined in the psci->reboot-mode DT Node with the arguments that
> are defined and supported by the firmware.
> 2. Further, such requirement will now be taken care by the underlying firmware that supports
> PSCI vendor-specific reset. When we call into firmware with vendor specific reset arguments,
> firmware will take care of the defined HW writes and warm/cold reset as per the mapping of the
> defined arguments. Firmware and the Linux kernel here are in agreement for executing the
> vendor-specific resets.
So that means
echo warm > /sys/kernel/reboot/mode
reboot bootloader
and
echo cold > /sys/kernel/reboot/mode
reboot bootloader
can not be distinguished.
The firmware can not know whether or not cold or warm reboot was
requested in this case by the user.
More importantly, if e.g. an OOPS / panic happens after the reboot
notifier has run (and set vendor_reset.valid because a reboot mode
was requested), a panic handler changing reboot_mode to warm to
retain RAM contents will have no effect, because the the original
code above making those distinctions can not be reached anymore.
Above scenario with OOPS / panic after reboot notifier could e.g.
happen as part of device_shutdown() - see kernel_shutdown_prepare()
> >
> > Doing warm reboot is useful if e.g. RAM contents needs to be retained
> > for crash recovery handling, or other reasons, while in normal cases
> > doing a more secure cold reboot.
> >
> > On the other hand, of course it's useful to be able to specify the
> > reboot target for normal reboots.
> >
> > Is this a problem with the PSCI spec or with this specific change
> > geared at the Qcom implementation?
>
> SoC vendor should define a vendor-specific reset in psci DT only when they support them in their
> firmware.
>
> Do we still think we are breaking anything in the spec or in the warm or the cold
> reset path? If so can we discuss such use-cases?
I don't know the spec, but see examples above.
Cheers,
Andre'
Powered by blists - more mailing lists