All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Yosry Ahmed <yosry@kernel.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
	 stable@vger.kernel.org
Subject: Re: [PATCH v3 5/8] KVM: nSVM: Always use NextRIP as vmcb02's NextRIP after first L2 VMRUN
Date: Wed, 4 Mar 2026 09:39:51 -0800	[thread overview]
Message-ID: <aahuZ4bg4aQKTZYj@google.com> (raw)
In-Reply-To: <CAO9r8zN21twRarvzvq8euUOHRtVrO+q8jMaiip7NPtGgZ2dWGw@mail.gmail.com>

On Wed, Mar 04, 2026, Yosry Ahmed wrote:
> On Tue, Feb 24, 2026 at 5:00 PM Yosry Ahmed <yosry@kernel.org> wrote:
> >
> > For guests with NRIPS disabled, L1 does not provide NextRIP when running
> > an L2 with an injected soft interrupt, instead it advances the current RIP
> > before running it. KVM uses the current RIP as the NextRIP in vmcb02 to
> > emulate a CPU without NRIPS.
> >
> > However, after L2 runs the first time, NextRIP will be updated by the
> > CPU and/or KVM, and the current RIP is no longer the correct value to
> > use in vmcb02.  Hence, after save/restore, use the current RIP if and
> > only if a nested run is pending, otherwise use NextRIP.
> >
> > Fixes: cc440cdad5b7 ("KVM: nSVM: implement KVM_GET_NESTED_STATE and KVM_SET_NESTED_STATE")
> > CC: stable@vger.kernel.org
> > Signed-off-by: Yosry Ahmed <yosry@kernel.org>
> > ---
> >  arch/x86/kvm/svm/nested.c | 25 ++++++++++++++++---------
> >  1 file changed, 16 insertions(+), 9 deletions(-)
> >
> > diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> > index 9909ff237e5ca..f3ed1bdbe76c9 100644
> > --- a/arch/x86/kvm/svm/nested.c
> > +++ b/arch/x86/kvm/svm/nested.c
> > @@ -845,17 +845,24 @@ static void nested_vmcb02_prepare_control(struct vcpu_svm *svm,
> >         vmcb02->control.event_inj_err       = svm->nested.ctl.event_inj_err;
> >
> >         /*
> > -        * next_rip is consumed on VMRUN as the return address pushed on the
> > +        * NextRIP is consumed on VMRUN as the return address pushed on the
> >          * stack for injected soft exceptions/interrupts.  If nrips is exposed
> > -        * to L1, take it verbatim from vmcb12.  If nrips is supported in
> > -        * hardware but not exposed to L1, stuff the actual L2 RIP to emulate
> > -        * what a nrips=0 CPU would do (L1 is responsible for advancing RIP
> > -        * prior to injecting the event).
> > +        * to L1, take it verbatim from vmcb12.
> > +        *
> > +        * If nrips is supported in hardware but not exposed to L1, stuff the
> > +        * actual L2 RIP to emulate what a nrips=0 CPU would do (L1 is
> > +        * responsible for advancing RIP prior to injecting the event). This is
> > +        * only the case for the first L2 run after VMRUN. After that (e.g.
> > +        * during save/restore), NextRIP is updated by the CPU and/or KVM, and
> > +        * the value of the L2 RIP from vmcb12 should not be used.
> >          */
> > -       if (guest_cpu_cap_has(vcpu, X86_FEATURE_NRIPS))
> > -               vmcb02->control.next_rip    = svm->nested.ctl.next_rip;
> > -       else if (boot_cpu_has(X86_FEATURE_NRIPS))
> > -               vmcb02->control.next_rip    = vmcb12_rip;
> > +       if (boot_cpu_has(X86_FEATURE_NRIPS)) {
> > +               if (guest_cpu_cap_has(vcpu, X86_FEATURE_NRIPS) ||
> > +                   !svm->nested.nested_run_pending)
> > +                       vmcb02->control.next_rip    = svm->nested.ctl.next_rip;
> > +               else
> > +                       vmcb02->control.next_rip    = vmcb12_rip;
> > +       }
> 
> This should probably also apply to soft_int_next_rip below the context
> lines. Otherwise after  patch 7 we keep it uninitialized if the guest
> doesn't have NRIPs and !nested_run_pending.

That's fine though, isn't it?  Because in that case, doesn't the soft int have to
comein through svm_update_soft_interrupt_rip()?  Ugh, no because nSVM migrates
control.event_inj.

IIUC, we want to end up with this?

diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 03b201fe9613..d12647080051 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -925,7 +925,8 @@ static void nested_vmcb02_prepare_control(struct vcpu_svm *svm)
         */
        if (is_evtinj_soft(vmcb02->control.event_inj)) {
                svm->soft_int_injected = true;
-               if (guest_cpu_cap_has(vcpu, X86_FEATURE_NRIPS))
+               if (guest_cpu_cap_has(vcpu, X86_FEATURE_NRIPS) ||
+                   !svm->nested.nested_run_pending)
                        svm->soft_int_next_rip = vmcb12_ctrl->next_rip;
        }

  reply	other threads:[~2026-03-04 17:39 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-25  0:59 [PATCH v3 0/8] KVM: nSVM: Save/restore fixes for (Next)RIP Yosry Ahmed
2026-02-25  0:59 ` [PATCH v3 1/8] KVM: nSVM: Sync NextRIP to cached vmcb12 after VMRUN of L2 Yosry Ahmed
2026-02-25  0:59 ` [PATCH v3 2/8] KVM: nSVM: Sync interrupt shadow " Yosry Ahmed
2026-02-27 17:53   ` Yosry Ahmed
2026-03-02 20:41     ` Sean Christopherson
2026-02-25  0:59 ` [PATCH v3 3/8] KVM: selftests: Extend state_test to check vGIF Yosry Ahmed
2026-02-25  0:59 ` [PATCH v3 4/8] KVM: selftests: Extend state_test to check next_rip Yosry Ahmed
2026-02-25  0:59 ` [PATCH v3 5/8] KVM: nSVM: Always use NextRIP as vmcb02's NextRIP after first L2 VMRUN Yosry Ahmed
2026-03-04 17:30   ` Yosry Ahmed
2026-03-04 17:39     ` Sean Christopherson [this message]
2026-03-04 17:41       ` Yosry Ahmed
2026-02-25  0:59 ` [PATCH v3 6/8] KVM: nSVM: Delay stuffing L2's current RIP into NextRIP until vCPU run Yosry Ahmed
2026-02-25  0:59 ` [PATCH v3 7/8] KVM: nSVM: Delay setting soft IRQ RIP tracking fields " Yosry Ahmed
2026-03-04 17:50   ` Yosry Ahmed
2026-03-04 18:34     ` Sean Christopherson
2026-03-04 18:39       ` Yosry Ahmed
2026-02-25  0:59 ` [PATCH v3 8/8] DO NOT MERGE: KVM: selftests: Reproduce nested RIP restore bug Yosry Ahmed
2026-03-05 17:08 ` [PATCH v3 0/8] KVM: nSVM: Save/restore fixes for (Next)RIP Sean Christopherson

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=aahuZ4bg4aQKTZYj@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@kernel.org \
    /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.