drivers/firmware/stratix10-svc.c | 225 ++++++++++-------- .../firmware/intel/stratix10-svc-client.h | 8 +- 2 files changed, 128 insertions(+), 105 deletions(-)
In the current implementation, SVC client drivers such as socfpga-hwmon,
intel_fcs, stratix10-soc, stratix10-rsu each send an SMC command that
triggers a single thread in the stratix10-svc driver. Upon receiving a
callback, the initiating client driver sends a stratix10-svc-done signal,
terminating the thread without waiting for other pending SMC commands to
complete. This leads to a timeout issue in the firmware SVC mailbox service
when multiple client drivers send SMC commands concurrently.
To resolve this issue, a dedicated thread is now created per channel. The
stratix10-svc driver will support up to the number of channels defined by
SVC_NUM_CHANNEL. Thread synchronization is handled using a mutex to prevent
simultaneous issuance of SMC commands by multiple threads.
Additionally, a thread task is now validated before invoking kthread_stop
when the user aborts, ensuring safe termination.
Timeout values have also been adjusted to accommodate the increased load
from concurrent client driver activity.
Fixes: 7ca5ce896524 ("firmware: add Intel Stratix10 service layer driver")
Cc: stable@vger.kernel.org
Signed-off-by: Ang Tien Sung <tien.sung.ang@altera.com>
Signed-off-by: Fong, Yan Kei <yankei.fong@altera.com>
Signed-off-by: Muhammad Amirul Asyraf Mohamad Jamian <muhammad.amirul.asyraf.mohamad.jamian@altera.com>
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202602032325.MuglZ156-lkp@intel.com/
---
Changes in v5:
- Fix mutex locking: replace guard(mutex) with explicit lock/unlock,
use mutex_lock_interruptible() so kthread_stop() can unblock the
thread when the SDM lock is contended, fix lock/unlock imbalances,
and on interrupted lock notify client with SVC_STATUS_ERROR so it
does not block on a response that never arrives
- Fix TOCTOU race in stratix10_svc_send(): protect chan->task creation
with chan->lock spinlock to prevent concurrent callers on the same
channel from spawning duplicate threads
- Fix probe error path: initialize controller->node with INIT_LIST_HEAD
and guard list_del() so it is only called when list_add_tail() was
reached, preventing NULL deref on early FIFO allocation failure
- Remove stray platform_device_unregister(svc->intel_svc_fcs) call in
remove function, as FCS device is not manually registered in probe;
also remove the now-unused INTEL_FCS macro, stale intel_svc_fcs field
from struct stratix10_svc, and stale @task kdoc from
struct stratix10_svc_controller
- Remove unnecessary NULL check on svc pointer in err_put_device
- Clean up: convert FIFO allocations to for loop, simplify
stratix10_svc_done() to a single debug message, fix whitespace,
typos and log messages
Changes in v4:
- Signed-off-by chain clean up
- Remove duplicate inline comment on sdm_lock field
- Move structure field comments to structure header
- Remove unnecessary checks in stratix10_svc_done()
- Fix error handling to properly free FIFOs on probe failure
- Reorder error labels for proper cleanup cascade
- Remove redundant fifo_size initialization
Changes in v3:
- Fix grammar: "will supports" -> "will support"
- Add Fixes tag and Cc stable as suggested by reviewer
Changes in v2:
- Initialize 'svc' to NULL and add NULL check in error path to fix uninitialized variable warning
---
---
drivers/firmware/stratix10-svc.c | 225 ++++++++++--------
.../firmware/intel/stratix10-svc-client.h | 8 +-
2 files changed, 128 insertions(+), 105 deletions(-)
diff --git a/drivers/firmware/stratix10-svc.c b/drivers/firmware/stratix10-svc.c
index 6f5c298582ab..2af95018f06d 100644
--- a/drivers/firmware/stratix10-svc.c
+++ b/drivers/firmware/stratix10-svc.c
@@ -37,15 +37,14 @@
* service layer will return error to FPGA manager when timeout occurs,
* timeout is set to 30 seconds (30 * 1000) at Intel Stratix10 SoC.
*/
-#define SVC_NUM_DATA_IN_FIFO 32
+#define SVC_NUM_DATA_IN_FIFO 8
#define SVC_NUM_CHANNEL 4
-#define FPGA_CONFIG_DATA_CLAIM_TIMEOUT_MS 200
+#define FPGA_CONFIG_DATA_CLAIM_TIMEOUT_MS 2000
#define FPGA_CONFIG_STATUS_TIMEOUT_SEC 30
#define BYTE_TO_WORD_SIZE 4
/* stratix10 service layer clients */
#define STRATIX10_RSU "stratix10-rsu"
-#define INTEL_FCS "intel-fcs"
/* Maximum number of SDM client IDs. */
#define MAX_SDM_CLIENT_IDS 16
@@ -105,11 +104,9 @@ struct stratix10_svc_chan;
/**
* struct stratix10_svc - svc private data
* @stratix10_svc_rsu: pointer to stratix10 RSU device
- * @intel_svc_fcs: pointer to the FCS device
*/
struct stratix10_svc {
struct platform_device *stratix10_svc_rsu;
- struct platform_device *intel_svc_fcs;
};
/**
@@ -251,12 +248,10 @@ struct stratix10_async_ctrl {
* @num_active_client: number of active service client
* @node: list management
* @genpool: memory pool pointing to the memory region
- * @task: pointer to the thread task which handles SMC or HVC call
- * @svc_fifo: a queue for storing service message data
* @complete_status: state for completion
- * @svc_fifo_lock: protect access to service message data queue
* @invoke_fn: function to issue secure monitor call or hypervisor call
* @svc: manages the list of client svc drivers
+ * @sdm_lock: only allows a single command single response to SDM
* @actrl: async control structure
*
* This struct is used to create communication channels for service clients, to
@@ -269,12 +264,10 @@ struct stratix10_svc_controller {
int num_active_client;
struct list_head node;
struct gen_pool *genpool;
- struct task_struct *task;
- struct kfifo svc_fifo;
struct completion complete_status;
- spinlock_t svc_fifo_lock;
svc_invoke_fn *invoke_fn;
struct stratix10_svc *svc;
+ struct mutex *sdm_lock;
struct stratix10_async_ctrl actrl;
};
@@ -283,6 +276,9 @@ struct stratix10_svc_controller {
* @ctrl: pointer to service controller which is the provider of this channel
* @scl: pointer to service client which owns the channel
* @name: service client name associated with the channel
+ * @task: pointer to the thread task which handles SMC or HVC call
+ * @svc_fifo: a queue for storing service message data (separate fifo for every channel)
+ * @svc_fifo_lock: protect access to service message data queue (locking pending fifo)
* @lock: protect access to the channel
* @async_chan: reference to asynchronous channel object for this channel
*
@@ -293,6 +289,9 @@ struct stratix10_svc_chan {
struct stratix10_svc_controller *ctrl;
struct stratix10_svc_client *scl;
char *name;
+ struct task_struct *task;
+ struct kfifo svc_fifo;
+ spinlock_t svc_fifo_lock;
spinlock_t lock;
struct stratix10_async_chan *async_chan;
};
@@ -527,10 +526,10 @@ static void svc_thread_recv_status_ok(struct stratix10_svc_data *p_data,
*/
static int svc_normal_to_secure_thread(void *data)
{
- struct stratix10_svc_controller
- *ctrl = (struct stratix10_svc_controller *)data;
- struct stratix10_svc_data *pdata;
- struct stratix10_svc_cb_data *cbdata;
+ struct stratix10_svc_chan *chan = (struct stratix10_svc_chan *)data;
+ struct stratix10_svc_controller *ctrl = chan->ctrl;
+ struct stratix10_svc_data *pdata = NULL;
+ struct stratix10_svc_cb_data *cbdata = NULL;
struct arm_smccc_res res;
unsigned long a0, a1, a2, a3, a4, a5, a6, a7;
int ret_fifo = 0;
@@ -555,12 +554,12 @@ static int svc_normal_to_secure_thread(void *data)
a6 = 0;
a7 = 0;
- pr_debug("smc_hvc_shm_thread is running\n");
+ pr_debug("%s: %s: Thread is running!\n", __func__, chan->name);
while (!kthread_should_stop()) {
- ret_fifo = kfifo_out_spinlocked(&ctrl->svc_fifo,
+ ret_fifo = kfifo_out_spinlocked(&chan->svc_fifo,
pdata, sizeof(*pdata),
- &ctrl->svc_fifo_lock);
+ &chan->svc_fifo_lock);
if (!ret_fifo)
continue;
@@ -569,9 +568,25 @@ static int svc_normal_to_secure_thread(void *data)
(unsigned int)pdata->paddr, pdata->command,
(unsigned int)pdata->size);
+ /* SDM can only process one command at a time */
+ pr_debug("%s: %s: Thread is waiting for mutex!\n",
+ __func__, chan->name);
+ if (mutex_lock_interruptible(ctrl->sdm_lock)) {
+ /* item already dequeued; notify client to unblock it */
+ cbdata->status = BIT(SVC_STATUS_ERROR);
+ cbdata->kaddr1 = NULL;
+ cbdata->kaddr2 = NULL;
+ cbdata->kaddr3 = NULL;
+ if (pdata->chan->scl)
+ pdata->chan->scl->receive_cb(pdata->chan->scl,
+ cbdata);
+ break;
+ }
+
switch (pdata->command) {
case COMMAND_RECONFIG_DATA_CLAIM:
svc_thread_cmd_data_claim(ctrl, pdata, cbdata);
+ mutex_unlock(ctrl->sdm_lock);
continue;
case COMMAND_RECONFIG:
a0 = INTEL_SIP_SMC_FPGA_CONFIG_START;
@@ -700,10 +715,11 @@ static int svc_normal_to_secure_thread(void *data)
break;
default:
pr_warn("it shouldn't happen\n");
- break;
+ mutex_unlock(ctrl->sdm_lock);
+ continue;
}
- pr_debug("%s: before SMC call -- a0=0x%016x a1=0x%016x",
- __func__,
+ pr_debug("%s: %s: before SMC call -- a0=0x%016x a1=0x%016x",
+ __func__, chan->name,
(unsigned int)a0,
(unsigned int)a1);
pr_debug(" a2=0x%016x\n", (unsigned int)a2);
@@ -712,8 +728,8 @@ static int svc_normal_to_secure_thread(void *data)
pr_debug(" a5=0x%016x\n", (unsigned int)a5);
ctrl->invoke_fn(a0, a1, a2, a3, a4, a5, a6, a7, &res);
- pr_debug("%s: after SMC call -- res.a0=0x%016x",
- __func__, (unsigned int)res.a0);
+ pr_debug("%s: %s: after SMC call -- res.a0=0x%016x",
+ __func__, chan->name, (unsigned int)res.a0);
pr_debug(" res.a1=0x%016x, res.a2=0x%016x",
(unsigned int)res.a1, (unsigned int)res.a2);
pr_debug(" res.a3=0x%016x\n", (unsigned int)res.a3);
@@ -728,6 +744,7 @@ static int svc_normal_to_secure_thread(void *data)
cbdata->kaddr2 = NULL;
cbdata->kaddr3 = NULL;
pdata->chan->scl->receive_cb(pdata->chan->scl, cbdata);
+ mutex_unlock(ctrl->sdm_lock);
continue;
}
@@ -801,6 +818,8 @@ static int svc_normal_to_secure_thread(void *data)
break;
}
+
+ mutex_unlock(ctrl->sdm_lock);
}
kfree(cbdata);
@@ -1697,21 +1716,25 @@ int stratix10_svc_send(struct stratix10_svc_chan *chan, void *msg)
return -ENOMEM;
/* first client will create kernel thread */
- if (!chan->ctrl->task) {
- chan->ctrl->task =
+ spin_lock(&chan->lock);
+ if (!chan->task) {
+ chan->task =
kthread_run_on_cpu(svc_normal_to_secure_thread,
- (void *)chan->ctrl,
+ (void *)chan,
cpu, "svc_smc_hvc_thread");
- if (IS_ERR(chan->ctrl->task)) {
+ if (IS_ERR(chan->task)) {
+ chan->task = NULL;
+ spin_unlock(&chan->lock);
dev_err(chan->ctrl->dev,
"failed to create svc_smc_hvc_thread\n");
kfree(p_data);
return -EINVAL;
}
}
+ spin_unlock(&chan->lock);
- pr_debug("%s: sent P-va=%p, P-com=%x, P-size=%u\n", __func__,
- p_msg->payload, p_msg->command,
+ pr_debug("%s: %s: sent P-va=%p, P-com=%x, P-size=%u\n", __func__,
+ chan->name, p_msg->payload, p_msg->command,
(unsigned int)p_msg->payload_length);
if (list_empty(&svc_data_mem)) {
@@ -1747,12 +1770,16 @@ int stratix10_svc_send(struct stratix10_svc_chan *chan, void *msg)
p_data->arg[2] = p_msg->arg[2];
p_data->size = p_msg->payload_length;
p_data->chan = chan;
- pr_debug("%s: put to FIFO pa=0x%016x, cmd=%x, size=%u\n", __func__,
- (unsigned int)p_data->paddr, p_data->command,
- (unsigned int)p_data->size);
- ret = kfifo_in_spinlocked(&chan->ctrl->svc_fifo, p_data,
+ pr_debug("%s: %s: put to FIFO pa=0x%016x, cmd=%x, size=%u\n",
+ __func__,
+ chan->name,
+ (unsigned int)p_data->paddr,
+ p_data->command,
+ (unsigned int)p_data->size);
+
+ ret = kfifo_in_spinlocked(&chan->svc_fifo, p_data,
sizeof(*p_data),
- &chan->ctrl->svc_fifo_lock);
+ &chan->svc_fifo_lock);
kfree(p_data);
@@ -1773,11 +1800,12 @@ EXPORT_SYMBOL_GPL(stratix10_svc_send);
*/
void stratix10_svc_done(struct stratix10_svc_chan *chan)
{
- /* stop thread when thread is running AND only one active client */
- if (chan->ctrl->task && chan->ctrl->num_active_client <= 1) {
- pr_debug("svc_smc_hvc_shm_thread is stopped\n");
- kthread_stop(chan->ctrl->task);
- chan->ctrl->task = NULL;
+ /* stop thread when thread is running */
+ if (chan->task) {
+ pr_debug("%s: %s: svc_smc_hvc_shm_thread is stopping\n",
+ __func__, chan->name);
+ kthread_stop(chan->task);
+ chan->task = NULL;
}
}
EXPORT_SYMBOL_GPL(stratix10_svc_done);
@@ -1817,8 +1845,8 @@ void *stratix10_svc_allocate_memory(struct stratix10_svc_chan *chan,
pmem->paddr = pa;
pmem->size = s;
list_add_tail(&pmem->node, &svc_data_mem);
- pr_debug("%s: va=%p, pa=0x%016x\n", __func__,
- pmem->vaddr, (unsigned int)pmem->paddr);
+ pr_debug("%s: %s: va=%p, pa=0x%016x\n", __func__,
+ chan->name, pmem->vaddr, (unsigned int)pmem->paddr);
return (void *)va;
}
@@ -1855,6 +1883,18 @@ static const struct of_device_id stratix10_svc_drv_match[] = {
{},
};
+/**
+ * mailbox_lock protects access to the SDM if thread is busy
+ */
+static DEFINE_MUTEX(mailbox_lock);
+
+static const char * const chan_names[SVC_NUM_CHANNEL] = {
+ SVC_CLIENT_FPGA,
+ SVC_CLIENT_RSU,
+ SVC_CLIENT_FCS,
+ SVC_CLIENT_HWMON
+};
+
static int stratix10_svc_drv_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -1862,11 +1902,11 @@ static int stratix10_svc_drv_probe(struct platform_device *pdev)
struct stratix10_svc_chan *chans;
struct gen_pool *genpool;
struct stratix10_svc_sh_memory *sh_memory;
- struct stratix10_svc *svc;
+ struct stratix10_svc *svc = NULL;
svc_invoke_fn *invoke_fn;
size_t fifo_size;
- int ret;
+ int ret, i = 0;
/* get SMC or HVC function */
invoke_fn = get_invoke_func(dev);
@@ -1905,8 +1945,8 @@ static int stratix10_svc_drv_probe(struct platform_device *pdev)
controller->num_active_client = 0;
controller->chans = chans;
controller->genpool = genpool;
- controller->task = NULL;
controller->invoke_fn = invoke_fn;
+ INIT_LIST_HEAD(&controller->node);
init_completion(&controller->complete_status);
ret = stratix10_svc_async_init(controller);
@@ -1917,32 +1957,24 @@ static int stratix10_svc_drv_probe(struct platform_device *pdev)
}
fifo_size = sizeof(struct stratix10_svc_data) * SVC_NUM_DATA_IN_FIFO;
- ret = kfifo_alloc(&controller->svc_fifo, fifo_size, GFP_KERNEL);
- if (ret) {
- dev_err(dev, "failed to allocate FIFO\n");
- goto err_async_exit;
- }
- spin_lock_init(&controller->svc_fifo_lock);
-
- chans[0].scl = NULL;
- chans[0].ctrl = controller;
- chans[0].name = SVC_CLIENT_FPGA;
- spin_lock_init(&chans[0].lock);
-
- chans[1].scl = NULL;
- chans[1].ctrl = controller;
- chans[1].name = SVC_CLIENT_RSU;
- spin_lock_init(&chans[1].lock);
-
- chans[2].scl = NULL;
- chans[2].ctrl = controller;
- chans[2].name = SVC_CLIENT_FCS;
- spin_lock_init(&chans[2].lock);
+ /*
+ * This mutex is used to block threads from utilizing
+ * SDM to prevent out of order command tx
+ */
+ controller->sdm_lock = &mailbox_lock;
- chans[3].scl = NULL;
- chans[3].ctrl = controller;
- chans[3].name = SVC_CLIENT_HWMON;
- spin_lock_init(&chans[3].lock);
+ for (i = 0; i < SVC_NUM_CHANNEL; i++) {
+ chans[i].scl = NULL;
+ chans[i].ctrl = controller;
+ chans[i].name = (char *)chan_names[i];
+ spin_lock_init(&chans[i].lock);
+ ret = kfifo_alloc(&chans[i].svc_fifo, fifo_size, GFP_KERNEL);
+ if (ret) {
+ dev_err(dev, "failed to allocate FIFO %d\n", i);
+ goto err_free_fifos;
+ }
+ spin_lock_init(&chans[i].svc_fifo_lock);
+ }
list_add_tail(&controller->node, &svc_ctrl);
platform_set_drvdata(pdev, controller);
@@ -1951,7 +1983,7 @@ static int stratix10_svc_drv_probe(struct platform_device *pdev)
svc = devm_kzalloc(dev, sizeof(*svc), GFP_KERNEL);
if (!svc) {
ret = -ENOMEM;
- goto err_free_kfifo;
+ goto err_free_fifos;
}
controller->svc = svc;
@@ -1959,51 +1991,40 @@ static int stratix10_svc_drv_probe(struct platform_device *pdev)
if (!svc->stratix10_svc_rsu) {
dev_err(dev, "failed to allocate %s device\n", STRATIX10_RSU);
ret = -ENOMEM;
- goto err_free_kfifo;
+ goto err_free_fifos;
}
ret = platform_device_add(svc->stratix10_svc_rsu);
- if (ret) {
- platform_device_put(svc->stratix10_svc_rsu);
- goto err_free_kfifo;
- }
-
- svc->intel_svc_fcs = platform_device_alloc(INTEL_FCS, 1);
- if (!svc->intel_svc_fcs) {
- dev_err(dev, "failed to allocate %s device\n", INTEL_FCS);
- ret = -ENOMEM;
- goto err_unregister_rsu_dev;
- }
-
- ret = platform_device_add(svc->intel_svc_fcs);
- if (ret) {
- platform_device_put(svc->intel_svc_fcs);
- goto err_unregister_rsu_dev;
- }
+ if (ret)
+ goto err_put_device;
ret = of_platform_default_populate(dev_of_node(dev), NULL, dev);
if (ret)
- goto err_unregister_fcs_dev;
+ goto err_put_device;
pr_info("Intel Service Layer Driver Initialized\n");
return 0;
-err_unregister_fcs_dev:
- platform_device_unregister(svc->intel_svc_fcs);
-err_unregister_rsu_dev:
- platform_device_unregister(svc->stratix10_svc_rsu);
-err_free_kfifo:
- kfifo_free(&controller->svc_fifo);
-err_async_exit:
+err_put_device:
+ platform_device_put(svc->stratix10_svc_rsu);
+err_free_fifos:
+ /* only remove from list if list_add_tail() was reached */
+ if (!list_empty(&controller->node))
+ list_del(&controller->node);
+ /* free only the FIFOs that were successfully allocated */
+ while (i--)
+ kfifo_free(&chans[i].svc_fifo);
stratix10_svc_async_exit(controller);
err_destroy_pool:
gen_pool_destroy(genpool);
+
return ret;
}
static void stratix10_svc_drv_remove(struct platform_device *pdev)
{
+ int i;
struct stratix10_svc_controller *ctrl = platform_get_drvdata(pdev);
struct stratix10_svc *svc = ctrl->svc;
@@ -2011,14 +2032,16 @@ static void stratix10_svc_drv_remove(struct platform_device *pdev)
of_platform_depopulate(ctrl->dev);
- platform_device_unregister(svc->intel_svc_fcs);
platform_device_unregister(svc->stratix10_svc_rsu);
- kfifo_free(&ctrl->svc_fifo);
- if (ctrl->task) {
- kthread_stop(ctrl->task);
- ctrl->task = NULL;
+ for (i = 0; i < SVC_NUM_CHANNEL; i++) {
+ if (ctrl->chans[i].task) {
+ kthread_stop(ctrl->chans[i].task);
+ ctrl->chans[i].task = NULL;
+ }
+ kfifo_free(&ctrl->chans[i].svc_fifo);
}
+
if (ctrl->genpool)
gen_pool_destroy(ctrl->genpool);
list_del(&ctrl->node);
diff --git a/include/linux/firmware/intel/stratix10-svc-client.h b/include/linux/firmware/intel/stratix10-svc-client.h
index d290060f4c73..91013161e9db 100644
--- a/include/linux/firmware/intel/stratix10-svc-client.h
+++ b/include/linux/firmware/intel/stratix10-svc-client.h
@@ -68,12 +68,12 @@
* timeout value used in Stratix10 FPGA manager driver.
* timeout value used in RSU driver
*/
-#define SVC_RECONFIG_REQUEST_TIMEOUT_MS 300
-#define SVC_RECONFIG_BUFFER_TIMEOUT_MS 720
-#define SVC_RSU_REQUEST_TIMEOUT_MS 300
+#define SVC_RECONFIG_REQUEST_TIMEOUT_MS 5000
+#define SVC_RECONFIG_BUFFER_TIMEOUT_MS 5000
+#define SVC_RSU_REQUEST_TIMEOUT_MS 2000
#define SVC_FCS_REQUEST_TIMEOUT_MS 2000
#define SVC_COMPLETED_TIMEOUT_MS 30000
-#define SVC_HWMON_REQUEST_TIMEOUT_MS 300
+#define SVC_HWMON_REQUEST_TIMEOUT_MS 2000
struct stratix10_svc_chan;
--
2.43.7
On 3/1/26 23:39, Muhammad Amirul Asyraf Mohamad Jamian wrote:
> In the current implementation, SVC client drivers such as socfpga-hwmon,
> intel_fcs, stratix10-soc, stratix10-rsu each send an SMC command that
> triggers a single thread in the stratix10-svc driver. Upon receiving a
> callback, the initiating client driver sends a stratix10-svc-done signal,
> terminating the thread without waiting for other pending SMC commands to
> complete. This leads to a timeout issue in the firmware SVC mailbox service
> when multiple client drivers send SMC commands concurrently.
>
> To resolve this issue, a dedicated thread is now created per channel. The
> stratix10-svc driver will support up to the number of channels defined by
> SVC_NUM_CHANNEL. Thread synchronization is handled using a mutex to prevent
> simultaneous issuance of SMC commands by multiple threads.
>
> Additionally, a thread task is now validated before invoking kthread_stop
> when the user aborts, ensuring safe termination.
>
> Timeout values have also been adjusted to accommodate the increased load
> from concurrent client driver activity.
>
> Fixes: 7ca5ce896524 ("firmware: add Intel Stratix10 service layer driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Ang Tien Sung <tien.sung.ang@altera.com>
> Signed-off-by: Fong, Yan Kei <yankei.fong@altera.com>
> Signed-off-by: Muhammad Amirul Asyraf Mohamad Jamian <muhammad.amirul.asyraf.mohamad.jamian@altera.com>
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202602032325.MuglZ156-lkp@intel.com/
> ---
> Changes in v5:
> - Fix mutex locking: replace guard(mutex) with explicit lock/unlock,
> use mutex_lock_interruptible() so kthread_stop() can unblock the
> thread when the SDM lock is contended, fix lock/unlock imbalances,
> and on interrupted lock notify client with SVC_STATUS_ERROR so it
> does not block on a response that never arrives
> - Fix TOCTOU race in stratix10_svc_send(): protect chan->task creation
> with chan->lock spinlock to prevent concurrent callers on the same
> channel from spawning duplicate threads
> - Fix probe error path: initialize controller->node with INIT_LIST_HEAD
> and guard list_del() so it is only called when list_add_tail() was
> reached, preventing NULL deref on early FIFO allocation failure
> - Remove stray platform_device_unregister(svc->intel_svc_fcs) call in
> remove function, as FCS device is not manually registered in probe;
> also remove the now-unused INTEL_FCS macro, stale intel_svc_fcs field
> from struct stratix10_svc, and stale @task kdoc from
> struct stratix10_svc_controller
> - Remove unnecessary NULL check on svc pointer in err_put_device
> - Clean up: convert FIFO allocations to for loop, simplify
> stratix10_svc_done() to a single debug message, fix whitespace,
> typos and log messages
>
> Changes in v4:
> - Signed-off-by chain clean up
> - Remove duplicate inline comment on sdm_lock field
> - Move structure field comments to structure header
> - Remove unnecessary checks in stratix10_svc_done()
> - Fix error handling to properly free FIFOs on probe failure
> - Reorder error labels for proper cleanup cascade
> - Remove redundant fifo_size initialization
>
> Changes in v3:
> - Fix grammar: "will supports" -> "will support"
> - Add Fixes tag and Cc stable as suggested by reviewer
>
> Changes in v2:
> - Initialize 'svc' to NULL and add NULL check in error path to fix uninitialized variable warning
> ---
> ---
> drivers/firmware/stratix10-svc.c | 225 ++++++++++--------
> .../firmware/intel/stratix10-svc-client.h | 8 +-
> 2 files changed, 128 insertions(+), 105 deletions(-)
>
> diff --git a/drivers/firmware/stratix10-svc.c b/drivers/firmware/stratix10-svc.c
> index 6f5c298582ab..2af95018f06d 100644
> --- a/drivers/firmware/stratix10-svc.c
> +++ b/drivers/firmware/stratix10-svc.c
> @@ -37,15 +37,14 @@
> * service layer will return error to FPGA manager when timeout occurs,
> * timeout is set to 30 seconds (30 * 1000) at Intel Stratix10 SoC.
> */
> -#define SVC_NUM_DATA_IN_FIFO 32
> +#define SVC_NUM_DATA_IN_FIFO 8
Your commit message does not explain this change. I don't see how this
change is related to this patch.
> #define SVC_NUM_CHANNEL 4
> -#define FPGA_CONFIG_DATA_CLAIM_TIMEOUT_MS 200
> +#define FPGA_CONFIG_DATA_CLAIM_TIMEOUT_MS 2000
> #define FPGA_CONFIG_STATUS_TIMEOUT_SEC 30
> #define BYTE_TO_WORD_SIZE 4
>
> /* stratix10 service layer clients */
> #define STRATIX10_RSU "stratix10-rsu"
> -#define INTEL_FCS "intel-fcs"
>
> /* Maximum number of SDM client IDs. */
> #define MAX_SDM_CLIENT_IDS 16
> @@ -105,11 +104,9 @@ struct stratix10_svc_chan;
> /**
> * struct stratix10_svc - svc private data
> * @stratix10_svc_rsu: pointer to stratix10 RSU device
> - * @intel_svc_fcs: pointer to the FCS device
> */
> struct stratix10_svc {
> struct platform_device *stratix10_svc_rsu;
> - struct platform_device *intel_svc_fcs;
> };
>
> /**
> @@ -251,12 +248,10 @@ struct stratix10_async_ctrl {
> * @num_active_client: number of active service client
> * @node: list management
> * @genpool: memory pool pointing to the memory region
> - * @task: pointer to the thread task which handles SMC or HVC call
> - * @svc_fifo: a queue for storing service message data
> * @complete_status: state for completion
> - * @svc_fifo_lock: protect access to service message data queue
> * @invoke_fn: function to issue secure monitor call or hypervisor call
> * @svc: manages the list of client svc drivers
> + * @sdm_lock: only allows a single command single response to SDM
> * @actrl: async control structure
> *
> * This struct is used to create communication channels for service clients, to
> @@ -269,12 +264,10 @@ struct stratix10_svc_controller {
> int num_active_client;
> struct list_head node;
> struct gen_pool *genpool;
> - struct task_struct *task;
> - struct kfifo svc_fifo;
> struct completion complete_status;
> - spinlock_t svc_fifo_lock;
> svc_invoke_fn *invoke_fn;
> struct stratix10_svc *svc;
> + struct mutex *sdm_lock;
> struct stratix10_async_ctrl actrl;
> };
>
> @@ -283,6 +276,9 @@ struct stratix10_svc_controller {
> * @ctrl: pointer to service controller which is the provider of this channel
> * @scl: pointer to service client which owns the channel
> * @name: service client name associated with the channel
> + * @task: pointer to the thread task which handles SMC or HVC call
> + * @svc_fifo: a queue for storing service message data (separate fifo for every channel)
> + * @svc_fifo_lock: protect access to service message data queue (locking pending fifo)
> * @lock: protect access to the channel
> * @async_chan: reference to asynchronous channel object for this channel
> *
> @@ -293,6 +289,9 @@ struct stratix10_svc_chan {
> struct stratix10_svc_controller *ctrl;
> struct stratix10_svc_client *scl;
> char *name;
> + struct task_struct *task;
> + struct kfifo svc_fifo;
> + spinlock_t svc_fifo_lock;
> spinlock_t lock;
> struct stratix10_async_chan *async_chan;
> };
> @@ -527,10 +526,10 @@ static void svc_thread_recv_status_ok(struct stratix10_svc_data *p_data,
> */
> static int svc_normal_to_secure_thread(void *data)
> {
> - struct stratix10_svc_controller
> - *ctrl = (struct stratix10_svc_controller *)data;
> - struct stratix10_svc_data *pdata;
> - struct stratix10_svc_cb_data *cbdata;
> + struct stratix10_svc_chan *chan = (struct stratix10_svc_chan *)data;
> + struct stratix10_svc_controller *ctrl = chan->ctrl;
> + struct stratix10_svc_data *pdata = NULL;
> + struct stratix10_svc_cb_data *cbdata = NULL;
> struct arm_smccc_res res;
> unsigned long a0, a1, a2, a3, a4, a5, a6, a7;
> int ret_fifo = 0;
> @@ -555,12 +554,12 @@ static int svc_normal_to_secure_thread(void *data)
> a6 = 0;
> a7 = 0;
>
> - pr_debug("smc_hvc_shm_thread is running\n");
> + pr_debug("%s: %s: Thread is running!\n", __func__, chan->name);
>
> while (!kthread_should_stop()) {
> - ret_fifo = kfifo_out_spinlocked(&ctrl->svc_fifo,
> + ret_fifo = kfifo_out_spinlocked(&chan->svc_fifo,
> pdata, sizeof(*pdata),
> - &ctrl->svc_fifo_lock);
> + &chan->svc_fifo_lock);
>
> if (!ret_fifo)
> continue;
> @@ -569,9 +568,25 @@ static int svc_normal_to_secure_thread(void *data)
> (unsigned int)pdata->paddr, pdata->command,
> (unsigned int)pdata->size);
>
> + /* SDM can only process one command at a time */
> + pr_debug("%s: %s: Thread is waiting for mutex!\n",
> + __func__, chan->name);
> + if (mutex_lock_interruptible(ctrl->sdm_lock)) {
> + /* item already dequeued; notify client to unblock it */
> + cbdata->status = BIT(SVC_STATUS_ERROR);
> + cbdata->kaddr1 = NULL;
> + cbdata->kaddr2 = NULL;
> + cbdata->kaddr3 = NULL;
> + if (pdata->chan->scl)
> + pdata->chan->scl->receive_cb(pdata->chan->scl,
> + cbdata);
> + break;
> + }
> +
> switch (pdata->command) {
> case COMMAND_RECONFIG_DATA_CLAIM:
> svc_thread_cmd_data_claim(ctrl, pdata, cbdata);
> + mutex_unlock(ctrl->sdm_lock);
> continue;
> case COMMAND_RECONFIG:
> a0 = INTEL_SIP_SMC_FPGA_CONFIG_START;
> @@ -700,10 +715,11 @@ static int svc_normal_to_secure_thread(void *data)
> break;
> default:
> pr_warn("it shouldn't happen\n");
> - break;
> + mutex_unlock(ctrl->sdm_lock);
> + continue;
> }
> - pr_debug("%s: before SMC call -- a0=0x%016x a1=0x%016x",
> - __func__,
> + pr_debug("%s: %s: before SMC call -- a0=0x%016x a1=0x%016x",
> + __func__, chan->name,
> (unsigned int)a0,
> (unsigned int)a1);
> pr_debug(" a2=0x%016x\n", (unsigned int)a2);
> @@ -712,8 +728,8 @@ static int svc_normal_to_secure_thread(void *data)
> pr_debug(" a5=0x%016x\n", (unsigned int)a5);
> ctrl->invoke_fn(a0, a1, a2, a3, a4, a5, a6, a7, &res);
>
> - pr_debug("%s: after SMC call -- res.a0=0x%016x",
> - __func__, (unsigned int)res.a0);
> + pr_debug("%s: %s: after SMC call -- res.a0=0x%016x",
> + __func__, chan->name, (unsigned int)res.a0);
> pr_debug(" res.a1=0x%016x, res.a2=0x%016x",
> (unsigned int)res.a1, (unsigned int)res.a2);
> pr_debug(" res.a3=0x%016x\n", (unsigned int)res.a3);
> @@ -728,6 +744,7 @@ static int svc_normal_to_secure_thread(void *data)
> cbdata->kaddr2 = NULL;
> cbdata->kaddr3 = NULL;
> pdata->chan->scl->receive_cb(pdata->chan->scl, cbdata);
> + mutex_unlock(ctrl->sdm_lock);
> continue;
> }
>
> @@ -801,6 +818,8 @@ static int svc_normal_to_secure_thread(void *data)
> break;
>
> }
> +
> + mutex_unlock(ctrl->sdm_lock);
> }
>
> kfree(cbdata);
> @@ -1697,21 +1716,25 @@ int stratix10_svc_send(struct stratix10_svc_chan *chan, void *msg)
> return -ENOMEM;
>
> /* first client will create kernel thread */
> - if (!chan->ctrl->task) {
> - chan->ctrl->task =
> + spin_lock(&chan->lock);
> + if (!chan->task) {
> + chan->task =
> kthread_run_on_cpu(svc_normal_to_secure_thread,
> - (void *)chan->ctrl,
> + (void *)chan,
> cpu, "svc_smc_hvc_thread");
> - if (IS_ERR(chan->ctrl->task)) {
> + if (IS_ERR(chan->task)) {
> + chan->task = NULL;
> + spin_unlock(&chan->lock);
> dev_err(chan->ctrl->dev,
> "failed to create svc_smc_hvc_thread\n");
> kfree(p_data);
> return -EINVAL;
> }
> }
> + spin_unlock(&chan->lock);
>
> - pr_debug("%s: sent P-va=%p, P-com=%x, P-size=%u\n", __func__,
> - p_msg->payload, p_msg->command,
> + pr_debug("%s: %s: sent P-va=%p, P-com=%x, P-size=%u\n", __func__,
> + chan->name, p_msg->payload, p_msg->command,
> (unsigned int)p_msg->payload_length);
>
> if (list_empty(&svc_data_mem)) {
> @@ -1747,12 +1770,16 @@ int stratix10_svc_send(struct stratix10_svc_chan *chan, void *msg)
> p_data->arg[2] = p_msg->arg[2];
> p_data->size = p_msg->payload_length;
> p_data->chan = chan;
> - pr_debug("%s: put to FIFO pa=0x%016x, cmd=%x, size=%u\n", __func__,
> - (unsigned int)p_data->paddr, p_data->command,
> - (unsigned int)p_data->size);
> - ret = kfifo_in_spinlocked(&chan->ctrl->svc_fifo, p_data,
> + pr_debug("%s: %s: put to FIFO pa=0x%016x, cmd=%x, size=%u\n",
> + __func__,
> + chan->name,
> + (unsigned int)p_data->paddr,
> + p_data->command,
> + (unsigned int)p_data->size);
> +
> + ret = kfifo_in_spinlocked(&chan->svc_fifo, p_data,
> sizeof(*p_data),
> - &chan->ctrl->svc_fifo_lock);
> + &chan->svc_fifo_lock);
>
> kfree(p_data);
>
> @@ -1773,11 +1800,12 @@ EXPORT_SYMBOL_GPL(stratix10_svc_send);
> */
> void stratix10_svc_done(struct stratix10_svc_chan *chan)
> {
> - /* stop thread when thread is running AND only one active client */
> - if (chan->ctrl->task && chan->ctrl->num_active_client <= 1) {
> - pr_debug("svc_smc_hvc_shm_thread is stopped\n");
> - kthread_stop(chan->ctrl->task);
> - chan->ctrl->task = NULL;
> + /* stop thread when thread is running */
> + if (chan->task) {
> + pr_debug("%s: %s: svc_smc_hvc_shm_thread is stopping\n",
> + __func__, chan->name);
> + kthread_stop(chan->task);
> + chan->task = NULL;
> }
> }
> EXPORT_SYMBOL_GPL(stratix10_svc_done);
> @@ -1817,8 +1845,8 @@ void *stratix10_svc_allocate_memory(struct stratix10_svc_chan *chan,
> pmem->paddr = pa;
> pmem->size = s;
> list_add_tail(&pmem->node, &svc_data_mem);
> - pr_debug("%s: va=%p, pa=0x%016x\n", __func__,
> - pmem->vaddr, (unsigned int)pmem->paddr);
> + pr_debug("%s: %s: va=%p, pa=0x%016x\n", __func__,
> + chan->name, pmem->vaddr, (unsigned int)pmem->paddr);
>
> return (void *)va;
> }
> @@ -1855,6 +1883,18 @@ static const struct of_device_id stratix10_svc_drv_match[] = {
> {},
> };
>
> +/**
> + * mailbox_lock protects access to the SDM if thread is busy
> + */
> +static DEFINE_MUTEX(mailbox_lock);
Should you use a global mutex? I'd consider using a local mutex that you
already have defined in the stratix10_svc_controller->sdm_lock.
> +
> +static const char * const chan_names[SVC_NUM_CHANNEL] = {
> + SVC_CLIENT_FPGA,
> + SVC_CLIENT_RSU,
> + SVC_CLIENT_FCS,
> + SVC_CLIENT_HWMON
> +};
> +
> static int stratix10_svc_drv_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> @@ -1862,11 +1902,11 @@ static int stratix10_svc_drv_probe(struct platform_device *pdev)
> struct stratix10_svc_chan *chans;
> struct gen_pool *genpool;
> struct stratix10_svc_sh_memory *sh_memory;
> - struct stratix10_svc *svc;
> + struct stratix10_svc *svc = NULL;
>
> svc_invoke_fn *invoke_fn;
> size_t fifo_size;
> - int ret;
> + int ret, i = 0;
>
> /* get SMC or HVC function */
> invoke_fn = get_invoke_func(dev);
> @@ -1905,8 +1945,8 @@ static int stratix10_svc_drv_probe(struct platform_device *pdev)
> controller->num_active_client = 0;
> controller->chans = chans;
> controller->genpool = genpool;
> - controller->task = NULL;
> controller->invoke_fn = invoke_fn;
> + INIT_LIST_HEAD(&controller->node);
> init_completion(&controller->complete_status);
>
> ret = stratix10_svc_async_init(controller);
> @@ -1917,32 +1957,24 @@ static int stratix10_svc_drv_probe(struct platform_device *pdev)
> }
>
> fifo_size = sizeof(struct stratix10_svc_data) * SVC_NUM_DATA_IN_FIFO;
> - ret = kfifo_alloc(&controller->svc_fifo, fifo_size, GFP_KERNEL);
> - if (ret) {
> - dev_err(dev, "failed to allocate FIFO\n");
> - goto err_async_exit;
> - }
> - spin_lock_init(&controller->svc_fifo_lock);
> -
> - chans[0].scl = NULL;
> - chans[0].ctrl = controller;
> - chans[0].name = SVC_CLIENT_FPGA;
> - spin_lock_init(&chans[0].lock);
> -
> - chans[1].scl = NULL;
> - chans[1].ctrl = controller;
> - chans[1].name = SVC_CLIENT_RSU;
> - spin_lock_init(&chans[1].lock);
> -
> - chans[2].scl = NULL;
> - chans[2].ctrl = controller;
> - chans[2].name = SVC_CLIENT_FCS;
> - spin_lock_init(&chans[2].lock);
> + /*
> + * This mutex is used to block threads from utilizing
> + * SDM to prevent out of order command tx
> + */
> + controller->sdm_lock = &mailbox_lock;
>
> - chans[3].scl = NULL;
> - chans[3].ctrl = controller;
> - chans[3].name = SVC_CLIENT_HWMON;
> - spin_lock_init(&chans[3].lock);
> + for (i = 0; i < SVC_NUM_CHANNEL; i++) {
> + chans[i].scl = NULL;
> + chans[i].ctrl = controller;
> + chans[i].name = (char *)chan_names[i];
> + spin_lock_init(&chans[i].lock);
> + ret = kfifo_alloc(&chans[i].svc_fifo, fifo_size, GFP_KERNEL);
> + if (ret) {
> + dev_err(dev, "failed to allocate FIFO %d\n", i);
> + goto err_free_fifos;
> + }
> + spin_lock_init(&chans[i].svc_fifo_lock);
> + }
>
> list_add_tail(&controller->node, &svc_ctrl);
> platform_set_drvdata(pdev, controller);
> @@ -1951,7 +1983,7 @@ static int stratix10_svc_drv_probe(struct platform_device *pdev)
> svc = devm_kzalloc(dev, sizeof(*svc), GFP_KERNEL);
> if (!svc) {
> ret = -ENOMEM;
> - goto err_free_kfifo;
> + goto err_free_fifos;
> }
> controller->svc = svc;
>
> @@ -1959,51 +1991,40 @@ static int stratix10_svc_drv_probe(struct platform_device *pdev)
> if (!svc->stratix10_svc_rsu) {
> dev_err(dev, "failed to allocate %s device\n", STRATIX10_RSU);
> ret = -ENOMEM;
> - goto err_free_kfifo;
> + goto err_free_fifos;
> }
>
> ret = platform_device_add(svc->stratix10_svc_rsu);
> - if (ret) {
> - platform_device_put(svc->stratix10_svc_rsu);
> - goto err_free_kfifo;
> - }
> -
> - svc->intel_svc_fcs = platform_device_alloc(INTEL_FCS, 1);
> - if (!svc->intel_svc_fcs) {
> - dev_err(dev, "failed to allocate %s device\n", INTEL_FCS);
> - ret = -ENOMEM;
> - goto err_unregister_rsu_dev;
> - }
> -
> - ret = platform_device_add(svc->intel_svc_fcs);
> - if (ret) {
> - platform_device_put(svc->intel_svc_fcs);
> - goto err_unregister_rsu_dev;
> - }
> + if (ret)
> + goto err_put_device;
>
> ret = of_platform_default_populate(dev_of_node(dev), NULL, dev);
> if (ret)
> - goto err_unregister_fcs_dev;
> + goto err_put_device;
You need a new error path here because platform_device_add() has
succeeded, but your error path is only calling platform_device_put()
which only decrements the refcount, but does not unregister. You need an
error path here that will call platform_device_unregister().
>
> pr_info("Intel Service Layer Driver Initialized\n");
>
> return 0;
>
> -err_unregister_fcs_dev:
> - platform_device_unregister(svc->intel_svc_fcs);
> -err_unregister_rsu_dev:
> - platform_device_unregister(svc->stratix10_svc_rsu);
> -err_free_kfifo:
> - kfifo_free(&controller->svc_fifo);
> -err_async_exit:
> +err_put_device:
> + platform_device_put(svc->stratix10_svc_rsu);
> +err_free_fifos:
> + /* only remove from list if list_add_tail() was reached */
> + if (!list_empty(&controller->node))
> + list_del(&controller->node);
> + /* free only the FIFOs that were successfully allocated */
> + while (i--)
> + kfifo_free(&chans[i].svc_fifo);
> stratix10_svc_async_exit(controller);
> err_destroy_pool:
> gen_pool_destroy(genpool);
> +
> return ret;
> }
>
> static void stratix10_svc_drv_remove(struct platform_device *pdev)
> {
> + int i;
> struct stratix10_svc_controller *ctrl = platform_get_drvdata(pdev);
> struct stratix10_svc *svc = ctrl->svc;
>
> @@ -2011,14 +2032,16 @@ static void stratix10_svc_drv_remove(struct platform_device *pdev)
>
> of_platform_depopulate(ctrl->dev);
>
> - platform_device_unregister(svc->intel_svc_fcs);
> platform_device_unregister(svc->stratix10_svc_rsu);
>
> - kfifo_free(&ctrl->svc_fifo);
> - if (ctrl->task) {
> - kthread_stop(ctrl->task);
> - ctrl->task = NULL;
> + for (i = 0; i < SVC_NUM_CHANNEL; i++) {
> + if (ctrl->chans[i].task) {
> + kthread_stop(ctrl->chans[i].task);
> + ctrl->chans[i].task = NULL;
> + }
> + kfifo_free(&ctrl->chans[i].svc_fifo);
> }
> +
> if (ctrl->genpool)
> gen_pool_destroy(ctrl->genpool);
> list_del(&ctrl->node);
> diff --git a/include/linux/firmware/intel/stratix10-svc-client.h b/include/linux/firmware/intel/stratix10-svc-client.h
> index d290060f4c73..91013161e9db 100644
> --- a/include/linux/firmware/intel/stratix10-svc-client.h
> +++ b/include/linux/firmware/intel/stratix10-svc-client.h
> @@ -68,12 +68,12 @@
> * timeout value used in Stratix10 FPGA manager driver.
> * timeout value used in RSU driver
> */
> -#define SVC_RECONFIG_REQUEST_TIMEOUT_MS 300
> -#define SVC_RECONFIG_BUFFER_TIMEOUT_MS 720
> -#define SVC_RSU_REQUEST_TIMEOUT_MS 300
> +#define SVC_RECONFIG_REQUEST_TIMEOUT_MS 5000
> +#define SVC_RECONFIG_BUFFER_TIMEOUT_MS 5000
> +#define SVC_RSU_REQUEST_TIMEOUT_MS 2000
> #define SVC_FCS_REQUEST_TIMEOUT_MS 2000
> #define SVC_COMPLETED_TIMEOUT_MS 30000
> -#define SVC_HWMON_REQUEST_TIMEOUT_MS 300
> +#define SVC_HWMON_REQUEST_TIMEOUT_MS 2000
>
> struct stratix10_svc_chan;
>
Dinh
On 3/3/2026 12:21 pm, Dinh Nguyen wrote:
>
>
> On 3/1/26 23:39, Muhammad Amirul Asyraf Mohamad Jamian wrote:
>> In the current implementation, SVC client drivers such as socfpga-hwmon,
>> intel_fcs, stratix10-soc, stratix10-rsu each send an SMC command that
>> triggers a single thread in the stratix10-svc driver. Upon receiving a
>> callback, the initiating client driver sends a stratix10-svc-done signal,
>> terminating the thread without waiting for other pending SMC commands to
>> complete. This leads to a timeout issue in the firmware SVC mailbox
>> service
>> when multiple client drivers send SMC commands concurrently.
>>
>> To resolve this issue, a dedicated thread is now created per channel. The
>> stratix10-svc driver will support up to the number of channels defined by
>> SVC_NUM_CHANNEL. Thread synchronization is handled using a mutex to
>> prevent
>> simultaneous issuance of SMC commands by multiple threads.
>>
>> Additionally, a thread task is now validated before invoking kthread_stop
>> when the user aborts, ensuring safe termination.
>>
>> Timeout values have also been adjusted to accommodate the increased load
>> from concurrent client driver activity.
>>
>> Fixes: 7ca5ce896524 ("firmware: add Intel Stratix10 service layer
>> driver")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Ang Tien Sung <tien.sung.ang@altera.com>
>> Signed-off-by: Fong, Yan Kei <yankei.fong@altera.com>
>> Signed-off-by: Muhammad Amirul Asyraf Mohamad Jamian
>> <muhammad.amirul.asyraf.mohamad.jamian@altera.com>
>> Reported-by: kernel test robot <lkp@intel.com>
>> Closes: https://nam10.safelinks.protection.outlook.com/?
>> url=https%3A%2F%2Flore.kernel.org%2Foe-kbuild-
>> all%2F202602032325.MuglZ156-
>> lkp%40intel.com%2F&data=05%7C02%7Cmuhammad.amirul.asyraf.mohamad.jamian%40altera.com%7C689df731286b4be4a84908de78dc5730%7Cfbd72e03d4a54110adce614d51f2077a%7C0%7C0%7C639081084966139250%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=T%2FeAwfU0qMEv8MKVe0ZAQq%2FANAWWsBAvcJ1g8ZvvC18%3D&reserved=0
>> ---
>> Changes in v5:
>> - Fix mutex locking: replace guard(mutex) with explicit lock/unlock,
>> use mutex_lock_interruptible() so kthread_stop() can unblock the
>> thread when the SDM lock is contended, fix lock/unlock imbalances,
>> and on interrupted lock notify client with SVC_STATUS_ERROR so it
>> does not block on a response that never arrives
>> - Fix TOCTOU race in stratix10_svc_send(): protect chan->task creation
>> with chan->lock spinlock to prevent concurrent callers on the same
>> channel from spawning duplicate threads
>> - Fix probe error path: initialize controller->node with INIT_LIST_HEAD
>> and guard list_del() so it is only called when list_add_tail() was
>> reached, preventing NULL deref on early FIFO allocation failure
>> - Remove stray platform_device_unregister(svc->intel_svc_fcs) call in
>> remove function, as FCS device is not manually registered in probe;
>> also remove the now-unused INTEL_FCS macro, stale intel_svc_fcs field
>> from struct stratix10_svc, and stale @task kdoc from
>> struct stratix10_svc_controller
>> - Remove unnecessary NULL check on svc pointer in err_put_device
>> - Clean up: convert FIFO allocations to for loop, simplify
>> stratix10_svc_done() to a single debug message, fix whitespace,
>> typos and log messages
>>
>> Changes in v4:
>> - Signed-off-by chain clean up
>> - Remove duplicate inline comment on sdm_lock field
>> - Move structure field comments to structure header
>> - Remove unnecessary checks in stratix10_svc_done()
>> - Fix error handling to properly free FIFOs on probe failure
>> - Reorder error labels for proper cleanup cascade
>> - Remove redundant fifo_size initialization
>>
>> Changes in v3:
>> - Fix grammar: "will supports" -> "will support"
>> - Add Fixes tag and Cc stable as suggested by reviewer
>>
>> Changes in v2:
>> - Initialize 'svc' to NULL and add NULL check in error path to fix
>> uninitialized variable warning
>> ---
>> ---
>> drivers/firmware/stratix10-svc.c | 225 ++++++++++--------
>> .../firmware/intel/stratix10-svc-client.h | 8 +-
>> 2 files changed, 128 insertions(+), 105 deletions(-)
>>
>> diff --git a/drivers/firmware/stratix10-svc.c b/drivers/firmware/
>> stratix10-svc.c
>> index 6f5c298582ab..2af95018f06d 100644
>> --- a/drivers/firmware/stratix10-svc.c
>> +++ b/drivers/firmware/stratix10-svc.c
>> @@ -37,15 +37,14 @@
>> * service layer will return error to FPGA manager when timeout occurs,
>> * timeout is set to 30 seconds (30 * 1000) at Intel Stratix10 SoC.
>> */
>> -#define SVC_NUM_DATA_IN_FIFO 32
>> +#define SVC_NUM_DATA_IN_FIFO 8
>
> Your commit message does not explain this change. I don't see how this
> change is related to this patch.
>
>
>> #define SVC_NUM_CHANNEL 4
>> -#define FPGA_CONFIG_DATA_CLAIM_TIMEOUT_MS 200
>> +#define FPGA_CONFIG_DATA_CLAIM_TIMEOUT_MS 2000
>> #define FPGA_CONFIG_STATUS_TIMEOUT_SEC 30
>> #define BYTE_TO_WORD_SIZE 4
>> /* stratix10 service layer clients */
>> #define STRATIX10_RSU "stratix10-rsu"
>> -#define INTEL_FCS "intel-fcs"
>> /* Maximum number of SDM client IDs. */
>> #define MAX_SDM_CLIENT_IDS 16
>> @@ -105,11 +104,9 @@ struct stratix10_svc_chan;
>> /**
>> * struct stratix10_svc - svc private data
>> * @stratix10_svc_rsu: pointer to stratix10 RSU device
>> - * @intel_svc_fcs: pointer to the FCS device
>> */
>> struct stratix10_svc {
>> struct platform_device *stratix10_svc_rsu;
>> - struct platform_device *intel_svc_fcs;
>> };
>> /**
>> @@ -251,12 +248,10 @@ struct stratix10_async_ctrl {
>> * @num_active_client: number of active service client
>> * @node: list management
>> * @genpool: memory pool pointing to the memory region
>> - * @task: pointer to the thread task which handles SMC or HVC call
>> - * @svc_fifo: a queue for storing service message data
>> * @complete_status: state for completion
>> - * @svc_fifo_lock: protect access to service message data queue
>> * @invoke_fn: function to issue secure monitor call or hypervisor call
>> * @svc: manages the list of client svc drivers
>> + * @sdm_lock: only allows a single command single response to SDM
>> * @actrl: async control structure
>> *
>> * This struct is used to create communication channels for service
>> clients, to
>> @@ -269,12 +264,10 @@ struct stratix10_svc_controller {
>> int num_active_client;
>> struct list_head node;
>> struct gen_pool *genpool;
>> - struct task_struct *task;
>> - struct kfifo svc_fifo;
>> struct completion complete_status;
>> - spinlock_t svc_fifo_lock;
>> svc_invoke_fn *invoke_fn;
>> struct stratix10_svc *svc;
>> + struct mutex *sdm_lock;
>> struct stratix10_async_ctrl actrl;
>> };
>> @@ -283,6 +276,9 @@ struct stratix10_svc_controller {
>> * @ctrl: pointer to service controller which is the provider of
>> this channel
>> * @scl: pointer to service client which owns the channel
>> * @name: service client name associated with the channel
>> + * @task: pointer to the thread task which handles SMC or HVC call
>> + * @svc_fifo: a queue for storing service message data (separate fifo
>> for every channel)
>> + * @svc_fifo_lock: protect access to service message data queue
>> (locking pending fifo)
>> * @lock: protect access to the channel
>> * @async_chan: reference to asynchronous channel object for this
>> channel
>> *
>> @@ -293,6 +289,9 @@ struct stratix10_svc_chan {
>> struct stratix10_svc_controller *ctrl;
>> struct stratix10_svc_client *scl;
>> char *name;
>> + struct task_struct *task;
>> + struct kfifo svc_fifo;
>> + spinlock_t svc_fifo_lock;
>> spinlock_t lock;
>> struct stratix10_async_chan *async_chan;
>> };
>> @@ -527,10 +526,10 @@ static void svc_thread_recv_status_ok(struct
>> stratix10_svc_data *p_data,
>> */
>> static int svc_normal_to_secure_thread(void *data)
>> {
>> - struct stratix10_svc_controller
>> - *ctrl = (struct stratix10_svc_controller *)data;
>> - struct stratix10_svc_data *pdata;
>> - struct stratix10_svc_cb_data *cbdata;
>> + struct stratix10_svc_chan *chan = (struct stratix10_svc_chan *)data;
>> + struct stratix10_svc_controller *ctrl = chan->ctrl;
>> + struct stratix10_svc_data *pdata = NULL;
>> + struct stratix10_svc_cb_data *cbdata = NULL;
>> struct arm_smccc_res res;
>> unsigned long a0, a1, a2, a3, a4, a5, a6, a7;
>> int ret_fifo = 0;
>> @@ -555,12 +554,12 @@ static int svc_normal_to_secure_thread(void *data)
>> a6 = 0;
>> a7 = 0;
>> - pr_debug("smc_hvc_shm_thread is running\n");
>> + pr_debug("%s: %s: Thread is running!\n", __func__, chan->name);
>> while (!kthread_should_stop()) {
>> - ret_fifo = kfifo_out_spinlocked(&ctrl->svc_fifo,
>> + ret_fifo = kfifo_out_spinlocked(&chan->svc_fifo,
>> pdata, sizeof(*pdata),
>> - &ctrl->svc_fifo_lock);
>> + &chan->svc_fifo_lock);
>> if (!ret_fifo)
>> continue;
>> @@ -569,9 +568,25 @@ static int svc_normal_to_secure_thread(void *data)
>> (unsigned int)pdata->paddr, pdata->command,
>> (unsigned int)pdata->size);
>> + /* SDM can only process one command at a time */
>> + pr_debug("%s: %s: Thread is waiting for mutex!\n",
>> + __func__, chan->name);
>> + if (mutex_lock_interruptible(ctrl->sdm_lock)) {
>> + /* item already dequeued; notify client to unblock it */
>> + cbdata->status = BIT(SVC_STATUS_ERROR);
>> + cbdata->kaddr1 = NULL;
>> + cbdata->kaddr2 = NULL;
>> + cbdata->kaddr3 = NULL;
>> + if (pdata->chan->scl)
>> + pdata->chan->scl->receive_cb(pdata->chan->scl,
>> + cbdata);
>> + break;
>> + }
>> +
>> switch (pdata->command) {
>> case COMMAND_RECONFIG_DATA_CLAIM:
>> svc_thread_cmd_data_claim(ctrl, pdata, cbdata);
>> + mutex_unlock(ctrl->sdm_lock);
>> continue;
>> case COMMAND_RECONFIG:
>> a0 = INTEL_SIP_SMC_FPGA_CONFIG_START;
>> @@ -700,10 +715,11 @@ static int svc_normal_to_secure_thread(void *data)
>> break;
>> default:
>> pr_warn("it shouldn't happen\n");
>> - break;
>> + mutex_unlock(ctrl->sdm_lock);
>> + continue;
>> }
>> - pr_debug("%s: before SMC call -- a0=0x%016x a1=0x%016x",
>> - __func__,
>> + pr_debug("%s: %s: before SMC call -- a0=0x%016x a1=0x%016x",
>> + __func__, chan->name,
>> (unsigned int)a0,
>> (unsigned int)a1);
>> pr_debug(" a2=0x%016x\n", (unsigned int)a2);
>> @@ -712,8 +728,8 @@ static int svc_normal_to_secure_thread(void *data)
>> pr_debug(" a5=0x%016x\n", (unsigned int)a5);
>> ctrl->invoke_fn(a0, a1, a2, a3, a4, a5, a6, a7, &res);
>> - pr_debug("%s: after SMC call -- res.a0=0x%016x",
>> - __func__, (unsigned int)res.a0);
>> + pr_debug("%s: %s: after SMC call -- res.a0=0x%016x",
>> + __func__, chan->name, (unsigned int)res.a0);
>> pr_debug(" res.a1=0x%016x, res.a2=0x%016x",
>> (unsigned int)res.a1, (unsigned int)res.a2);
>> pr_debug(" res.a3=0x%016x\n", (unsigned int)res.a3);
>> @@ -728,6 +744,7 @@ static int svc_normal_to_secure_thread(void *data)
>> cbdata->kaddr2 = NULL;
>> cbdata->kaddr3 = NULL;
>> pdata->chan->scl->receive_cb(pdata->chan->scl, cbdata);
>> + mutex_unlock(ctrl->sdm_lock);
>> continue;
>> }
>> @@ -801,6 +818,8 @@ static int svc_normal_to_secure_thread(void *data)
>> break;
>> }
>> +
>> + mutex_unlock(ctrl->sdm_lock);
>> }
>> kfree(cbdata);
>> @@ -1697,21 +1716,25 @@ int stratix10_svc_send(struct
>> stratix10_svc_chan *chan, void *msg)
>> return -ENOMEM;
>> /* first client will create kernel thread */
>> - if (!chan->ctrl->task) {
>> - chan->ctrl->task =
>> + spin_lock(&chan->lock);
>> + if (!chan->task) {
>> + chan->task =
>> kthread_run_on_cpu(svc_normal_to_secure_thread,
>> - (void *)chan->ctrl,
>> + (void *)chan,
>> cpu, "svc_smc_hvc_thread");
>> - if (IS_ERR(chan->ctrl->task)) {
>> + if (IS_ERR(chan->task)) {
>> + chan->task = NULL;
>> + spin_unlock(&chan->lock);
>> dev_err(chan->ctrl->dev,
>> "failed to create svc_smc_hvc_thread\n");
>> kfree(p_data);
>> return -EINVAL;
>> }
>> }
>> + spin_unlock(&chan->lock);
>> - pr_debug("%s: sent P-va=%p, P-com=%x, P-size=%u\n", __func__,
>> - p_msg->payload, p_msg->command,
>> + pr_debug("%s: %s: sent P-va=%p, P-com=%x, P-size=%u\n", __func__,
>> + chan->name, p_msg->payload, p_msg->command,
>> (unsigned int)p_msg->payload_length);
>> if (list_empty(&svc_data_mem)) {
>> @@ -1747,12 +1770,16 @@ int stratix10_svc_send(struct
>> stratix10_svc_chan *chan, void *msg)
>> p_data->arg[2] = p_msg->arg[2];
>> p_data->size = p_msg->payload_length;
>> p_data->chan = chan;
>> - pr_debug("%s: put to FIFO pa=0x%016x, cmd=%x, size=%u\n", __func__,
>> - (unsigned int)p_data->paddr, p_data->command,
>> - (unsigned int)p_data->size);
>> - ret = kfifo_in_spinlocked(&chan->ctrl->svc_fifo, p_data,
>> + pr_debug("%s: %s: put to FIFO pa=0x%016x, cmd=%x, size=%u\n",
>> + __func__,
>> + chan->name,
>> + (unsigned int)p_data->paddr,
>> + p_data->command,
>> + (unsigned int)p_data->size);
>> +
>> + ret = kfifo_in_spinlocked(&chan->svc_fifo, p_data,
>> sizeof(*p_data),
>> - &chan->ctrl->svc_fifo_lock);
>> + &chan->svc_fifo_lock);
>> kfree(p_data);
>> @@ -1773,11 +1800,12 @@ EXPORT_SYMBOL_GPL(stratix10_svc_send);
>> */
>> void stratix10_svc_done(struct stratix10_svc_chan *chan)
>> {
>> - /* stop thread when thread is running AND only one active client */
>> - if (chan->ctrl->task && chan->ctrl->num_active_client <= 1) {
>> - pr_debug("svc_smc_hvc_shm_thread is stopped\n");
>> - kthread_stop(chan->ctrl->task);
>> - chan->ctrl->task = NULL;
>> + /* stop thread when thread is running */
>> + if (chan->task) {
>> + pr_debug("%s: %s: svc_smc_hvc_shm_thread is stopping\n",
>> + __func__, chan->name);
>> + kthread_stop(chan->task);
>> + chan->task = NULL;
>> }
>> }
>> EXPORT_SYMBOL_GPL(stratix10_svc_done);
>> @@ -1817,8 +1845,8 @@ void *stratix10_svc_allocate_memory(struct
>> stratix10_svc_chan *chan,
>> pmem->paddr = pa;
>> pmem->size = s;
>> list_add_tail(&pmem->node, &svc_data_mem);
>> - pr_debug("%s: va=%p, pa=0x%016x\n", __func__,
>> - pmem->vaddr, (unsigned int)pmem->paddr);
>> + pr_debug("%s: %s: va=%p, pa=0x%016x\n", __func__,
>> + chan->name, pmem->vaddr, (unsigned int)pmem->paddr);
>> return (void *)va;
>> }
>> @@ -1855,6 +1883,18 @@ static const struct of_device_id
>> stratix10_svc_drv_match[] = {
>> {},
>> };
>> +/**
>> + * mailbox_lock protects access to the SDM if thread is busy
>> + */
>> +static DEFINE_MUTEX(mailbox_lock);
>
> Should you use a global mutex? I'd consider using a local mutex that you
> already have defined in the stratix10_svc_controller->sdm_lock.
>
>> +
>> +static const char * const chan_names[SVC_NUM_CHANNEL] = {
>> + SVC_CLIENT_FPGA,
>> + SVC_CLIENT_RSU,
>> + SVC_CLIENT_FCS,
>> + SVC_CLIENT_HWMON
>> +};
>> +
>> static int stratix10_svc_drv_probe(struct platform_device *pdev)
>> {
>> struct device *dev = &pdev->dev;
>> @@ -1862,11 +1902,11 @@ static int stratix10_svc_drv_probe(struct
>> platform_device *pdev)
>> struct stratix10_svc_chan *chans;
>> struct gen_pool *genpool;
>> struct stratix10_svc_sh_memory *sh_memory;
>> - struct stratix10_svc *svc;
>> + struct stratix10_svc *svc = NULL;
>> svc_invoke_fn *invoke_fn;
>> size_t fifo_size;
>> - int ret;
>> + int ret, i = 0;
>> /* get SMC or HVC function */
>> invoke_fn = get_invoke_func(dev);
>> @@ -1905,8 +1945,8 @@ static int stratix10_svc_drv_probe(struct
>> platform_device *pdev)
>> controller->num_active_client = 0;
>> controller->chans = chans;
>> controller->genpool = genpool;
>> - controller->task = NULL;
>> controller->invoke_fn = invoke_fn;
>> + INIT_LIST_HEAD(&controller->node);
>> init_completion(&controller->complete_status);
>> ret = stratix10_svc_async_init(controller);
>> @@ -1917,32 +1957,24 @@ static int stratix10_svc_drv_probe(struct
>> platform_device *pdev)
>> }
>> fifo_size = sizeof(struct stratix10_svc_data) *
>> SVC_NUM_DATA_IN_FIFO;
>> - ret = kfifo_alloc(&controller->svc_fifo, fifo_size, GFP_KERNEL);
>> - if (ret) {
>> - dev_err(dev, "failed to allocate FIFO\n");
>> - goto err_async_exit;
>> - }
>> - spin_lock_init(&controller->svc_fifo_lock);
>> -
>> - chans[0].scl = NULL;
>> - chans[0].ctrl = controller;
>> - chans[0].name = SVC_CLIENT_FPGA;
>> - spin_lock_init(&chans[0].lock);
>> -
>> - chans[1].scl = NULL;
>> - chans[1].ctrl = controller;
>> - chans[1].name = SVC_CLIENT_RSU;
>> - spin_lock_init(&chans[1].lock);
>> -
>> - chans[2].scl = NULL;
>> - chans[2].ctrl = controller;
>> - chans[2].name = SVC_CLIENT_FCS;
>> - spin_lock_init(&chans[2].lock);
>> + /*
>> + * This mutex is used to block threads from utilizing
>> + * SDM to prevent out of order command tx
>> + */
>> + controller->sdm_lock = &mailbox_lock;
>> - chans[3].scl = NULL;
>> - chans[3].ctrl = controller;
>> - chans[3].name = SVC_CLIENT_HWMON;
>> - spin_lock_init(&chans[3].lock);
>> + for (i = 0; i < SVC_NUM_CHANNEL; i++) {
>> + chans[i].scl = NULL;
>> + chans[i].ctrl = controller;
>> + chans[i].name = (char *)chan_names[i];
>> + spin_lock_init(&chans[i].lock);
>> + ret = kfifo_alloc(&chans[i].svc_fifo, fifo_size, GFP_KERNEL);
>> + if (ret) {
>> + dev_err(dev, "failed to allocate FIFO %d\n", i);
>> + goto err_free_fifos;
>> + }
>> + spin_lock_init(&chans[i].svc_fifo_lock);
>> + }
>> list_add_tail(&controller->node, &svc_ctrl);
>> platform_set_drvdata(pdev, controller);
>> @@ -1951,7 +1983,7 @@ static int stratix10_svc_drv_probe(struct
>> platform_device *pdev)
>> svc = devm_kzalloc(dev, sizeof(*svc), GFP_KERNEL);
>> if (!svc) {
>> ret = -ENOMEM;
>> - goto err_free_kfifo;
>> + goto err_free_fifos;
>> }
>> controller->svc = svc;
>> @@ -1959,51 +1991,40 @@ static int stratix10_svc_drv_probe(struct
>> platform_device *pdev)
>> if (!svc->stratix10_svc_rsu) {
>> dev_err(dev, "failed to allocate %s device\n", STRATIX10_RSU);
>> ret = -ENOMEM;
>> - goto err_free_kfifo;
>> + goto err_free_fifos;
>> }
>> ret = platform_device_add(svc->stratix10_svc_rsu);
>> - if (ret) {
>> - platform_device_put(svc->stratix10_svc_rsu);
>> - goto err_free_kfifo;
>> - }
>> -
>> - svc->intel_svc_fcs = platform_device_alloc(INTEL_FCS, 1);
>> - if (!svc->intel_svc_fcs) {
>> - dev_err(dev, "failed to allocate %s device\n", INTEL_FCS);
>> - ret = -ENOMEM;
>> - goto err_unregister_rsu_dev;
>> - }
>> -
>> - ret = platform_device_add(svc->intel_svc_fcs);
>> - if (ret) {
>> - platform_device_put(svc->intel_svc_fcs);
>> - goto err_unregister_rsu_dev;
>> - }
>> + if (ret)
>> + goto err_put_device;
>> ret = of_platform_default_populate(dev_of_node(dev), NULL, dev);
>> if (ret)
>> - goto err_unregister_fcs_dev;
>> + goto err_put_device;
>
> You need a new error path here because platform_device_add() has
> succeeded, but your error path is only calling platform_device_put()
> which only decrements the refcount, but does not unregister. You need an
> error path here that will call platform_device_unregister().
>
>> pr_info("Intel Service Layer Driver Initialized\n");
>> return 0;
>> -err_unregister_fcs_dev:
>> - platform_device_unregister(svc->intel_svc_fcs);
>> -err_unregister_rsu_dev:
>> - platform_device_unregister(svc->stratix10_svc_rsu);
>> -err_free_kfifo:
>> - kfifo_free(&controller->svc_fifo);
>> -err_async_exit:
>> +err_put_device:
>> + platform_device_put(svc->stratix10_svc_rsu);
>> +err_free_fifos:
>> + /* only remove from list if list_add_tail() was reached */
>> + if (!list_empty(&controller->node))
>> + list_del(&controller->node);
>> + /* free only the FIFOs that were successfully allocated */
>> + while (i--)
>> + kfifo_free(&chans[i].svc_fifo);
>> stratix10_svc_async_exit(controller);
>> err_destroy_pool:
>> gen_pool_destroy(genpool);
>> +
>> return ret;
>> }
>> static void stratix10_svc_drv_remove(struct platform_device *pdev)
>> {
>> + int i;
>> struct stratix10_svc_controller *ctrl = platform_get_drvdata(pdev);
>> struct stratix10_svc *svc = ctrl->svc;
>> @@ -2011,14 +2032,16 @@ static void stratix10_svc_drv_remove(struct
>> platform_device *pdev)
>> of_platform_depopulate(ctrl->dev);
>> - platform_device_unregister(svc->intel_svc_fcs);
>> platform_device_unregister(svc->stratix10_svc_rsu);
>> - kfifo_free(&ctrl->svc_fifo);
>> - if (ctrl->task) {
>> - kthread_stop(ctrl->task);
>> - ctrl->task = NULL;
>> + for (i = 0; i < SVC_NUM_CHANNEL; i++) {
>> + if (ctrl->chans[i].task) {
>> + kthread_stop(ctrl->chans[i].task);
>> + ctrl->chans[i].task = NULL;
>> + }
>> + kfifo_free(&ctrl->chans[i].svc_fifo);
>> }
>> +
>> if (ctrl->genpool)
>> gen_pool_destroy(ctrl->genpool);
>> list_del(&ctrl->node);
>> diff --git a/include/linux/firmware/intel/stratix10-svc-client.h b/
>> include/linux/firmware/intel/stratix10-svc-client.h
>> index d290060f4c73..91013161e9db 100644
>> --- a/include/linux/firmware/intel/stratix10-svc-client.h
>> +++ b/include/linux/firmware/intel/stratix10-svc-client.h
>> @@ -68,12 +68,12 @@
>> * timeout value used in Stratix10 FPGA manager driver.
>> * timeout value used in RSU driver
>> */
>> -#define SVC_RECONFIG_REQUEST_TIMEOUT_MS 300
>> -#define SVC_RECONFIG_BUFFER_TIMEOUT_MS 720
>> -#define SVC_RSU_REQUEST_TIMEOUT_MS 300
>> +#define SVC_RECONFIG_REQUEST_TIMEOUT_MS 5000
>> +#define SVC_RECONFIG_BUFFER_TIMEOUT_MS 5000
>> +#define SVC_RSU_REQUEST_TIMEOUT_MS 2000
>> #define SVC_FCS_REQUEST_TIMEOUT_MS 2000
>> #define SVC_COMPLETED_TIMEOUT_MS 30000
>> -#define SVC_HWMON_REQUEST_TIMEOUT_MS 300
>> +#define SVC_HWMON_REQUEST_TIMEOUT_MS 2000
>> struct stratix10_svc_chan;
>
> Dinh
Addressed in v6
https://lore.kernel.org/all/20260305093151.2678-1-muhammad.amirul.asyraf.mohamad.jamian@altera.com/
Thanks,
Amirul
© 2016 - 2026 Red Hat, Inc.