linux-kernel - Re: [PATCH v2 3/5] scsi: ufs: qcom: Add a readl() to make sure ref

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <YmQYtHfSYJxKwivg@ripper>
Date:   Sat, 23 Apr 2022 08:18:12 -0700
From:   Bjorn Andersson <bjorn.andersson@...aro.org>
To:     Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
Cc:     martin.petersen@...cle.com, jejb@...ux.ibm.com,
        avri.altman@....com, alim.akhtar@...sung.com,
        linux-arm-msm@...r.kernel.org, quic_asutoshd@...cinc.com,
        quic_cang@...cinc.com, linux-scsi@...r.kernel.org,
        linux-kernel@...r.kernel.org, bvanassche@....org,
        ahalaney@...hat.com, stable@...r.kernel.org
Subject: Re: [PATCH v2 3/5] scsi: ufs: qcom: Add a readl() to make sure
 ref_clk gets enabled

On Sat 23 Apr 07:02 PDT 2022, Manivannan Sadhasivam wrote:

> In ufs_qcom_dev_ref_clk_ctrl(), it was noted that the ref_clk needs to be
> stable for at least 1us. Even though there is wmb() to make sure the write
> gets "completed", there is no guarantee that the write actually reached
> the UFS device. There is a good chance that the write could be stored in
> a Write Buffer (WB). In that case, even though the CPU waits for 1us, the
> ref_clk might not be stable for that period.
> 
> So lets do a readl() to make sure that the previous write has reached the
> UFS device before udelay().
> 
> Also, the wmb() after writel_relaxed is not really needed. Both writel and
> readl are ordered on all architectures and the CPU won't speculate
> instructions after readl() due to the in-built control dependency with
> read value on weakly ordered architectures. So it can be safely removed.
> 
> Cc: stable@...r.kernel.org
> Fixes: f06fcc7155dc ("scsi: ufs-qcom: add QUniPro hardware support and power optimizations")
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>

Reviewed-by: Bjorn Andersson <bjorn.andersson@...aro.org>

> ---
>  drivers/scsi/ufs/ufs-qcom.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
> index 6ee33cc0ad09..f47a16b7cff5 100644
> --- a/drivers/scsi/ufs/ufs-qcom.c
> +++ b/drivers/scsi/ufs/ufs-qcom.c
> @@ -687,8 +687,11 @@ static void ufs_qcom_dev_ref_clk_ctrl(struct ufs_qcom_host *host, bool enable)
>  
>  		writel_relaxed(temp, host->dev_ref_clk_ctrl_mmio);
>  
> -		/* ensure that ref_clk is enabled/disabled before we return */
> -		wmb();
> +		/*
> +		 * Make sure the write to ref_clk reaches the destination and
> +		 * not stored in a Write Buffer (WB).
> +		 */
> +		readl(host->dev_ref_clk_ctrl_mmio);
>  
>  		/*
>  		 * If we call hibern8 exit after this, we need to make sure that
> -- 
> 2.25.1
>