Re: [PATCH v1 2/4] KVM: nSVM: Delay stuffing L2's current RIP into NextRIP until vCPU run
From: Yosry Ahmed
Date: Tue Feb 24 2026 - 20:15:38 EST
On Tue, Feb 24, 2026 at 5:10 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
>
> On Tue, Feb 24, 2026, Yosry Ahmed wrote:
> > > > Doing this in svm_prepare_switch_to_guest() is wrong, or at least
> > > > after the svm->guest_state_loaded check. It's possible to emulate the
> > > > nested VMRUN without doing a vcpu_put(), which means
> > > > svm->guest_state_loaded will remain true and this code will be
> > > > skipped.
> > > >
> > > > In fact, this breaks the svm_nested_soft_inject_test test. Funny
> > > > enough, I was only running it with my repro changes, which papered
> > > > over the bug because it forced an exit to userspace after VMRUN due to
> > > > single-stepping, so svm->guest_state_loaded got cleared and the code
> > > > was executed on the next KVM_RUN, before L2 runs.
> > > >
> > > > I can move it above the svm->guest_state_loaded check, but I think I
> > > > will just put it in pre_svm_run() instead.
> > >
> > > I would rather not expand pre_svm_run(), and instead just open code it in
> > > svm_vcpu_run(). pre_svm_run() probably should never have been added, because
> > > it's far from a generic "pre run" API. E.g. if we want to keep the helper around,
> > > it should probably be named something something ASID.
> >
> > I sent a new version before I saw your response.. sorry.
> >
> > How strongly do you feel about this? :P
>
> Strong enough that I'll fix it up when applying, unless it's a sticking point on
> your end.
It's just that 99% of the time someone is reading svm_vcpu_run(), they
won't care about this code, and it's also cognitive load to filter it
out. We can add a helper for this code (and the soft IRQ inject),
something like svm_fixup_nested_rips() or sth.
We discussed a helper before and you didn't like it, but that was in a
different context (a helper that combined normal and special cases).
WDYT?
>
> E.g. one thing that I don't love is that the code never runs for SEV guests.
> Which is fine, because in practice I can't imagine KVM ever supporting nested
> SVM for SEV, but it adds unnecessary cognitive load, because readers need to
> reason through why the code only applies to !SEV guests.