Re: [PATCH v23 04/13] firmware: psci: Introduce command-based resets

[Shivendra Pratap]

On 22-07-2026 14:54, Pavan Kondeti wrote:
> On Tue, Jul 14, 2026 at 10:46:32PM +0530, Shivendra Pratap wrote:
> > PSCI currently supports only two resets - SYSTEM_RESET and SYSTEM_RESET2
> > ARCH WARM reset. The reset patch is selected based on the Linux
> > reboot_mode variable. The PSCI specification now includes SYSTEM_RESET2
> > for vendor-specific resets but there's no mechanism to issue these
> > through psci_sys_reset().
> >
> > Add a command-based reset mechanism that allows external drivers to set
> > the PSCI reset command by passing a reset_type and a cookie.
> >
> > Add support for the following reset commands:
> >    - SYSTEM_RESET2 vendor-specific resets.
> >    - SYSTEM_RESET2 ARCH WARM reset and SYSTEM_RESET (reset_type = 0 and
> >      cookie maps to psci_standard_resets).
> > Unsupported commands fall back to the regular PSCI reset path.
> >
> > Default to the existing reboot_mode-based reset flow unless a reset
> > command is configured. If a kernel panic occurs after a reset command is
> > set but before the final PSCI reset is issued, ignore the reset command
> > and follow the normal reboot_mode-based reset path.
> >
> > Signed-off-by: Shivendra Pratap <shivendra.pratap@xxxxxxxxxxxxxxxx>
> > ---
> >   drivers/firmware/psci/psci.c | 106 ++++++++++++++++++++++++++++++++++++++++++-
> >   include/linux/psci.h         |  36 +++++++++++++++
> >   2 files changed, 140 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
> > index e73bae6cb23a..881819d09911 100644
> > --- a/drivers/firmware/psci/psci.c
> > +++ b/drivers/firmware/psci/psci.c
> > @@ -12,7 +12,9 @@
> >   #include <linux/debugfs.h>
> >   #include <linux/errno.h>
> >   #include <linux/linkage.h>
> > +#include <linux/mutex.h>
> >   #include <linux/of.h>
> > +#include <linux/panic.h>
> >   #include <linux/pm.h>
> >   #include <linux/printk.h>
> >   #include <linux/psci.h>
> > @@ -51,6 +53,14 @@ static int resident_cpu = -1;
> >   struct psci_operations psci_ops;
> >   static enum arm_smccc_conduit psci_conduit = SMCCC_CONDUIT_NONE;
> >   
> > +struct psci_system_reset_cmd {
> > +	u32 reset_type;
> > +	u64 cookie;
> > +};
> > +
> > +static struct psci_system_reset_cmd reset_cmd;
> > +static DEFINE_MUTEX(reset_cmd_mutex);
> > +
> >   bool psci_tos_resident_on(int cpu)
> >   {
> >   	return cpu == resident_cpu;
> > @@ -80,6 +90,62 @@ static u32 psci_cpu_suspend_feature;
> >   static bool psci_system_reset2_supported;
> >   static bool psci_system_off2_hibernate_supported;
> >   
> > +static u32 psci_get_sys_reset_fn(void)
> > +{
> > +	switch (reset_cmd.cookie) {
> > +	case PSCI_SYSTEM_RESET2_ARCH_WARM_RESET:
> > +		if (psci_system_reset2_supported)
> > +			return PSCI_FN_NATIVE(1_1, SYSTEM_RESET2);
> > +		return 0;
> > +	case PSCI_SYSTEM_RESET_COLD_RESET:
> > +		return PSCI_0_2_FN_SYSTEM_RESET;
> > +	default:
> > +		return 0;
> > +	}
> > +}
> > +
> > +/** psci_set_reset_cmd() - Configure PSCI reset command
> > + * @reset_type: SYSTEM_RESET2 vendor-specific reset_type as defined by
> > + *		firmware, or 0 for standard resets
> > + * @cookie: SYSTEM_RESET2 vendor-specific cookie as defined by firmware or one
> > + *		of enum psci_standard_resets when @reset_type is set to 0
> > + *
> > + * Supported commands:
> > + * - PSCI SYSTEM_RESET2 vendor-specific reset:
> > + *   - @reset_type and @cookie must follow platform-specific SYSTEM_RESET2
> > + *     vendor-specific resets.
> > + * - Standard reset selector:
> > + *   - @reset_type must be 0.
> > + *   - @cookie must be one of enum psci_standard_resets.
> > + *
> > + * This is an in-kernel helper intended for built-in reboot flow callers.
> > + * reset command can be set only one time per boot cycle.
>
> This comment may need re-phrasing. Please check.

sure. thanks.

> > + *
> > + * Return: 0 on success, -EINVAL if both inputs are zero, -EBUSY if reset
> > + * command is already set.
> > + */
> > +int psci_set_reset_cmd(u32 reset_type, u64 cookie)
> > +{
> > +	if (!reset_type && !cookie)
> > +		return -EINVAL;
> > +
> > +	scoped_guard(mutex, &reset_cmd_mutex) {
> > +		if (reset_cmd.reset_type || reset_cmd.cookie)
> > +			return -EBUSY;
> > +
> > +		reset_cmd.reset_type = reset_type;
> > +		reset_cmd.cookie = cookie;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +bool psci_has_system_reset2_support(void)
> > +{
> > +	return psci_system_reset2_supported;
> > +}
> > +EXPORT_SYMBOL_GPL(psci_has_system_reset2_support);
> > +
>
> Do we want to add namespace based restriction on who gets to import
> EXPORTs added in this patch?

you mean like
EXPORT_SYMBOL_NS_GPL(psci_has_system_reset2_support, PSCI);?

> >   static inline bool psci_has_ext_power_state(void)
> >   {
> >   	return psci_cpu_suspend_feature &
> > @@ -306,8 +372,24 @@ static int get_set_conduit_method(const struct device_node *np)
> >   	return 0;
> >   }
> >   
> > -static int psci_sys_reset(struct notifier_block *nb, unsigned long action,
> > -			  void *data)
> > +static void psci_handle_reset_cmd(void)
> > +{
> > +	u32 psci_sys_reset_fn;
> > +
> > +	if ((reset_cmd.reset_type & PSCI_1_1_RESET_TYPE_VENDOR_START) &&
>
> What is `_START` significance in PSCI_1_1_RESET_TYPE_VENDOR_START macro?

Its the 31st bit that decides if its valid vendor reset.

> > +	    psci_system_reset2_supported) {
> > +		/* PSCI SYSTEM_RESET2 Vendor-specific reset */
> > +		invoke_psci_fn(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2),
> > +			       reset_cmd.reset_type, reset_cmd.cookie, 0);
> > +	} else {
> > +		/* Retrieve the psci reset function from reset_cmd */
> > +		psci_sys_reset_fn = psci_get_sys_reset_fn();
> > +		if (!reset_cmd.reset_type && psci_sys_reset_fn)
> > +			invoke_psci_fn(psci_sys_reset_fn, 0, 0, 0);
> > +	}
> > +}
> > +
> > +static void psci_handle_reboot_mode(void)
> >   {
> >   	if ((reboot_mode == REBOOT_WARM || reboot_mode == REBOOT_SOFT) &&
> >   	    psci_system_reset2_supported) {
> > @@ -320,6 +402,26 @@ static int psci_sys_reset(struct notifier_block *nb, unsigned long action,
> >   	} else {
> >   		invoke_psci_fn(PSCI_0_2_FN_SYSTEM_RESET, 0, 0, 0);
> >   	}
> > +}
> > +
> > +static int psci_sys_reset(struct notifier_block *nb, unsigned long action,
> > +			  void *data)
> > +{
> > +	/* The function psci_handle_reboot_mode follows reboot_mode based
> > +	 * reset flow and psci_handle_reset_cmd uses reset_cmd based reset flow.
> > +	 *
> > +	 * The reset_cmd is configured at the reboot_notifier phase. If a kernel
> > +	 * panic occurs between the reboot_notifier and this final reset, skip the
> > +	 * command-based reset and let reboot_mode drive the reset flow.
> > +	 *
> > +	 * The function psci_handle_reset_cmd invokes non-returning PSCI SYSTEM_RESET
> > +	 * calls to reset the device. If it returns, either the reset failed, or the
> > +	 * command was unsupported. Fallback to reboot_mode based reset flow.
> > +	 */
> > +	if ((reset_cmd.reset_type || reset_cmd.cookie) && !panic_in_progress())
> > +		psci_handle_reset_cmd();
> > +
> > +	psci_handle_reboot_mode();
> >   
> >   	return NOTIFY_DONE;
> >   }
> > diff --git a/include/linux/psci.h b/include/linux/psci.h
> > index 4ca0060a3fc4..39fe0edec651 100644
> > --- a/include/linux/psci.h
> > +++ b/include/linux/psci.h
> > @@ -8,6 +8,7 @@
> >   #define __LINUX_PSCI_H
> >   
> >   #include <linux/arm-smccc.h>
> > +#include <linux/errno.h>
> >   #include <linux/init.h>
> >   #include <linux/types.h>
> >   
> > @@ -21,6 +22,16 @@ bool psci_power_state_is_valid(u32 state);
> >   int psci_set_osi_mode(bool enable);
> >   bool psci_has_osi_support(void);
> >   
> > +/**
> > + * enum psci_standard_resets - Standard reset selectors for PSCI reset
> > + * @PSCI_SYSTEM_RESET_COLD_RESET: Standard SYSTEM_RESET command.
> > + * @PSCI_SYSTEM_RESET2_ARCH_WARM_RESET: SYSTEM_RESET2 architectural warm reset.
> > + */
> > +enum psci_standard_resets {
> > +	PSCI_SYSTEM_RESET_COLD_RESET = 1,
> > +	PSCI_SYSTEM_RESET2_ARCH_WARM_RESET,
> > +};
> > +
> >   struct psci_operations {
> >   	u32 (*get_version)(void);
> >   	int (*cpu_suspend)(u32 state, unsigned long entry_point);
> > @@ -45,8 +56,33 @@ struct psci_0_1_function_ids get_psci_0_1_function_ids(void);
> >   
> >   #if defined(CONFIG_ARM_PSCI_FW)
> >   int __init psci_dt_init(void);
> > +/**
> > + * psci_set_reset_cmd() - Configure PSCI reset command
> > + * @reset_type: SYSTEM_RESET2 vendor-specific reset_type as defined by
> > + *		firmware, or 0 for standard resets
> > + * @cookie: SYSTEM_RESET2 vendor-specific cookie as defined by firmware or one
> > + *		of enum psci_standard_resets when @reset_type is set to 0
> > + *
> > + * Supported commands:
> > + * - PSCI SYSTEM_RESET2 vendor-specific reset:
> > + *   - @reset_type and @cookie must follow platform-specific SYSTEM_RESET2
> > + *     vendor-reset encoding.
> > + * - Standard reset selector:
> > + *   - @reset_type must be 0.
> > + *   - @cookie must be one of enum psci_standard_resets.
> > + *
> > + * This is an in-kernel helper intended for built-in reboot flow callers.
> > + * reset command can be set only one time per boot cycle.
> > + *
> > + * Return: 0 on success, -EINVAL if both inputs are zero, -EBUSY if reset
> > + * command is already set.
> > + */
> > +int psci_set_reset_cmd(u32 reset_type, u64 cookie);
>
> The very same comment is present in the implementation of this function.
> Please fix it.

sure.

thanks,
Shivendra