All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Christian Borntraeger <borntraeger@linux.ibm.com>
Cc: Janosch Frank <frankja@linux.ibm.com>,
	Claudio Imbrenda <imbrenda@linux.ibm.com>,
	 Paolo Bonzini <pbonzini@redhat.com>, KVM <kvm@vger.kernel.org>,
	 linux-s390 <linux-s390@vger.kernel.org>,
	Shuah Khan <shuah@kernel.org>
Subject: Re: [PATCH v2 2/2] KVM: selftests: Test that KVM_RUN fails for unresolvable host faults
Date: Thu, 22 Oct 2026 11:18:19 -0700	[thread overview]
Message-ID: <atpTa2FMU19UGitv@google.com> (raw)
In-Reply-To: <20261020175818.375700-3-borntraeger@linux.ibm.com>

On Tue, Oct 20, 2026, Christian Borntraeger wrote:
> Add a test in which the guest writes to a shmem-backed memslot beyond
> the end of the file.  KVM_RUN must fail with -EFAULT.
> 
> On x86 the guest enables interrupts, and on s390 the test sets up
> pfault, so that KVM could handle the fault with an asynchronous page
> fault.  If KVM mistakes the error for a fault that needs I/O, the vCPU
> retries the access forever, and the test fails after a timeout.
> 
> The test is not built for LoongArch, which handles a failed mapping as
> an MMIO access.
> 
> Signed-off-by: Christian Borntraeger <borntraeger@linux.ibm.com>
> ---
>  tools/testing/selftests/kvm/Makefile.kvm      |   1 +
>  .../selftests/kvm/faultin_sigbus_test.c       | 169 ++++++++++++++++++

The "sigbus" part of this is confusing, because there is never actually a sigbus.
Kinda silly, but maybe faultin_efault_test?

> +static void arch_enable_async_pf(struct kvm_vm *vm, struct kvm_vcpu *vcpu)
> +{

...

> +}
> +#else
> +/* x86 needs guest interrupts on, see guest_code(); nothing to do elsewhere. */
> +static void arch_enable_async_pf(struct kvm_vm *vm, struct kvm_vcpu *vcpu)

Nit, maybe arch_enable_host_async_pf()?  Because x86 has PV async #PFs, where
instead of scheduling a worker task in the host, KVM (conditionally) reflects
the #PF into the guest so that the guest can schedule in a different task.  A
lot of people, myself included, see "async #PF" and think of the PV version first,
e.g. I initially was looking for the PV side of things here.


> +#endif
> +
> +static void guest_code(u64 gva, u64 size)
> +{
> +#ifdef __x86_64__
> +	/* KVM does async #PF, or its halt fallback, only with IRQs enabled. */
> +	sti_nop();
> +#endif
> +	/* Within EOF: must work, proves that the mapping is set up right. */
> +	WRITE_ONCE(*(u64 *)gva, 1);
> +	GUEST_SYNC(0);
> +
> +	/* Beyond EOF: KVM_RUN must fail with -EFAULT. */
> +	WRITE_ONCE(*(u64 *)(gva + size - sizeof(u64)), 1);
> +	GUEST_FAIL("Store beyond EOF succeeded");
> +}
> +
> +static void sig_alarm(int sig)
> +{
> +}

...

> +	TEST_ASSERT(!sigaction(SIGALRM, &sa, NULL), "sigaction() failed");
> +	alarm(TIMEOUT_SECS);
> +	r = __vcpu_run(vcpu);
> +	alarm(0);
> +
> +	TEST_ASSERT(!(r == -1 && errno == EINTR),
> +		    "KVM_RUN did not return within %d seconds: the fault on the SIGBUS page is retried forever",
> +		    TIMEOUT_SECS);

Rather than assert on EINTR, what if we have sig_alarm() explicitly TEST_FAIL()?
That way readers don't have to piece together that the EINTR is coming from the
alarm, and there's less chance of the test somehow getting a false pass because
KVM manages to morph the signal into EFAULT.  That would also allow using the
single-underscore _vcpu_run(), though I don't know if that's actually a good idea.

E.g.

diff --git a/tools/testing/selftests/kvm/faultin_sigbus_test.c b/tools/testing/selftests/kvm/faultin_sigbus_test.c
index 9394b04efcd6..9d90c32f3c6a 100644
--- a/tools/testing/selftests/kvm/faultin_sigbus_test.c
+++ b/tools/testing/selftests/kvm/faultin_sigbus_test.c
@@ -106,6 +106,7 @@ static void guest_code(u64 gva, u64 size)
 
 static void sig_alarm(int sig)
 {
+	TEST_FAIL("%d second timeout expired before KVM_RUN hit -EFAULT", TIMEOUT_SECS);
 }
 
 int main(void)
@@ -154,12 +155,9 @@ int main(void)
 
 	TEST_ASSERT(!sigaction(SIGALRM, &sa, NULL), "sigaction() failed");
 	alarm(TIMEOUT_SECS);
-	r = __vcpu_run(vcpu);
+	r = _vcpu_run(vcpu);
 	alarm(0);
 
-	TEST_ASSERT(!(r == -1 && errno == EINTR),
-		    "KVM_RUN did not return within %d seconds: the fault on the SIGBUS page is retried forever",
-		    TIMEOUT_SECS);
 	TEST_ASSERT(r == -1 && errno == EFAULT,
 		    "Expected KVM_RUN to fail with EFAULT, got r = %d, errno = %d (%s), exit reason %s",
 		    r, errno, strerror(errno), exit_reason_str(vcpu->run->exit_reason));

  parent reply	other threads:[~2026-10-22 18:18 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-20 17:58 [PATCH v2 0/2] Fix error handling for async_pf Christian Borntraeger
2026-10-20 17:58 ` [PATCH v2 1/2] KVM: Set up async page faults only for faults that need to wait Christian Borntraeger
2026-10-20 18:02   ` sashiko-bot
2026-10-22  2:23   ` Sean Christopherson
2026-10-20 17:58 ` [PATCH v2 2/2] KVM: selftests: Test that KVM_RUN fails for unresolvable host faults Christian Borntraeger
2026-10-20 18:09   ` sashiko-bot
2026-10-22 18:18   ` Sean Christopherson [this message]
2026-10-23 12:35     ` Christian Borntraeger
2026-10-23 14:05       ` [PATCH v3 " Christian Borntraeger
2026-10-23 15:06         ` 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=atpTa2FMU19UGitv@google.com \
    --to=seanjc@google.com \
    --cc=borntraeger@linux.ibm.com \
    --cc=frankja@linux.ibm.com \
    --cc=imbrenda@linux.ibm.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=shuah@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.