Re: [PATCHv4 2/2] kvm: deliver msi interrupts from irq handler

[Gleb Natapov <gleb@redhat.com>]

On Wed, Nov 28, 2012 at 01:56:16PM +0200, Michael S. Tsirkin wrote:
> On Wed, Nov 28, 2012 at 01:43:34PM +0200, Gleb Natapov wrote:
> > On Wed, Oct 17, 2012 at 06:06:06PM +0200, Michael S. Tsirkin wrote:
> > > We can deliver certain interrupts, notably MSI,
> > > from atomic context.  Use kvm_set_irq_inatomic,
> > > to implement an irq handler for msi.
> > > 
> > > This reduces the pressure on scheduler in case
> > > where host and guest irq share a host cpu.
> > > 
> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > ---
> > >  virt/kvm/assigned-dev.c | 36 ++++++++++++++++++++++++++----------
> > >  1 file changed, 26 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/virt/kvm/assigned-dev.c b/virt/kvm/assigned-dev.c
> > > index 23a41a9..3642239 100644
> > > --- a/virt/kvm/assigned-dev.c
> > > +++ b/virt/kvm/assigned-dev.c
> > > @@ -105,6 +105,15 @@ static irqreturn_t kvm_assigned_dev_thread_intx(int irq, void *dev_id)
> > >  }
> > >  
> > >  #ifdef __KVM_HAVE_MSI
> > > +static irqreturn_t kvm_assigned_dev_msi(int irq, void *dev_id)
> > > +{
> > > +	struct kvm_assigned_dev_kernel *assigned_dev = dev_id;
> > > +	int ret = kvm_set_irq_inatomic(assigned_dev->kvm,
> > > +				       assigned_dev->irq_source_id,
> > > +				       assigned_dev->guest_irq, 1);
> > Why not use kvm_set_msi_inatomic() and drop kvm_set_irq_inatomic() from
> > previous patch? 
> 
> kvm_set_msi_inatomic needs a routing entry, and
> we don't have the routing entry at this level.
> 
Yes, right. BTW is this interface will be used only for legacy assigned
device or there will be other users too?

> Further, guest irq might not be an MSI: host MSI
> can cause guest intx injection I think, we need to
> bounce it to thread as we did earlier.
Ah, so msi in kvm_assigned_dev_msi() is about host msi? Can host be intx
but guest msi? You seems to not handle this case. Also injection of intx
via ioapic is the same as injecting MSI. The format and the capability
of irq message are essentially the same.


> 
> > > +	return unlikely(ret == -EWOULDBLOCK) ? IRQ_WAKE_THREAD : IRQ_HANDLED;
> > > +}
> > > +
> > >  static irqreturn_t kvm_assigned_dev_thread_msi(int irq, void *dev_id)
> > >  {
> > >  	struct kvm_assigned_dev_kernel *assigned_dev = dev_id;
> > > @@ -117,6 +126,23 @@ static irqreturn_t kvm_assigned_dev_thread_msi(int irq, void *dev_id)
> > >  #endif
> > >  
> > >  #ifdef __KVM_HAVE_MSIX
> > > +static irqreturn_t kvm_assigned_dev_msix(int irq, void *dev_id)
> > > +{
> > > +	struct kvm_assigned_dev_kernel *assigned_dev = dev_id;
> > > +	int index = find_index_from_host_irq(assigned_dev, irq);
> > > +	u32 vector;
> > > +	int ret = 0;
> > > +
> > > +	if (index >= 0) {
> > > +		vector = assigned_dev->guest_msix_entries[index].vector;
> > > +		ret = kvm_set_irq_inatomic(assigned_dev->kvm,
> > > +					   assigned_dev->irq_source_id,
> > > +					   vector, 1);
> > > +	}
> > > +
> > > +	return unlikely(ret == -EWOULDBLOCK) ? IRQ_WAKE_THREAD : IRQ_HANDLED;
> > > +}
> > > +
> > >  static irqreturn_t kvm_assigned_dev_thread_msix(int irq, void *dev_id)
> > >  {
> > >  	struct kvm_assigned_dev_kernel *assigned_dev = dev_id;
> > > @@ -334,11 +360,6 @@ static int assigned_device_enable_host_intx(struct kvm *kvm,
> > >  }
> > >  
> > >  #ifdef __KVM_HAVE_MSI
> > > -static irqreturn_t kvm_assigned_dev_msi(int irq, void *dev_id)
> > > -{
> > > -	return IRQ_WAKE_THREAD;
> > > -}
> > > -
> > >  static int assigned_device_enable_host_msi(struct kvm *kvm,
> > >  					   struct kvm_assigned_dev_kernel *dev)
> > >  {
> > > @@ -363,11 +384,6 @@ static int assigned_device_enable_host_msi(struct kvm *kvm,
> > >  #endif
> > >  
> > >  #ifdef __KVM_HAVE_MSIX
> > > -static irqreturn_t kvm_assigned_dev_msix(int irq, void *dev_id)
> > > -{
> > > -	return IRQ_WAKE_THREAD;
> > > -}
> > > -
> > >  static int assigned_device_enable_host_msix(struct kvm *kvm,
> > >  					    struct kvm_assigned_dev_kernel *dev)
> > >  {
> > > -- 
> > > MST
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe kvm" in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> > --
> > 			Gleb.

--
			Gleb.