KVM: x86: don't hold kvm->lock in KVM_SET_GSI_ROUTING

We needed the lock to avoid racing with creation of the irqchip on x86. As
kvm_set_irq_routing() calls srcu_synchronize_expedited(), this lock
might be held for a longer time.

Let's introduce an arch specific callback to check if we can actually
add irq routes. For x86, all we have to do is check if we have an
irqchip in the kernel. We don't need kvm->lock at that point as the
irqchip is marked as inititalized only when actually fully created.

Reported-by: Steve Rutherford <srutherford@google.com>
Reviewed-by: Radim Krčmář <rkrcmar@redhat.com>
Fixes: 1df6ddede10a ("KVM: x86: race between KVM_SET_GSI_ROUTING and KVM_CREATE_IRQCHIP")
Signed-off-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 84c8489531bb7c7a0d84282be2e2bd3077024ea1..f5bddf92faba81675594db3c6ead4d1baf94f68f 100644 (file)
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -728,7 +728,6 @@ struct kvm_hv {
 
 enum kvm_irqchip_mode {
        KVM_IRQCHIP_NONE,
-       KVM_IRQCHIP_INIT_IN_PROGRESS, /* temporarily set during creation */
        KVM_IRQCHIP_KERNEL,       /* created with KVM_CREATE_IRQCHIP */
        KVM_IRQCHIP_SPLIT,        /* created with KVM_CAP_SPLIT_IRQCHIP */
 };

diff --git a/arch/x86/kvm/irq.h b/arch/x86/kvm/irq.h
index 0edd22c3344c9e80fc3f8a609f9c102feb628b0a..d5005cc265217c4fa4d5de7c2ecc2a29fb599c19 100644 (file)
--- a/arch/x86/kvm/irq.h
+++ b/arch/x86/kvm/irq.h
@@ -111,7 +111,7 @@ static inline int irqchip_in_kernel(struct kvm *kvm)
 
        /* Matches smp_wmb() when setting irqchip_mode */
        smp_rmb();
-       return mode > KVM_IRQCHIP_INIT_IN_PROGRESS;
+       return mode != KVM_IRQCHIP_NONE;
 }
 
 void kvm_inject_pending_timer_irqs(struct kvm_vcpu *vcpu);

diff --git a/arch/x86/kvm/irq_comm.c b/arch/x86/kvm/irq_comm.c
index 4517a4c2ac3a1edd791dbf44c66b3b360aadc327..3cc3b2d130a0c277e6b4cb6f59aa2a391d2253f6 100644 (file)
--- a/arch/x86/kvm/irq_comm.c
+++ b/arch/x86/kvm/irq_comm.c
@@ -274,16 +274,19 @@ void kvm_fire_mask_notifiers(struct kvm *kvm, unsigned irqchip, unsigned pin,
        srcu_read_unlock(&kvm->irq_srcu, idx);
 }
 
+bool kvm_arch_can_set_irq_routing(struct kvm *kvm)
+{
+       return irqchip_in_kernel(kvm);
+}
+
 int kvm_set_routing_entry(struct kvm *kvm,
                          struct kvm_kernel_irq_routing_entry *e,
                          const struct kvm_irq_routing_entry *ue)
 {
-       /* also allow creation of routes during KVM_IRQCHIP_INIT_IN_PROGRESS */
-       if (kvm->arch.irqchip_mode == KVM_IRQCHIP_NONE)
-               return -EINVAL;
-
-       /* Matches smp_wmb() when setting irqchip_mode */
-       smp_rmb();
+       /* We can't check irqchip_in_kernel() here as some callers are
+        * currently inititalizing the irqchip. Other callers should therefore
+        * check kvm_arch_can_set_irq_routing() before calling this function.
+        */
        switch (ue->type) {
        case KVM_IRQ_ROUTING_IRQCHIP:
                if (irqchip_split(kvm))

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index be2ade58edb9f0fca4f05bd7b4ab8a1921557dc8..2fe9aa116288b4c4bc09940eb1232188ed193479 100644 (file)
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3919,14 +3919,9 @@ static int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
                        goto split_irqchip_unlock;
                if (kvm->created_vcpus)
                        goto split_irqchip_unlock;
-               kvm->arch.irqchip_mode = KVM_IRQCHIP_INIT_IN_PROGRESS;
                r = kvm_setup_empty_irq_routing(kvm);
-               if (r) {
-                       kvm->arch.irqchip_mode = KVM_IRQCHIP_NONE;
-                       /* Pairs with smp_rmb() when reading irqchip_mode */
-                       smp_wmb();
+               if (r)
                        goto split_irqchip_unlock;
-               }
                /* Pairs with irqchip_in_kernel. */
                smp_wmb();
                kvm->arch.irqchip_mode = KVM_IRQCHIP_SPLIT;
@@ -4012,12 +4007,8 @@ long kvm_arch_vm_ioctl(struct file *filp,
                        goto create_irqchip_unlock;
                }
 
-               kvm->arch.irqchip_mode = KVM_IRQCHIP_INIT_IN_PROGRESS;
                r = kvm_setup_default_irq_routing(kvm);
                if (r) {
-                       kvm->arch.irqchip_mode = KVM_IRQCHIP_NONE;
-                       /* Pairs with smp_rmb() when reading irqchip_mode */
-                       smp_wmb();
                        kvm_ioapic_destroy(kvm);
                        kvm_pic_destroy(kvm);
                        goto create_irqchip_unlock;

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index a5bfffa8c8d43936f020d3cd59e82f408f9ece49..25cf258a1c9bf2236b9792bfa3f0aab1c332ad7e 100644 (file)
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -1018,6 +1018,7 @@ static inline int mmu_notifier_retry(struct kvm *kvm, unsigned long mmu_seq)
 #define KVM_MAX_IRQ_ROUTES 1024
 #endif
 
+bool kvm_arch_can_set_irq_routing(struct kvm *kvm);
 int kvm_set_irq_routing(struct kvm *kvm,
                        const struct kvm_irq_routing_entry *entries,
                        unsigned nr,

diff --git a/virt/kvm/irqchip.c b/virt/kvm/irqchip.c
index cc30d01a56bee9e93b02aa0b586b83288765b9e8..31e40c9e81df41de6f8cadb076515677ac5cae9c 100644 (file)
--- a/virt/kvm/irqchip.c
+++ b/virt/kvm/irqchip.c
@@ -172,6 +172,11 @@ void __attribute__((weak)) kvm_arch_irq_routing_update(struct kvm *kvm)
 {
 }
 
+bool __weak kvm_arch_can_set_irq_routing(struct kvm *kvm)
+{
+       return true;
+}
+
 int kvm_set_irq_routing(struct kvm *kvm,
                        const struct kvm_irq_routing_entry *ue,
                        unsigned nr,

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 035bc51f656f99e88d5016f8c2d14901e5a979da..6281cc2446d59c109df694757231199b9f4f8649 100644 (file)
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -3075,6 +3075,8 @@ static long kvm_vm_ioctl(struct file *filp,
                if (copy_from_user(&routing, argp, sizeof(routing)))
                        goto out;
                r = -EINVAL;
+               if (!kvm_arch_can_set_irq_routing(kvm))
+                       goto out;
                if (routing.nr > KVM_MAX_IRQ_ROUTES)
                        goto out;
                if (routing.flags)
@@ -3090,11 +3092,8 @@ static long kvm_vm_ioctl(struct file *filp,
                                           routing.nr * sizeof(*entries)))
                                goto out_free_irq_routing;
                }
-               /* avoid races with KVM_CREATE_IRQCHIP on x86 */
-               mutex_lock(&kvm->lock);
                r = kvm_set_irq_routing(kvm, entries, routing.nr,
                                        routing.flags);
-               mutex_unlock(&kvm->lock);
 out_free_irq_routing:
                vfree(entries);
                break;