Re: [patch 02/22] x86/cpu: Add conistent CPU match macros
On Fri, Mar 20, 2020 at 3:18 PM Thomas Gleixner tglx@linutronix.de wrote:
Finding all places which build x86_cpu_id match tables is tedious and the logic is hidden in lots of differently named macro wrappers.
Most of these initializer macros use plain C89 initializers which rely on the ordering of the struct members. So new members could only be added at the end of the struct, but that's ugly as hell and C99 initializers are really the right thing to use.
Provide a set of macros which:
Have a proper naming scheme, starting with X86_MATCH_
Use C99 initializers
The set of provided macros are all subsets of the base macro
X86_MATCH_VENDOR_FAM_MODEL_FEATURE()
which allows to supply all possible selection criteria:
vendor, family, model, feature
The other macros shorten this to avoid typing all arguments when they are not needed and would require one of the _ANY constants. They have been created due to the requirements of the existing usage sites.
Also a add a few model constants for Centaur CPUs and QUARK.
I would perhaps made this as a separate change(s).
...
+#define X86_MATCH_VENDOR_FAM_MODEL_FEATURE(_vendor, _family, _model, \
_feature, _data) { \
I would leave it on one line despite the length, but it's up to you.
.vendor = X86_VENDOR_##_vendor, \
.family = _family, \
.model = _model, \
.feature = _feature, \
.driver_data = (unsigned long) _data \
For sake of consistency shouldn't be this kernel_ulong_t ? Or we are going to get rid of that type?
}
...
+#define X86_MATCH_VENDOR_FAM_FEATURE(vendor, family, feature, data) \
X86_MATCH_VENDOR_FAM_MODEL_FEATURE(vendor, family, \
X86_MODEL_ANY, feature, data)
I would leave it on one line despite the length, but it's up to you.
...
+#define X86_MATCH_VENDOR_FAM_MODEL(vendor, family, model, data) \
X86_MATCH_VENDOR_FAM_MODEL_FEATURE(vendor, family, model, \
X86_FEATURE_ANY, data)
Ditto.
...
- X86_MATCH_VENDOR_FAM_MODEL_FEATURE(INTEL, 6, INTEL_FAM6_BROADWELL,
X86_FEATURE_ANY, NULL);
Perhaps one line?
Andy Shevchenko andy.shevchenko@gmail.com writes:
Also a add a few model constants for Centaur CPUs and QUARK.
I would perhaps made this as a separate change(s).
Can do.
+#define X86_MATCH_VENDOR_FAM_MODEL_FEATURE(_vendor, _family, _model, \
_feature, _data) { \
I would leave it on one line despite the length, but it's up to you.
.vendor = X86_VENDOR_##_vendor, \
.family = _family, \
.model = _model, \
.feature = _feature, \
.driver_data = (unsigned long) _data \
For sake of consistency shouldn't be this kernel_ulong_t ?
I can change that though in kernel space this does not matter.
Or we are going to get rid of that type?
No.
Thanks,
tglx
participants (2)
-
Andy Shevchenko
-
Thomas Gleixner