linux-kernel - Re: [PATCH v2] remoteproc: k3: support for graceful shutdown of remote cores

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <ba318271-79d1-4fbc-ac39-30d61191ec30@ti.com>
Date: Mon, 19 Jan 2026 10:21:58 +0530
From: Beleswar Prasad Padhi <b-padhi@...com>
To: Patrick Oppenlander <patrick.oppenlander@...il.com>
CC: Mathieu Poirier <mathieu.poirier@...aro.org>, <andersson@...nel.org>,
	<richard.genoud@...tlin.com>, <afd@...com>, <hnagalla@...com>, <jm@...com>,
	<u-kumar1@...com>, <jan.kiszka@...mens.com>, <christophe.jaillet@...adoo.fr>,
	<linux-remoteproc@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] remoteproc: k3: support for graceful shutdown of
 remote cores


On 16/01/26 13:57, Patrick Oppenlander wrote:
> On Fri, 16 Jan 2026 at 16:58, Beleswar Prasad Padhi <b-padhi@...com> wrote:
>>
>> On 15/01/26 03:57, Patrick Oppenlander wrote:


[...]

>>>>>>> [...]
>>>>>>> +
>>>>>>> +/**
>>>>>>> + * notify_shutdown_rproc - Prepare the remoteproc for a shutdown
>>>>>>> + * @kproc: remote core pointer used for sending mbox msg
>>>>>>> + *
>>>>>>> + * This function sends the shutdown prepare message to remote processor and
>>>>>>> + * waits for an ACK. Further, it checks if the remote processor has entered
>>>>>>> + * into WFI mode. It is invoked in shutdown sequence to ensure the rproc
>>>>>>> + * has relinquished its resources before asserting a reset, so the shutdown
>>>>>>> + * happens cleanly.
>>>>>>> + */
>>>>>>> +int notify_shutdown_rproc(struct k3_rproc *kproc)
>>>>>>> +{
>>>>>>> +    bool wfi_status = false;
>>>>>>> +    int ret;
>>>>>>> +
>>>>>>> +    reinit_completion(&kproc->shutdown_complete);
>>>>>>> +
>>>>>>> +    ret = mbox_send_message(kproc->mbox, (void *)(uintptr_t)RP_MBOX_SHUTDOWN);
>>>>>>> +    if (ret < 0) {
>>>>>>> +            dev_err(kproc->dev, "PM mbox_send_message failed: %d\n", ret);
>>>>>>> +            return ret;
>>>>>>> +    }
>>>>>>> +
>>>>>>> +    ret = wait_for_completion_timeout(&kproc->shutdown_complete,
>>>>>>> +                                      msecs_to_jiffies(5000));
>>>>>>> +    if (ret == 0) {
>>>>>>> +            dev_err(kproc->dev, "%s: timeout waiting for rproc completion event\n",
>>>>>>> +                    __func__);
>>>>>>> +            return -EBUSY;
>>>>>>> +    }
>>>>>>> +
>>>>>> Won't that create an issue on systems with an older FW that doesn't send a
>>>>>> RP_MBOX_SHUDOWN_ACK message?  Unless I'm missing something, this kind of feature
>>>>>> needs to be backward compatible.
>>>>> I feel it would be unsafe to just abruptly power off a core without some
>>>>> handshake.. The core could be executing something, there could be
>>>>> pending bus transactions leading to system hangs etc.. We start the
>>>>> IPC mechanism with a handshake, so we should end it with a
>>>>> handshake too.. And for firmwares that don't support this handshake,
>>>>> IMO its better to reject the shutdown request. What do you think?
>>>>>
>>>> We can't affect the behavior of systems where old FW is coupled with a
>>>> new kernel.  If people want to address the bugs you referred to, they
>>>> can update their FW as they see fit.  As such things need to be
>>>> backward compatible.  NXP has recently implemented a handshake
>>>> mechanism such as this, but to assert the readiness of a booting
>>>> remote processor. They used the vendor specific resource table to
>>>> store a flag that enables the handshake - I suggest using the same
>>>> heuristic to implement this feature.
>>> A flag in a resource table enabling the new behaviour could work, but
>>> we would probably need another way to do the new thing, maybe with a
>>> devicetree flag.
>>
>> That's hacky. Device tree is for hardware description only. We
>> should not be putting SW required quirks in DT. It should be
>> rightly placed in the vendor specific resource table.
> There's plenty of places in devicetree such things are already done
> (most stuff starting with "linux,", and plenty of others).


Yeah, but we should not keep repeating the same mistake :) DT is for
non-discoverable hardware. This sort of info can always be grabbed
from querying firmware at runtime or from the resource table at
build time. The Devicetree is not just used by Linux, but various
other projects (bootloaders like U-Boot to count) which might not
make use of those custom "Linux" properties we put here.

Thanks,
Beleswar

> [...]