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

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

On 9/15/2026 7:32 PM, Shivendra Pratap wrote:
> 
> 
> 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

Adding Lorenzo and Arnd for views on this review.

thanks,
Shivendra