> Oops, I wrote the race with wrong IOCTL -- it should be KVM_IRQ_LINE. > > 1) set KVM_CAP_SPLIT_IRQCHIP (unlocks KVM_IRQ_LINE) > a) call KVM_CREATE_IRQCHIP (creates routes while !kvm->arch.vpic) > b) concurrently call KVM_IRQ_LINE for PIO routes (dereferences NULL) > > The problem is that we use pic_in_kernel() as irqchip_in_kernel(), so it > cannot be set before we set up routes, but we then cannot reject routes > when pic is not in use. The best effort is to do this for pic routes in > kvm_set_routing_entry(): > > // initialization is the only place where pic_in_kernel() != > ioapic_in_kernel() > if (!pic_in_kernel(kvm) && !ioapic_in_kernel(kvm)) > goto out; > > and similar for ioapic routes: > > if (!ioapic_in_kernel(kvm)) > goto out; > > I think it would work if we forbade KVM_CREATE_IRQCHIP after > KVM_CAP_SPLIT_IRQCHIP (which we want to do anyway).
Yeah, definitely. > And adding a new > variable for irqchip_in_kernel() would allow us to make the pic > condition reasonabled. Or change kvm->arch.irqchip_split to an enum. > I'll do something like that for 4.10, but the current patch is better > suited for stable. > > Would fixing the comment be enough? Yes, fine! > Do you want the following hunk already in 4.9? > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 6f9c9ad13f88..dbed51045c37 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -3901,7 +3901,7 @@ long kvm_arch_vm_ioctl(struct file *filp, > > mutex_lock(&kvm->lock); > r = -EEXIST; > - if (kvm->arch.vpic) > + if (irqchip_in_kernel(kvm)) > goto create_irqchip_unlock; > r = -EINVAL; > if (kvm->created_vcpus) No, it's unnecessary. Paolo
