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
Subject: Re: [PATCH v3 10/10] KVM: selftests: Trigger L2->L1 exits stress save+restore and #PF test
Date: Fri, 24 Jul 2026 11:25:04 -0700 [thread overview]
Message-ID: <amOuALTiqI9u4tFh@google.com> (raw)
In-Reply-To: <CAO9r8zOy3qyW11ByE4o+VWn92rZJN_kbF3XJTt7qf3TnqXXuWg@mail.gmail.com>
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.
> > > +
> > > 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:
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);
...
if ((i & inject_ud) && kvm_x86_state_is_guest_mode(&state)
kvm_x86_state_inject_ud(&state);
> > > + vcpu_state_inject_ud(state);
> >
> > Honestly, I'd rather open code this whole thing, because this doesn't actually
> > inject a #UD. It _prepares_ state, but doesn't send that into KVM. E.g.
>
> It injects a #UD into the state. The function name and parameter
> should make it clear. I prefer the helper, but I won't die on this
> hill.
I'm ok with a helper, it's the "vcpu" part that I find misleading, because the
"vcpu" namespace in selftest typically means "send this command into a vCPU ioctl".
kvm_x86_state_xxx() appears to be the standard namespace, let's go with that?
next prev parent reply other threads:[~2026-07-24 18:25 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-29 18:37 [PATCH v3 00/10] KVM: selftests: Stress save+restore and #PF (ft. nested) Yosry Ahmed
2026-06-29 18:37 ` [PATCH v3 01/10] KVM: selftests: Move STR() and XSTR() definitions to test_util.h Yosry Ahmed
2026-07-24 15:26 ` Sean Christopherson
2026-06-29 18:37 ` [PATCH v3 02/10] KVM: selftests: Fix RAX and RFLAGS VMCB offsets when running L2 Yosry Ahmed
2026-06-29 18:37 ` [PATCH v3 03/10] KVM: selftests: Use an array for guest_regs (and fix offsets) Yosry Ahmed
2026-07-24 15:33 ` Sean Christopherson
2026-07-24 16:18 ` Yosry Ahmed
2026-07-24 17:03 ` Sean Christopherson
2026-07-24 17:44 ` Yosry Ahmed
2026-06-29 18:37 ` [PATCH v3 04/10] KVM: selftests: Move GPR load/save definitions outside of nSVM code Yosry Ahmed
2026-06-29 18:37 ` [PATCH v3 05/10] KVM: selftests: Reuse GPR switching logic for nVMX Yosry Ahmed
2026-06-29 18:49 ` sashiko-bot
2026-06-29 20:26 ` Yosry Ahmed
2026-06-29 18:37 ` [PATCH v3 06/10] KVM: selftests: Drop HORRIFIC_L2_UCALL_CLOBBER_HACK Yosry Ahmed
2026-07-24 15:37 ` Sean Christopherson
2026-07-24 16:20 ` Yosry Ahmed
2026-06-29 18:37 ` [PATCH v3 07/10] KVM: selftests: Add basic stress test for save+restore and #PF handling Yosry Ahmed
2026-07-24 16:45 ` Sean Christopherson
2026-07-24 17:38 ` Yosry Ahmed
2026-07-24 18:12 ` Sean Christopherson
2026-07-24 18:25 ` Yosry Ahmed
2026-06-29 18:37 ` [PATCH v3 08/10] KVM: selftests: Trigger save+restore randomly in the #PF stress test Yosry Ahmed
2026-06-29 18:48 ` sashiko-bot
2026-06-29 20:29 ` Yosry Ahmed
2026-06-29 18:37 ` [PATCH v3 09/10] KVM: selftests: Support running stress save+restore and #PF test in L2 Yosry Ahmed
2026-07-24 17:37 ` Sean Christopherson
2026-07-24 17:40 ` Yosry Ahmed
2026-06-29 18:37 ` [PATCH v3 10/10] KVM: selftests: Trigger L2->L1 exits stress save+restore and #PF test Yosry Ahmed
2026-07-24 17:50 ` Sean Christopherson
2026-07-24 18:15 ` Yosry Ahmed
2026-07-24 18:25 ` Sean Christopherson [this message]
2026-07-24 18:35 ` Yosry Ahmed
2026-07-24 20:32 ` 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=amOuALTiqI9u4tFh@google.com \
--to=seanjc@google.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=pbonzini@redhat.com \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox