[PATCH] firmware: stratix10-svc: add PSCI secondary CPU offline on warm reboot for agilex and stratix10

[PATCH] firmware: stratix10-svc: add PSCI secondary CPU offline on warm reboot for agilex and stratix10

[Adrian Ng Ho Yin]

On Agilex and Stratix10 SoCs, secondary CPUs must be offlined before a
warm reboot to ensure a clean PSCI state. Add a reboot notifier that
queues work items to offline each secondary CPU via remove_cpu() when a
SYS_RESTART is triggered in REBOOT_WARM mode.

The workqueue, completion, pending CPU counter and reboot notifier block
are stored in stratix10_svc_controller to maintain per-device ownership.
The notifier callback recovers the controller pointer via container_of().

CPUs are pre-counted before any work is queued to avoid a race where a
fast-completing worker could signal the completion before all CPUs are
enqueued. The mechanism is guarded by of_device_is_compatible() and is
not activated on platforms such as Agilex5 that do not require it.

Signed-off-by: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
---
 drivers/firmware/stratix10-svc.c | 134 ++++++++++++++++++++++++++++++-
 1 file changed, 133 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/stratix10-svc.c b/drivers/firmware/stratix10-svc.c
index de938ab2db0b..0924027b41d7 100644
--- a/drivers/firmware/stratix10-svc.c
+++ b/drivers/firmware/stratix10-svc.c
@@ -24,6 +24,10 @@
 #include <linux/firmware/intel/stratix10-smc.h>
 #include <linux/firmware/intel/stratix10-svc-client.h>
 #include <linux/types.h>
+#include <linux/psci.h>
+#include <linux/of.h>
+#include <linux/cpuhplock.h>
+#include <linux/reboot.h>
 
 /**
  * SVC_NUM_DATA_IN_FIFO - number of struct stratix10_svc_data in the FIFO
@@ -96,6 +100,12 @@
 #define STRATIX10_GET_SDM_STATUS_CODE(status) \
 	(FIELD_GET(STRATIX10_SDM_STATUS_MASK, status))
 
+struct psci_cpu_off_work {
+	struct work_struct work;
+	struct stratix10_svc_controller *ctrl;
+	int cpu;
+};
+
 typedef void (svc_invoke_fn)(unsigned long, unsigned long, unsigned long,
 			     unsigned long, unsigned long, unsigned long,
 			     unsigned long, unsigned long,
@@ -280,6 +290,10 @@ struct stratix10_svc_chan {
  * @svc: manages the list of client svc drivers
  * @sdm_lock: only allows a single command single response to SDM
  * @actrl: async control structure
+ * @psci_reboot_nb: reboot notifier for PSCI secondary CPU offlining
+ * @psci_cpu_off_wq: workqueue used to offline secondary CPUs on warm reboot
+ * @psci_offline_done: completion signalled when all secondary CPUs are offline
+ * @psci_pending_cpus: number of secondary CPUs not yet offlined
  * @chans: array of service channels
  *
  * This struct is used to create communication channels for service clients, to
@@ -296,6 +310,10 @@ struct stratix10_svc_controller {
 	struct stratix10_svc *svc;
 	struct mutex sdm_lock;
 	struct stratix10_async_ctrl actrl;
+	struct notifier_block psci_reboot_nb;
+	struct workqueue_struct *psci_cpu_off_wq;
+	struct completion psci_offline_done;
+	atomic_t psci_pending_cpus;
 	struct stratix10_svc_chan chans[] __counted_by(num_chans);
 };
 
@@ -1981,6 +1999,92 @@ void stratix10_svc_free_memory(struct stratix10_svc_chan *chan, void *kaddr)
 }
 EXPORT_SYMBOL_GPL(stratix10_svc_free_memory);
 
+static void psci_cpu_off_worker(struct work_struct *work)
+{
+	struct psci_cpu_off_work *cw =
+		container_of(work, struct psci_cpu_off_work, work);
+	struct stratix10_svc_controller *ctrl = cw->ctrl;
+	int ret;
+
+	ret = remove_cpu(cw->cpu);
+	if (ret)
+		pr_err("psci_cpu_off: failed to offline CPU%d: %d\n", cw->cpu, ret);
+
+	if (atomic_dec_and_test(&ctrl->psci_pending_cpus))
+		complete(&ctrl->psci_offline_done);
+
+	kfree(cw);
+}
+
+static void psci_offline_secondary_cpus(struct stratix10_svc_controller *ctrl)
+{
+	int cpu, me;
+	struct psci_cpu_off_work *cw;
+
+	/* safely get current CPU id (disables preemption briefly) */
+	me = get_cpu();
+	put_cpu();
+
+	/* Pre-count before queuing to avoid a worker calling dec_and_test
+	 * and completing before all work items have been queued.
+	 */
+	atomic_set(&ctrl->psci_pending_cpus, 0);
+	for_each_online_cpu(cpu)
+		if (cpu != me)
+			atomic_inc(&ctrl->psci_pending_cpus);
+
+	if (!atomic_read(&ctrl->psci_pending_cpus))
+		return;
+
+	reinit_completion(&ctrl->psci_offline_done);
+
+	for_each_online_cpu(cpu) {
+		if (cpu == me)
+			continue;
+
+		cw = kzalloc_obj(*cw, GFP_KERNEL);
+		if (!cw) {
+			if (atomic_dec_and_test(&ctrl->psci_pending_cpus))
+				complete(&ctrl->psci_offline_done);
+			continue;
+		}
+
+		cw->ctrl = ctrl;
+		INIT_WORK(&cw->work, psci_cpu_off_worker);
+		cw->cpu = cpu;
+		queue_work(ctrl->psci_cpu_off_wq, &cw->work);
+	}
+
+	if (!wait_for_completion_timeout(&ctrl->psci_offline_done,
+					 msecs_to_jiffies(1000)))
+		pr_warn("psci_cpu_off: timed out waiting for secondary CPUs to offline\n");
+}
+
+static int psci_cpu_off_reboot_notifier(struct notifier_block *nb,
+					unsigned long action, void *data)
+{
+	struct stratix10_svc_controller *ctrl =
+		container_of(nb, struct stratix10_svc_controller, psci_reboot_nb);
+
+	if (reboot_mode != REBOOT_WARM)
+		return NOTIFY_DONE;
+
+	if (action == SYS_RESTART)
+		psci_offline_secondary_cpus(ctrl);
+
+	return NOTIFY_OK;
+}
+
+static void psci_cpu_off_teardown(struct stratix10_svc_controller *ctrl)
+{
+	unregister_reboot_notifier(&ctrl->psci_reboot_nb);
+	if (ctrl->psci_cpu_off_wq) {
+		flush_workqueue(ctrl->psci_cpu_off_wq);
+		destroy_workqueue(ctrl->psci_cpu_off_wq);
+		ctrl->psci_cpu_off_wq = NULL;
+	}
+}
+
 static const struct of_device_id stratix10_svc_drv_match[] = {
 	{.compatible = "intel,stratix10-svc"},
 	{.compatible = "intel,agilex-svc"},
@@ -2001,6 +2105,7 @@ static int stratix10_svc_drv_probe(struct platform_device *pdev)
 	struct gen_pool *genpool;
 	struct stratix10_svc_sh_memory *sh_memory;
 	struct stratix10_svc *svc = NULL;
+	struct device_node *node = pdev->dev.of_node;
 
 	svc_invoke_fn *invoke_fn;
 	size_t fifo_size;
@@ -2040,13 +2145,36 @@ static int stratix10_svc_drv_probe(struct platform_device *pdev)
 	INIT_LIST_HEAD(&controller->node);
 	init_completion(&controller->complete_status);
 
+	if (of_device_is_compatible(node, "intel,agilex-svc") ||
+	    of_device_is_compatible(node, "intel,stratix10-svc")) {
+		init_completion(&controller->psci_offline_done);
+		atomic_set(&controller->psci_pending_cpus, 0);
+
+		controller->psci_cpu_off_wq =
+			alloc_workqueue("psci_cpu_off_wq", WQ_UNBOUND, 0);
+		if (!controller->psci_cpu_off_wq) {
+			ret = -ENOMEM;
+			goto err_destroy_pool;
+		}
+
+		controller->psci_reboot_nb.notifier_call =
+			psci_cpu_off_reboot_notifier;
+		controller->psci_reboot_nb.priority = INT_MAX;
+
+		ret = register_reboot_notifier(&controller->psci_reboot_nb);
+		if (ret) {
+			dev_err(dev, "failed to register reboot notifier: %d\n", ret);
+			goto err_free_workqueue;
+		}
+	}
+
 	ret = stratix10_svc_async_init(controller);
 	if (ret == -EOPNOTSUPP) {
 		dev_info(dev, "Intel Service Layer Driver Initialized (sync-only mode)\n");
 	} else if (ret) {
 		dev_dbg(dev, "Intel Service Layer Driver: Error on stratix10_svc_async_init %d\n",
 			ret);
-		goto err_destroy_pool;
+		goto err_free_workqueue;
 	} else {
 		dev_info(dev, "Intel Service Layer Driver Initialized\n");
 	}
@@ -2110,6 +2238,8 @@ static int stratix10_svc_drv_probe(struct platform_device *pdev)
 	while (i--)
 		kfifo_free(&controller->chans[i].svc_fifo);
 	stratix10_svc_async_exit(controller);
+err_free_workqueue:
+	psci_cpu_off_teardown(controller);
 err_destroy_pool:
 	gen_pool_destroy(genpool);
 
@@ -2139,6 +2269,8 @@ static void stratix10_svc_drv_remove(struct platform_device *pdev)
 	if (ctrl->genpool)
 		gen_pool_destroy(ctrl->genpool);
 	list_del(&ctrl->node);
+
+	psci_cpu_off_teardown(ctrl);
 }
 
 static struct platform_driver stratix10_svc_driver = {
-- 
2.49.GIT

Re: [PATCH] firmware: stratix10-svc: add PSCI secondary CPU offline on warm reboot for agilex and stratix10

[Dinh Nguyen]

On 7/7/26 02:39, Adrian Ng Ho Yin wrote:
> On Agilex and Stratix10 SoCs, secondary CPUs must be offlined before a
> warm reboot to ensure a clean PSCI state. Add a reboot notifier that
> queues work items to offline each secondary CPU via remove_cpu() when a
> SYS_RESTART is triggered in REBOOT_WARM mode.
> 
> The workqueue, completion, pending CPU counter and reboot notifier block
> are stored in stratix10_svc_controller to maintain per-device ownership.
> The notifier callback recovers the controller pointer via container_of().
> 
> CPUs are pre-counted before any work is queued to avoid a race where a
> fast-completing worker could signal the completion before all CPUs are
> enqueued. The mechanism is guarded by of_device_is_compatible() and is
> not activated on platforms such as Agilex5 that do not require it.
> 
> Signed-off-by: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
> ---
>   drivers/firmware/stratix10-svc.c | 134 ++++++++++++++++++++++++++++++-
>   1 file changed, 133 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/firmware/stratix10-svc.c b/drivers/firmware/stratix10-svc.c
> index de938ab2db0b..0924027b41d7 100644
> --- a/drivers/firmware/stratix10-svc.c
> +++ b/drivers/firmware/stratix10-svc.c
> @@ -24,6 +24,10 @@
>   #include <linux/firmware/intel/stratix10-smc.h>
>   #include <linux/firmware/intel/stratix10-svc-client.h>
>   #include <linux/types.h>
> +#include <linux/psci.h>
> +#include <linux/of.h>
> +#include <linux/cpuhplock.h>
> +#include <linux/reboot.h>
>   
>   /**
>    * SVC_NUM_DATA_IN_FIFO - number of struct stratix10_svc_data in the FIFO
> @@ -96,6 +100,12 @@
>   #define STRATIX10_GET_SDM_STATUS_CODE(status) \
>   	(FIELD_GET(STRATIX10_SDM_STATUS_MASK, status))
>   
> +struct psci_cpu_off_work {
> +	struct work_struct work;
> +	struct stratix10_svc_controller *ctrl;
> +	int cpu;
> +};
> +
>   typedef void (svc_invoke_fn)(unsigned long, unsigned long, unsigned long,
>   			     unsigned long, unsigned long, unsigned long,
>   			     unsigned long, unsigned long,
> @@ -280,6 +290,10 @@ struct stratix10_svc_chan {
>    * @svc: manages the list of client svc drivers
>    * @sdm_lock: only allows a single command single response to SDM
>    * @actrl: async control structure
> + * @psci_reboot_nb: reboot notifier for PSCI secondary CPU offlining
> + * @psci_cpu_off_wq: workqueue used to offline secondary CPUs on warm reboot
> + * @psci_offline_done: completion signalled when all secondary CPUs are offline
> + * @psci_pending_cpus: number of secondary CPUs not yet offlined
>    * @chans: array of service channels
>    *
>    * This struct is used to create communication channels for service clients, to
> @@ -296,6 +310,10 @@ struct stratix10_svc_controller {
>   	struct stratix10_svc *svc;
>   	struct mutex sdm_lock;
>   	struct stratix10_async_ctrl actrl;
> +	struct notifier_block psci_reboot_nb;
> +	struct workqueue_struct *psci_cpu_off_wq;
> +	struct completion psci_offline_done;
> +	atomic_t psci_pending_cpus;
>   	struct stratix10_svc_chan chans[] __counted_by(num_chans);
>   };
>   
> @@ -1981,6 +1999,92 @@ void stratix10_svc_free_memory(struct stratix10_svc_chan *chan, void *kaddr)
>   }
>   EXPORT_SYMBOL_GPL(stratix10_svc_free_memory);
>   
> +static void psci_cpu_off_worker(struct work_struct *work)
> +{
> +	struct psci_cpu_off_work *cw =
> +		container_of(work, struct psci_cpu_off_work, work);
> +	struct stratix10_svc_controller *ctrl = cw->ctrl;
> +	int ret;
> +
> +	ret = remove_cpu(cw->cpu);

Could cw->cpu be NULL here?

> +	if (ret)
> +		pr_err("psci_cpu_off: failed to offline CPU%d: %d\n", cw->cpu, ret);
> +
> +	if (atomic_dec_and_test(&ctrl->psci_pending_cpus))
> +		complete(&ctrl->psci_offline_done);
If remove_cpu() failed, but you continue to decrement the pending count 
and potentially trigger the completion.

The parent thread waiting on psci_offline_done will wake up assuming the 
system is cleanly parked, even if a CPU refused to offline.

If this is intended as a "best-effort" shutdown before a forced reboot, 
this is fine.

If the caller requires all CPUs to be successfully offlined before 
proceeding (to avoid hardware deadlocks), you need to pass a failure 
state back to the ctrl structure so the caller knows the operation was 
compromised.

> +
> +	kfree(cw);
> +}
> +
> +static void psci_offline_secondary_cpus(struct stratix10_svc_controller *ctrl)
> +{
> +	int cpu, me;
> +	struct psci_cpu_off_work *cw;
> +
> +	/* safely get current CPU id (disables preemption briefly) */
> +	me = get_cpu();
> +	put_cpu();
> +
> +	/* Pre-count before queuing to avoid a worker calling dec_and_test
> +	 * and completing before all work items have been queued.
> +	 */
> +	atomic_set(&ctrl->psci_pending_cpus, 0);
> +	for_each_online_cpu(cpu)
> +		if (cpu != me)
> +			atomic_inc(&ctrl->psci_pending_cpus);
> +
> +	if (!atomic_read(&ctrl->psci_pending_cpus))
> +		return;
> +
> +	reinit_completion(&ctrl->psci_offline_done);
> +
> +	for_each_online_cpu(cpu) {
> +		if (cpu == me)
> +			continue;
> +
> +		cw = kzalloc_obj(*cw, GFP_KERNEL);
> +		if (!cw) {
> +			if (atomic_dec_and_test(&ctrl->psci_pending_cpus))
> +				complete(&ctrl->psci_offline_done);
> +			continue;
> +		}
> +
> +		cw->ctrl = ctrl;
> +		INIT_WORK(&cw->work, psci_cpu_off_worker);

I dug deeper in the sashiko warning about this[0]. From Gemini 3.1 Pro:

"Spawning multiple threads here adds unnecessary scheduler overhead, 
memory allocations (kfree(cw)), and code complexity for zero performance 
gain. You should replace this fan-in worker pattern with a single work 
item that simply loops through the target CPUs and offlines them 
sequentially."

> +		cw->cpu = cpu;
> +		queue_work(ctrl->psci_cpu_off_wq, &cw->work);
> +	}
> +
> +	if (!wait_for_completion_timeout(&ctrl->psci_offline_done,
> +					 msecs_to_jiffies(1000)))
> +		pr_warn("psci_cpu_off: timed out waiting for secondary CPUs to offline\n");
> +}
> +
> +static int psci_cpu_off_reboot_notifier(struct notifier_block *nb,
> +					unsigned long action, void *data)
> +{
> +	struct stratix10_svc_controller *ctrl =
> +		container_of(nb, struct stratix10_svc_controller, psci_reboot_nb);
> +
> +	if (reboot_mode != REBOOT_WARM)
> +		return NOTIFY_DONE;
> +
> +	if (action == SYS_RESTART)
> +		psci_offline_secondary_cpus(ctrl);
> +
> +	return NOTIFY_OK;
> +}
> +
> +static void psci_cpu_off_teardown(struct stratix10_svc_controller *ctrl)
> +{
> +	unregister_reboot_notifier(&ctrl->psci_reboot_nb);
> +	if (ctrl->psci_cpu_off_wq) {
> +		flush_workqueue(ctrl->psci_cpu_off_wq);
> +		destroy_workqueue(ctrl->psci_cpu_off_wq);
> +		ctrl->psci_cpu_off_wq = NULL;
> +	}
> +}
> +
>   static const struct of_device_id stratix10_svc_drv_match[] = {
>   	{.compatible = "intel,stratix10-svc"},
>   	{.compatible = "intel,agilex-svc"},
> @@ -2001,6 +2105,7 @@ static int stratix10_svc_drv_probe(struct platform_device *pdev)
>   	struct gen_pool *genpool;
>   	struct stratix10_svc_sh_memory *sh_memory;
>   	struct stratix10_svc *svc = NULL;
> +	struct device_node *node = pdev->dev.of_node;
>   
>   	svc_invoke_fn *invoke_fn;
>   	size_t fifo_size;
> @@ -2040,13 +2145,36 @@ static int stratix10_svc_drv_probe(struct platform_device *pdev)
>   	INIT_LIST_HEAD(&controller->node);
>   	init_completion(&controller->complete_status);
>   
> +	if (of_device_is_compatible(node, "intel,agilex-svc") ||
> +	    of_device_is_compatible(node, "intel,stratix10-svc")) {

It'd be better if you avoid using these funtions and instead have a data 
structure associated with these compatibles.


Dinh

[0] 
https://sashiko.dev/#/patchset/1717fff5f780948a2288fa8a57821cb5d19c30f5.1783409844.git.adrian.ho.yin.ng%40altera.com