stratix10: Add Interrupt support for asynchronous communication with SDM

PATCH In review [email protected] 3 participants 9 messages Aug 13 – Sep 21 1 message not in the archive

Subsystems (3)

Device tree bindings Arm/socfpga architecture Intel STRATIX10 firmware drivers

lore

Share & tools

lore

[email protected]

author

2026-08-13 06:35 UTC #0 lore raw

v3 cover stratix10: Add Interrupt support for asynchronous communication with SDM

From: Tze Yee Ng <[email protected]>

This patch-set adds support for interrupt from Secure Device Manager (SDM) to Stratix10 SoC for asynchronous communication.

Following things are added in this patch-set:
- Add support for interrupt from SDM to Stratix10 SoC.
- Add documentation for SDM interrupt in Stratix10 SoC.
- Enable SDM interrupt in Stratix10 SoC.
- Enable SDM interrupt in Agilex SoC.

I am taking over this series from Mahesh Rao and posting v3 on his behalf. Reference:
https://lore.kernel.org/r/[email protected]

Changes in v3:
- Reflow commit message body to ~75 columns (fix Dinh's comment in v2 review)
- Add a newline before stratix10_async_workqueue_handler description (fix Dinh's comment in v2 review)
- Treat of_irq_get() failure as irq <= 0, since 0 is also a failing value (fix Dinh's comment in v2 review)
- Fix IRQ disable race: only call disable_irq_nosync() when queue_work() newly queues work, and disable the fired irq argument rather than actrl->irq
- Do not invoke client callbacks under trx_list_lock; copy cb/cb_arg under the lock and call the callback after unlock
- Use disable_irq() + flush_work() + devm_free_irq() on teardown, and treat unset IRQ as 0
- Demote successful IRQ registration log from dev_alert() to dev_info(), and demote async poll failure log from dev_err() to dev_dbg()
- Clarify INTEL_SIP_SMC_ASYNC_POLL_ON_IRQ kdoc to match a1-a4 bitmap usage
- Drop Reviewed-by: Matthew Gerlach from all patches
(no longer at Altera)
- Link to RESEND v2:
https://lore.kernel.org/r/[email protected]

Changes in v2:
- Separated from the larger SIP SVC upstream series into a dedicated interrupt-support series as requested
- Link to v1:
https://lore.kernel.org/r/[email protected]

Mahesh Rao (4):
dt-bindings: firmware: Add interrupt specification for Intel Stratix
10 Service Layer.
dts: stratix10: Add support for SDM mailbox interrupt for Intel
Stratix10 SoC FPGA.
dts: agilex: Add support for SDM mailbox interrupt for Intel Agilex
SoC FPGA.
firmware: stratix10-svc: Add for SDM mailbox doorbell interrupt

 .../firmware/intel,stratix10-svc.yaml         |  10 ++
 .../boot/dts/altera/socfpga_stratix10.dtsi    |   4 +
 arch/arm64/boot/dts/intel/socfpga_agilex.dtsi |   2 +
 drivers/firmware/stratix10-svc.c              | 135 ++++++++++++++++--
 include/linux/firmware/intel/stratix10-smc.h  |  21 +++
 5 files changed, 161 insertions(+), 11 deletions(-)

Signature · 1 line

2.43.7

[email protected]

author

2026-08-13 06:35 UTC #1 lore raw

PATCH v3 1/4 dt-bindings: firmware: Add interrupt specification for Intel Stratix 10 Service Layer.

From: Mahesh Rao <[email protected]>

Add interrupt specification for Intel Stratix10 Service layer for asynchronous communication.

Reviewed-by Rob Herring (Arm) <[email protected]>

Signed-off-by Mahesh Rao <[email protected]>

Signed-off-by Tze Yee Ng <[email protected]>

---
Changes in v3:
- Reflow commit message body to ~75 columns (fix Dinh's comment in v2 review)
- No functional change
- Drop Reviewed-by: Matthew Gerlach (no longer at Altera)
- Add Signed-off-by: Tze Yee Ng <[email protected]>

---
 .../bindings/firmware/intel,stratix10-svc.yaml         | 10 ++++++++++
 1 file changed, 10 insertions(+)

Documentation/devicetree/bindings/firmware/intel,stratix10-svc.yaml +10 -0

index b42cfa78b28b..1af8ee3f1f3b 100644
@@ -55,6 +55,12 @@ properties:
       reserved memory region for the service layer driver to
       communicate with the secure device manager.
 
+  interrupts:
+    maxItems: 1
+    description:
+      This interrupt is used by the Secure Device Manager (SDM) to signal
+      completion of an asynchronous operation to service layer driver.
+
   iommus:
     maxItems: 1

@@ -82,6 +88,8 @@ additionalProperties: false
 
 examples:
   - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
     reserved-memory {
       #address-cells = <2>;
       #size-cells = <2>;

@@ -99,6 +107,8 @@ examples:
         compatible = "intel,stratix10-svc";
         method = "smc";
         memory-region = <&service_reserved>;
+        interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>;
+        interrupt-parent = <&intc0>;
 
         fpga-mgr {
           compatible = "intel,stratix10-soc-fpga-mgr";

Signature · 1 line

2.43.7

[email protected]

author

2026-08-13 06:35 UTC #2 lore raw

PATCH v3 2/4 dts: stratix10: Add support for SDM mailbox interrupt for Intel Stratix10 SoC FPGA.

In reply to [email protected]

From: Mahesh Rao <[email protected]>

Add support for Secure Device Manager (SDM) mailbox doorbell interrupt on Stratix10 SoC FPGA for supporting asynchronous transactions.

Signed-off-by Mahesh Rao <[email protected]>

Signed-off-by Tze Yee Ng <[email protected]>

---
Changes in v3:
- Reflow commit message body to ~75 columns (fix Dinh's comment in v2 review)
- No functional change
- Add Signed-off-by: Tze Yee Ng <[email protected]>

---
 arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi +4 -0

index 0d9cad0c0351..5428b138d0ac 100644
@@ -1,11 +1,13 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
  * Copyright Altera Corporation (C) 2015. All rights reserved.
+ * Copyright (C) 2025, Altera Corporation
  */
 
 /dts-v1/;
 #include <dt-bindings/reset/altr,rst-mgr-s10.h>
 #include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/clock/stratix10-clock.h>
 
 / {

@@ -74,6 +76,8 @@ svc {
 			compatible = "intel,stratix10-svc";
 			method = "smc";
 			memory-region = <&service_reserved>;
+			interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-parent = <&intc>;
 
 			fpga_mgr: fpga-mgr {
 				compatible = "intel,stratix10-soc-fpga-mgr";

Signature · 1 line

2.43.7

[email protected]

author

2026-08-13 06:35 UTC #3 lore raw

PATCH v3 3/4 dts: agilex: Add support for SDM mailbox interrupt for Intel Agilex SoC FPGA.

In reply to [email protected]

From: Mahesh Rao <[email protected]>

Add support for Secure Device Manager (SDM) mailbox doorbell interrupt on Agilex SoC FPGA for supporting asynchronous transactions.

Signed-off-by Mahesh Rao <[email protected]>

Signed-off-by Tze Yee Ng <[email protected]>

---
 arch/arm64/boot/dts/intel/socfpga_agilex.dtsi | 2 ++
 1 file changed, 2 insertions(+)

arch/arm64/boot/dts/intel/socfpga_agilex.dtsi +2 -0

index 0dfbafde8822..3168449dcd3d 100644
@@ -65,6 +65,8 @@ svc {
 			compatible = "intel,agilex-svc";
 			method = "smc";
 			memory-region = <&service_reserved>;
+			interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-parent = <&intc>;
 
 			fpga_mgr: fpga-mgr {
 				compatible = "intel,agilex-soc-fpga-mgr";

Signature · 1 line

2.43.7

[email protected]

author

2026-08-13 06:35 UTC #4 lore raw

PATCH v3 4/4 firmware: stratix10-svc: Add for SDM mailbox doorbell interrupt

In reply to [email protected]

From: Mahesh Rao <[email protected]>

Add support for SDM (Secure Device Manager) mailbox doorbell interrupt for async transactions. On interrupt, a workqueue is triggered which polls the ATF for pending responses and retrieves the bitmap of all retrieved and unprocessed transaction ids of mailbox responses from SDM. It then triggers the corresponding registered callbacks.

Signed-off-by Mahesh Rao <[email protected]>

Signed-off-by Tze Yee Ng <[email protected]>

---
Changes in v3:
- Reflow commit message body to ~75 columns (fix Dinh's comment in v2 review)
- Add a newline before stratix10_async_workqueue_handler description (fix Dinh's comment in v2 review)
- Treat of_irq_get() failure as irq <= 0, since 0 is also a failing value (fix Dinh's comment in v2 review)
- Fix IRQ disable race: only call disable_irq_nosync() when queue_work() newly queues work, and disable the fired irq argument rather than actrl->irq
- Do not invoke client callbacks under trx_list_lock; copy cb/cb_arg under the lock and call the callback after unlock
- Use disable_irq() + flush_work() + devm_free_irq() on teardown, and treat unset IRQ as 0
- Demote successful IRQ registration log from dev_alert() to dev_info(), and demote async poll failure log from dev_err() to dev_dbg()
- Clarify INTEL_SIP_SMC_ASYNC_POLL_ON_IRQ kdoc to match a1-a4 bitmap usage
- Drop Reviewed-by: Matthew Gerlach (no longer at Altera)
- Add Signed-off-by: Tze Yee Ng <[email protected]>

---
 drivers/firmware/stratix10-svc.c             | 135 +++++++++++++++++--
 include/linux/firmware/intel/stratix10-smc.h |  21 +++
 2 files changed, 145 insertions(+), 11 deletions(-)

drivers/firmware/stratix10-svc.c +124 ~~-11~~

index 08395fe3ddaa..3035bf46b65b 100644
@@ -10,12 +10,14 @@
 #include <linux/genalloc.h>
 #include <linux/hashtable.h>
 #include <linux/idr.h>
+#include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/kfifo.h>
 #include <linux/kthread.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/of.h>
+#include <linux/of_irq.h>
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>

@@ -24,6 +26,7 @@
 #include <linux/firmware/intel/stratix10-smc.h>
 #include <linux/firmware/intel/stratix10-svc-client.h>
 #include <linux/types.h>
+#include <linux/workqueue.h>
 
 /**
  * SVC_NUM_DATA_IN_FIFO - number of struct stratix10_svc_data in the FIFO

@@ -219,6 +222,7 @@ struct stratix10_async_chan {
  * @supported: Flag indicating whether the system supports async operations
  * @initialized: Flag indicating whether the control structure has
  *               been initialized
+ * @irq: Interrupt request number associated with the asynchronous control
  * @invoke_fn: Function pointer for invoking Stratix10 service calls
  *             to EL3 secure firmware
  * @async_id_pool: Pointer to the ID pool used for asynchronous

@@ -229,12 +233,14 @@ struct stratix10_async_chan {
  *                     structure
  * @trx_list_lock: Spinlock for protecting the transaction list
  *                     operations
+ * @async_work: Work structure for scheduling asynchronous work
  * @trx_list: Hash table for managing asynchronous transactions
  */
 
 struct stratix10_async_ctrl {
 	bool supported;
 	bool initialized;
+	int irq;
 	void (*invoke_fn)(struct stratix10_async_ctrl *actrl,
 			  const struct arm_smccc_1_2_regs *args,
 			  struct arm_smccc_1_2_regs *res);

@@ -243,6 +249,7 @@ struct stratix10_async_ctrl {
 	struct stratix10_async_chan *common_async_chan;
 	/* spinlock to protect trx_list hash table */
 	spinlock_t trx_list_lock;
+	struct work_struct async_work;
 	DECLARE_HASHTABLE(trx_list, ASYNC_TRX_HASH_BITS);
 };

@@ -1595,7 +1602,7 @@ int stratix10_svc_async_poll(struct stratix10_svc_chan *chan,
 		return -EAGAIN;
 	}
 
-	dev_err(ctrl->dev,
+	dev_dbg(ctrl->dev,
 		"Failed to poll async message ,got status as %ld\n",
 		handle->res.a0);
 	return -EINVAL;

@@ -1658,14 +1665,96 @@ static inline void stratix10_smc_1_2(struct stratix10_async_ctrl *actrl,
 	arm_smccc_1_2_smc(args, res);
 }
 
+static irqreturn_t stratix10_svc_async_irq_handler(int irq, void *dev_id)
+{
+	struct stratix10_svc_controller *ctrl = dev_id;
+	struct stratix10_async_ctrl *actrl = &ctrl->actrl;
+
+	/*
+	 * Only disable the IRQ when work is newly queued. If the work is
+	 * already pending/running, queue_work() returns false; an extra
+	 * disable_irq_nosync() would leave the IRQ disabled permanently
+	 * after enable_irq() in the work handler.
+	 */
+	if (queue_work(system_bh_wq, &actrl->async_work))
+		disable_irq_nosync(irq);
+	return IRQ_HANDLED;
+}
+
+/**
+ * stratix10_async_workqueue_handler - Handler for the asynchronous
+ * workqueue in Stratix10 service controller.
+ * @work: Pointer to the work structure that contains the asynchronous
+ *        workqueue handler.
+ *
+ * This function is the handler for the asynchronous workqueue. It performs
+ * the following tasks:
+ * - Invokes the asynchronous polling on interrupt supervisory call.
+ * - On success, it retrieves the bitmap of pending transactions from mailbox
+ *   fifo in ATF.
+ * - It processes each pending transaction by calling the corresponding
+ *   callback function.
+ *
+ * The function ensures that the IRQ is enabled after processing the
+ * transactions.
+ */
+static void stratix10_async_workqueue_handler(struct work_struct *work)
+{
+	struct stratix10_async_ctrl *actrl =
+		container_of(work, struct stratix10_async_ctrl, async_work);
+	struct arm_smccc_1_2_regs
+		args = { .a0 = INTEL_SIP_SMC_ASYNC_POLL_ON_IRQ }, res;
+	DECLARE_BITMAP(pend_on_irq, TOTAL_TRANSACTION_IDS);
+	struct stratix10_svc_async_handler *handler;
+	unsigned long transaction_id = 0;
+	u64 bitmap_array[4];
+	async_callback_t cb;
+	void *cb_arg;
+
+	actrl->invoke_fn(actrl, &args, &res);
+	if (res.a0 == INTEL_SIP_SMC_STATUS_OK) {
+		bitmap_array[0] = res.a1;
+		bitmap_array[1] = res.a2;
+		bitmap_array[2] = res.a3;
+		bitmap_array[3] = res.a4;
+		bitmap_from_arr64(pend_on_irq, bitmap_array, TOTAL_TRANSACTION_IDS);
+		do {
+			transaction_id = find_next_bit(pend_on_irq,
+						       TOTAL_TRANSACTION_IDS,
+						       transaction_id);
+			if (transaction_id >= TOTAL_TRANSACTION_IDS)
+				break;
+
+			cb = NULL;
+			cb_arg = NULL;
+			spin_lock(&actrl->trx_list_lock);
+			hash_for_each_possible(actrl->trx_list, handler,
+					       next, transaction_id) {
+				if (handler->transaction_id == transaction_id) {
+					cb = handler->cb;
+					cb_arg = handler->cb_arg;
+					break;
+				}
+			}
+			spin_unlock(&actrl->trx_list_lock);
+			if (cb)
+				cb(cb_arg);
+
+			transaction_id++;
+		} while (transaction_id < TOTAL_TRANSACTION_IDS);
+	}
+	enable_irq(actrl->irq);
+}
+
 /**
  * stratix10_svc_async_init - Initialize the Stratix10 service
  *                            controller for asynchronous operations.
  * @controller: Pointer to the Stratix10 service controller structure.
  *
  * This function initializes the asynchronous service controller by
- * setting up the necessary data structures and initializing the
- * transaction list.
+ * setting up the necessary data structures ,initializing the
+ * transaction list and registering the IRQ handler for asynchronous
+ * transactions.
  *
  * Return: 0 on success, -EINVAL if the controller is NULL or already
  *         initialized, -ENOMEM if memory allocation fails,

@@ -1678,7 +1767,7 @@ static int stratix10_svc_async_init(struct stratix10_svc_controller *controller)
 	struct stratix10_async_ctrl *actrl;
 	struct arm_smccc_res res;
 	struct device *dev;
-	int ret;
+	int ret, irq;
 
 	if (!controller)
 		return -EINVAL;

@@ -1726,6 +1815,22 @@ static int stratix10_svc_async_init(struct stratix10_svc_controller *controller)
 	hash_init(actrl->trx_list);
 	atomic_set(&actrl->common_achan_refcount, 0);
 
+	irq = of_irq_get(dev_of_node(dev), 0);
+	if (irq <= 0) {
+		dev_warn(dev, "Failed to get IRQ, falling back to polling mode\n");
+	} else {
+		ret = devm_request_any_context_irq(dev, irq, stratix10_svc_async_irq_handler,
+						   IRQF_NO_AUTOEN, "stratix10_svc", controller);
+		if (ret == 0) {
+			dev_info(dev,
+				 "Registered IRQ %d for sip async operations\n",
+				 irq);
+			actrl->irq = irq;
+			INIT_WORK(&actrl->async_work, stratix10_async_workqueue_handler);
+			enable_irq(actrl->irq);
+		}
+	}
+
 	actrl->initialized = true;
 	return 0;
 }

@@ -1735,13 +1840,14 @@ static int stratix10_svc_async_init(struct stratix10_svc_controller *controller)
  *                            service controller
  * @ctrl: Pointer to the stratix10_svc_controller structure
  *
- * This function performs the necessary cleanup for the asynchronous
- * service controller. It checks if the controller is valid and if it
- * has been initialized. It then locks the transaction list and safely
- * removes and deallocates each handler in the list. The function also
- * removes any asynchronous clients associated with the controller's
- * channels and destroys the asynchronous ID pool. Finally, it resets
- * the asynchronous ID pool and invoke function pointers to NULL.
+ * This function performs the necessary cleanup for the asynchronous service
+ * controller. It checks if the controller is valid and if it has been
+ * initialized. Also If the controller has an IRQ assigned, it frees the IRQ
+ * and flushes any pending asynchronous work. It then locks the transaction
+ * list and safely removes and deallocates each handler in the list.
+ * The function also removes any asynchronous clients associated with the
+ * controller's channels and destroys the asynchronous ID pool. Finally, it
+ * resets the asynchronous ID pool and invoke function pointers to NULL.
  *
  * Return: 0 on success, -EINVAL if the controller is invalid or not
  *         initialized.

@@ -1763,6 +1869,13 @@ static int stratix10_svc_async_exit(struct stratix10_svc_controller *ctrl)
 
 	actrl->initialized = false;
 
+	if (actrl->irq > 0) {
+		disable_irq(actrl->irq);
+		flush_work(&actrl->async_work);
+		devm_free_irq(ctrl->dev, actrl->irq, ctrl);
+		actrl->irq = 0;
+	}
+
 	scoped_guard(spinlock_bh, &actrl->trx_list_lock) {
 		hash_for_each_safe(actrl->trx_list, i, tmp, handler, next) {
 			ida_free(&handler->achan->job_id_pool,

include/linux/firmware/intel/stratix10-smc.h +21 -0

index ce1d2c7203b8..355a74b2134d 100644
@@ -718,6 +718,27 @@ INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_FPGA_CONFIG_COMPLETED_WRITE)
 #define INTEL_SIP_SMC_ASYNC_POLL \
 	INTEL_SIP_SMC_ASYNC_VAL(INTEL_SIP_SMC_ASYNC_FUNC_ID_POLL)
 
+/**
+ * Request INTEL_SIP_SMC_ASYNC_POLL_ON_IRQ
+ * Async call used by service driver at EL1 to retrieve the transaction IDs
+ * of available responses from the SDM mailbox (typically after an IRQ).
+ *
+ * Call register usage:
+ * a0 INTEL_SIP_SMC_ASYNC_POLL_ON_IRQ
+ * a1-a17 not used
+ *
+ * Return status
+ * a0 INTEL_SIP_SMC_STATUS_OK
+ * a1-a4 contain a bitmap of pending transaction IDs (set bit positions)
+ * a5-a17 not used
+ * Or
+ * a0 INTEL_SIP_SMC_STATUS_NO_RESPONSE
+ * a1-a17 not used
+ */
+#define INTEL_SIP_SMC_ASYNC_FUNC_ID_IRQ_POLL (0xC9)
+#define INTEL_SIP_SMC_ASYNC_POLL_ON_IRQ \
+	INTEL_SIP_SMC_ASYNC_VAL(INTEL_SIP_SMC_ASYNC_FUNC_ID_IRQ_POLL)
+
 /**
  * Request INTEL_SIP_SMC_ASYNC_HWMON_READTEMP
  * Async call to request temperature

Signature · 1 line

2.43.7

[email protected]

AI review

2026-08-13 06:51 UTC #5 lore raw

Re: [PATCH v3 4/4] firmware: stratix10-svc: Add for SDM mailbox doorbell interrupt

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] devm_request_any_context_irq() positive success values are incorrectly rejected by strict `ret ==…

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] devm_request_any_context_irq() positive success values are incorrectly rejected by strict ret == 0 check.
- [High] Dropping trx_list_lock before callback execution allows client stack popping, leading to UAF/stack corruption.
- [Medium] Improper handling of -EPROBE_DEFER from of_irq_get().
--

--- Patch [4]: [PATCH v3 4/4] firmware: stratix10-svc: Add for SDM mailbox doorbell interrupt --- commit 0c0971a9e6fe55c0bfa458e38981b259f988e3ae
Author: Mahesh Rao <[email protected]>

firmware: stratix10-svc: Add for SDM mailbox doorbell interrupt

Add support for SDM mailbox doorbell interrupt for async transactions. On interrupt, a workqueue is triggered which polls the ATF for pending responses and retrieves the bitmap of all retrieved and unprocessed transaction ids of mailbox responses from SDM. It then triggers the corresponding registered callbacks.

Quoted 4 lines

drivers/firmware/stratix10-svc.c +0 -0
index f8c2da207cb4e..8c86441eae58b 100644

[ ... ]

Quoted 4 lines

drivers/firmware/stratix10-svc.c +0 -0

@@ -1659,14 +1666,96 @@ static inline void stratix10_smc_1_2(struct stratix10_async_ctrl *actrl,
 	arm_smccc_1_2_smc(args, res);
 }

[ ... ]

Quoted 2 lines

drivers/firmware/stratix10-svc.c +2 -0

+static void stratix10_async_workqueue_handler(struct work_struct *work)
+{

[ ... ]

Quoted 11 lines

drivers/firmware/stratix10-svc.c +11 -0

+		do {
+			transaction_id = find_next_bit(pend_on_irq,
+						       TOTAL_TRANSACTION_IDS,
+						       transaction_id);
+			if (transaction_id >= TOTAL_TRANSACTION_IDS)
+				break;
+
+			cb = NULL;
+			cb_arg = NULL;
+			spin_lock(&actrl->trx_list_lock);
+			hash_for_each_possible(actrl->trx_list, handler,

drivers/firmware/stratix10-svc.c static inline void stratix10_smc_1_2(struct stratix10_async_ctrl *actrl, · line 1741

Quoted 10 lines

drivers/firmware/stratix10-svc.c +10 -0

+					       next, transaction_id) {
+				if (handler->transaction_id == transaction_id) {
+					cb = handler->cb;
+					cb_arg = handler->cb_arg;
+					break;
+				}
+			}
+			spin_unlock(&actrl->trx_list_lock);
+			if (cb)
+				cb(cb_arg);

[Severity: High]
Is there a risk of stack corruption or a use-after-free here?

By executing the callback after dropping trx_list_lock, a racing client timeout might execute stratix10_svc_async_done() and delete the handler.

This could result in popping the client stack before the workqueue executes the callback on the now-invalid stack address cb_arg. Should the lock be held during the callback or is there another way to ensure cb_arg remains valid?

Quoted 6 lines

drivers/firmware/stratix10-svc.c +6 -0

+
+			transaction_id++;
+		} while (transaction_id < TOTAL_TRANSACTION_IDS);
+	}
+	enable_irq(actrl->irq);
+}

[ ... ]

drivers/firmware/stratix10-svc.c static int stratix10_svc_async_init(struct stratix10_svc_controller *controller) · line 1820

Quoted 6 lines

drivers/firmware/stratix10-svc.c +2 -0

@@ -1727,6 +1816,22 @@ static int stratix10_svc_async_init(struct stratix10_svc_controller *controller)
 	hash_init(actrl->trx_list);
 	atomic_set(&actrl->common_achan_refcount, 0);
 
+	irq = of_irq_get(dev_of_node(dev), 0);
+	if (irq <= 0) {

[Severity: Medium]
Does this improperly swallow -EPROBE_DEFER?

Because of_irq_get() can return -EPROBE_DEFER as a negative value, this check will convert the probe deferral into a permanent fallback to polling mode. Should this propagate -EPROBE_DEFER instead?

drivers/firmware/stratix10-svc.c static int stratix10_svc_async_init(struct stratix10_svc_controller *controller) · line 1824

Quoted 5 lines

drivers/firmware/stratix10-svc.c +5 -0

+		dev_warn(dev, "Failed to get IRQ, falling back to polling mode\n");
+	} else {
+		ret = devm_request_any_context_irq(dev, irq, stratix10_svc_async_irq_handler,
+						   IRQF_NO_AUTOEN, "stratix10_svc", controller);
+		if (ret == 0) {

[Severity: High]
Will this correctly handle nested IRQs?

devm_request_any_context_irq() returns IRQC_IS_NESTED (which is 1) on success for nested IRQs.

By strictly checking for ret == 0, successful nested IRQ assignments will be rejected. This skips INIT_WORK and enable_irq(), leaving the registered IRQ permanently disabled and causing asynchronous transactions to time out. Should this check for ret >= 0 instead?

Quoted 8 lines

drivers/firmware/stratix10-svc.c +8 -0

+			dev_info(dev,
+				 "Registered IRQ %d for sip async operations\n",
+				 irq);
+			actrl->irq = irq;
+			INIT_WORK(&actrl->async_work, stratix10_async_workqueue_handler);
+			enable_irq(actrl->irq);
+		}
+	}

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4

NG, TZE YEE

author

2026-08-14 08:42 UTC #6 lore raw

Re: [PATCH v3 4/4] firmware: stratix10-svc: Add for SDM mailbox doorbell interrupt

Hi,

See my reply inline.

Quoted 58 lines · On 13/8/2026 2:51 pm, [email protected] wrote:

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] devm_request_any_context_irq() positive success values are incorrectly rejected by strict ret == 0 check.
- [High] Dropping trx_list_lock before callback execution allows client stack popping, leading to UAF/stack corruption.
- [Medium] Improper handling of -EPROBE_DEFER from of_irq_get().
--
--- Patch [4]: [PATCH v3 4/4] firmware: stratix10-svc: Add for SDM mailbox doorbell interrupt --- commit 0c0971a9e6fe55c0bfa458e38981b259f988e3ae
Author: Mahesh Rao <[email protected]>
firmware: stratix10-svc: Add for SDM mailbox doorbell interrupt
Add support for SDM mailbox doorbell interrupt for async transactions. On interrupt, a workqueue is triggered which polls the ATF for pending responses and retrieves the bitmap of all retrieved and unprocessed transaction ids of mailbox responses from SDM. It then triggers the corresponding registered callbacks.
Quoted 4 lines
drivers/firmware/stratix10-svc.c +0 -0
index f8c2da207cb4e..8c86441eae58b 100644
[ ... ]
Quoted 4 lines
drivers/firmware/stratix10-svc.c +0 -0
@@ -1659,14 +1666,96 @@ static inline void stratix10_smc_1_2(struct stratix10_async_ctrl *actrl,
  	arm_smccc_1_2_smc(args, res);
  }
 
[ ... ]
Quoted 2 lines
drivers/firmware/stratix10-svc.c +2 -0
+static void stratix10_async_workqueue_handler(struct work_struct *work)
+{
[ ... ]
Quoted 11 lines
drivers/firmware/stratix10-svc.c +11 -0
+		do {
+			transaction_id = find_next_bit(pend_on_irq,
+						       TOTAL_TRANSACTION_IDS,
+						       transaction_id);
+			if (transaction_id >= TOTAL_TRANSACTION_IDS)
+				break;
+
+			cb = NULL;
+			cb_arg = NULL;
+			spin_lock(&actrl->trx_list_lock);
+			hash_for_each_possible(actrl->trx_list, handler,
drivers/firmware/stratix10-svc.c static inline void stratix10_smc_1_2(struct stratix10_async_ctrl *actrl, · line 1741
Quoted 10 lines
drivers/firmware/stratix10-svc.c +10 -0
+					       next, transaction_id) {
+				if (handler->transaction_id == transaction_id) {
+					cb = handler->cb;
+					cb_arg = handler->cb_arg;
+					break;
+				}
+			}
+			spin_unlock(&actrl->trx_list_lock);
+			if (cb)
+				cb(cb_arg);
[Severity: High]
Is there a risk of stack corruption or a use-after-free here?

Quoted 6 lines

By executing the callback after dropping trx_list_lock, a racing client timeout might execute stratix10_svc_async_done() and delete the handler.
This could result in popping the client stack before the workqueue executes the callback on the now-invalid stack address cb_arg. Should the lock be held during the callback or is there another way to ensure cb_arg remains valid?

Agreed. We copy cb/cb_arg under the lock (so the handler itself is safe), but a client that times out and calls stratix10_svc_async_done() on another CPU can still free cb_arg before the callback runs.

Since callbacks run in BH context (system_bh_wq) and are already atomic, we can simply invoke the callback under the lock so async_done() waits for it to finish:

scoped_guard(spinlock, &actrl->trx_list_lock) {
	hash_for_each_possible(actrl->trx_list, handler,
		next, transaction_id) {
	if (handler->transaction_id == transaction_id) {
		if (handler->cb)
			handler->cb(handler->cb_arg);
		break;
		}
	}
}

The only constraint is that a callback must not call back into async_send()/async_done() (same lock). Does this approach look good to you?

Quoted 20 lines

Quoted 7 lines
drivers/firmware/stratix10-svc.c +6 -0
+
+			transaction_id++;
+		} while (transaction_id < TOTAL_TRANSACTION_IDS);
+	}
+	enable_irq(actrl->irq);
+}
[ ... ]
drivers/firmware/stratix10-svc.c static int stratix10_svc_async_init(struct stratix10_svc_controller *controller) · line 1820
Quoted 6 lines
drivers/firmware/stratix10-svc.c +2 -0
@@ -1727,6 +1816,22 @@ static int stratix10_svc_async_init(struct stratix10_svc_controller *controller)
  	hash_init(actrl->trx_list);
  	atomic_set(&actrl->common_achan_refcount, 0);
 
+	irq = of_irq_get(dev_of_node(dev), 0);
+	if (irq <= 0) {
[Severity: Medium]
Does this improperly swallow -EPROBE_DEFER?

Quoted 4 lines

Because of_irq_get() can return -EPROBE_DEFER as a negative value, this check will convert the probe deferral into a permanent fallback to polling mode. Should this propagate -EPROBE_DEFER instead?

Agreed. The parent here is the GIC (initialized early), so it shouldn't defer in practice, but propagating it is more correct:

irq = of_irq_get(dev_of_node(dev), 0);

if (irq == -EPROBE_DEFER) {
	ida_destroy(&actrl->async_id_pool);
	actrl->invoke_fn = NULL;
	return -EPROBE_DEFER;
}
if (irq <= 0) {
	dev_warn(dev, "Failed to get IRQ, falling back to polling mode\n");
} else {
	...
}

Since the IRQ is optional (polling is a valid fallback), do you want us to propagate -EPROBE_DEFER, or just keep the polling fallback on any failure?

Quoted 9 lines

drivers/firmware/stratix10-svc.c static int stratix10_svc_async_init(struct stratix10_svc_controller *controller) · line 1824
Quoted 5 lines
drivers/firmware/stratix10-svc.c +5 -0
+		dev_warn(dev, "Failed to get IRQ, falling back to polling mode\n");
+	} else {
+		ret = devm_request_any_context_irq(dev, irq, stratix10_svc_async_irq_handler,
+						   IRQF_NO_AUTOEN, "stratix10_svc", controller);
+		if (ret == 0) {
[Severity: High]
Will this correctly handle nested IRQs?

Quoted 8 lines

devm_request_any_context_irq() returns IRQC_IS_NESTED (which is 1) on success for nested IRQs.
By strictly checking for ret == 0, successful nested IRQ assignments will be rejected. This skips INIT_WORK and enable_irq(), leaving the registered IRQ permanently disabled and causing asynchronous transactions to time out. Should this check for ret >= 0 instead?

devm_request_any_context_irq() returns 0 (hard IRQ) or 1 (nested) on success. On our devices the SDM doorbell is a normal GIC SPI (hard IRQ), so ret == 0 is always correct here and never nested. ret >= 0 would be the more idiomatic check.

Do you prefer we keep ret == 0, or switch to ret >= 0?

Thanks,
Tze Yee

Quoted 9 lines

Quoted 8 lines

drivers/firmware/stratix10-svc.c +8 -0

+			dev_info(dev,
+				 "Registered IRQ %d for sip async operations\n",
+				 irq);
+			actrl->irq = irq;
+			INIT_WORK(&actrl->async_work, stratix10_async_workqueue_handler);
+			enable_irq(actrl->irq);
+		}
+	}

A message that is not in the archive (sent off-list or not yet received): SJ0PR03MB5950C6799C40C5724B048197A1842@SJ0PR03MB5950.namprd03.prod.outlook.com

NG, TZE YEE

author

2026-09-21 06:40 UTC #8 lore raw

Quoted 59 lines · TZE YEE

*From:* NG, TZE YEE
*Sent:* Thursday, August 13, 2026 2:35 PM
*To:* Dinh Nguyen <[email protected]>; Rob Herring <[email protected]>; Krzysztof Kozlowski <[email protected]>; Conor Dooley <[email protected]>; Rao, Mahesh <[email protected]>; linux- [email protected] <[email protected]>; [email protected] <[email protected]> *Subject:* [PATCH v3 0/4] stratix10: Add Interrupt support for asynchronous communication with SDM
From: Tze Yee Ng <[email protected]>
This patch-set adds support for interrupt from Secure Device Manager (SDM) to Stratix10 SoC for asynchronous communication.
Following things are added in this patch-set:
- Add support for interrupt from SDM to Stratix10 SoC.
- Add documentation for SDM interrupt in Stratix10 SoC.
- Enable SDM interrupt in Stratix10 SoC.
- Enable SDM interrupt in Agilex SoC.
I am taking over this series from Mahesh Rao and posting v3 on his behalf.
Reference:
https://lore.kernel.org/r/20250812-sip_svc_irq- [email protected] <https:// nam10.safelinks.protection.outlook.com/? url=https%3A%2F%2Flore.kernel.org%2Fr%2F20250812-sip_svc_irq- v2-0-53098e11705a%40altera.com&data=05%7C02%7Ctze.yee.ng%40altera.com%7Ceafe6fcdec264d130fd408df17aad714%7Cfbd72e03d4a54110adce614d51f2077a%7C0%7C0%7C639255694657134969%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=hOumheIvzlbZt1UApZy2F9CCpJFiH25iWSaYeCcdjKo%3D&reserved=0>
Changes in v3:
- Reflow commit message body to ~75 columns (fix Dinh's comment in v2
review)
- Add a newline before stratix10_async_workqueue_handler
description (fix Dinh's comment in v2 review)
- Treat of_irq_get() failure as irq <= 0, since 0 is also a
failing value (fix Dinh's comment in v2 review)
- Fix IRQ disable race: only call disable_irq_nosync() when
queue_work() newly queues work, and disable the fired irq
argument rather than actrl->irq
- Do not invoke client callbacks under trx_list_lock; copy
cb/cb_arg under the lock and call the callback after unlock
- Use disable_irq() + flush_work() + devm_free_irq() on
teardown, and treat unset IRQ as 0
- Demote successful IRQ registration log from dev_alert() to
dev_info(), and demote async poll failure log from
dev_err() to dev_dbg()
- Clarify INTEL_SIP_SMC_ASYNC_POLL_ON_IRQ kdoc to match
a1-a4 bitmap usage
- Drop Reviewed-by: Matthew Gerlach from all patches
(no longer at Altera)
- Link to RESEND v2:
https://lore.kernel.org/r/20250812-sip_svc_irq- [email protected] <https:// nam10.safelinks.protection.outlook.com/? url=https%3A%2F%2Flore.kernel.org%2Fr%2F20250812-sip_svc_irq- v2-0-53098e11705a%40altera.com&data=05%7C02%7Ctze.yee.ng%40altera.com%7Ceafe6fcdec264d130fd408df17aad714%7Cfbd72e03d4a54110adce614d51f2077a%7C0%7C0%7C639255694657165509%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=uDzBqNLWQSLSTmXyM72mbyHy5JfchKhk9a6xhrFF5pc%3D&reserved=0>
Changes in v2:
- Separated from the larger SIP SVC upstream series into a
dedicated interrupt-support series as requested
- Link to v1:
https://lore.kernel.org/r/20250722-sip_svc_irq-v1-0- [email protected] <https:// nam10.safelinks.protection.outlook.com/? url=https%3A%2F%2Flore.kernel.org%2Fr%2F20250722-sip_svc_irq-v1-0- c5b9e02c1ce6%40altera.com&data=05%7C02%7Ctze.yee.ng%40altera.com%7Ceafe6fcdec264d130fd408df17aad714%7Cfbd72e03d4a54110adce614d51f2077a%7C0%7C0%7C639255694657185496%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=dSLMGTCQPtY6nHXWwKPeen2XIuPzs1vZ6NKBcMYVwEA%3D&reserved=0>
Mahesh Rao (4):
dt-bindings: firmware: Add interrupt specification for Intel Stratix 10 Service Layer.
dts: stratix10: Add support for SDM mailbox interrupt for Intel Stratix10 SoC FPGA.
dts: agilex: Add support for SDM mailbox interrupt for Intel Agilex SoC FPGA.
firmware: stratix10-svc: Add for SDM mailbox doorbell interrupt

Quoted 10 lines

  .../firmware/intel,stratix10-svc.yaml         |  10 ++
  .../boot/dts/altera/socfpga_stratix10.dtsi    |   4 +
  arch/arm64/boot/dts/intel/socfpga_agilex.dtsi |   2 +
  drivers/firmware/stratix10-svc.c              | 135 ++++++++++++++++--
  include/linux/firmware/intel/stratix10-smc.h  |  21 +++
  5 files changed, 161 insertions(+), 11 deletions(-)

Signature · 1 line

2.43.7

Hi,

Gentle ping on this patch. It has been over a month with no feedback.

Happy to address any review comments.

Thanks,
Tze Yee

Dinh Nguyen

maintainer

2026-09-21 19:34 UTC #9 lore raw

Quoted 67 lines · TZE YEE

Quoted 66 lines · TZE YEE
*From:* NG, TZE YEE
*Sent:* Thursday, August 13, 2026 2:35 PM
*To:* Dinh Nguyen <[email protected]>; Rob Herring <[email protected]>; Krzysztof Kozlowski <[email protected]>; Conor Dooley <[email protected]>; Rao, Mahesh <[email protected]>; linux- [email protected] <[email protected]>; [email protected] <[email protected]> *Subject:* [PATCH v3 0/4] stratix10: Add Interrupt support for asynchronous communication with SDM
From: Tze Yee Ng <[email protected]>
This patch-set adds support for interrupt from Secure Device Manager (SDM) to Stratix10 SoC for asynchronous communication.
Following things are added in this patch-set:
- Add support for interrupt from SDM to Stratix10 SoC.
- Add documentation for SDM interrupt in Stratix10 SoC.
- Enable SDM interrupt in Stratix10 SoC.
- Enable SDM interrupt in Agilex SoC.
I am taking over this series from Mahesh Rao and posting v3 on his behalf.
Reference:
https://lore.kernel.org/r/20250812-sip_svc_irq- [email protected] <https:// nam10.safelinks.protection.outlook.com/? url=https%3A%2F%2Flore.kernel.org%2Fr%2F20250812-sip_svc_irq- v2-0-53098e11705a%40altera.com&data=05%7C02%7Ctze.yee.ng%40altera.com%7Ceafe6fcdec264d130fd408df17aad714%7Cfbd72e03d4a54110adce614d51f2077a%7C0%7C0%7C639255694657134969%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=hOumheIvzlbZt1UApZy2F9CCpJFiH25iWSaYeCcdjKo%3D&reserved=0>
Changes in v3:
- Reflow commit message body to ~75 columns (fix Dinh's comment in v2
   review)
- Add a newline before stratix10_async_workqueue_handler
   description (fix Dinh's comment in v2 review)
- Treat of_irq_get() failure as irq <= 0, since 0 is also a
   failing value (fix Dinh's comment in v2 review)
- Fix IRQ disable race: only call disable_irq_nosync() when
   queue_work() newly queues work, and disable the fired irq
   argument rather than actrl->irq
- Do not invoke client callbacks under trx_list_lock; copy
   cb/cb_arg under the lock and call the callback after unlock
- Use disable_irq() + flush_work() + devm_free_irq() on
   teardown, and treat unset IRQ as 0
- Demote successful IRQ registration log from dev_alert() to
   dev_info(), and demote async poll failure log from
   dev_err() to dev_dbg()
- Clarify INTEL_SIP_SMC_ASYNC_POLL_ON_IRQ kdoc to match
   a1-a4 bitmap usage
- Drop Reviewed-by: Matthew Gerlach from all patches
   (no longer at Altera)
- Link to RESEND v2:
https://lore.kernel.org/r/20250812-sip_svc_irq- [email protected] <https:// nam10.safelinks.protection.outlook.com/? url=https%3A%2F%2Flore.kernel.org%2Fr%2F20250812-sip_svc_irq- v2-0-53098e11705a%40altera.com&data=05%7C02%7Ctze.yee.ng%40altera.com%7Ceafe6fcdec264d130fd408df17aad714%7Cfbd72e03d4a54110adce614d51f2077a%7C0%7C0%7C639255694657165509%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=uDzBqNLWQSLSTmXyM72mbyHy5JfchKhk9a6xhrFF5pc%3D&reserved=0>
Changes in v2:
- Separated from the larger SIP SVC upstream series into a
   dedicated interrupt-support series as requested
- Link to v1:
https://lore.kernel.org/r/20250722-sip_svc_irq-v1-0- [email protected] <https:// nam10.safelinks.protection.outlook.com/? url=https%3A%2F%2Flore.kernel.org%2Fr%2F20250722-sip_svc_irq-v1-0- c5b9e02c1ce6%40altera.com&data=05%7C02%7Ctze.yee.ng%40altera.com%7Ceafe6fcdec264d130fd408df17aad714%7Cfbd72e03d4a54110adce614d51f2077a%7C0%7C0%7C639255694657185496%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=dSLMGTCQPtY6nHXWwKPeen2XIuPzs1vZ6NKBcMYVwEA%3D&reserved=0>
Mahesh Rao (4):
   dt-bindings: firmware: Add interrupt specification for Intel Stratix 10 Service Layer.
   dts: stratix10: Add support for SDM mailbox interrupt for Intel Stratix10 SoC FPGA.
   dts: agilex: Add support for SDM mailbox interrupt for Intel Agilex SoC FPGA.
   firmware: stratix10-svc: Add for SDM mailbox doorbell interrupt
  .../firmware/intel,stratix10-svc.yaml         | 10 ++ .../boot/dts/altera/socfpga_stratix10.dtsi    |   4 + arch/arm64/boot/dts/intel/socfpga_agilex.dtsi |   2 + drivers/firmware/stratix10-svc.c              | 135 ++++++++++++++++-- include/linux/firmware/intel/stratix10-smc.h | 21 +++
  5 files changed, 161 insertions(+), 11 deletions(-)

Quoted 8 lines

Quoted 3 lines
Signature · 1 line
2.43.7
Hi,
Gentle ping on this patch. It has been over a month with no feedback.

Thanks for the ping. All patches applied!

Dinh