linux-kernel - Re: [PATCH v1 0/2] x86/smp: Fix power regression introduced by commit 96040f7273e2

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: <20250528133807.GC39944@noisy.programming.kicks-ass.net>
Date: Wed, 28 May 2025 15:38:07 +0200
From: Peter Zijlstra <peterz@...radead.org>
To: "Rafael J. Wysocki" <rafael@...nel.org>
Cc: "Rafael J. Wysocki" <rjw@...ysocki.net>,
	x86 Maintainers <x86@...nel.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Linux PM <linux-pm@...r.kernel.org>, Len Brown <lenb@...nel.org>,
	Thomas Gleixner <tglx@...utronix.de>, Borislav Petkov <bp@...e.de>,
	Dave Hansen <dave.hansen@...ux.intel.com>,
	Artem Bityutskiy <artem.bityutskiy@...ux.intel.com>,
	"Gautham R. Shenoy" <gautham.shenoy@....com>,
	Ingo Molnar <mingo@...hat.com>,
	Todd Brandt <todd.e.brandt@...ux.intel.com>
Subject: Re: [PATCH v1 0/2] x86/smp: Fix power regression introduced by
 commit 96040f7273e2

On Wed, May 28, 2025 at 03:20:16PM +0200, Rafael J. Wysocki wrote:
> On Wed, May 28, 2025 at 3:18 PM Peter Zijlstra <peterz@...radead.org> wrote:
> >
> > On Wed, May 28, 2025 at 02:53:13PM +0200, Rafael J. Wysocki wrote:
> > > Hi Everyone,
> > >
> > > Commit 96040f7273e2 ("x86/smp: Eliminate mwait_play_dead_cpuid_hint()")
> > > that shipped in 6.15 introduced a nasty power regression on systems that
> > > start with "nosmt" in the kernel command line which prevents it from entering
> > > deep package idle states (for instance, PC10) later on.  Idle power, including
> > > power in suspend-to-idle, goes up significantly on those systems as a result.
> > >
> > > Address this by reverting commit 96040f7273e2 (patch [1/2]) and using a
> > > different approach, which is to retain mwait_play_dead_cpuid_hint() and
> > > still prefer it to hlt_play_dead() in case it is needed when cpuidle is
> > > not available, but prefer cpuidle_play_dead() to it by default (patch [2/2]).
> >
> > I don't understand. The revert says the reason it regresses is that it
> > goes into play_dead before cpuidle is initialized. The fix is then to
> > call cpuidle first.
> >
> > But if cpuidle isn't initialized yet, how does that fix anything?
> 
> The revert fixes the bug.

This is not what I asked.

> The other patch does what the reverted commit was supposed to be
> doing, but differently.

No, it does not.

The whole point was that mwait_play_dead did not DTRT because hints are
stupid and it could not select the deepest C state in an unambiguous
fashion.

And now you're restoring that -- code you all argued was fundamentally
buggered.

Yes is 'fixes' things on old platforms, but it is equally broken on the
new platforms where you all argued it was broken on. So either way
around you're going to need to fix those, and this isn't it.

Now, SMT siblings are all AP, by definition. So can't we simply send
them INIT instead of doing CLI;HLT, that way they drop into
Wait-for-SIPI and the ucode can sort it out?