This replaces raw model numbers (and comments in some cases) with names. For
probe_mwait_errata(), merge the comments with the table to make it easier to
see which erratum is which, and drop a stray "Problem" in LNL030.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
There isn't an obvious helper in Linux to use which can avoid the data
parameter; Linux doesn't really do lists like this which are *just* a VFM
match with nothing else.
Either way, I've opted for a few extra NULLs than a proliferation of
X86_MATCH_*() helpers.
---
xen/arch/x86/acpi/cpu_idle.c | 48 ++++++++++++++-------------------
xen/arch/x86/cpu/intel.c | 51 +++++++++++++++++-------------------
2 files changed, 44 insertions(+), 55 deletions(-)
diff --git a/xen/arch/x86/acpi/cpu_idle.c b/xen/arch/x86/acpi/cpu_idle.c
index fee29353439e..78e98e9c134d 100644
--- a/xen/arch/x86/acpi/cpu_idle.c
+++ b/xen/arch/x86/acpi/cpu_idle.c
@@ -583,7 +583,6 @@ bool errata_c6_workaround(void)
if ( unlikely(fix_needed == -1) )
{
-#define INTEL_FAM6_MODEL(m) { X86_VENDOR_INTEL, 6, m, X86_FEATURE_ALWAYS }
/*
* Errata AAJ72: EOI Transaction May Not be Sent if Software Enters
* Core C6 During an Interrupt Service Routine"
@@ -594,12 +593,12 @@ bool errata_c6_workaround(void)
* there is an EOI pending.
*/
static const struct x86_cpu_id eoi_errata[] = {
- INTEL_FAM6_MODEL(0x1a),
- INTEL_FAM6_MODEL(0x1e),
- INTEL_FAM6_MODEL(0x1f),
- INTEL_FAM6_MODEL(0x25),
- INTEL_FAM6_MODEL(0x2c),
- INTEL_FAM6_MODEL(0x2f),
+ X86_MATCH_VFM(INTEL_NEHALEM_EP, NULL),
+ X86_MATCH_VFM(INTEL_NEHALEM, NULL),
+ X86_MATCH_VFM(INTEL_NEHALEM_G, NULL),
+ X86_MATCH_VFM(INTEL_WESTMERE, NULL),
+ X86_MATCH_VFM(INTEL_WESTMERE_EP, NULL),
+ X86_MATCH_VFM(INTEL_WESTMERE_EX, NULL),
{ }
};
/*
@@ -617,29 +616,22 @@ bool errata_c6_workaround(void)
* discovered on Haswell hardware, and is affected.
*/
static const struct x86_cpu_id isr_errata[] = {
- /* Haswell */
- INTEL_FAM6_MODEL(0x3c),
- INTEL_FAM6_MODEL(0x3f),
- INTEL_FAM6_MODEL(0x45),
- INTEL_FAM6_MODEL(0x46),
- /* Broadwell */
- INTEL_FAM6_MODEL(0x47),
- INTEL_FAM6_MODEL(0x3d),
- INTEL_FAM6_MODEL(0x4f),
- INTEL_FAM6_MODEL(0x56),
- /* Skylake (client) */
- INTEL_FAM6_MODEL(0x5e),
- INTEL_FAM6_MODEL(0x4e),
- /* {Sky/Cascade}lake (server) */
- INTEL_FAM6_MODEL(0x55),
- /* {Kaby/Coffee/Whiskey/Amber} Lake */
- INTEL_FAM6_MODEL(0x9e),
- INTEL_FAM6_MODEL(0x8e),
- /* Cannon Lake */
- INTEL_FAM6_MODEL(0x66),
+ X86_MATCH_VFM(INTEL_HASWELL, NULL),
+ X86_MATCH_VFM(INTEL_HASWELL_X, NULL),
+ X86_MATCH_VFM(INTEL_HASWELL_L, NULL),
+ X86_MATCH_VFM(INTEL_HASWELL_G, NULL),
+ X86_MATCH_VFM(INTEL_BROADWELL, NULL),
+ X86_MATCH_VFM(INTEL_BROADWELL_G, NULL),
+ X86_MATCH_VFM(INTEL_BROADWELL_X, NULL),
+ X86_MATCH_VFM(INTEL_BROADWELL_D, NULL),
+ X86_MATCH_VFM(INTEL_SKYLAKE_L, NULL),
+ X86_MATCH_VFM(INTEL_SKYLAKE, NULL),
+ X86_MATCH_VFM(INTEL_SKYLAKE_X, NULL),
+ X86_MATCH_VFM(INTEL_KABYLAKE_L, NULL),
+ X86_MATCH_VFM(INTEL_KABYLAKE, NULL),
+ X86_MATCH_VFM(INTEL_CANNONLAKE_L, NULL),
{ }
};
-#undef INTEL_FAM6_MODEL
fix_needed = cpu_has_apic &&
((!directed_eoi_enabled && x86_match_cpu(eoi_errata)) ||
diff --git a/xen/arch/x86/cpu/intel.c b/xen/arch/x86/cpu/intel.c
index 26a171aa363e..2028a609453b 100644
--- a/xen/arch/x86/cpu/intel.c
+++ b/xen/arch/x86/cpu/intel.c
@@ -382,16 +382,12 @@ static void cf_check early_init_intel(struct cpuinfo_x86 *c)
*/
static void probe_c3_errata(const struct cpuinfo_x86 *c)
{
-#define INTEL_FAM6_MODEL(m) { X86_VENDOR_INTEL, 6, m, X86_FEATURE_ALWAYS }
static const struct x86_cpu_id models[] = {
- /* Nehalem */
- INTEL_FAM6_MODEL(0x1a),
- INTEL_FAM6_MODEL(0x1e),
- INTEL_FAM6_MODEL(0x1f),
- INTEL_FAM6_MODEL(0x2e),
- /* Westmere (note Westmere-EX is not affected) */
- INTEL_FAM6_MODEL(0x2c),
- INTEL_FAM6_MODEL(0x25),
+ X86_MATCH_VFM(INTEL_NEHALEM_EP, NULL),
+ X86_MATCH_VFM(INTEL_NEHALEM, NULL),
+ X86_MATCH_VFM(INTEL_NEHALEM_G, NULL),
+ X86_MATCH_VFM(INTEL_WESTMERE, NULL),
+ X86_MATCH_VFM(INTEL_WESTMERE_EP, NULL),
{ }
};
@@ -405,29 +401,30 @@ 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 __init probe_mwait_errata(void)
{
static const struct x86_cpu_id __initconst models[] = {
- INTEL_FAM6_MODEL(INTEL_FAM6_ATOM_GOLDMONT), /* APL30 */
- INTEL_FAM6_MODEL(INTEL_FAM6_ICELAKE_X), /* ICX143 */
- INTEL_FAM6_MODEL(INTEL_FAM6_LUNARLAKE_M), /* LNL030 */
+ /*
+ * 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.
+ */
+ X86_MATCH_VFM(INTEL_ATOM_GOLDMONT, NULL),
+
+ /*
+ * ICX143: Under complex microarchitectural conditions, a monitor that
+ * is armed with the MWAIT instruction may not be triggered, leading
+ * to a processor hang.
+ */
+ X86_MATCH_VFM(INTEL_ICELAKE_X, NULL),
+
+ /*
+ * LNL030: P-cores may not exit power state Core C6 on monitor hit.
+ */
+ X86_MATCH_VFM(INTEL_LUNARLAKE_M, NULL),
{ }
};
-#undef INTEL_FAM6_MODEL
if ( boot_cpu_has(X86_FEATURE_MONITOR) && x86_match_cpu(models) )
{
--
2.39.5
On 16.07.2025 19:31, Andrew Cooper wrote:
> This replaces raw model numbers (and comments in some cases) with names. For
> probe_mwait_errata(), merge the comments with the table to make it easier to
> see which erratum is which, and drop a stray "Problem" in LNL030.
>
> No functional change.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
>
> There isn't an obvious helper in Linux to use which can avoid the data
> parameter; Linux doesn't really do lists like this which are *just* a VFM
> match with nothing else.
>
> Either way, I've opted for a few extra NULLs than a proliferation of
> X86_MATCH_*() helpers.
+1
> --- a/xen/arch/x86/acpi/cpu_idle.c
> +++ b/xen/arch/x86/acpi/cpu_idle.c
> @@ -583,7 +583,6 @@ bool errata_c6_workaround(void)
>
> if ( unlikely(fix_needed == -1) )
> {
> -#define INTEL_FAM6_MODEL(m) { X86_VENDOR_INTEL, 6, m, X86_FEATURE_ALWAYS }
> /*
> * Errata AAJ72: EOI Transaction May Not be Sent if Software Enters
> * Core C6 During an Interrupt Service Routine"
> @@ -594,12 +593,12 @@ bool errata_c6_workaround(void)
> * there is an EOI pending.
> */
> static const struct x86_cpu_id eoi_errata[] = {
> - INTEL_FAM6_MODEL(0x1a),
> - INTEL_FAM6_MODEL(0x1e),
> - INTEL_FAM6_MODEL(0x1f),
> - INTEL_FAM6_MODEL(0x25),
> - INTEL_FAM6_MODEL(0x2c),
> - INTEL_FAM6_MODEL(0x2f),
> + X86_MATCH_VFM(INTEL_NEHALEM_EP, NULL),
> + X86_MATCH_VFM(INTEL_NEHALEM, NULL),
> + X86_MATCH_VFM(INTEL_NEHALEM_G, NULL),
> + X86_MATCH_VFM(INTEL_WESTMERE, NULL),
> + X86_MATCH_VFM(INTEL_WESTMERE_EP, NULL),
> + X86_MATCH_VFM(INTEL_WESTMERE_EX, NULL),
> { }
> };
Along the lines of a comment further down, maybe make explicit that Nehalem-EX
is intentionally omitted here (assuming that's not in fact an oversight)?
> --- a/xen/arch/x86/cpu/intel.c
> +++ b/xen/arch/x86/cpu/intel.c
> @@ -382,16 +382,12 @@ static void cf_check early_init_intel(struct cpuinfo_x86 *c)
> */
> static void probe_c3_errata(const struct cpuinfo_x86 *c)
> {
> -#define INTEL_FAM6_MODEL(m) { X86_VENDOR_INTEL, 6, m, X86_FEATURE_ALWAYS }
> static const struct x86_cpu_id models[] = {
> - /* Nehalem */
> - INTEL_FAM6_MODEL(0x1a),
> - INTEL_FAM6_MODEL(0x1e),
> - INTEL_FAM6_MODEL(0x1f),
> - INTEL_FAM6_MODEL(0x2e),
> - /* Westmere (note Westmere-EX is not affected) */
> - INTEL_FAM6_MODEL(0x2c),
> - INTEL_FAM6_MODEL(0x25),
> + X86_MATCH_VFM(INTEL_NEHALEM_EP, NULL),
> + X86_MATCH_VFM(INTEL_NEHALEM, NULL),
> + X86_MATCH_VFM(INTEL_NEHALEM_G, NULL),
> + X86_MATCH_VFM(INTEL_WESTMERE, NULL),
> + X86_MATCH_VFM(INTEL_WESTMERE_EP, NULL),
> { }
> };
You lost NEHALEM_EX here. For Westmere-EX I think the comment (part) would
better be retained, to clarify that this isn't an oversight.
Jan
On 17/07/2025 8:44 am, Jan Beulich wrote:
> On 16.07.2025 19:31, Andrew Cooper wrote:
>> --- a/xen/arch/x86/acpi/cpu_idle.c
>> +++ b/xen/arch/x86/acpi/cpu_idle.c
>> @@ -583,7 +583,6 @@ bool errata_c6_workaround(void)
>>
>> if ( unlikely(fix_needed == -1) )
>> {
>> -#define INTEL_FAM6_MODEL(m) { X86_VENDOR_INTEL, 6, m, X86_FEATURE_ALWAYS }
>> /*
>> * Errata AAJ72: EOI Transaction May Not be Sent if Software Enters
>> * Core C6 During an Interrupt Service Routine"
>> @@ -594,12 +593,12 @@ bool errata_c6_workaround(void)
>> * there is an EOI pending.
>> */
>> static const struct x86_cpu_id eoi_errata[] = {
>> - INTEL_FAM6_MODEL(0x1a),
>> - INTEL_FAM6_MODEL(0x1e),
>> - INTEL_FAM6_MODEL(0x1f),
>> - INTEL_FAM6_MODEL(0x25),
>> - INTEL_FAM6_MODEL(0x2c),
>> - INTEL_FAM6_MODEL(0x2f),
>> + X86_MATCH_VFM(INTEL_NEHALEM_EP, NULL),
>> + X86_MATCH_VFM(INTEL_NEHALEM, NULL),
>> + X86_MATCH_VFM(INTEL_NEHALEM_G, NULL),
>> + X86_MATCH_VFM(INTEL_WESTMERE, NULL),
>> + X86_MATCH_VFM(INTEL_WESTMERE_EP, NULL),
>> + X86_MATCH_VFM(INTEL_WESTMERE_EX, NULL),
>> { }
>> };
> Along the lines of a comment further down, maybe make explicit that Nehalem-EX
> is intentionally omitted here (assuming that's not in fact an oversight)?
It looks to be an oversight. I've submitted a separate patch, so it can
be backported more easily.
In practice, it's covered by probe_c3_errata() which blanket disables C3
and C6 on Nehalem.
>
>> --- a/xen/arch/x86/cpu/intel.c
>> +++ b/xen/arch/x86/cpu/intel.c
>> @@ -382,16 +382,12 @@ static void cf_check early_init_intel(struct cpuinfo_x86 *c)
>> */
>> static void probe_c3_errata(const struct cpuinfo_x86 *c)
>> {
>> -#define INTEL_FAM6_MODEL(m) { X86_VENDOR_INTEL, 6, m, X86_FEATURE_ALWAYS }
>> static const struct x86_cpu_id models[] = {
>> - /* Nehalem */
>> - INTEL_FAM6_MODEL(0x1a),
>> - INTEL_FAM6_MODEL(0x1e),
>> - INTEL_FAM6_MODEL(0x1f),
>> - INTEL_FAM6_MODEL(0x2e),
>> - /* Westmere (note Westmere-EX is not affected) */
>> - INTEL_FAM6_MODEL(0x2c),
>> - INTEL_FAM6_MODEL(0x25),
>> + X86_MATCH_VFM(INTEL_NEHALEM_EP, NULL),
>> + X86_MATCH_VFM(INTEL_NEHALEM, NULL),
>> + X86_MATCH_VFM(INTEL_NEHALEM_G, NULL),
>> + X86_MATCH_VFM(INTEL_WESTMERE, NULL),
>> + X86_MATCH_VFM(INTEL_WESTMERE_EP, NULL),
>> { }
>> };
> You lost NEHALEM_EX here.
Oops, too much copy/paste.
> For Westmere-EX I think the comment (part) would
> better be retained, to clarify that this isn't an oversight.
I can't find anything which looks related for Westmere EX. I'll retain
the comment.
~Andrew
On 17.07.2025 19:57, Andrew Cooper wrote:
> On 17/07/2025 8:44 am, Jan Beulich wrote:
>> On 16.07.2025 19:31, Andrew Cooper wrote:
>>> --- a/xen/arch/x86/acpi/cpu_idle.c
>>> +++ b/xen/arch/x86/acpi/cpu_idle.c
>>> @@ -583,7 +583,6 @@ bool errata_c6_workaround(void)
>>>
>>> if ( unlikely(fix_needed == -1) )
>>> {
>>> -#define INTEL_FAM6_MODEL(m) { X86_VENDOR_INTEL, 6, m, X86_FEATURE_ALWAYS }
>>> /*
>>> * Errata AAJ72: EOI Transaction May Not be Sent if Software Enters
>>> * Core C6 During an Interrupt Service Routine"
>>> @@ -594,12 +593,12 @@ bool errata_c6_workaround(void)
>>> * there is an EOI pending.
>>> */
>>> static const struct x86_cpu_id eoi_errata[] = {
>>> - INTEL_FAM6_MODEL(0x1a),
>>> - INTEL_FAM6_MODEL(0x1e),
>>> - INTEL_FAM6_MODEL(0x1f),
>>> - INTEL_FAM6_MODEL(0x25),
>>> - INTEL_FAM6_MODEL(0x2c),
>>> - INTEL_FAM6_MODEL(0x2f),
>>> + X86_MATCH_VFM(INTEL_NEHALEM_EP, NULL),
>>> + X86_MATCH_VFM(INTEL_NEHALEM, NULL),
>>> + X86_MATCH_VFM(INTEL_NEHALEM_G, NULL),
>>> + X86_MATCH_VFM(INTEL_WESTMERE, NULL),
>>> + X86_MATCH_VFM(INTEL_WESTMERE_EP, NULL),
>>> + X86_MATCH_VFM(INTEL_WESTMERE_EX, NULL),
>>> { }
>>> };
>> Along the lines of a comment further down, maybe make explicit that Nehalem-EX
>> is intentionally omitted here (assuming that's not in fact an oversight)?
>
> It looks to be an oversight. I've submitted a separate patch, so it can
> be backported more easily.
>
> In practice, it's covered by probe_c3_errata() which blanket disables C3
> and C6 on Nehalem.
>
>>
>>> --- a/xen/arch/x86/cpu/intel.c
>>> +++ b/xen/arch/x86/cpu/intel.c
>>> @@ -382,16 +382,12 @@ static void cf_check early_init_intel(struct cpuinfo_x86 *c)
>>> */
>>> static void probe_c3_errata(const struct cpuinfo_x86 *c)
>>> {
>>> -#define INTEL_FAM6_MODEL(m) { X86_VENDOR_INTEL, 6, m, X86_FEATURE_ALWAYS }
>>> static const struct x86_cpu_id models[] = {
>>> - /* Nehalem */
>>> - INTEL_FAM6_MODEL(0x1a),
>>> - INTEL_FAM6_MODEL(0x1e),
>>> - INTEL_FAM6_MODEL(0x1f),
>>> - INTEL_FAM6_MODEL(0x2e),
>>> - /* Westmere (note Westmere-EX is not affected) */
>>> - INTEL_FAM6_MODEL(0x2c),
>>> - INTEL_FAM6_MODEL(0x25),
>>> + X86_MATCH_VFM(INTEL_NEHALEM_EP, NULL),
>>> + X86_MATCH_VFM(INTEL_NEHALEM, NULL),
>>> + X86_MATCH_VFM(INTEL_NEHALEM_G, NULL),
>>> + X86_MATCH_VFM(INTEL_WESTMERE, NULL),
>>> + X86_MATCH_VFM(INTEL_WESTMERE_EP, NULL),
>>> { }
>>> };
>> You lost NEHALEM_EX here.
>
> Oops, too much copy/paste.
>
>> For Westmere-EX I think the comment (part) would
>> better be retained, to clarify that this isn't an oversight.
>
> I can't find anything which looks related for Westmere EX. I'll retain
> the comment.
With the adjustments:
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Jan
© 2016 - 2025 Red Hat, Inc.