Gleb Natapov wrote on 2013-03-19: > On Tue, Mar 19, 2013 at 12:42:01PM +0000, Zhang, Yang Z wrote: >>>>>> local_irq_disable(); >>>>>> + kvm_x86_ops->posted_intr_clear_on(vcpu); >>>>>> + >>>>> Why is this separate from pir_to_irr syncing? >>>> This is the result of discussion with Marcelo. It is more reasonable to >>>> put it here to avoid unnecessary posted interrupt between: >>>> >>>> vcpu->mode = IN_GUEST_MODE; >>>> >>>> <--interrupt may arrived here and this is unnecessary. >>>> >>>> local_irq_disable(); >>>> >>> >>> But this still can happen as far as I see: >>> >>> vcpu0 vcpu1: >>> pi_test_and_set_pir() kvm_make_request(KVM_REQ_EVENT) >>> if (KVM_REQ_EVENT) >>> sync_pir_to_irr() >>> vcpu->mode = >>> IN_GUEST_MODE; >>> if (vcpu->mode == IN_GUEST_MODE) >>> if (!pi_test_and_set_on()) >>> apic->send_IPI_mask() >>> --> IPI arrives here >>> local_irq_disable(); >>> posted_intr_clear_on() >> Current solution is trying to block other Posted Interrupt from other VCPUs at > same time. It only mitigates it but cannot solve it. The case you mentioned still > exists but it should be rare. >> > I am not sure I follow. What scenario exactly are you talking about. I > looked over past discussion about it and saw that Marcelo gives an > example how IPI can be lost, but I think that's because we set "on" bit > after KVM_REQ_EVENT: The IPI will not lost in his example(he misread the patch). > cpu0 cpu1 vcpu0 > test_and_set_bit(PIR-A) set KVM_REQ_EVENT > process > REQ_EVENT > PIR-A->IRR > > vcpu->mode=IN_GUEST > > if (vcpu0->guest_mode) > if (!t_a_s_bit(PIR notif)) > send IPI > linux_pir_handler > > t_a_s_b(PIR-B)=1 > no PIR IPI sent > > But what if on delivery we do: > pi_test_and_set_pir() > r = pi_test_and_set_on() > kvm_make_request(KVM_REQ_EVENT) > if (!r) > send_IPI_mask() else kvm_vcpu_kick() > And on vcpu entry we do: > if (kvm_check_request(KVM_REQ_EVENT) > if (test_and_clear_bit(on)) > kvm_apic_update_irr() > What are the downsides? Can we lost interrupts this way? Need to check guest mode before sending IPI. Otherwise hypervisor may receive IPI. I think current logic is ok. Only problem is that when to clear Outstanding Notification bit. Actually I prefer your suggestion to clear it before sync_pir_irr. But Marcelo prefer to clear ON bit after disabling irq. >>> May be move vcpu->mode = IN_GUEST_MODE after local_irq_disable()? >> Yes, this will solve it. But I am not sure whether it will introduce >> any regressions. Is there any check relies on this sequence? >> > Do not think so. > > -- > Gleb. Best regards, Yang -- 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
