[PATCH v9 0/4] SRF: Fix offline CPU preventing pc6 entry

Patryk Wlazlyn posted 4 patches 11 months, 1 week ago
arch/x86/include/asm/smp.h    |  3 +++
arch/x86/kernel/acpi/cstate.c | 10 ++++++++
arch/x86/kernel/smpboot.c     | 46 ++++-------------------------------
drivers/acpi/processor_idle.c |  2 ++
drivers/idle/intel_idle.c     | 15 ++++++++++++
include/acpi/processor.h      |  5 ++++
6 files changed, 40 insertions(+), 41 deletions(-)
[PATCH v9 0/4] SRF: Fix offline CPU preventing pc6 entry
Posted by Patryk Wlazlyn 11 months, 1 week ago
Code for determining the mwait hint for the deepest C-state by
inspecting CPUID leaf 0x5 in mwait_play_dead() assumes that, if the
number of sub-states for a given major C-state is nonzero, those
sub-states are always represented by consecutive numbers starting from
0. This assumption is not based on the documented platform behavior and
in fact it is not met on recent Intel platforms.

Changes since v8:
  * Revert the deletion of cpuidle_state_table check for enter and
    s2idle handlers in intel_idle in 3/4:
-               if (!cpuidle_state_table[cstate].enter &&
-                   !cpuidle_state_table[cstate].enter_s2idle)
+               if (!cpuidle_state_table[cstate].enter)

  * Apply the changelog wording suggested by Rafael in v8 review.

Patryk Wlazlyn (4):
  x86/smp: Allow calling mwait_play_dead with an arbitrary hint
  ACPI: processor_idle: Add FFH state handling
  intel_idle: Provide the default enter_dead() handler
  x86/smp: Eliminate mwait_play_dead_cpuid_hint()

 arch/x86/include/asm/smp.h    |  3 +++
 arch/x86/kernel/acpi/cstate.c | 10 ++++++++
 arch/x86/kernel/smpboot.c     | 46 ++++-------------------------------
 drivers/acpi/processor_idle.c |  2 ++
 drivers/idle/intel_idle.c     | 15 ++++++++++++
 include/acpi/processor.h      |  5 ++++
 6 files changed, 40 insertions(+), 41 deletions(-)

-- 
2.47.1
Re: [PATCH v9 0/4] SRF: Fix offline CPU preventing pc6 entry
Posted by Dave Hansen 11 months, 1 week ago
On 1/10/25 03:59, Patryk Wlazlyn wrote:
> Patryk Wlazlyn (4):
>   x86/smp: Allow calling mwait_play_dead with an arbitrary hint
>   ACPI: processor_idle: Add FFH state handling
>   intel_idle: Provide the default enter_dead() handler
>   x86/smp: Eliminate mwait_play_dead_cpuid_hint()
> 
>  arch/x86/include/asm/smp.h    |  3 +++
>  arch/x86/kernel/acpi/cstate.c | 10 ++++++++
>  arch/x86/kernel/smpboot.c     | 46 ++++-------------------------------
>  drivers/acpi/processor_idle.c |  2 ++
>  drivers/idle/intel_idle.c     | 15 ++++++++++++
>  include/acpi/processor.h      |  5 ++++
>  6 files changed, 40 insertions(+), 41 deletions(-)

Is everybody happy with this now?

I noticed there are no Fixes: or Cc:stable@ tags on this. Should we be
treating this like a new feature or a bug fix?
Re: [PATCH v9 0/4] SRF: Fix offline CPU preventing pc6 entry
Posted by Rafael J. Wysocki 11 months, 1 week ago
On Fri, Jan 10, 2025 at 4:17 PM Dave Hansen <dave.hansen@intel.com> wrote:
>
> On 1/10/25 03:59, Patryk Wlazlyn wrote:
> > Patryk Wlazlyn (4):
> >   x86/smp: Allow calling mwait_play_dead with an arbitrary hint
> >   ACPI: processor_idle: Add FFH state handling
> >   intel_idle: Provide the default enter_dead() handler
> >   x86/smp: Eliminate mwait_play_dead_cpuid_hint()
> >
> >  arch/x86/include/asm/smp.h    |  3 +++
> >  arch/x86/kernel/acpi/cstate.c | 10 ++++++++
> >  arch/x86/kernel/smpboot.c     | 46 ++++-------------------------------
> >  drivers/acpi/processor_idle.c |  2 ++
> >  drivers/idle/intel_idle.c     | 15 ++++++++++++
> >  include/acpi/processor.h      |  5 ++++
> >  6 files changed, 40 insertions(+), 41 deletions(-)
>
> Is everybody happy with this now?

It is fine by me.

Please feel free to add

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

to all patches.

Thanks!
Re: [PATCH v9 0/4] SRF: Fix offline CPU preventing pc6 entry
Posted by Artem Bityutskiy 11 months, 1 week ago
On Fri, 2025-01-10 at 07:17 -0800, Dave Hansen wrote:
> On 1/10/25 03:59, Patryk Wlazlyn wrote:
> > Patryk Wlazlyn (4):
> >   x86/smp: Allow calling mwait_play_dead with an arbitrary hint
> >   ACPI: processor_idle: Add FFH state handling
> >   intel_idle: Provide the default enter_dead() handler
> >   x86/smp: Eliminate mwait_play_dead_cpuid_hint()
> > 
> >  arch/x86/include/asm/smp.h    |  3 +++
> >  arch/x86/kernel/acpi/cstate.c | 10 ++++++++
> >  arch/x86/kernel/smpboot.c     | 46 ++++-------------------------------
> >  drivers/acpi/processor_idle.c |  2 ++
> >  drivers/idle/intel_idle.c     | 15 ++++++++++++
> >  include/acpi/processor.h      |  5 ++++
> >  6 files changed, 40 insertions(+), 41 deletions(-)
> 
> Is everybody happy with this now?
> 
> I noticed there are no Fixes: or Cc:stable@ tags on this. Should we be
> treating this like a new feature or a bug fix?

It would be very helpful to have this in v6.12, because it is LTS. So I would
suggest

Cc: stable@vger.kernel.org # v6.12

Thanks,
Artem.
Re: [PATCH v9 0/4] SRF: Fix offline CPU preventing pc6 entry
Posted by Dave Hansen 11 months, 1 week ago
On 1/10/25 07:26, Artem Bityutskiy wrote:
>> I noticed there are no Fixes: or Cc:stable@ tags on this. Should we be
>> treating this like a new feature or a bug fix?
> It would be very helpful to have this in v6.12, because it is LTS. So I would
> suggest
> 
> Cc: stable@vger.kernel.org # v6.12

I was _kinda_ hoping to hear how this affects users.

Is this a big deal for real users out in the field today? Or is this for
some theoretical savings of 0.2% of battery life for a platform that's
coming out in 2029?
Re: [PATCH v9 0/4] SRF: Fix offline CPU preventing pc6 entry
Posted by Artem Bityutskiy 11 months, 1 week ago
On Fri, 2025-01-10 at 08:07 -0800, Dave Hansen wrote:
> On 1/10/25 07:26, Artem Bityutskiy wrote:
> > > I noticed there are no Fixes: or Cc:stable@ tags on this. Should we be
> > > treating this like a new feature or a bug fix?
> > It would be very helpful to have this in v6.12, because it is LTS. So I
> > would
> > suggest
> > 
> > Cc: stable@vger.kernel.org # v6.12
> 
> I was _kinda_ hoping to hear how this affects users.
> 
> Is this a big deal for real users out in the field today?

Not a big deal. Most SRF users should have a BIOS with the workaround by now.

>  Or is this for
> some theoretical savings of 0.2% of battery life for a platform that's
> coming out in 2029?

If I interpret this question as "what is your motivation precisely", I would
answer this way:

* Because version 6.12 is LTS, there is a good chance that users of near-future
new platforms will run 6.12 on them.
* If a near-future platform happens to miss the firmware workaround for this
issue, having these patches in 6.12 will likely mean that most users are OK.

No other motivation.

Overall, I would categorize back-porting this patch-set to 6.12 as "nice to
have".

Thank you!
Re: [PATCH v9 0/4] SRF: Fix offline CPU preventing pc6 entry
Posted by Patryk Wlazlyn 11 months ago
> * Because version 6.12 is LTS, there is a good chance that users of near-future
> new platforms will run 6.12 on them.
> * If a near-future platform happens to miss the firmware workaround for this
> issue, having these patches in 6.12 will likely mean that most users are OK.

Make sense to me. Any objections to adding "Cc stable v6.12"?
Re: [PATCH v9 0/4] SRF: Fix offline CPU preventing pc6 entry
Posted by Rafael J. Wysocki 11 months ago
On Thu, Jan 16, 2025 at 5:02 PM Patryk Wlazlyn
<patryk.wlazlyn@linux.intel.com> wrote:
>
> > * Because version 6.12 is LTS, there is a good chance that users of near-future
> > new platforms will run 6.12 on them.
> > * If a near-future platform happens to miss the firmware workaround for this
> > issue, having these patches in 6.12 will likely mean that most users are OK.
>
> Make sense to me. Any objections to adding "Cc stable v6.12"?

Well, that's not what you add anyway.  "Cc stable" is a maintainer thing.