[PATCH 0/3] Test: nSVM: Test effects of host EFLAGS.RF on VMRUN

[PATCH 0/3] Test: nSVM: Test effects of host EFLAGS.RF on VMRUN

[Krish Sadhukhan]

Patch# 1: Moves the setter/getter functions for the DR registers, to the
	  common library so that other tests can re-use them.
Patch# 2: Adds the #define for the RF bit in EFLAGS register.
Patch# 3: Adds a test for the behavior of host EFLAGS.RF bit on VMRUN,
	  based on the following from section "VMRUN and TF/RF Bits in
	  EFLAGS" in APM vol 2,

	      "EFLAGS.RF suppresses any potential instruction breakpoint
	       match on the VMRUN. Completion of the VMRUN instruction
	       instruction clears the host EFLAGS.RF bit."


[PATCH 1/3] Test: x86: Move setter/getter for Debug registers to
[PATCH 2/3] Test: x86: Add a #define for the RF bit in EFLAGS
[PATCH 3/3] Test: nSVM: Test effects of host EFLAGS.RF on VMRUN

 lib/x86/processor.h | 33 ++++++++++++++++++++++
 x86/debug.c         | 79 +++++++++++++---------------------------------------
 x86/svm_tests.c     | 74 ++++++++++++++++++++++++++++++++++++++----------
 3 files changed, 111 insertions(+), 75 deletions(-)

Krish Sadhukhan (3):
      Test: x86: Move setter/getter for Debug registers to common library
      Test: x86: Add a #define for the RF bit in EFLAGS register
      Test: nSVM: Test effects of host EFLAGS.RF on VMRUN

[PATCH 1/3] Test: x86: Move setter/getter for Debug registers to common library

[Krish Sadhukhan]

The setter/getter functions for the DR0..DR4 registers exist in debug.c
test and hence they can not be re-used by other tests. Therefore, move
them to the common library.

Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
---
 lib/x86/processor.h | 32 ++++++++++++++++++
 x86/debug.c         | 79 +++++++++++----------------------------------
 2 files changed, 51 insertions(+), 60 deletions(-)

diff --git a/lib/x86/processor.h b/lib/x86/processor.h
index 173520f..ec2e508 100644
--- a/lib/x86/processor.h
+++ b/lib/x86/processor.h
@@ -470,6 +470,38 @@ static inline u16 str(void)
     return val;
 }
 
+static inline void write_dr0(void *val)
+{
+    asm volatile ("mov %0, %%dr0" : : "r"(val) : "memory");
+}
+
+static inline void write_dr1(void *val)
+{
+    asm volatile ("mov %0, %%dr1" : : "r"(val) : "memory");
+}
+
+static inline void write_dr2(void *val)
+{
+    asm volatile ("mov %0, %%dr2" : : "r"(val) : "memory");
+}
+
+static inline void write_dr3(void *val)
+{
+    asm volatile ("mov %0, %%dr3" : : "r"(val) : "memory");
+}
+
+static inline void write_dr4(ulong val)
+{
+    asm volatile ("mov %0, %%dr4" : : "r"(val) : "memory");
+}
+
+static inline ulong read_dr4(void)
+{
+    ulong val;
+    asm volatile ("mov %%dr4, %0" : "=r"(val));
+    return val;
+}
+
 static inline void write_dr6(ulong val)
 {
     asm volatile ("mov %0, %%dr6" : : "r"(val) : "memory");
diff --git a/x86/debug.c b/x86/debug.c
index 382fdde..6b3ff4e 100644
--- a/x86/debug.c
+++ b/x86/debug.c
@@ -18,58 +18,17 @@ static volatile unsigned long db_addr[10], dr6[10];
 static volatile unsigned int n;
 static volatile unsigned long value;
 
-static unsigned long get_dr4(void)
-{
-	unsigned long value;
-
-	asm volatile("mov %%dr4, %0" : "=r" (value));
-	return value;
-}
-
-static unsigned long get_dr6(void)
-{
-	unsigned long value;
-
-	asm volatile("mov %%dr6,%0" : "=r" (value));
-	return value;
-}
-
-static void set_dr0(void *value)
-{
-	asm volatile("mov %0,%%dr0" : : "r" (value));
-}
-
-static void set_dr1(void *value)
-{
-	asm volatile("mov %0,%%dr1" : : "r" (value));
-}
-
-static void set_dr4(unsigned long value)
-{
-	asm volatile("mov %0,%%dr4" : : "r" (value));
-}
-
-static void set_dr6(unsigned long value)
-{
-	asm volatile("mov %0,%%dr6" : : "r" (value));
-}
-
-static void set_dr7(unsigned long value)
-{
-	asm volatile("mov %0,%%dr7" : : "r" (value));
-}
-
 static void handle_db(struct ex_regs *regs)
 {
 	db_addr[n] = regs->rip;
-	dr6[n] = get_dr6();
+	dr6[n] = read_dr6();
 
 	if (dr6[n] & 0x1)
 		regs->rflags |= (1 << 16);
 
 	if (++n >= 10) {
 		regs->rflags &= ~(1 << 8);
-		set_dr7(0x00000400);
+		write_dr7(0x00000400);
 	}
 }
 
@@ -105,15 +64,15 @@ int main(int ac, char **av)
 	got_ud = 0;
 	cr4 = read_cr4();
 	write_cr4(cr4 & ~X86_CR4_DE);
-	set_dr4(0);
-	set_dr6(0xffff4ff2);
-	report(get_dr4() == 0xffff4ff2 && !got_ud, "reading DR4 with CR4.DE == 0");
+	write_dr4(0);
+	write_dr6(0xffff4ff2);
+	report(read_dr4() == 0xffff4ff2 && !got_ud, "reading DR4 with CR4.DE == 0");
 
 	cr4 = read_cr4();
 	write_cr4(cr4 | X86_CR4_DE);
-	get_dr4();
+	read_dr4();
 	report(got_ud, "reading DR4 with CR4.DE == 1");
-	set_dr6(0);
+	write_dr6(0);
 
 	extern unsigned char sw_bp;
 	asm volatile("int3; sw_bp:");
@@ -121,8 +80,8 @@ int main(int ac, char **av)
 
 	n = 0;
 	extern unsigned char hw_bp1;
-	set_dr0(&hw_bp1);
-	set_dr7(0x00000402);
+	write_dr0(&hw_bp1);
+	write_dr7(0x00000402);
 	asm volatile("hw_bp1: nop");
 	report(n == 1 &&
 	       db_addr[0] == ((unsigned long)&hw_bp1) && dr6[0] == 0xffff0ff1,
@@ -130,15 +89,15 @@ int main(int ac, char **av)
 
 	n = 0;
 	extern unsigned char hw_bp2;
-	set_dr0(&hw_bp2);
-	set_dr6(0x00004002);
+	write_dr0(&hw_bp2);
+	write_dr6(0x00004002);
 	asm volatile("hw_bp2: nop");
 	report(n == 1 &&
 	       db_addr[0] == ((unsigned long)&hw_bp2) && dr6[0] == 0xffff4ff1,
 	       "hw breakpoint (test that dr6.BS is not cleared)");
 
 	n = 0;
-	set_dr6(0);
+	write_dr6(0);
 	asm volatile(
 		"pushf\n\t"
 		"pop %%rax\n\t"
@@ -161,7 +120,7 @@ int main(int ac, char **av)
 	 * emulated. Test that single stepping works on emulated instructions.
 	 */
 	n = 0;
-	set_dr6(0);
+	write_dr6(0);
 	asm volatile(
 		"pushf\n\t"
 		"pop %%rax\n\t"
@@ -188,8 +147,8 @@ int main(int ac, char **av)
 	       "single step emulated instructions");
 
 	n = 0;
-	set_dr1((void *)&value);
-	set_dr7(0x00d0040a); // 4-byte write
+	write_dr1((void *)&value);
+	write_dr7(0x00d0040a); // 4-byte write
 
 	extern unsigned char hw_wp1;
 	asm volatile(
@@ -201,7 +160,7 @@ int main(int ac, char **av)
 	       "hw watchpoint (test that dr6.BS is not cleared)");
 
 	n = 0;
-	set_dr6(0);
+	write_dr6(0);
 
 	extern unsigned char hw_wp2;
 	asm volatile(
@@ -213,16 +172,16 @@ int main(int ac, char **av)
 	       "hw watchpoint (test that dr6.BS is not set)");
 
 	n = 0;
-	set_dr6(0);
+	write_dr6(0);
 	extern unsigned char sw_icebp;
 	asm volatile(".byte 0xf1; sw_icebp:");
 	report(n == 1 &&
 	       db_addr[0] == (unsigned long)&sw_icebp && dr6[0] == 0xffff0ff0,
 	       "icebp");
 
-	set_dr7(0x400);
+	write_dr7(0x400);
 	value = KERNEL_DS;
-	set_dr7(0x00f0040a); // 4-byte read or write
+	write_dr7(0x00f0040a); // 4-byte read or write
 
 	/*
 	 * Each invocation of the handler should shift n by 1 and set bit 0 to 1.
-- 
2.27.0

[PATCH 2/3] Test: x86: Add a #define for the RF bit in EFLAGS register

[Krish Sadhukhan]

Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
---
 lib/x86/processor.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/x86/processor.h b/lib/x86/processor.h
index ec2e508..33ddd50 100644
--- a/lib/x86/processor.h
+++ b/lib/x86/processor.h
@@ -70,6 +70,7 @@
 #define X86_EFLAGS_OF    0x00000800
 #define X86_EFLAGS_IOPL  0x00003000
 #define X86_EFLAGS_NT    0x00004000
+#define X86_EFLAGS_RF    0x00010000
 #define X86_EFLAGS_VM    0x00020000
 #define X86_EFLAGS_AC    0x00040000
 
-- 
2.27.0

[PATCH 3/3] Test: nSVM: Test effects of host EFLAGS.RF on VMRUN

[Krish Sadhukhan]

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

    "EFLAGS.RF suppresses any potential instruction breakpoint match on
     the VMRUN. Completion of the VMRUN instruction clears the host
     EFLAGS.RF bit."

Test that the RIP detected by the #DB handler when a #DB is triggered by
configuring the debug registers, is the RIP of the VMRUN instruction and
that setting EFLAGS.RF in the #DB handler will no more trigger any #DB
following the completion of VMRUN. Also, test that the processor clears
EFLAGS.RF on completion of VMRUN.

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

diff --git a/x86/svm_tests.c b/x86/svm_tests.c
index a56a197..1f12504 100644
--- a/x86/svm_tests.c
+++ b/x86/svm_tests.c
@@ -1965,6 +1965,10 @@ static bool init_intercept_check(struct svm_test *test)
  * Setting host EFLAGS.TF causes a #DB trap after the VMRUN completes on the
  * host side (i.e., after the #VMEXIT from the guest).
  *
+ * Setting host EFLAGS.RF suppresses any potential instruction breakpoint
+ * match on the VMRUN and completion of the VMRUN instruction clears the
+ * host EFLAGS.RF bit.
+ *
  * [AMD APM]
  */
 static volatile u8 host_rflags_guest_main_flag = 0;
@@ -1972,7 +1976,8 @@ static volatile u8 host_rflags_db_handler_flag = 0;
 static volatile bool host_rflags_ss_on_vmrun = false;
 static volatile bool host_rflags_vmrun_reached = false;
 static volatile bool host_rflags_set_tf = false;
-static u64 post_vmrun_rip;
+static volatile bool host_rflags_set_rf = false;
+static u64 rip_detected;
 
 extern u64 *vmrun_rip;
 
@@ -1980,11 +1985,27 @@ static void host_rflags_db_handler(struct ex_regs *r)
 {
 	if (host_rflags_ss_on_vmrun) {
 		if (host_rflags_vmrun_reached) {
-			r->rflags &= ~X86_EFLAGS_TF;
-			post_vmrun_rip = r->rip;
+			if (!host_rflags_set_rf) {
+				r->rflags &= ~X86_EFLAGS_TF;
+				rip_detected = r->rip;
+			} else {
+				r->rflags |= X86_EFLAGS_RF;
+				++host_rflags_db_handler_flag;
+			}
 		} else {
-			if (r->rip == (u64)&vmrun_rip)
+			if (r->rip == (u64)&vmrun_rip) {
 				host_rflags_vmrun_reached = true;
+
+				if (host_rflags_set_rf) {
+					host_rflags_guest_main_flag = 0;
+					rip_detected = r->rip;
+					r->rflags &= ~X86_EFLAGS_TF;
+
+					/* Trigger #DB via debug registers */
+					write_dr0((void *)&vmrun_rip);
+					write_dr7(0x403);
+				}
+			}
 		}
 	} else {
 		r->rflags &= ~X86_EFLAGS_TF;
@@ -2007,11 +2028,15 @@ static void host_rflags_prepare_gif_clear(struct svm_test *test)
 static void host_rflags_test(struct svm_test *test)
 {
 	while (1) {
-		if (get_test_stage(test) > 0 && host_rflags_set_tf &&
-		    (!host_rflags_ss_on_vmrun) &&
-		    (!host_rflags_db_handler_flag))
-			host_rflags_guest_main_flag = 1;
-		if (get_test_stage(test) == 3)
+		if (get_test_stage(test) > 0) {
+			if ((host_rflags_set_tf && (!host_rflags_ss_on_vmrun) &&
+			    (!host_rflags_db_handler_flag)) ||
+			    (host_rflags_set_rf &&
+			    host_rflags_db_handler_flag == 1))
+				host_rflags_guest_main_flag = 1;
+		}
+
+		if (get_test_stage(test) == 4)
 			break;
 		vmmcall();
 	}
@@ -2051,26 +2076,45 @@ static bool host_rflags_finished(struct svm_test *test)
 		break;
 	case 2:
 		if (vmcb->control.exit_code != SVM_EXIT_VMMCALL ||
-		    (post_vmrun_rip - (u64)&vmrun_rip) != 3) {
+		    (rip_detected - (u64)&vmrun_rip) != 3) {
 			report(false, "Unexpected VMEXIT or RIP mismatch."
-			    " Exit reason 0x%x, VMRUN RIP: %lx, post-VMRUN"
-			    " RIP: %lx", vmcb->control.exit_code,
-			    (u64)&vmrun_rip, post_vmrun_rip);
+			    " Exit reason 0x%x, RIP actual: %lx, RIP expected: "
+			    "%lx", vmcb->control.exit_code,
+			    (u64)&vmrun_rip, rip_detected - 3);
+			return true;
+		}
+		host_rflags_set_rf = true;
+		host_rflags_guest_main_flag = 0;
+		host_rflags_vmrun_reached = false;
+		vmcb->save.rip += 3;
+		break;
+	case 3:
+		if (vmcb->control.exit_code != SVM_EXIT_VMMCALL ||
+		    rip_detected != (u64)&vmrun_rip ||
+		    host_rflags_guest_main_flag != 1 ||
+		    host_rflags_db_handler_flag > 1 ||
+		    read_rflags() & X86_EFLAGS_RF) {
+			report(false, "Unexpected VMEXIT or RIP mismatch or "
+			    "EFLAGS.RF not cleared."
+			    " Exit reason 0x%x, RIP actual: %lx, RIP expected: "
+			    "%lx", vmcb->control.exit_code,
+			    (u64)&vmrun_rip, rip_detected);
 			return true;
 		}
 		host_rflags_set_tf = false;
+		host_rflags_set_rf = false;
 		vmcb->save.rip += 3;
 		break;
 	default:
 		return true;
 	}
 	inc_test_stage(test);
-	return get_test_stage(test) == 4;
+	return get_test_stage(test) == 5;
 }
 
 static bool host_rflags_check(struct svm_test *test)
 {
-	return get_test_stage(test) == 3;
+	return get_test_stage(test) == 4;
 }
 
 #define TEST(name) { #name, .v2 = name }
-- 
2.27.0

Re: [PATCH 0/3] Test: nSVM: Test effects of host EFLAGS.RF on VMRUN

[Paolo Bonzini]

On 26/07/21 20:02, Krish Sadhukhan wrote:
> Patch# 1: Moves the setter/getter functions for the DR registers, to the
> 	  common library so that other tests can re-use them.
> Patch# 2: Adds the #define for the RF bit in EFLAGS register.
> Patch# 3: Adds a test for the behavior of host EFLAGS.RF bit on VMRUN,
> 	  based on the following from section "VMRUN and TF/RF Bits in
> 	  EFLAGS" in APM vol 2,
> 
> 	      "EFLAGS.RF suppresses any potential instruction breakpoint
> 	       match on the VMRUN. Completion of the VMRUN instruction
> 	       instruction clears the host EFLAGS.RF bit."
> 
> 
> [PATCH 1/3] Test: x86: Move setter/getter for Debug registers to
> [PATCH 2/3] Test: x86: Add a #define for the RF bit in EFLAGS
> [PATCH 3/3] Test: nSVM: Test effects of host EFLAGS.RF on VMRUN
> 
>   lib/x86/processor.h | 33 ++++++++++++++++++++++
>   x86/debug.c         | 79 +++++++++++++---------------------------------------
>   x86/svm_tests.c     | 74 ++++++++++++++++++++++++++++++++++++++----------
>   3 files changed, 111 insertions(+), 75 deletions(-)
> 
> Krish Sadhukhan (3):
>        Test: x86: Move setter/getter for Debug registers to common library
>        Test: x86: Add a #define for the RF bit in EFLAGS register
>        Test: nSVM: Test effects of host EFLAGS.RF on VMRUN
> 

Queued, thanks.

Paolo