[PATCH 2/2] KVM: x86: Clear pv_unhalted on all transitions to KVM_MP_STATE_RUNNABLE

Jim Mattson posted 2 patches 11 months, 1 week ago
[PATCH 2/2] KVM: x86: Clear pv_unhalted on all transitions to KVM_MP_STATE_RUNNABLE
Posted by Jim Mattson 11 months, 1 week ago
In kvm_set_mp_state(), ensure that vcpu->arch.pv.pv_unhalted is always
cleared on a transition to KVM_MP_STATE_RUNNABLE, so that the next HLT
instruction will be respected.

The "fixes" list may be incompplete.

Fixes: 6aef266c6e17 ("kvm hypervisor : Add a hypercall to KVM hypervisor to support pv-ticketlocks")
Fixes: b6b8a1451fc4 ("KVM: nVMX: Rework interception of IRQs and NMIs")
Fixes: 38c0b192bd6d ("KVM: SVM: leave halted state on vmexit")
Signed-off-by: Jim Mattson <jmattson@google.com>
---
 arch/x86/kvm/svm/sev.c | 1 -
 arch/x86/kvm/x86.c     | 1 -
 arch/x86/kvm/x86.h     | 2 ++
 3 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index b4d9efd7537d..73e15e7658c2 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -3869,7 +3869,6 @@ static int __sev_snp_update_protected_guest_state(struct kvm_vcpu *vcpu)
 		svm->vmcb->control.vmsa_pa = pfn_to_hpa(pfn);
 
 		/* Mark the vCPU as runnable */
-		vcpu->arch.pv.pv_unhalted = false;
 		kvm_set_mp_state(vcpu, KVM_MP_STATE_RUNNABLE);
 
 		svm->sev_es.snp_vmsa_gpa = INVALID_PAGE;
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index d6679df95a75..4b21ed6803c6 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -11207,7 +11207,6 @@ static inline int vcpu_block(struct kvm_vcpu *vcpu)
 	switch(vcpu->arch.mp_state) {
 	case KVM_MP_STATE_HALTED:
 	case KVM_MP_STATE_AP_RESET_HOLD:
-		vcpu->arch.pv.pv_unhalted = false;
 		kvm_set_mp_state(vcpu, KVM_MP_STATE_RUNNABLE);
 		fallthrough;
 	case KVM_MP_STATE_RUNNABLE:
diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index bc3b5a9490c6..cc06631027bd 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -124,6 +124,8 @@ static inline bool kvm_vcpu_has_run(struct kvm_vcpu *vcpu)
 static inline void kvm_set_mp_state(struct kvm_vcpu *vcpu, int mp_state)
 {
 	vcpu->arch.mp_state = mp_state;
+	if (mp_state == KVM_MP_STATE_RUNNABLE)
+		vcpu->arch.pv.pv_unhalted = false;
 }
 
 static inline bool kvm_is_exception_pending(struct kvm_vcpu *vcpu)
-- 
2.47.1.688.g23fc6f90ad-goog
Re: [PATCH 2/2] KVM: x86: Clear pv_unhalted on all transitions to KVM_MP_STATE_RUNNABLE
Posted by Jim Mattson 10 months, 2 weeks ago
On Mon, Jan 13, 2025 at 12:02 PM Jim Mattson <jmattson@google.com> wrote:
>
> In kvm_set_mp_state(), ensure that vcpu->arch.pv.pv_unhalted is always
> cleared on a transition to KVM_MP_STATE_RUNNABLE, so that the next HLT
> instruction will be respected.
>
> The "fixes" list may be incomplete.

The only commit I'm not sure of is commit 1a65105a5aba ("KVM: x86/xen:
handle PV spinlocks slowpath"). That commit introduces an mp_state
transition to KVM_MP_STATE_RUNNABLE  without clearing pv_unhalted, so
perhaps it should be in the "fixes" list. OTOH, this seems to be an
independent implementation of PV spinlocks, so maybe it's not a
problem.

> Fixes: 6aef266c6e17 ("kvm hypervisor : Add a hypercall to KVM hypervisor to support pv-ticketlocks")
> Fixes: b6b8a1451fc4 ("KVM: nVMX: Rework interception of IRQs and NMIs")
> Fixes: 38c0b192bd6d ("KVM: SVM: leave halted state on vmexit")
> Signed-off-by: Jim Mattson <jmattson@google.com>
Re: [PATCH 2/2] KVM: x86: Clear pv_unhalted on all transitions to KVM_MP_STATE_RUNNABLE
Posted by Sean Christopherson 10 months, 1 week ago
On Wed, Feb 05, 2025, Jim Mattson wrote:
> On Mon, Jan 13, 2025 at 12:02 PM Jim Mattson <jmattson@google.com> wrote:
> >
> > In kvm_set_mp_state(), ensure that vcpu->arch.pv.pv_unhalted is always
> > cleared on a transition to KVM_MP_STATE_RUNNABLE, so that the next HLT
> > instruction will be respected.
> >
> > The "fixes" list may be incomplete.
> 
> The only commit I'm not sure of is commit 1a65105a5aba ("KVM: x86/xen:
> handle PV spinlocks slowpath"). That commit introduces an mp_state
> transition to KVM_MP_STATE_RUNNABLE  without clearing pv_unhalted, so
> perhaps it should be in the "fixes" list. OTOH, this seems to be an
> independent implementation of PV spinlocks, so maybe it's not a
> problem.

I'll add it to the list, and drop the "list may be incomplete" line.
KVM_HC_KICK_CPU is unreachable if Xen hypercalls are enabled, but nothing would
prevent a clever guest from sending an virtual IPI with APIC_DM_REMRD.  Whether
or not that makes the Xen code a KVM bug is definitely debatable, but I can't
imagine will care about Fixes being slightly overzealous.