[PATCH 0/2] KVM: Fault injection

[PATCH 0/2] KVM: Fault injection

[James Houghton]

Hi Sean and Paolo,

I've prepared a patch that adds some fault injection points into KVM MMU
code to better catch bugs in the future. I put the documentation changes
in their own patch; I'm happy to squash them if you want.

The three points I've added here are:
1. Make KVM think that MMU invalidations happen more often.
2. Make KVM think that cmpxchg for TDP MMU is failing more often.
3. Make KVM think that the MMU lock is contended while iterating over
   TDP MMU SPTEs.

Unfortunately I haven't caught any bugs with this yet, but a while ago
we added something like this to consistently reproduce a bug in the
Direct MMU (the old, Google-internal implementation of TDP MMU).

I tried putting a WARN in when the TDP MMU cmpxchg fails to simulate a
bug when cmpxchg fails (this was the case for us with Direct MMU :)),
and running a few of the selftests, I get a few cmpxchg failures at the
beginning of the test, but even with several vCPUs, they only appear at
the beginning of the test. With fault injection, we can get them
constantly, exposing more code paths to cmpxchg failures.

It would be really great if this could be hooked into syzkaller for
better coverage; not sure what's needed for that.

Also if you have any ideas for what other fault injection points make
sense, I'd be happy to add them.

Please let me know what you think. Thanks!

This patch is based on the tip of Linus's tree.

James Houghton (2):
  KVM: Add fault injection for some MMU operations
  Documentation: fault-injection: Add entries for KVM fault injection
    points

 .../fault-injection/fault-injection.rst       | 12 +++++++
 arch/x86/kvm/Makefile                         |  1 +
 arch/x86/kvm/debugfs.c                        |  6 ++++
 arch/x86/kvm/mmu/fault_injection.c            | 36 +++++++++++++++++++
 arch/x86/kvm/mmu/fault_injection.h            | 31 ++++++++++++++++
 arch/x86/kvm/mmu/mmu.c                        |  1 +
 arch/x86/kvm/mmu/tdp_mmu.c                    | 10 ++++--
 include/linux/kvm_host.h                      | 19 ++++++++--
 lib/Kconfig.debug                             |  8 +++++
 virt/kvm/kvm_main.c                           | 25 +++++++++++++
 10 files changed, 143 insertions(+), 6 deletions(-)
 create mode 100644 arch/x86/kvm/mmu/fault_injection.c
 create mode 100644 arch/x86/kvm/mmu/fault_injection.h


base-commit: cca7a0aae8958c9b1cd14116cb8b2f22ace2205e
-- 
2.50.1.703.g449372360f-goog

[PATCH 1/2] KVM: Add fault injection for some MMU operations

[James Houghton]

Provide fault injection hooks for three operations:
1. For all architectures, retries due to invalidation notifiers.
2. For x86, TDP MMU cmpxchg updates for SPTEs.
3. For x86, TDP MMU SPTE iteration rescheduling.

For all of these, fault injection can induce the uncommon cases: (1)
that an invalidation occurred, (2) a cmpxchg failed, and (3) that the
MMU lock is contended.

All of the debugfs directories for each of these injection points will
show up under the KVM debugfs directory (usually /sys/kernel/debug/kvm)
with the following names:
1. fail_kvm_mmu_invalidate_retry
2. fail_tdp_mmu_cmpxchg
3. fail_tdp_mmu_resched

Signed-off-by: James Houghton <jthoughton@google.com>
---
 arch/x86/kvm/Makefile              |  1 +
 arch/x86/kvm/debugfs.c             |  6 +++++
 arch/x86/kvm/mmu/fault_injection.c | 36 ++++++++++++++++++++++++++++++
 arch/x86/kvm/mmu/fault_injection.h | 31 +++++++++++++++++++++++++
 arch/x86/kvm/mmu/mmu.c             |  1 +
 arch/x86/kvm/mmu/tdp_mmu.c         | 10 ++++++---
 include/linux/kvm_host.h           | 19 +++++++++++++---
 lib/Kconfig.debug                  |  8 +++++++
 virt/kvm/kvm_main.c                | 25 +++++++++++++++++++++
 9 files changed, 131 insertions(+), 6 deletions(-)
 create mode 100644 arch/x86/kvm/mmu/fault_injection.c
 create mode 100644 arch/x86/kvm/mmu/fault_injection.h

diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
index c4b8950c7abee..8d2a16fd396b3 100644
--- a/arch/x86/kvm/Makefile
+++ b/arch/x86/kvm/Makefile
@@ -10,6 +10,7 @@ kvm-y			+= x86.o emulate.o irq.o lapic.o cpuid.o pmu.o mtrr.o \
 
 kvm-$(CONFIG_X86_64) += mmu/tdp_iter.o mmu/tdp_mmu.o
 kvm-$(CONFIG_KVM_IOAPIC) += i8259.o i8254.o ioapic.o
+kvm-$(CONFIG_KVM_FAULT_INJECTION) += mmu/fault_injection.o
 kvm-$(CONFIG_KVM_HYPERV) += hyperv.o
 kvm-$(CONFIG_KVM_XEN)	+= xen.o
 kvm-$(CONFIG_KVM_SMM)	+= smm.o
diff --git a/arch/x86/kvm/debugfs.c b/arch/x86/kvm/debugfs.c
index 999227fc7c665..e901581a0901c 100644
--- a/arch/x86/kvm/debugfs.c
+++ b/arch/x86/kvm/debugfs.c
@@ -10,6 +10,7 @@
 #include <linux/debugfs.h>
 #include "lapic.h"
 #include "mmu.h"
+#include "mmu/fault_injection.h"
 #include "mmu/mmu_internal.h"
 
 static int vcpu_get_timer_advance_ns(void *data, u64 *val)
@@ -194,3 +195,8 @@ void kvm_arch_create_vm_debugfs(struct kvm *kvm)
 	debugfs_create_file("mmu_rmaps_stat", 0644, kvm->debugfs_dentry, kvm,
 			    &mmu_rmaps_stat_fops);
 }
+
+void kvm_arch_create_debugfs(struct dentry *dentry)
+{
+	kvm_mmu_fault_injection_init(dentry);
+}
diff --git a/arch/x86/kvm/mmu/fault_injection.c b/arch/x86/kvm/mmu/fault_injection.c
new file mode 100644
index 0000000000000..25155ecd70a3b
--- /dev/null
+++ b/arch/x86/kvm/mmu/fault_injection.c
@@ -0,0 +1,36 @@
+// SPDX-License-Identifier: GPL-2.0
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include "fault_injection.h"
+
+#include <linux/dcache.h>
+#include <linux/debugfs.h>
+#include <linux/error-injection.h>
+#include <linux/fault-inject.h>
+#include <linux/init.h>
+#include <linux/kvm_host.h>
+
+static DECLARE_FAULT_ATTR(fail_tdp_mmu_cmpxchg);
+static DECLARE_FAULT_ATTR(fail_tdp_mmu_resched);
+
+bool tdp_mmu_cmpxchg_should_fail(void)
+{
+	return should_fail(&fail_tdp_mmu_cmpxchg, 1);
+}
+ALLOW_ERROR_INJECTION(tdp_mmu_cmpxchg_should_fail, TRUE);
+
+bool tdp_mmu_should_inject_resched(void)
+{
+	return should_fail(&fail_tdp_mmu_resched, 1);
+}
+ALLOW_ERROR_INJECTION(tdp_mmu_should_inject_resched, TRUE);
+
+void kvm_mmu_fault_injection_init(struct dentry *dentry)
+{
+#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS
+	fault_create_debugfs_attr("fail_tdp_mmu_cmpxchg", dentry,
+				  &fail_tdp_mmu_cmpxchg);
+	fault_create_debugfs_attr("fail_tdp_mmu_resched", dentry,
+				  &fail_tdp_mmu_resched);
+#endif
+}
diff --git a/arch/x86/kvm/mmu/fault_injection.h b/arch/x86/kvm/mmu/fault_injection.h
new file mode 100644
index 0000000000000..e8d21491d12e8
--- /dev/null
+++ b/arch/x86/kvm/mmu/fault_injection.h
@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __KVM_X86_MMU_FAULT_INJECTION_H
+#define __KVM_X86_MMU_FAULT_INJECTION_H
+
+#include <linux/types.h>
+#include <linux/init.h>
+#include <linux/dcache.h>
+
+#ifdef CONFIG_KVM_FAULT_INJECTION
+
+void kvm_mmu_fault_injection_init(struct dentry *dentry);
+bool tdp_mmu_cmpxchg_should_fail(void);
+bool tdp_mmu_should_inject_resched(void);
+
+#else
+
+static inline void kvm_mmu_fault_injection_init(struct dentry *dentry)
+{
+}
+static inline bool tdp_mmu_cmpxchg_should_fail(void)
+{
+	return false;
+}
+static inline bool tdp_mmu_should_inject_resched(void)
+{
+	return false;
+}
+
+#endif /* CONFIG_FAULT_INJECTION_KVM */
+
+#endif /* __KVM_X86_MMU_FAULT_INJECTION_H */
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 6e838cb6c9e12..2739b8bddeb9f 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -28,6 +28,7 @@
 #include "page_track.h"
 #include "cpuid.h"
 #include "spte.h"
+#include "fault_injection.h"
 
 #include <linux/kvm_host.h>
 #include <linux/types.h>
diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
index 7f3d7229b2c1f..5ff63fe21dace 100644
--- a/arch/x86/kvm/mmu/tdp_mmu.c
+++ b/arch/x86/kvm/mmu/tdp_mmu.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+#include "fault_injection.h"
 #include "mmu.h"
 #include "mmu_internal.h"
 #include "mmutrace.h"
@@ -530,7 +531,8 @@ static int __must_check set_external_spte_present(struct kvm *kvm, tdp_ptep_t sp
 	 * page table has been modified. Use FROZEN_SPTE similar to
 	 * the zapping case.
 	 */
-	if (!try_cmpxchg64(rcu_dereference(sptep), &old_spte, FROZEN_SPTE))
+	if (tdp_mmu_cmpxchg_should_fail() ||
+	    !try_cmpxchg64(rcu_dereference(sptep), &old_spte, FROZEN_SPTE))
 		return -EBUSY;
 
 	/*
@@ -689,7 +691,8 @@ static inline int __must_check __tdp_mmu_set_spte_atomic(struct kvm *kvm,
 		 * operates on fresh data, e.g. if it retries
 		 * tdp_mmu_set_spte_atomic()
 		 */
-		if (!try_cmpxchg64(sptep, &iter->old_spte, new_spte))
+		if (tdp_mmu_cmpxchg_should_fail() ||
+		    !try_cmpxchg64(sptep, &iter->old_spte, new_spte))
 			return -EBUSY;
 	}
 
@@ -796,7 +799,8 @@ static inline void tdp_mmu_iter_set_spte(struct kvm *kvm, struct tdp_iter *iter,
 static inline bool __must_check tdp_mmu_iter_need_resched(struct kvm *kvm,
 							  struct tdp_iter *iter)
 {
-	if (!need_resched() && !rwlock_needbreak(&kvm->mmu_lock))
+	if (!need_resched() && !rwlock_needbreak(&kvm->mmu_lock) &&
+	    !tdp_mmu_should_inject_resched())
 		return false;
 
 	/* Ensure forward progress has been made before yielding. */
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 15656b7fba6c7..28be52f1f6a61 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -1629,6 +1629,7 @@ bool kvm_arch_dy_has_pending_interrupt(struct kvm_vcpu *vcpu);
 bool kvm_arch_vcpu_preempted_in_kernel(struct kvm_vcpu *vcpu);
 void kvm_arch_pre_destroy_vm(struct kvm *kvm);
 void kvm_arch_create_vm_debugfs(struct kvm *kvm);
+void kvm_arch_create_debugfs(struct dentry *dentry);
 
 #ifndef __KVM_HAVE_ARCH_VM_ALLOC
 /*
@@ -2084,6 +2085,15 @@ extern const struct _kvm_stats_desc kvm_vm_stats_desc[];
 extern const struct kvm_stats_header kvm_vcpu_stats_header;
 extern const struct _kvm_stats_desc kvm_vcpu_stats_desc[];
 
+#ifdef CONFIG_KVM_FAULT_INJECTION
+bool kvm_mmu_invalidate_retry_should_fail(void);
+#else
+static inline bool kvm_mmu_invalidate_retry_should_fail(void)
+{
+	return false;
+}
+#endif
+
 #ifdef CONFIG_KVM_GENERIC_MMU_NOTIFIER
 static inline int mmu_invalidate_retry(struct kvm *kvm, unsigned long mmu_seq)
 {
@@ -2102,7 +2112,8 @@ static inline int mmu_invalidate_retry(struct kvm *kvm, unsigned long mmu_seq)
 	 * kvm->mmu_lock to keep things ordered.
 	 */
 	smp_rmb();
-	if (kvm->mmu_invalidate_seq != mmu_seq)
+	if (kvm->mmu_invalidate_seq != mmu_seq ||
+	    kvm_mmu_invalidate_retry_should_fail())
 		return 1;
 	return 0;
 }
@@ -2132,7 +2143,8 @@ static inline int mmu_invalidate_retry_gfn(struct kvm *kvm,
 			return 1;
 	}
 
-	if (kvm->mmu_invalidate_seq != mmu_seq)
+	if (kvm->mmu_invalidate_seq != mmu_seq ||
+	    kvm_mmu_invalidate_retry_should_fail())
 		return 1;
 	return 0;
 }
@@ -2160,7 +2172,8 @@ static inline bool mmu_invalidate_retry_gfn_unsafe(struct kvm *kvm,
 	    gfn < kvm->mmu_invalidate_range_end)
 		return true;
 
-	return READ_ONCE(kvm->mmu_invalidate_seq) != mmu_seq;
+	return READ_ONCE(kvm->mmu_invalidate_seq) != mmu_seq ||
+	       kvm_mmu_invalidate_retry_should_fail();
 }
 #endif
 
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index dc0e0c6ed075e..3e029ce6497aa 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2126,6 +2126,14 @@ config FAIL_SKB_REALLOC
 	  For more information, check
 	  Documentation/fault-injection/fault-injection.rst
 
+config KVM_FAULT_INJECTION
+	bool "Fault-injection capability for KVM"
+	depends on FAULT_INJECTION_DEBUG_FS && KVM_COMMON
+	help
+	  Provide fault-injection capability for KVM.
+	  This will cause rare MMU operations to occur more frequently (e.g.
+	  SPTE cmpxchg failure).
+
 config FAULT_INJECTION_CONFIGFS
 	bool "Configfs interface for fault-injection capabilities"
 	depends on FAULT_INJECTION
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 6c07dd423458c..c371ec3eeed1c 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -49,6 +49,7 @@
 #include <linux/lockdep.h>
 #include <linux/kthread.h>
 #include <linux/suspend.h>
+#include <linux/fault-inject.h>
 
 #include <asm/processor.h>
 #include <asm/ioctl.h>
@@ -1103,6 +1104,14 @@ void __weak kvm_arch_create_vm_debugfs(struct kvm *kvm)
 {
 }
 
+/*
+ * Called after kvm->debugfs_dentry is created to create KVM-global
+ * arch-specific debugfs entries. dentry might not be valid.
+ */
+void __weak kvm_arch_create_debugfs(struct dentry *dentry)
+{
+}
+
 static struct kvm *kvm_create_vm(unsigned long type, const char *fdname)
 {
 	struct kvm *kvm = kvm_arch_alloc_vm();
@@ -6301,6 +6310,16 @@ static void kvm_uevent_notify_change(unsigned int type, struct kvm *kvm)
 	kfree(env);
 }
 
+static DECLARE_FAULT_ATTR(fail_kvm_mmu_invalidate_retry);
+
+#ifdef CONFIG_KVM_FAULT_INJECTION
+bool kvm_mmu_invalidate_retry_should_fail(void)
+{
+	return should_fail(&fail_kvm_mmu_invalidate_retry, 1);
+}
+EXPORT_SYMBOL_GPL(kvm_mmu_invalidate_retry_should_fail);
+#endif
+
 static void kvm_init_debug(void)
 {
 	const struct file_operations *fops;
@@ -6330,6 +6349,12 @@ static void kvm_init_debug(void)
 				kvm_debugfs_dir,
 				(void *)(long)pdesc->desc.offset, fops);
 	}
+
+	fault_create_debugfs_attr("fail_kvm_mmu_invalidate_retry",
+				  kvm_debugfs_dir,
+				  &fail_kvm_mmu_invalidate_retry);
+
+	kvm_arch_create_debugfs(kvm_debugfs_dir);
 }
 
 static inline
-- 
2.50.1.703.g449372360f-goog

[PATCH 2/2] Documentation: fault-injection: Add entries for KVM fault injection points

[James Houghton]

KVM now supports three fault injection points:
1. fail_kvm_mmu_invalidate_retry, for all architectures.
2. fail_tdp_mmu_cmpxchg, for x86 only.
3. fail_tdp_mmu_resched, for x86 only.

Provide basic documentation for them.

Signed-off-by: James Houghton <jthoughton@google.com>
---
 Documentation/fault-injection/fault-injection.rst | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/Documentation/fault-injection/fault-injection.rst b/Documentation/fault-injection/fault-injection.rst
index c2d3996b5b407..da75c921d6c4d 100644
--- a/Documentation/fault-injection/fault-injection.rst
+++ b/Documentation/fault-injection/fault-injection.rst
@@ -87,6 +87,18 @@ Available fault injection capabilities
   inject init_hctx() errors by setting config items under
   /sys/kernel/config/nullb/<disk>/init_hctx_fault_inject.
 
+- fail_kvm_mmu_invalidate_retry
+
+  For KVM, injects fake MMU invalidations.
+
+- fail_tdp_mmu_cmpxchg
+
+  For KVM/x86, injects cmpxchg failures for TDP MMU SPTE updates.
+
+- fail_tdp_mmu_resched
+
+  For KVM/x86, injects fake MMU lock contention for TDP MMU SPTE iteration.
+
 Configure fault-injection capabilities behavior
 -----------------------------------------------
 
-- 
2.50.1.703.g449372360f-goog