From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marcelo Tosatti <mtosatti@redhat.com>
Subject: Re: [PATCH 1/3] KVM: Emulation MSI-X mask bits for assigned devices
Date: Mon, 11 Oct 2010 14:11:59 -0300
Message-ID: <20101011171159.GA6316@amt.cnet>
References: <1285667052-24907-1-git-send-email-sheng@linux.intel.com>
 <1285667052-24907-2-git-send-email-sheng@linux.intel.com>
 <20100930161809.GA29881@amt.cnet>
 <201010111749.17107.sheng@linux.intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: Avi Kivity <avi@redhat.com>, kvm@vger.kernel.org
To: Sheng Yang <sheng@linux.intel.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:63416 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1755244Ab0JKRPM (ORCPT <rfc822;kvm@vger.kernel.org>);
	Mon, 11 Oct 2010 13:15:12 -0400
Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22])
	by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o9BHFBr9016121
	(version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK)
	for <kvm@vger.kernel.org>; Mon, 11 Oct 2010 13:15:12 -0400
Content-Disposition: inline
In-Reply-To: <201010111749.17107.sheng@linux.intel.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Mon, Oct 11, 2010 at 05:49:17PM +0800, Sheng Yang wrote:
> On Friday 01 October 2010 00:18:10 Marcelo Tosatti wrote:
> > On Tue, Sep 28, 2010 at 05:44:10PM +0800, Sheng Yang wrote:
> > > This patch enable per-vector mask for assigned devices using MSI-X.
> > > 
> > > Signed-off-by: Sheng Yang <sheng@linux.intel.com>
> > > ---
> > > 
> > >  arch/x86/kvm/x86.c       |    1 +
> > >  include/linux/kvm.h      |    9 ++++++++-
> > >  include/linux/kvm_host.h |    1 +
> > >  virt/kvm/assigned-dev.c  |   39 
> +++++++++++++++++++++++++++++++++++++++
> > >  4 files changed, 49 insertions(+), 1 deletions(-)
> > >  
> > >  
> > >  #include <linux/slab.h>
> > > 
> > > +#include <linux/irqnr.h>
> > > +
> > > 
> > >  #include "irq.h"
> > >  
> > >  static struct kvm_assigned_dev_kernel *kvm_find_assigned_dev(struct
> > >  list_head *head,
> > > 
> > > @@ -666,6 +668,30 @@ msix_nr_out:
> > >  	return r;
> > >  
> > >  }
> > > 
> > > +static void update_msix_mask(struct kvm_assigned_dev_kernel
> > > *assigned_dev, +			     int index)
> > > +{
> > > +	int irq;
> > > +
> > > +	if (!assigned_dev->dev->msix_enabled ||
> > > +	    !(assigned_dev->irq_requested_type & KVM_DEV_IRQ_HOST_MSIX))
> > > +		return;
> > > +
> > > +	irq = assigned_dev->host_msix_entries[index].vector;
> > > +
> > > +	ASSERT(irq != 0);
> > > +
> > > +	if (assigned_dev->guest_msix_entries[index].flags &
> > > +			KVM_ASSIGNED_MSIX_MASK)
> > > +		disable_irq(irq);
> > > +	else {
> > > +		enable_irq(irq);
> > > +		if (assigned_dev->guest_msix_entries[index].flags &
> > > +				KVM_ASSIGNED_MSIX_PENDING)
> > > +			schedule_work(&assigned_dev->interrupt_work);
> > > +	}
> > > +}
> > > +
> > 
> > Should flush the workqueue after disabling irq. As you say, the
> > schedule_work should be unnecessary.
> > 
> > Also must be careful with races.
> 
> Do we need to flush the workqueue? I think the return of writing MSI-X mask bit 
> doesn't have implicit meaning that we have handled all pending interrupts. So I 
> think leave the work later should also be fine.

The point is the interrupt will be delivered to the guest after the mask
bit is set. AFAICS, it should either be either before or after (before
is easier to implement, flushing the workqueue).