On Sun, Feb 03, 2013 at 10:57:00PM -0200, Marcelo Tosatti wrote:
> On Thu, Jan 31, 2013 at 03:55:56PM +0200, Gleb Natapov wrote:
> > On Thu, Jan 31, 2013 at 11:44:43AM -0200, Marcelo Tosatti wrote:
> > > On Thu, Jan 31, 2013 at 03:38:37PM +0200, Gleb Natapov wrote:
> > > > On Thu, Jan 31, 2013 at 11:32:45AM -0200, Marcelo Tosatti wrote:
> > > > > On Thu, Jan 31, 2013 at 11:43:48AM +0200, Gleb Natapov wrote:
> > > > > > On Wed, Jan 30, 2013 at 09:03:11PM -0200, Marcelo Tosatti wrote:
> > > > > > > Posted interrupt patch:
> > > > > > > 2) Must move IN_GUEST_MODE assignment after local_irq_disable, in
> > > > > > > vcpu_enter_guest function. Otherwise:
> > > > > > >
> > > > > > > cpu0 vcpu1<->cpu1
> > > > > > >
> > > > > > > vcpu->mode = IN_GUEST_MODE
> > > > > > >
> > > > > > > if IN_GUEST_MODE == true
> > > > > > > send IPI
> > > > > > > local_irq_disable
> > > > > > >
> > > > > > > PIR not transferred to VIRR, misses interrupt.
> > > > > > >
> > > > > > cpu0 will set KVM_REQ_EVENT, so vmentry will be aborted after
> > > > > > local_irq_disable() by ->requests check.
> > > > >
> > > > > Yes, but you don't want KVM_REQ_EVENT+kick. It defeats the purpose
> > > > > of posted interrupts. You want
> > > > >
> > > > > if vcpu in guest mode
> > > > > send posted interrupt IPI
> > > > > else
> > > > > KVM_REQ_EVENT+kick
> > > > >
> > > > I am thinking:
> > > >
> > > > set KVM_REQ_EVENT
> > > > if pi is enabled
> > > > send posted interrupt IPI
> > > > else
> > > > kick
> > >
> > > KVM_REQ_EVENT must be after sending posted interrupt IPI. Otherwise on
> > > the vcpu entry side
> > >
> > > test_and_clear(KVM_REQ_EVENT) {
> > > No bits set in PIR
> > > }
> > >
> > It should be after updating PIR, but before sending posted interrupt
> > IPI. Otherwise:
> >
> > cpu0 cpu1/vcpu
> >
> > KVM_REQ_EVENT is not set
> > set pir
> > send IPI
> > irq_disable()
> > ->request is empty.
> > set KVM_REQ_EVENT
> >
> > That's the same sequence as with IRR update, KVM_REQ_EVENT and kick
> > today.
>
> Can only send IPI if vcpu->mode == IN_GUEST_MODE, which must be set
> after interrupt flag is cleared as noted.
>
> Also KVM_REQ_EVENT is processed outside of interrupt disabled region today.
But it is checked in interrupt disabled section and vcpu entry is
aborted if event is pending.
>
> Or maybe i don't get what you say... write a complete
> description?
>
I am saying that we do not need to move vcpu->mode = IN_GUEST_MODE to
irq_disable() section to make things work. Just do:
set bit in pir
set KVM_REQ_EVENT
if in guest mode do IPI
> > > What about item 4 below?
> > >
> > That's for Yang to answer :)
>
> "If more than one interrupt is generated with the same vector number,
> the local APIC can set the bit for the vector both in the IRR and ISR.
> This means that for the Pentium 4 and Intel Xeon processors, the IRR
> and ISR can queue two interrupts for each interrupt vector: one in the
> IRR and one in the ISR. Any additional interrupts issued for the same
> interrupt vector are collapsed into the single bit in IRR"
>
> Which would mean KVM emulation differs... "Eventually 3 interrupts can
> be queued: one in IRR, one in ISR, one in PIR".
I do not think this is the case though. PIR will be folded into IRR
either during a guest entry or by vcpu itself on receiving of
notification vector IPI.
>
> Any example how software relies on such two-interrupts-queued-in-IRR/ISR behaviour?
Don't know about guests, but KVM relies on it to detect interrupt
coalescing. So if interrupt is set in IRR but not in PIR interrupt will
not be reported as coalesced, but it will be coalesced during PIR->IRR
merge.
--
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
