[v3,2/4] x86: KVM: AVIC: stop using 'is_running' bit in avic_vcpu_put()

Commit Message

Maxim Levitsky Oct. 2, 2023, 11:57 a.m. UTC

An optimization was added to avic_vcpu_load() in commit 782f64558de7
("KVM: SVM: Skip AVIC and IRTE updates when loading blocking vCPU")
to avoid re-enabling AVIC if the vCPU is about to block.

Such situation arises when a vCPU thread is preempted in between the call
to kvm_arch_vcpu_blocking() and before the matching call to
kvm_arch_vcpu_unblocking() which in case of AVIC disables/enables the AVIC
on this vCPU.

The same optimization was done in avic_vcpu_put() however the code was
based on physical id table's 'is_running' bit, building upon assumption
that if avic_vcpu_load() didn't set it, then kvm doesn't need to disable
avic (since it wasn't really enabled).

However, once AVIC's IPI virtualization is made optional, this bit
might be always false regardless if a vCPU is running or not.

To fix this, instead of checking this bit, check the same
'kvm_vcpu_is_blocking()' condition.

Also, as a bonus, re-enable the warning for already set 'is_running' bit,
if it was found set, during avic_vcpu_put() execution and the vCPU was not
blocking a condition which indicates a bug.

Signed-off-by: Maxim Levitsky <[e-mail]>
---
 arch/x86/kvm/svm/avic.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

Patch

diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
index 2092db892d7d052..4c75ca15999fcd4 100644
--- a/arch/x86/kvm/svm/avic.c
+++ b/arch/x86/kvm/svm/avic.c
@@ -1075,16 +1075,10 @@  void avic_vcpu_put(struct kvm_vcpu *vcpu)
 	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.
+	 * If the vcpu is blocking, there is no need to do anything.
+	 * See the comment in avic_vcpu_load().
 	 */
-	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))
+	if (kvm_vcpu_is_blocking(vcpu))
 		return;
 
 	/*
@@ -1099,6 +1093,9 @@  void avic_vcpu_put(struct kvm_vcpu *vcpu)
 
 	avic_update_iommu_vcpu_affinity(vcpu, -1, 0);
 
+	entry = READ_ONCE(*(svm->avic_physical_id_cache));
+	WARN_ON_ONCE(!(entry & AVIC_PHYSICAL_ID_ENTRY_IS_RUNNING_MASK));
+
 	entry &= ~AVIC_PHYSICAL_ID_ENTRY_IS_RUNNING_MASK;
 	WRITE_ONCE(*(svm->avic_physical_id_cache), entry);