Re: [PATCH v3 13/13] KVM: selftests: Add and use double-underscore versions of vmlaunch() and vmresume()

[Yosry Ahmed]

On Thu, Aug 27, 2026 at 1:28 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
>
> On Wed, Aug 26, 2026, Yosry Ahmed wrote:
> > On Wed, Aug 26, 2026 at 4:39 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
> > > diff --git a/tools/testing/selftests/kvm/x86/aperfmperf_test.c b/tools/testing/selftests/kvm/x86/aperfmperf_test.c
> > > index 11f5894d8ac7..a082658c1e48 100644
> > > --- a/tools/testing/selftests/kvm/x86/aperfmperf_test.c
> > > +++ b/tools/testing/selftests/kvm/x86/aperfmperf_test.c
> > > @@ -83,7 +83,7 @@ static void l1_vmx_code(struct vmx_pages *vmx)
> > >                 vmreadz(CPU_BASED_VM_EXEC_CONTROL) | CPU_BASED_USE_MSR_BITMAPS);
> > >
> > >         GUEST_ASSERT(!vmwrite(GUEST_RIP, (u64)l2_guest_code));
> >
> > Do we wanna give the same treatment to vmwrite()?
>
> Yes, and vmreadz().  Assuming '0' is simultaneously a reasonable failure and "safe"
> value is so stupid, especially since AFAICT literally none of the users actually
> need to gracefully tolerate failure.
>
> LOL, and it can't possibly work, because vmread() clobbers the zeroed value on
> VM-Fail.  Hilarious.
>
> I didn't include those changes purely because I had "#$*@ this code" fatigue. :-)
> But since this is going to conflict the world over, yeah, it makes sense to just
> fix everything.
>
> > > -       GUEST_ASSERT(!vmlaunch());
> > > +       vmlaunch();
> > >  }
> > >
> > >  static void guest_code(void *nested_test_data)
> > [..]
> > > diff --git a/tools/testing/selftests/kvm/x86/nested_emulation_test.c b/tools/testing/selftests/kvm/x86/nested_emulation_test.c
> > > index 2b38eff4f516..11fe4a31eda0 100644
> > > --- a/tools/testing/selftests/kvm/x86/nested_emulation_test.c
> > > +++ b/tools/testing/selftests/kvm/x86/nested_emulation_test.c
> > > @@ -102,7 +102,7 @@ static void guest_code(void *test_data)
> > >                         exit_insn_len = vmcb->control.next_rip - vmcb->save.rip;
> > >                         GUEST_ASSERT_EQ(vmcb->save.rip, (u64)l2_instruction);
> > >                 } else {
> > > -                       GUEST_ASSERT_EQ(i ? vmresume() : vmlaunch(), 0);
> > > +                       i ? vmresume() : vmlaunch();
> >
> > Ewww
> >
> > >                         exit_reason = vmreadz(VM_EXIT_REASON);
> > >                         exit_insn_len = vmreadz(VM_EXIT_INSTRUCTION_LEN);
> > >                         GUEST_ASSERT_EQ(vmreadz(GUEST_RIP), (u64)l2_instruction);
> > > diff --git a/tools/testing/selftests/kvm/x86/nested_exceptions_test.c b/tools/testing/selftests/kvm/x86/nested_exceptions_test.c
> > > index 50c271a03692..c36c11b5f518 100644
> > > --- a/tools/testing/selftests/kvm/x86/nested_exceptions_test.c
> > > +++ b/tools/testing/selftests/kvm/x86/nested_exceptions_test.c
> > > @@ -112,7 +112,7 @@ static void vmx_run_l2(void *l2_code, int vector, u32 error_code)
> > >  {
> > >         GUEST_ASSERT(!vmwrite(GUEST_RIP, (u64)l2_code));
> > >
> > > -       GUEST_ASSERT_EQ(vector == SS_VECTOR ? vmlaunch() : vmresume(), 0);
> > > +       vector == SS_VECTOR ? vmlaunch() : vmresume();
> >
> > Ewwwww
> >
> > Can we make these if/else statements? It was already ugly, but it
> > looks even more ugly outside of GUEST_ASSERT_EQ().
>
> Yeah.  Another idea would be:
>
>   void vmenter(bool do_vmlaunch)
>   {
>         if (do_vmlaunch)
>                 vmlaunch();
>         else
>                 vmresume();
>   }
>
> To yield:
>
>         vmenter(vector == SS_VECTOR);
>         vmenter(!i);
>
> Never mind, that's a terrible idea, the call sites are inscrutable.

Yeah, but I do agree that vmlaunch() vs vmresume() handling in
selftests is annoying (e.g. see run_l2() in [1]).

If you wanna go scorched earth on the nested VMX infra, what if we
track launch vs resume (e.g. set launch=true in prepare_vmcs() and
lauch=false in vmlaunch()), then provide a single vmenter() API for
tests (with no arguments)?

[1]https://lore.kernel.org/kvm/20260728003557.1136583-29-yosry@xxxxxxxxxx/