Re: [PATCH -v2 6/7] x86, NMI, Add support to notify hardware error with unknown NMI

[Huang Ying <ying.huang@intel.com>]

Hi, Don,

On Thu, 2010-09-30 at 12:36 +0800, Don Zickus wrote:
> On Wed, Sep 29, 2010 at 04:17:30PM +0800, huang ying wrote:
> > On Tue, Sep 28, 2010 at 11:32 PM, Don Zickus <dzickus@redhat.com> wrote:
> > 
> > > But the problem is you have to export all this platform specific stuff to
> > > traps.c and surround the code with #ifdef's, which start to look ugly.
> > 
> > There is no #ifdef in my final default_do_nmi(), so I think the code
> > can be cleaned up without converting everything into notifier block. I
> > think the rule can be: architecture specific thing should go direct
> > call, while device driver should be turned into notifier block.
> 
> That sounds like a good rule, but then my definition of architecture
> specific is whatever is written in the intel/amd x86_64 architecture
> manual (that sits on my desk, dated 2002), which wouldn't include any
> of the error handling you propose, nor MCE, nor perf.

MCE is at least in new version of Intel SDM vol 3A. But we do not need
to process MCE in NMI handler. Performance monitor counter is in Intel
SDM Vol 3B.

> I guess I look at all that stuff as cpu features because not all the cpus
> on the market have them.  Shouldn't traps.c just contain core architecture
> stuff and all those hardware error features could go under
> arch/x86/kernel/cpu with the rest of the features, no?

Yes. Both MCE and perf are CPU features. I think they can be thought as
optional architectural features. I think it is good to put similar
features into arch/x86/kernel/cpu instead of traps.c. But if necessary,
we can put direct call in traps.c instead of notifier block.

> > > Is there any reason why traps.c should know about MCA/HEST/<other hardware
> > > errors>?  Shouldn't it be abstracted away?
> > 
> > Yes. The device drivers should be abstracted away, leaving
> > architectural logic, such as port 0x61 as direct call. We need
> > notifier chain, but I just suggest reduce its usage if possible.
> > 
> > > Honestly, I would be interested in creating a southbridge driver and
> > > moving the port 0x61 code there and keeping the default_do_nmi() function
> > > stupidly simple (just a call to the die_chain and the
> > > unknown_nmi_error()).
> > 
> > I think the southbridge drivers should go notifier block, but the port
> > 0x61 code is architectural and should be kept in default_do_nmi().
> 
> Is port 0x61 architectural?  I thought it a southbridge thing.  In fact I
> thought with modern chipsets you can access the same thing through port
> 0x70 or 0x71 (I can't seem to figure out which Intel doc I saw that in).
> (Not that this conversation has any bearing on your patchset, just an idea
> I had).

At least until now, I think port 0x61 can be considered architectural
(just like we think PIT is architectural before). Maybe in the future it
will become deprecated and turned into something like a device driver.
But why not wait until it be.

Best Regards,
Huang Ying