Re: [V4][PATCH 4/6] x86, nmi: add in logic to handle multiple events and unknown NMIs

[Robert Richter <robert.richter@amd.com>]

On 14.09.11 13:58:09, Don Zickus wrote:
> On Wed, Sep 14, 2011 at 06:26:53PM +0200, Robert Richter wrote:
> > On 13.09.11 16:58:27, Don Zickus wrote:
> > > @@ -87,6 +87,16 @@ static int notrace __kprobes nmi_handle(unsigned int type, struct pt_regs *regs)
> > >  
> > >  		handled += a->handler(type, regs);
> > >  
> > > +		/*
> > > + 		 * Optimization: only loop once if this is not a 
> > > + 		 * back-to-back NMI.  The idea is nothing is dropped
> > > + 		 * on the first NMI, only on the second of a back-to-back
> > > + 		 * NMI.  No need to waste cycles going through all the
> > > + 		 * handlers.
> > > + 		 */
> > > +		if (!b2b && handled)
> > > +			break;
> > 
> > Don, if I am not missing something, this actually does not work
> > because perfctr NMIs do not re-trigger. Suppose a handler running
> > before perfctr. It sets 'handled' and the chain is stopped here. To
> > run through the perfctr handler the NMI must retrigger which it
> > doesn't.
> 
> Your patch is incorrect.  Your dummy handler does not handle a _real_ NMI.
> Which means no _real_ NMI was ever generated.  Of course perf won't work.
> You just swallowed its NMI.
> 
> The change I made is for nmi handlers that actually have an NMI associated
> with them.  The idea is if somebody generated an NMI, it will get handled
> by a handler.  If perf comes along and generates another NMI, it should
> get latched.  Upon handling the first NMI, the perf NMI should be sitting
> queued up and cause the back-to-back NMI. In this case all the handlers
> will be executed (to handle dropped NMIs).

Yes, your thought about the latched NMI could work. Though I better
test this with some real nmis from different sources.  Unfortunately
this is much harder to trigger. Will give it a try. It would be a
pretty nice optimization then.

> My only question to you is the IBS stuff you were working on.  Does that
> generate a _real_ NMI or does it just piggy back off of the perf NMI?

Yes, IBS generates real NMIs, there is an own interrupt vector for
it.

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center