diff options
| author | Dinh Nguyen <dinguyen@kernel.org> | 2026-05-15 17:15:55 -0500 |
|---|---|---|
| committer | Dinh Nguyen <dinguyen@kernel.org> | 2026-06-08 22:37:35 +0900 |
| commit | 30b1f763bfc1b0db42f58904ec378066cc886c44 (patch) | |
| tree | 433e6858f3d949733ee7ea7662ac012c70958ee9 /drivers/firmware | |
| parent | 4b0a32016347bfd6ae9849f21b1b767905f68d14 (diff) | |
| download | linux-next-30b1f763bfc1b0db42f58904ec378066cc886c44.tar.gz linux-next-30b1f763bfc1b0db42f58904ec378066cc886c44.zip | |
firmware: stratix10-rsu: avoid blocking reboot_image sysfs when busy
Writes to the reboot_image sysfs attribute went through rsu_send_msg(),
which unconditionally takes priv->lock with mutex_lock(). If another RSU
operation is in flight (e.g. a DCMF status query from probe or a
concurrent sysfs read path), userspace writers get stuck in the kernel
waiting on the mutex instead of being told the device is busy.
Split rsu_send_msg() into an inner __rsu_send_msg_locked() helper that
performs the SMC transaction with the caller holding priv->lock, plus
two thin wrappers: rsu_send_msg() preserves the original blocking
behaviour for existing callers, and rsu_try_send_msg() uses
mutex_trylock() and returns -EBUSY immediately when the lock is held.
Use rsu_try_send_msg() from reboot_image_store() so the write returns
-EBUSY without blocking when an RSU operation is already running.
Userspace can retry on -EBUSY. No functional change for other sysfs
attributes.
This keeps blocking rsu_send_msg() for existing callers, add
rsu_try_send_msg() with -EBUSY only for reboot_image_store(). That
matches the original goal (avoid a second reboot_image write blocking
behind priv->lock) without changing sysfs behaviour for the other
attributes. The earlier idea of using mutex_trylock() in all of
rsu_send_msg() and returning -EAGAIN would have been harder to justify
for userspace (echo does not retry on that).
Tze Yee tested the patch on an Agilex SoC devkit.
[Test 1] Idle reboot_image write (success path)
Result:
# insmod stratix10-rsu.ko
# echo 0x01000000 > .../reboot_image
# echo "exit=$?"
exit=0
# ./rsu_client --log
VERSION: 0x00000202
STATE: 0x00000000
CURRENT IMAGE: 0x0000000001000000
FAIL IMAGE: 0x0000000000000000
ERROR LOC: 0x00000000
ERROR DETAILS: 0x00000000
RETRY COUNTER: 0x00000000
Operation completed
[Test 2] reboot_image while priv->lock is held (-EBUSY path)
To get a deterministic busy window without flooding the service layer,
add a local debug helper (module parameter debug_hold_lock_sec +
kthread that holds priv->lock for N seconds after probe).
Result:
# insmod stratix10-rsu.ko debug_hold_lock_sec=60
[ 121.220904] stratix10-rsu stratix10-rsu.0: TEST: RSU lock held for
60 s - try reboot_image now
# echo 0x01000000 > .../reboot_image
-sh: echo: write error: Device or resource busy
# echo "during hold: exit=$?"
during hold: exit=1
[ 183.268706] stratix10-rsu stratix10-rsu.0: TEST: RSU lock released
# echo 0x01000000 > .../reboot_image
# echo "after release: exit=$?"
after release: exit=0
Together, these results match the intended behaviour: reboot_image
fails fast with -EBUSY when the RSU mutex is already held, and
succeeds once the lock is available.
Assisted-by: Claude:claude-opus-4-7
Tested-by: Tze Yee Ng <tze.yee.ng@altera.com>
Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
Diffstat (limited to 'drivers/firmware')
| -rw-r--r-- | drivers/firmware/stratix10-rsu.c | 85 |
1 files changed, 73 insertions, 12 deletions
diff --git a/drivers/firmware/stratix10-rsu.c b/drivers/firmware/stratix10-rsu.c index e1912108a0fe..018a933943fb 100644 --- a/drivers/firmware/stratix10-rsu.c +++ b/drivers/firmware/stratix10-rsu.c @@ -244,27 +244,26 @@ static void rsu_async_get_spt_table_callback(struct device *dev, } /** - * rsu_send_msg() - send a message to Intel service layer + * __rsu_send_msg_locked() - send a message to Intel service layer * @priv: pointer to rsu private data * @command: RSU status or update command * @arg: the request argument, the bitstream address or notify status * @callback: function pointer for the callback (status or update) * - * Start an Intel service layer transaction to perform the SMC call that - * is necessary to get RSU boot log or set the address of bitstream to - * boot after reboot. + * Perform the actual SMC transaction. The caller must hold @priv->lock. * - * Returns 0 on success or -ETIMEDOUT on error. + * Returns 0 on success or a negative errno on failure. */ -static int rsu_send_msg(struct stratix10_rsu_priv *priv, - enum stratix10_svc_command_code command, - unsigned long arg, - rsu_callback callback) +static int __rsu_send_msg_locked(struct stratix10_rsu_priv *priv, + enum stratix10_svc_command_code command, + unsigned long arg, + rsu_callback callback) { struct stratix10_svc_client_msg msg; int ret; - mutex_lock(&priv->lock); + lockdep_assert_held(&priv->lock); + reinit_completion(&priv->completion); priv->client.receive_cb = callback; @@ -293,6 +292,59 @@ static int rsu_send_msg(struct stratix10_rsu_priv *priv, status_done: stratix10_svc_done(priv->chan); + return ret; +} + +/** + * rsu_send_msg() - send a message to Intel service layer + * @priv: pointer to rsu private data + * @command: RSU status or update command + * @arg: the request argument, the bitstream address or notify status + * @callback: function pointer for the callback (status or update) + * + * Start an Intel service layer transaction to perform the SMC call that + * is necessary to get RSU boot log or set the address of bitstream to + * boot after reboot. This call will block until the RSU lock can be + * acquired. + * + * Returns 0 on success or a negative errno on failure. + */ +static int rsu_send_msg(struct stratix10_rsu_priv *priv, + enum stratix10_svc_command_code command, + unsigned long arg, + rsu_callback callback) +{ + int ret; + + mutex_lock(&priv->lock); + ret = __rsu_send_msg_locked(priv, command, arg, callback); + mutex_unlock(&priv->lock); + return ret; +} + +/** + * rsu_try_send_msg() - non-blocking variant of rsu_send_msg() + * @priv: pointer to rsu private data + * @command: RSU status or update command + * @arg: the request argument, the bitstream address or notify status + * @callback: function pointer for the callback (status or update) + * + * Same as rsu_send_msg() but returns -EBUSY immediately when another + * RSU operation is already in flight, instead of waiting for the lock. + * + * Returns 0 on success, -EBUSY if the RSU is busy, or another negative + * errno on failure. + */ +static int rsu_try_send_msg(struct stratix10_rsu_priv *priv, + enum stratix10_svc_command_code command, + unsigned long arg, + rsu_callback callback) +{ + int ret; + + if (!mutex_trylock(&priv->lock)) + return -EBUSY; + ret = __rsu_send_msg_locked(priv, command, arg, callback); mutex_unlock(&priv->lock); return ret; } @@ -595,8 +647,17 @@ static ssize_t reboot_image_store(struct device *dev, if (ret) return ret; - ret = rsu_send_msg(priv, COMMAND_RSU_UPDATE, - address, rsu_command_callback); + /* + * Use the non-blocking variant so a write to this sysfs attribute + * does not stall the caller while another RSU operation is in + * flight. Userspace can retry on -EBUSY. + */ + ret = rsu_try_send_msg(priv, COMMAND_RSU_UPDATE, + address, rsu_command_callback); + if (ret == -EBUSY) { + dev_dbg(dev, "RSU busy, reboot_image write rejected\n"); + return ret; + } if (ret) { dev_err(dev, "Error, RSU update returned %i\n", ret); return ret; |
