Re: [PATCH v4 4/6] KVM: x86/pmu: Re-evaluate Host-Only/Guest-Only on nested SVM transitions

Sean Christopherson
2026-04-22T22:42:11Z

On Tue, Apr 21, 2026, Yosry Ahmed wrote:
> On Thu, Apr 09, 2026 at 02:21:14PM -0700, Sean Christopherson wrote:
> > On Thu, Apr 09, 2026, Sean Christopherson wrote:
> > > On Thu, Apr 09, 2026, Jim Mattson wrote:
> > > > On Thu, Apr 9, 2026 at 10:48 AM Sean Christopherson <seanjc@google.com> wrote:
> > > > > On Thu, Apr 09, 2026, Jim Mattson wrote:
> > > > > > > > In general, this deferral is misguided. The G/H bits should be
> > > > > > > > re-evaluated before we call kvm_pmu_instruction_retired() for an
> > > > > > > > emulated instruction.

...

> > > > > > > > This happens too late for VMRUN, since we have already called
> > > > > > > > kvm_pmu_instruction_retired() via kvm_skip_emulated_instruction(), and
> > > > > > > > VMRUN counts as a *guest* instruction.
> > > > > > >
> > > > > > > It's just VMRUN that's problematic though, correct? I.e. the scheme as a whole
> > > > > > > is fine, we just need to special case VMRUN due to SVM's erratum^Warchitecture.
> > > > > > > Alternatively, maybe we could get AMD to document the silly VMRUN behavior as an
> > > > > > > erratum, then we could claim KVM is architecturally superior. :-D
> > > > > >
> > > > > > Here, it's just VMRUN. Above, it's WRMSR(EFER).
> > > > >
> > > > > But clearing EFER.SVME while in the guest generates architecturally undefined
> > > > > behavior. I don't see any reason to complicate PMU virtualization for that
> > > > > scenario, especially now that KVM synthesizes triple fault for L1.
> > > >
> > > > L1 can clear the virtual EFER.SVME. That is well-defined.
> > >
> > > Gah, I forgot that the H/G bits are ignored when EFER.SVME=0. That's really
> > > annoying.
> >
> > What do you think about having two flavors of kvm_pmu_handle_nested_transition()?
> > One that defers via a request, and a "special" (SVM-only?) version that does
> > direct updates.
> >
> > Poking into PMU state in arbitrary contexts makes me nervous. E.g. when calling
> > svm_leave_nested(), odds are good EFER isn't even correct, and the update *needs*
> > to be deferred.
>
> Hmm is it really that bad?

It's not horrible, but it's a lot of "I think" and "should" and whatnot. I
generally agree that it's unlikely to be a problem, but I can point at far too
many bugs where KVM unexpectedly invokes a helper and consumes stale state.

I'm not completely opposed to non-deferred updates, but I really don't want to
use them for svm_leave_nested().

> - In the emulated VMRUN and #VMEXIT paths, EFER.SVME should be set in
> both L1 and L2, so it should be fine.
>
> - In the restore path entering guest mode, EFER.SVME should also be set
> in both L1 and L2.
>
> So I think svm_leave_nested() is the only interesting case:
>
> - In the restore path, svm_leave_nested() should only be called if the
> CPU is in guest mode and EFER.SVME is set in both L1 and L2.
>
> - In the EFER update path, L1 will get a shutdown if we forcefully leave
> nested anyway, unless userspace is setting state. Either way, the
> value of EFER should be correct and valid to use to update the PMU
> here.
>
> - In the vCPU free path, it shouldn't really matter, but the value of
> EFER should still be correct.

> So I *think* generally the value of EFER should be fine to use. The
> other inputs are is_guest_mode() and eventsel. In both cases we should
> just make sure to update the PMU *after* updating the state.
>
> So I think we'd end up with something similar to Jim's v2:
> https://lore.kernel.org/kvm/20260129232835.3710773-1-jmattson@google.com/
>
> We will directly re-evaluate eventsel_hw on nested transitions, EFER
> updates, and PMU MSR updates -- without deferring anything.
>
> We'd still need to make other changes:
> - Always disable the PMC if EFER.SVME is clear and either H/G bit (or
> both) is set.
>
> - Handle VMRUN correctly. I was going to suggest just moving the call to
> kvm_skip_emulated_instruction() to the end of the function, but that
> doesn't handle the case where we inject #VMEXIT(INVALID) due to a
> VMRUN failure (e.g. consistency checks, loading CR3, etc).
>
> I am actually not sure if the instruction should count in host or
> guest mode in this case. Logically, we never entered the guest, so
> perhaps counting it in host mode is the right thing to do? I think
> we'll also need to test what HW does.
>
> Honestly, it would be a lot easier of someone from AMD could just tell
> us these things :)
>
> Basically:
> - Does the PMU generally count based on processor state (e.g. guest
> mode, EFER.SVME) before or after instruction retirement?
> - A successful VMRUN will be counted in guest mode, what about a
> failed VMRUN that produces #VMEXIT(INVALID)?
>
> > I definitely don't love having two separate update mechanisms, but it seems like
> > the safest option in this case.
>
> Same here, and I like the deferred handling, but to Jim's point I think
> we can use it anywhere :/

Why can't we defer the svm_leave_nested() case? The only flows the invoke
svm_leave_nested() are non-architectural, being precise there doesn't matter at
all (and I'm not convinced it matters in general given none of us can figure
out what hardware is _supposed_ to do).

Having a synchronous path for architectural flows, and a deferred mechanism for
everything else seems reasonable, and would all but eliminate my concerns about
consuming stale state and/or doing things like attempting to write MSRs while
freeing a vCPU.