linux-kernel - Re: [PATCH v2 2/2] ACPI / Sleep: Check low power idle constraints for debug 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] [day] [month] [year] [list]

Date:   Sat, 12 Aug 2017 16:07:17 +0200
From:   "Rafael J. Wysocki" <rjw@...ysocki.net>
To:     Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>
Cc:     Mario.Limonciello@...l.com, lenb@...nel.org,
        linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-acpi@...r.kernel.org, lukas@...ner.de
Subject: Re: [PATCH v2 2/2] ACPI / Sleep: Check low power idle constraints for debug only

On Friday, August 11, 2017 6:18:50 PM CEST Srinivas Pandruvada wrote:
> On Fri, 2017-08-11 at 14:43 +0000, Mario.Limonciello@...l.com wrote:
> > > 
> > > -----Original Message-----
> > > From: Srinivas Pandruvada [mailto:srinivas.pandruvada@...ux.intel.c
> > > om]
> > > Sent: Thursday, August 10, 2017 5:54 PM
> > > To: Limonciello, Mario <Mario_Limonciello@...l.com>; rjw@...ysocki.
> > > net;
> > > lenb@...nel.org
> > > Cc: linux-pm@...r.kernel.org; linux-kernel@...r.kernel.org; linux-
> > > acpi@...r.kernel.org; lukas@...ner.de
> > > Subject: Re: [PATCH v2 2/2] ACPI / Sleep: Check low power idle
> > > constraints for
> > > debug only
> > > 
> > > On Thu, 2017-08-10 at 22:07 +0000, Mario.Limonciello@...l.com
> > > wrote:
> > > > 
> > > > 
> > > > > 
> > > > > 
> > > > > 
> > > [...]
> > > 
> > > > 
> > > > > 
> > > > > +
> > > > > +		ret = acpi_device_get_power(adev, &state);
> > > > > +		if (!ret)
> > > > > +			pr_debug("LPI: %s required min power
> > > > > state
> > > > > %d, current
> > > > > power state %d, real power state %d\n",
> > > > > +				 lpi_constraints_table[i].name
> > > > > ,
> > > > > +				 lpi_constraints_table[i].min_
> > > > > dsta
> > > > > te,
> > > > > +				 adev->power.state, state);
> > > > Isn't this superfluous to be showing the state returned from
> > > > acpi_device_get_power and
> > > > also probing directly at the state? You can't just rely on the
> > > > information you got
> > > > back from apci_device_get_power?
> > > They can be different as one is real power state and the other is
> > > what
> > > was set.
> > > For example on Dell 9365 it shows
> > > 
> > > [ 1924.393653] LPI: \_SB.PCI0.XHC required min power state 3,
> > > current
> > > power state 3, real power state 255
> > > 
> > Isn't 255 ACPI_STATE_UNKNOWN?  That makes it seem like it
> > is a logic problem in acpi_device_get_power (or somewhere down the
> > chain)
> > doesn't it?
> There is no _PSC for XHC device. So it will return unknown. This is an
> optional object, so I think that dumping the status is fine, but
> matching with output of acpi_device_get_power() as it relies on _PSC is
> not correct for the constraint.

Right.  Moreover, acpi_device_get_power() is basically mostly intended for
initialization, so the constraint should be matched against power.state,
as that's the current ACPI power state of the device as far as the kernel
can say.

Thanks,
Rafael