mbox series

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

Message ID 20250110115953.6058-1-patryk.wlazlyn@linux.intel.com (mailing list archive)
Headers show
Series SRF: Fix offline CPU preventing pc6 entry | expand

Message

Patryk Wlazlyn Jan. 10, 2025, 11:59 a.m. UTC
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(-)

Comments

Dave Hansen Jan. 10, 2025, 3:17 p.m. UTC | #1
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?
Artem Bityutskiy Jan. 10, 2025, 3:26 p.m. UTC | #2
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.
Dave Hansen Jan. 10, 2025, 4:07 p.m. UTC | #3
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?
Artem Bityutskiy Jan. 10, 2025, 4:50 p.m. UTC | #4
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!
Rafael J. Wysocki Jan. 13, 2025, 7:31 p.m. UTC | #5
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!