linux-kernel - RE: [PATCH v2 2/3] scsi: ufs: Keep device active mode only fWriteBoosterBufferFlushDuringHibernate == 1

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: <DM6PR04MB657584B4590496FA6BF11725FCCD0@DM6PR04MB6575.namprd04.prod.outlook.com>
Date:   Tue, 8 Dec 2020 09:05:20 +0000
From:   Avri Altman <Avri.Altman@....com>
To:     Bean Huo <huobean@...il.com>,
        "alim.akhtar@...sung.com" <alim.akhtar@...sung.com>,
        "asutoshd@...eaurora.org" <asutoshd@...eaurora.org>,
        "jejb@...ux.ibm.com" <jejb@...ux.ibm.com>,
        "martin.petersen@...cle.com" <martin.petersen@...cle.com>,
        "stanley.chu@...iatek.com" <stanley.chu@...iatek.com>,
        "beanhuo@...ron.com" <beanhuo@...ron.com>,
        "bvanassche@....org" <bvanassche@....org>,
        "tomas.winkler@...el.com" <tomas.winkler@...el.com>,
        "cang@...eaurora.org" <cang@...eaurora.org>
CC:     "linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v2 2/3] scsi: ufs: Keep device active mode only
 fWriteBoosterBufferFlushDuringHibernate == 1

> On Mon, 2020-12-07 at 08:02 +0000, Avri Altman wrote:
> > > According to the JEDEC UFS 3.1 Spec, If
> > > fWriteBoosterBufferFlushDuringHibernate
> > > is set to one, the device flushes the WriteBooster Buffer data
> > > automatically
> > > whenever the link enters the hibernate (HIBERN8) state. While the
> > > flushing
> > > operation is in progress, the device should be kept in Active power
> > > mode.
> > > Currently, we set this flag during the UFSHCD probe stage, but we
> > > didn't deal
> > > with its programming failure. Even this failure is less likely to
> > > occur, but
> > > still it is possible.
> > > This patch is to add checkup of
> > > fWriteBoosterBufferFlushDuringHibernate
> > > setting,
> > > keep the device as "active power mode" only when this flag be
> > > successfully
> > > set
> > > to 1.
> > >
> > > Fixes: 51dd905bd2f6 ("scsi: ufs: Fix WriteBooster flush during
> > > runtime
> > > suspend")
> > > Signed-off-by: Bean Huo <beanhuo@...ron.com>
> >
> > You've added the fixes tag,
> 
> yes,it is a bug.
> 
> >  but my previous comment is still unanswered:
> > you are adding protection to a single device management command.
> > Why this command in particular?
> > What makes it so special that it needs this extra care?
> >
> 
> see the Spec:
> "
> If fWriteBoosterBufferFlushDuringHibernate is set to one, the device
> flushes the WriteBooster Buffer data automatically whenever the link
> enters the hibernate (HIBERN8) state.
> 
> The device shall stop the flushing operation if
> fWriteBoosterBufferFlushDuringHibernate are set to zero.
> ....
> 
> "
> If fWriteBoosterBufferFlushDuringHibernate ==0, device will not flush
> WB, even if you keep device as "active mode" and LINK in hibernate
> state.
OK, so what you are actually saying, is that since we are only toggling this flag once per
link startup / recovery, in case of a failure, however rare - the host may be still keep vcc on
for nothing, as the device will do nothing in that extra wake time.  Right?

But every time ufshcd_wb_need_flush() is called we are reading some other flags/attributes?
What about them? Why not protecting them as well?

Sorry - the whole idea doesn't make any sense to me.

Thanks,
Avri
> 
> Bean
> Thanks,
>