[kvm-unit-tests PATCH 0/4] x86: svm: bare-metal fixes

[kvm-unit-tests PATCH 0/4] x86: svm: bare-metal fixes

[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.

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(-)

-- 
2.25.1

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

[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.

Signed-off-by: Nadav Amit <namit@vmware.com>
---
 x86/svm_tests.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/x86/svm_tests.c b/x86/svm_tests.c
index d4d130f..9adee23 100644
--- a/x86/svm_tests.c
+++ b/x86/svm_tests.c
@@ -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)
-- 
2.25.1

[kvm-unit-tests PATCH 2/4] x86: svm: present bit is set on nested page-faults

[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.

Signed-off-by: Nadav Amit <namit@vmware.com>
---
 x86/svm_tests.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/x86/svm_tests.c b/x86/svm_tests.c
index 9adee23..7069e0b 100644
--- a/x86/svm_tests.c
+++ b/x86/svm_tests.c
@@ -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)
-- 
2.25.1

[kvm-unit-tests PATCH 3/4] x86: remove blind writes from setup_mmu()

[Nadav Amit]

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

Signed-off-by: Nadav Amit <namit@vmware.com>
---
 lib/x86/vm.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/lib/x86/vm.c b/lib/x86/vm.c
index 2bc2a39..41d6d96 100644
--- a/lib/x86/vm.c
+++ b/lib/x86/vm.c
@@ -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));
-- 
2.25.1

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

[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.

Signed-off-by: Nadav Amit <namit@vmware.com>
---
 lib/x86/fwcfg.c | 4 ++++
 lib/x86/fwcfg.h | 1 +
 lib/x86/setup.c | 7 +++++++
 3 files changed, 12 insertions(+)

diff --git a/lib/x86/fwcfg.c b/lib/x86/fwcfg.c
index 06ef62c..c2aaf5a 100644
--- a/lib/x86/fwcfg.c
+++ b/lib/x86/fwcfg.c
@@ -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;
 }
 
diff --git a/lib/x86/fwcfg.h b/lib/x86/fwcfg.h
index 2f17461..64d4c6e 100644
--- a/lib/x86/fwcfg.h
+++ b/lib/x86/fwcfg.h
@@ -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
diff --git a/lib/x86/setup.c b/lib/x86/setup.c
index b5941cd..7befe09 100644
--- a/lib/x86/setup.c
+++ b/lib/x86/setup.c
@@ -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;
 	}
 
-- 
2.25.1

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

[Paolo Bonzini]

On 10/07/20 20:33, Nadav Amit wrote:
> 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.
> 
> Signed-off-by: Nadav Amit <namit@vmware.com>
> ---
>  x86/svm_tests.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/x86/svm_tests.c b/x86/svm_tests.c
> index d4d130f..9adee23 100644
> --- a/x86/svm_tests.c
> +++ b/x86/svm_tests.c
> @@ -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 <pbonzini@redhat.com>
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.

Reported-by: Nadav Amit <namit@vmware.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

diff --git a/x86/cstart.S b/x86/cstart.S
index 409cb00..e63e4e2 100644
--- a/x86/cstart.S
+++ b/x86/cstart.S
@@ -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
diff --git a/x86/cstart64.S b/x86/cstart64.S
index fabcdbf..3ae98d3 100644
--- a/x86/cstart64.S
+++ b/x86/cstart64.S
@@ -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

Re: [kvm-unit-tests PATCH 0/4] x86: svm: bare-metal fixes

[Paolo Bonzini]

On 10/07/20 20:33, Nadav Amit wrote:
> 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.

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

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

[Nadav Amit]

> On Jul 10, 2020, at 1:45 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
> On 10/07/20 20:33, Nadav Amit wrote:
>> 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.

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

[Paolo Bonzini]

On 10/07/20 23:21, Nadav Amit wrote:
>> On Jul 10, 2020, at 1:45 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>> On 10/07/20 20:33, Nadav Amit wrote:
>>> 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.
> 

Done now.

Paolo

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

[Nadav Amit]

> On Jul 10, 2020, at 11:33 AM, Nadav Amit <namit@vmware.com> wrote:
> 
> 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).