Re: [PATCH 1/3] KVM: Emulation MSI-X mask bits for assigned devices

[Marcelo Tosatti <mtosatti@redhat.com>]

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).