> -----Original Message----- > From: [email protected] <[email protected]> On > Behalf Of Borislav Petkov > Sent: Friday, May 17, 2019 2:35 PM > To: Luck, Tony <[email protected]> > Cc: Ghannam, Yazen <[email protected]>; [email protected]; > [email protected]; [email protected] > Subject: Re: [PATCH v3 5/6] x86/MCE: Save MCA control bits that get set in > hardware > > > On Fri, May 17, 2019 at 11:06:07AM -0700, Luck, Tony wrote: > > and thus end up with that extra level on indent for the rest > > of the function. > > Ok: > > --- > diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c > index 5bcecadcf4d9..25e501a853cd 100644 > --- a/arch/x86/kernel/cpu/mce/core.c > +++ b/arch/x86/kernel/cpu/mce/core.c > @@ -1493,6 +1493,11 @@ static int __mcheck_cpu_mce_banks_init(void) > for (i = 0; i < n_banks; i++) { > struct mce_bank *b = &mce_banks[i]; > > + /* > + * Init them all, __mcheck_cpu_apply_quirks() is going to > apply > + * the required vendor quirks before > + * __mcheck_cpu_init_clear_banks() does the final bank setup. > + */ > b->ctl = -1ULL; > b->init = 1; > } > @@ -1562,6 +1567,7 @@ static void __mcheck_cpu_init_generic(void) > static void __mcheck_cpu_init_clear_banks(void) > { > struct mce_bank *mce_banks = this_cpu_read(mce_banks_array); > + u64 msrval; > int i; > > for (i = 0; i < this_cpu_read(mce_num_banks); i++) { > @@ -1569,7 +1575,13 @@ static void __mcheck_cpu_init_clear_banks(void) > > if (!b->init) > continue; > + > + /* Check if any bits are implemented in h/w */ > wrmsrl(msr_ops.ctl(i), b->ctl); > + rdmsrl(msr_ops.ctl(i), msrval); > + > + b->init = !!msrval; > +
Just a minor nit, but can we group the comment, RDMSR, and check together? The WRMSR is part of normal operation and isn't tied to the check. Thanks, Yazen

