linux-kernel - [PATCH v3 2/4] x86: KVM: AVIC: stop using 'is_running' bit in avic_vcpu

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Mon,  2 Oct 2023 14:57:21 +0300
From:   Maxim Levitsky <mlevitsk@...hat.com>
To:     kvm@...r.kernel.org
Cc:     Will Deacon <will@...nel.org>, linux-kernel@...r.kernel.org,
        Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
        Ingo Molnar <mingo@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Joerg Roedel <joro@...tes.org>,
        Suravee Suthikulpanit <suravee.suthikulpanit@....com>,
        Sean Christopherson <seanjc@...gle.com>,
        Maxim Levitsky <mlevitsk@...hat.com>,
        Robin Murphy <robin.murphy@....com>, iommu@...ts.linux.dev,
        Paolo Bonzini <pbonzini@...hat.com>
Subject: [PATCH v3 2/4] x86: KVM: AVIC: stop using 'is_running' bit in avic_vcpu_put()

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 <mlevitsk@...hat.com>
---
 arch/x86/kvm/svm/avic.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

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);

-- 
2.26.3