Re: Ping: [PATCH 0/5] x86: more power-efficient CPU parking

From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Jan Beulich <JBeulich@suse.com>
Cc: xen-devel <xen-devel@lists.xenproject.org>,
	Wei Liu <wei.liu2@citrix.com>,
	Roger Pau Monne <roger.pau@citrix.com>
Subject: Re: Ping: [PATCH 0/5] x86: more power-efficient CPU parking
Date: Wed, 3 Apr 2019 12:14:47 +0100	[thread overview]
Message-ID: <6dab54cd-0246-2dff-42dd-93477c7c19e3@citrix.com> (raw)
In-Reply-To: <5CA4870A0200007800224351@prv1-mh.provo.novell.com>

On 03/04/2019 11:12, Jan Beulich wrote:
>>>> On 01.08.18 at 16:22,  wrote:
>> When putting CPUs to sleep permanently, we should try to put them into
>> the most power conserving state possible. For now it is unclear whether,
>> especially in a deep C-state, the P-state also matters, so this series only
>> arranges for the C-state side of things (plus some cleanup).
>>
>> 1: x86/cpuidle: replace a pointless NULL check
>> 2: x86/idle: re-arrange dead-idle handling
>> 3: x86/cpuidle: push parked CPUs into deeper sleep states when possible
>> 4: x86/cpuidle: clean up Cx dumping
>> 5: x86: place non-parked CPUs into wait-for-SIPI state after offlining
> So patch 5 is understandably controversial, and I'm explicitly
> excluding it from the ping.

Considering that it causes EFI firmware to explode in several
interesting ways, I'm afraid it is a complete nonstarter.

> Patch 1 has gone in long ago and
> patch 4 has been acked. While there was some feedback on
> 2 (albeit stalled then after my reply), I don't think I've had any
> substantial feedback on 3 though, yet _they_ are supposed to
> provide the main improvement here. Patch 5 really was meant
> to be more optional than I had expressed, hence its placement
> even after the pure cleanup patch 4.

Some of the patches have cycled out of my inbox, so I'm reviewing from
the mail archive.

2) Now that default_dead_idle() isn't a terminal function, you must use
spec_ctrl_enter_idle() on the way back out for safety.

I'm still going to insist on something about #MC, even if it is just a
note in the commit message saying "this doesn't yet make #MC any safer
for parked CPUs".

The real problem is that your comment about NMI now makes the code read
as if everything is safe, and while this patch is an improvement (and
therefore ok on its own merits), it doesn't actually make the result #MC
safe.

3) Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel