| Message ID | e8d11f3ce0f64a9f9a4cefcc8059747b@zhaoxin.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/4] x86/mce: Add Zhaoxin MCE support | expand |
On Tue, Sep 10, 2019 at 08:19:44AM +0000, Tony W Wang-oc wrote: > @@ -1777,6 +1777,29 @@ static void mce_centaur_feature_init(struct cpuinfo_x86 *c) > } > } > > +#ifdef CONFIG_CPU_SUP_ZHAOXIN What's that ifdeffery for since you have it in the header already? > +void mce_zhaoxin_feature_init(struct cpuinfo_x86 *c) > +{ > + struct mce_bank *mce_banks = this_cpu_ptr(mce_banks_array); > + > + /* > + * These CPUs bank8 SVAD error may be triggered unexpected when "These CPUs bank8 SVAD"?? > + * bringup virtual machine. it is not hardware bug. Always disable > + * bank8 SVAD error by default. > + */ That comment is incomprehensible. Please rewrite. > + if ((c->x86 == 6 && c->x86_model == 0x19 && > + (c->x86_stepping > 3 && c->x86_stepping < 8)) || > + (c->x86 == 6 && c->x86_model == 0x1f) || > + (c->x86 == 7 && c->x86_model == 0x1b)) { As before: potential to simplify the test here? > + if (this_cpu_read(mce_num_banks) > 8) > + mce_banks[8].ctl = 0; > + } > + > + intel_init_cmci(); > + mce_adjust_timer = cmci_intel_adjust_timer; > +} > +#endif > + > static void __mcheck_cpu_init_vendor(struct cpuinfo_x86 *c) > { > switch (c->x86_vendor) { > @@ -1798,6 +1821,10 @@ static void __mcheck_cpu_init_vendor(struct cpuinfo_x86 *c) > mce_centaur_feature_init(c); > break; > > + case X86_VENDOR_ZHAOXIN: > + mce_zhaoxin_feature_init(c); > + break; > + > default: > break; > } > diff --git a/arch/x86/kernel/cpu/mce/intel.c b/arch/x86/kernel/cpu/mce/intel.c > index eee4b12..b49cba7 100644 > --- a/arch/x86/kernel/cpu/mce/intel.c > +++ b/arch/x86/kernel/cpu/mce/intel.c > @@ -85,7 +85,8 @@ static int cmci_supported(int *banks) > * initialization is vendor keyed and this > * makes sure none of the backdoors are entered otherwise. > */ That comment above needs fixing too. > - if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL) > + if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL && > + boot_cpu_data.x86_vendor != X86_VENDOR_ZHAOXIN) > return 0; <---- newline here. > if (!boot_cpu_has(X86_FEATURE_APIC) || lapic_get_maxlvt() < 6) > return 0; > -- > 2.7.4
On Tue, Sep 10, 2019, Borislav Petkov wrote: >On Tue, Sep 10, 2019 at 08:19:44AM +0000, Tony W Wang-oc wrote: >> @@ -1777,6 +1777,29 @@ static void mce_centaur_feature_init(struct >cpuinfo_x86 *c) >> } >> } >> >> +#ifdef CONFIG_CPU_SUP_ZHAOXIN > >What's that ifdeffery for since you have it in the header already? Sorry for that. Since this function actually be called only in mce/core.c, will remove the declare in the header file and make this function static. > >> +void mce_zhaoxin_feature_init(struct cpuinfo_x86 *c) >> +{ >> + struct mce_bank *mce_banks = this_cpu_ptr(mce_banks_array); >> + >> + /* >> + * These CPUs bank8 SVAD error may be triggered unexpected when > >"These CPUs bank8 SVAD"?? These Zhaoxin CPUs have MCA bank 8, that only have one error called SVAD( System View Address Decoder) which be controlled by IA32_MC8_CTL.0 > >> + * bringup virtual machine. it is not hardware bug. Always disable >> + * bank8 SVAD error by default. >> + */ > >That comment is incomprehensible. Please rewrite. Ok, will rewrite comment in v3. > >> + if ((c->x86 == 6 && c->x86_model == 0x19 && >> + (c->x86_stepping > 3 && c->x86_stepping < 8)) || >> + (c->x86 == 6 && c->x86_model == 0x1f) || >> + (c->x86 == 7 && c->x86_model == 0x1b)) { > >As before: potential to simplify the test here? Ok, will simplify as before. > >> + if (this_cpu_read(mce_num_banks) > 8) >> + mce_banks[8].ctl = 0; >> + } >> + >> + intel_init_cmci(); >> + mce_adjust_timer = cmci_intel_adjust_timer; >> +} >> +#endif >> + >> static void __mcheck_cpu_init_vendor(struct cpuinfo_x86 *c) >> { >> switch (c->x86_vendor) { >> @@ -1798,6 +1821,10 @@ static void __mcheck_cpu_init_vendor(struct >cpuinfo_x86 *c) >> mce_centaur_feature_init(c); >> break; >> >> + case X86_VENDOR_ZHAOXIN: >> + mce_zhaoxin_feature_init(c); >> + break; >> + >> default: >> break; >> } >> diff --git a/arch/x86/kernel/cpu/mce/intel.c b/arch/x86/kernel/cpu/mce/intel.c >> index eee4b12..b49cba7 100644 >> --- a/arch/x86/kernel/cpu/mce/intel.c >> +++ b/arch/x86/kernel/cpu/mce/intel.c >> @@ -85,7 +85,8 @@ static int cmci_supported(int *banks) >> * initialization is vendor keyed and this >> * makes sure none of the backdoors are entered otherwise. >> */ > >That comment above needs fixing too. Ok. > >> - if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL) >> + if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL && >> + boot_cpu_data.x86_vendor != X86_VENDOR_ZHAOXIN) >> return 0; > ><---- newline here. Ok. Sincerely TonyWWang-oc
diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h index dc2d4b2..0986a11 100644 --- a/arch/x86/include/asm/mce.h +++ b/arch/x86/include/asm/mce.h @@ -350,4 +350,10 @@ umc_normaddr_to_sysaddr(u64 norm_addr, u16 nid, u8 umc, u64 *sys_addr) { return static inline void mce_hygon_feature_init(struct cpuinfo_x86 *c) { return mce_amd_feature_init(c); } +#ifdef CONFIG_CPU_SUP_ZHAOXIN +void mce_zhaoxin_feature_init(struct cpuinfo_x86 *c); +#else +static inline void mce_zhaoxin_feature_init(struct cpuinfo_x86 *c) { } +#endif + #endif /* _ASM_X86_MCE_H */ diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c index 3f878f6..8a36833 100644 --- a/arch/x86/kernel/cpu/mce/core.c +++ b/arch/x86/kernel/cpu/mce/core.c @@ -1777,6 +1777,29 @@ static void mce_centaur_feature_init(struct cpuinfo_x86 *c) } } +#ifdef CONFIG_CPU_SUP_ZHAOXIN +void mce_zhaoxin_feature_init(struct cpuinfo_x86 *c) +{ + struct mce_bank *mce_banks = this_cpu_ptr(mce_banks_array); + + /* + * These CPUs bank8 SVAD error may be triggered unexpected when + * bringup virtual machine. it is not hardware bug. Always disable + * bank8 SVAD error by default. + */ + if ((c->x86 == 6 && c->x86_model == 0x19 && + (c->x86_stepping > 3 && c->x86_stepping < 8)) || + (c->x86 == 6 && c->x86_model == 0x1f) || + (c->x86 == 7 && c->x86_model == 0x1b)) { + if (this_cpu_read(mce_num_banks) > 8) + mce_banks[8].ctl = 0; + } + + intel_init_cmci(); + mce_adjust_timer = cmci_intel_adjust_timer; +} +#endif + static void __mcheck_cpu_init_vendor(struct cpuinfo_x86 *c) { switch (c->x86_vendor) { @@ -1798,6 +1821,10 @@ static void __mcheck_cpu_init_vendor(struct cpuinfo_x86 *c) mce_centaur_feature_init(c); break; + case X86_VENDOR_ZHAOXIN: + mce_zhaoxin_feature_init(c); + break; + default: break; } diff --git a/arch/x86/kernel/cpu/mce/intel.c b/arch/x86/kernel/cpu/mce/intel.c index eee4b12..b49cba7 100644 --- a/arch/x86/kernel/cpu/mce/intel.c +++ b/arch/x86/kernel/cpu/mce/intel.c @@ -85,7 +85,8 @@ static int cmci_supported(int *banks) * initialization is vendor keyed and this * makes sure none of the backdoors are entered otherwise. */ - if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL) + if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL && + boot_cpu_data.x86_vendor != X86_VENDOR_ZHAOXIN) return 0; if (!boot_cpu_has(X86_FEATURE_APIC) || lapic_get_maxlvt() < 6) return 0;
All Zhaoxin newer CPUs support CMCI that compatible with Intel's "Machine-Check Architecture", so add support for Zhaoxin CMCI in mce/core.c and mce/intel.c. Signed-off-by: Tony W Wang-oc <TonyWWang-oc@zhaoxin.com> --- v1->v2: - Fix redefinition of "mce_zhaoxin_feature_init" arch/x86/include/asm/mce.h | 6 ++++++ arch/x86/kernel/cpu/mce/core.c | 27 +++++++++++++++++++++++++++ arch/x86/kernel/cpu/mce/intel.c | 3 ++- 3 files changed, 35 insertions(+), 1 deletion(-)