[PATCH v3] x86/cpu: Add INTEL_LUNARLAKE_M to X86_BUG_MONITOR

[PATCH v3] x86/cpu: Add INTEL_LUNARLAKE_M to X86_BUG_MONITOR

[Len Brown]

From: Len Brown <len.brown@intel.com>

Under some conditions, MONITOR wakeups on Lunar Lake processors
can be lost, resulting in significant user-visible delays.

Add LunarLake to X86_BUG_MONITOR so that wake_up_idle_cpu()
always sends an IPI, avoiding this potential delay.

Closes: https://bugzilla.kernel.org/show_bug.cgi?id=219364

Cc: stable@vger.kernel.org # 6.11
Signed-off-by: Len Brown <len.brown@intel.com>
---
v3 syntax tweak
v2 leave smp_kick_mwait_play_dead() alone

 arch/x86/kernel/cpu/intel.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index e7656cbef68d..4b5f3d052151 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -586,7 +586,9 @@ static void init_intel(struct cpuinfo_x86 *c)
 	     c->x86_vfm == INTEL_WESTMERE_EX))
 		set_cpu_bug(c, X86_BUG_CLFLUSH_MONITOR);
 
-	if (boot_cpu_has(X86_FEATURE_MWAIT) && c->x86_vfm == INTEL_ATOM_GOLDMONT)
+	if (boot_cpu_has(X86_FEATURE_MWAIT) &&
+	    (c->x86_vfm == INTEL_ATOM_GOLDMONT ||
+	     c->x86_vfm == INTEL_LUNARLAKE_M))
 		set_cpu_bug(c, X86_BUG_MONITOR);
 
 #ifdef CONFIG_X86_64
-- 
2.43.0

Re: [PATCH v3] x86/cpu: Add INTEL_LUNARLAKE_M to X86_BUG_MONITOR

[Rafael J. Wysocki]

On Wed, Nov 13, 2024 at 3:07 AM Len Brown <lenb@kernel.org> wrote:
>
> From: Len Brown <len.brown@intel.com>
>
> Under some conditions, MONITOR wakeups on Lunar Lake processors
> can be lost, resulting in significant user-visible delays.
>
> Add LunarLake to X86_BUG_MONITOR so that wake_up_idle_cpu()
> always sends an IPI, avoiding this potential delay.
>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=219364
>
> Cc: stable@vger.kernel.org # 6.11
> Signed-off-by: Len Brown <len.brown@intel.com>

Still

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

> ---
> v3 syntax tweak
> v2 leave smp_kick_mwait_play_dead() alone
>
>  arch/x86/kernel/cpu/intel.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
> index e7656cbef68d..4b5f3d052151 100644
> --- a/arch/x86/kernel/cpu/intel.c
> +++ b/arch/x86/kernel/cpu/intel.c
> @@ -586,7 +586,9 @@ static void init_intel(struct cpuinfo_x86 *c)
>              c->x86_vfm == INTEL_WESTMERE_EX))
>                 set_cpu_bug(c, X86_BUG_CLFLUSH_MONITOR);
>
> -       if (boot_cpu_has(X86_FEATURE_MWAIT) && c->x86_vfm == INTEL_ATOM_GOLDMONT)
> +       if (boot_cpu_has(X86_FEATURE_MWAIT) &&
> +           (c->x86_vfm == INTEL_ATOM_GOLDMONT ||
> +            c->x86_vfm == INTEL_LUNARLAKE_M))
>                 set_cpu_bug(c, X86_BUG_MONITOR);
>
>  #ifdef CONFIG_X86_64
> --
> 2.43.0
>

Re: [PATCH v3] x86/cpu: Add INTEL_LUNARLAKE_M to X86_BUG_MONITOR

[Dave Hansen]

On 11/12/24 18:07, Len Brown wrote:
> From: Len Brown <len.brown@intel.com>
> 
> Under some conditions, MONITOR wakeups on Lunar Lake processors
> can be lost, resulting in significant user-visible delays.
> 
> Add LunarLake to X86_BUG_MONITOR so that wake_up_idle_cpu()
> always sends an IPI, avoiding this potential delay.

This kinda implies that X86_BUG_MONITOR only does one thing.  What about
the two other places in the tree that check it.  Are those relevant?

> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=219364
> 
> Cc: stable@vger.kernel.org # 6.11
> Signed-off-by: Len Brown <len.brown@intel.com>

This obviously conflicts with the VFM infrastructure, but shouldn't this
also get backported to even older stable kernels?

I thought the "# 6.11" was to tell folks where it is *needed*, not where
it actually applies.

Re: [PATCH v3] x86/cpu: Add INTEL_LUNARLAKE_M to X86_BUG_MONITOR

[Rafael J. Wysocki]

On Wed, Nov 20, 2024 at 8:12 PM Dave Hansen <dave.hansen@intel.com> wrote:
>
> On 11/12/24 18:07, Len Brown wrote:
> > From: Len Brown <len.brown@intel.com>
> >
> > Under some conditions, MONITOR wakeups on Lunar Lake processors
> > can be lost, resulting in significant user-visible delays.
> >
> > Add LunarLake to X86_BUG_MONITOR so that wake_up_idle_cpu()
> > always sends an IPI, avoiding this potential delay.
>
> This kinda implies that X86_BUG_MONITOR only does one thing.  What about
> the two other places in the tree that check it.  Are those relevant?

They are relevant, but related.

The first one prevents mwait_idle() from becoming the default idle
function, which only matters if cpuidle is not used, but this is
consistent with the mwait_idle_with_hints() behavior.

The second one prevents KVM from using MWAIT in the guest which I
would think is a good idea in this case.

> > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=219364
> >
> > Cc: stable@vger.kernel.org # 6.11
> > Signed-off-by: Len Brown <len.brown@intel.com>
>
> This obviously conflicts with the VFM infrastructure, but shouldn't this
> also get backported to even older stable kernels?

As a matter of principle, it should go to all of the stable kernel
series still in use, but it obviously needs backporting and I'm not
really sure how attractive the old kernel series will be for LNL users
(quite likely not at all).

> I thought the "# 6.11" was to tell folks where it is *needed*, not where
> it actually applies.

My interpretation is slightly different: This is the oldest series one
wants the given patch to go to.

Re: [PATCH v3] x86/cpu: Add INTEL_LUNARLAKE_M to X86_BUG_MONITOR

[Dave Hansen]

On 11/20/24 12:08, Rafael J. Wysocki wrote:
>>> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=219364
>>>
>>> Cc: stable@vger.kernel.org # 6.11
>>> Signed-off-by: Len Brown <len.brown@intel.com>
>> This obviously conflicts with the VFM infrastructure, but shouldn't this
>> also get backported to even older stable kernels?
> As a matter of principle, it should go to all of the stable kernel
> series still in use, but it obviously needs backporting and I'm not
> really sure how attractive the old kernel series will be for LNL users
> (quite likely not at all).

I'm not going to lose sleep over it, but as a policy, I think we should
backport CPU fixes to all the stable kernels. I don't feel like I have a
good enough handle on what kernels folks run on new systems to make a
prediction.

Re: [PATCH v3] x86/cpu: Add INTEL_LUNARLAKE_M to X86_BUG_MONITOR

[Len Brown]

On Wed, Nov 20, 2024 at 3:21 PM Dave Hansen <dave.hansen@intel.com> wrote:

> I'm not going to lose sleep over it, but as a policy, I think we should
> backport CPU fixes to all the stable kernels. I don't feel like I have a
> good enough handle on what kernels folks run on new systems to make a
> prediction.

FWIW, I sent a backport of a slightly earlier version of this patch,
but all I got back was vitriol about violating the kernel Documentation on
sending to stable.

Maybe a native english speaker could re-write that Documentation,
so that a native english speaker can understand it?
Or better yet, somebody can write a script or update checkpatch so that
developers can more likely avoid the Soup Nazi treatment?

ie. I agree with you, and I'm happy to help, but it isn't clear how.

Len Brown, Intel

Re: [PATCH v3] x86/cpu: Add INTEL_LUNARLAKE_M to X86_BUG_MONITOR

[Thomas Gleixner]

On Thu, Nov 21 2024 at 05:22, Len Brown wrote:
> On Wed, Nov 20, 2024 at 3:21 PM Dave Hansen <dave.hansen@intel.com> wrote:
>> I'm not going to lose sleep over it, but as a policy, I think we should
>> backport CPU fixes to all the stable kernels. I don't feel like I have a
>> good enough handle on what kernels folks run on new systems to make a
>> prediction.
>
> FWIW, I sent a backport of a slightly earlier version of this patch,
> but all I got back was vitriol about violating the kernel Documentation on
> sending to stable.
>
> Maybe a native english speaker could re-write that Documentation,
> so that a native english speaker can understand it?

What's so hard to understand?

 There are three options to submit a change to -stable trees:

  1. Add a'stable tag' to the description of a patch you then submit for mainline
     inclusion.

  2. Ask the stable team to pick up a patch already mainlined.

  3. Submit a patch to the stable team that is equivalent to a change
     already mainlined.

Is very clear and understandable english, no?

#1    is the preferred one and only requires a "stable tag"

#2/#3 can only be done once the fix is upstream as they require the
      upstream commit id.

It's clearly spelled out in the detailed descriptions of the three
options.

Thanks,

        tglx