All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Yosry Ahmed <yosry.ahmed@linux.dev>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
	 stable@vger.kernel.org
Subject: Re: [PATCH v5 09/26] KVM: nSVM: Call enter_guest_mode() before switching to VMCB02
Date: Fri, 20 Feb 2026 17:12:52 -0800	[thread overview]
Message-ID: <aZkGlFwWeRx0ZGCV@google.com> (raw)
In-Reply-To: <20260206190851.860662-10-yosry.ahmed@linux.dev>

On Fri, Feb 06, 2026, Yosry Ahmed wrote:
> In preparation for moving more changes that rely on is_guest_mode()
> before switching to VMCB02, move entering guest mode a bit earlier.
> 
> Nothing between the new callsite(s) and the old ones rely on
> is_guest_mode(), so this should be safe.
> 
> No functional change intended.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Yosry Ahmed <yosry.ahmed@linux.dev>
> ---
>  arch/x86/kvm/svm/nested.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index 29069fc5e8cb..607d99172e2b 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -741,9 +741,6 @@ static void nested_vmcb02_prepare_control(struct vcpu_svm *svm,
>  
>  	nested_svm_transition_tlb_flush(vcpu);
>  
> -	/* Enter Guest-Mode */
> -	enter_guest_mode(vcpu);
> -
>  	/*
>  	 * Filled at exit: exit_code, exit_info_1, exit_info_2, exit_int_info,
>  	 * exit_int_info_err, next_rip, insn_len, insn_bytes.
> @@ -944,6 +941,8 @@ int enter_svm_guest_mode(struct kvm_vcpu *vcpu, u64 vmcb12_gpa,
>  
>  	WARN_ON(svm->vmcb == svm->nested.vmcb02.ptr);
>  
> +	enter_guest_mode(vcpu);
> +
>  	nested_svm_copy_common_state(svm->vmcb01.ptr, svm->nested.vmcb02.ptr);
>  
>  	svm_switch_vmcb(svm, &svm->nested.vmcb02);
> @@ -1890,6 +1889,7 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
>  	svm_copy_vmrun_state(&svm->vmcb01.ptr->save, save);
>  	nested_copy_vmcb_control_to_cache(svm, ctl);
>  
> +	enter_guest_mode(vcpu);
>  	svm_switch_vmcb(svm, &svm->nested.vmcb02);
>  	nested_vmcb02_prepare_control(svm, svm->vmcb->save.rip, svm->vmcb->save.cs.base);


LOL, guess what!  Today end's in 'y', which means there's a nSVM bug!  It's a
super minor one though, especially in the broader context, I just happened to
see it when looking at this patch.

As per 3f6821aa147b ("KVM: x86: Forcibly leave nested if RSM to L2 hits shutdown"),
shutdown on RSM is suppose to hit L1, not L2.  But if enter_svm_guest_mode() fails,
svm_leave_smm() bails without leaving guest code.  Syzkaller probably hasn't found
the bug because nested_run_pending doesn't get set, but it's still technically
wrong.

Of course, as the comment in emulator_leave_smm() says, the *entire* RSM flow is
wrong, because it's not a VM-Enter/VMRUN, it's somethign else entirely.

Anyways, I don't think there's anything to do in this series, but at some point
we should probably do:

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index a2452b8ec49d..5cc9ad9b750d 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -4877,13 +4877,15 @@ static int svm_leave_smm(struct kvm_vcpu *vcpu, const union kvm_smram *smram)
        vmcb12 = map.hva;
        nested_copy_vmcb_control_to_cache(svm, &vmcb12->control);
        nested_copy_vmcb_save_to_cache(svm, &vmcb12->save);
+
        ret = enter_svm_guest_mode(vcpu, smram64->svm_guest_vmcb_gpa, vmcb12, false);
-
        if (ret)
-               goto unmap_save;
+               goto leave_nested;
 
        svm->nested.nested_run_pending = 1;
 
+leave_nested:
+       svm_leave_nested(vcpu);
 unmap_save:
        kvm_vcpu_unmap(vcpu, &map_save);
 unmap_map:

  reply	other threads:[~2026-02-21  1:12 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-06 19:08 [PATCH v5 00/26] Nested SVM fixes, cleanups, and hardening Yosry Ahmed
2026-02-06 19:08 ` [PATCH v5 01/26] KVM: nSVM: Avoid clearing VMCB_LBR in vmcb12 Yosry Ahmed
2026-02-06 19:08 ` [PATCH v5 02/26] KVM: SVM: Switch svm_copy_lbrs() to a macro Yosry Ahmed
2026-02-06 19:08 ` [PATCH v5 03/26] KVM: SVM: Add missing save/restore handling of LBR MSRs Yosry Ahmed
2026-02-06 19:08 ` [PATCH v5 04/26] KVM: selftests: Add a test for LBR save/restore (ft. nested) Yosry Ahmed
2026-02-06 19:08 ` [PATCH v5 05/26] KVM: nSVM: Always inject a #GP if mapping VMCB12 fails on nested VMRUN Yosry Ahmed
2026-02-06 19:08 ` [PATCH v5 06/26] KVM: nSVM: Triple fault if mapping VMCB12 fails on nested #VMEXIT Yosry Ahmed
2026-02-24  0:35   ` Sean Christopherson
2026-02-24  0:51     ` Yosry Ahmed
2026-02-24  1:17       ` Sean Christopherson
2026-02-06 19:08 ` [PATCH v5 07/26] KVM: nSVM: Triple fault if restore host CR3 " Yosry Ahmed
2026-02-24  0:38   ` Sean Christopherson
2026-02-06 19:08 ` [PATCH v5 08/26] KVM: nSVM: Drop nested_vmcb_check_{save/control}() wrappers Yosry Ahmed
2026-02-06 19:08 ` [PATCH v5 09/26] KVM: nSVM: Call enter_guest_mode() before switching to VMCB02 Yosry Ahmed
2026-02-21  1:12   ` Sean Christopherson [this message]
2026-02-21  1:26     ` Jim Mattson
2026-02-21  9:06     ` Yosry Ahmed
2026-02-06 19:08 ` [PATCH v5 10/26] KVM: nSVM: Make nested_svm_merge_msrpm() return an errno Yosry Ahmed
2026-02-06 19:08 ` [PATCH v5 11/26] KVM: nSVM: Call nested_svm_merge_msrpm() from enter_svm_guest_mode() Yosry Ahmed
2026-02-06 19:08 ` [PATCH v5 12/26] KVM: nSVM: Call nested_svm_init_mmu_context() before switching to VMCB02 Yosry Ahmed
2026-02-06 19:08 ` [PATCH v5 13/26] KVM: nSVM: Refactor minimal #VMEXIT handling out of nested_svm_vmexit() Yosry Ahmed
2026-02-06 19:08 ` [PATCH v5 14/26] KVM: nSVM: Unify handling of VMRUN failures with proper cleanup Yosry Ahmed
2026-02-06 19:08 ` [PATCH v5 15/26] KVM: nSVM: Clear EVENTINJ field in VMCB12 on nested #VMEXIT Yosry Ahmed
2026-02-06 19:08 ` [PATCH v5 16/26] KVM: nSVM: Drop the non-architectural consistency check for NP_ENABLE Yosry Ahmed
2026-02-06 19:08 ` [PATCH v5 17/26] KVM: nSVM: Add missing consistency check for nCR3 validity Yosry Ahmed
2026-02-06 19:08 ` [PATCH v5 18/26] KVM: nSVM: Add missing consistency check for hCR0.PG and NP_ENABLE Yosry Ahmed
2026-02-06 19:08 ` [PATCH v5 19/26] KVM: nSVM: Add missing consistency check for EFER, CR0, CR4, and CS Yosry Ahmed
2026-02-06 19:08 ` [PATCH v5 20/26] KVM: nSVM: Add missing consistency check for event_inj Yosry Ahmed
2026-02-06 19:08 ` [PATCH v5 21/26] KVM: SVM: Rename vmcb->nested_ctl to vmcb->misc_ctl Yosry Ahmed
2026-02-06 19:08 ` [PATCH v5 22/26] KVM: SVM: Rename vmcb->virt_ext to vmcb->misc_ctl2 Yosry Ahmed
2026-02-06 19:08 ` [PATCH v5 23/26] KVM: nSVM: Cache all used fields from VMCB12 Yosry Ahmed
2026-02-06 19:08 ` [PATCH v5 24/26] KVM: nSVM: Restrict mapping VMCB12 on nested VMRUN Yosry Ahmed
2026-02-21  1:24   ` Sean Christopherson
2026-02-06 19:08 ` [PATCH v5 25/26] KVM: nSVM: Sanitize control fields copied from VMCB12 Yosry Ahmed
2026-02-23 23:15   ` Sean Christopherson
2026-02-24  1:02     ` Yosry Ahmed
2026-02-24  1:27       ` Sean Christopherson
2026-02-06 19:08 ` [PATCH v5 26/26] KVM: nSVM: Only copy SVM_MISC_ENABLE_NP from VMCB01's misc_ctl Yosry Ahmed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aZkGlFwWeRx0ZGCV@google.com \
    --to=seanjc@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=stable@vger.kernel.org \
    --cc=yosry.ahmed@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.