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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 14 Sep 2011 22:16:12 +0200
From:	Robert Richter <robert.richter@....com>
To:	Don Zickus <dzickus@...hat.com>
CC:	"x86@...nel.org" <x86@...nel.org>,
	Andi Kleen <andi@...stfloor.org>,
	Peter Zijlstra <peterz@...radead.org>,
	"ying.huang@...el.com" <ying.huang@...el.com>,
	LKML <linux-kernel@...r.kernel.org>,
	"paulmck@...ux.vnet.ibm.com" <paulmck@...ux.vnet.ibm.com>,
	"avi@...hat.com" <avi@...hat.com>,
	"jeremy@...p.org" <jeremy@...p.org>
Subject: Re: [V4][PATCH 4/6] x86, nmi:  add in logic to handle multiple
 events and unknown NMIs

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/