[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 13/19] x86/mce_intel: detect and enable LMCE on Intel host
On 02/22/17 08:10 -0700, Jan Beulich wrote: > >>> On 17.02.17 at 07:39, <haozhong.zhang@xxxxxxxxx> wrote: > > --- a/xen/arch/x86/cpu/mcheck/mce.h > > +++ b/xen/arch/x86/cpu/mcheck/mce.h > > @@ -38,6 +38,7 @@ enum mcheck_type { > > }; > > > > extern uint8_t cmci_apic_vector; > > +extern bool lmce_support; > > > > /* Init functions */ > > enum mcheck_type amd_mcheck_init(struct cpuinfo_x86 *c); > > diff --git a/xen/arch/x86/cpu/mcheck/mce_intel.c > > b/xen/arch/x86/cpu/mcheck/mce_intel.c > > index 9e5ee3d..b4cc41a 100644 > > --- a/xen/arch/x86/cpu/mcheck/mce_intel.c > > +++ b/xen/arch/x86/cpu/mcheck/mce_intel.c > > @@ -29,6 +29,9 @@ boolean_param("mce_fb", mce_force_broadcast); > > > > static int __read_mostly nr_intel_ext_msrs; > > > > +/* If mce_force_broadcast == 1, lmce_support will be disabled forcibly. */ > > +bool __read_mostly lmce_support = 0; > > false (but really there's no need for an initializer here) > > > @@ -677,10 +680,34 @@ static int mce_is_broadcast(struct cpuinfo_x86 *c) > > return 0; > > } > > > > +static bool intel_enable_lmce(void) > > +{ > > + uint64_t msr_content; > > + > > + /* > > + * Section "Enabling Local Machine Check" in Intel SDM Vol 3 > > + * requires software must ensure the LOCK bit and LMCE_ON bit > > + * of MSR_IA32_FEATURE_CONTROL are set before setting > > + * MSR_IA32_MCG_EXT_CTL.LMCE_EN. > > + */ > > + > > + if ( rdmsr_safe(MSR_IA32_FEATURE_CONTROL, msr_content) ) > > + return 0; > > false (and so on further down) I'll fix these boolean stuffs here and in other patches. > > > + if ( msr_content & > > + (IA32_FEATURE_CONTROL_LOCK | IA32_FEATURE_CONTROL_LMCE_ON) ) > > This checks whether at least one of the bits is on, which isn't in > line with the comment. > I'll fix this check. > > static void intel_init_mca(struct cpuinfo_x86 *c) > > { > > - bool_t broadcast, cmci = 0, ser = 0; > > + bool_t broadcast, cmci = 0, ser = 0, lmce = 0; > > Please use the opportunity to change to plain bool (and false). sure > > > @@ -700,26 +727,31 @@ static void intel_init_mca(struct cpuinfo_x86 *c) > > > > first = mce_firstbank(c); > > > > + if ( !mce_force_broadcast && (msr_content & MCG_LMCE_P) ) > > Please make all your additions match the prevailing coding style in > this file (which admittedly is neither ours nor Linux'es, but a mix). The problem is the existing style in this file is not consistent. Both if ( cond ) and if (cond) are being used in this file. I chose to use Xen style in the new code. > > > + lmce = intel_enable_lmce(); > > + > > if (smp_processor_id() == 0) > > { > > dprintk(XENLOG_INFO, "MCA Capability: BCAST %x SER %x" > > - " CMCI %x firstbank %x extended MCE MSR %x\n", > > - broadcast, ser, cmci, first, ext_num); > > + " CMCI %x firstbank %x extended MCE MSR %x LMCE %x\n", > > + broadcast, ser, cmci, first, ext_num, lmce); > > Please can you switch over to not printing booleans as numbers > here, but simply omitting the respective string from the output if > a feature is not there? Only actual numbers should be printed as > such. sure Thanks, Haozhong _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |