[PATCH v6 2/2] mfd: loongson-se: Fix miscellaneous issues
From: Qunqin Zhao <hidden>
Date: 2026-09-07 02:39:04
Also in:
lkml, loongarch
Subsystem:
loongson security engine drivers, multifunction devices (mfd), the rest · Maintainers:
Qunqin Zhao, Lee Jones, Linus Torvalds
From: Qunqin Zhao <zhaoqunqin@loongson.cn> Address multiple historical driver issues discovered by the Sashiko Automation system within the loongson_se_probe() initialization flow and the driver's interrupt service routines [1]. - Add an explicit bounds check in se_irq_handler() before accessing the engines array to prevent an out-of-bounds memory access. - Switch from devm_kmalloc() to devm_kzalloc() and explicitly initialize all engine completion structures in probe() to avoid a kernel panic from complete() dereferencing a NULL wait head when a spurious interrupt fires before child drivers call loongson_se_init_engine(). - Serialize command submission with a new poll_lock spinlock and protect the completion reset with the interrupt lock. Before issuing a command, mask the target interrupt, clear any pending status, and reinitialize the completion under dev_lock, then re-enable the interrupt after the command has been triggered. This closes the race where a stale interrupt from a previously interrupted command could complete the current command's completion. The spin_lock_irq() around the 10 ms poll is dropped so interrupts are not disabled for the full busy-wait. - Fix EPROBE_DEFER handling: propagate the error directly from platform_irq_count() instead of overwriting it with ENODEV so that probe deferral works when the interrupt provider is not yet ready. - Validate dmam_size from firmware against the minimum required size to keep the command buffers of all engines within engine 0's data region and prevent overlapping DMA buffers. - Return the error code from devm_request_irq() instead of silently continuing to prevent an indefinite hang. - Add a loongson_se_stop() cleanup handler registered with devm_add_action_or_reset() before loongson_se_init() so that a failed init still stops the controller and masks all interrupts via devres, preventing DMA access to freed memory. - Zero-initialize the local controller command structures in loongson_se_init() and loongson_se_init_engine() to prevent uninitialized stack data from being written to device registers. - Add the SE_CMD_STOP command definition. Link: https://lore.kernel.org/all/20260618095949.GB1672911@google.com/ (local) [1] Fixes: e551fa3159e3 ("mfd: Add support for Loongson Security Engine chip controller") Signed-off-by: Qunqin Zhao <zhaoqunqin@loongson.cn> --- drivers/mfd/loongson-se.c | 90 +++++++++++++++++++++++++++++---- include/linux/mfd/loongson-se.h | 1 + 2 files changed, 82 insertions(+), 9 deletions(-)
diff --git a/drivers/mfd/loongson-se.c b/drivers/mfd/loongson-se.c
index 7f552a8ee..1d332c418 100644
--- a/drivers/mfd/loongson-se.c
+++ b/drivers/mfd/loongson-se.c@@ -23,6 +23,8 @@ struct loongson_se { void __iomem *base; spinlock_t dev_lock; + /* Synchronizes command submission between users of different engines */ + spinlock_t poll_lock; struct completion cmd_completion; void *dmam_base;
@@ -42,7 +44,7 @@ static int loongson_se_poll(struct loongson_se *se, u32 int_bit) u32 status; int err; - spin_lock_irq(&se->dev_lock); + spin_lock(&se->poll_lock); /* Notify the controller that the engine needs to be started */ writel(int_bit, se->base + SE_L2SINT_SET);
@@ -52,17 +54,48 @@ static int loongson_se_poll(struct loongson_se *se, u32 int_bit) !(status & int_bit), 1, LOONGSON_ENGINE_CMD_TIMEOUT_US); - spin_unlock_irq(&se->dev_lock); + /* + * Re-enable the interrupt that loongson_se_reinit_completion() masked. + * The hardware guarantees that once the interrupt is re-enabled, only + * interrupts for the command just issued can arrive, so a stale + * interrupt from a previously interrupted command can never complete + * this command's completion. + */ + writel(int_bit | readl(se->base + SE_S2LINT_EN), se->base + SE_S2LINT_EN); + + spin_unlock(&se->poll_lock); return err; } +/* + * Prepare a completion for a new command: mask the corresponding interrupt, + * clear any pending interrupt status, and reset the completion. This runs + * under dev_lock so that the IRQ handler cannot race with it. The interrupt + * is re-enabled in loongson_se_poll() after the command has been issued. + */ +static void loongson_se_reinit_completion(struct loongson_se *se, + struct completion *completion, u32 int_bit) +{ + spin_lock_irq(&se->dev_lock); + + writel(readl(se->base + SE_S2LINT_EN) & ~int_bit, se->base + SE_S2LINT_EN); + + writel(int_bit, se->base + SE_S2LINT_CL); + + reinit_completion(completion); + + spin_unlock_irq(&se->dev_lock); +} + static int loongson_se_send_controller_cmd(struct loongson_se *se, struct loongson_se_controller_cmd *cmd) { u32 *send_cmd = (u32 *)cmd; int err, i; + loongson_se_reinit_completion(se, &se->cmd_completion, SE_INT_CONTROLLER); + for (i = 0; i < SE_SEND_CMD_REG_LEN; i++) writel(send_cmd[i], se->base + SE_SEND_CMD_REG + i * 4);
@@ -75,12 +108,16 @@ static int loongson_se_send_controller_cmd(struct loongson_se *se, int loongson_se_send_engine_cmd(struct loongson_se_engine *engine) { + int err; + + loongson_se_reinit_completion(engine->se, &engine->completion, BIT(engine->id)); + /* * After engine initialization, the controller already knows * where to obtain engine commands from. Now all we need to * do is notify the controller that the engine needs to be started. */ - int err = loongson_se_poll(engine->se, BIT(engine->id)); + err = loongson_se_poll(engine->se, BIT(engine->id)); if (err) return err;
@@ -93,7 +130,7 @@ struct loongson_se_engine *loongson_se_init_engine(struct device *dev, int id) { struct loongson_se *se = dev_get_drvdata(dev); struct loongson_se_engine *engine = &se->engines[id]; - struct loongson_se_controller_cmd cmd; + struct loongson_se_controller_cmd cmd = {0}; engine->se = se; engine->id = id;
@@ -155,7 +192,8 @@ static irqreturn_t se_irq_handler(int irq, void *dev_id) /* For engines */ while (int_status) { id = __ffs(int_status); - complete(&se->engines[id].completion); + if (id < SE_ENGINE_MAX) + complete(&se->engines[id].completion); int_status &= ~BIT(id); writel(BIT(id), se->base + SE_S2LINT_CL); }
@@ -167,7 +205,7 @@ static irqreturn_t se_irq_handler(int irq, void *dev_id) static int loongson_se_init(struct loongson_se *se, dma_addr_t addr, int size) { - struct loongson_se_controller_cmd cmd; + struct loongson_se_controller_cmd cmd = {0}; int err; cmd.command_id = SE_CMD_START;
@@ -188,6 +226,17 @@ static const struct mfd_cell engines[] = { { .name = "tpm_loongson" }, }; +static void loongson_se_stop(void *data) +{ + struct loongson_se *se = data; + struct loongson_se_controller_cmd cmd = {0}; + + cmd.command_id = SE_CMD_STOP; + loongson_se_send_controller_cmd(se, &cmd); + + writel(0, se->base + SE_S2LINT_EN); +} + static int loongson_se_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev;
@@ -195,19 +244,34 @@ static int loongson_se_probe(struct platform_device *pdev) int nr_irq, irq, err, i; dma_addr_t paddr; - se = devm_kmalloc(dev, sizeof(*se), GFP_KERNEL); + se = devm_kzalloc(dev, sizeof(*se), GFP_KERNEL); if (!se) return -ENOMEM; dev_set_drvdata(dev, se); init_completion(&se->cmd_completion); spin_lock_init(&se->dev_lock); + spin_lock_init(&se->poll_lock); mutex_init(&se->engine_init_lock); + for (i = 0; i < SE_ENGINE_MAX; i++) + init_completion(&se->engines[i].completion); + dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64)); if (device_property_read_u32(dev, "dmam_size", &se->dmam_size)) return -ENODEV; + /* + * Engine 0 does not exist and its data buffer is reused as the command + * buffer for the other engines. The command buffers of all engines + * therefore must fit within engine 0's data region, i.e. + * dmam_size / SE_ENGINE_MAX >= SE_ENGINE_MAX * 2 * SE_ENGINE_CMD_SIZE. + * Enforce this to keep each engine's data and command buffers from + * overlapping. + */ + if ((se->dmam_size / SE_ENGINE_MAX) < (SE_ENGINE_MAX * 2 * SE_ENGINE_CMD_SIZE)) + return -EINVAL; + se->dmam_base = dmam_alloc_coherent(dev, se->dmam_size, &paddr, GFP_KERNEL); if (!se->dmam_base) return -ENOMEM;
@@ -217,20 +281,28 @@ static int loongson_se_probe(struct platform_device *pdev) return PTR_ERR(se->base); nr_irq = platform_irq_count(pdev); - if (nr_irq <= 0) + if (nr_irq == 0) return -ENODEV; + if (nr_irq < 0) + return nr_irq; writel(SE_INT_ALL, se->base + SE_S2LINT_CL); for (i = 0; i < nr_irq; i++) { irq = platform_get_irq(pdev, i); err = devm_request_irq(dev, irq, se_irq_handler, IRQF_SHARED, "loongson-se", se); - if (err) + if (err) { dev_err(dev, "failed to request IRQ: %d\n", irq); + return err; + } } writel(SE_INT_ALL, se->base + SE_S2LINT_EN); + err = devm_add_action_or_reset(dev, loongson_se_stop, se); + if (err) + return err; + err = loongson_se_init(se, paddr, se->dmam_size); if (err) return err;
diff --git a/include/linux/mfd/loongson-se.h b/include/linux/mfd/loongson-se.h
index 07afa0c25..8237ccab7 100644
--- a/include/linux/mfd/loongson-se.h
+++ b/include/linux/mfd/loongson-se.h@@ -9,6 +9,7 @@ #define SE_SEND_CMD_REG_LEN 0x8 /* Controller command ID */ #define SE_CMD_START 0x0 +#define SE_CMD_STOP 0x1 #define SE_CMD_SET_DMA 0x3 #define SE_CMD_SET_ENGINE_CMDBUF 0x4
--
2.47.2