Re: [V3][PATCH 3/6] x86, nmi: wire up NMI handlers to new routines

[Corey Minyard <minyard@acm.org>]

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