On Tue, Mar 19, 2013 at 12:13:11PM -0300, Marcelo Tosatti wrote:
> On Tue, Mar 19, 2013 at 03:29:24PM +0200, Gleb Natapov wrote:
> > 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:
> >
> > 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?
>
> You should not ever enter guest mode with ON bit set on PIR (because that will
> prevent PIR IPI from waking up interrupt injection logic). This is why
> the ON bit should be cleared on VM entry.
I agree that we should not, but I claim that this will not happen since
KVM_REQ_EVENT will prevent guest entry with ON bit set.
>
> > > > 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.
>
> Can't see what is the problem with the code as it is?
>
Just the useless IPI that can be prevented. I agree that this is not a
big deal unless shown otherwise.
--
Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
