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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 1 Feb 2019 05:44:08 +0000
From:   "Kang, Luwei" <luwei.kang@...el.com>
To:     Paolo Bonzini <pbonzini@...hat.com>,
        "rkrcmar@...hat.com" <rkrcmar@...hat.com>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "bp@...en8.de" <bp@...en8.de>, "hpa@...or.com" <hpa@...or.com>,
        "x86@...nel.org" <x86@...nel.org>
CC:     "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH V3] KVM: x86: Sync the pending Posted-Interrupts

> > 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@...el.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