linux-kernel - Re: [PATCH v1 1/2] scsi: ufs: set device as default active power mode during initialization only

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]

Date:   Tue, 31 Dec 2019 15:44:50 +0800
From:   Stanley Chu <stanley.chu@...iatek.com>
To:     Can Guo <cang@...eaurora.org>
CC:     <linux-scsi-owner@...r.kernel.org>, <linux-scsi@...r.kernel.org>,
        <martin.petersen@...cle.com>, <subhashj@...eaurora.org>,
        <jejb@...ux.ibm.com>, <chun-hung.wu@...iatek.com>,
        <kuohong.wang@...iatek.com>, <linux-kernel@...r.kernel.org>,
        <stable@...r.kernel.org>, <avri.altman@....com>,
        <linux-mediatek@...ts.infradead.org>, <peter.wang@...iatek.com>,
        <alim.akhtar@...sung.com>, <andy.teng@...iatek.com>,
        <matthias.bgg@...il.com>, <beanhuo@...ron.com>,
        <pedrom.sousa@...opsys.com>, <bvanassche@....org>,
        <linux-arm-kernel@...ts.infradead.org>, <asutoshd@...eaurora.org>
Subject: Re: [PATCH v1 1/2] scsi: ufs: set device as default active power
 mode during initialization only

Hi Can,

On Tue, 2019-12-31 at 12:22 +0800, Stanley Chu wrote:
> Hi Can,
> 
> 
> > Hi Stanley,
> > 
> > I see skipping ufshcd_set_ufs_dev_active() in ufshcd_probe_hba()
> > if it is called from ufshcd_resume() path is the purpose here.
> > 
> > If so, then ufshcd_set_dev_pwr_mode() would be called, meaning
> > SSU command will be sent. Why is this SSU command needed to be
> > sent after a full host reset and restore? Is ufshcd_probe_hba()
> > not enough to make UFS device fully functional?
> 
> After resume (for both runtime resume and system resume), device power
> mode shall be back to "Active" to service incoming requests.
> 
> I see two cases need device power mode transition during resume flow
> 1. Device Power Mode = Sleep
> 2. Device Power Mode = PowerDown
> 
> For 1, ufshcd_probe_hba() is not invoked during resume flow,
> hba->curr_dev_pwr_mode = SLEEP, thus ufshcd_resume() can invoke
> ufshcd_set_dev_pwr_mode() to change device power mode.
> 
> For 2, ufshcd_probe_hba() is invoked during resume flow, before this
> fix, hba->curr_dev_pwr_mode will be set to ACTIVE (note that only this
> flag is set as ACTIVE, but device may be still in PowerDown state if
> device power is not fully shutdown or device HW reset is not executed
> before resume), in the end, ufshcd_resume() will not invoke
> ufshcd_set_dev_pwr_mode() to send SSU command to make device change back
> to Active power mode.
> 
> But if device is fully reset before resume (not by current mainstream
> driver), device can be already in "Active" power mode after power on
> again during resume flow. In this case, it is OK to set
> hba->curr_dev_pwr_mode as ACTIVE in ufshcd_probe_hba() and SSU command
> is not necessary.
> 
> Thanks,
> Stanley

I think currently the assumption in ufshcd_probe_hba() that "device
shall be already in Active power mode" makes sense because many device
commands will be sent to device in ufshcd_probe_hba() but device is not
promised to handle those commands in PowerDown power mode according to
UFS spec. 

So, maybe always ensuring device being Active power mode before leaving
ufshcd_probe_hba() is more reasonable. If so, I will drop this patch
first.

Thanks so much for your reviews.

Happy new year!

Thanks,
Stanley