[PATCH] ufs: sysfs: fix current_power_mode read without SSU

[PATCH] ufs: sysfs: fix current_power_mode read without SSU

[Yiwei Lin]

current_power_mode was generated by UFS_ATTRIBUTE(), which calls
ufshcd_rpm_get_sync() before issuing the query.

That resumes the UFS device WLUN, and ufshcd_wl_runtime_resume() sends
START STOP UNIT to bring the device back to Active. As a result
bCurrentPowerMode always read back as 0x11 (Active) no matter what
state the device was actually in. Sleep could never be observed through
sysfs.

Per the UFS spec, bCurrentPowerMode is the one attribute the device
must answer in any power mode, so there is no need to wake it.

Tested on hardware: with the device runtime-suspended, reading
current_power_mode used to return 0x11; after this change it returns
0x22 (Sleep), matching the device's real state.

Assisted-by: LLM
Signed-off-by: Yiwei Lin <s921975628@gmail.com>
---
 drivers/ufs/core/ufs-sysfs.c | 59 +++++++++++++++++++++++++++++++++++-
 1 file changed, 58 insertions(+), 1 deletion(-)

diff --git a/drivers/ufs/core/ufs-sysfs.c b/drivers/ufs/core/ufs-sysfs.c
index 63e670d1a9d9e..36cffd535ab7d 100644
--- a/drivers/ufs/core/ufs-sysfs.c
+++ b/drivers/ufs/core/ufs-sysfs.c
@@ -1786,7 +1786,64 @@ out:									\
 static DEVICE_ATTR_RO(_name)
 
 UFS_ATTRIBUTE(boot_lun_enabled, _BOOT_LU_EN);
-UFS_ATTRIBUTE(current_power_mode, _POWER_MODE);
+
+/*
+ * Per UFS spec, bCurrentPowerMode is the only attribute the device must
+ * respond to in any power mode.  Read it without waking the device to
+ * Active: restore HBA clocks via pm_runtime_get_sync(hba->dev) only
+ * (avoids triggering ufshcd_wl_runtime_resume and its START STOP UNIT),
+ * exit Hibern8 if needed so the link can carry the UPIU, then query.
+ */
+static ssize_t current_power_mode_show(struct device *dev,
+	struct device_attribute *attr, char *buf)
+{
+	struct ufs_hba *hba = dev_get_drvdata(dev);
+	u32 value;
+	int ret;
+
+	down(&hba->host_sem);
+	if (!ufshcd_is_user_access_allowed(hba)) {
+		up(&hba->host_sem);
+		return -EBUSY;
+	}
+
+	ret = pm_runtime_get_sync(hba->dev);
+	if (ret < 0) {
+		pr_warn("%s: pm_runtime_get_sync failed %d\n", __func__, ret);
+		pm_runtime_put_noidle(hba->dev);
+		up(&hba->host_sem);
+		return ret;
+	}
+	ret = 0;
+
+	ufshcd_hold(hba);
+
+	if (ufshcd_is_link_hibern8(hba)) {
+		ret = ufshcd_uic_hibern8_exit(hba);
+		if (!ret)
+			ufshcd_set_link_active(hba);
+		else
+			dev_err(hba->dev, "%s: hibern8 exit failed %d\n",
+				__func__, ret);
+	}
+
+	if (!ret)
+		ret = ufshcd_query_attr(hba, UPIU_QUERY_OPCODE_READ_ATTR,
+			QUERY_ATTR_IDN_POWER_MODE, 0, 0, &value);
+
+	ufshcd_release(hba);
+	pm_runtime_put(hba->dev);
+
+	if (ret) {
+		up(&hba->host_sem);
+		return -EINVAL;
+	}
+	ret = sysfs_emit(buf, "0x%08X\n", value);
+	up(&hba->host_sem);
+	return ret;
+}
+static DEVICE_ATTR_RO(current_power_mode);
+
 UFS_ATTRIBUTE(active_icc_level, _ACTIVE_ICC_LVL);
 UFS_ATTRIBUTE(ooo_data_enabled, _OOO_DATA_EN);
 UFS_ATTRIBUTE(bkops_status, _BKOPS_STATUS);
-- 
2.34.1

Re: [PATCH] ufs: sysfs: fix current_power_mode read without SSU

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Exiting Hibern8 manually without restoring it leaves the UFS link in the ACTIVE state, leading to hardware state corruption and power leaks when the host controller auto-suspends.
- [High] A Time-Of-Check to Time-Of-Use (TOCTOU) race exists between the sysfs read and WLUN runtime resume, potentially causing duplicate Hibern8 exits and forcing severe link recovery.
- [High] The sysfs read blindly sends a UPIU query over an OFF link if the UFS device was suspended to a DeepSleep/PowerDown state.
--

commit f4fe909880e2766ce73fa9889b03cdb85f6b474d
Author: Yiwei Lin <s921975628@gmail.com>

ufs: sysfs: fix current_power_mode read without SSU

This patch changes how current_power_mode is read from sysfs to avoid waking
the device to Active state. It directly fetches the PM sync reference, exits
Hibern8 if necessary, queries the attribute, and drops the PM reference.

> diff --git a/drivers/ufs/core/ufs-sysfs.c b/drivers/ufs/core/ufs-sysfs.c
> index cf49ad0258e14..86705432aa42d 100644
> --- a/drivers/ufs/core/ufs-sysfs.c
> +++ b/drivers/ufs/core/ufs-sysfs.c
[ ... ]
> +	ufshcd_hold(hba);
> +
> +	if (ufshcd_is_link_hibern8(hba)) {

[Severity: High]
Could this introduce a Time-Of-Check to Time-Of-Use (TOCTOU) race with a
concurrent WLUN runtime resume?

If a sysfs read of current_power_mode executes concurrently with an I/O
request that triggers ufshcd_wl_runtime_resume(), both threads might check
ufshcd_is_link_hibern8() at the same time without holding uic_cmd_mutex.

If both see the link in Hibern8, they will serialize on uic_cmd_mutex inside
ufshcd_uic_pwr_ctrl(). The first thread will set the link to ACTIVE, and the
second thread will send a duplicate UIC_CMD_DME_HIBER_EXIT command to the
already ACTIVE link, which can cause a hardware command failure and trigger
a severe link recovery.

> +		ret = ufshcd_uic_hibern8_exit(hba);
> +		if (!ret)
> +			ufshcd_set_link_active(hba);
> +		else
> +			dev_err(hba->dev, "%s: hibern8 exit failed %d\n",
> +				__func__, ret);
> +	}
> +
> +	if (!ret)
> +		ret = ufshcd_query_attr(hba, UPIU_QUERY_OPCODE_READ_ATTR,
> +			QUERY_ATTR_IDN_POWER_MODE, 0, 0, &value);

[Severity: High]
Does this blindly send a UPIU query over an OFF link if the UFS device was
suspended to a DeepSleep/PowerDown state?

When the UFS WLUN is suspended to a level that puts the link into
UIC_LINK_OFF_STATE, ufshcd_is_link_hibern8() evaluates to false. This
function will skip link initialization entirely and leave it in the OFF
state, proceeding to call ufshcd_query_attr().

Sending a query to unpowered/unlinked hardware typically fails with a
hardware timeout or fatal controller error.

> +
> +	ufshcd_release(hba);
> +	pm_runtime_put(hba->dev);

[Severity: High]
Does dropping the host controller's runtime PM reference here leave the
hardware in an invalid state if we explicitly exited Hibern8?

Because the link was manually transitioned to ACTIVE in this function
without an explicit mechanism to restore it to Hibern8, the UFS link
remains in the ACTIVE state.

When pm_runtime_put() drops the usage count, ufshcd_runtime_suspend()
can execute and unconditionally turn off the UFS clocks. Turning off
clocks while the link is physically ACTIVE violates the UFS specification
and can lead to hardware lockup or fatal controller errors upon the
next access.

> +
> +	if (ret) {
> +		up(&hba->host_sem);
> +		return -EINVAL;
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914170032.5551-1-s921975628@gmail.com?part=1

Re: [PATCH] ufs: sysfs: fix current_power_mode read without SSU

[Yiwei Lin]

On 2026/09/15 01:00, Yiwei Lin wrote:

> current_power_mode was generated by UFS_ATTRIBUTE(), which calls
> ufshcd_rpm_get_sync() before issuing the query.
>
> That resumes the UFS device WLUN, and ufshcd_wl_runtime_resume() sends
> START STOP UNIT to bring the device back to Active. As a result
> bCurrentPowerMode always read back as 0x11 (Active) no matter what
> state the device was actually in. Sleep could never be observed through
> sysfs.
>
> Per the UFS spec, bCurrentPowerMode is the one attribute the device
> must answer in any power mode, so there is no need to wake it.
>
> Tested on hardware: with the device runtime-suspended, reading
> current_power_mode used to return 0x11; after this change it returns
> 0x22 (Sleep), matching the device's real state.
>
> Assisted-by: LLM
> Signed-off-by: Yiwei Lin <s921975628@gmail.com>
> ---
>  drivers/ufs/core/ufs-sysfs.c | 59 +++++++++++++++++++++++++++++++++++-
>  1 file changed, 58 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/ufs/core/ufs-sysfs.c b/drivers/ufs/core/ufs-sysfs.c
> index 63e670d1a9d9e..36cffd535ab7d 100644
> --- a/drivers/ufs/core/ufs-sysfs.c
> +++ b/drivers/ufs/core/ufs-sysfs.c
> @@ -1786,7 +1786,64 @@ out:									\
>  static DEVICE_ATTR_RO(_name)
>  
>  UFS_ATTRIBUTE(boot_lun_enabled, _BOOT_LU_EN);
> -UFS_ATTRIBUTE(current_power_mode, _POWER_MODE);
> +
> +/*
> + * Per UFS spec, bCurrentPowerMode is the only attribute the device must
> + * respond to in any power mode.  Read it without waking the device to
> + * Active: restore HBA clocks via pm_runtime_get_sync(hba->dev) only
> + * (avoids triggering ufshcd_wl_runtime_resume and its START STOP UNIT),
> + * exit Hibern8 if needed so the link can carry the UPIU, then query.
> + */
> +static ssize_t current_power_mode_show(struct device *dev,
> +	struct device_attribute *attr, char *buf)
> +{
> +	struct ufs_hba *hba = dev_get_drvdata(dev);
> +	u32 value;
> +	int ret;
> +
> +	down(&hba->host_sem);
> +	if (!ufshcd_is_user_access_allowed(hba)) {
> +		up(&hba->host_sem);
> +		return -EBUSY;
> +	}
> +
> +	ret = pm_runtime_get_sync(hba->dev);
> +	if (ret < 0) {
> +		pr_warn("%s: pm_runtime_get_sync failed %d\n", __func__, ret);
> +		pm_runtime_put_noidle(hba->dev);
> +		up(&hba->host_sem);
> +		return ret;
> +	}
> +	ret = 0;
> +
> +	ufshcd_hold(hba);
> +
> +	if (ufshcd_is_link_hibern8(hba)) {
> +		ret = ufshcd_uic_hibern8_exit(hba);
> +		if (!ret)
> +			ufshcd_set_link_active(hba);
> +		else
> +			dev_err(hba->dev, "%s: hibern8 exit failed %d\n",
> +				__func__, ret);
> +	}
> +
> +	if (!ret)
> +		ret = ufshcd_query_attr(hba, UPIU_QUERY_OPCODE_READ_ATTR,
> +			QUERY_ATTR_IDN_POWER_MODE, 0, 0, &value);
> +
> +	ufshcd_release(hba);
> +	pm_runtime_put(hba->dev);
> +
> +	if (ret) {
> +		up(&hba->host_sem);
> +		return -EINVAL;
> +	}
> +	ret = sysfs_emit(buf, "0x%08X\n", value);
> +	up(&hba->host_sem);
> +	return ret;
> +}
> +static DEVICE_ATTR_RO(current_power_mode);
> +
>  UFS_ATTRIBUTE(active_icc_level, _ACTIVE_ICC_LVL);
>  UFS_ATTRIBUTE(ooo_data_enabled, _OOO_DATA_EN);
>  UFS_ATTRIBUTE(bkops_status, _BKOPS_STATUS);

According to [Sashiki's review](https://sashiko.dev/#/patchset/20260914170032.5551-1-s9      21975628@gmail.com?part=1), I think I overlooked the condition that the link is not forcefully active, which may lead to a race condition or unexpected behavior. 

I'll provide v2 with careful review and validation.

> -- 
> 2.34.1
>