Skip to contentSkip to conversation

x86: svm: bare-metal fixes

PATCH In review Nadav Amit 2 participants 10 messages Jul 10, 2020 KVM

lore
Share & tools
lore

Nadav Amit

author
#0 lore raw

cover x86: svm: bare-metal fixes

These patches are intended to allow the svm tests to run on bare-metal. The second patch indicates there is a bug in KVM.

Unfortunately, two tests still fail on bare-metal for reasons that I could not figure out, with my somewhat limited SVM knowledge.

The first failure is "direct NMI while running guest". For some reason the NMI is not delivered. Note that "direct NMI + hlt" and others pass.

The second is npt_rw_pfwalk_check. Even after the relevant fixes, exit_info_2 has a mismatch, when the expected value (of the faulting guest physical address) is 0x641000 and the actual is 0x641208. It might be related to the fact that the physical server has more memory, but I could not reproduce it on a VM with more physical memory.

Nadav Amit (4):
x86: svm: clear CR4.DE on DR intercept test
x86: svm: present bit is set on nested page-faults
x86: remove blind writes from setup_mmu()
x86: Allow to limit maximum RAM address

 lib/x86/fwcfg.c | 4 ++++
 lib/x86/fwcfg.h | 1 +
 lib/x86/setup.c | 7 +++++++
 lib/x86/vm.c    | 3 ---
 x86/svm_tests.c | 5 +++--
 5 files changed, 15 insertions(+), 5 deletions(-)
Signature · 1 line
2.25.1
author
#1 lore raw

PATCH 1/4 x86: svm: clear CR4.DE on DR intercept test

DR4/DR5 can only be written when CR4.DE is clear, and otherwise trigger a #GP exception. The BIOS might not clear CR4.DE so update the tests not to make this assumption.

---
 x86/svm_tests.c | 1 +
 1 file changed, 1 insertion(+)
x86/svm_tests.c +1 -0
index d4d130f..9adee23 100644
@@ -171,6 +171,7 @@ static void prepare_dr_intercept(struct svm_test *test)
     default_prepare(test);
     vmcb->control.intercept_dr_read = 0xff;
     vmcb->control.intercept_dr_write = 0xff;
+    vmcb->save.cr4 &= ~X86_CR4_DE;
 }
 
 static void test_dr_intercept(struct svm_test *test)
Signature · 1 line
2.25.1
maintainer
#2 lore raw

Re: [kvm-unit-tests PATCH 1/4] x86: svm: clear CR4.DE on DR intercept test

Quoted 15 lines · Nadav Amit

DR4/DR5 can only be written when CR4.DE is clear, and otherwise trigger a #GP exception. The BIOS might not clear CR4.DE so update the tests not to make this assumption.

---
 x86/svm_tests.c | 1 +
 1 file changed, 1 insertion(+)
x86/svm_tests.c +0 -0
index d4d130f..9adee23 100644

x86/svm_tests.c static void prepare_dr_intercept(struct svm_test *test) · line 177

Quoted 7 lines
x86/svm_tests.c +1 -0
@@ -171,6 +171,7 @@ static void prepare_dr_intercept(struct svm_test *test)
     default_prepare(test);
     vmcb->control.intercept_dr_read = 0xff;
     vmcb->control.intercept_dr_write = 0xff;
+    vmcb->save.cr4 &= ~X86_CR4_DE;
 }
 
 static void test_dr_intercept(struct svm_test *test)

I think we should just start with a clean slate and clear CR4 in cstart*.S:

------------ 8< ------------
From d86ef5851964521c4558e73e43187912718e6746 Mon Sep 17 00:00:00 2001
From: Paolo Bonzini <[email protected]>
Date: Fri, 10 Jul 2020 16:44:18 -0400
Subject: [PATCH kvm-unit-tests] cstart: do not assume CR4 starts as zero

The BIOS might leave some bits set in CR4; for example, CR4.DE=1 would cause the SVM test for the DR intercept to fail, because DR4/DR5 can only be written when CR4.DE is clear, and otherwise trigger a #GP exception.

x86/cstart.S +1 -2
index 409cb00..e63e4e2 100644
@@ -125,8 +125,7 @@ start:
         jmpl $8, $start32
 
 prepare_32:
-	mov %cr4, %eax
-	bts $4, %eax  // pse
+	mov %(1 << 4), %eax // pse
 	mov %eax, %cr4
 
 	mov $pt, %eax
x86/cstart64.S +1 -2
index fabcdbf..3ae98d3 100644
@@ -176,8 +176,7 @@ prepare_64:
 	setup_segments
 
 enter_long_mode:
-	mov %cr4, %eax
-	bts $5, %eax  // pae
+	mov $(1 << 5), %eax // pae
 	mov %eax, %cr4
 
 	mov pt_root, %eax

WDYT?

Paolo

author
#3 lore raw

Re: [kvm-unit-tests PATCH 1/4] x86: svm: clear CR4.DE on DR intercept test

Quoted 10 lines · Paolo Bonzini
Quoted 5 lines · Nadav Amit

DR4/DR5 can only be written when CR4.DE is clear, and otherwise trigger a #GP exception. The BIOS might not clear CR4.DE so update the tests not to make this assumption.

I think we should just start with a clean slate and clear CR4 in cstart*.S:

Your change seems fine. If you can push it (with the rest of the recent svm changes), I would prefer to run it, before I need to return my AMD machine.

maintainer
#4 lore raw

Re: [kvm-unit-tests PATCH 1/4] x86: svm: clear CR4.DE on DR intercept test

Quoted 9 lines · Nadav Amit
Quoted 9 lines · Paolo Bonzini
Quoted 5 lines · Nadav Amit

DR4/DR5 can only be written when CR4.DE is clear, and otherwise trigger a #GP exception. The BIOS might not clear CR4.DE so update the tests not to make this assumption.

Quoted 5 lines
Quoted 1 line

I think we should just start with a clean slate and clear CR4 in cstart*.S:

Your change seems fine. If you can push it (with the rest of the recent svm changes), I would prefer to run it, before I need to return my AMD machine.

Done now.

Paolo

author
#5 lore raw

PATCH 2/4 x86: svm: present bit is set on nested page-faults

In reply to Nadav Amit

On nested page-faults due to write-protect or reserved bits, the present-bit in EXITINFO1 is set, as confirmed on bare-metal. Set the expected result accordingly.

This indicates that KVM has a bug.

---
 x86/svm_tests.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
x86/svm_tests.c +2 -2
index 9adee23..7069e0b 100644
@@ -816,7 +816,7 @@ static bool npt_rw_pfwalk_check(struct svm_test *test)
     *pte |= (1ULL << 1);
 
     return (vmcb->control.exit_code == SVM_EXIT_NPF)
-           && (vmcb->control.exit_info_1 == 0x200000006ULL)
+           && (vmcb->control.exit_info_1 == 0x200000007ULL)
 	   && (vmcb->control.exit_info_2 == read_cr3());
 }
 
@@ -835,7 +835,7 @@ static bool npt_rsvd_pfwalk_check(struct svm_test *test)
     pdpe[0] &= ~(1ULL << 8);
 
     return (vmcb->control.exit_code == SVM_EXIT_NPF)
-            && (vmcb->control.exit_info_1 == 0x20000000eULL);
+            && (vmcb->control.exit_info_1 == 0x20000000fULL);
 }
 
 static void npt_l1mmio_prepare(struct svm_test *test)
Signature · 1 line
2.25.1
author
#6 lore raw

PATCH 3/4 x86: remove blind writes from setup_mmu()

In reply to Nadav Amit

Recent changes cause end_of_memory to be disregarded in 32-bit. Remove the blind writes to it.

---
 lib/x86/vm.c | 3 ---
 1 file changed, 3 deletions(-)
lib/x86/vm.c +0 -3
index 2bc2a39..41d6d96 100644
@@ -151,9 +151,6 @@ void *setup_mmu(phys_addr_t end_of_memory)
 
     setup_mmu_range(cr3, 0, end_of_memory);
 #else
-    if (end_of_memory > (1ul << 31))
-	    end_of_memory = (1ul << 31);
-
     setup_mmu_range(cr3, 0, (2ul << 30));
     setup_mmu_range(cr3, 3ul << 30, (1ul << 30));
     init_alloc_vpage((void*)(3ul << 30));
Signature · 1 line
2.25.1
author
#7 lore raw

PATCH 4/4 x86: Allow to limit maximum RAM address

In reply to Nadav Amit

While there is a feature to limit RAM memory, we should also be able to limit the maximum RAM address. Specifically, svm can only work when the maximum RAM address is lower than 4G, as it does not map the rest of the memory into the NPT.

Allow to do so using the firmware, when in fact the expected use-case is to provide this infomation on bare-metal using the MEMLIMIT parameter in initrd.

---
 lib/x86/fwcfg.c | 4 ++++
 lib/x86/fwcfg.h | 1 +
 lib/x86/setup.c | 7 +++++++
 3 files changed, 12 insertions(+)
lib/x86/fwcfg.c +4 -0
index 06ef62c..c2aaf5a 100644
@@ -28,6 +28,10 @@ static void read_cfg_override(void)
 	if ((str = getenv("TEST_DEVICE")))
 		no_test_device = !atol(str);
 
+	if ((str = getenv("MEMLIMIT")))
+		fw_override[FW_CFG_MAX_RAM] = atol(str) * 1024 * 1024;
+
+
     fw_override_done = true;
 }
 
lib/x86/fwcfg.h +1 -0
index 2f17461..64d4c6e 100644
@@ -21,6 +21,7 @@
 #define FW_CFG_BOOT_MENU        0x0e
 #define FW_CFG_MAX_CPUS         0x0f
 #define FW_CFG_MAX_ENTRY        0x10
+#define FW_CFG_MAX_RAM		0x11
 
 #define FW_CFG_WRITE_CHANNEL    0x4000
 #define FW_CFG_ARCH_LOCAL       0x8000
lib/x86/setup.c +7 -0
index b5941cd..7befe09 100644
@@ -66,6 +66,9 @@ void find_highmem(void)
 	u64 upper_end = bootinfo->mem_upper * 1024ull;
 	u64 best_start = (uintptr_t) &edata;
 	u64 best_end = upper_end;
+	u64 max_end = fwcfg_get_u64(FW_CFG_MAX_RAM);
+	if (max_end == 0)
+		max_end = -1ull;
 	bool found = false;
 
 	uintptr_t mmap = bootinfo->mmap_addr;
@@ -79,8 +82,12 @@ void find_highmem(void)
 			continue;
 		if (mem->length < best_end - best_start)
 			continue;
+		if (mem->base_addr >= max_end)
+			continue;
 		best_start = mem->base_addr;
 		best_end = mem->base_addr + mem->length;
+		if (best_end > max_end)
+			best_end = max_end;
 		found = true;
 	}
 
Signature · 1 line
2.25.1
author
#8 lore raw

Re: [kvm-unit-tests PATCH 4/4] x86: Allow to limit maximum RAM address

Quoted 10 lines · Nadav Amit

While there is a feature to limit RAM memory, we should also be able to limit the maximum RAM address. Specifically, svm can only work when the maximum RAM address is lower than 4G, as it does not map the rest of the memory into the NPT.

Allow to do so using the firmware, when in fact the expected use-case is to provide this infomation on bare-metal using the MEMLIMIT parameter in initrd.

Sorry for my clumsiness/laziness in rechecking. This patch was broken. I will send an update soon.

Anyhow, the latest SVM tests pass as well (excluding this screw-up of mine).

maintainer
#9 lore raw
In reply to Nadav Amit
Quoted 6 lines · Nadav Amit

These patches are intended to allow the svm tests to run on bare-metal. The second patch indicates there is a bug in KVM.

Unfortunately, two tests still fail on bare-metal for reasons that I could not figure out, with my somewhat limited SVM knowledge.

Quoted 8 lines

The first failure is "direct NMI while running guest". For some reason the NMI is not delivered. Note that "direct NMI + hlt" and others pass.

The second is npt_rw_pfwalk_check. Even after the relevant fixes, exit_info_2 has a mismatch, when the expected value (of the faulting guest physical address) is 0x641000 and the actual is 0x641208. It might be related to the fact that the physical server has more memory, but I could not reproduce it on a VM with more physical memory.

Could be much worse---and could be bugs in KVM too, though we're definitely faring better than six months ago.

Thanks, queued patches 2-4 and sent a replacement for patch 1.

Paolo