lkml.org 
[lkml]   [2018]   [Jul]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH v2] x86/mce: add CMCI support for Centaur CPUs
Date


> -----Original Mail-----
> Sender: Borislav Petkov [mailto:bp@alien8.de]
> Time: 2018年6月26日 22:30
> Receiver: David Wang <davidwang@zhaoxin.com>
> CC: tony.luck@intel.com; mingo@redhat.com; tglx@linutronix.de;
> hpa@zytor.com; x86@kernel.org; linux-kernel@vger.kernel.org;
> linux-edac@vger.kernel.org; cooperyan@zhaoxin.com;
> qiyuanwang@zhaoxin.com; benjaminpan@viatech.com;
> lukelin@viacpu.com; timguo@zhaoxin.com
> Topic : Re: [PATCH v2] x86/mce: add CMCI support for Centaur CPUs
>
> On Mon, Jun 04, 2018 at 10:37:33AM +0800, David Wang wrote:
> > New Centaur CPU support CMCI mechanism, which is compatible with
> INTEL CMCI.
> >
> > Signed-off-by: David Wang <davidwang@zhaoxin.com>
>
> ...
>
> > diff --git a/arch/x86/kernel/cpu/mcheck/mce.c
> > b/arch/x86/kernel/cpu/mcheck/mce.c
> > index cd76380..2ebafc7 100644
> > --- a/arch/x86/kernel/cpu/mcheck/mce.c
> > +++ b/arch/x86/kernel/cpu/mcheck/mce.c
> > @@ -1727,6 +1727,7 @@ static void __mcheck_cpu_init_early(struct
> cpuinfo_x86 *c)
> > }
> > }
> >
> > +#ifdef CONFIG_X86_MCE_CENTAUR
> > static void mce_centaur_feature_init(struct cpuinfo_x86 *c) {
> > struct mca_config *cfg = &mca_cfg;
> > @@ -1740,7 +1741,12 @@ static void mce_centaur_feature_init(struct
> cpuinfo_x86 *c)
> > if (cfg->monarch_timeout < 0)
> > cfg->monarch_timeout = USEC_PER_SEC;
> > }
> > + mce_intel_feature_init(c);
> > + mce_adjust_timer = cmci_intel_adjust_timer;
>
> This ...
>
> > }
> > +#else
> > +static inline void mce_centaur_feature_init(struct cpuinfo_x86 *c) {
> > +} #endif
> >
> > static void __mcheck_cpu_init_vendor(struct cpuinfo_x86 *c) { diff
> > --git a/arch/x86/kernel/cpu/mcheck/mce_intel.c
> > b/arch/x86/kernel/cpu/mcheck/mce_intel.c
> > index d05be30..5b1b68f 100644
> > --- a/arch/x86/kernel/cpu/mcheck/mce_intel.c
> > +++ b/arch/x86/kernel/cpu/mcheck/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_CENTAUR))
> > return 0;
> > if (!boot_cpu_has(X86_FEATURE_APIC) || lapic_get_maxlvt() < 6)
> > return 0;
> > @@ -506,10 +507,20 @@ static void intel_ppin_init(struct cpuinfo_x86
> > *c)
> >
> > void mce_intel_feature_init(struct cpuinfo_x86 *c) {
> > - intel_init_thermal(c);
> > - intel_init_cmci();
> > - intel_init_lmce();
> > - intel_ppin_init(c);
> > +
> > + switch (c->x86_vendor) {
> > + case X86_VENDOR_INTEL:
> > + intel_init_thermal(c);
> > + intel_init_cmci();
> > + intel_init_lmce();
> > + intel_ppin_init(c);
> > + break;
> > + case X86_VENDOR_CENTAUR:
> > + intel_init_cmci();
>
> ... and this I really don't like for the simple reason that if the Intel side gets
> changed, it could potentially break Centaur. And we don't want that. And
> the vendor should be free to change their code without asking another
> vendor for permission even if the other vendor is almost copying the
> code...
>
> Long story short, I think you should extract the facilities you're going to
> need into generic, library-like ones and call them from centaur-specific
> compilation units which get enabled when CPU_SUP_CENTAUR is enabled.
>
> So that the code can still be shared but there's no dependency on other
> vendors and so that one vendor doesn't break the other one and
> vice-versa.
>
> IMO.
>
> Thx.
>
OK. I will adjust code.
Thank you.
> --
> Regards/Gruss,
> Boris.
>
> Good mailing practices for 400: avoid top-posting and trim the reply.

\
 
 \ /
  Last update: 2018-07-04 04:46    [W:0.042 / U:3.260 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site