Messages in this thread | | | Date | Thu, 9 Dec 2021 15:11:17 +0100 | Subject | Re: [PATCH 3/6] KVM: SVM: fix AVIC race of host->guest IPI delivery vs AVIC inhibition | From | Paolo Bonzini <> |
| |
On 12/9/21 12:54, Maxim Levitsky wrote: > If svm_deliver_avic_intr is called just after the target vcpu's AVIC got > inhibited, it might read a stale value of vcpu->arch.apicv_active > which can lead to the target vCPU not noticing the interrupt. > > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> > --- > arch/x86/kvm/svm/avic.c | 16 +++++++++++++--- > 1 file changed, 13 insertions(+), 3 deletions(-) > > diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c > index 859ad2dc50f1..8c1b934bfa9b 100644 > --- a/arch/x86/kvm/svm/avic.c > +++ b/arch/x86/kvm/svm/avic.c > @@ -691,6 +691,15 @@ int svm_deliver_avic_intr(struct kvm_vcpu *vcpu, int vec) > * automatically process AVIC interrupts at VMRUN. > */ > if (vcpu->mode == IN_GUEST_MODE) { > + > + /* > + * At this point we had read the vcpu->arch.apicv_active == true > + * and the vcpu->mode == IN_GUEST_MODE. > + * Since we have a memory barrier after setting IN_GUEST_MODE, > + * it ensures that AVIC inhibition is complete and thus > + * the target is really running with AVIC enabled. > + */ > + > int cpu = READ_ONCE(vcpu->cpu);
I don't think it's correct. The vCPU has apicv_active written (in kvm_vcpu_update_apicv) before vcpu->mode.
For the acquire/release pair to work properly you need to 1) read apicv_active *after* vcpu->mode here 2) use store_release and load_acquire for vcpu->mode, respectively in vcpu_enter_guest and here.
Paolo
> /* > @@ -706,10 +715,11 @@ int svm_deliver_avic_intr(struct kvm_vcpu *vcpu, int vec) > put_cpu(); > } else { > /* > - * Wake the vCPU if it was blocking. KVM will then detect the > - * pending IRQ when checking if the vCPU has a wake event. > + * Kick the target vCPU otherwise, to make sure > + * it processes the interrupt even if its AVIC is inhibited. > */ > - kvm_vcpu_wake_up(vcpu); > + kvm_make_request(KVM_REQ_EVENT, vcpu); > + kvm_vcpu_kick(vcpu); > } > > return 0; >
| |