Re: [PATCH v25 03/10] firmware: psci: Introduce command-based resets

[Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>]

On 9/14/2026 8:44 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] The restart handler `psci_sys_reset()` unintentionally skips configured reset commands for standard reboots due to checking if `data` is non-NULL.
> --
> 
> commit 15791809d0deec27e24e64483b3317dacb5842a5
> Author: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>
> 
> firmware: psci: Introduce command-based resets
> 
> This commit introduces a command-based reset mechanism allowing reboot-mode
> drivers to specify custom PSCI reset commands like vendor-specific
> SYSTEM_RESET2. It defaults to the existing reboot_mode flow unless a valid
> reset command is configured.
> 
>> diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
>> index e73bae6cb23a3..d489ba5d147ce 100644
>> --- a/drivers/firmware/psci/psci.c
>> +++ b/drivers/firmware/psci/psci.c
> 
> [ ... ]
> 
>> @@ -320,6 +409,30 @@ static int psci_sys_reset(struct notifier_block *nb, unsigned long action,
> 
> [ ... ]
> 
>> +static int psci_sys_reset(struct notifier_block *nb, unsigned long action,
>> +			  void *data)
>> +{
>> +	const struct psci_system_reset_cmd *cmd;
>> +
>> +	/* 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 panic() or emergency_restart() occurs between the reboot_notifier
>> +	 * and this final reset, skip 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.
>> +	 */
>> +	cmd = smp_load_acquire(&reset_cmd);
>> +	if (data && cmd && !panic_in_progress())
>> +		psci_handle_reset_cmd(cmd);
> 
> [Severity: High]
> Will this check unintentionally skip custom reset commands during a standard
> reboot?

By design - Normal reboot with NULL command should go through regular 
reboot flow of psci_sys_reset.

> 
> When a standard userspace reboot is triggered via the reboot syscall with
> LINUX_REBOOT_CMD_RESTART, the reboot sequence ultimately calls
> kernel_restart(NULL).
> 
> This propagates down to the atomic restart notifier chain in
> do_kernel_restart(NULL), which passes a NULL command pointer to the notifier
> handlers.
> 
> In psci_sys_reset(), the data parameter receives this NULL pointer. Because
> of the check for a non-NULL data variable, the condition evaluates to false,
> causing the handler to skip psci_handle_reset_cmd(). 

Yes, do_kernel_restart(NULL) should take regular reboot flow and ignore 
any pre-set commands.

> It then incorrectly
> falls back to psci_handle_reboot_mode(), even if a valid command was
> successfully configured by a reboot-mode driver.
> 
> Is the data check acting contrary to the commit message's goal of ignoring
> reset commands during panic and emergency_restart?
> 

If the commit message is not clear, i can add a line to it, stating the 
same.

thanks,
Shivendra