[PATCH] drivers: scmi: Add completion timeout handling for raw mode transfers

[PATCH] drivers: scmi: Add completion timeout handling for raw mode transfers

[Artem Shimko]

Fix race conditions in SCMI raw mode implementation by adding proper
completion timeout handling. Multiple tests[1] in the SCMI test suite
were failing due to early clearing of SCMI_XFER_FLAG_IS_RAW flag in
scmi_xfer_raw_put() function.

TRANS=raw
PROTOCOLS=base,clock,power_domain,performance,system_power,sensor,
voltage,reset,powercap,pin_control VERBOSE=5

The root cause:
Tests were failing on poll() system calls with this condition:
    if (!raw || (idx == SCMI_RAW_REPLY_QUEUE && !SCMI_XFER_IS_RAW(xfer)))
        return;

The SCMI_XFER_FLAG_IS_RAW flag was being cleared prematurely before
the transfer completion was properly acknowledged, causing the poll
to return on timeout and tests to fail.

Сhanges implemented:
1. Add completion wait with timeout in  scmi_xfer_raw_worker()
2. Signal completion in scmi_raw_message_report()

This ensures:
- Proper synchronization between transfer completion and flag clearing
- Prevention of indefinite blocking with timeout safety mechanism
- Stable test execution by maintaining correct flag states

TRANS=raw
PROTOCOLS=base,clock,power_domain,performance,system_power,sensor,
voltage,reset,powercap,pin_control VERBOSE=5

An example of a random test failure:
 817: Voltage get ext name for invalid domain
     [Check 1] Get extended name for invalid domain
       MSG HDR        : 0x04585c09
       NUM PARAM      : 1
       PARAMETER[00]  : 0x0000000c
       CHECK STATUS   : PASSED [SCMI_NOT_FOUND_ERR]
       CHECK HEADER   : PASSED [0x04585c09]
       RETURN COUNT   : 0
       NUM DOMAINS    : 11
       VOLTAGE DOMAIN : 0
     [Check 2] Get extended name for unsupp. domain
       MSG HDR        : 0x045c5c09
       NUM PARAM      : 1
       PARAMETER[00]  : 0x00000000
       CHECK STATUS   : FAILED
           EXPECTED   : SCMI_NOT_FOUND_ERR
           RECEIVED   : SCMI_GENERIC_ERROR  : NON CONFORMANT 

After making these changes, the tests stopped failing.

mount -t debugfs none /sys/kernel/debug 
scmi_test_agent
[  127.865032] arm-scmi arm-scmi.1.auto: Resetting SCMI Raw stack.
[  128.360503] arm-scmi arm-scmi.1.auto: Using Base channel for protocol 0x12
tail -n 6 arm_scmi_test_log.txt
****************************************************
  TOTAL TESTS: 167    PASSED: 120    FAILED: 0    SKIPPED: 47
****************************************************

Link [1] https://gitlab.arm.com/tests/scmi-tests/-/releases

Signed-off-by: Artem Shimko <a.shimko.dev@gmail.com>
---
Hello maintainers and reviewers,

This patch addresses a race condition in the SCMI raw mode implementation
that was causing multiple test failures in the SCMI test suite.

The issue manifested as poll() timeouts in tests when using raw mode
transfers. The root cause was premature completion signaling and
SCMI_XFER_FLAG_IS_RAW flag clearing before transfers were fully
acknowledged.

Thank you for your consideration.

Best regards,
Artem Shimko

 drivers/firmware/arm_scmi/raw_mode.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/firmware/arm_scmi/raw_mode.c b/drivers/firmware/arm_scmi/raw_mode.c
index 73db5492ab44..130d45192beb 100644
--- a/drivers/firmware/arm_scmi/raw_mode.c
+++ b/drivers/firmware/arm_scmi/raw_mode.c
@@ -468,6 +468,14 @@ static void scmi_xfer_raw_worker(struct work_struct *work)
 
 		ret = scmi_xfer_raw_wait_for_message_response(cinfo, xfer,
 							      timeout_ms);
+		if (!ret)
+			if (!wait_for_completion_timeout(&xfer->done, timeout_ms)) {
+				dev_err(dev,
+					"timed out in RAW resp - HDR:%08X\n",
+					pack_scmi_header(&xfer->hdr));
+				ret = -ETIMEDOUT;
+			}
+
 		if (!ret && xfer->hdr.status)
 			ret = scmi_to_linux_errno(xfer->hdr.status);
 
@@ -1381,6 +1389,8 @@ void scmi_raw_message_report(void *r, struct scmi_xfer *xfer,
 	if (!raw || (idx == SCMI_RAW_REPLY_QUEUE && !SCMI_XFER_IS_RAW(xfer)))
 		return;
 
+	complete(&xfer->done);
+
 	dev = raw->handle->dev;
 	q = scmi_raw_queue_select(raw, idx,
 				  SCMI_XFER_IS_CHAN_SET(xfer) ? chan_id : 0);
-- 
2.43.0

Re: [PATCH] drivers: scmi: Add completion timeout handling for raw mode transfers

[Cristian Marussi]

On Mon, Sep 29, 2025 at 05:28:55PM +0300, Artem Shimko wrote:
> Fix race conditions in SCMI raw mode implementation by adding proper
> completion timeout handling. Multiple tests[1] in the SCMI test suite
> were failing due to early clearing of SCMI_XFER_FLAG_IS_RAW flag in
> scmi_xfer_raw_put() function.

Hi Artem, 

> 
> TRANS=raw
> PROTOCOLS=base,clock,power_domain,performance,system_power,sensor,
> voltage,reset,powercap,pin_control VERBOSE=5
> 

Glad to see that someone is using the test-suite im RAW mode out there
in the real world :P

> The root cause:
> Tests were failing on poll() system calls with this condition:
>     if (!raw || (idx == SCMI_RAW_REPLY_QUEUE && !SCMI_XFER_IS_RAW(xfer)))
>         return;
> 
> The SCMI_XFER_FLAG_IS_RAW flag was being cleared prematurely before
> the transfer completion was properly acknowledged, causing the poll
> to return on timeout and tests to fail.
> 

I have to say I had seen some anomalies, hardly reproducible, and I
think you have a point here, good catch !

The xfer flags handling is racy and it could lead to the observed
anomalies...BUT....

> Сhanges implemented:
> 1. Add completion wait with timeout in  scmi_xfer_raw_worker()
> 2. Signal completion in scmi_raw_message_report()
> 

.. I think the fix in your patch is overkill...it is really not needed
to add another layer of synchronization like you are doing, because the
bug is really around the xfer put and it is very well evident, now that
you have pointed at it...

What about, instead of your patch, this:

-----8<----
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 801d59e6b3bc..d3453130896a 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -822,6 +822,8 @@ __scmi_xfer_put(struct scmi_xfers_info *minfo, struct scmi_xfer *xfer)
 
                        scmi_dec_count(info->dbg->counters, XFERS_INFLIGHT);
                }
+               xfer->flags &= ~SCMI_XFER_FLAG_IS_RAW;
+               xfer->flags &= ~SCMI_XFER_FLAG_CHAN_SET;
                hlist_add_head(&xfer->node, &minfo->free_xfers);
        }
        spin_unlock_irqrestore(&minfo->xfer_lock, flags);
@@ -840,8 +842,6 @@ void scmi_xfer_raw_put(const struct scmi_handle *handle, struct scmi_xfer *xfer)
 {
        struct scmi_info *info = handle_to_scmi_info(handle);
 
-       xfer->flags &= ~SCMI_XFER_FLAG_IS_RAW;
-       xfer->flags &= ~SCMI_XFER_FLAG_CHAN_SET;
        return __scmi_xfer_put(&info->tx_minfo, xfer);
 }
---->8----

..because the xfer itself is refcounted properly, BUT as of now the
flags are cleared out of the block inside __scmi_xfer_put:

	if (refcount_dec_and_test(&xfer->users))

...and that is the problem.

The above patch solves for me all the observed anomalies...

... probably EVEN BETTER if you just clear all:

---8<-----
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 801d59e6b3bc..914510de3abb 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -822,6 +822,7 @@ __scmi_xfer_put(struct scmi_xfers_info *minfo, struct scmi_xfer *xfer)
 
                        scmi_dec_count(info->dbg->counters, XFERS_INFLIGHT);
                }
+               xfer->flags = 0;
                hlist_add_head(&xfer->node, &minfo->free_xfers);
        }
        spin_unlock_irqrestore(&minfo->xfer_lock, flags);
@@ -840,8 +841,6 @@ void scmi_xfer_raw_put(const struct scmi_handle *handle, struct scmi_xfer *xfer)
 {
        struct scmi_info *info = handle_to_scmi_info(handle);
 
-       xfer->flags &= ~SCMI_XFER_FLAG_IS_RAW;
-       xfer->flags &= ~SCMI_XFER_FLAG_CHAN_SET;
        return __scmi_xfer_put(&info->tx_minfo, xfer);
 }

---->8----
 
...to simply clear all flags when xfer is put...

If this solves for you too, please send a new patch with also a Fixes tag.

Last but not least, are you running the test-suite against a Kernel
configured with RAW cohexistence disabled ?

	CONFIG_ARM_SCMI_RAW_MODE_SUPPORT_COEX = n

...because having RAW COEX enabled when running the test-suite could lead to more
false positives even with your patch applied, due to the interference of regular
drivers..

Thanks for this !
Cristian

[PATCH v2] drivers: scmi: Add completion timeout handling for raw mode transfers

[Artem Shimko]

Fix race conditions in SCMI raw mode implementation by adding proper
completion timeout handling. Multiple tests in the SCMI test suite
were failing due to early clearing of SCMI_XFER_FLAG_IS_RAW flag in
scmi_xfer_raw_put() function.

TRANS=raw
PROTOCOLS=base,clock,power_domain,performance,system_power,sensor,
voltage,reset,powercap,pin_control VERBOSE=5

The root cause:
Tests were failing on poll() system calls with this condition:
    if (!raw || (idx == SCMI_RAW_REPLY_QUEUE && !SCMI_XFER_IS_RAW(xfer)))
        return;

The SCMI_XFER_FLAG_IS_RAW flag was being cleared prematurely before
the transfer completion was properly acknowledged, causing the poll
to return on timeout and tests to fail.

Fix ensures:
- Proper synchronization between transfer completion and flag clearing
- Stable test execution by maintaining correct flag states

An example of a random test failure:
 817: Voltage get ext name for invalid domain
     [Check 1] Get extended name for invalid domain
       MSG HDR        : 0x04585c09
       NUM PARAM      : 1
       PARAMETER[00]  : 0x0000000c
       CHECK STATUS   : PASSED [SCMI_NOT_FOUND_ERR]
       CHECK HEADER   : PASSED [0x04585c09]
       RETURN COUNT   : 0
       NUM DOMAINS    : 11
       VOLTAGE DOMAIN : 0
     [Check 2] Get extended name for unsupp. domain
       MSG HDR        : 0x045c5c09
       NUM PARAM      : 1
       PARAMETER[00]  : 0x00000000
       CHECK STATUS   : FAILED
           EXPECTED   : SCMI_NOT_FOUND_ERR
           RECEIVED   : SCMI_GENERIC_ERROR  : NON CONFORMANT

After making these changes, the tests stopped failing.

$mount -t debugfs none /sys/kernel/debug
$scmi_test_agent
[  127.865032] arm-scmi arm-scmi.1.auto: Resetting SCMI Raw stack.
[  128.360503] arm-scmi arm-scmi.1.auto: Using Base channel for protocol 0x12
$tail -n 6 arm_scmi_test_log.txt
****************************************************
  TOTAL TESTS: 167    PASSED: 120    FAILED: 0    SKIPPED: 47
****************************************************

An ftrace log with of passed test:
0)               |  scmi_rx_callback()
0)               |    scmi_raw_message_report()
7)               |    scmi_xfer_raw_wait_for_message_response()
7) + 22.000 us   |      scmi_wait_for_reply();
0)               |        /* scmi_raw_message_report*/
7)               |    scmi_xfer_raw_put()

An ftrace log with of failed test:
0)               |  scmi_rx_callback() {
0)               |    scmi_raw_message_report()
5)               |    scmi_xfer_raw_wait_for_message_response()
5) ! 383.000 us  |      scmi_wait_for_reply();
5)               |    scmi_xfer_raw_put() {
0)               |  /* scmi_raw_message_report*/

Link [1] https://gitlab.arm.com/tests/scmi-tests/-/releases

Fixes: 3095a3e25d8f7 (firmware: arm_scmi: Add xfer helpers to provide raw access)
Suggested-by: Cristian Marussi <cristian.marussi@arm.com>
Signed-off-by: Artem Shimko <a.shimko.dev@gmail.com>
---
Hi Cristian,

Good point about CONFIG_ARM_SCMI_RAW_MODE_SUPPORT_COEX. 

I can confirm this setting doesn't impact the test failures in my environment.
The issue reproduces consistently with COEX both enabled and disabled.

Thank you!

Best regards,
Artem Shimko

ChangeLog:
  v1:
    * https://lore.kernel.org/arm-scmi/20250929142856.540590-1-a.shimko.dev@gmail.com/
  v2:
    * Use simpler approach suggested by Cristian Marussi
    * Clear all xfer flags in __scmi_xfer_put() under spinlock protection  
    * Add Fixes tag as requested
    * Drop completion timeout mechanism from v1

 drivers/firmware/arm_scmi/driver.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index bd56a877fdfc..0976bfdbb44b 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -821,6 +821,7 @@ __scmi_xfer_put(struct scmi_xfers_info *minfo, struct scmi_xfer *xfer)
 
 			scmi_dec_count(info->dbg->counters, XFERS_INFLIGHT);
 		}
+		xfer->flags = 0;
 		hlist_add_head(&xfer->node, &minfo->free_xfers);
 	}
 	spin_unlock_irqrestore(&minfo->xfer_lock, flags);
@@ -839,8 +840,6 @@ void scmi_xfer_raw_put(const struct scmi_handle *handle, struct scmi_xfer *xfer)
 {
 	struct scmi_info *info = handle_to_scmi_info(handle);
 
-	xfer->flags &= ~SCMI_XFER_FLAG_IS_RAW;
-	xfer->flags &= ~SCMI_XFER_FLAG_CHAN_SET;
 	return __scmi_xfer_put(&info->tx_minfo, xfer);
 }
 
-- 
2.43.0

Re: [PATCH v2] drivers: scmi: Add completion timeout handling for raw mode transfers

[Cristian Marussi]

On Fri, Oct 03, 2025 at 10:22:33PM +0300, Artem Shimko wrote:
> Fix race conditions in SCMI raw mode implementation by adding proper
> completion timeout handling. Multiple tests in the SCMI test suite
> were failing due to early clearing of SCMI_XFER_FLAG_IS_RAW flag in
> scmi_xfer_raw_put() function.

Hi Artem,

LGTM now .... but ... now the commit message is no more describing what you
are doing, right ? ... it is no more handled with completions...

Please fix the commit message to reflect what you are doing; also it
would be good to at first explain the issue (like you are doing
already), and THEN describe the solution applied...

Following the rules in "Describe your changes" in:

	https://www.kernel.org/doc/html/v6.17/process/submitting-patches.html

(if you already know this ... just ignore me)

> 
> TRANS=raw
> PROTOCOLS=base,clock,power_domain,performance,system_power,sensor,
> voltage,reset,powercap,pin_control VERBOSE=5
> 
> The root cause:
> Tests were failing on poll() system calls with this condition:
>     if (!raw || (idx == SCMI_RAW_REPLY_QUEUE && !SCMI_XFER_IS_RAW(xfer)))
>         return;
> 
> The SCMI_XFER_FLAG_IS_RAW flag was being cleared prematurely before
> the transfer completion was properly acknowledged, causing the poll
> to return on timeout and tests to fail.
> 
> Fix ensures:
> - Proper synchronization between transfer completion and flag clearing
> - Stable test execution by maintaining correct flag states
> 
> An example of a random test failure:
>  817: Voltage get ext name for invalid domain
>      [Check 1] Get extended name for invalid domain
>        MSG HDR        : 0x04585c09
>        NUM PARAM      : 1
>        PARAMETER[00]  : 0x0000000c
>        CHECK STATUS   : PASSED [SCMI_NOT_FOUND_ERR]
>        CHECK HEADER   : PASSED [0x04585c09]
>        RETURN COUNT   : 0
>        NUM DOMAINS    : 11
>        VOLTAGE DOMAIN : 0
>      [Check 2] Get extended name for unsupp. domain
>        MSG HDR        : 0x045c5c09
>        NUM PARAM      : 1
>        PARAMETER[00]  : 0x00000000
>        CHECK STATUS   : FAILED
>            EXPECTED   : SCMI_NOT_FOUND_ERR
>            RECEIVED   : SCMI_GENERIC_ERROR  : NON CONFORMANT
> 
> After making these changes, the tests stopped failing.
> 

I think also you can trim and drop this further explanation down here...
you have described clearly enough the issue above...

> $mount -t debugfs none /sys/kernel/debug
> $scmi_test_agent
> [  127.865032] arm-scmi arm-scmi.1.auto: Resetting SCMI Raw stack.
> [  128.360503] arm-scmi arm-scmi.1.auto: Using Base channel for protocol 0x12
> $tail -n 6 arm_scmi_test_log.txt
> ****************************************************
>   TOTAL TESTS: 167    PASSED: 120    FAILED: 0    SKIPPED: 47
> ****************************************************
> 
> An ftrace log with of passed test:
> 0)               |  scmi_rx_callback()
> 0)               |    scmi_raw_message_report()
> 7)               |    scmi_xfer_raw_wait_for_message_response()
> 7) + 22.000 us   |      scmi_wait_for_reply();
> 0)               |        /* scmi_raw_message_report*/
> 7)               |    scmi_xfer_raw_put()
> 
> An ftrace log with of failed test:
> 0)               |  scmi_rx_callback() {
> 0)               |    scmi_raw_message_report()
> 5)               |    scmi_xfer_raw_wait_for_message_response()
> 5) ! 383.000 us  |      scmi_wait_for_reply();
> 5)               |    scmi_xfer_raw_put() {
> 0)               |  /* scmi_raw_message_report*/
> 
> Link [1] https://gitlab.arm.com/tests/scmi-tests/-/releases
> 
> Fixes: 3095a3e25d8f7 (firmware: arm_scmi: Add xfer helpers to provide raw access)
> Suggested-by: Cristian Marussi <cristian.marussi@arm.com>
> Signed-off-by: Artem Shimko <a.shimko.dev@gmail.com>
> ---
> Hi Cristian,
> 
> Good point about CONFIG_ARM_SCMI_RAW_MODE_SUPPORT_COEX. 
> 
> I can confirm this setting doesn't impact the test failures in my environment.
> The issue reproduces consistently with COEX both enabled and disabled.
> 
> Thank you!
> 

Good...

Thanks to you
Cristian

[PATCH v3] firmware: arm_scmi: Fix premature SCMI_XFER_FLAG_IS_RAW clearing in raw mode

[Artem Shimko]

The SCMI_XFER_FLAG_IS_RAW flag was being cleared prematurely in
scmi_xfer_raw_put() before transfer completion was properly acknowledged
by the raw message handlers.

Move the SCMI_XFER_FLAG_IS_RAW and SCMI_XFER_FLAG_CHAN_SET flag clearing
from scmi_xfer_raw_put() to __scmi_xfer_put() to ensure flags remain set
throughout the entire raw message processing pipeline until the transfer
returns to the free pool.

Fixes: 3095a3e25d8f ("firmware: arm_scmi: Add xfer helpers to provide raw access")
Suggested-by: Cristian Marussi <cristian.marussi@arm.com>
Signed-off-by: Artem Shimko <a.shimko.dev@gmail.com>
---
Hi Cristian,

Oh, sorry, sure.

Could you please have a look to a new version of commit.

Thank you!

Best regards,
Artem

ChangeLog:
  v1:
    * https://lore.kernel.org/arm-scmi/20250929142856.540590-1-a.shimko.dev@gmail.com/
  v2:
    * Use simpler approach suggested by Cristian Marussi
    * Clear all xfer flags in __scmi_xfer_put() under spinlock protection  
    * Add Fixes tag as requested
    * Drop completion timeout mechanism from v1
  v3:
    * Updated commit description as suggested by Cristian Marussi

 drivers/firmware/arm_scmi/driver.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index bd56a877fdfc..0976bfdbb44b 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -821,6 +821,7 @@ __scmi_xfer_put(struct scmi_xfers_info *minfo, struct scmi_xfer *xfer)
 
 			scmi_dec_count(info->dbg->counters, XFERS_INFLIGHT);
 		}
+		xfer->flags = 0;
 		hlist_add_head(&xfer->node, &minfo->free_xfers);
 	}
 	spin_unlock_irqrestore(&minfo->xfer_lock, flags);
@@ -839,8 +840,6 @@ void scmi_xfer_raw_put(const struct scmi_handle *handle, struct scmi_xfer *xfer)
 {
 	struct scmi_info *info = handle_to_scmi_info(handle);
 
-	xfer->flags &= ~SCMI_XFER_FLAG_IS_RAW;
-	xfer->flags &= ~SCMI_XFER_FLAG_CHAN_SET;
 	return __scmi_xfer_put(&info->tx_minfo, xfer);
 }
 
-- 
2.43.0

Re: [PATCH v3] firmware: arm_scmi: Fix premature SCMI_XFER_FLAG_IS_RAW clearing in raw mode

[Cristian Marussi]

On Wed, Oct 08, 2025 at 12:10:57PM +0300, Artem Shimko wrote:
> The SCMI_XFER_FLAG_IS_RAW flag was being cleared prematurely in
> scmi_xfer_raw_put() before transfer completion was properly acknowledged
> by the raw message handlers.
> 
> Move the SCMI_XFER_FLAG_IS_RAW and SCMI_XFER_FLAG_CHAN_SET flag clearing
> from scmi_xfer_raw_put() to __scmi_xfer_put() to ensure flags remain set
> throughout the entire raw message processing pipeline until the transfer
> returns to the free pool.
> 
> Fixes: 3095a3e25d8f ("firmware: arm_scmi: Add xfer helpers to provide raw access")
> Suggested-by: Cristian Marussi <cristian.marussi@arm.com>
> Signed-off-by: Artem Shimko <a.shimko.dev@gmail.com>

Perfect, LGTM.

Reviewed-by: Cristian Marussi <cristian.marussi@arm.com>

Thanks,
Cristian

Re: [PATCH v3] firmware: arm_scmi: Fix premature SCMI_XFER_FLAG_IS_RAW clearing in raw mode

[Sudeep Holla]

On Wed, 08 Oct 2025 12:10:57 +0300, Artem Shimko wrote:
> The SCMI_XFER_FLAG_IS_RAW flag was being cleared prematurely in
> scmi_xfer_raw_put() before transfer completion was properly acknowledged
> by the raw message handlers.
> 
> Move the SCMI_XFER_FLAG_IS_RAW and SCMI_XFER_FLAG_CHAN_SET flag clearing
> from scmi_xfer_raw_put() to __scmi_xfer_put() to ensure flags remain set
> throughout the entire raw message processing pipeline until the transfer
> returns to the free pool.
> 
> [...]

Applied to sudeep.holla/linux (for-next/scmi/fixes), thanks!

[1/1] firmware: arm_scmi: Fix premature SCMI_XFER_FLAG_IS_RAW clearing in raw mode
      https://git.kernel.org/sudeep.holla/c/20b93a0088a5
-- 
Regards,
Sudeep