RE: [PATCH V3] KVM: x86: Sync the pending Posted-Interrupts

Kang, Luwei
2019-02-01T05:44:08Z

> > Some Posted-Interrupts from passthrough devices may be lost or
> > overwritten when the vCPU is in runnable state.
> >
> > The SN (Suppress Notification) of PID (Posted Interrupt Descriptor)
> > will be set when the vCPU is preempted (vCPU in
> KVM_MP_STATE_RUNNABLE
> > state but not running on physical CPU). If a posted interrupt coming
> > at this time, the irq remmaping facility will set the bit of PIR
> > (Posted Interrupt Requests) without ON (Outstanding Notification).
> > So this interrupt can't be sync to APIC virtualization register and
> > will not be handled by Guest because ON is zero.
> >
> > Signed-off-by: Luwei Kang <luwei.kang@intel.com>
> > ---
> > arch/x86/kvm/vmx/vmx.c | 5 +++++
> > arch/x86/kvm/x86.c | 2 +-
> > 2 files changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index
> > 4341175..8ed9634 100644
> > --- a/arch/x86/kvm/vmx/vmx.c
> > +++ b/arch/x86/kvm/vmx/vmx.c
> > @@ -1221,6 +1221,11 @@ static void vmx_vcpu_pi_load(struct kvm_vcpu
> *vcpu, int cpu)
> > new.sn = 0;
> > } while (cmpxchg64(&pi_desc->control, old.control,
> > new.control) != old.control);
> > +
>
> /*
> * Clear SN before reading the bitmap. The VT-d firmware
> * writes the bitmap and reads SN atomically (5.2.3 in the
> * spec), so it doesn't really have a memory barrier that
> * pairs with this, but we cannot do that and we need one.
> */
>
> > + smp_mb__after_atomic();
> > +
> > + if (!bitmap_empty((unsigned long *)pi_desc->pir, NR_VECTORS))
> > + pi_test_and_set_on(pi_desc);
>
> You can add pi_set_on for use here. The fast path with pi_clear_sn should
> also be removed.

Do you mean remove the blow code in vmx_vcpu_pi_load() function to make the ON can be set if PIR is not zero?

--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -1192,21 +1192,6 @@ static void vmx_vcpu_pi_load(struct kvm_vcpu *vcpu, int cpu)
if (!pi_test_sn(pi_desc) && vcpu->cpu == cpu)
return;

- /*
- * First handle the simple case where no cmpxchg is necessary; just
- * allow posting non-urgent interrupts.
- *
- * If the 'nv' field is POSTED_INTR_WAKEUP_VECTOR, do not change
- * PI.NDST: pi_post_block will do it for us and the wakeup_handler
- * expects the VCPU to be on the blocked_vcpu_list that matches
- * PI.NDST.
- */
- if (pi_desc->nv == POSTED_INTR_WAKEUP_VECTOR ||
- vcpu->cpu == cpu) {
- pi_clear_sn(pi_desc);
- return;
- }

Thanks,
Luwei Kang