From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marcelo Tosatti <mtosatti@redhat.com>
Subject: Re: [RFC] KVM: Fix simultaneous NMIs
Date: Mon, 19 Sep 2011 11:54:03 -0300
Message-ID: <20110919145403.GB6356@amt.cnet>
References: <1316097911-16424-1-git-send-email-avi@redhat.com>
 <4E722140.4070702@siemens.com>
 <4E722FA8.2030006@redhat.com>
 <4E7234F0.2080609@siemens.com>
 <4E723A8A.7050405@redhat.com>
 <20110919135421.GA5468@amt.cnet>
 <4E775203.2020504@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: Jan Kiszka <jan.kiszka@siemens.com>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>
To: Avi Kivity <avi@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:39218 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1751236Ab1ISOy1 (ORCPT <rfc822;kvm@vger.kernel.org>);
	Mon, 19 Sep 2011 10:54:27 -0400
Content-Disposition: inline
In-Reply-To: <4E775203.2020504@redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Mon, Sep 19, 2011 at 05:30:27PM +0300, Avi Kivity wrote:
> On 09/19/2011 04:54 PM, Marcelo Tosatti wrote:
> >>  >>
> >>  >>   Yes, due to NMI-blocked-by-STI.  A really touchy area.
> >>  >And we don't need the window exit notification then? I don't understand
> >>  >what nmi_in_progress is supposed to do here.
> >>
> >>  We need the window notification in both cases.  If we're recovering
> >>  from STI, then we don't need to collapse NMIs.  If we're completing
> >>  an NMI handler, then we do need to collapse NMIs (since the queue
> >>  length is two, and we just completed one).
> >
> >I don't understand what is the point with nmi_in_progress, and the above
> >hunk, either. Can't inject_nmi do:
> >
> >if (nmi_injected + atomic_read(nmi_pending)<  2)
> >     atomic_inc(nmi_pending)
> >
> >Instead of collapsing somewhere else?
> 
> We could.  It's not atomic though - two threads executing in
> parallel could raise the value to three.  Could do a cmpxchg loop
> does an increment bounded to two.  I guess this is a lot clearer,
> thanks.
> 
> >You'd also have to change
> >nmi_injected handling in arch code so its value is not "hidden", in
> >complete_interrupts().
> 
> Or maybe make raising nmi_injected not decrement nmi_pending.  So:
> 
>   nmi_pending: total number of interrupts in queue
>   nmi_injected: of these, how many are currently being injected
> 
> yes?

Yes, at the expense of decrementing on subarch code (which is fine,
apparently).