Messages in this thread | | | Date | Tue, 06 Sep 2011 12:59:51 -0500 | From | Corey Minyard <> | Subject | Re: [V3][PATCH 3/6] x86, nmi: wire up NMI handlers to new routines |
| |
On 09/06/2011 12:49 PM, Don Zickus wrote: > On Tue, Sep 06, 2011 at 12:20:19PM -0500, Corey Minyard wrote: >>> +ipmi_nmi(unsigned int val, struct pt_regs *regs) >>> { >>> - struct die_args *args = data; >>> - >>> - if (val != DIE_NMIUNKNOWN) >>> - return NOTIFY_OK; >>> - >>> - /* Hack, if it's a memory or I/O error, ignore it. */ >>> - if (args->err& 0xc0) >>> - return NOTIFY_OK; >>> - >>> /* >>> * If we get here, it's an NMI that's not a memory or I/O >>> * error. We can't truly tell if it's from IPMI or not >> From the comment above, this will not actually work for the IPMI >> watchdog. The only way to tell if an NMI is from the IPMI >> controller is to send a message to the IPMI controller and wait for >> the response. Not an option in an NMI handler. So the IPMI >> watchdog driver relied on it being last, and if nothing else handled >> the NMI, then it must have been from the IPMI controller. >> >> This is stupid, I know, but that's the way it works in IPMI. If we >> decide to do what you are suggesting we will have to disable this >> function in the IPMI watchdog driver or we need some way to say "if >> nothing else handled the NMI, call this." > Hi Corey, > > Thanks for the review. > > I know and you aren't the only driver that has an issue like this, hpwdt > is in the same boat. This is why I registered the driver against > NMI_UNKNOWN instead of NMI_LOCAL. The ipmi driver should only be called > when no one else claims the NMI (including a possible external one). > > The only drivers I know that have to do it this way are hpwdt, sgi and > ipmi. By design, none of those drivers should be loaded at the same time > (as they are all uniquely hardware specific). > > BTW, this is also why I removed the 'Hack, if it's a memory or I/O error', > because the ipmi handler should not be called for an external NMI now. > > I hope that clears things up. Ok, that should work. I didn't read through the whole set of patches and I missed that. This is good for me. Thanks.
-corey
| |