Re: [PATCH v3 10/10] KVM: selftests: Trigger L2->L1 exits stress save+restore and #PF test

[Sean Christopherson <seanjc@google.com>]

On Fri, Jul 24, 2026, Yosry Ahmed wrote:
> On Fri, Jul 24, 2026 at 11:25 AM Sean Christopherson <seanjc@google.com> wrote:
> >
> > On Fri, Jul 24, 2026, Yosry Ahmed wrote:
> > > On Fri, Jul 24, 2026 at 10:50 AM Sean Christopherson <seanjc@google.com> wrote:
> > > > >  static bool parse_args_nested(int argc, char *argv[])
> > > > >  {
> > > > >       bool nested = false;
> > > > > @@ -192,10 +218,13 @@ int main(int argc, char *argv[])
> > > > >       gva_t gva;
> > > > >       u64 pte;
> > > > >
> > > > > +     TEST_REQUIRE(kvm_has_cap(KVM_CAP_EXCEPTION_PAYLOAD));
> > > >
> > > > But KVM_CAP_EXCEPTION_PAYLOAD _isn't_ required, it's an optional feature.  Actually,
> > > > this is ridiculous.  The test is injecting a #UD, it doesn't have a payload.
> > > > Bad AI, bad.
> > >
> > > KVM_CAP_EXCEPTION_PAYLOAD is required to inject a *pending* exception,
> > > which is needed as KVM won't check for interception on already
> > > injected exceptions. I will add a comment.
> >
> > Drop the TEST_REQUIRE(), and instead only inject the pending #UD if the CAP
> > is supported.
> 
> I prefer to keep the TEST_REQUIRE() as I don't like the test coverage
> silently changing based on the available caps tbh.

That ship already sailed by agreeing to make nested support conditional :-)

> Does it matter in practice? IIUC it's only possible for KVM_CAP_EXCEPTION_PAYLOAD
> to not exist if the test is run on an older kernel, right?

Correct, but people do run upstream selftests on older kernels, myself included.
We do effectively break/drop backwards compatibility if there's sufficient
justification, e.g. KVM_SET_USER_MEMORY_REGION2 is required for tests that create
memslots so that selftests doesn't have to carry fallback code, but I don't think
this one-off test qualifies for exemption.

> > > > > +
> > > > >       nested = parse_args_nested(argc, argv);
> > > > >
> > > > >       vm = vm_create_with_one_vcpu(&vcpu, nested ? l1_guest_code : guest_access_memory);
> > > > >       vm_install_exception_handler(vm, PF_VECTOR, guest_pf_handler);
> > > > > +     vm_enable_cap(vm, KVM_CAP_EXCEPTION_PAYLOAD, -2ul);
> > > >
> > > > -2ul?
> > >
> > > That was actually me, not the AI. Apparently the other selftests also
> > > write -2ul for some reason. I was too lazy to go change them and/or
> > > figure out why -2ul, it didn't make any sense to me. I chose the lazy
> > > option and just used the same thing here.
> > >
> > > Someone named Sean Christopherson wrote the other two, maybe we can ask him? :P
> >
> > Nah, that guy's an asshole, not worth your time.
> >
> > > > >       if (nested) {
> > > > >               TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_SVM) || kvm_cpu_has(X86_FEATURE_VMX));
> > > > > @@ -270,8 +299,16 @@ int main(int argc, char *argv[])
> > > > >
> > > > >               state = vcpu_save_state(vcpu);
> > > > >
> > > > > +             /*
> > > > > +              * If the vCPU is in guest mode, inject a #UD to trigger an
> > > > > +              * L2->L1 VM-Exit every other iteration.
> > > > > +              */
> > > > > +             if (nested && vcpu_state_is_guest_mode(state) && count % 2 == 0)
> > > >
> > > > Checking "nested" here is unnecessary.
> > >
> > > It is, actually. Sashiko pointed out [1] that
> > > vcpu_state_is_guest_mode() will read uninitialized memory otherwise.
> > >
> > > [1]https://sashiko.dev/#/patchset/20260518202514.2037078-1-yosry%40kernel.org?part=8
> >
> > Oof.  That's subtle.  At the risk of true evil, what if we did:
> 
> As I mentioned above, I prefer taking a dependency on
> KVM_CAP_EXCEPTION_PAYLOAD rather than varying coverage. At least for
> the nested case, I think I can move the TEST_REQUIRE() into the
> 'nested' case, but then it's probably better to leave it as an arg to
> allow for meaningfully running the test in L0.

The problem is that TEST_REQUIRE() skips the whole test, regardless of when it's
called.  I.e. if you go that route, passes for the non-nested case will show up
as skips.  TEST_REQUIRE() really is intended to be used when something is a hard
requirement.

> Actually, what if we just skip the nested case completely if
> KVM_CAP_EXCEPTION_PAYLOAD isn't available?

Can you use KVM_NMI instead of KVM_SET_VCPU_EVENTS to effect the nested #VMEXIT?
That ioctl is so ancient that I'm a-ok skipping any CAP check for it.

> >         int inject_ud;
> >
> >         /*
> >          * Big comment here, explaining both the CAP dependency and the need to
> >          * check for nested before querying guest state.
> >          */
> >         inject_ud = nested && kvm_has_cap(KVM_CAP_EXCEPTION_PAYLOAD);
> 
> Either way yeah I think we should document why checking 'nested' is needed.
> 
> Or.. what if we just check state->nested.size inside
> kvm_x86_state_is_guest_mode()? or explicitly clear state->nested in
> vcpu_save_state() if nested_size == 0?

I like both of those ideas.  If you do the former, definitely make
kvm_x86_state_is_guest_mode() a "public" API.