On Tue, Mar 19, 2013 at 04:51:04PM +0200, Gleb Natapov wrote: > 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? > Just to clarify the advantages that I see are: one less callback, no need to sync pir to irr on each event and, arguably, a little bit simpler logic. -- 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
