Skip to content

Commit

Permalink
KVM: SVM: Disable preemption across AVIC load/put during APICv refresh
Browse files Browse the repository at this point in the history
Disable preemption when loading/putting the AVIC during an APICv refresh.
If the vCPU task is preempted and migrated ot a different pCPU, the
unprotected avic_vcpu_load() could set the wrong pCPU in the physical ID
cache/table.

Pull the necessary code out of avic_vcpu_{,un}blocking() and into a new
helper to reduce the probability of introducing this exact bug a third
time.

Fixes: df7e482 ("KVM: SVM: call avic_vcpu_load/avic_vcpu_put when enabling/disabling AVIC")
Cc: stable@vger.kernel.org
Reported-by: Maxim Levitsky <mlevitsk@redhat.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
  • Loading branch information
Sean Christopherson authored and Paolo Bonzini committed Mar 1, 2022
1 parent aa9f584 commit b652de1
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 50 deletions.
101 changes: 55 additions & 46 deletions arch/x86/kvm/svm/avic.c
Original file line number Diff line number Diff line change
Expand Up @@ -616,38 +616,6 @@ static int avic_set_pi_irte_mode(struct kvm_vcpu *vcpu, bool activate)
return ret;
}

void avic_refresh_apicv_exec_ctrl(struct kvm_vcpu *vcpu)
{
struct vcpu_svm *svm = to_svm(vcpu);
struct vmcb *vmcb = svm->vmcb01.ptr;
bool activated = kvm_vcpu_apicv_active(vcpu);

if (!enable_apicv)
return;

if (activated) {
/**
* During AVIC temporary deactivation, guest could update
* APIC ID, DFR and LDR registers, which would not be trapped
* by avic_unaccelerated_access_interception(). In this case,
* we need to check and update the AVIC logical APIC ID table
* accordingly before re-activating.
*/
avic_apicv_post_state_restore(vcpu);
vmcb->control.int_ctl |= AVIC_ENABLE_MASK;
} else {
vmcb->control.int_ctl &= ~AVIC_ENABLE_MASK;
}
vmcb_mark_dirty(vmcb, VMCB_AVIC);

if (activated)
avic_vcpu_load(vcpu, vcpu->cpu);
else
avic_vcpu_put(vcpu);

avic_set_pi_irte_mode(vcpu, activated);
}

static void svm_ir_list_del(struct vcpu_svm *svm, struct amd_iommu_pi_data *pi)
{
unsigned long flags;
Expand Down Expand Up @@ -899,7 +867,7 @@ avic_update_iommu_vcpu_affinity(struct kvm_vcpu *vcpu, int cpu, bool r)
return ret;
}

void avic_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
void __avic_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
{
u64 entry;
/* ID = 0xff (broadcast), ID > 0xff (reserved) */
Expand Down Expand Up @@ -936,7 +904,7 @@ void avic_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
avic_update_iommu_vcpu_affinity(vcpu, h_physical_id, true);
}

void avic_vcpu_put(struct kvm_vcpu *vcpu)
void __avic_vcpu_put(struct kvm_vcpu *vcpu)
{
u64 entry;
struct vcpu_svm *svm = to_svm(vcpu);
Expand All @@ -955,13 +923,63 @@ void avic_vcpu_put(struct kvm_vcpu *vcpu)
WRITE_ONCE(*(svm->avic_physical_id_cache), entry);
}

static void avic_vcpu_load(struct kvm_vcpu *vcpu)
{
int cpu = get_cpu();

WARN_ON(cpu != vcpu->cpu);

__avic_vcpu_load(vcpu, cpu);

put_cpu();
}

static void avic_vcpu_put(struct kvm_vcpu *vcpu)
{
preempt_disable();

__avic_vcpu_put(vcpu);

preempt_enable();
}

void avic_refresh_apicv_exec_ctrl(struct kvm_vcpu *vcpu)
{
struct vcpu_svm *svm = to_svm(vcpu);
struct vmcb *vmcb = svm->vmcb01.ptr;
bool activated = kvm_vcpu_apicv_active(vcpu);

if (!enable_apicv)
return;

if (activated) {
/**
* During AVIC temporary deactivation, guest could update
* APIC ID, DFR and LDR registers, which would not be trapped
* by avic_unaccelerated_access_interception(). In this case,
* we need to check and update the AVIC logical APIC ID table
* accordingly before re-activating.
*/
avic_apicv_post_state_restore(vcpu);
vmcb->control.int_ctl |= AVIC_ENABLE_MASK;
} else {
vmcb->control.int_ctl &= ~AVIC_ENABLE_MASK;
}
vmcb_mark_dirty(vmcb, VMCB_AVIC);

if (activated)
avic_vcpu_load(vcpu);
else
avic_vcpu_put(vcpu);

avic_set_pi_irte_mode(vcpu, activated);
}

void avic_vcpu_blocking(struct kvm_vcpu *vcpu)
{
if (!kvm_vcpu_apicv_active(vcpu))
return;

preempt_disable();

/*
* Unload the AVIC when the vCPU is about to block, _before_
* the vCPU actually blocks.
Expand All @@ -976,21 +994,12 @@ void avic_vcpu_blocking(struct kvm_vcpu *vcpu)
* the cause of errata #1235).
*/
avic_vcpu_put(vcpu);

preempt_enable();
}

void avic_vcpu_unblocking(struct kvm_vcpu *vcpu)
{
int cpu;

if (!kvm_vcpu_apicv_active(vcpu))
return;

cpu = get_cpu();
WARN_ON(cpu != vcpu->cpu);

avic_vcpu_load(vcpu, cpu);

put_cpu();
avic_vcpu_load(vcpu);
}
4 changes: 2 additions & 2 deletions arch/x86/kvm/svm/svm.c
Original file line number Diff line number Diff line change
Expand Up @@ -1318,13 +1318,13 @@ static void svm_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
indirect_branch_prediction_barrier();
}
if (kvm_vcpu_apicv_active(vcpu))
avic_vcpu_load(vcpu, cpu);
__avic_vcpu_load(vcpu, cpu);
}

static void svm_vcpu_put(struct kvm_vcpu *vcpu)
{
if (kvm_vcpu_apicv_active(vcpu))
avic_vcpu_put(vcpu);
__avic_vcpu_put(vcpu);

svm_prepare_host_switch(vcpu);

Expand Down
4 changes: 2 additions & 2 deletions arch/x86/kvm/svm/svm.h
Original file line number Diff line number Diff line change
Expand Up @@ -576,8 +576,8 @@ void avic_init_vmcb(struct vcpu_svm *svm);
int avic_incomplete_ipi_interception(struct kvm_vcpu *vcpu);
int avic_unaccelerated_access_interception(struct kvm_vcpu *vcpu);
int avic_init_vcpu(struct vcpu_svm *svm);
void avic_vcpu_load(struct kvm_vcpu *vcpu, int cpu);
void avic_vcpu_put(struct kvm_vcpu *vcpu);
void __avic_vcpu_load(struct kvm_vcpu *vcpu, int cpu);
void __avic_vcpu_put(struct kvm_vcpu *vcpu);
void avic_apicv_post_state_restore(struct kvm_vcpu *vcpu);
void avic_set_virtual_apic_mode(struct kvm_vcpu *vcpu);
void avic_refresh_apicv_exec_ctrl(struct kvm_vcpu *vcpu);
Expand Down

0 comments on commit b652de1

Please sign in to comment.