[PATCH v20 04/10] firmware: psci: Introduce command-based reset in psci_sys_reset

Shivendra Pratap posted 10 patches 1 month ago
[PATCH v20 04/10] firmware: psci: Introduce command-based reset in psci_sys_reset
Posted by Shivendra Pratap 1 month ago
PSCI currently supports only COLD reset and ARCH WARM reset 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 via a new psci_set_reset_cmd() function.

The psci command-based reset is disabled by default and the
psci_sys_reset follows its original flow until a psci_reset command is
set. In kernel panic path, psci_reset command is ignored.

Signed-off-by: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>
---
 drivers/firmware/psci/psci.c | 45 ++++++++++++++++++++++++++++++++++++++++++--
 include/linux/psci.h         |  2 ++
 2 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
index 38ca190d4a22d6e7e0f06420e8478a2b0ec2fe6f..ae6f7a0aead913d740070080d4b2a3da15b29485 100644
--- a/drivers/firmware/psci/psci.c
+++ b/drivers/firmware/psci/psci.c
@@ -51,6 +51,15 @@ static int resident_cpu = -1;
 struct psci_operations psci_ops;
 static enum arm_smccc_conduit psci_conduit = SMCCC_CONDUIT_NONE;
 
+struct psci_sys_reset_params {
+	u32 system_reset;
+	u32 reset_type;
+	u32 cookie;
+	bool cmd;
+};
+
+static struct psci_sys_reset_params psci_reset;
+
 bool psci_tos_resident_on(int cpu)
 {
 	return cpu == resident_cpu;
@@ -80,6 +89,28 @@ static u32 psci_cpu_suspend_feature;
 static bool psci_system_reset2_supported;
 static bool psci_system_off2_hibernate_supported;
 
+/**
+ * psci_set_reset_cmd - Sets the psci_reset_cmd for command-based
+ * reset which will be used in psci_sys_reset call.
+ *
+ * @cmd_sys_rst2: Set to true for SYSTEM_RESET2 based resets.
+ * @cmd_reset_type: Set the reset_type argument for psci_sys_reset.
+ * @cmd_cookie: Set the cookie argument for psci_sys_reset.
+ */
+void psci_set_reset_cmd(bool cmd_sys_rst2, u32 cmd_reset_type, u32 cmd_cookie)
+{
+	if (cmd_sys_rst2 && psci_system_reset2_supported) {
+		psci_reset.system_reset = PSCI_FN_NATIVE(1_1, SYSTEM_RESET2);
+		psci_reset.reset_type = cmd_reset_type;
+		psci_reset.cookie = cmd_cookie;
+	} else {
+		psci_reset.system_reset = PSCI_0_2_FN_SYSTEM_RESET;
+		psci_reset.reset_type = 0;
+		psci_reset.cookie = 0;
+	}
+	psci_reset.cmd = true;
+}
+
 static inline bool psci_has_ext_power_state(void)
 {
 	return psci_cpu_suspend_feature &
@@ -309,14 +340,24 @@ 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) &&
-	    psci_system_reset2_supported) {
+	if (((reboot_mode == REBOOT_WARM || reboot_mode == REBOOT_SOFT) &&
+	     psci_system_reset2_supported) && (panic_in_progress() || !psci_reset.cmd)) {
 		/*
 		 * reset_type[31] = 0 (architectural)
 		 * reset_type[30:0] = 0 (SYSTEM_WARM_RESET)
 		 * cookie = 0 (ignored by the implementation)
 		 */
 		invoke_psci_fn(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2), 0, 0, 0);
+	} else if (!panic_in_progress() && psci_reset.cmd) {
+		/*
+		 * Commands are being set in psci_set_reset_cmd
+		 * This issues, SYSTEM_RESET2 arch warm reset or
+		 * SYSTEM_RESET2 vendor-specific reset or
+		 * a SYSTEM_RESET cold reset in accordance with
+		 * the reboot-mode command.
+		 */
+		invoke_psci_fn(psci_reset.system_reset, psci_reset.reset_type,
+			       psci_reset.cookie, 0);
 	} else {
 		invoke_psci_fn(PSCI_0_2_FN_SYSTEM_RESET, 0, 0, 0);
 	}
diff --git a/include/linux/psci.h b/include/linux/psci.h
index 4ca0060a3fc42ba1ca751c7862fb4ad8dda35a4c..d13ceca88eab8932894051e7c86e806c2ad8a73a 100644
--- a/include/linux/psci.h
+++ b/include/linux/psci.h
@@ -45,8 +45,10 @@ 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);
+void psci_set_reset_cmd(bool cmd_sys_rst2, u32 cmd_reset_type, u32 cmd_cookie);
 #else
 static inline int psci_dt_init(void) { return 0; }
+static inline void psci_set_reset_cmd(bool cmd_sys_rst2, u32 cmd_reset_type, u32 cmd_cookie) { }
 #endif
 
 #if defined(CONFIG_ARM_PSCI_FW) && defined(CONFIG_ACPI)

-- 
2.34.1
Re: [PATCH v20 04/10] firmware: psci: Introduce command-based reset in psci_sys_reset
Posted by Lorenzo Pieralisi 1 week, 6 days ago
On Wed, Mar 04, 2026 at 11:33:04PM +0530, Shivendra Pratap wrote:
> PSCI currently supports only COLD reset and ARCH WARM reset 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 via a new psci_set_reset_cmd() function.
> 
> The psci command-based reset is disabled by default and the
> psci_sys_reset follows its original flow until a psci_reset command is
> set. In kernel panic path, psci_reset command is ignored.

If it is function calls you should add parenthesis (eg psci_sys_reset ->
psci_sys_reset()).

You must explain why the kernel panic path requires separate handling
here AND in the code - think about looking at this years down the line
and figure out why kernel panics are special here.

> Signed-off-by: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>
> ---
>  drivers/firmware/psci/psci.c | 45 ++++++++++++++++++++++++++++++++++++++++++--
>  include/linux/psci.h         |  2 ++
>  2 files changed, 45 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
> index 38ca190d4a22d6e7e0f06420e8478a2b0ec2fe6f..ae6f7a0aead913d740070080d4b2a3da15b29485 100644
> --- a/drivers/firmware/psci/psci.c
> +++ b/drivers/firmware/psci/psci.c
> @@ -51,6 +51,15 @@ static int resident_cpu = -1;
>  struct psci_operations psci_ops;
>  static enum arm_smccc_conduit psci_conduit = SMCCC_CONDUIT_NONE;
>  
> +struct psci_sys_reset_params {
> +	u32 system_reset;
> +	u32 reset_type;
> +	u32 cookie;
> +	bool cmd;
> +};
> +
> +static struct psci_sys_reset_params psci_reset;
> +
>  bool psci_tos_resident_on(int cpu)
>  {
>  	return cpu == resident_cpu;
> @@ -80,6 +89,28 @@ static u32 psci_cpu_suspend_feature;
>  static bool psci_system_reset2_supported;
>  static bool psci_system_off2_hibernate_supported;
>  
> +/**
> + * psci_set_reset_cmd - Sets the psci_reset_cmd for command-based
> + * reset which will be used in psci_sys_reset call.
> + *
> + * @cmd_sys_rst2: Set to true for SYSTEM_RESET2 based resets.
> + * @cmd_reset_type: Set the reset_type argument for psci_sys_reset.
> + * @cmd_cookie: Set the cookie argument for psci_sys_reset.
> + */
> +void psci_set_reset_cmd(bool cmd_sys_rst2, u32 cmd_reset_type, u32 cmd_cookie)
> +{

I don't think cmd_sys_rst2 is needed, as a replied in a different thread.

> +	if (cmd_sys_rst2 && psci_system_reset2_supported) {
> +		psci_reset.system_reset = PSCI_FN_NATIVE(1_1, SYSTEM_RESET2);
> +		psci_reset.reset_type = cmd_reset_type;
> +		psci_reset.cookie = cmd_cookie;
> +	} else {
> +		psci_reset.system_reset = PSCI_0_2_FN_SYSTEM_RESET;
> +		psci_reset.reset_type = 0;
> +		psci_reset.cookie = 0;
> +	}
> +	psci_reset.cmd = true;
> +}
> +
>  static inline bool psci_has_ext_power_state(void)
>  {
>  	return psci_cpu_suspend_feature &
> @@ -309,14 +340,24 @@ 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) &&
> -	    psci_system_reset2_supported) {
> +	if (((reboot_mode == REBOOT_WARM || reboot_mode == REBOOT_SOFT) &&
> +	     psci_system_reset2_supported) && (panic_in_progress() || !psci_reset.cmd)) {
>  		/*
>  		 * reset_type[31] = 0 (architectural)
>  		 * reset_type[30:0] = 0 (SYSTEM_WARM_RESET)
>  		 * cookie = 0 (ignored by the implementation)
>  		 */
>  		invoke_psci_fn(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2), 0, 0, 0);
> +	} else if (!panic_in_progress() && psci_reset.cmd) {
> +		/*
> +		 * Commands are being set in psci_set_reset_cmd
> +		 * This issues, SYSTEM_RESET2 arch warm reset or
> +		 * SYSTEM_RESET2 vendor-specific reset or
> +		 * a SYSTEM_RESET cold reset in accordance with
> +		 * the reboot-mode command.
> +		 */
> +		invoke_psci_fn(psci_reset.system_reset, psci_reset.reset_type,
> +			       psci_reset.cookie, 0);
>  	} else {
>  		invoke_psci_fn(PSCI_0_2_FN_SYSTEM_RESET, 0, 0, 0);

This is very hard to parse. IMO, what you should do is:

- Split this into two different paths: reboot_mode vs psci_reset.cmd == true.
- Document very clearly why a panic needs separate handling.

Something like:

if (psci_reset.cmd)
	handle_reset_cmd();
else
	handle_reboot_mode();

I don't think we are far from converging but I want to be able to maintain
this code going forward.

Thanks,
Lorenzo

>  	}
> diff --git a/include/linux/psci.h b/include/linux/psci.h
> index 4ca0060a3fc42ba1ca751c7862fb4ad8dda35a4c..d13ceca88eab8932894051e7c86e806c2ad8a73a 100644
> --- a/include/linux/psci.h
> +++ b/include/linux/psci.h
> @@ -45,8 +45,10 @@ 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);
> +void psci_set_reset_cmd(bool cmd_sys_rst2, u32 cmd_reset_type, u32 cmd_cookie);
>  #else
>  static inline int psci_dt_init(void) { return 0; }
> +static inline void psci_set_reset_cmd(bool cmd_sys_rst2, u32 cmd_reset_type, u32 cmd_cookie) { }
>  #endif
>  
>  #if defined(CONFIG_ARM_PSCI_FW) && defined(CONFIG_ACPI)
> 
> -- 
> 2.34.1
>
Re: [PATCH v20 04/10] firmware: psci: Introduce command-based reset in psci_sys_reset
Posted by Shivendra Pratap 1 week, 2 days ago

On 27-03-2026 19:37, Lorenzo Pieralisi wrote:
> On Wed, Mar 04, 2026 at 11:33:04PM +0530, Shivendra Pratap wrote:
>> PSCI currently supports only COLD reset and ARCH WARM reset 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 via a new psci_set_reset_cmd() function.
>>
>> The psci command-based reset is disabled by default and the
>> psci_sys_reset follows its original flow until a psci_reset command is
>> set. In kernel panic path, psci_reset command is ignored.
> 
> If it is function calls you should add parenthesis (eg psci_sys_reset ->
> psci_sys_reset()).
> 
> You must explain why the kernel panic path requires separate handling
> here AND in the code - think about looking at this years down the line
> and figure out why kernel panics are special here.

Ack.

> 
>> Signed-off-by: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>
>> ---
>>   drivers/firmware/psci/psci.c | 45 ++++++++++++++++++++++++++++++++++++++++++--
>>   include/linux/psci.h         |  2 ++
>>   2 files changed, 45 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
>> index 38ca190d4a22d6e7e0f06420e8478a2b0ec2fe6f..ae6f7a0aead913d740070080d4b2a3da15b29485 100644
>> --- a/drivers/firmware/psci/psci.c
>> +++ b/drivers/firmware/psci/psci.c
>> @@ -51,6 +51,15 @@ static int resident_cpu = -1;
>>   struct psci_operations psci_ops;
>>   static enum arm_smccc_conduit psci_conduit = SMCCC_CONDUIT_NONE;
>>   
>> +struct psci_sys_reset_params {
>> +	u32 system_reset;
>> +	u32 reset_type;
>> +	u32 cookie;
>> +	bool cmd;
>> +};
>> +
>> +static struct psci_sys_reset_params psci_reset;
>> +
>>   bool psci_tos_resident_on(int cpu)
>>   {
>>   	return cpu == resident_cpu;
>> @@ -80,6 +89,28 @@ static u32 psci_cpu_suspend_feature;
>>   static bool psci_system_reset2_supported;
>>   static bool psci_system_off2_hibernate_supported;
>>   
>> +/**
>> + * psci_set_reset_cmd - Sets the psci_reset_cmd for command-based
>> + * reset which will be used in psci_sys_reset call.
>> + *
>> + * @cmd_sys_rst2: Set to true for SYSTEM_RESET2 based resets.
>> + * @cmd_reset_type: Set the reset_type argument for psci_sys_reset.
>> + * @cmd_cookie: Set the cookie argument for psci_sys_reset.
>> + */
>> +void psci_set_reset_cmd(bool cmd_sys_rst2, u32 cmd_reset_type, u32 cmd_cookie)
>> +{
> 
> I don't think cmd_sys_rst2 is needed, as a replied in a different thread.

Need bit more clarification. The issue is that at some point we need to 
decide - sys_rst2 or the reboot_mode based reset. Are you suggesting 
that this check should be in psci driver instead of a pre-check in 
psci_reboot_mode driver?

> 
>> +	if (cmd_sys_rst2 && psci_system_reset2_supported) {
>> +		psci_reset.system_reset = PSCI_FN_NATIVE(1_1, SYSTEM_RESET2);
>> +		psci_reset.reset_type = cmd_reset_type;
>> +		psci_reset.cookie = cmd_cookie;
>> +	} else {
>> +		psci_reset.system_reset = PSCI_0_2_FN_SYSTEM_RESET;
>> +		psci_reset.reset_type = 0;
>> +		psci_reset.cookie = 0;
>> +	}
>> +	psci_reset.cmd = true;
>> +}
>> +
>>   static inline bool psci_has_ext_power_state(void)
>>   {
>>   	return psci_cpu_suspend_feature &
>> @@ -309,14 +340,24 @@ 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) &&
>> -	    psci_system_reset2_supported) {
>> +	if (((reboot_mode == REBOOT_WARM || reboot_mode == REBOOT_SOFT) &&
>> +	     psci_system_reset2_supported) && (panic_in_progress() || !psci_reset.cmd)) {
>>   		/*
>>   		 * reset_type[31] = 0 (architectural)
>>   		 * reset_type[30:0] = 0 (SYSTEM_WARM_RESET)
>>   		 * cookie = 0 (ignored by the implementation)
>>   		 */
>>   		invoke_psci_fn(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2), 0, 0, 0);
>> +	} else if (!panic_in_progress() && psci_reset.cmd) {
>> +		/*
>> +		 * Commands are being set in psci_set_reset_cmd
>> +		 * This issues, SYSTEM_RESET2 arch warm reset or
>> +		 * SYSTEM_RESET2 vendor-specific reset or
>> +		 * a SYSTEM_RESET cold reset in accordance with
>> +		 * the reboot-mode command.
>> +		 */
>> +		invoke_psci_fn(psci_reset.system_reset, psci_reset.reset_type,
>> +			       psci_reset.cookie, 0);
>>   	} else {
>>   		invoke_psci_fn(PSCI_0_2_FN_SYSTEM_RESET, 0, 0, 0);
> 
> This is very hard to parse. IMO, what you should do is:
> 
> - Split this into two different paths: reboot_mode vs psci_reset.cmd == true.
> - Document very clearly why a panic needs separate handling.
> 
> Something like:
> 
> if (psci_reset.cmd)
> 	handle_reset_cmd();
> else
> 	handle_reboot_mode();
> 
> I don't think we are far from converging but I want to be able to maintain
> this code going forward.

Ack. Will restructure it as suggested.

thanks,
Shivendra