This is a note to let you know that I've just added the patch titled
KVM: SVM: Take and hold ir_list_lock when updating vCPU's Physical ID entry
to the 6.5-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
kvm-svm-take-and-hold-ir_list_lock-when-updating-vcpu-s-physical-id-entry.patch
and it can be found in the queue-6.5 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.
>From 4c08e737f056fec930b416a2bd37ed266d724f95 Mon Sep 17 00:00:00 2001
From: Sean Christopherson <seanjc@xxxxxxxxxx>
Date: Tue, 8 Aug 2023 16:31:31 -0700
Subject: KVM: SVM: Take and hold ir_list_lock when updating vCPU's Physical ID entry
From: Sean Christopherson <seanjc@xxxxxxxxxx>
commit 4c08e737f056fec930b416a2bd37ed266d724f95 upstream.
Hoist the acquisition of ir_list_lock from avic_update_iommu_vcpu_affinity()
to its two callers, avic_vcpu_load() and avic_vcpu_put(), specifically to
encapsulate the write to the vCPU's entry in the AVIC Physical ID table.
This will allow a future fix to pull information from the Physical ID entry
when updating the IRTE, without potentially consuming stale information,
i.e. without racing with the vCPU being (un)loaded.
Add a comment to call out that ir_list_lock does NOT protect against
multiple writers, specifically that reading the Physical ID entry in
avic_vcpu_put() outside of the lock is safe.
To preserve some semblance of independence from ir_list_lock, keep the
READ_ONCE() in avic_vcpu_load() even though acuiring the spinlock
effectively ensures the load(s) will be generated after acquiring the
lock.
Cc: stable@xxxxxxxxxxxxxxx
Tested-by: Alejandro Jimenez <alejandro.j.jimenez@xxxxxxxxxx>
Reviewed-by: Joao Martins <joao.m.martins@xxxxxxxxxx>
Link: https://lore.kernel.org/r/20230808233132.2499764-2-seanjc@xxxxxxxxxx
Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
---
arch/x86/kvm/svm/avic.c | 31 +++++++++++++++++++++++--------
1 file changed, 23 insertions(+), 8 deletions(-)
--- a/arch/x86/kvm/svm/avic.c
+++ b/arch/x86/kvm/svm/avic.c
@@ -986,10 +986,11 @@ static inline int
avic_update_iommu_vcpu_affinity(struct kvm_vcpu *vcpu, int cpu, bool r)
{
int ret = 0;
- unsigned long flags;
struct amd_svm_iommu_ir *ir;
struct vcpu_svm *svm = to_svm(vcpu);
+ lockdep_assert_held(&svm->ir_list_lock);
+
if (!kvm_arch_has_assigned_device(vcpu->kvm))
return 0;
@@ -997,19 +998,15 @@ avic_update_iommu_vcpu_affinity(struct k
* Here, we go through the per-vcpu ir_list to update all existing
* interrupt remapping table entry targeting this vcpu.
*/
- spin_lock_irqsave(&svm->ir_list_lock, flags);
-
if (list_empty(&svm->ir_list))
- goto out;
+ return 0;
list_for_each_entry(ir, &svm->ir_list, node) {
ret = amd_iommu_update_ga(cpu, r, ir->data);
if (ret)
- break;
+ return ret;
}
-out:
- spin_unlock_irqrestore(&svm->ir_list_lock, flags);
- return ret;
+ return 0;
}
void avic_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
@@ -1017,6 +1014,7 @@ void avic_vcpu_load(struct kvm_vcpu *vcp
u64 entry;
int h_physical_id = kvm_cpu_get_apicid(cpu);
struct vcpu_svm *svm = to_svm(vcpu);
+ unsigned long flags;
lockdep_assert_preemption_disabled();
@@ -1033,6 +1031,8 @@ void avic_vcpu_load(struct kvm_vcpu *vcp
if (kvm_vcpu_is_blocking(vcpu))
return;
+ spin_lock_irqsave(&svm->ir_list_lock, flags);
+
entry = READ_ONCE(*(svm->avic_physical_id_cache));
WARN_ON_ONCE(entry & AVIC_PHYSICAL_ID_ENTRY_IS_RUNNING_MASK);
@@ -1042,25 +1042,40 @@ void avic_vcpu_load(struct kvm_vcpu *vcp
WRITE_ONCE(*(svm->avic_physical_id_cache), entry);
avic_update_iommu_vcpu_affinity(vcpu, h_physical_id, true);
+
+ spin_unlock_irqrestore(&svm->ir_list_lock, flags);
}
void avic_vcpu_put(struct kvm_vcpu *vcpu)
{
u64 entry;
struct vcpu_svm *svm = to_svm(vcpu);
+ unsigned long flags;
lockdep_assert_preemption_disabled();
+ /*
+ * Note, reading the Physical ID entry outside of ir_list_lock is safe
+ * as only the pCPU that has loaded (or is loading) the vCPU is allowed
+ * to modify the entry, and preemption is disabled. I.e. the vCPU
+ * can't be scheduled out and thus avic_vcpu_{put,load}() can't run
+ * recursively.
+ */
entry = READ_ONCE(*(svm->avic_physical_id_cache));
/* Nothing to do if IsRunning == '0' due to vCPU blocking. */
if (!(entry & AVIC_PHYSICAL_ID_ENTRY_IS_RUNNING_MASK))
return;
+ spin_lock_irqsave(&svm->ir_list_lock, flags);
+
avic_update_iommu_vcpu_affinity(vcpu, -1, 0);
entry &= ~AVIC_PHYSICAL_ID_ENTRY_IS_RUNNING_MASK;
WRITE_ONCE(*(svm->avic_physical_id_cache), entry);
+
+ spin_unlock_irqrestore(&svm->ir_list_lock, flags);
+
}
void avic_refresh_virtual_apic_mode(struct kvm_vcpu *vcpu)
Patches currently in stable-queue which might be from seanjc@xxxxxxxxxx are
queue-6.5/perf-header-fix-missing-pmu-caps.patch
queue-6.5/kvm-svm-don-t-inject-ud-if-kvm-attempts-to-skip-sev-guest-insn.patch
queue-6.5/kvm-nsvm-check-instead-of-asserting-on-nested-tsc-scaling-support.patch
queue-6.5/drm-i915-gvt-verify-pfn-is-valid-before-dereferencin.patch
queue-6.5/kvm-nsvm-load-l1-s-tsc-multiplier-based-on-l1-state-not-l2-state.patch
queue-6.5/x86-virt-drop-unnecessary-check-on-extended-cpuid-le.patch
queue-6.5/kvm-svm-get-source-vcpus-from-source-vm-for-sev-es-intrahost-migration.patch
queue-6.5/kvm-svm-skip-vmsa-init-in-sev_es_init_vmcb-if-pointer-is-null.patch
queue-6.5/kvm-svm-take-and-hold-ir_list_lock-when-updating-vcpu-s-physical-id-entry.patch
queue-6.5/kvm-svm-set-target-pcpu-during-irte-update-if-target-vcpu-is-running.patch
queue-6.5/perf-lock-don-t-pass-an-err_ptr-directly-to-perf_ses.patch
queue-6.5/drm-i915-gvt-put-the-page-reference-obtained-by-kvm-.patch
queue-6.5/drm-i915-gvt-drop-unused-helper-intel_vgpu_reset_gtt.patch
queue-6.5/kvm-vmx-refresh-available-regs-and-idt-vectoring-info-before-nmi-handling.patch
queue-6.5/kvm-svm-don-t-defer-nmi-unblocking-until-next-exit-f.patch
