[PATCH] x86/intel: workaround several MONITOR/MWAIT errata

[PATCH] x86/intel: workaround several MONITOR/MWAIT errata

[Roger Pau Monne]

There are several errata on Intel regarding the usage of the MONITOR/MWAIT
instructions, all having in common that stores to the monitored region
might not wake up the CPU.

Fix them by forcing the sending of an IPI for the affected models.

The Ice Lake issue has been reproduced internally on XenServer hardware,
and the fix does seem to prevent it.  The symptom was APs getting stuck in
the idle loop immediately after bring up, which in turn prevented the BSP
from making progress.  This would happen before the watchdog was
initialized, and hence the whole system would get stuck.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Apollo and Lunar Lake fixes have not been tested, due to lack of hardware.
---
 xen/arch/x86/acpi/cpu_idle.c     |  6 +++++
 xen/arch/x86/cpu/intel.c         | 41 +++++++++++++++++++++++++++++++-
 xen/arch/x86/include/asm/mwait.h |  3 +++
 3 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/acpi/cpu_idle.c b/xen/arch/x86/acpi/cpu_idle.c
index 420198406def..f8f11f3c31e4 100644
--- a/xen/arch/x86/acpi/cpu_idle.c
+++ b/xen/arch/x86/acpi/cpu_idle.c
@@ -441,8 +441,14 @@ void cpuidle_wakeup_mwait(cpumask_t *mask)
     cpumask_andnot(mask, mask, &target);
 }
 
+/* Force sending of a wakeup IPI regardless of mwait usage. */
+bool force_mwait_ipi_wakeup __read_mostly;
+
 bool arch_skip_send_event_check(unsigned int cpu)
 {
+    if ( force_mwait_ipi_wakeup )
+        return false;
+
     /*
      * This relies on softirq_pending() and mwait_wakeup() to access data
      * on the same cache line.
diff --git a/xen/arch/x86/cpu/intel.c b/xen/arch/x86/cpu/intel.c
index 6a680ba38dc9..9d7c6ea297a9 100644
--- a/xen/arch/x86/cpu/intel.c
+++ b/xen/arch/x86/cpu/intel.c
@@ -8,6 +8,7 @@
 #include <asm/intel-family.h>
 #include <asm/processor.h>
 #include <asm/msr.h>
+#include <asm/mwait.h>
 #include <asm/uaccess.h>
 #include <asm/mpspec.h>
 #include <asm/apic.h>
@@ -368,7 +369,6 @@ static void probe_c3_errata(const struct cpuinfo_x86 *c)
         INTEL_FAM6_MODEL(0x25),
         { }
     };
-#undef INTEL_FAM6_MODEL
 
     /* Serialized by the AP bringup code. */
     if ( max_cstate > 1 && (c->apicid & (c->x86_num_siblings - 1)) &&
@@ -380,6 +380,43 @@ static void probe_c3_errata(const struct cpuinfo_x86 *c)
     }
 }
 
+/*
+ * APL30: One use of the MONITOR/MWAIT instruction pair is to allow a logical
+ * processor to wait in a sleep state until a store to the armed address range
+ * occurs. Due to this erratum, stores to the armed address range may not
+ * trigger MWAIT to resume execution.
+ *
+ * ICX143: Under complex microarchitectural conditions, a monitor that is armed
+ * with the MWAIT instruction may not be triggered, leading to a processor
+ * hang.
+ *
+ * LNL030: Problem P-cores may not exit power state Core C6 on monitor hit.
+ *
+ * Force the sending of an IPI in those cases.
+ */
+static void probe_mwait_errata(void)
+{
+    static const struct x86_cpu_id models[] = {
+        /* Apollo Lake */
+        INTEL_FAM6_MODEL(0x5C),
+        /* Ice Lake */
+        INTEL_FAM6_MODEL(0x6A),
+        INTEL_FAM6_MODEL(0x6C),
+        /* Lunar Lake */
+        INTEL_FAM6_MODEL(0xBD),
+        { }
+    };
+#undef INTEL_FAM6_MODEL
+
+    if ( boot_cpu_has(X86_FEATURE_MONITOR) && !force_mwait_ipi_wakeup &&
+         x86_match_cpu(models) )
+    {
+        printk(XENLOG_WARNING
+               "Forcing IPI MWAIT wakeup due to CPU erratum\n");
+        force_mwait_ipi_wakeup = true;
+    }
+}
+
 /*
  * P4 Xeon errata 037 workaround.
  * Hardware prefetcher may cause stale data to be loaded into the cache.
@@ -406,6 +443,8 @@ static void Intel_errata_workarounds(struct cpuinfo_x86 *c)
 		__set_bit(X86_FEATURE_CLFLUSH_MONITOR, c->x86_capability);
 
 	probe_c3_errata(c);
+	if (c == &boot_cpu_data)
+		probe_mwait_errata();
 }
 
 
diff --git a/xen/arch/x86/include/asm/mwait.h b/xen/arch/x86/include/asm/mwait.h
index 000a692f6d19..c52cd3f51011 100644
--- a/xen/arch/x86/include/asm/mwait.h
+++ b/xen/arch/x86/include/asm/mwait.h
@@ -13,6 +13,9 @@
 
 #define MWAIT_ECX_INTERRUPT_BREAK	0x1
 
+/* Force sending of a wakeup IPI regardless of mwait usage. */
+extern bool force_mwait_ipi_wakeup;
+
 void mwait_idle_with_hints(unsigned int eax, unsigned int ecx);
 #ifdef CONFIG_INTEL
 bool mwait_pc10_supported(void);
-- 
2.48.1

Re: [PATCH] x86/intel: workaround several MONITOR/MWAIT errata

[Jan Beulich]

On 17.04.2025 18:19, Roger Pau Monne wrote:
> --- a/xen/arch/x86/acpi/cpu_idle.c
> +++ b/xen/arch/x86/acpi/cpu_idle.c
> @@ -441,8 +441,14 @@ void cpuidle_wakeup_mwait(cpumask_t *mask)
>      cpumask_andnot(mask, mask, &target);
>  }
>  
> +/* Force sending of a wakeup IPI regardless of mwait usage. */
> +bool force_mwait_ipi_wakeup __read_mostly;

Nit: Annotation between type and identifier, please.

> @@ -380,6 +380,43 @@ static void probe_c3_errata(const struct cpuinfo_x86 *c)
>      }
>  }
>  
> +/*
> + * APL30: One use of the MONITOR/MWAIT instruction pair is to allow a logical
> + * processor to wait in a sleep state until a store to the armed address range
> + * occurs. Due to this erratum, stores to the armed address range may not
> + * trigger MWAIT to resume execution.
> + *
> + * ICX143: Under complex microarchitectural conditions, a monitor that is armed
> + * with the MWAIT instruction may not be triggered, leading to a processor
> + * hang.
> + *
> + * LNL030: Problem P-cores may not exit power state Core C6 on monitor hit.

I didn't manage to spot all three spec updates; none of these have a ucode fix,
hence permitting the workaround to be avoided?

Since CPX is 3rd Gen Xeon Scalable just like ICX is, I'm surprised that one's
unaffected. The most recent spec update there is a year old than ICX'es, so
may simply be too old to include the erratum?

Sunny Cove is used by further Icelake models - they're known to be unaffected?

> + * Force the sending of an IPI in those cases.
> + */
> +static void probe_mwait_errata(void)
> +{
> +    static const struct x86_cpu_id models[] = {
> +        /* Apollo Lake */
> +        INTEL_FAM6_MODEL(0x5C),
> +        /* Ice Lake */
> +        INTEL_FAM6_MODEL(0x6A),
> +        INTEL_FAM6_MODEL(0x6C),
> +        /* Lunar Lake */
> +        INTEL_FAM6_MODEL(0xBD),

Use identifiers from intel-family.h here?

> +        { }
> +    };
> +#undef INTEL_FAM6_MODEL
> +
> +    if ( boot_cpu_has(X86_FEATURE_MONITOR) && !force_mwait_ipi_wakeup &&
> +         x86_match_cpu(models) )
> +    {
> +        printk(XENLOG_WARNING
> +               "Forcing IPI MWAIT wakeup due to CPU erratum\n");
> +        force_mwait_ipi_wakeup = true;
> +    }
> +}

Do we really need to cater for asymmetric systems? IOW can't we do this once
on the BSP? Otherwise - why the use of boot_cpu_has() here? Oh, wait ...

> @@ -406,6 +443,8 @@ static void Intel_errata_workarounds(struct cpuinfo_x86 *c)
>  		__set_bit(X86_FEATURE_CLFLUSH_MONITOR, c->x86_capability);
>  
>  	probe_c3_errata(c);
> +	if (c == &boot_cpu_data)
> +		probe_mwait_errata();
>  }

..., you do this for the BSP only. Then why's the function not __init and
the global variable not __ro_after_init (and models[] __initconst)?

(Later) Except that this path is also taken for S3 resume, from
recheck_cpu_features(). This shouldn't alter the variable value anymore,
though. A disagreement ought to result in recheck_cpu_features() to
report failure. (Imo perhaps better to avoid the call above during resume.)

Jan

Re: [PATCH] x86/intel: workaround several MONITOR/MWAIT errata

[Andrew Cooper]

On 22/04/2025 9:26 am, Jan Beulich wrote:
> On 17.04.2025 18:19, Roger Pau Monne wrote:
>> @@ -380,6 +380,43 @@ static void probe_c3_errata(const struct cpuinfo_x86 *c)
>>      }
>>  }
>>  
>> +/*
>> + * APL30: One use of the MONITOR/MWAIT instruction pair is to allow a logical
>> + * processor to wait in a sleep state until a store to the armed address range
>> + * occurs. Due to this erratum, stores to the armed address range may not
>> + * trigger MWAIT to resume execution.
>> + *
>> + * ICX143: Under complex microarchitectural conditions, a monitor that is armed
>> + * with the MWAIT instruction may not be triggered, leading to a processor
>> + * hang.
>> + *
>> + * LNL030: Problem P-cores may not exit power state Core C6 on monitor hit.
> I didn't manage to spot all three spec updates; none of these have a ucode fix,
> hence permitting the workaround to be avoided?
>
> Since CPX is 3rd Gen Xeon Scalable just like ICX is, I'm surprised that one's
> unaffected. The most recent spec update there is a year old than ICX'es, so
> may simply be too old to include the erratum?

CPX being "3rd generation" is especially creative marketing.  It's
literally another stepping of SKX/CLX with some extra AVX512
instructions, so is an entire "tock" away from ICX.

> Sunny Cove is used by further Icelake models - they're known to be unaffected?

ICX143 is specific to the server design.  It doesn't affect the client
design.

~Andrew

Re: [PATCH] x86/intel: workaround several MONITOR/MWAIT errata

[Roger Pau Monné]

On Tue, Apr 22, 2025 at 10:26:37AM +0200, Jan Beulich wrote:
> On 17.04.2025 18:19, Roger Pau Monne wrote:
> > @@ -380,6 +380,43 @@ static void probe_c3_errata(const struct cpuinfo_x86 *c)
> >      }
> >  }
> >  
> > +/*
> > + * APL30: One use of the MONITOR/MWAIT instruction pair is to allow a logical
> > + * processor to wait in a sleep state until a store to the armed address range
> > + * occurs. Due to this erratum, stores to the armed address range may not
> > + * trigger MWAIT to resume execution.
> > + *
> > + * ICX143: Under complex microarchitectural conditions, a monitor that is armed
> > + * with the MWAIT instruction may not be triggered, leading to a processor
> > + * hang.
> > + *
> > + * LNL030: Problem P-cores may not exit power state Core C6 on monitor hit.
> 
> I didn't manage to spot all three spec updates; none of these have a ucode fix,
> hence permitting the workaround to be avoided?
> 
> Since CPX is 3rd Gen Xeon Scalable just like ICX is, I'm surprised that one's
> unaffected. The most recent spec update there is a year old than ICX'es, so
> may simply be too old to include the erratum?
> 
> Sunny Cove is used by further Icelake models - they're known to be unaffected?

FWIW, client IceLake also seems to be unaffected, so I don't really
know.  So far I've only found this issue in the ICX spec update.  The
fix for Linux limits this further to model 106 only.

> > + * Force the sending of an IPI in those cases.
> > + */
> > +static void probe_mwait_errata(void)
> > +{
> > +    static const struct x86_cpu_id models[] = {
> > +        /* Apollo Lake */
> > +        INTEL_FAM6_MODEL(0x5C),
> > +        /* Ice Lake */
> > +        INTEL_FAM6_MODEL(0x6A),
> > +        INTEL_FAM6_MODEL(0x6C),
> > +        /* Lunar Lake */
> > +        INTEL_FAM6_MODEL(0xBD),
> 
> Use identifiers from intel-family.h here?

Sure.

> > +        { }
> > +    };
> > +#undef INTEL_FAM6_MODEL
> > +
> > +    if ( boot_cpu_has(X86_FEATURE_MONITOR) && !force_mwait_ipi_wakeup &&
> > +         x86_match_cpu(models) )
> > +    {
> > +        printk(XENLOG_WARNING
> > +               "Forcing IPI MWAIT wakeup due to CPU erratum\n");
> > +        force_mwait_ipi_wakeup = true;
> > +    }
> > +}
> 
> Do we really need to cater for asymmetric systems? IOW can't we do this once
> on the BSP? Otherwise - why the use of boot_cpu_has() here? Oh, wait ...
> 
> > @@ -406,6 +443,8 @@ static void Intel_errata_workarounds(struct cpuinfo_x86 *c)
> >  		__set_bit(X86_FEATURE_CLFLUSH_MONITOR, c->x86_capability);
> >  
> >  	probe_c3_errata(c);
> > +	if (c == &boot_cpu_data)
> > +		probe_mwait_errata();
> >  }
> 
> ..., you do this for the BSP only. Then why's the function not __init and
> the global variable not __ro_after_init (and models[] __initconst)?
> 
> (Later) Except that this path is also taken for S3 resume, from
> recheck_cpu_features(). This shouldn't alter the variable value anymore,
> though. A disagreement ought to result in recheck_cpu_features() to
> report failure. (Imo perhaps better to avoid the call above during resume.)

I did consider to limit the call based on system_state, wasn't sure
whether that would be more churn than help.

LNL030 has a reference to: "It may be possible for the BIOS to contain
a workaround for this erratum." so wasn't fully sure we wouldn't need
to check for this in all cores if there's some firmware fix for it
that Xen could identify.

Thanks, Roger.

Re: [PATCH] x86/intel: workaround several MONITOR/MWAIT errata

[Jan Beulich]

On 23.04.2025 10:45, Roger Pau Monné wrote:
> On Tue, Apr 22, 2025 at 10:26:37AM +0200, Jan Beulich wrote:
>> On 17.04.2025 18:19, Roger Pau Monne wrote:
>>> @@ -380,6 +380,43 @@ static void probe_c3_errata(const struct cpuinfo_x86 *c)
>>>      }
>>>  }
>>>  
>>> +/*
>>> + * APL30: One use of the MONITOR/MWAIT instruction pair is to allow a logical
>>> + * processor to wait in a sleep state until a store to the armed address range
>>> + * occurs. Due to this erratum, stores to the armed address range may not
>>> + * trigger MWAIT to resume execution.
>>> + *
>>> + * ICX143: Under complex microarchitectural conditions, a monitor that is armed
>>> + * with the MWAIT instruction may not be triggered, leading to a processor
>>> + * hang.
>>> + *
>>> + * LNL030: Problem P-cores may not exit power state Core C6 on monitor hit.
>>
>> I didn't manage to spot all three spec updates; none of these have a ucode fix,
>> hence permitting the workaround to be avoided?
>>
>> Since CPX is 3rd Gen Xeon Scalable just like ICX is, I'm surprised that one's
>> unaffected. The most recent spec update there is a year old than ICX'es, so
>> may simply be too old to include the erratum?
>>
>> Sunny Cove is used by further Icelake models - they're known to be unaffected?
> 
> FWIW, client IceLake also seems to be unaffected, so I don't really
> know.  So far I've only found this issue in the ICX spec update.  The
> fix for Linux limits this further to model 106 only.
> 
>>> + * Force the sending of an IPI in those cases.
>>> + */
>>> +static void probe_mwait_errata(void)
>>> +{
>>> +    static const struct x86_cpu_id models[] = {
>>> +        /* Apollo Lake */
>>> +        INTEL_FAM6_MODEL(0x5C),
>>> +        /* Ice Lake */
>>> +        INTEL_FAM6_MODEL(0x6A),
>>> +        INTEL_FAM6_MODEL(0x6C),
>>> +        /* Lunar Lake */
>>> +        INTEL_FAM6_MODEL(0xBD),
>>
>> Use identifiers from intel-family.h here?
> 
> Sure.
> 
>>> +        { }
>>> +    };
>>> +#undef INTEL_FAM6_MODEL
>>> +
>>> +    if ( boot_cpu_has(X86_FEATURE_MONITOR) && !force_mwait_ipi_wakeup &&
>>> +         x86_match_cpu(models) )
>>> +    {
>>> +        printk(XENLOG_WARNING
>>> +               "Forcing IPI MWAIT wakeup due to CPU erratum\n");
>>> +        force_mwait_ipi_wakeup = true;
>>> +    }
>>> +}
>>
>> Do we really need to cater for asymmetric systems? IOW can't we do this once
>> on the BSP? Otherwise - why the use of boot_cpu_has() here? Oh, wait ...
>>
>>> @@ -406,6 +443,8 @@ static void Intel_errata_workarounds(struct cpuinfo_x86 *c)
>>>  		__set_bit(X86_FEATURE_CLFLUSH_MONITOR, c->x86_capability);
>>>  
>>>  	probe_c3_errata(c);
>>> +	if (c == &boot_cpu_data)
>>> +		probe_mwait_errata();
>>>  }
>>
>> ..., you do this for the BSP only. Then why's the function not __init and
>> the global variable not __ro_after_init (and models[] __initconst)?
>>
>> (Later) Except that this path is also taken for S3 resume, from
>> recheck_cpu_features(). This shouldn't alter the variable value anymore,
>> though. A disagreement ought to result in recheck_cpu_features() to
>> report failure. (Imo perhaps better to avoid the call above during resume.)
> 
> I did consider to limit the call based on system_state, wasn't sure
> whether that would be more churn than help.

The fundamental expectation is that such aspects don't changes across
S3. I'd suggest to add the extra check, but I wouldn't insist; what I'd
like to see is the variable becoming __ro_after_init, though (whichever
way this is being arranged for).

> LNL030 has a reference to: "It may be possible for the BIOS to contain
> a workaround for this erratum." so wasn't fully sure we wouldn't need
> to check for this in all cores if there's some firmware fix for it
> that Xen could identify.

Usually this means a ucode update (occasionally it may also be a chipset
config that firmware can fiddle with), in which case we wouldn't
normally (i.e. unless in critical situations) add a workaround at all.

Jan

Re: [PATCH] x86/intel: workaround several MONITOR/MWAIT errata

[Frediano Ziglio]

On Thu, Apr 17, 2025 at 5:19 PM Roger Pau Monne <roger.pau@citrix.com> wrote:
>
> There are several errata on Intel regarding the usage of the MONITOR/MWAIT
> instructions, all having in common that stores to the monitored region
> might not wake up the CPU.
>
> Fix them by forcing the sending of an IPI for the affected models.
>
> The Ice Lake issue has been reproduced internally on XenServer hardware,
> and the fix does seem to prevent it.  The symptom was APs getting stuck in
> the idle loop immediately after bring up, which in turn prevented the BSP
> from making progress.  This would happen before the watchdog was
> initialized, and hence the whole system would get stuck.
>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> Apollo and Lunar Lake fixes have not been tested, due to lack of hardware.
> ---
>  xen/arch/x86/acpi/cpu_idle.c     |  6 +++++
>  xen/arch/x86/cpu/intel.c         | 41 +++++++++++++++++++++++++++++++-
>  xen/arch/x86/include/asm/mwait.h |  3 +++
>  3 files changed, 49 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/x86/acpi/cpu_idle.c b/xen/arch/x86/acpi/cpu_idle.c
> index 420198406def..f8f11f3c31e4 100644
> --- a/xen/arch/x86/acpi/cpu_idle.c
> +++ b/xen/arch/x86/acpi/cpu_idle.c
> @@ -441,8 +441,14 @@ void cpuidle_wakeup_mwait(cpumask_t *mask)
>      cpumask_andnot(mask, mask, &target);
>  }
>
> +/* Force sending of a wakeup IPI regardless of mwait usage. */
> +bool force_mwait_ipi_wakeup __read_mostly;
> +
>  bool arch_skip_send_event_check(unsigned int cpu)
>  {
> +    if ( force_mwait_ipi_wakeup )
> +        return false;
> +
>      /*
>       * This relies on softirq_pending() and mwait_wakeup() to access data
>       * on the same cache line.
> diff --git a/xen/arch/x86/cpu/intel.c b/xen/arch/x86/cpu/intel.c
> index 6a680ba38dc9..9d7c6ea297a9 100644
> --- a/xen/arch/x86/cpu/intel.c
> +++ b/xen/arch/x86/cpu/intel.c
> @@ -8,6 +8,7 @@
>  #include <asm/intel-family.h>
>  #include <asm/processor.h>
>  #include <asm/msr.h>
> +#include <asm/mwait.h>
>  #include <asm/uaccess.h>
>  #include <asm/mpspec.h>
>  #include <asm/apic.h>
> @@ -368,7 +369,6 @@ static void probe_c3_errata(const struct cpuinfo_x86 *c)
>          INTEL_FAM6_MODEL(0x25),
>          { }
>      };
> -#undef INTEL_FAM6_MODEL
>
>      /* Serialized by the AP bringup code. */
>      if ( max_cstate > 1 && (c->apicid & (c->x86_num_siblings - 1)) &&
> @@ -380,6 +380,43 @@ static void probe_c3_errata(const struct cpuinfo_x86 *c)
>      }
>  }
>
> +/*
> + * APL30: One use of the MONITOR/MWAIT instruction pair is to allow a logical
> + * processor to wait in a sleep state until a store to the armed address range
> + * occurs. Due to this erratum, stores to the armed address range may not
> + * trigger MWAIT to resume execution.
> + *
> + * ICX143: Under complex microarchitectural conditions, a monitor that is armed
> + * with the MWAIT instruction may not be triggered, leading to a processor
> + * hang.
> + *
> + * LNL030: Problem P-cores may not exit power state Core C6 on monitor hit.
> + *
> + * Force the sending of an IPI in those cases.
> + */
> +static void probe_mwait_errata(void)
> +{
> +    static const struct x86_cpu_id models[] = {
> +        /* Apollo Lake */
> +        INTEL_FAM6_MODEL(0x5C),
> +        /* Ice Lake */
> +        INTEL_FAM6_MODEL(0x6A),
> +        INTEL_FAM6_MODEL(0x6C),

Intel patch for Linux only adds model 0x6a, not 0x6c. Did we manage to
reproduce on 0x6c? Which patch is more correct? Surely we are on the
safer side.

> +        /* Lunar Lake */
> +        INTEL_FAM6_MODEL(0xBD),
> +        { }
> +    };
> +#undef INTEL_FAM6_MODEL
> +
> +    if ( boot_cpu_has(X86_FEATURE_MONITOR) && !force_mwait_ipi_wakeup &&
> +         x86_match_cpu(models) )
> +    {
> +        printk(XENLOG_WARNING
> +               "Forcing IPI MWAIT wakeup due to CPU erratum\n");
> +        force_mwait_ipi_wakeup = true;
> +    }
> +}
> +
>  /*
>   * P4 Xeon errata 037 workaround.
>   * Hardware prefetcher may cause stale data to be loaded into the cache.
> @@ -406,6 +443,8 @@ static void Intel_errata_workarounds(struct cpuinfo_x86 *c)
>                 __set_bit(X86_FEATURE_CLFLUSH_MONITOR, c->x86_capability);
>
>         probe_c3_errata(c);
> +       if (c == &boot_cpu_data)
> +               probe_mwait_errata();
>  }
>
>
> diff --git a/xen/arch/x86/include/asm/mwait.h b/xen/arch/x86/include/asm/mwait.h
> index 000a692f6d19..c52cd3f51011 100644
> --- a/xen/arch/x86/include/asm/mwait.h
> +++ b/xen/arch/x86/include/asm/mwait.h
> @@ -13,6 +13,9 @@
>
>  #define MWAIT_ECX_INTERRUPT_BREAK      0x1
>
> +/* Force sending of a wakeup IPI regardless of mwait usage. */
> +extern bool force_mwait_ipi_wakeup;
> +
>  void mwait_idle_with_hints(unsigned int eax, unsigned int ecx);
>  #ifdef CONFIG_INTEL
>  bool mwait_pc10_supported(void);

Frediano

Re: [PATCH] x86/intel: workaround several MONITOR/MWAIT errata

[Andrew Cooper]

On 22/04/2025 11:40 am, Frediano Ziglio wrote:
> On Thu, Apr 17, 2025 at 5:19 PM Roger Pau Monne <roger.pau@citrix.com> wrote:
>> diff --git a/xen/arch/x86/cpu/intel.c b/xen/arch/x86/cpu/intel.c
>> index 6a680ba38dc9..9d7c6ea297a9 100644
>> --- a/xen/arch/x86/cpu/intel.c
>> +++ b/xen/arch/x86/cpu/intel.c
>> @@ -380,6 +380,43 @@ static void probe_c3_errata(const struct cpuinfo_x86 *c)
>>      }
>>  }
>>
>> +/*
>> + * APL30: One use of the MONITOR/MWAIT instruction pair is to allow a logical
>> + * processor to wait in a sleep state until a store to the armed address range
>> + * occurs. Due to this erratum, stores to the armed address range may not
>> + * trigger MWAIT to resume execution.
>> + *
>> + * ICX143: Under complex microarchitectural conditions, a monitor that is armed
>> + * with the MWAIT instruction may not be triggered, leading to a processor
>> + * hang.
>> + *
>> + * LNL030: Problem P-cores may not exit power state Core C6 on monitor hit.
>> + *
>> + * Force the sending of an IPI in those cases.
>> + */
>> +static void probe_mwait_errata(void)
>> +{
>> +    static const struct x86_cpu_id models[] = {
>> +        /* Apollo Lake */
>> +        INTEL_FAM6_MODEL(0x5C),
>> +        /* Ice Lake */
>> +        INTEL_FAM6_MODEL(0x6A),
>> +        INTEL_FAM6_MODEL(0x6C),
> Intel patch for Linux only adds model 0x6a, not 0x6c. Did we manage to
> reproduce on 0x6c? Which patch is more correct? Surely we are on the
> safer side.

It only affects 0x6a, but I can't go into why.  Now that Linux is
picking up this workaround, we should follow Dave Hansen managed to get
out of Intel's internal bugtracker on the matter.

~Andrew