On Tue, Mar 19, 2013 at 01:59:21PM +0000, Zhang, Yang Z wrote: > 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. Of course, forget it. So if (!r) should be if (!r && mode == IN_GUEST) > 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. Marcelo what is the problem with the logic above? -- Gleb. -- 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
