On Wed, Apr 21, 2010 at 11:32:49AM +0300, Avi Kivity wrote: > On 04/20/2010 06:54 PM, Marcelo Tosatti wrote: > >The assigned device interrupt work handler calls kvm_set_irq, which > >can sleep, for example, waiting for the ioapic mutex, from irq disabled > >section. > > > >https://bugzilla.kernel.org/show_bug.cgi?id=15725 > > > >Fix by dropping assigned_dev_lock (and re-enabling interrupts) > >before invoking kvm_set_irq for the KVM_DEV_IRQ_HOST_MSIX case. Other > >cases do not require the lock or interrupts disabled (a new work > >instance will be queued in case of concurrent interrupt). > > > >KVM-Stable-Tag. > >Signed-off-by: Marcelo Tosatti<mtosatti@xxxxxxxxxx> > > > >diff --git a/virt/kvm/assigned-dev.c b/virt/kvm/assigned-dev.c > >index 47ca447..7ac7bbe 100644 > >--- a/virt/kvm/assigned-dev.c > >+++ b/virt/kvm/assigned-dev.c > >@@ -64,24 +64,33 @@ static void kvm_assigned_dev_interrupt_work_handler(struct work_struct *work) > > interrupt_work); > > kvm = assigned_dev->kvm; > > > >- spin_lock_irq(&assigned_dev->assigned_dev_lock); > > if (assigned_dev->irq_requested_type& KVM_DEV_IRQ_HOST_MSIX) { > > struct kvm_guest_msix_entry *guest_entries = > > assigned_dev->guest_msix_entries; > >+ > >+ spin_lock_irq(&assigned_dev->assigned_dev_lock); > > for (i = 0; i< assigned_dev->entries_nr; i++) { > > if (!(guest_entries[i].flags& > > KVM_ASSIGNED_MSIX_PENDING)) > > continue; > > guest_entries[i].flags&= ~KVM_ASSIGNED_MSIX_PENDING; > >+ /* > >+ * If kvm_assigned_dev_intr sets pending for an > >+ * entry smaller than this work instance is > >+ * currently processing, a new work instance > >+ * will be queued. > >+ */ > >+ spin_unlock_irq(&assigned_dev->assigned_dev_lock); > > What happens if assigned_dev->entries_nr is changed here? It does not. Interrupts and work instances are stopped before irq_requested_type or entries_nr can change. > > kvm_set_irq(assigned_dev->kvm, > > assigned_dev->irq_source_id, > > guest_entries[i].vector, 1); > >+ spin_lock_irq(&assigned_dev->assigned_dev_lock); > > } > >+ spin_unlock_irq(&assigned_dev->assigned_dev_lock); > > } else > > kvm_set_irq(assigned_dev->kvm, assigned_dev->irq_source_id, > > assigned_dev->guest_irq, 1); > > > >- spin_unlock_irq(&assigned_dev->assigned_dev_lock); > > } > > A less dangerous fix is to copy all vectors to be triggered into a > local array, drop the lock, and replay the array into kvm_set_irq(). > The else branch could do this as well. Its a bit large, 256 max vectors. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
