[PATCH v2 0/2] KVM: x86/mmu: Fill memory_fault for unresolvable guest page faults

[PATCH v2 0/2] KVM: x86/mmu: Fill memory_fault for unresolvable guest page faults

[mike.malyshev]

From: Mikhail Malyshev <mike.malyshev@gmail.com>

When KVM cannot resolve a guest page fault, kvm_handle_error_pfn() returns
a bare -EFAULT.  KVM_CAP_MEMORY_FAULT_INFO documents the opposite: that
KVM_RUN fills kvm_run.memory_fault "if KVM cannot resolve a guest page
fault VM-Exit, e.g. if there is a valid memslot but no backing VMA for the
corresponding host virtual address".

The gap has a practical cost.  On a host that assigns an Intel integrated
GPU (Raptor Lake-P) to a guest via vfio-pci, the guest's display driver
clears PCI_COMMAND.MEM on one vCPU while another vCPU is mid-MMIO to BAR0
of the same device.  vfio-pci zaps the BAR's mmap, so the second vCPU's
fault finds a valid memslot whose VM_PFNMAP fault handler declines to
install a PTE, and KVM_RUN fails with -EFAULT and nothing else.  The VMM
has nothing to act on and kills the VM, even though the guest did nothing
architecturally invalid and the condition clears as soon as the driver
re-enables memory decoding.  This reproduces on demand and has been
observed in the field on production edge hardware.

Patch 1 is Anish Moorthy's 2024 patch [1], code unchanged, with the
changelog rewritten around that user.  Patch 2 converts the remaining
WARN_ON_ONCE()-protected -EFAULTs in x86's fault path to KVM_BUG_ON() +
-EIO, so that an -EFAULT out of the fault path consistently means "guest
access KVM could not resolve" rather than "KVM is broken".

Changes since v1
================

v1 [2] took a different approach: it mapped KVM_PFN_ERR_PFNMAP to MMIO
semantics inside KVM.  Sean objected that giving PFNMAP memory emulated
MMIO semantics changed long-standing ABI inconsistently, and outlined this
approach instead [3][4], including the shape of both patches.  v2
implements that outline:

  - report the fault via KVM_EXIT_MEMORY_FAULT rather than inventing MMIO
    semantics for PFNMAP memory;
  - take Anish's patch rather than writing an equivalent one, crediting
    both authors;
  - add the KVM_BUG_ON() + -EIO conversions as a separate patch;
  - x86 only, and based on kvm-x86/next rather than linus/master.

The userspace half
==================

This series reports the fault; it deliberately does not define what an
access to a BAR with memory decoding disabled returns.  That policy belongs
in userspace, and QEMU does not implement it yet: QEMU routes
KVM_EXIT_MEMORY_FAULT into its guest_memfd conversion path, which rejects
a vfio BAR -- a ram_device region with no guest_memfd -- and stops the VM
with an internal error.  Unmodified QEMU therefore still loses the VM
exactly as it does today on a bare -EFAULT; only the reported error
changes.  A QEMU series emulating Unsupported Request semantics for this
case -- reads return all ones, writes are dropped, as the hardware does --
will follow and will link back here.

So this series does not regress today's userspace, but neither does it
fix anything for it: it is the enabling half, not a standalone crash fix.
That is also why patch 1 carries a Fixes: tag but is not tagged for
stable: a backport would not help any userspace that exists today.

Testing
=======

arch/x86/kvm/ builds clean with no new warnings for x86_64 and for i386
allmodconfig, the latter covering the 32-bit-only hunk in patch 2.
checkpatch.pl --strict reports 0 errors, 0 warnings, 0 checks on both
patches and on this cover letter.  End-to-end testing against the
reproducer requires the QEMU counterpart and will be reported with that
series.

Link: https://lore.kernel.org/all/20240809205158.1340255-1-amoorthy@google.com/ [1]
Link: https://lore.kernel.org/all/20260621133708.3454718-1-mike.malyshev@gmail.com/ [2]
Link: https://lore.kernel.org/all/ajnEAkFGyJWmomhq@google.com/ [3]
Link: https://lore.kernel.org/all/aj1Vc13mqb-fXiow@google.com/ [4]

Anish Moorthy (1):
  KVM: x86/mmu: Report a memory fault exit when the fault handler
    EFAULTs

Mikhail Malyshev (1):
  KVM: x86/mmu: Convert can't-happen fault path EFAULTs to KVM_BUG_ON()
    + -EIO

 arch/x86/kvm/mmu/mmu.c         | 13 +++++++------
 arch/x86/kvm/mmu/paging_tmpl.h |  4 ++--
 2 files changed, 9 insertions(+), 8 deletions(-)


base-commit: 70c944caf570fda2d79baa71435589a8db39f048
-- 
2.43.0

[PATCH v2 1/2] KVM: x86/mmu: Report a memory fault exit when the fault handler EFAULTs

[mike.malyshev]

From: Anish Moorthy <amoorthy@google.com>

KVM_CAP_MEMORY_FAULT_INFO documents that KVM_RUN will fill
kvm_run.memory_fault when KVM cannot resolve a guest page fault VM-Exit,
"e.g. if there is a valid memslot but no backing VMA for the
corresponding host virtual address".  kvm_handle_error_pfn() does not
honor that guarantee.  It returns a bare -EFAULT, leaving userspace with
no indication of which guest physical address faulted, or even that the
exit was a memory fault at all.  Userspace cannot distinguish a
transient, resolvable condition from a fatal one, so in practice the VMM
terminates the guest.

Fill kvm_run.memory_fault before returning -EFAULT.

A concrete user is an Intel integrated GPU assigned to a guest via
vfio-pci.  The guest driver clears PCI_COMMAND.MEM on one vCPU while
another vCPU is mid-MMIO to a BAR of the same device.  Clearing
PCI_COMMAND.MEM makes vfio-pci zap the BAR's mmap, so the second vCPU's
fault finds a valid memslot whose VMA can no longer supply a PFN, and
KVM_RUN fails with a bare -EFAULT.  The VM dies, even though the guest
did nothing architecturally invalid and the condition clears as soon as
the driver re-enables memory decoding.

Reproduce by pairing a vCPU that spins on accesses to the assigned
device's BAR0 with a vCPU that toggles PCI_COMMAND.MEM; the race is hit
within minutes.  The same crash has been observed in the field on
production edge hardware.

Reporting the fault does not by itself define the access semantics.
Userspace still has to decide what a read or write to a BAR with memory
decoding disabled returns.  But it is the information userspace needs in
order to make that decision instead of killing the guest.

Suggested-by: Sean Christopherson <seanjc@google.com>
Fixes: 16f95f3b95ca ("KVM: Add KVM_EXIT_MEMORY_FAULT exit to report faults to userspace")
Link: https://lore.kernel.org/all/20240809205158.1340255-1-amoorthy@google.com/
Link: https://lore.kernel.org/all/Zr-8M9rYplgN6IS3@google.com/
Signed-off-by: Anish Moorthy <amoorthy@google.com>
Co-developed-by: Mikhail Malyshev <mike.malyshev@gmail.com>
Signed-off-by: Mikhail Malyshev <mike.malyshev@gmail.com>
---
 arch/x86/kvm/mmu/mmu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 9788ff1803740..244575f576071 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -3616,6 +3616,7 @@ static int kvm_handle_error_pfn(struct kvm_vcpu *vcpu, struct kvm_page_fault *fa
 		return RET_PF_RETRY;
 	}
 
+	kvm_mmu_prepare_memory_fault_exit(vcpu, fault);
 	return -EFAULT;
 }
 
-- 
2.43.0

[PATCH v2 2/2] KVM: x86/mmu: Convert can't-happen fault path EFAULTs to KVM_BUG_ON() + -EIO

[mike.malyshev]

From: Mikhail Malyshev <mike.malyshev@gmail.com>

Four -EFAULT returns in x86's page fault path are guarded by
WARN_ON_ONCE() because they describe KVM bugs, not conditions the guest
or userspace can reach:

 - direct_map() and FNAME(fetch)() completing the walk at a level other
   than the goal level, i.e. KVM's shadow page table walk disagreeing
   with the mapping level KVM itself just computed.

 - a CR2 with bits 63:32 set on 32-bit KVM, which the hardware cannot
   produce.

 - PFERR_PRIVATE_ACCESS set on a reserved-bit fault.  KVM sets that
   synthetic bit itself, and only when PFERR_RSVD_MASK is clear.

Returning -EFAULT for a KVM bug conflicts with KVM_CAP_MEMORY_FAULT_INFO,
which promises that an -EFAULT out of KVM_RUN on a guest page fault
VM-Exit is accompanied by kvm_run.memory_fault.  Describing a broken
KVM invariant as a memory fault would be actively misleading, as there
is no guest access for userspace to resolve and retry.

Convert the four sites to KVM_BUG_ON() + -EIO, the established way to
report that KVM is hosed, so that -EFAULT out of the fault path always
means "guest memory access KVM could not resolve".

Suggested-by: Sean Christopherson <seanjc@google.com>
Link: https://lore.kernel.org/all/Zr-8M9rYplgN6IS3@google.com/
Signed-off-by: Mikhail Malyshev <mike.malyshev@gmail.com>
---
 arch/x86/kvm/mmu/mmu.c         | 12 ++++++------
 arch/x86/kvm/mmu/paging_tmpl.h |  4 ++--
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 244575f576071..6529ff9e98358 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -3577,8 +3577,8 @@ static int direct_map(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault)
 					     fault->req_level >= it.level);
 	}
 
-	if (WARN_ON_ONCE(it.level != fault->goal_level))
-		return -EFAULT;
+	if (KVM_BUG_ON(it.level != fault->goal_level, vcpu->kvm))
+		return -EIO;
 
 	ret = mmu_set_spte(vcpu, fault->slot, it.sptep, access,
 			   base_gfn, fault->pfn, fault);
@@ -4942,8 +4942,8 @@ int kvm_handle_page_fault(struct kvm_vcpu *vcpu, u64 error_code,
 
 #ifndef CONFIG_X86_64
 	/* A 64-bit CR2 should be impossible on 32-bit KVM. */
-	if (WARN_ON_ONCE(fault_address >> 32))
-		return -EFAULT;
+	if (KVM_BUG_ON(fault_address >> 32, vcpu->kvm))
+		return -EIO;
 #endif
 	/*
 	 * Legacy #PF exception only have a 32-bit error code.  Simply drop the
@@ -6658,8 +6658,8 @@ int noinline kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa, u64 err
 
 	r = RET_PF_INVALID;
 	if (unlikely(error_code & PFERR_RSVD_MASK)) {
-		if (WARN_ON_ONCE(error_code & PFERR_PRIVATE_ACCESS))
-			return -EFAULT;
+		if (KVM_BUG_ON(error_code & PFERR_PRIVATE_ACCESS, vcpu->kvm))
+			return -EIO;
 
 		r = handle_mmio_page_fault(vcpu, cr2_or_gpa, direct);
 		if (r == RET_PF_EMULATE)
diff --git a/arch/x86/kvm/mmu/paging_tmpl.h b/arch/x86/kvm/mmu/paging_tmpl.h
index c8ec47b09264b..8e350095508c5 100644
--- a/arch/x86/kvm/mmu/paging_tmpl.h
+++ b/arch/x86/kvm/mmu/paging_tmpl.h
@@ -778,8 +778,8 @@ static int FNAME(fetch)(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault,
 					     fault->req_level >= it.level);
 	}
 
-	if (WARN_ON_ONCE(it.level != fault->goal_level))
-		return -EFAULT;
+	if (KVM_BUG_ON(it.level != fault->goal_level, vcpu->kvm))
+		return -EIO;
 
 	ret = mmu_set_spte(vcpu, fault->slot, it.sptep, gw->pte_access,
 			   base_gfn, fault->pfn, fault);
-- 
2.43.0