* [RFC][PATCH 0/4] x86/cpu: Remove duplicate microcode version matching infrastructure
@ 2024-11-20 20:24 Dave Hansen
2024-11-20 20:24 ` [RFC][PATCH 1/4] x86/cpu: Introduce new microcode matching helper Dave Hansen
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Dave Hansen @ 2024-11-20 20:24 UTC (permalink / raw)
To: linux-kernel; +Cc: x86, tglx, bp, kan.liang, Dave Hansen
x86 has generic CPU matching infrastructure. This lets you build
tables of CPUs with some property. It's mostly used for enumerating
model-specific features, but it is quite a bit more flexible than
that. In includes a facility to match steppings and microcode
versions. This generic infrastructure is built around 'struct
x86_cpu_id'.
There is a less generic, parallel CPU matching facility built around
'struct x86_cpu_desc'. It is used only for matching specific microcode
revisions. All of the 'struct x86_cpu_desc' users can be converted to
'struct x86_cpu_id'.
Do that conversion then remove the 'struct x86_cpu_desc'
infrastructure.
Testing or acks would be much appreciated!
--
events/intel/core.c | 72 +++++++++++++++++++-------------------
include/asm/cpu_device_id.h | 55 ++++++++++-------------------
kernel/cpu/amd.c | 8 ++--
kernel/cpu/match.c | 28 ++------------
4 files changed, 63 insertions(+), 100 deletions(-)
^ permalink raw reply [flat|nested] 6+ messages in thread* [RFC][PATCH 1/4] x86/cpu: Introduce new microcode matching helper 2024-11-20 20:24 [RFC][PATCH 0/4] x86/cpu: Remove duplicate microcode version matching infrastructure Dave Hansen @ 2024-11-20 20:24 ` Dave Hansen 2024-11-20 20:24 ` [RFC][PATCH 2/4] x86/cpu: Replace 'x86_cpu_desc' use with 'x86_cpu_id' Dave Hansen ` (2 subsequent siblings) 3 siblings, 0 replies; 6+ messages in thread From: Dave Hansen @ 2024-11-20 20:24 UTC (permalink / raw) To: linux-kernel; +Cc: x86, tglx, bp, kan.liang, Dave Hansen From: Dave Hansen <dave.hansen@linux.intel.com> The 'x86_cpu_id' and 'x86_cpu_desc' structures are very similar and need to be consolidated. There is a microcode version matching function for 'x86_cpu_desc' but not 'x86_cpu_id'. Create one for 'x86_cpu_id'. This essentially just leverages the x86_cpu_id->driver_data field to replace the less generic x86_cpu_desc->x86_microcode_rev field. Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com> --- b/arch/x86/include/asm/cpu_device_id.h | 1 + b/arch/x86/kernel/cpu/match.c | 11 +++++++++++ 2 files changed, 12 insertions(+) diff -puN arch/x86/include/asm/cpu_device_id.h~min-ucode-rev arch/x86/include/asm/cpu_device_id.h --- a/arch/x86/include/asm/cpu_device_id.h~min-ucode-rev 2024-11-20 12:22:04.944380677 -0800 +++ b/arch/x86/include/asm/cpu_device_id.h 2024-11-20 12:22:04.948380830 -0800 @@ -278,5 +278,6 @@ struct x86_cpu_desc { extern const struct x86_cpu_id *x86_match_cpu(const struct x86_cpu_id *match); extern bool x86_cpu_has_min_microcode_rev(const struct x86_cpu_desc *table); +extern bool x86_match_min_microcode_rev(const struct x86_cpu_id *table); #endif /* _ASM_X86_CPU_DEVICE_ID */ diff -puN arch/x86/kernel/cpu/match.c~min-ucode-rev arch/x86/kernel/cpu/match.c --- a/arch/x86/kernel/cpu/match.c~min-ucode-rev 2024-11-20 12:22:04.944380677 -0800 +++ b/arch/x86/kernel/cpu/match.c 2024-11-20 12:22:04.948380830 -0800 @@ -86,3 +86,14 @@ bool x86_cpu_has_min_microcode_rev(const return true; } EXPORT_SYMBOL_GPL(x86_cpu_has_min_microcode_rev); + +bool x86_match_min_microcode_rev(const struct x86_cpu_id *table) +{ + const struct x86_cpu_id *res = x86_match_cpu(table); + + if (!res || res->driver_data > boot_cpu_data.microcode) + return false; + + return true; +} +EXPORT_SYMBOL_GPL(x86_match_min_microcode_rev); _ ^ permalink raw reply [flat|nested] 6+ messages in thread
* [RFC][PATCH 2/4] x86/cpu: Replace 'x86_cpu_desc' use with 'x86_cpu_id' 2024-11-20 20:24 [RFC][PATCH 0/4] x86/cpu: Remove duplicate microcode version matching infrastructure Dave Hansen 2024-11-20 20:24 ` [RFC][PATCH 1/4] x86/cpu: Introduce new microcode matching helper Dave Hansen @ 2024-11-20 20:24 ` Dave Hansen 2024-11-21 9:04 ` Ingo Molnar 2024-11-20 20:24 ` [RFC][PATCH 3/4] x86/cpu: Move AMD erratum 1386 table over to 'x86_cpu_id' Dave Hansen 2024-11-20 20:24 ` [RFC][PATCH 4/4] x86/cpu: Remove 'x86_cpu_desc' infrastructure Dave Hansen 3 siblings, 1 reply; 6+ messages in thread From: Dave Hansen @ 2024-11-20 20:24 UTC (permalink / raw) To: linux-kernel; +Cc: x86, tglx, bp, kan.liang, Dave Hansen From: Dave Hansen <dave.hansen@linux.intel.com> The 'x86_cpu_desc' and 'x86_cpu_id' structures are very similar. Reduce duplicate infrastructure by moving the few users of 'x86_cpu_id' to the much more common variant. The existing X86_MATCH_VFM_STEPPINGS() helper matches ranges of steppings. Introduce a new helper to match a single stepping to make the macro use a bit less verbose. I'm a _bit_ nervous about this because X86_MATCH_VFM_STEPPING(INTEL_SANDYBRIDGE_X, 7, 0x0000070c), and X86_MATCH_VFM_STEPPINGS(INTEL_SANDYBRIDGE_X, 7, 0x0000070c), look very similar but the second one is buggy. Any suggestions for making this more foolproof would be appreciated. Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com> --- b/arch/x86/events/intel/core.c | 72 ++++++++++++++++----------------- b/arch/x86/include/asm/cpu_device_id.h | 18 ++++++++ 2 files changed, 54 insertions(+), 36 deletions(-) diff -puN arch/x86/events/intel/core.c~zap-x86_cpu_desc arch/x86/events/intel/core.c --- a/arch/x86/events/intel/core.c~zap-x86_cpu_desc 2024-11-20 12:22:05.468400705 -0800 +++ b/arch/x86/events/intel/core.c 2024-11-20 12:22:05.472400858 -0800 @@ -5340,42 +5340,42 @@ static __init void intel_clovertown_quir x86_pmu.pebs_constraints = NULL; } -static const struct x86_cpu_desc isolation_ucodes[] = { - INTEL_CPU_DESC(INTEL_HASWELL, 3, 0x0000001f), - INTEL_CPU_DESC(INTEL_HASWELL_L, 1, 0x0000001e), - INTEL_CPU_DESC(INTEL_HASWELL_G, 1, 0x00000015), - INTEL_CPU_DESC(INTEL_HASWELL_X, 2, 0x00000037), - INTEL_CPU_DESC(INTEL_HASWELL_X, 4, 0x0000000a), - INTEL_CPU_DESC(INTEL_BROADWELL, 4, 0x00000023), - INTEL_CPU_DESC(INTEL_BROADWELL_G, 1, 0x00000014), - INTEL_CPU_DESC(INTEL_BROADWELL_D, 2, 0x00000010), - INTEL_CPU_DESC(INTEL_BROADWELL_D, 3, 0x07000009), - INTEL_CPU_DESC(INTEL_BROADWELL_D, 4, 0x0f000009), - INTEL_CPU_DESC(INTEL_BROADWELL_D, 5, 0x0e000002), - INTEL_CPU_DESC(INTEL_BROADWELL_X, 1, 0x0b000014), - INTEL_CPU_DESC(INTEL_SKYLAKE_X, 3, 0x00000021), - INTEL_CPU_DESC(INTEL_SKYLAKE_X, 4, 0x00000000), - INTEL_CPU_DESC(INTEL_SKYLAKE_X, 5, 0x00000000), - INTEL_CPU_DESC(INTEL_SKYLAKE_X, 6, 0x00000000), - INTEL_CPU_DESC(INTEL_SKYLAKE_X, 7, 0x00000000), - INTEL_CPU_DESC(INTEL_SKYLAKE_X, 11, 0x00000000), - INTEL_CPU_DESC(INTEL_SKYLAKE_L, 3, 0x0000007c), - INTEL_CPU_DESC(INTEL_SKYLAKE, 3, 0x0000007c), - INTEL_CPU_DESC(INTEL_KABYLAKE, 9, 0x0000004e), - INTEL_CPU_DESC(INTEL_KABYLAKE_L, 9, 0x0000004e), - INTEL_CPU_DESC(INTEL_KABYLAKE_L, 10, 0x0000004e), - INTEL_CPU_DESC(INTEL_KABYLAKE_L, 11, 0x0000004e), - INTEL_CPU_DESC(INTEL_KABYLAKE_L, 12, 0x0000004e), - INTEL_CPU_DESC(INTEL_KABYLAKE, 10, 0x0000004e), - INTEL_CPU_DESC(INTEL_KABYLAKE, 11, 0x0000004e), - INTEL_CPU_DESC(INTEL_KABYLAKE, 12, 0x0000004e), - INTEL_CPU_DESC(INTEL_KABYLAKE, 13, 0x0000004e), +static const struct x86_cpu_id isolation_ucodes[] = { + X86_MATCH_VFM_STEPPING(INTEL_HASWELL, 3, 0x0000001f), + X86_MATCH_VFM_STEPPING(INTEL_HASWELL_L, 1, 0x0000001e), + X86_MATCH_VFM_STEPPING(INTEL_HASWELL_G, 1, 0x00000015), + X86_MATCH_VFM_STEPPING(INTEL_HASWELL_X, 2, 0x00000037), + X86_MATCH_VFM_STEPPING(INTEL_HASWELL_X, 4, 0x0000000a), + X86_MATCH_VFM_STEPPING(INTEL_BROADWELL, 4, 0x00000023), + X86_MATCH_VFM_STEPPING(INTEL_BROADWELL_G, 1, 0x00000014), + X86_MATCH_VFM_STEPPING(INTEL_BROADWELL_D, 2, 0x00000010), + X86_MATCH_VFM_STEPPING(INTEL_BROADWELL_D, 3, 0x07000009), + X86_MATCH_VFM_STEPPING(INTEL_BROADWELL_D, 4, 0x0f000009), + X86_MATCH_VFM_STEPPING(INTEL_BROADWELL_D, 5, 0x0e000002), + X86_MATCH_VFM_STEPPING(INTEL_BROADWELL_X, 1, 0x0b000014), + X86_MATCH_VFM_STEPPING(INTEL_SKYLAKE_X, 3, 0x00000021), + X86_MATCH_VFM_STEPPING(INTEL_SKYLAKE_X, 4, 0x00000000), + X86_MATCH_VFM_STEPPING(INTEL_SKYLAKE_X, 5, 0x00000000), + X86_MATCH_VFM_STEPPING(INTEL_SKYLAKE_X, 6, 0x00000000), + X86_MATCH_VFM_STEPPING(INTEL_SKYLAKE_X, 7, 0x00000000), + X86_MATCH_VFM_STEPPING(INTEL_SKYLAKE_X, 11, 0x00000000), + X86_MATCH_VFM_STEPPING(INTEL_SKYLAKE_L, 3, 0x0000007c), + X86_MATCH_VFM_STEPPING(INTEL_SKYLAKE, 3, 0x0000007c), + X86_MATCH_VFM_STEPPING(INTEL_KABYLAKE, 9, 0x0000004e), + X86_MATCH_VFM_STEPPING(INTEL_KABYLAKE_L, 9, 0x0000004e), + X86_MATCH_VFM_STEPPING(INTEL_KABYLAKE_L, 10, 0x0000004e), + X86_MATCH_VFM_STEPPING(INTEL_KABYLAKE_L, 11, 0x0000004e), + X86_MATCH_VFM_STEPPING(INTEL_KABYLAKE_L, 12, 0x0000004e), + X86_MATCH_VFM_STEPPING(INTEL_KABYLAKE, 10, 0x0000004e), + X86_MATCH_VFM_STEPPING(INTEL_KABYLAKE, 11, 0x0000004e), + X86_MATCH_VFM_STEPPING(INTEL_KABYLAKE, 12, 0x0000004e), + X86_MATCH_VFM_STEPPING(INTEL_KABYLAKE, 13, 0x0000004e), {} }; static void intel_check_pebs_isolation(void) { - x86_pmu.pebs_no_isolation = !x86_cpu_has_min_microcode_rev(isolation_ucodes); + x86_pmu.pebs_no_isolation = !x86_match_min_microcode_rev(isolation_ucodes); } static __init void intel_pebs_isolation_quirk(void) @@ -5385,16 +5385,16 @@ static __init void intel_pebs_isolation_ intel_check_pebs_isolation(); } -static const struct x86_cpu_desc pebs_ucodes[] = { - INTEL_CPU_DESC(INTEL_SANDYBRIDGE, 7, 0x00000028), - INTEL_CPU_DESC(INTEL_SANDYBRIDGE_X, 6, 0x00000618), - INTEL_CPU_DESC(INTEL_SANDYBRIDGE_X, 7, 0x0000070c), +static const struct x86_cpu_id pebs_ucodes[] = { + X86_MATCH_VFM_STEPPING(INTEL_SANDYBRIDGE, 7, 0x00000028), + X86_MATCH_VFM_STEPPING(INTEL_SANDYBRIDGE_X, 6, 0x00000618), + X86_MATCH_VFM_STEPPING(INTEL_SANDYBRIDGE_X, 7, 0x0000070c), {} }; static bool intel_snb_pebs_broken(void) { - return !x86_cpu_has_min_microcode_rev(pebs_ucodes); + return !x86_match_min_microcode_rev(pebs_ucodes); } static void intel_snb_check_microcode(void) diff -puN arch/x86/include/asm/cpu_device_id.h~zap-x86_cpu_desc arch/x86/include/asm/cpu_device_id.h --- a/arch/x86/include/asm/cpu_device_id.h~zap-x86_cpu_desc 2024-11-20 12:22:05.468400705 -0800 +++ b/arch/x86/include/asm/cpu_device_id.h 2024-11-20 12:22:05.472400858 -0800 @@ -226,6 +226,24 @@ steppings, X86_FEATURE_ANY, data) /** + * X86_MATCH_VFM_STEPPING - Match encoded vendor/family/model/stepping + * @vfm: Encoded 8-bits each for vendor, family, model + * @stepping: A single integer stepping + * @data: Driver specific data or NULL. The internal storage + * format is unsigned long. The supplied value, pointer + * etc. is cast to unsigned long internally. + * + * feature is set to wildcard + */ +#define X86_MATCH_VFM_STEPPING(vfm, stepping, data) \ + X86_MATCH_VENDORID_FAM_MODEL_STEPPINGS_FEATURE( \ + VFM_VENDOR(vfm), \ + VFM_FAMILY(vfm), \ + VFM_MODEL(vfm), \ + X86_STEPPINGS(stepping, stepping), \ + X86_FEATURE_ANY, data) + +/** * X86_MATCH_VFM_FEATURE - Match encoded vendor/family/model/feature * @vfm: Encoded 8-bits each for vendor, family, model * @feature: A X86_FEATURE bit _ ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC][PATCH 2/4] x86/cpu: Replace 'x86_cpu_desc' use with 'x86_cpu_id' 2024-11-20 20:24 ` [RFC][PATCH 2/4] x86/cpu: Replace 'x86_cpu_desc' use with 'x86_cpu_id' Dave Hansen @ 2024-11-21 9:04 ` Ingo Molnar 0 siblings, 0 replies; 6+ messages in thread From: Ingo Molnar @ 2024-11-21 9:04 UTC (permalink / raw) To: Dave Hansen; +Cc: linux-kernel, x86, tglx, bp, kan.liang * Dave Hansen <dave.hansen@linux.intel.com> wrote: > > From: Dave Hansen <dave.hansen@linux.intel.com> > > The 'x86_cpu_desc' and 'x86_cpu_id' structures are very similar. > Reduce duplicate infrastructure by moving the few users of > 'x86_cpu_id' to the much more common variant. > > The existing X86_MATCH_VFM_STEPPINGS() helper matches ranges of > steppings. Introduce a new helper to match a single stepping to make > the macro use a bit less verbose. > > I'm a _bit_ nervous about this because > > X86_MATCH_VFM_STEPPING(INTEL_SANDYBRIDGE_X, 7, 0x0000070c), > and > X86_MATCH_VFM_STEPPINGS(INTEL_SANDYBRIDGE_X, 7, 0x0000070c), > > look very similar but the second one is buggy. Any suggestions for > making this more foolproof would be appreciated. > +static const struct x86_cpu_id isolation_ucodes[] = { > + X86_MATCH_VFM_STEPPING(INTEL_HASWELL, 3, 0x0000001f), > + X86_MATCH_VFM_STEPPING(INTEL_HASWELL_L, 1, 0x0000001e), > + X86_MATCH_VFM_STEPPING(INTEL_HASWELL_G, 1, 0x00000015), > + X86_MATCH_VFM_STEPPING(INTEL_HASWELL_X, 2, 0x00000037), > /** > + * X86_MATCH_VFM_STEPPING - Match encoded vendor/family/model/stepping > + * @vfm: Encoded 8-bits each for vendor, family, model > + * @stepping: A single integer stepping > + * @data: Driver specific data or NULL. The internal storage > + * format is unsigned long. The supplied value, pointer > + * etc. is cast to unsigned long internally. > + * > + * feature is set to wildcard > + */ > +#define X86_MATCH_VFM_STEPPING(vfm, stepping, data) \ > + X86_MATCH_VENDORID_FAM_MODEL_STEPPINGS_FEATURE( \ > + VFM_VENDOR(vfm), \ > + VFM_FAMILY(vfm), \ > + VFM_MODEL(vfm), \ > + X86_STEPPINGS(stepping, stepping), \ > + X86_FEATURE_ANY, data) Yeah, this mixed with X86_MATCH_VFM_STEPPINGS() indeed looks fragile: /** * X86_MATCH_VFM_STEPPINGS - Match encoded vendor/family/model/stepping * @vfm: Encoded 8-bits each for vendor, family, model * @steppings: Bitmask of steppings to match * @data: Driver specific data or NULL. The internal storage * format is unsigned long. The supplied value, pointer * etc. is cast to unsigned long internally. * * feature is set to wildcard */ #define X86_MATCH_VFM_STEPPINGS(vfm, steppings, data) \ X86_MATCH_VENDORID_FAM_MODEL_STEPPINGS_FEATURE( \ VFM_VENDOR(vfm), \ VFM_FAMILY(vfm), \ VFM_MODEL(vfm), \ steppings, X86_FEATURE_ANY, data) I'd solve this by unifying on a single min-max range-interface: X86_MATCH_VFM_STEPPINGS(vfm, stepping_min, stepping_max, data) which simply passes GENMASK(stepping_min, stepping_max) to .steppings field. Note how almost all existing uses of X86_MATCH_VFM_STEPPINGS() already open-codes this: arch/x86/include/asm/cpu_device_id.h: * X86_MATCH_VFM_STEPPINGS - Match encoded vendor/family/model/stepping arch/x86/include/asm/cpu_device_id.h:#define X86_MATCH_VFM_STEPPINGS(vfm, steppings, data) \ arch/x86/kernel/apic/apic.c: X86_MATCH_VFM_STEPPINGS(INTEL_HASWELL_X, X86_STEPPINGS(0x2, 0x2), 0x3a), /* EP */ arch/x86/kernel/apic/apic.c: X86_MATCH_VFM_STEPPINGS(INTEL_HASWELL_X, X86_STEPPINGS(0x4, 0x4), 0x0f), /* EX */ arch/x86/kernel/apic/apic.c: X86_MATCH_VFM_STEPPINGS(INTEL_BROADWELL_D, X86_STEPPINGS(0x2, 0x2), 0x00000011), arch/x86/kernel/apic/apic.c: X86_MATCH_VFM_STEPPINGS(INTEL_BROADWELL_D, X86_STEPPINGS(0x3, 0x3), 0x0700000e), arch/x86/kernel/apic/apic.c: X86_MATCH_VFM_STEPPINGS(INTEL_BROADWELL_D, X86_STEPPINGS(0x4, 0x4), 0x0f00000c), arch/x86/kernel/apic/apic.c: X86_MATCH_VFM_STEPPINGS(INTEL_BROADWELL_D, X86_STEPPINGS(0x5, 0x5), 0x0e000003), arch/x86/kernel/apic/apic.c: X86_MATCH_VFM_STEPPINGS(INTEL_SKYLAKE_X, X86_STEPPINGS(0x3, 0x3), 0x01000136), arch/x86/kernel/apic/apic.c: X86_MATCH_VFM_STEPPINGS(INTEL_SKYLAKE_X, X86_STEPPINGS(0x4, 0x4), 0x02000014), arch/x86/kernel/apic/apic.c: X86_MATCH_VFM_STEPPINGS(INTEL_SKYLAKE_X, X86_STEPPINGS(0x5, 0xf), 0), arch/x86/kernel/cpu/common.c: X86_MATCH_VFM_STEPPINGS(vfm, steppings, issues) drivers/edac/i10nm_base.c: X86_MATCH_VFM_STEPPINGS(INTEL_ATOM_TREMONT_D, X86_STEPPINGS(0x0, 0x3), &i10nm_cfg0), drivers/edac/i10nm_base.c: X86_MATCH_VFM_STEPPINGS(INTEL_ATOM_TREMONT_D, X86_STEPPINGS(0x4, 0xf), &i10nm_cfg1), drivers/edac/i10nm_base.c: X86_MATCH_VFM_STEPPINGS(INTEL_ICELAKE_X, X86_STEPPINGS(0x0, 0x3), &i10nm_cfg0), drivers/edac/i10nm_base.c: X86_MATCH_VFM_STEPPINGS(INTEL_ICELAKE_X, X86_STEPPINGS(0x4, 0xf), &i10nm_cfg1), drivers/edac/i10nm_base.c: X86_MATCH_VFM_STEPPINGS(INTEL_ICELAKE_D, X86_STEPPINGS(0x0, 0xf), &i10nm_cfg1), drivers/edac/i10nm_base.c: X86_MATCH_VFM_STEPPINGS(INTEL_SAPPHIRERAPIDS_X, X86_STEPPINGS(0x0, 0xf), &spr_cfg), drivers/edac/i10nm_base.c: X86_MATCH_VFM_STEPPINGS(INTEL_EMERALDRAPIDS_X, X86_STEPPINGS(0x0, 0xf), &spr_cfg), drivers/edac/i10nm_base.c: X86_MATCH_VFM_STEPPINGS(INTEL_GRANITERAPIDS_X, X86_STEPPINGS(0x0, 0xf), &gnr_cfg), drivers/edac/i10nm_base.c: X86_MATCH_VFM_STEPPINGS(INTEL_ATOM_CRESTMONT_X, X86_STEPPINGS(0x0, 0xf), &gnr_cfg), drivers/edac/i10nm_base.c: X86_MATCH_VFM_STEPPINGS(INTEL_ATOM_CRESTMONT, X86_STEPPINGS(0x0, 0xf), &gnr_cfg), drivers/edac/skx_base.c: X86_MATCH_VFM_STEPPINGS(INTEL_SKYLAKE_X, X86_STEPPINGS(0x0, 0xf), &skx_cfg), So I'd start by a patch that changes X86_MATCH_VFM_STEPPINGS() and converts these usecases, and then your patch can just use the expanded parameters of X86_MATCH_VFM_STEPPINGS() with the same min-max value: X86_MATCH_VFM_STEPPINGS(INTEL_HASWELL, 3, 3, 0x0000001f), That tiny bit of verbosity is far better than the fragility of the proposed interface, IMHO. Also, sometimes single-stepping ranges will expand as quirks/features expand in scope, so this is the more natural interface anyway. ... or you can define a trivial single-stepping wrapper: X86_MATCH_VFM_STEPPING(vfm, stepping, data) \ X86_MATCH_VFM_STEPPINGS(vfm, stepping, stepping, data) Note how this is not nearly as fragile, as typoing the interface would result in a build failure, not a silently broken kernel. Thanks, Ingo ^ permalink raw reply [flat|nested] 6+ messages in thread
* [RFC][PATCH 3/4] x86/cpu: Move AMD erratum 1386 table over to 'x86_cpu_id' 2024-11-20 20:24 [RFC][PATCH 0/4] x86/cpu: Remove duplicate microcode version matching infrastructure Dave Hansen 2024-11-20 20:24 ` [RFC][PATCH 1/4] x86/cpu: Introduce new microcode matching helper Dave Hansen 2024-11-20 20:24 ` [RFC][PATCH 2/4] x86/cpu: Replace 'x86_cpu_desc' use with 'x86_cpu_id' Dave Hansen @ 2024-11-20 20:24 ` Dave Hansen 2024-11-20 20:24 ` [RFC][PATCH 4/4] x86/cpu: Remove 'x86_cpu_desc' infrastructure Dave Hansen 3 siblings, 0 replies; 6+ messages in thread From: Dave Hansen @ 2024-11-20 20:24 UTC (permalink / raw) To: linux-kernel; +Cc: x86, tglx, bp, kan.liang, Dave Hansen From: Dave Hansen <dave.hansen@linux.intel.com> The AMD erratum 1386 detection code uses and old style 'x86_cpu_desc' table. Replace it with 'x86_cpu_id' so the old style can be removed. I did not create a new helper macro here. The new table is certainly more noisy than the old and it can be improved on. But I was hesitant to create a new macro just for a single site that is only two ugly lines in the end. Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com> --- b/arch/x86/kernel/cpu/amd.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff -puN arch/x86/kernel/cpu/amd.c~amd-x86_cpu_id arch/x86/kernel/cpu/amd.c --- a/arch/x86/kernel/cpu/amd.c~amd-x86_cpu_id 2024-11-20 12:22:05.988420580 -0800 +++ b/arch/x86/kernel/cpu/amd.c 2024-11-20 12:22:05.992420733 -0800 @@ -795,9 +795,9 @@ static void init_amd_bd(struct cpuinfo_x clear_rdrand_cpuid_bit(c); } -static const struct x86_cpu_desc erratum_1386_microcode[] = { - AMD_CPU_DESC(0x17, 0x1, 0x2, 0x0800126e), - AMD_CPU_DESC(0x17, 0x31, 0x0, 0x08301052), +static const struct x86_cpu_id erratum_1386_microcode[] = { + X86_MATCH_VFM_STEPPING(VFM_MAKE(X86_VENDOR_AMD, 0x17, 0x01), 0x2, 0x0800126e), + X86_MATCH_VFM_STEPPING(VFM_MAKE(X86_VENDOR_AMD, 0x17, 0x31), 0x0, 0x08301052), }; static void fix_erratum_1386(struct cpuinfo_x86 *c) @@ -813,7 +813,7 @@ static void fix_erratum_1386(struct cpui * Clear the feature flag only on microcode revisions which * don't have the fix. */ - if (x86_cpu_has_min_microcode_rev(erratum_1386_microcode)) + if (x86_match_min_microcode_rev(erratum_1386_microcode)) return; clear_cpu_cap(c, X86_FEATURE_XSAVES); _ ^ permalink raw reply [flat|nested] 6+ messages in thread
* [RFC][PATCH 4/4] x86/cpu: Remove 'x86_cpu_desc' infrastructure 2024-11-20 20:24 [RFC][PATCH 0/4] x86/cpu: Remove duplicate microcode version matching infrastructure Dave Hansen ` (2 preceding siblings ...) 2024-11-20 20:24 ` [RFC][PATCH 3/4] x86/cpu: Move AMD erratum 1386 table over to 'x86_cpu_id' Dave Hansen @ 2024-11-20 20:24 ` Dave Hansen 3 siblings, 0 replies; 6+ messages in thread From: Dave Hansen @ 2024-11-20 20:24 UTC (permalink / raw) To: linux-kernel; +Cc: x86, tglx, bp, kan.liang, Dave Hansen From: Dave Hansen <dave.hansen@linux.intel.com> All the users of 'x86_cpu_desc' are gone. Zap it from the tree. Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com> --- b/arch/x86/include/asm/cpu_device_id.h | 36 --------------------------------- b/arch/x86/kernel/cpu/match.c | 31 ---------------------------- 2 files changed, 67 deletions(-) diff -puN arch/x86/include/asm/cpu_device_id.h~zap-x86_cpu_desc-3 arch/x86/include/asm/cpu_device_id.h --- a/arch/x86/include/asm/cpu_device_id.h~zap-x86_cpu_desc-3 2024-11-20 12:22:06.484439538 -0800 +++ b/arch/x86/include/asm/cpu_device_id.h 2024-11-20 12:22:06.488439691 -0800 @@ -260,42 +260,6 @@ VFM_MODEL(vfm), \ X86_STEPPING_ANY, feature, data) -/* - * Match specific microcode revisions. - * - * vendor/family/model/stepping must be all set. - * - * Only checks against the boot CPU. When mixed-stepping configs are - * valid for a CPU model, add a quirk for every valid stepping and - * do the fine-tuning in the quirk handler. - */ - -struct x86_cpu_desc { - u8 x86_family; - u8 x86_vendor; - u8 x86_model; - u8 x86_stepping; - u32 x86_microcode_rev; -}; - -#define INTEL_CPU_DESC(vfm, stepping, revision) { \ - .x86_family = VFM_FAMILY(vfm), \ - .x86_vendor = VFM_VENDOR(vfm), \ - .x86_model = VFM_MODEL(vfm), \ - .x86_stepping = (stepping), \ - .x86_microcode_rev = (revision), \ -} - -#define AMD_CPU_DESC(fam, model, stepping, revision) { \ - .x86_family = (fam), \ - .x86_vendor = X86_VENDOR_AMD, \ - .x86_model = (model), \ - .x86_stepping = (stepping), \ - .x86_microcode_rev = (revision), \ -} - -extern const struct x86_cpu_id *x86_match_cpu(const struct x86_cpu_id *match); -extern bool x86_cpu_has_min_microcode_rev(const struct x86_cpu_desc *table); extern bool x86_match_min_microcode_rev(const struct x86_cpu_id *table); #endif /* _ASM_X86_CPU_DEVICE_ID */ diff -puN arch/x86/kernel/cpu/match.c~zap-x86_cpu_desc-3 arch/x86/kernel/cpu/match.c --- a/arch/x86/kernel/cpu/match.c~zap-x86_cpu_desc-3 2024-11-20 12:22:06.484439538 -0800 +++ b/arch/x86/kernel/cpu/match.c 2024-11-20 12:22:06.488439691 -0800 @@ -56,37 +56,6 @@ const struct x86_cpu_id *x86_match_cpu(c } EXPORT_SYMBOL(x86_match_cpu); -static const struct x86_cpu_desc * -x86_match_cpu_with_stepping(const struct x86_cpu_desc *match) -{ - struct cpuinfo_x86 *c = &boot_cpu_data; - const struct x86_cpu_desc *m; - - for (m = match; m->x86_family | m->x86_model; m++) { - if (c->x86_vendor != m->x86_vendor) - continue; - if (c->x86 != m->x86_family) - continue; - if (c->x86_model != m->x86_model) - continue; - if (c->x86_stepping != m->x86_stepping) - continue; - return m; - } - return NULL; -} - -bool x86_cpu_has_min_microcode_rev(const struct x86_cpu_desc *table) -{ - const struct x86_cpu_desc *res = x86_match_cpu_with_stepping(table); - - if (!res || res->x86_microcode_rev > boot_cpu_data.microcode) - return false; - - return true; -} -EXPORT_SYMBOL_GPL(x86_cpu_has_min_microcode_rev); - bool x86_match_min_microcode_rev(const struct x86_cpu_id *table) { const struct x86_cpu_id *res = x86_match_cpu(table); _ ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-11-21 9:04 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-11-20 20:24 [RFC][PATCH 0/4] x86/cpu: Remove duplicate microcode version matching infrastructure Dave Hansen 2024-11-20 20:24 ` [RFC][PATCH 1/4] x86/cpu: Introduce new microcode matching helper Dave Hansen 2024-11-20 20:24 ` [RFC][PATCH 2/4] x86/cpu: Replace 'x86_cpu_desc' use with 'x86_cpu_id' Dave Hansen 2024-11-21 9:04 ` Ingo Molnar 2024-11-20 20:24 ` [RFC][PATCH 3/4] x86/cpu: Move AMD erratum 1386 table over to 'x86_cpu_id' Dave Hansen 2024-11-20 20:24 ` [RFC][PATCH 4/4] x86/cpu: Remove 'x86_cpu_desc' infrastructure Dave Hansen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®