Re: [PATCH] perf/x86: check ucode before disabling PEBS on SandyBridge

[Henrique de Moraes Holschuh <hmh@hmh.eng.br>]

On Wed, 13 Jun 2012, Borislav Petkov wrote:
> On Tue, Jun 12, 2012 at 10:04:13PM -0300, Henrique de Moraes Holschuh wrote:
> > On Tue, 12 Jun 2012, Borislav Petkov wrote:
> > > I'd guess this is still there to support mixed ucode revisions for some
> > 
> > It is still there because of the stable ABI rules.  As far as I can tell,
> > it was a crap interface when it got in, and it still is a crap interface
> > now because there simply isn't any sane usercase that requires it, it is
> > dangerous as implemented right now (at least in the Intel case), and even
> > if we fixed the kernel to do the right thing, userspace would not be able
> > to know that and would still need to request 1 microcode refresh per core.
> 
> Well, I'm disabling it on AMD.
> 
> We could make it iterate over _all_ cores and do the update on each one
> of them even though the user does a sysfs write only for a single core..

Yes, please.  I suggest we use core 0 for that.  Using my Debian
maintainer hat, I'd rather you got rid of the sysfs entries for every
other core while at it, as it will make our life a lot simpler,
distro-side.

This is still not the proper fix, which would be to add a new sysfs node
to access the proper update-every-core functionality, but it is a damn
good start in the right direction and required to make it safe without
ripping out the old ABI entirely without a deprecation period.

Since Intel processors don't want the per-core behaviour either, you
could fix it in the microcode core itself...

> > Perhaps we could make it work only for CPU 0 and return EBUSY or whatever
> > for all others (or just don't publish the sysfs attribute for the others),
> > and change the CPU0 refresh firmware request into a refresh all cores call.
> 
> That could also work, it sounds like a sane thing to do having in mind
> the current sysfs layout. I will cook up something soonish.
> 
> > We could then add a new sysfs attribute to cleanly do the
> > update-all-cores request.
> 
> That's what Fenghua is doing.

Ok.  Please CC me in the patches, if the new ABI arrives in time, I'll
even be able to get it supported on the next Debian stable (and push to
get this stuff backported to the kernel 3.2 which we will ship, I
consider this an important bug-fix to a pontentially very serious issue.
We have *zero* chance of finding out what's wrong if an users' system
start getting subtly crazy because it is running with skewed microcode
among cores.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh