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@xxxxxxxxxxxxxxx> I think I see an issue here, noted below. Some general comments: - The mask bit seems broken for injecting interrupts from userspace (with interrupts/ioctls). I think we must test it on injection path. - We'll need a way to support the pending bit. Disabling interrupts will not let us do it. > --- > 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(-) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 8412c91..e6933e6 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -1927,6 +1927,7 @@ int kvm_dev_ioctl_check_extension(long ext) > case KVM_CAP_DEBUGREGS: > case KVM_CAP_X86_ROBUST_SINGLESTEP: > case KVM_CAP_XSAVE: > + case KVM_CAP_DEVICE_MSIX_MASK: > r = 1; > break; > case KVM_CAP_COALESCED_MMIO: > diff --git a/include/linux/kvm.h b/include/linux/kvm.h > index 919ae53..f2b7cdc 100644 > --- a/include/linux/kvm.h > +++ b/include/linux/kvm.h > @@ -540,6 +540,9 @@ struct kvm_ppc_pvinfo { > #endif > #define KVM_CAP_PPC_GET_PVINFO 57 > #define KVM_CAP_PPC_IRQ_LEVEL 58 > +#ifdef __KVM_HAVE_MSIX > +#define KVM_CAP_DEVICE_MSIX_MASK 59 > +#endif > > #ifdef KVM_CAP_IRQ_ROUTING > > @@ -787,11 +790,15 @@ struct kvm_assigned_msix_nr { > }; > > #define KVM_MAX_MSIX_PER_DEV 256 > + > +#define KVM_MSIX_FLAG_MASK 1 > + > struct kvm_assigned_msix_entry { > __u32 assigned_dev_id; > __u32 gsi; > __u16 entry; /* The index of entry in the MSI-X table */ > - __u16 padding[3]; > + __u16 flags; > + __u16 padding[2]; > }; > > #endif /* __LINUX_KVM_H */ > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h > index 0b89d00..a43405c 100644 > --- a/include/linux/kvm_host.h > +++ b/include/linux/kvm_host.h > @@ -415,6 +415,7 @@ struct kvm_irq_ack_notifier { > }; > > #define KVM_ASSIGNED_MSIX_PENDING 0x1 > +#define KVM_ASSIGNED_MSIX_MASK 0x2 > struct kvm_guest_msix_entry { > u32 vector; > u16 entry; > diff --git a/virt/kvm/assigned-dev.c b/virt/kvm/assigned-dev.c > index 7c98928..15b8c32 100644 > --- a/virt/kvm/assigned-dev.c > +++ b/virt/kvm/assigned-dev.c > @@ -17,6 +17,8 @@ > #include <linux/pci.h> > #include <linux/interrupt.h> > #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); > + } > +} > + What happens if guest masks an entry and then we hot-unplug the device and remove it from guest? It looks like interrupt will stay disabled? > static int kvm_vm_ioctl_set_msix_entry(struct kvm *kvm, > struct kvm_assigned_msix_entry *entry) > { > @@ -688,6 +714,19 @@ static int kvm_vm_ioctl_set_msix_entry(struct kvm *kvm, > adev->guest_msix_entries[i].entry = entry->entry; > adev->guest_msix_entries[i].vector = entry->gsi; > adev->host_msix_entries[i].entry = entry->entry; > + if ((entry->flags & KVM_MSIX_FLAG_MASK) && > + !(adev->guest_msix_entries[i].flags & > + KVM_ASSIGNED_MSIX_MASK)) { > + adev->guest_msix_entries[i].flags |= > + KVM_ASSIGNED_MSIX_MASK; > + update_msix_mask(adev, i); > + } else if (!(entry->flags & KVM_MSIX_FLAG_MASK) && > + (adev->guest_msix_entries[i].flags & > + KVM_ASSIGNED_MSIX_MASK)) { > + adev->guest_msix_entries[i].flags &= > + ~KVM_ASSIGNED_MSIX_MASK; > + update_msix_mask(adev, i); > + } > break; > } > if (i == adev->entries_nr) { > -- > 1.7.0.1 > > -- > 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 -- 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