Kernel KVM virtualization development
 help / color / mirror / Atom feed
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 10:50:42 -0700	[thread overview]
Message-ID: <amOl8kczWlZP_ftH@google.com> (raw)
In-Reply-To: <20260629183746.699840-11-yosry@kernel.org>

On Mon, Jun 29, 2026, Yosry Ahmed wrote:
> Extend the testing coverage in L2 by injecting a #UD into the vCPU every
> other iteration during restore, and intercepting #UD from L1,
> essentially forcing an L2 -> L1 VM-Exit directly after save+restore.

Assuming the #UD is a means to an end, make that the focus of the intro blurb.
I read this changelog without looking at the shortlog, and was about to ask why
injecting a #UD is interesting, and then I saw the comment.  It'd be helpful
to add a bit more context too, as it took me a few seconds to piece together
that the goal is to force the exit while L0 has control, i.e. a more obvious
hypercall from L2 wouldn't suffice.

> With this change, the test reliably reproduces the CR2 bug fixed by
> commit 5c247d08bc81 ("KVM: nSVM: Use vcpu->arch.cr2 when updating vmcb12
> on nested #VMEXIT") -- at least on Milan, Genoa, and Turin CPUs.
> 
> Assisted-by: Gemini:gemini-3.1-pro
> Signed-off-by: Yosry Ahmed <yosry@kernel.org>
> ---
>  .../kvm/x86/stress_save_restore_pf_test.c     | 47 +++++++++++++++++--
>  1 file changed, 42 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/x86/stress_save_restore_pf_test.c b/tools/testing/selftests/kvm/x86/stress_save_restore_pf_test.c
> index 9ab52d27a61d9..2b76e56f744e7 100644
> --- a/tools/testing/selftests/kvm/x86/stress_save_restore_pf_test.c
> +++ b/tools/testing/selftests/kvm/x86/stress_save_restore_pf_test.c
> @@ -105,8 +105,12 @@ static void guest_access_memory(void *arg)
>  static void l1_svm_code(struct svm_test_data *svm)
>  {
>  	generic_svm_setup(svm, guest_access_memory);
> -	run_guest(svm->vmcb, svm->vmcb_gpa);
> -	GUEST_ASSERT(false);
> +	svm->vmcb->control.intercept_exceptions |= BIT(UD_VECTOR);
> +
> +	while (1) {
> +		run_guest(svm->vmcb, svm->vmcb_gpa);
> +		GUEST_ASSERT_EQ(svm->vmcb->control.exit_code, (SVM_EXIT_EXCP_BASE + UD_VECTOR));

Please wrap this one, it's so long that I find it genuinely difficult to parse.

		GUEST_ASSERT_EQ(svm->vmcb->control.exit_code,
				(SVM_EXIT_EXCP_BASE + UD_VECTOR));

> +	}
>  }
>  
>  static void l1_vmx_code(struct vmx_pages *vmx)
> @@ -115,13 +119,17 @@ static void l1_vmx_code(struct vmx_pages *vmx)
>  	GUEST_ASSERT(load_vmcs(vmx));
>  	prepare_vmcs(vmx, guest_access_memory);
>  
> -	/* Ignore any #PF */
> -	GUEST_ASSERT(!vmwrite(EXCEPTION_BITMAP, BIT(PF_VECTOR)));
> +	/* Intercept UD, ignore any #PF */
> +	GUEST_ASSERT(!vmwrite(EXCEPTION_BITMAP, BIT(UD_VECTOR) | BIT(PF_VECTOR)));
>  	GUEST_ASSERT(!vmwrite(PAGE_FAULT_ERROR_CODE_MASK, 0));
>  	GUEST_ASSERT(!vmwrite(PAGE_FAULT_ERROR_CODE_MATCH, -1));
>  
>  	GUEST_ASSERT(!vmlaunch());
> -	GUEST_ASSERT(false);
> +	while (1) {
> +		GUEST_ASSERT_EQ(vmreadz(VM_EXIT_REASON), EXIT_REASON_EXCEPTION_NMI);
> +		GUEST_ASSERT_EQ(vmreadz(VM_EXIT_INTR_INFO) & 0xff, UD_VECTOR);
> +		GUEST_ASSERT(!vmresume());
> +	}
>  }
>  
>  static void l1_guest_code(void *test_data)
> @@ -159,6 +167,24 @@ static void vcpu_sigusr_ignore(void)
>  	sigaction(SIGUSR1, &sa, NULL);
>  }
>  
> +static bool vcpu_state_is_guest_mode(struct kvm_x86_state *state)
> +{
> +	return !!(state->nested.flags & KVM_STATE_NESTED_GUEST_MODE);
> +}
> +
> +static void vcpu_state_inject_ud(struct kvm_x86_state *state)
> +{
> +	if (state->events.exception.pending || state->events.exception.injected)
> +		return;
> +
> +	state->events.flags |= KVM_VCPUEVENT_VALID_PAYLOAD;
> +	state->events.exception.pending = true;
> +	state->events.exception.injected = false;
> +	state->events.exception.nr = UD_VECTOR;
> +	state->events.exception.has_error_code = false;
> +	state->events.exception_has_payload = false;
> +}
> +
>  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.

> +
>  	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?

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

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

		/*
		 * If the vCPU is in guest mode, inject a #UD to trigger an
		 * L2->L1 VM-Exit every other iteration, unless the vCPU has.  Take care not to
		 * clobber any exceptions
		 */
		if ((i & 1) && (state.nested.flags & KVM_STATE_NESTED_GUEST_MODE) &&
		    !state.events.exception.pending && !state.events.exception.injected) {
			state->events.exception.pending = true;
			state->events.exception.injected = false;
			state->events.exception.nr = UD_VECTOR;
			state->events.exception.has_error_code = false;
		}			

> +
>  		kvm_vm_release(vm);
>  		vcpu = vm_recreate_with_one_vcpu(vm);
> +		vm_enable_cap(vm, KVM_CAP_EXCEPTION_PAYLOAD, -2ul);
>  		vcpu_load_state(vcpu, state);
>  		kvm_x86_state_cleanup(state);
>  
> -- 
> 2.55.0.rc0.799.gd6f94ed593-goog
> 

  reply	other threads:[~2026-07-24 17:50 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 [this message]
2026-07-24 18:15     ` Yosry Ahmed
2026-07-24 18:25       ` Sean Christopherson
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=amOl8kczWlZP_ftH@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