Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

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>

authored by

David Hildenbrand and committed by
Paolo Bonzini
5c0aea0e bcb85c88

+19 -21
-1
arch/x86/include/asm/kvm_host.h
··· 728 728 729 729 enum kvm_irqchip_mode { 730 730 KVM_IRQCHIP_NONE, 731 - KVM_IRQCHIP_INIT_IN_PROGRESS, /* temporarily set during creation */ 732 731 KVM_IRQCHIP_KERNEL, /* created with KVM_CREATE_IRQCHIP */ 733 732 KVM_IRQCHIP_SPLIT, /* created with KVM_CAP_SPLIT_IRQCHIP */ 734 733 };
+1 -1
arch/x86/kvm/irq.h
··· 111 111 112 112 /* Matches smp_wmb() when setting irqchip_mode */ 113 113 smp_rmb(); 114 - return mode > KVM_IRQCHIP_INIT_IN_PROGRESS; 114 + return mode != KVM_IRQCHIP_NONE; 115 115 } 116 116 117 117 void kvm_inject_pending_timer_irqs(struct kvm_vcpu *vcpu);
+9 -6
arch/x86/kvm/irq_comm.c
··· 274 274 srcu_read_unlock(&kvm->irq_srcu, idx); 275 275 } 276 276 277 + bool kvm_arch_can_set_irq_routing(struct kvm *kvm) 278 + { 279 + return irqchip_in_kernel(kvm); 280 + } 281 + 277 282 int kvm_set_routing_entry(struct kvm *kvm, 278 283 struct kvm_kernel_irq_routing_entry *e, 279 284 const struct kvm_irq_routing_entry *ue) 280 285 { 281 - /* also allow creation of routes during KVM_IRQCHIP_INIT_IN_PROGRESS */ 282 - if (kvm->arch.irqchip_mode == KVM_IRQCHIP_NONE) 283 - return -EINVAL; 284 - 285 - /* Matches smp_wmb() when setting irqchip_mode */ 286 - smp_rmb(); 286 + /* We can't check irqchip_in_kernel() here as some callers are 287 + * currently inititalizing the irqchip. Other callers should therefore 288 + * check kvm_arch_can_set_irq_routing() before calling this function. 289 + */ 287 290 switch (ue->type) { 288 291 case KVM_IRQ_ROUTING_IRQCHIP: 289 292 if (irqchip_split(kvm))
+1 -10
arch/x86/kvm/x86.c
··· 3919 3919 goto split_irqchip_unlock; 3920 3920 if (kvm->created_vcpus) 3921 3921 goto split_irqchip_unlock; 3922 - kvm->arch.irqchip_mode = KVM_IRQCHIP_INIT_IN_PROGRESS; 3923 3922 r = kvm_setup_empty_irq_routing(kvm); 3924 - if (r) { 3925 - kvm->arch.irqchip_mode = KVM_IRQCHIP_NONE; 3926 - /* Pairs with smp_rmb() when reading irqchip_mode */ 3927 - smp_wmb(); 3923 + if (r) 3928 3924 goto split_irqchip_unlock; 3929 - } 3930 3925 /* Pairs with irqchip_in_kernel. */ 3931 3926 smp_wmb(); 3932 3927 kvm->arch.irqchip_mode = KVM_IRQCHIP_SPLIT; ··· 4007 4012 goto create_irqchip_unlock; 4008 4013 } 4009 4014 4010 - kvm->arch.irqchip_mode = KVM_IRQCHIP_INIT_IN_PROGRESS; 4011 4015 r = kvm_setup_default_irq_routing(kvm); 4012 4016 if (r) { 4013 - kvm->arch.irqchip_mode = KVM_IRQCHIP_NONE; 4014 - /* Pairs with smp_rmb() when reading irqchip_mode */ 4015 - smp_wmb(); 4016 4017 kvm_ioapic_destroy(kvm); 4017 4018 kvm_pic_destroy(kvm); 4018 4019 goto create_irqchip_unlock;
+1
include/linux/kvm_host.h
··· 1018 1018 #define KVM_MAX_IRQ_ROUTES 1024 1019 1019 #endif 1020 1020 1021 + bool kvm_arch_can_set_irq_routing(struct kvm *kvm); 1021 1022 int kvm_set_irq_routing(struct kvm *kvm, 1022 1023 const struct kvm_irq_routing_entry *entries, 1023 1024 unsigned nr,
+5
virt/kvm/irqchip.c
··· 172 172 { 173 173 } 174 174 175 + bool __weak kvm_arch_can_set_irq_routing(struct kvm *kvm) 176 + { 177 + return true; 178 + } 179 + 175 180 int kvm_set_irq_routing(struct kvm *kvm, 176 181 const struct kvm_irq_routing_entry *ue, 177 182 unsigned nr,
+2 -3
virt/kvm/kvm_main.c
··· 3075 3075 if (copy_from_user(&routing, argp, sizeof(routing))) 3076 3076 goto out; 3077 3077 r = -EINVAL; 3078 + if (!kvm_arch_can_set_irq_routing(kvm)) 3079 + goto out; 3078 3080 if (routing.nr > KVM_MAX_IRQ_ROUTES) 3079 3081 goto out; 3080 3082 if (routing.flags) ··· 3092 3090 routing.nr * sizeof(*entries))) 3093 3091 goto out_free_irq_routing; 3094 3092 } 3095 - /* avoid races with KVM_CREATE_IRQCHIP on x86 */ 3096 - mutex_lock(&kvm->lock); 3097 3093 r = kvm_set_irq_routing(kvm, entries, routing.nr, 3098 3094 routing.flags); 3099 - mutex_unlock(&kvm->lock); 3100 3095 out_free_irq_routing: 3101 3096 vfree(entries); 3102 3097 break;