diff mbox series

[1/1] x86/cpu: Add INTEL_LUNARLAKE_M to X86_BUG_MONITOR

Message ID 351549432f8d766842dec74ccab443077ea0af91.1731389117.git.len.brown@intel.com (mailing list archive)
State Handled Elsewhere, archived
Headers show
Series x86/cpu: Add INTEL_LUNARLAKE_M to X86_BUG_MONITOR | expand

Commit Message

Len Brown Nov. 12, 2024, 5:28 a.m. UTC 
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>
---
 arch/x86/kernel/cpu/intel.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Rafael J. Wysocki Nov. 12, 2024, 11:44 a.m. UTC | #1 
On Tue, Nov 12, 2024 at 6:37 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>

So again

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

and see one super-minor nit below.

> ---
>  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..284cd561499c 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))

I would put the || at the end of the previous line, that is

> +           (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
>
Rafael J. Wysocki Nov. 12, 2024, 1:14 p.m. UTC | #2 
On Tue, Nov 12, 2024 at 2:12 PM Len Brown <lenb@kernel.org> wrote:
>
> On Tue, Nov 12, 2024 at 6:44 AM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> > > -       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))
> >
> > I would put the || at the end of the previous line, that is
>
>
> It isn't my personal preference for human readability either,
> but this is what scripts/Lindent does...

Well, it doesn't match the coding style of the first line ...
Rafael J. Wysocki Nov. 12, 2024, 3:02 p.m. UTC | #3 
On Tue, Nov 12, 2024 at 3:02 PM Len Brown <lenb@kernel.org> wrote:
>
> On Tue, Nov 12, 2024 at 8:14 AM Rafael J. Wysocki <rafael@kernel.org> wrote:
> >
> > On Tue, Nov 12, 2024 at 2:12 PM Len Brown <lenb@kernel.org> wrote:
> > >
> > > On Tue, Nov 12, 2024 at 6:44 AM Rafael J. Wysocki <rafael@kernel.org> wrote:
> > >
> > > > > -       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))
> > > >
> > > > I would put the || at the end of the previous line, that is
> > >
> > >
> > > It isn't my personal preference for human readability either,
> > > but this is what scripts/Lindent does...
> >
> > Well, it doesn't match the coding style of the first line ...
>
> Fair observation.
>
> I'll bite.
>
> If you took the existing intel.c and added it as a patch to the kernel,
> the resulting checkpatch would have 6 errors and 33 warnings.
>
> If you ran Lindent on the existing intel.c, the resulting diff would be
> 408 lines --  1 file changed, 232 insertions(+), 176 deletions(-)
>
> This for a file that is only 1300 lines long.
>
> If whitespace nirvana is the goal, tools are the answer, not the valuable
> cycles of human reviewers.

Well, the advice always given is to follow the coding style of the
given fine in the first place.

checkpatch reflects the preferences of its author is this particular
respect and maintainers' preferences tend to differ from one to
another.
diff mbox series

Patch

diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index e7656cbef68d..284cd561499c 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