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

[Sean Christopherson]

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.