On Mon, Feb 25, 2013 at 06:55:58AM +0000, Zhang, Yang Z wrote:
> > 
> > p1)
> > 
> >>> 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
> > 
> > p2)
> > 
> >> No, this exists with your implementation not mine.
> >> As I said, in this patch, it will make request after vcpu ==guest mode, 
> >> then send
> > ipi. Even the ipi is lost, but the request still will be tracked. Because 
> > we have the
> > follow check:
> >> vcpu->mode = GUEST_MODE
> >> (ipi may arrived here and lost)
> >> local irq disable
> >> check request (this will ensure the pir modification will be picked up by 
> >> vcpu
> > before vmentry)
> > 
> > Please read the sequence above again, between p1) and p2). Yes, if the
> > IPI is lost, the request _for the CPU which sent PIR IPI_ is guaranteed
> > to be processed, but not the request for another cpu (cpu1).
> If no PIR IPI sent in cpu1, the delivered is false, and it will roll back to 
> old logic:
> __apic_accept_irq():
> if (!delivered) {
>              kvm_make_request(KVM_REQ_EVENT, vcpu);
>              kvm_vcpu_kick(vcpu);
> }
> This can solve the problem you mentioned above. Right?

Should not be doing this kick in the first place. One result of it is
that you'll always take a VM-exit if a second injection happens while a VCPU
has not handled the first one.

What is the drawback of the suggestion to unconditionally clear PIR
notification bit on VM-entry?

We can avoid it, but lets get it correct first  (BTW can stick a comment
saying FIXME: optimize) on that PIR clearing.

> > cpu0
> > 
> > check pir(pass)
> > check irr(pass)
> > injected = !test_and_set_bit(pir)
> > 
> > versus
> > 
> > cpu1
> >xchg(pir)

> > 
> > No information can be lost because all accesses to shared data are
> > atomic.
> Sorry, I cannot get your point. Why 'xchg(pir)' can solve the problem I 
> mentioned above? Can you elaborate it?
> The spinlock I used is trying to protect the whole procedure of sync pir to 
> irr, not just access pir.

You're right, its the same problem as with the hardware update.

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to