[PATCH 0/2] Fix error handling for async_pf

[PATCH 0/2] Fix error handling for async_pf

[Christian Borntraeger]

Return mapping errors also for async_pf cases. Otherwise guests
will loop forever. Nothing critical just missing error handling.

can certainly wait after the merge window, but if anyone has time
to review..


Christian Borntraeger (2):
  KVM: Set up async page faults only for faults that need to wait
  KVM: selftests: Test that KVM_RUN fails for unresolvable host faults

 tools/testing/selftests/kvm/Makefile.kvm      |   1 +
 .../selftests/kvm/faultin_sigbus_test.c       | 168 ++++++++++++++++++
 virt/kvm/kvm_main.c                           |   1 +
 3 files changed, 170 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/faultin_sigbus_test.c

-- 
2.53.0

[PATCH 1/2] KVM: Set up async page faults only for faults that need to wait

[Christian Borntraeger]

hva_to_pfn() checks a FOLL_NOWAIT gup() for EINTR, EAGAIN and EHWPOISON.
There might be others like -EFAULT for VM_FAULT_SIGBUS.  For those other
errors, reporting KVM_PFN_ERR_NEEDS_IO would be wrong as it triggers an
async_pf resulting in endless retries.

Return KVM_PFN_ERR_NEEDS_IO only if gup() returned 0 pages and report
the error otherwise.

Fixes: 0857b9e95c1a ("KVM: Enable async page fault processing")
Signed-off-by: Christian Borntraeger <borntraeger@linux.ibm.com>
---
 virt/kvm/kvm_main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 5a782849521c..34b38d7512c3 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -3048,6 +3048,7 @@ kvm_pfn_t hva_to_pfn(struct kvm_follow_pfn *kfp)
 			pfn = KVM_PFN_ERR_FAULT;
 	} else {
 		if ((kfp->flags & FOLL_NOWAIT) &&
+		    npages == 0 && /* no PFN_ERR_NEEDS_IO on errors */
 		    vma_is_valid(vma, kfp->flags & FOLL_WRITE))
 			pfn = KVM_PFN_ERR_NEEDS_IO;
 		else
-- 
2.53.0

[PATCH 2/2] KVM: selftests: Test that KVM_RUN fails for unresolvable host faults

[Christian Borntraeger]

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       | 168 ++++++++++++++++++
 2 files changed, 169 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/faultin_sigbus_test.c

diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm
index 68bcec3d0070..24fdfc3e0eba 100644
--- a/tools/testing/selftests/kvm/Makefile.kvm
+++ b/tools/testing/selftests/kvm/Makefile.kvm
@@ -59,6 +59,7 @@ TEST_PROGS_x86 += x86/nx_huge_pages_test.sh
 # Compiled test targets valid on all architectures with libkvm support
 TEST_GEN_PROGS_COMMON = demand_paging_test
 TEST_GEN_PROGS_COMMON += dirty_log_test
+TEST_GEN_PROGS_COMMON += faultin_sigbus_test
 TEST_GEN_PROGS_COMMON += guest_print_test
 TEST_GEN_PROGS_COMMON += irqfd_test
 TEST_GEN_PROGS_COMMON += kvm_binary_stats_test
diff --git a/tools/testing/selftests/kvm/faultin_sigbus_test.c b/tools/testing/selftests/kvm/faultin_sigbus_test.c
new file mode 100644
index 000000000000..b9d673260ed5
--- /dev/null
+++ b/tools/testing/selftests/kvm/faultin_sigbus_test.c
@@ -0,0 +1,168 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * A guest access to memory whose host mapping cannot be faulted in (here: a
+ * shmem page beyond EOF, i.e. SIGBUS) must make KVM_RUN fail with -EFAULT.
+ *
+ * On architectures with asynchronous page faults, KVM first tries to fault
+ * the page in without waiting (FOLL_NOWAIT) and hands the fault to a worker
+ * if that would need I/O.  The error must not be mistaken for "needs I/O",
+ * otherwise the vCPU retries the access forever and KVM_RUN never returns.
+ * The architecture hooks below make sure that the async path is actually
+ * possible, i.e. that the error would be misclassified if KVM got it wrong.
+ */
+#include <signal.h>
+#include <unistd.h>
+#include <linux/sizes.h>
+#include <linux/stringify.h>
+
+#include "kvm_util.h"
+#include "processor.h"
+#include "test_util.h"
+#include "ucall_common.h"
+
+#define TEST_SLOT	10
+#define TEST_SIZE	SZ_1M
+#define TIMEOUT_SECS	10
+
+#ifdef __s390x__
+#define __LC_EXT_NEW_PSW		0x1b0
+#define __LC_SAVED_PSW			0x200	/* free for the OS */
+#define __LC_WAIT_PSW			0x210	/* free for the OS */
+#define PSW_MASK_EXT			0x0100000000000000ULL
+#define PSW_MASK_DAT			0x0400000000000000ULL
+#define PSW_MASK_WAIT			0x0002000000000000ULL
+#define CR0_SERVICE_SIGNAL_SUBMASK	BIT_ULL(63 - 54)
+
+/*
+ * Handle pfault like Linux does: on the init interrupt (ext cpu address
+ * 0x0600) remember the interrupted PSW and wait until the done interrupt
+ * (0x0680), then retry the access.  A guest that retried at once would
+ * fill up KVM's queue of async page faults and get the synchronous fault
+ * path.  Runs with DAT off, so lowcore is addressed by real addresses:
+ * 0x84 is the ext cpu address, 0x130 the ext old PSW.  Done differs from
+ * init only in bit 0x80 of 0x85, test it like Linux tests PF_COMPLETE.
+ */
+extern char ext_int_handler[];
+asm(".pushsection .text\n"
+	"ext_int_handler:\n"
+	"	tm	0x85,0x80\n"
+	"	jo	1f\n"
+	"	mvc	" __stringify(__LC_SAVED_PSW) "(16),0x130\n"
+	"	lpswe	" __stringify(__LC_WAIT_PSW) "\n"
+	"1:	lpswe	" __stringify(__LC_SAVED_PSW) "\n"
+	".popsection\n");
+
+static void arch_enable_async_pf(struct kvm_vm *vm, struct kvm_vcpu *vcpu)
+{
+	u64 *lowcore = addr_gpa2hva(vm, 0);
+	struct kvm_sregs sregs;
+	gva_t token_gva;
+	gpa_t handler;
+	int flic;
+
+	flic = kvm_create_device(vm, KVM_DEV_TYPE_FLIC);
+	kvm_device_attr_set(flic, KVM_DEV_FLIC_APF_ENABLE, 0, NULL);
+
+	/* The DIAG 0x258 handshake, with masks that match any guest PSW. */
+	token_gva = vm_alloc_page(vm);
+	vcpu_set_reg(vcpu, KVM_REG_S390_PFTOKEN, addr_gva2gpa(vm, token_gva));
+	vcpu_set_reg(vcpu, KVM_REG_S390_PFSELECT, 0);
+	vcpu_set_reg(vcpu, KVM_REG_S390_PFCOMPARE, 0);
+
+	vcpu_sregs_get(vcpu, &sregs);
+	sregs.crs[0] |= CR0_SERVICE_SIGNAL_SUBMASK;
+	vcpu_sregs_set(vcpu, &sregs);
+	vcpu->run->psw_mask |= PSW_MASK_EXT;
+
+	handler = addr_gva2gpa(vm, (gva_t)ext_int_handler);
+	lowcore[__LC_EXT_NEW_PSW / 8] = vcpu->run->psw_mask &
+					~(PSW_MASK_EXT | PSW_MASK_DAT);
+	lowcore[__LC_EXT_NEW_PSW / 8 + 1] = handler;
+	lowcore[__LC_WAIT_PSW / 8] = (vcpu->run->psw_mask & ~PSW_MASK_DAT) |
+				     PSW_MASK_WAIT;
+	lowcore[__LC_WAIT_PSW / 8 + 1] = handler;
+}
+#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)
+{
+}
+#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)
+{
+}
+
+int main(void)
+{
+	struct sigaction sa = { .sa_handler = sig_alarm };	/* no SA_RESTART */
+	struct userspace_mem_region *region;
+	struct kvm_vcpu *vcpu;
+	struct kvm_vm *vm;
+	struct ucall uc;
+	gpa_t gpa;
+	gva_t gva;
+	u64 npages;
+	int r;
+
+	vm = vm_create_with_one_vcpu(&vcpu, guest_code);
+
+	/* After slot 0 and the page that some architectures use for ucalls. */
+	region = memslot2region(vm, 0);
+	gpa = align_up(region->region.guest_phys_addr +
+		       region->region.memory_size + vm->page_size, TEST_SIZE);
+	npages = TEST_SIZE / vm->page_size;
+
+	vm_mem_add(vm, VM_MEM_SRC_SHMEM, gpa, TEST_SLOT, npages, 0, -1, 0);
+	region = memslot2region(vm, TEST_SLOT);
+	/* Only the first host page is backed, everything behind it is SIGBUS. */
+	TEST_ASSERT(!ftruncate(region->fd, getpagesize()), "ftruncate() failed");
+
+	gva = vm_unused_gva_gap(vm, TEST_SIZE, KVM_UTIL_MIN_VADDR);
+	virt_map(vm, gva, gpa, npages);
+	vcpu_args_set(vcpu, 2, gva, TEST_SIZE);
+
+	arch_enable_async_pf(vm, vcpu);
+
+	vcpu_run(vcpu);
+	switch (get_ucall(vcpu, &uc)) {
+	case UCALL_SYNC:
+		break;
+	case UCALL_ABORT:
+		REPORT_GUEST_ASSERT(uc);
+		break;
+	default:
+		TEST_FAIL("Unexpected ucall %lu", uc.cmd);
+	}
+	TEST_ASSERT_EQ(*(u64 *)region->host_mem, 1);
+
+	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);
+	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));
+
+	kvm_vm_free(vm);
+	return 0;
+}
-- 
2.53.0

Re: [PATCH 2/2] KVM: selftests: Test that KVM_RUN fails for unresolvable host faults

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] KVM: selftests: missing gmem_flags argument to vm_mem_add() in main()
--

commit 3a1c0f90404979b6638dbd3c2769d7b78633b104
Author: Christian Borntraeger <borntraeger@linux.ibm.com>

KVM: selftests: Test that KVM_RUN fails for unresolvable host faults

This commit adds a selftest in which a guest writes to a shmem-backed
memslot beyond the end of the file to verify that KVM_RUN fails with
-EFAULT rather than retrying an asynchronous page fault indefinitely.

> diff --git a/tools/testing/selftests/kvm/faultin_sigbus_test.c b/tools/testing/selftests/kvm/faultin_sigbus_test.c
> new file mode 100644
> index 0000000000000..b9d673260ed5a
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/faultin_sigbus_test.c
> @@ -0,0 +1,168 @@

[ ... ]

> +int main(void)
> +{
> +	struct sigaction sa = { .sa_handler = sig_alarm };	/* no SA_RESTART */
> +	struct userspace_mem_region *region;
> +	struct kvm_vcpu *vcpu;
> +	struct kvm_vm *vm;
> +	struct ucall uc;
> +	gpa_t gpa;
> +	gva_t gva;
> +	u64 npages;
> +	int r;
> +
> +	vm = vm_create_with_one_vcpu(&vcpu, guest_code);
> +
> +	/* After slot 0 and the page that some architectures use for ucalls. */
> +	region = memslot2region(vm, 0);
> +	gpa = align_up(region->region.guest_phys_addr +
> +		       region->region.memory_size + vm->page_size, TEST_SIZE);
> +	npages = TEST_SIZE / vm->page_size;
> +
> +	vm_mem_add(vm, VM_MEM_SRC_SHMEM, gpa, TEST_SLOT, npages, 0, -1, 0);

[Severity: Medium]
Does this fail to build when compiling KVM selftests?

In tools/testing/selftests/kvm/include/kvm_util.h (and
tools/testing/selftests/kvm/lib/kvm_util.c), vm_mem_add() takes 9
arguments following commit ee455183c92be7:

void vm_mem_add(struct kvm_vm *vm, enum vm_mem_backing_src_type src_type,
		gpa_t gpa, u32 slot, u64 npages, u32 flags,
		int gmem_fd, u64 gmem_offset, u64 gmem_flags);

Calling vm_mem_add() in main() with only 8 arguments omits the final
gmem_flags parameter and fails compilation with too few arguments to
function vm_mem_add. Should this pass the 9th argument (0), or use the
6-argument helper vm_userspace_mem_region_add(vm, VM_MEM_SRC_SHMEM, gpa,
TEST_SLOT, npages, 0) instead?

> +	region = memslot2region(vm, TEST_SLOT);
> +	/* Only the first host page is backed, everything behind it is SIGBUS. */
> +	TEST_ASSERT(!ftruncate(region->fd, getpagesize()), "ftruncate() failed");

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261020143308.228328-1-borntraeger@linux.ibm.com?part=2

Re: [PATCH 2/2] KVM: selftests: Test that KVM_RUN fails for unresolvable host faults

[Christian Borntraeger]

Am 20.10.26 um 16:40 schrieb sashiko-bot@kernel.org:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] KVM: selftests: missing gmem_flags argument to vm_mem_add() in main()
> --
> 
> commit 3a1c0f90404979b6638dbd3c2769d7b78633b104
> Author: Christian Borntraeger <borntraeger@linux.ibm.com>
> 
> KVM: selftests: Test that KVM_RUN fails for unresolvable host faults
> 
> This commit adds a selftest in which a guest writes to a shmem-backed
> memslot beyond the end of the file to verify that KVM_RUN fails with
> -EFAULT rather than retrying an asynchronous page fault indefinitely.
> 
>> diff --git a/tools/testing/selftests/kvm/faultin_sigbus_test.c b/tools/testing/selftests/kvm/faultin_sigbus_test.c
>> new file mode 100644
>> index 0000000000000..b9d673260ed5a
>> --- /dev/null
>> +++ b/tools/testing/selftests/kvm/faultin_sigbus_test.c
>> @@ -0,0 +1,168 @@
> 
> [ ... ]
> 
>> +int main(void)
>> +{
>> +	struct sigaction sa = { .sa_handler = sig_alarm };	/* no SA_RESTART */
>> +	struct userspace_mem_region *region;
>> +	struct kvm_vcpu *vcpu;
>> +	struct kvm_vm *vm;
>> +	struct ucall uc;
>> +	gpa_t gpa;
>> +	gva_t gva;
>> +	u64 npages;
>> +	int r;
>> +
>> +	vm = vm_create_with_one_vcpu(&vcpu, guest_code);
>> +
>> +	/* After slot 0 and the page that some architectures use for ucalls. */
>> +	region = memslot2region(vm, 0);
>> +	gpa = align_up(region->region.guest_phys_addr +
>> +		       region->region.memory_size + vm->page_size, TEST_SIZE);
>> +	npages = TEST_SIZE / vm->page_size;
>> +
>> +	vm_mem_add(vm, VM_MEM_SRC_SHMEM, gpa, TEST_SLOT, npages, 0, -1, 0);
> 
> [Severity: Medium]
> Does this fail to build when compiling KVM selftests?
> 
> In tools/testing/selftests/kvm/include/kvm_util.h (and
> tools/testing/selftests/kvm/lib/kvm_util.c), vm_mem_add() takes 9
> arguments following commit ee455183c92be7:
> 
> void vm_mem_add(struct kvm_vm *vm, enum vm_mem_backing_src_type src_type,
> 		gpa_t gpa, u32 slot, u64 npages, u32 flags,
> 		int gmem_fd, u64 gmem_offset, u64 gmem_flags);
> 
> Calling vm_mem_add() in main() with only 8 arguments omits the final
> gmem_flags parameter and fails compilation with too few arguments to
> function vm_mem_add. Should this pass the 9th argument (0), or use the
> 6-argument helper vm_userspace_mem_region_add(vm, VM_MEM_SRC_SHMEM, gpa,
> TEST_SLOT, npages, 0) instead?

Yes, this works on 7.3 but fails on next

Re: [PATCH 1/2] KVM: Set up async page faults only for faults that need to wait

[Sean Christopherson]

On Tue, Oct 20, 2026, Christian Borntraeger wrote:
> hva_to_pfn() checks a FOLL_NOWAIT gup() for EINTR, EAGAIN and EHWPOISON.
> There might be others like -EFAULT for VM_FAULT_SIGBUS.  For those other
> errors, reporting KVM_PFN_ERR_NEEDS_IO would be wrong as it triggers an
> async_pf resulting in endless retries.
> 
> Return KVM_PFN_ERR_NEEDS_IO only if gup() returned 0 pages and report
> the error otherwise.
> 
> Fixes: 0857b9e95c1a ("KVM: Enable async page fault processing")
> Signed-off-by: Christian Borntraeger <borntraeger@linux.ibm.com>
> ---
>  virt/kvm/kvm_main.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 5a782849521c..34b38d7512c3 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -3048,6 +3048,7 @@ kvm_pfn_t hva_to_pfn(struct kvm_follow_pfn *kfp)
>  			pfn = KVM_PFN_ERR_FAULT;
>  	} else {
>  		if ((kfp->flags & FOLL_NOWAIT) &&
> +		    npages == 0 && /* no PFN_ERR_NEEDS_IO on errors */

!npages is generally preferred over "== 0".


>  		    vma_is_valid(vma, kfp->flags & FOLL_WRITE))
>  			pfn = KVM_PFN_ERR_NEEDS_IO;
>  		else

Maybe instead of calling out "on errors", explicitly call out that GUP returns '0'
when the fault needs to be retried?  And if we move the npages check first, then
IMO we can use a block comment instead of a trailing comment, e.g.

		/* Note, GUP returns '0' if retry is needed. */
		if (!npages && (kfp->flags & FOLL_NOWAIT) &&
		    vma_is_valid(vma, kfp->flags & FOLL_WRITE))
			pfn = KVM_PFN_ERR_NEEDS_IO;
		else
			pfn = KVM_PFN_ERR_FAULT;


For anyone else that found this surprising, while faultin_page() returns -EBUSY
or -EAGAIN, __get_user_pages() converts those to '0':

			ret = faultin_page(vma, start, gup_flags,
					   PTR_ERR(page) == -EMLINK, locked);
			switch (ret) {
			case 0:
				goto retry;
			case -EBUSY:
			case -EAGAIN:
				ret = 0;
				fallthrough;
			case -EFAULT:
			case -ENOMEM:
			case -EHWPOISON:
				goto out;
			}

It'd be loved if GUP return EWOULDBLOCK for this case, but I can only imagine the
pain that could cause for the rest of the kernel.