All of lore.kernel.org
 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: 32+ 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

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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.