Messages in this thread | | | From | Tony W Wang-oc <> | Subject | 答复: [PATCH v2 3/4] x86/mce: Add Zhaoxin C MCI support | Date | Wed, 11 Sep 2019 10:12:42 +0000 |
| |
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
| |