Re: [PATCH v7 2/9] soc: qcom: Add QMI TMD support for remote thermal mitigation

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Returning success without saving the requested mitigation state when QMI is disconnected permanently loses thermal mitigation updates.
- [Medium] TOCTOU race condition when reading `tmd->cur_state` during service reconnection causes permanent thermal state desynchronization.
--

--- Patch [2]: [PATCH v7 2/9] soc: qcom: Add QMI TMD support for remote thermal mitigation ---
commit 02c2244c0a2cebd6be4e1bf4e41d51362be19b5f
Author: Casey Connolly <casey.connolly@linaro.org>

soc: qcom: Add QMI TMD support for remote thermal mitigation

This commit adds support for Qualcomm Messaging Interface (QMI) based
Thermal Mitigation Device (TMD) cooling devices provided by remote subsystems.
It allows client drivers to discover the service and forward cooling state
updates to remote processors.

diff --git a/drivers/soc/qcom/qmi_tmd.c b/drivers/soc/qcom/qmi_tmd.c
new file mode 100644
index 0000000000000..37e6ef19a0b34
--- /dev/null
+++ b/drivers/soc/qcom/qmi_tmd.c
[ ... ]
@@ -236,25 +236,54 @@
> +/* Notify the remote subsystem of the requested cooling state */
> +static int qmi_tmd_send_state_request(struct qmi_tmd *tmd, int state)
> +{
> +	struct tmd_set_level_resp resp = { 0 };
> +	struct tmd_set_level_req req = { 0 };
> +	struct qmi_tmd_client *qmi_tmd_cli = tmd->qmi_tmd_cli;
> +	struct qmi_txn txn;
> +	int ret = 0;
> +
> +	guard(mutex)(&qmi_tmd_cli->mutex);
> +
> +	if (!qmi_tmd_cli->connection_active)
> +		return 0;

[Severity: High]
Will returning 0 here without saving the new state to tmd->cur_state cause
thermal mitigation updates to be permanently lost?

If the thermal core requests a new state while the remote QMI service is
down, this returns success but drops the state update. When the service
reconnects, qmi_tmd_register() will re-send the old tmd->cur_state to the
hardware. 

Could this leave the hardware running unmitigated while the thermal framework
believes it is actively mitigating?

[ ... ]
@@ -298,34 +327,33 @@
> +static int qmi_tmd_register(struct qmi_tmd_client *qmi_tmd_cli,
> +			    const char *label, u8 max_state)
> +{
> +	struct device *dev = qmi_tmd_cli->dev;
> +	struct qmi_tmd *tmd;
> +	int index;
> +
> +	for (index = 0; index < qmi_tmd_cli->num_tmds; index++) {
> +		tmd = &qmi_tmd_cli->tmds[index];
> +
> +		if (!strncasecmp(tmd->name, label,
> +				 QMI_TMD_DEV_ID_LEN_MAX + 1))
> +			goto found;
> +	}
> +
> +	dev_dbg(qmi_tmd_cli->dev,
> +		"TMD '%s' available in firmware but not specified in DT\n",
> +		label);
> +	return 0;
> +
> +found:
> +	tmd->max_state = max_state;
> +
> +	/*
> +	 * If the cooling device already exists then the QMI service went away and
> +	 * came back. So just make sure the current cooling device state is
> +	 * reflected on the remote side and then return.
> +	 */
> +	if (tmd->cdev)
> +		return qmi_tmd_send_state_request(tmd, tmd->cur_state);

[Severity: Medium]
Is there a race condition between reading tmd->cur_state here and concurrent 
updates from the thermal core?

When the QMI service reconnects, tmd->cur_state is read outside of the 
qmi_tmd_cli->mutex. If a thermal trip point is crossed concurrently, could
the following sequence occur:

CPU1: qmi_tmd_register()
  reads tmd->cur_state (e.g., state A) locklessly
  
CPU2: qmi_tmd_set_cur_state()
  acquires qmi_tmd_cli->mutex inside qmi_tmd_send_state_request()
  sends state B to hardware
  updates tmd->cur_state = B
  releases mutex
  
CPU1: qmi_tmd_send_state_request(tmd, state A)
  acquires qmi_tmd_cli->mutex
  sends stale state A to hardware
  reverts tmd->cur_state = A

Will this overwrite the new state on the hardware and cause the thermal
framework's view of the mitigation state to become desynchronized?

> +
> +	tmd->cdev = thermal_of_cooling_device_register(dev->of_node, index,
> +						       label, tmd, &qmi_tmd_cooling_ops);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260731-b4-qmi-tmd-v7-0-3befd44403d5@oss.qualcomm.com?part=2