lkml.org 
[lkml]   [2015]   [Sep]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [Patch V0] x86, mce: Don't clear global error reporting banks during cpu_offline
Hi Boris

On Fri, Sep 04, 2015 at 01:50:29PM +0200, Borislav Petkov wrote:
> > Intel Secure Guard eXtentions will be disabled when these controls are cleared
> > from a security perspective. This patch enables SGX to work across
> > suspend/resume.
>
> What does that mean? What does SGX have to do with MCI_CTL registers?
> Explain that in the commit message so that !Intel people can understand.

I will add something more in the commit log and resubmit. Tony had the right
explanation in his response..


> For the whole patch text do:
>
> s/cpu/CPU/
>
> s/CPU's/CPUs/ and s/MSR's/MSRs/ if you mean plural. Also spellcheck all text.

Will do.

> > +static void _vendor_disable_error_reporting(void)
>
> Why the "_" prepended here?

Hummm.. just tried to keep the style as other parts of mcheck_cpu_init()
where we have _mcheck_cpu_init_vendor(). Its not a rule, but sometimes
local static functions have "__" varient.

Maybe it should have been the double "__"?

>
> > +{
> > + struct cpuinfo_x86 *c = &boot_cpu_data;
> > +
> > + switch (c->x86_vendor) {
> > + case X86_VENDOR_INTEL:
> > + /*
> > + * Don't clear on Intel CPU's. Some of these MSR's are
> > + * socket wide. Disabling them for just a single cpu offline
> > + * is bad, since it will inhibit reporting for all shared
> > + * resources.. such as LLC, iMC for e.g.
> > + */
> > + break;
> > + default:
> > + /*
> > + * Disble MCE reporting for all other CPU Vendor.
> > + * Don't want to break functionality on those
> > + */
> > + mce_disable_error_reporting();
> > + }
>
> I think the switch-case makes this unnecessarily bloated as code. Just
> do:


Makes sense... i will switch it as suggested.

>
> if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL)
> return;
>
> mce_disable_error_reporting();
>
> ...
>

Cheers,
Ashok



\
 
 \ /
  Last update: 2015-09-04 19:21    [W:0.117 / U:1.016 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site