KVM: MMU: performance tweaks for heavy CR0.WP users

There is no need to unload the MMU roots with TDP enabled when only CR0.WP has changed -- the paging structures are still valid, only the permission bitmap needs to be updated.

One heavy user of toggling CR0.WP is grsecurity's KERNEXEC feature to implement kernel W^X.

The optimization brings a huge performance gain for this case as the following micro-benchmark running 'ssdd 10 50000' from rt-tests[1] on a grsecurity L1 VM shows (runtime in seconds, lower is better):

legacy TDP shadow
kvm-x86/next@d8708b 8.43s 9.45s 70.3s

              +patch     5.39s    5.63s    70.2s

For legacy MMU this is ~36% faster, for TTP MMU even ~40% faster.

Quoted 11 lines · Mathias Krause

There is no need to unload the MMU roots with TDP enabled when only CR0.WP has changed -- the paging structures are still valid, only the permission bitmap needs to be updated.

One heavy user of toggling CR0.WP is grsecurity's KERNEXEC feature to implement kernel W^X.

The optimization brings a huge performance gain for this case as the following micro-benchmark running 'ssdd 10 50000' from rt-tests[1] on a grsecurity L1 VM shows (runtime in seconds, lower is better):

Quoted 5 lines

                        legacy     TDP    shadow
kvm-x86/next@d8708b     8.43s    9.45s    70.3s
              +patch     5.39s    5.63s    70.2s

For legacy MMU this is ~36% faster, for TTP MMU even ~40% faster.

TTP --> TDP

Quoted 14 lines

  void kvm_post_set_cr0(struct kvm_vcpu *vcpu, unsigned long old_cr0, unsigned long cr0)
  {
+    /*
+     * CR0.WP is incorporated into the MMU role, but only for non-nested,
+     * indirect shadow MMUs.  If TDP is enabled, the MMU's metadata needs
+     * to be updated, e.g. so that emulating guest translations does the +     * right thing, but there's no need to unload the root as CR0.WP +     * doesn't affect SPTEs.
+     */
+    if (tdp_enabled && (cr0 ^ old_cr0) == X86_CR0_WP) {

Curiously, this patch only affects tdp_enabled, why does legacy MMU also see comparable performance gains?

Quoted 14 lines

Quoted 13 lines

  void kvm_post_set_cr0(struct kvm_vcpu *vcpu, unsigned long old_cr0,
unsigned long cr0)
  {
+    /*
+     * CR0.WP is incorporated into the MMU role, but only for non-nested,
+     * indirect shadow MMUs.  If TDP is enabled, the MMU's metadata needs
+     * to be updated, e.g. so that emulating guest translations does the +     * right thing, but there's no need to unload the root as CR0.WP +     * doesn't affect SPTEs.
+     */
+    if (tdp_enabled && (cr0 ^ old_cr0) == X86_CR0_WP) {

Quoted 2 lines

Curiously, this patch only affects tdp_enabled, why does legacy MMU also see comparable performance gains?

Because 'tdp_enabled' just implies EPT / NPT and only 'tdp_mmu_enabled' decides which MMU mode to use -- either legacy or TDP MMU (see kvm_configure_mmu() and now gets invoked from vmx.c / svm.c).

Guests like grsecurity that make heavy use of CR0.WP to implement kernel level W^X will suffer from the implied VMEXITs.

With EPT there is no need to intercept a guest change of CR0.WP, so simply make it a guest owned bit if we can do so.

This implies that a read of a guest's CR0.WP bit might need a VMREAD. However, the only potentially affected user seems to be kvm_init_mmu() which is a heavy operation to begin with. But also most callers already cache the full value of CR0 anyway, so no additional VMREAD is needed. The only exception is nested_vmx_load_cr3().

This change is VMX-specific, as SVM has no such fine grained control register intercept control.

Suggested-and-co-developed-by Sean Christopherson <[email protected]>

Signed-off-by Mathias Krause <[email protected]>

---
  arch/x86/kvm/kvm_cache_regs.h |  2 +-
  arch/x86/kvm/vmx/nested.c     |  4 ++--
  arch/x86/kvm/vmx/vmx.c        |  2 +-
  arch/x86/kvm/vmx/vmx.h        | 18 ++++++++++++++++++
  4 files changed, 22 insertions(+), 4 deletions(-)

arch/x86/kvm/kvm_cache_regs.h +1 -1

index 4c91f626c058..e50d353b5c1c 100644
@@ -4,7 +4,7 @@
 
  #include <linux/kvm_host.h>
 
-#define KVM_POSSIBLE_CR0_GUEST_BITS X86_CR0_TS
+#define KVM_POSSIBLE_CR0_GUEST_BITS	(X86_CR0_TS | X86_CR0_WP)
  #define KVM_POSSIBLE_CR4_GUEST_BITS				  \
  	(X86_CR4_PVI | X86_CR4_DE | X86_CR4_PCE | X86_CR4_OSFXSR  \
  	 | X86_CR4_OSXMMEXCPT | X86_CR4_PGE | X86_CR4_TSD | X86_CR4_FSGSBASE)

arch/x86/kvm/vmx/nested.c +2 -2

index f63b28f46a71..61d940fc91ba 100644
@@ -4481,7 +4481,7 @@ static void load_vmcs12_host_state(struct kvm_vcpu *vcpu,
  	 * CR0_GUEST_HOST_MASK is already set in the original vmcs01
  	 * (KVM doesn't change it);
  	 */
-	vcpu->arch.cr0_guest_owned_bits = KVM_POSSIBLE_CR0_GUEST_BITS;
+	vcpu->arch.cr0_guest_owned_bits = vmx_l1_guest_owned_cr0_bits();
  	vmx_set_cr0(vcpu, vmcs12->host_cr0);
 
  	/* Same as above - no reason to call set_cr4_guest_host_mask().  */

@@ -4632,7 +4632,7 @@ static void nested_vmx_restore_host_state(struct kvm_vcpu *vcpu)
  	 */
  	vmx_set_efer(vcpu, nested_vmx_get_vmcs01_guest_efer(vmx));
 
-	vcpu->arch.cr0_guest_owned_bits = KVM_POSSIBLE_CR0_GUEST_BITS;
+	vcpu->arch.cr0_guest_owned_bits = vmx_l1_guest_owned_cr0_bits();
  	vmx_set_cr0(vcpu, vmcs_readl(CR0_READ_SHADOW));
 
  	vcpu->arch.cr4_guest_owned_bits = ~vmcs_readl(CR4_GUEST_HOST_MASK);

arch/x86/kvm/vmx/vmx.c +1 -1

index 8fc1a0c7856f..e501f6864a72 100644
@@ -4790,7 +4790,7 @@ static void init_vmcs(struct vcpu_vmx *vmx)
  	/* 22.2.1, 20.8.1 */
  	vm_entry_controls_set(vmx, vmx_vmentry_ctrl());
 
-	vmx->vcpu.arch.cr0_guest_owned_bits = KVM_POSSIBLE_CR0_GUEST_BITS;
+	vmx->vcpu.arch.cr0_guest_owned_bits = vmx_l1_guest_owned_cr0_bits();
  	vmcs_writel(CR0_GUEST_HOST_MASK, ~vmx->vcpu.arch.cr0_guest_owned_bits);
 
  	set_cr4_guest_host_mask(vmx);

arch/x86/kvm/vmx/vmx.h +18 -0

index 2acdc54bc34b..423e9d3c9c40 100644
@@ -640,6 +640,24 @@ BUILD_CONTROLS_SHADOW(tertiary_exec, TERTIARY_VM_EXEC_CONTROL, 64)
  				(1 << VCPU_EXREG_EXIT_INFO_1) | \
  				(1 << VCPU_EXREG_EXIT_INFO_2))
 
+static inline unsigned long vmx_l1_guest_owned_cr0_bits(void)
+{
+	unsigned long bits = KVM_POSSIBLE_CR0_GUEST_BITS;
+
+	/*
+	 * CR0.WP needs to be intercepted when KVM is shadowing legacy paging
+	 * in order to construct shadow PTEs with the correct protections.
+	 * Note!  CR0.WP technically can be passed through to the guest if
+	 * paging is disabled, but checking CR0.PG would generate a cyclical
+	 * dependency of sorts due to forcing the caller to ensure CR0 holds
+	 * the correct value prior to determining which CR0 bits can be owned
+	 * by L1.  Keep it simple and limit the optimization to EPT.
+	 */
+	if (!enable_ept)
+		bits &= ~X86_CR0_WP;
+	return bits;
+}
+
  static __always_inline struct kvm_vmx *to_kvm_vmx(struct kvm *kvm)
  {
  	return container_of(kvm, struct kvm_vmx, kvm);

Quoted 5 lines · Mathias Krause

Guests like grsecurity that make heavy use of CR0.WP to implement kernel level W^X will suffer from the implied VMEXITs.

With EPT there is no need to intercept a guest change of CR0.WP, so simply make it a guest owned bit if we can do so.

I'm interested in the performance gain. Do you have data like Patch 2?

Quoted 8 lines · Xiaoyao Li

Quoted 5 lines · Mathias Krause

Guests like grsecurity that make heavy use of CR0.WP to implement kernel level W^X will suffer from the implied VMEXITs.

With EPT there is no need to intercept a guest change of CR0.WP, so simply make it a guest owned bit if we can do so.

I'm interested in the performance gain. Do you have data like Patch 2?

It's mentioned in the cover letter[1], quoted below:

[1]
https://lore.kernel.org/kvm/[email protected]/

: I used 'ssdd 10 50000' from rt-tests[5] as a micro-benchmark, running on a : grsecurity L1 VM. Below table shows the results (runtime in seconds, lower : is better):
:
: legacy TDP shadow

:     kvm-x86/next@d8708b        8.43s    9.45s    70.3s
:     + patches 1-3              5.39s    5.63s    70.2s
:     + patches 4-6              3.51s    3.47s    67.8s

Thanks,
Mathias

Guests like grsecurity that make heavy use of CR0.WP to implement kernel level W^X will suffer from the implied VMEXITs.

With EPT there is no need to intercept a guest change of CR0.WP, so simply make it a guest owned bit if we can do so.

This implies that a read of a guest's CR0.WP bit might need a VMREAD. However, the only potentially affected user seems to be kvm_init_mmu() which is a heavy operation to begin with. But also most callers already cache the full value of CR0 anyway, so no additional VMREAD is needed. The only exception is nested_vmx_load_cr3().

This change is VMX-specific, as SVM has no such fine grained control register intercept control.

Suggested-and-co-developed-by Sean Christopherson <[email protected]>

Signed-off-by Mathias Krause <[email protected]>

---
 arch/x86/kvm/kvm_cache_regs.h |  2 +-
 arch/x86/kvm/vmx/nested.c     |  4 ++--
 arch/x86/kvm/vmx/vmx.c        |  2 +-
 arch/x86/kvm/vmx/vmx.h        | 18 ++++++++++++++++++
 4 files changed, 22 insertions(+), 4 deletions(-)

arch/x86/kvm/kvm_cache_regs.h +1 -1

index 4c91f626c058..e50d353b5c1c 100644
@@ -4,7 +4,7 @@
 
 #include <linux/kvm_host.h>
 
-#define KVM_POSSIBLE_CR0_GUEST_BITS X86_CR0_TS
+#define KVM_POSSIBLE_CR0_GUEST_BITS	(X86_CR0_TS | X86_CR0_WP)
 #define KVM_POSSIBLE_CR4_GUEST_BITS				  \
 	(X86_CR4_PVI | X86_CR4_DE | X86_CR4_PCE | X86_CR4_OSFXSR  \
 	 | X86_CR4_OSXMMEXCPT | X86_CR4_PGE | X86_CR4_TSD | X86_CR4_FSGSBASE)

arch/x86/kvm/vmx/nested.c +2 -2

index f63b28f46a71..61d940fc91ba 100644
@@ -4481,7 +4481,7 @@ static void load_vmcs12_host_state(struct kvm_vcpu *vcpu,
 	 * CR0_GUEST_HOST_MASK is already set in the original vmcs01
 	 * (KVM doesn't change it);
 	 */
-	vcpu->arch.cr0_guest_owned_bits = KVM_POSSIBLE_CR0_GUEST_BITS;
+	vcpu->arch.cr0_guest_owned_bits = vmx_l1_guest_owned_cr0_bits();
 	vmx_set_cr0(vcpu, vmcs12->host_cr0);
 
 	/* Same as above - no reason to call set_cr4_guest_host_mask().  */

@@ -4632,7 +4632,7 @@ static void nested_vmx_restore_host_state(struct kvm_vcpu *vcpu)
 	 */
 	vmx_set_efer(vcpu, nested_vmx_get_vmcs01_guest_efer(vmx));
 
-	vcpu->arch.cr0_guest_owned_bits = KVM_POSSIBLE_CR0_GUEST_BITS;
+	vcpu->arch.cr0_guest_owned_bits = vmx_l1_guest_owned_cr0_bits();
 	vmx_set_cr0(vcpu, vmcs_readl(CR0_READ_SHADOW));
 
 	vcpu->arch.cr4_guest_owned_bits = ~vmcs_readl(CR4_GUEST_HOST_MASK);

arch/x86/kvm/vmx/vmx.c +1 -1

index 8fc1a0c7856f..e501f6864a72 100644
@@ -4790,7 +4790,7 @@ static void init_vmcs(struct vcpu_vmx *vmx)
 	/* 22.2.1, 20.8.1 */
 	vm_entry_controls_set(vmx, vmx_vmentry_ctrl());
 
-	vmx->vcpu.arch.cr0_guest_owned_bits = KVM_POSSIBLE_CR0_GUEST_BITS;
+	vmx->vcpu.arch.cr0_guest_owned_bits = vmx_l1_guest_owned_cr0_bits();
 	vmcs_writel(CR0_GUEST_HOST_MASK, ~vmx->vcpu.arch.cr0_guest_owned_bits);
 
 	set_cr4_guest_host_mask(vmx);

arch/x86/kvm/vmx/vmx.h +18 -0

index 2acdc54bc34b..423e9d3c9c40 100644
@@ -640,6 +640,24 @@ BUILD_CONTROLS_SHADOW(tertiary_exec, TERTIARY_VM_EXEC_CONTROL, 64)
 				(1 << VCPU_EXREG_EXIT_INFO_1) | \
 				(1 << VCPU_EXREG_EXIT_INFO_2))
 
+static inline unsigned long vmx_l1_guest_owned_cr0_bits(void)
+{
+	unsigned long bits = KVM_POSSIBLE_CR0_GUEST_BITS;
+
+	/*
+	 * CR0.WP needs to be intercepted when KVM is shadowing legacy paging
+	 * in order to construct shadow PTEs with the correct protections.
+	 * Note!  CR0.WP technically can be passed through to the guest if
+	 * paging is disabled, but checking CR0.PG would generate a cyclical
+	 * dependency of sorts due to forcing the caller to ensure CR0 holds
+	 * the correct value prior to determining which CR0 bits can be owned
+	 * by L1.  Keep it simple and limit the optimization to EPT.
+	 */
+	if (!enable_ept)
+		bits &= ~X86_CR0_WP;
+	return bits;
+}
+
 static __always_inline struct kvm_vmx *to_kvm_vmx(struct kvm *kvm)
 {
 	return container_of(kvm, struct kvm_vmx, kvm);

Quoted 6 lines · Mathias Krause

Guests like grsecurity that make heavy use of CR0.WP to implement kernel level W^X will suffer from the implied VMEXITs.

With EPT there is no need to intercept a guest change of CR0.WP, so simply make it a guest owned bit if we can do so.

Quoted 8 lines

This implies that a read of a guest's CR0.WP bit might need a VMREAD. However, the only potentially affected user seems to be kvm_init_mmu() which is a heavy operation to begin with. But also most callers already cache the full value of CR0 anyway, so no additional VMREAD is needed. The only exception is nested_vmx_load_cr3().

This change is VMX-specific, as SVM has no such fine grained control register intercept control.

Just a heads up! We did more tests, especially with the backports we did internally already, and ran into a bug when running a nested guest on an ESXi host.

Setup is like: ESXi (L0) -> Linux (L1) -> Linux (L2)

The Linux system, especially the kernel, is the same for L1 and L2. It's a grsecurity kernel, so makes use of toggling CR0.WP at runtime.

The bug we see is that when L2 disables CR0.WP and tries to write to an r/o memory region (implicitly to the r/o GDT via LTR in our use case), this triggers a fault (EPT violation?) that gets ignored by L1, as, apparently, everything is fine from its point of view.

I suspect the L0 VMM to be at fault here, as the VMCS structures look good, IMO. Here is a dump of vmx->loaded_vmcs in handle_triple_fault():

The "host" (which is our L1 VMM, I guess) has CR0.WP enabled and that is what I think confuses ESXi to enforce the read-only property to the L2 guest as well -- for unknown reasons so far.

I tried to reproduce the bug with different KVM based L0 VMMs (with and without this series; vanilla and grsecurity kernels) but no luck. That's why I'm suspecting a ESXi bug.

I'm leaning to make CR0.WP guest owned only iff we're running on bare metal or the VMM is KVM to not play whack-a-mole for all the VMMs that might have similar bugs. (Will try to test a few others here as well.) However, that would prevent them from getting the performance gain, so I'd rather have this fixed / worked around in KVM instead.

Any ideas how to investigate this further?

PS: ...should have left the chicken bit of v3 to be able to disable the feature by a module parameter ;)

Quoted 9 lines · Mathias Krause

Just a heads up! We did more tests, especially with the backports we did internally already, and ran into a bug when running a nested guest on an ESXi host.

Setup is like: ESXi (L0) -> Linux (L1) -> Linux (L2)

The Linux system, especially the kernel, is the same for L1 and L2. It's a grsecurity kernel, so makes use of toggling CR0.WP at runtime.

Quoted 4 lines

The bug we see is that when L2 disables CR0.WP and tries to write to an r/o memory region (implicitly to the r/o GDT via LTR in our use case), this triggers a fault (EPT violation?) that gets ignored by L1, as, apparently, everything is fine from its point of view.

It's not an EPT violation if there's a triple fault. Given that you're dumping the VMCS from handle_triple_fault(), I assume that L2 gets an unexpected #PF that leads to a triple fault in L2.

Just to make sure we're on the same page: L1 is still alive after this, correct?

Quoted 2 lines

I suspect the L0 VMM to be at fault here, as the VMCS structures look good, IMO. Here is a dump of vmx->loaded_vmcs in handle_triple_fault():

...

Quoted 3 lines

The "host" (which is our L1 VMM, I guess) has CR0.WP enabled and that is what I think confuses ESXi to enforce the read-only property to the L2 guest as well -- for unknown reasons so far.

...

Quoted 3 lines

I tried to reproduce the bug with different KVM based L0 VMMs (with and without this series; vanilla and grsecurity kernels) but no luck. That's why I'm suspecting a ESXi bug.

...

Quoted 5 lines

I'm leaning to make CR0.WP guest owned only iff we're running on bare metal or the VMM is KVM to not play whack-a-mole for all the VMMs that might have similar bugs. (Will try to test a few others here as well.) However, that would prevent them from getting the performance gain, so I'd rather have this fixed / worked around in KVM instead.

Before we start putting bandaids on this, let's (a) confirm this appears to be an issue with ESXi and (b) pull in VMware folks to get their input.

Quoted 1 line

Any ideas how to investigate this further?

Does the host in question support UMIP?

Can you capture a tracepoint log from L1 to verify that L1 KVM is _not_ injecting any kind of exception? E.g. to get the KVM kitchen sink:

echo 1 > /sys/kernel/debug/tracing/tracing_on echo 1 > /sys/kernel/debug/tracing/events/kvm/enable

cat /sys/kernel/debug/tracing/trace > log

Or if that's too noisy, a more targeted trace (exception injection + emulation) woud be:

[...]

Quoted 2 lines

PS: ...should have left the chicken bit of v3 to be able to disable the feature by a module parameter ;)

A chicken bit isn't a good solution for this sort of thing. Toggling a KVM module param requires (a) knowing that it exists and (b) knowing the conditions under which it is/isn't safe to toggle the bit.

E.g. if this ends up being an ESXi L0 bug, then an option might be to add something in vmware_platform_setup() to communicate the bug to KVM so that KVM can precisely disable the optimization on affected platforms.

[...]
Maybe it's not a stale CR0 value but the page table walker not taking the guest's CR0.WP into account? Maybe a missing role update?

Quoted 3 lines · Mathias Krause

[...]
Maybe it's not a stale CR0 value but the page table walker not taking the guest's CR0.WP into account? Maybe a missing role update?

Indeed, it is. This does the trick for me:

arch/x86/kvm/x86.c +3 -0

index 31be188aa842..6a9e90725c84 100644
@@ -8372,6 +8372,9 @@ static void init_emulate_ctxt(struct kvm_vcpu *vcpu)
 
        init_decode_cache(ctxt);
        vcpu->arch.emulate_regs_need_sync_from_vcpu = false;
+       /* Ensure we're doing page table walks with an up2date MMU role */
+       if ((vcpu->arch.cr0 ^ kvm_read_cr0(vcpu)) == X86_CR0_WP)
+               kvm_init_mmu(vcpu);
 }
 
 void kvm_inject_realmode_interrupt(struct kvm_vcpu *vcpu, int irq, int inc_eip)

Very heavy weight and misplaced, but a start :)

It should (1) be limited to VMX as that's the only one that would make CR0.WP a guest owned bit and (2) limited to emulated instructions that actually do write operations, as read are harmless, obviously.

Quoted 6 lines · Mathias Krause

On 22.03.23 02:37, Mathias Krause wrote:
I'm leaning to make CR0.WP guest owned only iff we're running on bare metal or the VMM is KVM to not play whack-a-mole for all the VMMs that might have similar bugs. (Will try to test a few others here as well.) However, that would prevent them from getting the performance gain, so I'd rather have this fixed / worked around in KVM instead.

Before we start putting bandaids on this, let's (a) confirm this appears to be an issue with ESXi and (b) pull in VMware folks to get their input.

Quoted 1 line

Any ideas how to investigate this further?

Does the host in question support UMIP?

Can you capture a tracepoint log from L1 to verify that L1 KVM is _not_ injecting any kind of exception? E.g. to get the KVM kitchen sink:

echo 1 > /sys/kernel/debug/tracing/tracing_on echo 1 > /sys/kernel/debug/tracing/events/kvm/enable

cat /sys/kernel/debug/tracing/trace > log

Or if that's too noisy, a more targeted trace (exception injection + emulation) woud be:

echo 1 > /sys/kernel/debug/tracing/tracing_on

echo 1 > /sys/kernel/debug/tracing/events/kvm/kvm_emulate_insn/enable echo 1 > /sys/kernel/debug/tracing/events/kvm/kvm_inj_exception/enable echo 1 > /sys/kernel/debug/tracing/events/kvm/kvm_entry/enable echo 1 > /sys/kernel/debug/tracing/events/kvm/kvm_exit/enable

Duh, this is likely a KVM bug. I expect the issue will go away if you revert

fb509f76acc8 ("KVM: VMX: Make CR0.WP a guest owned bit")

KVM doesn't consume CR0.WP for _its_ MMU, but it does consume CR0.WP for the guest walker. By passing through CR0.WP, toggling only CR0.WP will not trap (obviously) and thus won't run through kvm_post_set_cr0(), thus resulting in stle information due to not invoking kvm_init_mmu().

I'm preetty sure I even called that we needed to refresh the permissions, but then obviously forgot to actually make that happen.

I believe this will remedy the issue. If it does, I'll post a proper patch (likely won't be until next week). Compile tested only.

---
 arch/x86/kvm/mmu.h     |  8 +++++++-
 arch/x86/kvm/mmu/mmu.c | 14 ++++++++++++++
 2 files changed, 21 insertions(+), 1 deletion(-)

arch/x86/kvm/mmu.h +7 -1

index 89f532516a45..4a303aa735dd 100644
@@ -113,6 +113,7 @@ void kvm_init_shadow_ept_mmu(struct kvm_vcpu *vcpu, bool execonly,
 bool kvm_can_do_async_pf(struct kvm_vcpu *vcpu);
 int kvm_handle_page_fault(struct kvm_vcpu *vcpu, u64 error_code,
 				u64 fault_address, char *insn, int insn_len);
+void kvm_mmu_refresh_passthrough_bits(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu);
 
 int kvm_mmu_load(struct kvm_vcpu *vcpu);
 void kvm_mmu_unload(struct kvm_vcpu *vcpu);

@@ -184,8 +185,13 @@ static inline u8 permission_fault(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu,
 	u64 implicit_access = access & PFERR_IMPLICIT_ACCESS;
 	bool not_smap = ((rflags & X86_EFLAGS_AC) | implicit_access) == X86_EFLAGS_AC;
 	int index = (pfec + (not_smap << PFERR_RSVD_BIT)) >> 1;
-	bool fault = (mmu->permissions[index] >> pte_access) & 1;
 	u32 errcode = PFERR_PRESENT_MASK;
+	bool fault;
+
+	if (tdp_enabled)
+		kvm_mmu_refresh_passthrough_bits(vcpu, mmu);
+
+	fault = (mmu->permissions[index] >> pte_access) & 1;
 
 	WARN_ON(pfec & (PFERR_PK_MASK | PFERR_RSVD_MASK));
 	if (unlikely(mmu->pkru_mask)) {

arch/x86/kvm/mmu/mmu.c +14 -0

index 4c874d4ec68f..2a63b5725f36 100644
@@ -5186,6 +5186,20 @@ static union kvm_cpu_role kvm_calc_cpu_role(struct kvm_vcpu *vcpu,
 	return role;
 }
 
+void kvm_mmu_refresh_passthrough_bits(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu)
+{
+	const bool cr0_wp = kvm_is_cr0_bit_set(vcpu, X86_CR0_WP);
+
+	BUILD_BUG_ON((KVM_MMU_CR0_ROLE_BITS & KVM_POSSIBLE_CR0_GUEST_BITS) != X86_CR0_WP);
+	BUILD_BUG_ON((KVM_MMU_CR4_ROLE_BITS & KVM_POSSIBLE_CR4_GUEST_BITS));
+
+	if (is_cr0_wp(mmu) == cr0_wp)
+		return;
+
+	mmu->cpu_role.base.cr0_wp = cr0_wp;
+	reset_guest_paging_metadata(vcpu, mmu);
+}
+
 static inline int kvm_mmu_get_tdp_level(struct kvm_vcpu *vcpu)
 {
 	/* tdp_root_level is architecture forced level, use it if nonzero */

base-commit: 27d6845d258b67f4eb3debe062b7dacc67e0c393

Quoted 61 lines · Sean Christopherson

I believe this will remedy the issue. If it does, I'll post a proper patch (likely won't be until next week). Compile tested only.

---
 arch/x86/kvm/mmu.h     |  8 +++++++-
 arch/x86/kvm/mmu/mmu.c | 14 ++++++++++++++
 2 files changed, 21 insertions(+), 1 deletion(-)

arch/x86/kvm/mmu.h +7 -1

index 89f532516a45..4a303aa735dd 100644
@@ -113,6 +113,7 @@ void kvm_init_shadow_ept_mmu(struct kvm_vcpu *vcpu, bool execonly,
 bool kvm_can_do_async_pf(struct kvm_vcpu *vcpu);
 int kvm_handle_page_fault(struct kvm_vcpu *vcpu, u64 error_code,
 				u64 fault_address, char *insn, int insn_len);
+void kvm_mmu_refresh_passthrough_bits(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu);
 
 int kvm_mmu_load(struct kvm_vcpu *vcpu);
 void kvm_mmu_unload(struct kvm_vcpu *vcpu);

@@ -184,8 +185,13 @@ static inline u8 permission_fault(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu,
 	u64 implicit_access = access & PFERR_IMPLICIT_ACCESS;
 	bool not_smap = ((rflags & X86_EFLAGS_AC) | implicit_access) == X86_EFLAGS_AC;
 	int index = (pfec + (not_smap << PFERR_RSVD_BIT)) >> 1;
-	bool fault = (mmu->permissions[index] >> pte_access) & 1;
 	u32 errcode = PFERR_PRESENT_MASK;
+	bool fault;
+
+	if (tdp_enabled)
+		kvm_mmu_refresh_passthrough_bits(vcpu, mmu);
+
+	fault = (mmu->permissions[index] >> pte_access) & 1;
 
 	WARN_ON(pfec & (PFERR_PK_MASK | PFERR_RSVD_MASK));
 	if (unlikely(mmu->pkru_mask)) {

arch/x86/kvm/mmu/mmu.c +14 -0

index 4c874d4ec68f..2a63b5725f36 100644
@@ -5186,6 +5186,20 @@ static union kvm_cpu_role kvm_calc_cpu_role(struct kvm_vcpu *vcpu,
 	return role;
 }
 
+void kvm_mmu_refresh_passthrough_bits(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu)
+{
+	const bool cr0_wp = kvm_is_cr0_bit_set(vcpu, X86_CR0_WP);
+
+	BUILD_BUG_ON((KVM_MMU_CR0_ROLE_BITS & KVM_POSSIBLE_CR0_GUEST_BITS) != X86_CR0_WP);
+	BUILD_BUG_ON((KVM_MMU_CR4_ROLE_BITS & KVM_POSSIBLE_CR4_GUEST_BITS));
+
+	if (is_cr0_wp(mmu) == cr0_wp)
+		return;
+
+	mmu->cpu_role.base.cr0_wp = cr0_wp;
+	reset_guest_paging_metadata(vcpu, mmu);
+}
+
 static inline int kvm_mmu_get_tdp_level(struct kvm_vcpu *vcpu)
 {
 	/* tdp_root_level is architecture forced level, use it if nonzero */

Quoted 1 line

base-commit: 27d6845d258b67f4eb3debe062b7dacc67e0c393

I tested a backported version of this patch on v6.1 as that's what I was testing with and it worked fine. :)

I'll do more thorough tests tomorrow and actually on kvm-x86/next's HEAD.

Bonus rant^Wbug report:
[VMs #GP'ing on WRMSR(IA32_PRED_CMD,...)]

v3: https://lore.kernel.org/kvm/[email protected]/

This series is the fourth iteration of resurrecting the missing pieces of Paolo's previous attempt[1] to avoid needless MMU roots unloading.

It's incorporating Sean's feedback to v3 and rebased on top of kvm-x86/next, namely commit d8708b80fa0e ("KVM: Change return type of kvm_arch_vm_ioctl() to "int"").

[...]

Quoted 10 lines · Mathias Krause

v3: https://lore.kernel.org/kvm/[email protected]/

This series is the fourth iteration of resurrecting the missing pieces of Paolo's previous attempt[1] to avoid needless MMU roots unloading.

It's incorporating Sean's feedback to v3 and rebased on top of kvm-x86/next, namely commit d8708b80fa0e ("KVM: Change return type of kvm_arch_vm_ioctl() to "int"").

[...]

Applied 1 and 5 to kvm-x86 mmu, and the rest to kvm-x86 misc, thanks!

[1/6] KVM: x86/mmu: Avoid indirect call for get_cr3

      https://github.com/kvm-x86/linux/commit/2fdcc1b32418
[2/6] KVM: x86: Do not unload MMU roots when only toggling CR0.WP with TDP enabled
      https://github.com/kvm-x86/linux/commit/01b31714bd90
[3/6] KVM: x86: Ignore CR0.WP toggles in non-paging mode
      https://github.com/kvm-x86/linux/commit/e40bcf9f3a18
[4/6] KVM: x86: Make use of kvm_read_cr*_bits() when testing bits
      https://github.com/kvm-x86/linux/commit/74cdc836919b
[5/6] KVM: x86/mmu: Fix comment typo
      https://github.com/kvm-x86/linux/commit/50f13998451e
[6/6] KVM: VMX: Make CR0.WP a guest owned bit
      https://github.com/kvm-x86/linux/commit/fb509f76acc8

Quoted 15 lines · Sean Christopherson

Quoted 10 lines · Mathias Krause

v3: https://lore.kernel.org/kvm/[email protected]/

This series is the fourth iteration of resurrecting the missing pieces of Paolo's previous attempt[1] to avoid needless MMU roots unloading.

It's incorporating Sean's feedback to v3 and rebased on top of kvm-x86/next, namely commit d8708b80fa0e ("KVM: Change return type of kvm_arch_vm_ioctl() to "int"").

[...]

Applied 1 and 5 to kvm-x86 mmu, and the rest to kvm-x86 misc, thanks!

[1/6] KVM: x86/mmu: Avoid indirect call for get_cr3

Quoted 11 lines

      https://github.com/kvm-x86/linux/commit/2fdcc1b32418
[2/6] KVM: x86: Do not unload MMU roots when only toggling CR0.WP with TDP enabled
      https://github.com/kvm-x86/linux/commit/01b31714bd90
[3/6] KVM: x86: Ignore CR0.WP toggles in non-paging mode
      https://github.com/kvm-x86/linux/commit/e40bcf9f3a18
[4/6] KVM: x86: Make use of kvm_read_cr*_bits() when testing bits
      https://github.com/kvm-x86/linux/commit/74cdc836919b
[5/6] KVM: x86/mmu: Fix comment typo
      https://github.com/kvm-x86/linux/commit/50f13998451e
[6/6] KVM: VMX: Make CR0.WP a guest owned bit
      https://github.com/kvm-x86/linux/commit/fb509f76acc8

Thanks a lot, Sean!

As this is a huge performance fix for us, we'd like to get it integrated into current stable kernels as well -- not without having the changes get some wider testing, of course, i.e. not before they end up in a non-rc version released by Linus. But I already did a backport to 5.4 to get a feeling how hard it would be and for the impact it has on older kernels.

Using the 'ssdd 10 50000' test I used before, I get promising results there as well. Without the patches it takes 9.31s, while with them we're down to 4.64s. Taking into account that this is the runtime of a workload in a VM that gets cut in half, I hope this qualifies as stable material, as it's a huge performance fix.

Greg, what's your opinion on it? Original series here:
https://lore.kernel.org/kvm/[email protected]/

Quoted 43 lines · Mathias Krause

Quoted 15 lines · Sean Christopherson

Quoted 10 lines · Mathias Krause

v3: https://lore.kernel.org/kvm/[email protected]/

This series is the fourth iteration of resurrecting the missing pieces of Paolo's previous attempt[1] to avoid needless MMU roots unloading.

It's incorporating Sean's feedback to v3 and rebased on top of kvm-x86/next, namely commit d8708b80fa0e ("KVM: Change return type of kvm_arch_vm_ioctl() to "int"").

[...]

Applied 1 and 5 to kvm-x86 mmu, and the rest to kvm-x86 misc, thanks!

[1/6] KVM: x86/mmu: Avoid indirect call for get_cr3

Quoted 11 lines

      https://github.com/kvm-x86/linux/commit/2fdcc1b32418
[2/6] KVM: x86: Do not unload MMU roots when only toggling CR0.WP with TDP enabled
      https://github.com/kvm-x86/linux/commit/01b31714bd90
[3/6] KVM: x86: Ignore CR0.WP toggles in non-paging mode
      https://github.com/kvm-x86/linux/commit/e40bcf9f3a18
[4/6] KVM: x86: Make use of kvm_read_cr*_bits() when testing bits
      https://github.com/kvm-x86/linux/commit/74cdc836919b
[5/6] KVM: x86/mmu: Fix comment typo
      https://github.com/kvm-x86/linux/commit/50f13998451e
[6/6] KVM: VMX: Make CR0.WP a guest owned bit
      https://github.com/kvm-x86/linux/commit/fb509f76acc8

Thanks a lot, Sean!

As this is a huge performance fix for us, we'd like to get it integrated into current stable kernels as well -- not without having the changes get some wider testing, of course, i.e. not before they end up in a non-rc version released by Linus. But I already did a backport to 5.4 to get a feeling how hard it would be and for the impact it has on older kernels.

Using the 'ssdd 10 50000' test I used before, I get promising results there as well. Without the patches it takes 9.31s, while with them we're down to 4.64s. Taking into account that this is the runtime of a workload in a VM that gets cut in half, I hope this qualifies as stable material, as it's a huge performance fix.

Quoted 2 lines

Greg, what's your opinion on it? Original series here:
https://lore.kernel.org/kvm/[email protected]/

I'll leave the judgement call up to the KVM maintainers, as they are the ones that need to ack any KVM patch added to stable trees.

Quoted 17 lines · Greg KH

Quoted 15 lines · Mathias Krause

As this is a huge performance fix for us, we'd like to get it integrated into current stable kernels as well -- not without having the changes get some wider testing, of course, i.e. not before they end up in a non-rc version released by Linus. But I already did a backport to 5.4 to get a feeling how hard it would be and for the impact it has on older kernels.

Using the 'ssdd 10 50000' test I used before, I get promising results there as well. Without the patches it takes 9.31s, while with them we're down to 4.64s. Taking into account that this is the runtime of a workload in a VM that gets cut in half, I hope this qualifies as stable material, as it's a huge performance fix.

Greg, what's your opinion on it? Original series here:
https://lore.kernel.org/kvm/[email protected]/

Quoted 2 lines

I'll leave the judgement call up to the KVM maintainers, as they are the ones that need to ack any KVM patch added to stable trees.

These are quite risky to backport. E.g. we botched patch 6[*], and my initial fix also had a subtle bug. There have also been quite a few KVM MMU changes since 5.4, so it's possible that an edge case may exist in 5.4 that doesn't exist in mainline.

I'm not totally opposed to the idea since our tests _should_ be provide solid coverage, e.g. existing tests caught my subtle bug, but I don't think we should backport these without a solid usecase, as there is a fairly high risk of breaking random KVM users that wouldn't see any meaningful benefit.

In other words, who cares enough about the performance of running grsecurity kernels in VMs to want these backported, but doesn't have the resources to maintain (or pay someone to maintain) their own host kernel?

[*] https://lkml.kernel.org/r/20230405002608.418442-1-seanjc%40google.com

Quoted 18 lines · Sean Christopherson

Quoted 17 lines · Greg KH

Quoted 15 lines · Mathias Krause

As this is a huge performance fix for us, we'd like to get it integrated into current stable kernels as well -- not without having the changes get some wider testing, of course, i.e. not before they end up in a non-rc version released by Linus. But I already did a backport to 5.4 to get a feeling how hard it would be and for the impact it has on older kernels.

Using the 'ssdd 10 50000' test I used before, I get promising results there as well. Without the patches it takes 9.31s, while with them we're down to 4.64s. Taking into account that this is the runtime of a workload in a VM that gets cut in half, I hope this qualifies as stable material, as it's a huge performance fix.

Greg, what's your opinion on it? Original series here:
https://lore.kernel.org/kvm/[email protected]/

Quoted 7 lines

Quoted 2 lines

I'll leave the judgement call up to the KVM maintainers, as they are the ones that need to ack any KVM patch added to stable trees.

These are quite risky to backport. E.g. we botched patch 6[*], and my initial fix also had a subtle bug. There have also been quite a few KVM MMU changes since 5.4, so it's possible that an edge case may exist in 5.4 that doesn't exist in mainline.

I totally agree. Getting the changes to work with older kernels needs more work. The MMU role handling was refactored in 5.14 and down to 5.4 it differs even more, so backports to earlier kernels definitely needs more care.

My plan would be to limit backporting of the whole series to kernels down to 5.15 (maybe 5.10 if it turns out to be doable) and for kernels before that only without patch 6. That would leave out the problematic change but still give us the benefits of dropping the needless mmu unloads for only toggling CR0.WP in the VM. This already helps us a lot!

Quoted 11 lines · Mathias Krause

Quoted 4 lines · Sean Christopherson

These are quite risky to backport. E.g. we botched patch 6[*], and my initial fix also had a subtle bug. There have also been quite a few KVM MMU changes since 5.4, so it's possible that an edge case may exist in 5.4 that doesn't exist in mainline.

I totally agree. Getting the changes to work with older kernels needs more work. The MMU role handling was refactored in 5.14 and down to 5.4 it differs even more, so backports to earlier kernels definitely needs more care.

Quoted 5 lines

My plan would be to limit backporting of the whole series to kernels down to 5.15 (maybe 5.10 if it turns out to be doable) and for kernels before that only without patch 6. That would leave out the problematic change but still give us the benefits of dropping the needless mmu unloads for only toggling CR0.WP in the VM. This already helps us a lot!

To back up the "helps us a lot" with some numbers, here are the results I got from running the 'ssdd 10 50000' micro-benchmark on the backports I did, running on a grsecurity L1 VM (host is a vanilla kernel, as stated below; runtime in seconds, lower is better):

                          legacy     TDP    shadow
    Linux v5.4.240          -        8.87s   56.8s
    + patches               -        5.84s   55.4s

Linux v5.10.177 10.37s 88.7s 69.7s
+ patches 4.88s 4.92s 70.1s

Linux v5.15.106 9.94s 66.1s 64.9s
+ patches 4.81s 4.79s 64.6s

Linux v6.1.23 7.65s 8.23s 68.7s
+ patches 3.36s 3.36s 69.1s

Linux v6.2.10 7.61s 7.98s 68.6s
+ patches 3.37s 3.41s 70.2s

I guess we can grossly ignore the shadow MMU numbers, beside noting them to regress from v5.4 to v5.10 (something to investigate?). The backports don't help (much) for shadow MMU setups and the flux in the measurements is likely related to the slab allocations involved.

Another unrelated data point is that TDP MMU is really broken for our use case on v5.10 and v5.15 -- it's even slower that shadow paging!

OTOH, the backports give nice speed-ups, ranging from ~2.2 times faster for pure EPT (legacy) MMU setups up to 18(!!!) times faster for TDP MMU on v5.10.

I backported the whole series down to v5.10 but left out the CR0.WP guest owning patch+fix for v5.4 as the code base is too different to get all the nuances right, as Sean already hinted. However, even this limited backport provides a big performance fix for our use case!

+Jeremi

Quoted 3 lines

OTOH, the backports give nice speed-ups, ranging from ~2.2 times faster for pure EPT (legacy) MMU setups up to 18(!!!) times faster for TDP MMU on v5.10.

Anyone that's enabling the TDP MMU on v5.10 is on their own, we didn't enable the TDP MMU by default until v5.14 for very good reasons.

Quoted 4 lines

I backported the whole series down to v5.10 but left out the CR0.WP guest owning patch+fix for v5.4 as the code base is too different to get all the nuances right, as Sean already hinted. However, even this limited backport provides a big performance fix for our use case!

As a compromise of sorts, I propose that we disable the TDP MMU by default on v5.15, and backport these fixes to v6.1. v5.15 and earlier won't get "ludicrous speed", but I think that's perfectly acceptable since KVM has had the suboptimal behavior literally since EPT/NPT support was first added.

I'm comfortable backporting to v6.1 as that is recent enough, and there weren't substantial MMU changes between v6.1 and v6.3 in this area. I.e. I have a decent level of confidence that we aren't overlooking some subtle dependency.

For v5.15, I am less confident in the safety of a backport, and more importantly, I think we should disable the TDP MMU by default to mitigate the underlying flaw that makes the 18x speedup possible. That flaw is that KVM can end up freeing and rebuilding TDP MMU roots every time CR0.WP is toggled or a vCPU transitions to/from SMM.

We mitigated the CR0.WP case between v5.15 and v6.1[1], which is why v6.1 doesn't exhibit the same pain as v5.10, but Jeremi discovered that the SMM case badly affects KVM-on-HyperV[2], e.g. when lauching KVM guests using WSL. I posted a fix[3] to finally resolve the underlying bug, but as Jeremi discovered[4], backporting the fix to v5.15 is going to be gnarly, to say the least. It'll be far worse than backporting these CR0.WP patches, and maybe even infeasible without a large scale rework (no thanks).

Anyone that will realize meaningful benefits from the TDP MMU is all but guaranteed to be rolling their own kernels, i.e. can do the backports themselves if they want to use a v5.15 based kernel. The big selling point of the TDP MMU is that it scales better to hundreds of vCPUs, particularly when live migrating such VMs. I highly doubt that anyone running a stock kernel is running 100+ vCPU VMs, let alone trying to live migrate them.

[1] https://lkml.kernel.org/r/20220209170020.1775368-1-pbonzini%40redhat.com
[2] https://lore.kernel.org/all/[email protected]
[3] https://lore.kernel.org/all/[email protected]
[4] https://lore.kernel.org/all/[email protected]

Quoted 2 lines · Sean Christopherson

+Jeremi

Adding myself :)

Quoted 1 line · Mathias Krause

...

Quoted 12 lines

Quoted 3 lines

OTOH, the backports give nice speed-ups, ranging from ~2.2 times faster for pure EPT (legacy) MMU setups up to 18(!!!) times faster for TDP MMU on v5.10.

Anyone that's enabling the TDP MMU on v5.10 is on their own, we didn't enable the TDP MMU by default until v5.14 for very good reasons.

Quoted 4 lines

I backported the whole series down to v5.10 but left out the CR0.WP guest owning patch+fix for v5.4 as the code base is too different to get all the nuances right, as Sean already hinted. However, even this limited backport provides a big performance fix for our use case!

Quoted 5 lines

As a compromise of sorts, I propose that we disable the TDP MMU by default on v5.15, and backport these fixes to v6.1. v5.15 and earlier won't get "ludicrous speed", but I think that's perfectly acceptable since KVM has had the suboptimal behavior literally since EPT/NPT support was first added.

Disabling TDP MMU for v5.15, and backporting things to v6.1 works for me.

Quoted 10 lines

I'm comfortable backporting to v6.1 as that is recent enough, and there weren't substantial MMU changes between v6.1 and v6.3 in this area. I.e. I have a decent level of confidence that we aren't overlooking some subtle dependency.

For v5.15, I am less confident in the safety of a backport, and more importantly, I think we should disable the TDP MMU by default to mitigate the underlying flaw that makes the 18x speedup possible. That flaw is that KVM can end up freeing and rebuilding TDP MMU roots every time CR0.WP is toggled or a vCPU transitions to/from SMM.

The interesting thing here is that these CR0.WP fixes seem to improve things with legacy MMU as well, and legacy MMU is not affected/touched by [3].

So I think you can consider Mathias' ask independent of disabling TDP MMU. On the one
hand: there is no regression here. On the other: the gain is big and seems important to him.

Quoted 5 lines · Jeremi Piotrowski

Quoted 2 lines · Sean Christopherson

+Jeremi

Adding myself :)

/facepalm

This isn't some mundane detail, Michael!!!

Quoted 35 lines

Quoted 1 line · Mathias Krause

...

Quoted 12 lines

Quoted 3 lines

OTOH, the backports give nice speed-ups, ranging from ~2.2 times faster for pure EPT (legacy) MMU setups up to 18(!!!) times faster for TDP MMU on v5.10.

Anyone that's enabling the TDP MMU on v5.10 is on their own, we didn't enable the TDP MMU by default until v5.14 for very good reasons.

Quoted 4 lines

I backported the whole series down to v5.10 but left out the CR0.WP guest owning patch+fix for v5.4 as the code base is too different to get all the nuances right, as Sean already hinted. However, even this limited backport provides a big performance fix for our use case!

Quoted 5 lines

As a compromise of sorts, I propose that we disable the TDP MMU by default on v5.15, and backport these fixes to v6.1. v5.15 and earlier won't get "ludicrous speed", but I think that's perfectly acceptable since KVM has had the suboptimal behavior literally since EPT/NPT support was first added.

Disabling TDP MMU for v5.15, and backporting things to v6.1 works for me.

Quoted 10 lines

I'm comfortable backporting to v6.1 as that is recent enough, and there weren't substantial MMU changes between v6.1 and v6.3 in this area. I.e. I have a decent level of confidence that we aren't overlooking some subtle dependency.

For v5.15, I am less confident in the safety of a backport, and more importantly, I think we should disable the TDP MMU by default to mitigate the underlying flaw that makes the 18x speedup possible. That flaw is that KVM can end up freeing and rebuilding TDP MMU roots every time CR0.WP is toggled or a vCPU transitions to/from SMM.

+Jeremi

Quoted 23 lines · Mathias Krause

Quoted 11 lines · Mathias Krause

Quoted 4 lines · Sean Christopherson

These are quite risky to backport. E.g. we botched patch 6[*], and my initial fix also had a subtle bug. There have also been quite a few KVM MMU changes since 5.4, so it's possible that an edge case may exist in 5.4 that doesn't exist in mainline.

I totally agree. Getting the changes to work with older kernels needs more work. The MMU role handling was refactored in 5.14 and down to 5.4 it differs even more, so backports to earlier kernels definitely needs more care.

Quoted 5 lines

My plan would be to limit backporting of the whole series to kernels down to 5.15 (maybe 5.10 if it turns out to be doable) and for kernels before that only without patch 6. That would leave out the problematic change but still give us the benefits of dropping the needless mmu unloads for only toggling CR0.WP in the VM. This already helps us a lot!

To back up the "helps us a lot" with some numbers, here are the results I got from running the 'ssdd 10 50000' micro-benchmark on the backports I did, running on a grsecurity L1 VM (host is a vanilla kernel, as stated below; runtime in seconds, lower is better):

Quoted 3 lines

                          legacy     TDP    shadow
    Linux v5.4.240          -        8.87s   56.8s
    + patches               -        5.84s   55.4s

I believe "legacy" and "TDP" are flip-flopped, the TDP MMU does't exist in v5.4.

Quoted 21 lines

Linux v5.10.177 10.37s 88.7s 69.7s
+ patches 4.88s 4.92s 70.1s

Linux v5.15.106 9.94s 66.1s 64.9s
+ patches 4.81s 4.79s 64.6s

Linux v6.1.23 7.65s 8.23s 68.7s
+ patches 3.36s 3.36s 69.1s

Linux v6.2.10 7.61s 7.98s 68.6s
+ patches 3.37s 3.41s 70.2s

I guess we can grossly ignore the shadow MMU numbers, beside noting them to regress from v5.4 to v5.10 (something to investigate?). The backports don't help (much) for shadow MMU setups and the flux in the measurements is likely related to the slab allocations involved.

Another unrelated data point is that TDP MMU is really broken for our use case on v5.10 and v5.15 -- it's even slower that shadow paging!

Quoted 3 lines

OTOH, the backports give nice speed-ups, ranging from ~2.2 times faster for pure EPT (legacy) MMU setups up to 18(!!!) times faster for TDP MMU on v5.10.

Anyone that's enabling the TDP MMU on v5.10 is on their own, we didn't enable the TDP MMU by default until v5.14 for very good reasons.

Quoted 5 lines

I backported the whole series down to v5.10 but left out the CR0.WP guest owning patch+fix for v5.4 as the code base is too different to get all the nuances right, as Sean already hinted. However, even this limited backport provides a big performance fix for our use case!

As a compromise of sorts, I propose that we disable the TDP MMU by default on v5.15, and backport these fixes to v6.1. v5.15 and earlier won't get "ludicrous speed", but I think that's perfectly acceptable since KVM has had the suboptimal behavior literally since EPT/NPT support was first added.

I'm comfortable backporting to v6.1 as that is recent enough, and there weren't substantial MMU changes between v6.1 and v6.3 in this area. I.e. I have a decent level of confidence that we aren't overlooking some subtle dependency.

For v5.15, I am less confident in the safety of a backport, and more importantly, I think we should disable the TDP MMU by default to mitigate the underlying flaw that makes the 18x speedup possible. That flaw is that KVM can end up freeing and rebuilding TDP MMU roots every time CR0.WP is toggled or a vCPU transitions to/from SMM.

We mitigated the CR0.WP case between v5.15 and v6.1[1], which is why v6.1 doesn't exhibit the same pain as v5.10, but Jeremi discovered that the SMM case badly affects KVM-on-HyperV[2], e.g. when lauching KVM guests using WSL. I posted a fix[3] to finally resolve the underlying bug, but as Jeremi discovered[4], backporting the fix to v5.15 is going to be gnarly, to say the least. It'll be far worse than backporting these CR0.WP patches, and maybe even infeasible without a large scale rework (no thanks).

Anyone that will realize meaningful benefits from the TDP MMU is all but guaranteed to be rolling their own kernels, i.e. can do the backports themselves if they want to use a v5.15 based kernel. The big selling point of the TDP MMU is that it scales better to hundreds of vCPUs, particularly when live migrating such VMs. I highly doubt that anyone running a stock kernel is running 100+ vCPU VMs, let alone trying to live migrate them.

[1] https://lkml.kernel.org/r/20220209170020.1775368-1-pbonzini%40redhat.com
[2] https://lore.kernel.org/all/[email protected]
[3] https://lore.kernel.org/all/[email protected]
[4] https://lore.kernel.org/all/[email protected]

[...]

I'll post the backports I did and maybe can convince you as well that it's not all that bad ;) But I see your proposal patch [3] got merged in the meantime and is cc:stable. Might make sense to re-do my benchmarks after it got applied to the older kernels.