2016-12-20 19:59+0800, Wanpeng Li: > 2016-11-24 20:42 GMT+08:00 Radim Krčmář <rkrcmar@xxxxxxxxxx>: >> 2016-11-23 22:58+0100, Paolo Bonzini: >>> On 23/11/2016 21:25, Radim Krčmář wrote: >>>> diff --git a/arch/x86/kvm/irq_comm.c b/arch/x86/kvm/irq_comm.c >>>> index 25810b144b58..ddd63b8b176e 100644 >>>> --- a/arch/x86/kvm/irq_comm.c >>>> +++ b/arch/x86/kvm/irq_comm.c >>>> @@ -41,6 +41,15 @@ static int kvm_set_pic_irq(struct kvm_kernel_irq_routing_entry *e, >>>> bool line_status) >>>> { >>>> struct kvm_pic *pic = pic_irqchip(kvm); >>>> + >>>> + /* >>>> + * XXX: rejecting pic routes when pic isn't in use would be better, >>>> + * but the default routing table is installed while kvm->arch.vpic is >>>> + * NULL and KVM_CREATE_IRQCHIP can race with KVM_SET_GSI_ROUTING. >>>> + */ >>>> + if (!pic) >>>> + return -1; >>>> + >>>> return kvm_pic_set_irq(pic, e->irqchip.pin, irq_source_id, level); >>>> } >>>> >>> >>> Can you explain the race with the default routing table better? It >>> seems to me that it can only make the routing table go from invalid to >>> valid (there is no KVM_DESTROY_IRQCHIP) so it's benign. >> >> 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) > > If we should not go through irqfd if irqchip is split? I also remember hearing about that -- do you remember where it was? The documentation does not say that and irqfd is mostly optimization for KVM_IRQ_LINE ... QEMU uses KVM_IRQ_LINE_STATUS with split irqchip, so we can't easily say the opposite now. -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html