Re: [PATCH 2/2] x86, apicv: Add Posted Interrupt supporting

Gleb Natapov
2013-02-07T14:01:15Z

On Wed, Feb 06, 2013 at 08:49:23PM -0200, Marcelo Tosatti wrote:
> > Second is that interrupt may be
> > reported as delivered, but it will be coalesced (possible only with the self
> > IPI with the same vector):
> >
> > Starting condition: PIR=0, IRR=0 vcpu is in a guest mode
> >
> > io thread | vcpu
> > accept_apic_interrupt() |
> > PIR and IRR is zero |
> > set PIR |
> > return delivered |
> > | self IPI
> > | set IRR
> > | merge PIR to IRR (*)
> >
> > At (*) interrupt that was reported as delivered is coalesced.
>
> Only vcpu itself should send self-IPI, so its fine.
>
It is fine only because this is not happening in practice (I hope) for single interrupt
we care about. Otherwise this is serious problem.

> > > Or:
> > >
> > > apic_accept_interrupt() {
> > >
> > > 1. Read ORIG_PIR=PIR, ORIG_IRR=IRR.
> > > Never set IRR when HWAPIC enabled, even if outside of guest mode.
> > > 2. Set PIR and let HW or SW VM-entry transfer it to IRR.
> > > 3. set_irq return value: (ORIG_PIR or ORIG_IRR set).
> > > }
> > >
> > This can report interrupt as coalesced, but it will be eventually delivered
> > as separate interrupt:
> >
> > Starting condition: PIR=0, IRR=1 vcpu is in a guest mode
> >
> > io thread | vcpu
> > |
> > accept_apic_interrupt() |
> > ORIG_PIR=0, ORIG_IRR=1 |
> > | EOI
> > | clear IRR, set ISR
> > set PIR |
> > return coalesced |
> > | clear PIR, set IRR
> > | EOI
> > | clear IRR, set ISR (*)
> >
> > At (*) interrupt that was reported as coalesced is delivered.
> >
> >
> > So still no perfect solution. But first one has much less serious
> > problems for our practical needs.
> >
> > > Two or more concurrent set_irq can race with each other, though. Can
> > > either document the race or add a lock.
> > >
> >
> > --
> > Gleb.
>
> Ok, then:
>
> accept_apic_irq:
> 1. coalesced = test_and_set_bit(PIR)
> 2. set KVM_REQ_EVENT bit (*)
> 3. if (vcpu->in_guest_mode)
> 4. if (test_and_set_bit(pir notification bit))
> 5. send PIR IPI
> 6. return coalesced
Do not see how it will help.

Starting condition: PIR=0, IRR=1 vcpu is in a guest mode

io thread | vcpu
accept_apic_interrupt() |
coalesced = 0, PIR=1 |
vcpu in a guest mode: |
send PIR IPI |
| receive PIR IPI
| merge PIR to IRR (*)
return not coalesced |

At (*) interrupt that was reported as delivered is coalesced.

The point is that we need to check PIR and IRR atomically and this is
impossible.

>
> Other sites:
> A: On VM-entry, after disabling interrupts, but before
> the last check for ->requests, clear pir notification bit
> (unconditionally).
>
> (*) This is _necessary_ also because during VM-exit a PIR IPI interrupt can
> be missed, so the KVM_REQ_EVENT indicates that SW is responsible for
> PIR->IRR transfer.
>

--
Gleb.