[PATCH 0/2 v2] Test: nSVM: Test the effect of guest EFLAGS.TF on VMRUN

[PATCH 0/2 v2] Test: nSVM: Test the effect of guest EFLAGS.TF on VMRUN

[Krish Sadhukhan]

v1 -> v2:
	In patch# 1, the new function is now called __svm_vmrun() per
	suggestion from Sean. I have also adjusted the commit header and
	the commit message.


Patch# 1: Adds a variant of svm_vmrun() so that custom guest code can be used.
Patch# 2: Tests the effects of guest EFLAGS.TF on VMRUN.

[PATCH 1/2 v2] nSVM: Add a variant of svm_vmrun() for setting guest RIP
[PATCH 2/2 v2] Test: nSVM: Test the effect of guest EFLAGS.TF on VMRUN

 x86/svm.c       |  9 +++++++--
 x86/svm.h       |  1 +
 x86/svm_tests.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 69 insertions(+), 2 deletions(-)

Krish Sadhukhan (2):
      nSVM: Add a variant of svm_vmrun() for setting guest RIP to custom code
      Test: nSVM: Test the effect of guest EFLAGS.TF on VMRUN

[PATCH 1/2 v2] nSVM: Add a variant of svm_vmrun() for setting guest RIP to custom code

[Krish Sadhukhan]

Current implementations of svm_vmrun() and test_run() set the guest RIP to a
wrapper function which executes the guest code being used by tests. This is
not suitable for tests like testing the effect of guest EFLAGS.TF on VMRUN
because the trap handler will point to the second guest instruction to which
the test code does not have access.

Therefore, move the contents of svm_vmrun() to a new function called
__svm_vmrun() and add guest RIP as a function parameter so that it will
set the VMCB guest RIP field to the memory location passed in. Call this
new function in svm_vmrun() and pass the wrapper guest code in order to
maintain the existing interface.

Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
---
 x86/svm.c | 9 +++++++--
 x86/svm.h | 1 +
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/x86/svm.c b/x86/svm.c
index f185ca0..4b46281 100644
--- a/x86/svm.c
+++ b/x86/svm.c
@@ -227,9 +227,9 @@ struct svm_test *v2_test;
 
 u64 guest_stack[10000];
 
-int svm_vmrun(void)
+int __svm_vmrun(u64 rip)
 {
-	vmcb->save.rip = (ulong)test_thunk;
+	vmcb->save.rip = (ulong)rip;
 	vmcb->save.rsp = (ulong)(guest_stack + ARRAY_SIZE(guest_stack));
 	regs.rdi = (ulong)v2_test;
 
@@ -244,6 +244,11 @@ int svm_vmrun(void)
 	return (vmcb->control.exit_code);
 }
 
+int svm_vmrun(void)
+{
+	return __svm_vmrun((u64)test_thunk);
+}
+
 extern u64 *vmrun_rip;
 
 static void test_run(struct svm_test *test)
diff --git a/x86/svm.h b/x86/svm.h
index 995b0f8..92fa277 100644
--- a/x86/svm.h
+++ b/x86/svm.h
@@ -408,6 +408,7 @@ void inc_test_stage(struct svm_test *test);
 void vmcb_ident(struct vmcb *vmcb);
 struct regs get_regs(void);
 void vmmcall(void);
+int __svm_vmrun(u64 rip);
 int svm_vmrun(void);
 void test_set_guest(test_guest_func func);
 
-- 
2.27.0

[PATCH 2/2 v2] Test: nSVM: Test the effect of guest EFLAGS.TF on VMRUN

[Krish Sadhukhan]

According to section "VMRUN and TF/RF Bits in EFLAGS." in APM vol 2,

     "When VMRUN loads a guest value of 1 in EFLAGS.TF, that value does not
      cause a trace trap between the VMRUN and the first guest instruction,
      but rather after completion of the first guest instruction."

Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
---
 x86/svm_tests.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/x86/svm_tests.c b/x86/svm_tests.c
index a56a197..7827d1e 100644
--- a/x86/svm_tests.c
+++ b/x86/svm_tests.c
@@ -2491,6 +2491,66 @@ static void test_vmrun_canonicalization(void)
 	TEST_CANONICAL(vmcb->save.tr.base, "TR");
 }
 
+/*
+ * When VMRUN loads a guest value of 1 in EFLAGS.TF, that value does not
+ * cause a trace trap between the VMRUN and the first guest instruction, but
+ * rather after completion of the first guest instruction.
+ *
+ * [APM vol 2]
+ */
+u64 guest_rflags_test_trap_rip;
+
+static void guest_rflags_test_db_handler(struct ex_regs *r)
+{
+	guest_rflags_test_trap_rip = r->rip;
+	r->rflags &= ~X86_EFLAGS_TF;
+}
+
+extern void guest_rflags_test_guest(struct svm_test *test);
+extern u64 *insn2;
+extern u64 *guest_end;
+
+asm("guest_rflags_test_guest:\n\t"
+    "push %rbp\n\t"
+    ".global insn2\n\t"
+    "insn2:\n\t"
+    "mov %rsp,%rbp\n\t"
+    "vmmcall\n\t"
+    "vmmcall\n\t"
+    ".global guest_end\n\t"
+    "guest_end:\n\t"
+    "vmmcall\n\t"
+    "pop %rbp\n\t"
+    "ret");
+
+static void test_guest_rflags(void)
+{
+	handle_exception(DB_VECTOR, guest_rflags_test_db_handler);
+
+	/*
+	 * Trap expected after completion of first guest instruction
+	 */
+	vmcb->save.rflags |= X86_EFLAGS_TF;
+	report (__svm_vmrun((u64)guest_rflags_test_guest) == SVM_EXIT_VMMCALL &&
+		guest_rflags_test_trap_rip == (u64)&insn2,
+               "Test EFLAGS.TF on VMRUN: trap expected  after completion of first guest instruction");
+	/*
+	 * No trap expected
+	 */
+	guest_rflags_test_trap_rip = 0;
+	vmcb->save.rip += 3;
+	vmcb->save.rflags |= X86_EFLAGS_TF;
+	report (__svm_vmrun(vmcb->save.rip) == SVM_EXIT_VMMCALL &&
+		guest_rflags_test_trap_rip == 0, "Test EFLAGS.TF on VMRUN: trap not expected");
+
+	/*
+	 * Let guest finish execution
+	 */
+	vmcb->save.rip += 3;
+	report (__svm_vmrun(vmcb->save.rip) == SVM_EXIT_VMMCALL &&
+		vmcb->save.rip == (u64)&guest_end, "Test EFLAGS.TF on VMRUN: guest execution completion");
+}
+
 static void svm_guest_state_test(void)
 {
 	test_set_guest(basic_guest_main);
@@ -2501,6 +2561,7 @@ static void svm_guest_state_test(void)
 	test_dr();
 	test_msrpm_iopm_bitmap_addrs();
 	test_vmrun_canonicalization();
+	test_guest_rflags();
 }
 
 static void __svm_npt_rsvd_bits_test(u64 *pxe, u64 rsvd_bits, u64 efer,
-- 
2.27.0

Re: [PATCH 0/2 v2] Test: nSVM: Test the effect of guest EFLAGS.TF on VMRUN

[Paolo Bonzini]

On 19/07/21 19:46, Krish Sadhukhan wrote:
> v1 -> v2:
> 	In patch# 1, the new function is now called __svm_vmrun() per
> 	suggestion from Sean. I have also adjusted the commit header and
> 	the commit message.
> 
> 
> Patch# 1: Adds a variant of svm_vmrun() so that custom guest code can be used.
> Patch# 2: Tests the effects of guest EFLAGS.TF on VMRUN.
> 
> [PATCH 1/2 v2] nSVM: Add a variant of svm_vmrun() for setting guest RIP
> [PATCH 2/2 v2] Test: nSVM: Test the effect of guest EFLAGS.TF on VMRUN
> 
>   x86/svm.c       |  9 +++++++--
>   x86/svm.h       |  1 +
>   x86/svm_tests.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 69 insertions(+), 2 deletions(-)
> 
> Krish Sadhukhan (2):
>        nSVM: Add a variant of svm_vmrun() for setting guest RIP to custom code
>        Test: nSVM: Test the effect of guest EFLAGS.TF on VMRUN
> 

Queued, thanks.  However, I placed this in a different test than 
svm_guest_state_test, since that one is more evaluating invalid (or 
silently canonicalized) data.

Paolo