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

[Yosry Ahmed]

> > 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.

Hmm actually I just realized I missed a CAP check for
KVM_CAP_NESTED_STATE here. I don't think it's worth looking into
dropping KVM_CAP_EXCEPTION_PAYLOAD, because we will need a CAP check
anyway, and both of these were introduced very close in time (4.19 and
4.20, both 8 years ago).

So I think the simplest thing to do here is just run the nested test
cases if both CAPs are available and skip it otherwise. Practically, I
doubt there will be a lot of people running upstream selftests on
<4.20 kernels for this to actually matter.

>
> > >         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.

Will do that.