linux-kernel - Re: [PATCH v20 1/2] scsi: ufs: Enable power management for wlun

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <a71d6ac3-0dc6-4fa8-3643-6d3473d08797@codeaurora.org>
Date:   Fri, 23 Apr 2021 16:44:26 -0700
From:   "Asutosh Das (asd)" <asutoshd@...eaurora.org>
To:     Adrian Hunter <adrian.hunter@...el.com>, cang@...eaurora.org,
        martin.petersen@...cle.com, linux-scsi@...r.kernel.org
Cc:     linux-arm-msm@...r.kernel.org,
        Alim Akhtar <alim.akhtar@...sung.com>,
        Avri Altman <avri.altman@....com>,
        "James E.J. Bottomley" <jejb@...ux.ibm.com>,
        Krzysztof Kozlowski <krzk@...nel.org>,
        Stanley Chu <stanley.chu@...iatek.com>,
        Andy Gross <agross@...nel.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Ingo Molnar <mingo@...hat.com>,
        Matthias Brugger <matthias.bgg@...il.com>,
        Lee Jones <lee.jones@...aro.org>,
        Bean Huo <beanhuo@...ron.com>,
        Kiwoong Kim <kwmad.kim@...sung.com>,
        Colin Ian King <colin.king@...onical.com>,
        Wei Yongjun <weiyongjun1@...wei.com>,
        Yue Hu <huyue2@...ong.com>,
        Bart van Assche <bvanassche@....org>,
        "Gustavo A. R. Silva" <gustavoars@...nel.org>,
        Dinghao Liu <dinghao.liu@....edu.cn>,
        Jaegeuk Kim <jaegeuk@...nel.org>,
        Satya Tangirala <satyat@...gle.com>,
        open list <linux-kernel@...r.kernel.org>,
        "moderated list:ARM/SAMSUNG S3C, S5P AND EXYNOS ARM ARCHITECTURES" 
        <linux-arm-kernel@...ts.infradead.org>,
        "open list:ARM/SAMSUNG S3C, S5P AND EXYNOS ARM ARCHITECTURES" 
        <linux-samsung-soc@...r.kernel.org>,
        "moderated list:UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER..." 
        <linux-mediatek@...ts.infradead.org>
Subject: Re: [PATCH v20 1/2] scsi: ufs: Enable power management for wlun

On 4/23/2021 1:01 AM, Adrian Hunter wrote:
>>
>>
> 

Hi Adrian,
Thanks for the help.
I made the changes and tried to reproduce it.
My setup becomes non-responsive and resets.
I don't think it's related to this issue though.

I hadn't set rpm_lvl and spm_lvl to 0 while testing the other day.
Setting those to 0, it proceeds further and now it becomes unresponsive. 
So I still don't clearly see the issue that you're seeing. However, I'd 
make the suggested changes and push it in next version.

> I think we also need to runtime resume RPMB WLUN before system suspend.
> e.g.
> 
> +static int ufshcd_rpmb_rpm_get_sync(struct ufs_hba *hba)
> +{
> +	return pm_runtime_get_sync(&hba->sdev_rpmb->sdev_gendev);
> +}
> +
> +static int ufshcd_rpmb_rpm_put(struct ufs_hba *hba)
> +{
> +	return pm_runtime_put(&hba->sdev_rpmb->sdev_gendev);
> +}
> +
>   void ufshcd_resume_complete(struct device *dev)
>   {
>   	struct ufs_hba *hba = dev_get_drvdata(dev);
>   
> +	if (hba->rpmb_complete_put) {
> +		hba->rpmb_complete_put = false;
> +		ufshcd_rpmb_rpm_put(hba);
> +	}
>   	if (hba->complete_put) {
>   		hba->complete_put = false;
>   		ufshcd_rpm_put(hba);
> @@ -9611,6 +9625,11 @@ int ufshcd_suspend_prepare(struct device *dev)
>   		return ret;
>   	}
>   	hba->complete_put = true;
> +
> +	if (hba->sdev_rpmb) {
> +		ufshcd_rpmb_rpm_get_sync(hba);
> +		hba->rpmb_complete_put = true;
> +	}
>   	return 0;
>   }
> 
> That also avoids another issue: if RPMB WLUN is runtime suspended at system resume, we have to skip clearing UAC, but SCSI PM will force the runtime status to RPM_ACTIVE after system resume, so the UAC never gets cleared in that case.
> 
> Furthermore, it seems better not to report errors from RPMB resume and instead let the error handler sort it out.
> So, with the above change, we can simplify a bit:
> 
> -static int ufshcd_rpmb_runtime_resume(struct device *dev)
> -{
> -	struct ufs_hba *hba = wlun_dev_to_hba(dev);
> -
> -	if (hba->sdev_rpmb)
> -		return ufshcd_clear_rpmb_uac(hba);
> -	return 0;
> -}
> -
>   static int ufshcd_rpmb_resume(struct device *dev)
>   {
>   	struct ufs_hba *hba = wlun_dev_to_hba(dev);
>   
> -	if (hba->sdev_rpmb && !pm_runtime_suspended(dev))
> -		return ufshcd_clear_rpmb_uac(hba);
> +	if (hba->sdev_rpmb)
> +		ufshcd_clear_rpmb_uac(hba);
>   	return 0;
>   }
>   
>   static const struct dev_pm_ops ufs_rpmb_pm_ops = {
> -	SET_RUNTIME_PM_OPS(NULL, ufshcd_rpmb_runtime_resume, NULL)
> +	SET_RUNTIME_PM_OPS(NULL, ufshcd_rpmb_resume, NULL)
>   	SET_SYSTEM_SLEEP_PM_OPS(NULL, ufshcd_rpmb_resume)
>   };
> 
> 


-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
Linux Foundation Collaborative Project