linux-kernel - Re: [PATCH 1/2] ata: libahci: Correct setting of DEVSLP register

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

Date:   Mon, 16 Jul 2018 20:43:27 -0700
From:   Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>
To:     AceLan Kao <acelan@...il.com>
Cc:     tj@...nel.org, hdegoede@...hat.com, rjw@...ysocki.net,
        alan.cox@...el.com, linux-ide@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] ata: libahci: Correct setting of DEVSLP register

On Tue, 2018-07-17 at 11:26 +0800, AceLan Kao wrote:
> Tested-by: AceLan Kao <acelan.kao@...onical.com>

Thanks Kao for the test.

-Srinivas

> 
> The patches help the power consumption a little bit on my test
> system,
> and no obvious issue I can observe. 
> 
> 2018-07-03 3:01 GMT+08:00 Srinivas Pandruvada <srinivas.pandruvada@li
> nux.intel.com>:
> > We have seen that on some platforms, SATA device never show any
> > DEVSLP
> > residency. This prevent power gating of SATA IP, which prevent
> > system
> > to transition to low power mode in systems with SLP_S0 aka modern
> > standby systems. The PHY logic is off only in DEVSLP not in
> > slumber.
> > Reference:
> > https://www.intel.com/content/dam/www/public/us/en/documents/datash
> > eets
> > /332995-skylake-i-o-platform-datasheet-volume-1.pdf
> > Section 28.7.6.1
> > 
> > Here driver is trying to do read-modify-write the devslp register.
> > But
> > not resetting the bits for which this driver will modify values
> > (DITO,
> > MDAT and DETO). So simply reset those bits before updating to new
> > values.
> > 
> > Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@...ux.intel
> > .com>
> > Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> > ---
> >  drivers/ata/libahci.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
> > index 965842a08743..f6795d261869 100644
> > --- a/drivers/ata/libahci.c
> > +++ b/drivers/ata/libahci.c
> > @@ -2159,6 +2159,8 @@ static void ahci_set_aggressive_devslp(struct
> > ata_port *ap, bool sleep)
> >                 deto = 20;
> >         }
> > 
> > +       /* Make dito, mdat, deto bits to 0s */
> > +       devslp &= ~GENMASK_ULL(24, 2);
> >         devslp |= ((dito << PORT_DEVSLP_DITO_OFFSET) |
> >                    (mdat << PORT_DEVSLP_MDAT_OFFSET) |
> >                    (deto << PORT_DEVSLP_DETO_OFFSET) |
> > -- 
> > 2.17.1
> > 
> 
>