| Message ID | 20240923073311.4290-4-TonyWWang-oc@zhaoxin.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | x86/mce: Add Zhaoxin MCE support and remove | expand |
> From: Tony W Wang-oc <TonyWWang-oc@zhaoxin.com> > [...] > +config X86_MCE_ZHAOXIN > + def_bool y > + prompt "Zhaoxin MCE features" > + depends on X86_MCE_INTEL > + help > + Additional support for zhaoxin specific MCE features such as s/zhaoxin/Zhaoxin > + the corrected machine check interrupt. > + > config X86_MCE_AMD > def_bool y > prompt "AMD MCE features" > diff --git a/arch/x86/kernel/cpu/mce/Makefile > b/arch/x86/kernel/cpu/mce/Makefile > index 015856abd..2e863e78d 100644 > --- a/arch/x86/kernel/cpu/mce/Makefile > +++ b/arch/x86/kernel/cpu/mce/Makefile > @@ -5,7 +5,7 @@ obj-$(CONFIG_X86_ANCIENT_MCE) += winchip.o p5.o > obj-$(CONFIG_X86_MCE_INTEL) += intel.o > obj-$(CONFIG_X86_MCE_AMD) += amd.o > obj-$(CONFIG_X86_MCE_THRESHOLD) += threshold.o > - > +obj-$(CONFIG_X86_MCE_ZHAOXIN) += zhaoxin.o Move this newly added item just after AMD's, so they're sorted in vendors. And keep a blank line here as it was. > mce-inject-y := inject.o > obj-$(CONFIG_X86_MCE_INJECT) += mce-inject.o > > [...] > --- /dev/null > +++ b/arch/x86/kernel/cpu/mce/zhaoxin.c > @@ -0,0 +1,65 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Zhaoxin specific MCE features > + * Author: Lyle Li > + */ > +#include <asm/msr.h> > +#include "internal.h" > + > +static void mce_zhaoxin_apply_mce_broadcast(struct cpuinfo_x86 *c) { > + struct mca_config *cfg = &mca_cfg; > + > + /* Older CPUs do not do MCE broadcast */ s/MCE broadcast/MCE broadcast:/ > + if (c->x86 < 6) > + return; > + /* > + * All newer Zhaoxin and Centaur CPUs support MCE broadcasting. > Enable > + * synchronization with a one second timeout. > + */ Instead of copying and pasting the redundant comments, could you use Dave's concise comments as suggested in: https://lore.kernel.org/all/a25f878e-83d9-440a-9741-4cf86db4a716@intel.com/ /* All newer ones do: */ > + if (c->x86 > 6) > + goto mce_broadcast; > + /* Family 6 is mixed: */ > + if (c->x86_vendor == X86_VENDOR_CENTAUR) { > + if (c->x86_model == 0xf && c->x86_stepping >= 0xe) > + goto mce_broadcast; > + } else if (c->x86_vendor == X86_VENDOR_ZHAOXIN) { > + if (c->x86_model == 0x19 || c->x86_model == 0x1f) > + goto mce_broadcast; > + } > + > + return; > [...]
On 2024/10/12 14:41, Zhuo, Qiuxu wrote: > > >> From: Tony W Wang-oc <TonyWWang-oc@zhaoxin.com> >> [...] >> +config X86_MCE_ZHAOXIN >> + def_bool y >> + prompt "Zhaoxin MCE features" >> + depends on X86_MCE_INTEL >> + help >> + Additional support for zhaoxin specific MCE features such as > > s/zhaoxin/Zhaoxin > >> + the corrected machine check interrupt. >> + >> config X86_MCE_AMD >> def_bool y >> prompt "AMD MCE features" >> diff --git a/arch/x86/kernel/cpu/mce/Makefile >> b/arch/x86/kernel/cpu/mce/Makefile >> index 015856abd..2e863e78d 100644 >> --- a/arch/x86/kernel/cpu/mce/Makefile >> +++ b/arch/x86/kernel/cpu/mce/Makefile >> @@ -5,7 +5,7 @@ obj-$(CONFIG_X86_ANCIENT_MCE) += winchip.o p5.o >> obj-$(CONFIG_X86_MCE_INTEL) += intel.o >> obj-$(CONFIG_X86_MCE_AMD) += amd.o >> obj-$(CONFIG_X86_MCE_THRESHOLD) += threshold.o >> - >> +obj-$(CONFIG_X86_MCE_ZHAOXIN) += zhaoxin.o > > Move this newly added item just after AMD's, so they're sorted in vendors. > And keep a blank line here as it was. > >> mce-inject-y := inject.o >> obj-$(CONFIG_X86_MCE_INJECT) += mce-inject.o >> >> [...] >> --- /dev/null >> +++ b/arch/x86/kernel/cpu/mce/zhaoxin.c >> @@ -0,0 +1,65 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Zhaoxin specific MCE features >> + * Author: Lyle Li >> + */ >> +#include <asm/msr.h> >> +#include "internal.h" >> + >> +static void mce_zhaoxin_apply_mce_broadcast(struct cpuinfo_x86 *c) { >> + struct mca_config *cfg = &mca_cfg; >> + >> + /* Older CPUs do not do MCE broadcast */ > > s/MCE broadcast/MCE broadcast:/ > >> + if (c->x86 < 6) >> + return; >> + /* >> + * All newer Zhaoxin and Centaur CPUs support MCE broadcasting. >> Enable >> + * synchronization with a one second timeout. >> + */ > > > Instead of copying and pasting the redundant comments, could you use Dave's concise comments as suggested in: > > https://lore.kernel.org/all/a25f878e-83d9-440a-9741-4cf86db4a716@intel.com/ > > /* All newer ones do: */ >> + if (c->x86 > 6) >> + goto mce_broadcast; >> + > > /* Family 6 is mixed: */ >> + if (c->x86_vendor == X86_VENDOR_CENTAUR) { >> + if (c->x86_model == 0xf && c->x86_stepping >= 0xe) >> + goto mce_broadcast; >> + } else if (c->x86_vendor == X86_VENDOR_ZHAOXIN) { >> + if (c->x86_model == 0x19 || c->x86_model == 0x1f) >> + goto mce_broadcast; >> + } >> + >> + return; >> [...] Thank you for your review. Should I add the tag for this patch: Reviewed-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com> Sincerely TonyWWang-oc
> From: Tony W Wang-oc <TonyWWang-oc@zhaoxin.com> > [...] > Subject: Re: [PATCH v4 3/4] x86/mce: Add zhaoxin.c to support Zhaoxin MCA > > > >> From: Tony W Wang-oc <TonyWWang-oc@zhaoxin.com> [...] > >> +config X86_MCE_ZHAOXIN > >> + def_bool y > >> + prompt "Zhaoxin MCE features" > >> + depends on X86_MCE_INTEL > >> + help > >> + Additional support for zhaoxin specific MCE features such as > > > > s/zhaoxin/Zhaoxin > > > >> + the corrected machine check interrupt. > >> + > >> config X86_MCE_AMD > >> def_bool y > >> prompt "AMD MCE features" > >> diff --git a/arch/x86/kernel/cpu/mce/Makefile > >> b/arch/x86/kernel/cpu/mce/Makefile > >> index 015856abd..2e863e78d 100644 > >> --- a/arch/x86/kernel/cpu/mce/Makefile > >> +++ b/arch/x86/kernel/cpu/mce/Makefile > >> @@ -5,7 +5,7 @@ obj-$(CONFIG_X86_ANCIENT_MCE) += winchip.o p5.o > >> obj-$(CONFIG_X86_MCE_INTEL) += intel.o > >> obj-$(CONFIG_X86_MCE_AMD) += amd.o > >> obj-$(CONFIG_X86_MCE_THRESHOLD) += threshold.o > >> - > >> +obj-$(CONFIG_X86_MCE_ZHAOXIN) += zhaoxin.o > > > > Move this newly added item just after AMD's, so they're sorted in vendors. > > And keep a blank line here as it was. > > > >> mce-inject-y := inject.o > >> obj-$(CONFIG_X86_MCE_INJECT) += mce-inject.o > >> > >> [...] > >> --- /dev/null > >> +++ b/arch/x86/kernel/cpu/mce/zhaoxin.c > >> @@ -0,0 +1,65 @@ > >> +// SPDX-License-Identifier: GPL-2.0 > >> +/* > >> + * Zhaoxin specific MCE features > >> + * Author: Lyle Li > >> + */ > >> +#include <asm/msr.h> > >> +#include "internal.h" > >> + > >> +static void mce_zhaoxin_apply_mce_broadcast(struct cpuinfo_x86 *c) { > >> + struct mca_config *cfg = &mca_cfg; > >> + > >> + /* Older CPUs do not do MCE broadcast */ > > > > s/MCE broadcast/MCE broadcast:/ > > > >> + if (c->x86 < 6) > >> + return; > >> + /* > >> + * All newer Zhaoxin and Centaur CPUs support MCE broadcasting. > >> Enable > >> + * synchronization with a one second timeout. > >> + */ > > > > > > Instead of copying and pasting the redundant comments, could you use > Dave's concise comments as suggested in: > > > > > > https://lore.kernel.org/all/a25f878e-83d9-440a-9741-4cf86db4a716@intel > > .com/ > > > > /* All newer ones do: */ > >> + if (c->x86 > 6) > >> + goto mce_broadcast; > >> + > > > > /* Family 6 is mixed: */ > >> + if (c->x86_vendor == X86_VENDOR_CENTAUR) { > >> + if (c->x86_model == 0xf && c->x86_stepping >= 0xe) > >> + goto mce_broadcast; > >> + } else if (c->x86_vendor == X86_VENDOR_ZHAOXIN) { > >> + if (c->x86_model == 0x19 || c->x86_model == 0x1f) > >> + goto mce_broadcast; > >> + } > >> + > >> + return; > >> [...] > > Thank you for your review. > Should I add the tag for this patch: > Reviewed-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com> Yes, you can, if the comments above are resolved. -Qiuxu
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index 1d7122a18..b908cdfb9 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -1171,6 +1171,14 @@ config X86_MCE_INTEL Additional support for intel specific MCE features such as the thermal monitor. +config X86_MCE_ZHAOXIN + def_bool y + prompt "Zhaoxin MCE features" + depends on X86_MCE_INTEL + help + Additional support for zhaoxin specific MCE features such as + the corrected machine check interrupt. + config X86_MCE_AMD def_bool y prompt "AMD MCE features" diff --git a/arch/x86/kernel/cpu/mce/Makefile b/arch/x86/kernel/cpu/mce/Makefile index 015856abd..2e863e78d 100644 --- a/arch/x86/kernel/cpu/mce/Makefile +++ b/arch/x86/kernel/cpu/mce/Makefile @@ -5,7 +5,7 @@ obj-$(CONFIG_X86_ANCIENT_MCE) += winchip.o p5.o obj-$(CONFIG_X86_MCE_INTEL) += intel.o obj-$(CONFIG_X86_MCE_AMD) += amd.o obj-$(CONFIG_X86_MCE_THRESHOLD) += threshold.o - +obj-$(CONFIG_X86_MCE_ZHAOXIN) += zhaoxin.o mce-inject-y := inject.o obj-$(CONFIG_X86_MCE_INJECT) += mce-inject.o diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c index 1654133da..37d4b6cd4 100644 --- a/arch/x86/kernel/cpu/mce/core.c +++ b/arch/x86/kernel/cpu/mce/core.c @@ -1963,17 +1963,6 @@ static int __mcheck_cpu_apply_quirks(struct cpuinfo_x86 *c) mce_flags.skx_repmov_quirk = 1; } - if (c->x86_vendor == X86_VENDOR_ZHAOXIN) { - /* - * All newer Zhaoxin CPUs support MCE broadcasting. Enable - * synchronization with a one second timeout. - */ - if (c->x86 > 6 || (c->x86_model == 0x19 || c->x86_model == 0x1f)) { - if (cfg->monarch_timeout < 0) - cfg->monarch_timeout = USEC_PER_SEC; - } - } - if (cfg->monarch_timeout < 0) cfg->monarch_timeout = 0; if (cfg->bootlog != 0) @@ -2016,49 +2005,6 @@ static void __mcheck_cpu_init_early(struct cpuinfo_x86 *c) } } -static void mce_centaur_feature_init(struct cpuinfo_x86 *c) -{ - struct mca_config *cfg = &mca_cfg; - - /* - * All newer Centaur CPUs support MCE broadcasting. Enable - * synchronization with a one second timeout. - */ - if ((c->x86 == 6 && c->x86_model == 0xf && c->x86_stepping >= 0xe) || - c->x86 > 6) { - if (cfg->monarch_timeout < 0) - cfg->monarch_timeout = USEC_PER_SEC; - } -} - -static void mce_zhaoxin_feature_init(struct cpuinfo_x86 *c) -{ - struct mce_bank *mce_banks = this_cpu_ptr(mce_banks_array); - - /* - * These CPUs have MCA bank 8 which reports only one error type called - * SVAD (System View Address Decoder). The reporting of that error is - * controlled by IA32_MC8.CTL.0. - * - * If enabled, prefetching on these CPUs will cause SVAD MCE when - * virtual machines start and result in a system panic. Always disable - * bank 8 SVAD error by default. - */ - if ((c->x86 == 7 && c->x86_model == 0x1b) || - (c->x86_model == 0x19 || c->x86_model == 0x1f)) { - if (this_cpu_read(mce_num_banks) > 8) - mce_banks[8].ctl = 0; - } - - intel_init_cmci(); - intel_init_lmce(); -} - -static void mce_zhaoxin_feature_clear(struct cpuinfo_x86 *c) -{ - intel_clear_lmce(); -} - static void __mcheck_cpu_init_vendor(struct cpuinfo_x86 *c) { switch (c->x86_vendor) { @@ -2076,9 +2022,6 @@ static void __mcheck_cpu_init_vendor(struct cpuinfo_x86 *c) break; case X86_VENDOR_CENTAUR: - mce_centaur_feature_init(c); - break; - case X86_VENDOR_ZHAOXIN: mce_zhaoxin_feature_init(c); break; diff --git a/arch/x86/kernel/cpu/mce/internal.h b/arch/x86/kernel/cpu/mce/internal.h index 01f8f0396..836e56027 100644 --- a/arch/x86/kernel/cpu/mce/internal.h +++ b/arch/x86/kernel/cpu/mce/internal.h @@ -334,4 +334,11 @@ static __always_inline u32 mca_msr_reg(int bank, enum mca_msr reg) } extern void (*mc_poll_banks)(void); +#ifdef CONFIG_X86_MCE_ZHAOXIN +void mce_zhaoxin_feature_init(struct cpuinfo_x86 *c); +void mce_zhaoxin_feature_clear(struct cpuinfo_x86 *c); +#else +static inline void mce_zhaoxin_feature_init(struct cpuinfo_x86 *c) { } +static inline void mce_zhaoxin_feature_clear(struct cpuinfo_x86 *c) { } +#endif #endif /* __X86_MCE_INTERNAL_H__ */ diff --git a/arch/x86/kernel/cpu/mce/zhaoxin.c b/arch/x86/kernel/cpu/mce/zhaoxin.c new file mode 100644 index 000000000..de69c560f --- /dev/null +++ b/arch/x86/kernel/cpu/mce/zhaoxin.c @@ -0,0 +1,65 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Zhaoxin specific MCE features + * Author: Lyle Li + */ +#include <asm/msr.h> +#include "internal.h" + +static void mce_zhaoxin_apply_mce_broadcast(struct cpuinfo_x86 *c) +{ + struct mca_config *cfg = &mca_cfg; + + /* Older CPUs do not do MCE broadcast */ + if (c->x86 < 6) + return; + /* + * All newer Zhaoxin and Centaur CPUs support MCE broadcasting. Enable + * synchronization with a one second timeout. + */ + if (c->x86 > 6) + goto mce_broadcast; + + if (c->x86_vendor == X86_VENDOR_CENTAUR) { + if (c->x86_model == 0xf && c->x86_stepping >= 0xe) + goto mce_broadcast; + } else if (c->x86_vendor == X86_VENDOR_ZHAOXIN) { + if (c->x86_model == 0x19 || c->x86_model == 0x1f) + goto mce_broadcast; + } + + return; + +mce_broadcast: + if (cfg->monarch_timeout <= 0) + cfg->monarch_timeout = USEC_PER_SEC; +} + +void mce_zhaoxin_feature_init(struct cpuinfo_x86 *c) +{ + struct mce_bank *mce_banks = this_cpu_ptr(mce_banks_array); + + /* + * These CPUs have MCA bank 8 which reports only one error type called + * SVAD (System View Address Decoder). The reporting of that error is + * controlled by IA32_MC8.CTL.0. + * + * If enabled, prefetching on these CPUs will cause SVAD MCE when + * virtual machines start and result in a system panic. Always disable + * bank 8 SVAD error by default. + */ + if ((c->x86 == 7 && c->x86_model == 0x1b) || + (c->x86_model == 0x19 || c->x86_model == 0x1f)) { + if (this_cpu_read(mce_num_banks) > 8) + mce_banks[8].ctl = 0; + } + + mce_zhaoxin_apply_mce_broadcast(c); + intel_init_cmci(); + intel_init_lmce(); +} + +void mce_zhaoxin_feature_clear(struct cpuinfo_x86 *c) +{ + intel_clear_lmce(); +}