Re: [tip:x86/mce] x86, mce: Rename cpu_specific_poll to mce_cpu_specific_poll

[Borislav Petkov <petkovbb@googlemail.com>]

(Adding some more interested parties to Cc:)

On Sat, Jan 23, 2010 at 06:17:17AM +0100, Ingo Molnar wrote:
> 
> * tip-bot for H. Peter Anvin <hpa@zytor.com> wrote:
> 
> > Commit-ID:  f91c4d2649531cc36e10c6bc0f92d0f99116b209
> > Gitweb:     http://git.kernel.org/tip/f91c4d2649531cc36e10c6bc0f92d0f99116b209
> > Author:     H. Peter Anvin <hpa@zytor.com>
> > AuthorDate: Thu, 21 Jan 2010 18:31:54 -0800
> > Committer:  H. Peter Anvin <hpa@zytor.com>
> > CommitDate: Thu, 21 Jan 2010 18:31:54 -0800
> > 
> > x86, mce: Rename cpu_specific_poll to mce_cpu_specific_poll
> > 
> > cpu_specific_poll is a global variable, and it should have a global
> > namespace name.  Since it is MCE-specific (it takes a struct mce *),
> > rename it mce_cpu_specific_poll.
> > 
> > Signed-off-by: H. Peter Anvin <hpa@zytor.com>
> > Cc: Andi Kleen <andi@firstfloor.org>
> > LKML-Reference: <20100121221711.GA8242@basil.fritz.box>
> 
> FYI, this commit triggered a -tip test failure:
> 
>  arch/x86/kernel/cpu/mcheck/mce-xeon75xx.c: In function 'xeon75xx_mce_init':
>  arch/x86/kernel/cpu/mcheck/mce-xeon75xx.c:340: error: implicit declaration of function 'pci_match_id'
> 
> I'm excluding it from tip:master.
> 
> But the bigger problem with this commit is structure of it - or the lack 
> thereof.
> 
> It just blindly goes into the direction the MCE code has been going for some 
> time, minimally enabling the hardware, ignoring both the new EDAC design and 
> the new performance monitoring related design i outlined some time ago.

I completely agree - from what I see this is adding vendor- or rather
vendor-and-machine-specific hooks to read out (1) the position of the
the memory translation table from PCI config space (0x8c), (2) then to
read out the offset from the first MCA status register in order to (3)
rdmsr the status information.

In AMD's case, we need similar hooks too, in order to evaluate
correctable MCEs for different RAS reasons like for example L3 cache or
data arrays errors for disabling L3 indices. I was looking into adding
hooks into machine_check_poll() and cpu_specific_poll() interface could
work.

Furthermore, lets leave mcheck be mcheck and do error decoding in EDAC
modules. For example, there was a core i7 EDAC module submission from
Mauro and the Xeon75xx-specific decoding bits could be added to it or
even as a new machine-specific module instead of mcelog.

With the evergrowing complexity of memory controller design I don't
think that the userspace mcelog approach will scale - you need the
whole decoding in the kernel where the module knows the exact memory
controllers setup and which DRAM addresses belong to which nodes and
whether you do memory hoisting and whether you interleave, if yes, how
and on what granilarity you interleave and on and on...

I believe Ingo also had some ideas about perf_event integration and this
is something we could add to the MCE polling routine too. Ingo?

-- 
Regards/Gruss,
    Boris.