mwait-idle's ICPU() is the most convenient place to get started. Introduce
X86_MATCH_CPU() and X86_MATCH_VFM() following their Linux counterparts.
This involves match-cpu.h including more headers, which in turn allows us to
drop a few.
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>
We now have X86_FEATURE_ANY and X86_FEATURE_ALWAYS as aliases of LM. Given
the contexts they're used in, I've left the naming as-is.
It's a bit nasty (preprocessing wise) triple-expanding VFM in X86_MATCH_VFM(),
but we need an Integer Constant Expression.
---
xen/arch/x86/cpu/intel.c | 1 -
xen/arch/x86/cpu/mwait-idle.c | 4 +---
xen/arch/x86/include/asm/match-cpu.h | 21 ++++++++++++++++++++-
3 files changed, 21 insertions(+), 5 deletions(-)
diff --git a/xen/arch/x86/cpu/intel.c b/xen/arch/x86/cpu/intel.c
index ee1ae92cd7e6..26a171aa363e 100644
--- a/xen/arch/x86/cpu/intel.c
+++ b/xen/arch/x86/cpu/intel.c
@@ -7,7 +7,6 @@
#include <asm/apic.h>
#include <asm/i387.h>
-#include <asm/intel-family.h>
#include <asm/match-cpu.h>
#include <asm/mpspec.h>
#include <asm/msr.h>
diff --git a/xen/arch/x86/cpu/mwait-idle.c b/xen/arch/x86/cpu/mwait-idle.c
index eec2823cbacf..e837cbf50eb3 100644
--- a/xen/arch/x86/cpu/mwait-idle.c
+++ b/xen/arch/x86/cpu/mwait-idle.c
@@ -51,7 +51,6 @@
#include <asm/cpuidle.h>
#include <asm/hpet.h>
-#include <asm/intel-family.h>
#include <asm/match-cpu.h>
#include <asm/msr.h>
#include <asm/mwait.h>
@@ -1302,8 +1301,7 @@ static const struct idle_cpu idle_cpu_srf = {
};
#define ICPU(model, cpu) \
- { X86_VENDOR_INTEL, 6, INTEL_FAM6_ ## model, X86_FEATURE_ALWAYS, \
- &idle_cpu_ ## cpu}
+ X86_MATCH_VFM(INTEL_ ## model, &idle_cpu_ ## cpu)
static const struct x86_cpu_id intel_idle_ids[] __initconstrel = {
ICPU(NEHALEM_EP, nehalem),
diff --git a/xen/arch/x86/include/asm/match-cpu.h b/xen/arch/x86/include/asm/match-cpu.h
index 2704b84d74c9..dcdc50a70d14 100644
--- a/xen/arch/x86/include/asm/match-cpu.h
+++ b/xen/arch/x86/include/asm/match-cpu.h
@@ -4,14 +4,33 @@
#include <xen/stdint.h>
+#include <asm/cpufeature.h>
+#include <asm/intel-family.h>
+#include <asm/x86-vendors.h>
+
+#define X86_FEATURE_ANY X86_FEATURE_LM
+
struct x86_cpu_id {
uint16_t vendor;
uint16_t family;
uint16_t model;
- uint16_t feature;
+ uint16_t feature; /* X86_FEATURE_*, or X86_FEATURE_ANY */
const void *driver_data;
};
+#define X86_MATCH_CPU(v, f, m, feat, data) \
+ { \
+ .vendor = (v), \
+ .family = (f), \
+ .model = (m), \
+ .feature = (feat), \
+ .driver_data = (const void *)(unsigned long)(data), \
+ }
+
+#define X86_MATCH_VFM(vfm, data) \
+ X86_MATCH_CPU(VFM_VENDOR(vfm), VFM_FAMILY(vfm), \
+ VFM_MODEL(vfm), X86_FEATURE_ANY, data)
+
/*
* x86_match_cpu() - match the CPU against an array of x86_cpu_ids[]
*
--
2.39.5
On 16.07.2025 19:31, Andrew Cooper wrote: > mwait-idle's ICPU() is the most convenient place to get started. Introduce > X86_MATCH_CPU() and X86_MATCH_VFM() following their Linux counterparts. > > This involves match-cpu.h including more headers, which in turn allows us to > drop a few. intel-cpu.h doesn't really need to move, does it? Conceivably there can be users of match-cpu.h which don't need the Intel constants. Hence no point in forcing them to see those. > 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> > > We now have X86_FEATURE_ANY and X86_FEATURE_ALWAYS as aliases of LM. Given > the contexts they're used in, I've left the naming as-is. What's wrong with sticking to ALWAYS, which we already have? > It's a bit nasty (preprocessing wise) triple-expanding VFM in X86_MATCH_VFM(), > but we need an Integer Constant Expression. Not sure what alternative you're alluding to, or in fact what nastiness you're seeing. But maybe I'm getting "triple-expanding" wrong: To me that means going through three layers of expansion, when you may mean the fact that X86_MATCH_VFM() evaluates its parameter three times. If so - yes, but as you say: What do you do (in this case). Jan
On 17/07/2025 8:35 am, Jan Beulich wrote:
> On 16.07.2025 19:31, Andrew Cooper wrote:
>> mwait-idle's ICPU() is the most convenient place to get started. Introduce
>> X86_MATCH_CPU() and X86_MATCH_VFM() following their Linux counterparts.
>>
>> This involves match-cpu.h including more headers, which in turn allows us to
>> drop a few.
> intel-cpu.h doesn't really need to move, does it? Conceivably there can be users
> of match-cpu.h which don't need the Intel constants. Hence no point in forcing
> them to see those.
There's no point not to. All users of x86_cpu_id want the Intel names.
I've already restricted it to only 5 TUs.
Even if we do get some AMD names (and I'm not entirely sure how that
would end up looking), it's just a few defines.
>
>> 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>
>>
>> We now have X86_FEATURE_ANY and X86_FEATURE_ALWAYS as aliases of LM. Given
>> the contexts they're used in, I've left the naming as-is.
> What's wrong with sticking to ALWAYS, which we already have?
For alternatives, something like:
alternative("", "foo", X86_FEATURE_ALWAYS);
is correct in context. However:
X86_MATCH_?(..., X86_FEATURE_ALWAYS, ...);
is borderline grammatically wrong, and ANY is a better name to use.
~Andrew
On 17.07.2025 19:39, Andrew Cooper wrote:
> On 17/07/2025 8:35 am, Jan Beulich wrote:
>> On 16.07.2025 19:31, Andrew Cooper wrote:
>>> mwait-idle's ICPU() is the most convenient place to get started. Introduce
>>> X86_MATCH_CPU() and X86_MATCH_VFM() following their Linux counterparts.
>>>
>>> This involves match-cpu.h including more headers, which in turn allows us to
>>> drop a few.
>> intel-cpu.h doesn't really need to move, does it? Conceivably there can be users
>> of match-cpu.h which don't need the Intel constants. Hence no point in forcing
>> them to see those.
>
> There's no point not to. All users of x86_cpu_id want the Intel names.
> I've already restricted it to only 5 TUs.
>
> Even if we do get some AMD names (and I'm not entirely sure how that
> would end up looking), it's just a few defines.
It's just a (slowly growing set of a) few, yes. Still goes against our
desire the limit #include dependencies.
>>> 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>
>>>
>>> We now have X86_FEATURE_ANY and X86_FEATURE_ALWAYS as aliases of LM. Given
>>> the contexts they're used in, I've left the naming as-is.
>> What's wrong with sticking to ALWAYS, which we already have?
>
> For alternatives, something like:
>
> alternative("", "foo", X86_FEATURE_ALWAYS);
>
> is correct in context. However:
>
> X86_MATCH_?(..., X86_FEATURE_ALWAYS, ...);
>
> is borderline grammatically wrong, and ANY is a better name to use.
Well, I don't necessarily agree, but then the extra name also isn't
a severe problem. It was actually you who called out the redundancy.
In any event:
Acked-by: Jan Beulich <jbeulich@suse.com>
Jan
© 2016 - 2025 Red Hat, Inc.