All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] nSVM: Test host RFLAGS.TF on VMRUN
@ 2021-02-03  1:28 Krish Sadhukhan
  2021-02-03  1:28 ` [PATCH 1/3] KVM: SVM: Replace hard-coded value with #define Krish Sadhukhan
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Krish Sadhukhan @ 2021-02-03  1:28 UTC (permalink / raw)
  To: kvm; +Cc: pbonzini, jmattson, seanjc

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

    "From the host point of view, VMRUN acts like a single instruction,
     even though an arbitrary number of guest instructions may execute
     before a #VMEXIT effectively completes the VMRUN. As a single
     host instruction, VMRUN interacts with EFLAGS.TF like ordinary
     instructions. EFLAGS.TF causes a #DB trap after the VMRUN completes
     on the host side (i.e., after the #VMEXIT from the guest)."

Patch# 1 replaces a hard-coded value with a #define.
Patch# 2 modifies the assembly in svm_vmrun() so that a Single-Step breakpoint
can placed right before the VMRUN instruction. It also adds helper functions
for setting/un-setting that breakpoint.
Patch# 3 adds a test for the RFLAGS.TF on VMRUN.

[PATCH 1/3] KVM: SVM: Replace hard-coded value with #define
[PATCH 2/3] nVMX: Add helper functions to set/unset host RFLAGS.TF on
[PATCH 3/3] nSVM: Test effect of host RFLAGS.TF on VMRUN

 arch/x86/kvm/svm/svm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Krish Sadhukhan (1):
      KVM: SVM: Replace hard-coded value with #define

 x86/svm.c       | 24 +++++++++++++--
 x86/svm.h       |  3 ++
 x86/svm_tests.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 118 insertions(+), 3 deletions(-)

Krish Sadhukhan (2):
      nVMX: Add helper functions to set/unset host RFLAGS.TF on the VMRUN instruction
      nSVM: Test effect of host RFLAGS.TF on VMRUN


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/3] KVM: SVM: Replace hard-coded value with #define
  2021-02-03  1:28 [PATCH 0/3] nSVM: Test host RFLAGS.TF on VMRUN Krish Sadhukhan
@ 2021-02-03  1:28 ` Krish Sadhukhan
  2021-02-03  1:28 ` [PATCH 2/3] nVMX: Add helper functions to set/unset host RFLAGS.TF on the VMRUN instruction Krish Sadhukhan
  2021-02-03  1:28 ` [PATCH 3/3] nSVM: Test effect of host RFLAGS.TF on VMRUN Krish Sadhukhan
  2 siblings, 0 replies; 8+ messages in thread
From: Krish Sadhukhan @ 2021-02-03  1:28 UTC (permalink / raw)
  To: kvm; +Cc: pbonzini, jmattson, seanjc

Replace the hard-coded value for bit# 1 in EFLAGS, with the available
#define.

Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
---
 arch/x86/kvm/svm/svm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index f923e14e87df..5435f5cb756b 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -1202,7 +1202,7 @@ static void init_vmcb(struct vcpu_svm *svm)
 
 	svm_set_efer(&svm->vcpu, 0);
 	save->dr6 = 0xffff0ff0;
-	kvm_set_rflags(&svm->vcpu, 2);
+	kvm_set_rflags(&svm->vcpu, X86_EFLAGS_FIXED);
 	save->rip = 0x0000fff0;
 	svm->vcpu.arch.regs[VCPU_REGS_RIP] = save->rip;
 
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/3] nVMX: Add helper functions to set/unset host RFLAGS.TF on the VMRUN instruction
  2021-02-03  1:28 [PATCH 0/3] nSVM: Test host RFLAGS.TF on VMRUN Krish Sadhukhan
  2021-02-03  1:28 ` [PATCH 1/3] KVM: SVM: Replace hard-coded value with #define Krish Sadhukhan
@ 2021-02-03  1:28 ` Krish Sadhukhan
  2021-02-03  8:15   ` Paolo Bonzini
  2021-02-03  1:28 ` [PATCH 3/3] nSVM: Test effect of host RFLAGS.TF on VMRUN Krish Sadhukhan
  2 siblings, 1 reply; 8+ messages in thread
From: Krish Sadhukhan @ 2021-02-03  1:28 UTC (permalink / raw)
  To: kvm; +Cc: pbonzini, jmattson, seanjc

Add helper functions to set host RFLAGS.TF immediately before the VMRUN
instruction. These will be used  by the next patch to test Single Stepping
on the VMRUN instruction from the host's perspective.

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

diff --git a/x86/svm.c b/x86/svm.c
index a1808c7..547f62a 100644
--- a/x86/svm.c
+++ b/x86/svm.c
@@ -179,6 +179,17 @@ void vmcb_ident(struct vmcb *vmcb)
 	}
 }
 
+static bool ss_bp_on_vmrun = false;
+void set_ss_bp_on_vmrun(void)
+{
+	ss_bp_on_vmrun = true;
+}
+
+void unset_ss_bp_on_vmrun(void)
+{
+	ss_bp_on_vmrun = false;
+}
+
 struct regs regs;
 
 struct regs get_regs(void)
@@ -215,6 +226,12 @@ struct svm_test *v2_test;
                 "mov regs, %%r15\n\t"           \
                 "mov %%r15, 0x1f8(%%rax)\n\t"   \
                 LOAD_GPR_C                      \
+                "cmpb $0, %[ss_bp]\n\t"         \
+                "je 1f\n\t"                     \
+                "pushf; pop %%r8\n\t"           \
+                "or $0x100, %%r8\n\t"           \
+                "push %%r8; popf\n\t"           \
+                "1: "                           \
                 "vmrun %%rax\n\t"               \
                 SAVE_GPR_C                      \
                 "mov 0x170(%%rax), %%r15\n\t"   \
@@ -234,7 +251,8 @@ int svm_vmrun(void)
 	asm volatile (
 		ASM_VMRUN_CMD
 		:
-		: "a" (virt_to_phys(vmcb))
+		: "a" (virt_to_phys(vmcb)),
+		[ss_bp]"m"(ss_bp_on_vmrun)
 		: "memory", "r15");
 
 	return (vmcb->control.exit_code);
@@ -253,6 +271,7 @@ static void test_run(struct svm_test *test)
 	do {
 		struct svm_test *the_test = test;
 		u64 the_vmcb = vmcb_phys;
+
 		asm volatile (
 			"clgi;\n\t" // semi-colon needed for LLVM compatibility
 			"sti \n\t"
@@ -266,7 +285,8 @@ static void test_run(struct svm_test *test)
 			"=b" (the_vmcb)             // callee save register!
 			: [test] "0" (the_test),
 			[vmcb_phys] "1"(the_vmcb),
-			[PREPARE_GIF_CLEAR] "i" (offsetof(struct svm_test, prepare_gif_clear))
+			[PREPARE_GIF_CLEAR] "i" (offsetof(struct svm_test, prepare_gif_clear)),
+			[ss_bp]"m"(ss_bp_on_vmrun)
 			: "rax", "rcx", "rdx", "rsi",
 			"r8", "r9", "r10", "r11" , "r12", "r13", "r14", "r15",
 			"memory");
diff --git a/x86/svm.h b/x86/svm.h
index a0863b8..d521972 100644
--- a/x86/svm.h
+++ b/x86/svm.h
@@ -391,6 +391,9 @@ void vmcb_ident(struct vmcb *vmcb);
 struct regs get_regs(void);
 void vmmcall(void);
 int svm_vmrun(void);
+int svm_vmrun1(void);
+void set_ss_bp_on_vmrun(void);
+void unset_ss_bp_on_vmrun(void);
 void test_set_guest(test_guest_func func);
 
 extern struct vmcb *vmcb;
diff --git a/x86/svm_tests.c b/x86/svm_tests.c
index 29a0b59..7bf3624 100644
--- a/x86/svm_tests.c
+++ b/x86/svm_tests.c
@@ -2046,7 +2046,6 @@ static void basic_guest_main(struct svm_test *test)
 {
 }
 
-
 #define SVM_TEST_REG_RESERVED_BITS(start, end, inc, str_name, reg, val,	\
 				   resv_mask)				\
 {									\
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/3] nSVM: Test effect of host RFLAGS.TF on VMRUN
  2021-02-03  1:28 [PATCH 0/3] nSVM: Test host RFLAGS.TF on VMRUN Krish Sadhukhan
  2021-02-03  1:28 ` [PATCH 1/3] KVM: SVM: Replace hard-coded value with #define Krish Sadhukhan
  2021-02-03  1:28 ` [PATCH 2/3] nVMX: Add helper functions to set/unset host RFLAGS.TF on the VMRUN instruction Krish Sadhukhan
@ 2021-02-03  1:28 ` Krish Sadhukhan
  2 siblings, 0 replies; 8+ messages in thread
From: Krish Sadhukhan @ 2021-02-03  1:28 UTC (permalink / raw)
  To: kvm; +Cc: pbonzini, jmattson, seanjc

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

	"From the host point of view, VMRUN acts like a single instruction,
	 even though an arbitrary number of guest instructions may execute
	 before a #VMEXIT effectively completes the VMRUN. As a single
	 host instruction, VMRUN interacts with EFLAGS.TF like ordinary
	 instructions. EFLAGS.TF causes a #DB trap after the VMRUN completes
	 on the host side (i.e., after the #VMEXIT from the guest).

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

diff --git a/x86/svm_tests.c b/x86/svm_tests.c
index 7bf3624..73bffe6 100644
--- a/x86/svm_tests.c
+++ b/x86/svm_tests.c
@@ -2002,6 +2002,96 @@ static bool init_intercept_check(struct svm_test *test)
     return init_intercept;
 }
 
+/*
+ * Setting host EFLAGS.TF causes a #DB trap after the VMRUN completes on the
+ * host side (i.e., after the #VMEXIT from the guest).
+ *
+ * [AMD APM]
+ */
+static volatile u8 host_rflags_guest_flag = 0;
+static volatile u8 host_rflags_isr_flag = 0;
+static void ss_bp_isr(struct ex_regs *r)
+{
+	host_rflags_isr_flag = (host_rflags_guest_flag == 1) ? 2 : 1;
+	r->rflags &= ~X86_EFLAGS_TF;
+}
+
+static void host_rflags_prepare(struct svm_test *test)
+{
+	default_prepare(test);
+	handle_exception(DB_VECTOR, ss_bp_isr);
+	host_rflags_guest_flag = host_rflags_isr_flag = 0;
+	set_test_stage(test, 0);
+}
+
+static void host_rflags_test(struct svm_test *test)
+{
+	while (1) {
+		if (get_test_stage(test) > 0)
+			host_rflags_guest_flag =
+			    (host_rflags_isr_flag == 1) ? 2 : 1;
+		vmmcall();
+		if (get_test_stage(test) == 3)
+			break;
+	}
+}
+
+static bool host_rflags_finished(struct svm_test *test)
+{
+	switch (get_test_stage(test)) {
+	case 0:
+		if (vmcb->control.exit_code != SVM_EXIT_VMMCALL &&
+		    host_rflags_isr_flag != 0 && host_rflags_guest_flag != 0) {
+			report(false, "Unexpected VMEXIT. Exit reason 0x%x",
+			    vmcb->control.exit_code);
+			return true;
+		}
+		/*
+		 * Setting host EFLAGS.TF not immediately before VMRUN, causes
+		 * #DB trap before first guest instruction is executed
+		 */
+		write_rflags(read_rflags() | X86_EFLAGS_TF);
+		vmcb->save.rip += 3;
+		break;
+	case 1:
+		if (vmcb->control.exit_code != SVM_EXIT_VMMCALL &&
+		    host_rflags_isr_flag != 1 && host_rflags_guest_flag != 2) {
+			report(false, "Unexpected VMEXIT. Exit reason 0x%x",
+			    vmcb->control.exit_code);
+			return true;
+		}
+		host_rflags_guest_flag = host_rflags_isr_flag = 0;
+		vmcb->save.rip += 3;
+		/*
+		 * Setting host EFLAGS.TF immediately before VMRUN, causes #DB
+		 * trap after VMRUN completes on the host side (i.e., after
+		 * VMEXIT from guest).
+		 */
+		set_ss_bp_on_vmrun();
+		break;
+	case 2:
+		if (vmcb->control.exit_code != SVM_EXIT_VMMCALL &&
+		    host_rflags_isr_flag != 2 && host_rflags_guest_flag != 1) {
+			report(false, "Unexpected VMEXIT. Exit reason 0x%x",
+			    vmcb->control.exit_code);
+			return true;
+		}
+		host_rflags_guest_flag = host_rflags_isr_flag = 0;
+		vmcb->save.rip += 3;
+		unset_ss_bp_on_vmrun();
+		break;
+	default:
+		return true;
+	}
+	inc_test_stage(test);
+	return get_test_stage(test) == 4;
+}
+
+static bool host_rflags_check(struct svm_test *test)
+{
+	return get_test_stage(test) == 3;
+}
+
 #define TEST(name) { #name, .v2 = name }
 
 /*
@@ -2491,6 +2581,9 @@ struct svm_test svm_tests[] = {
     { "svm_init_intercept_test", smp_supported, init_intercept_prepare,
       default_prepare_gif_clear, init_intercept_test,
       init_intercept_finished, init_intercept_check, .on_vcpu = 2 },
+    { "host_rflags", default_supported, host_rflags_prepare,
+      default_prepare_gif_clear, host_rflags_test,
+      host_rflags_finished, host_rflags_check },
     TEST(svm_cr4_osxsave_test),
     TEST(svm_guest_state_test),
     TEST(svm_vmrun_errata_test),
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/3] nVMX: Add helper functions to set/unset host RFLAGS.TF on the VMRUN instruction
  2021-02-03  1:28 ` [PATCH 2/3] nVMX: Add helper functions to set/unset host RFLAGS.TF on the VMRUN instruction Krish Sadhukhan
@ 2021-02-03  8:15   ` Paolo Bonzini
  2021-02-05  0:20     ` Krish Sadhukhan
  0 siblings, 1 reply; 8+ messages in thread
From: Paolo Bonzini @ 2021-02-03  8:15 UTC (permalink / raw)
  To: Krish Sadhukhan, kvm; +Cc: jmattson, seanjc

On 03/02/21 02:28, Krish Sadhukhan wrote:
> Add helper functions to set host RFLAGS.TF immediately before the VMRUN
> instruction. These will be used  by the next patch to test Single Stepping
> on the VMRUN instruction from the host's perspective.
> 
> Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>

I think you can use prepare_gif_clear to set RFLAGS.TF and the exception 
handler can:

1) look for VMRUN at the interrupted EIP.  If it is there store the 
VMRUN address and set a flag.

2) on the next #DB (flag set), store the EIP and clear the flag

The finished callback then checks that the EIP was stored and that the 
two EIPs are 3 bytes apart (the length of a VMRUN).

Paolo

> ---
>   x86/svm.c       | 24 ++++++++++++++++++++++--
>   x86/svm.h       |  3 +++
>   x86/svm_tests.c |  1 -
>   3 files changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/x86/svm.c b/x86/svm.c
> index a1808c7..547f62a 100644
> --- a/x86/svm.c
> +++ b/x86/svm.c
> @@ -179,6 +179,17 @@ void vmcb_ident(struct vmcb *vmcb)
>   	}
>   }
>   
> +static bool ss_bp_on_vmrun = false;
> +void set_ss_bp_on_vmrun(void)
> +{
> +	ss_bp_on_vmrun = true;
> +}
> +
> +void unset_ss_bp_on_vmrun(void)
> +{
> +	ss_bp_on_vmrun = false;
> +}
> +
>   struct regs regs;
>   
>   struct regs get_regs(void)
> @@ -215,6 +226,12 @@ struct svm_test *v2_test;
>                   "mov regs, %%r15\n\t"           \
>                   "mov %%r15, 0x1f8(%%rax)\n\t"   \
>                   LOAD_GPR_C                      \
> +                "cmpb $0, %[ss_bp]\n\t"         \
> +                "je 1f\n\t"                     \
> +                "pushf; pop %%r8\n\t"           \
> +                "or $0x100, %%r8\n\t"           \
> +                "push %%r8; popf\n\t"           \
> +                "1: "                           \
>                   "vmrun %%rax\n\t"               \
>                   SAVE_GPR_C                      \
>                   "mov 0x170(%%rax), %%r15\n\t"   \
> @@ -234,7 +251,8 @@ int svm_vmrun(void)
>   	asm volatile (
>   		ASM_VMRUN_CMD
>   		:
> -		: "a" (virt_to_phys(vmcb))
> +		: "a" (virt_to_phys(vmcb)),
> +		[ss_bp]"m"(ss_bp_on_vmrun)
>   		: "memory", "r15");
>   
>   	return (vmcb->control.exit_code);
> @@ -253,6 +271,7 @@ static void test_run(struct svm_test *test)
>   	do {
>   		struct svm_test *the_test = test;
>   		u64 the_vmcb = vmcb_phys;
> +
>   		asm volatile (
>   			"clgi;\n\t" // semi-colon needed for LLVM compatibility
>   			"sti \n\t"
> @@ -266,7 +285,8 @@ static void test_run(struct svm_test *test)
>   			"=b" (the_vmcb)             // callee save register!
>   			: [test] "0" (the_test),
>   			[vmcb_phys] "1"(the_vmcb),
> -			[PREPARE_GIF_CLEAR] "i" (offsetof(struct svm_test, prepare_gif_clear))
> +			[PREPARE_GIF_CLEAR] "i" (offsetof(struct svm_test, prepare_gif_clear)),
> +			[ss_bp]"m"(ss_bp_on_vmrun)
>   			: "rax", "rcx", "rdx", "rsi",
>   			"r8", "r9", "r10", "r11" , "r12", "r13", "r14", "r15",
>   			"memory");
> diff --git a/x86/svm.h b/x86/svm.h
> index a0863b8..d521972 100644
> --- a/x86/svm.h
> +++ b/x86/svm.h
> @@ -391,6 +391,9 @@ void vmcb_ident(struct vmcb *vmcb);
>   struct regs get_regs(void);
>   void vmmcall(void);
>   int svm_vmrun(void);
> +int svm_vmrun1(void);
> +void set_ss_bp_on_vmrun(void);
> +void unset_ss_bp_on_vmrun(void);
>   void test_set_guest(test_guest_func func);
>   
>   extern struct vmcb *vmcb;
> diff --git a/x86/svm_tests.c b/x86/svm_tests.c
> index 29a0b59..7bf3624 100644
> --- a/x86/svm_tests.c
> +++ b/x86/svm_tests.c
> @@ -2046,7 +2046,6 @@ static void basic_guest_main(struct svm_test *test)
>   {
>   }
>   
> -
>   #define SVM_TEST_REG_RESERVED_BITS(start, end, inc, str_name, reg, val,	\
>   				   resv_mask)				\
>   {									\
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/3] nVMX: Add helper functions to set/unset host RFLAGS.TF on the VMRUN instruction
  2021-02-03  8:15   ` Paolo Bonzini
@ 2021-02-05  0:20     ` Krish Sadhukhan
  2021-02-05  8:21       ` Paolo Bonzini
  0 siblings, 1 reply; 8+ messages in thread
From: Krish Sadhukhan @ 2021-02-05  0:20 UTC (permalink / raw)
  To: Paolo Bonzini, kvm; +Cc: jmattson, seanjc


On 2/3/21 12:15 AM, Paolo Bonzini wrote:
> On 03/02/21 02:28, Krish Sadhukhan wrote:
>> Add helper functions to set host RFLAGS.TF immediately before the VMRUN
>> instruction. These will be used  by the next patch to test Single 
>> Stepping
>> on the VMRUN instruction from the host's perspective.
>>
>> Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
>
> I think you can use prepare_gif_clear to set RFLAGS.TF and the 
> exception handler can:
>
> 1) look for VMRUN at the interrupted EIP.  If it is there store the 
> VMRUN address and set a flag.
>
> 2) on the next #DB (flag set), store the EIP and clear the flag
>
> The finished callback then checks that the EIP was stored and that the 
> two EIPs are 3 bytes apart (the length of a VMRUN).


Thanks for the suggestion. It worked fine and I have sent out v2.

However, I couldn't use the two RIPs (VMRUN and post-VMRUN) to check the 
result because the post-VMRUN RIP was more than the length of the VMRUN 
instruction i.e., when #DB handler got executed following guest exit, 
the RIP had moved forward a few instructions from VMRUN. So, I have used 
the same mechanism I used in v1, to check the results.

>
> Paolo
>
>> ---
>>   x86/svm.c       | 24 ++++++++++++++++++++++--
>>   x86/svm.h       |  3 +++
>>   x86/svm_tests.c |  1 -
>>   3 files changed, 25 insertions(+), 3 deletions(-)
>>
>> diff --git a/x86/svm.c b/x86/svm.c
>> index a1808c7..547f62a 100644
>> --- a/x86/svm.c
>> +++ b/x86/svm.c
>> @@ -179,6 +179,17 @@ void vmcb_ident(struct vmcb *vmcb)
>>       }
>>   }
>>   +static bool ss_bp_on_vmrun = false;
>> +void set_ss_bp_on_vmrun(void)
>> +{
>> +    ss_bp_on_vmrun = true;
>> +}
>> +
>> +void unset_ss_bp_on_vmrun(void)
>> +{
>> +    ss_bp_on_vmrun = false;
>> +}
>> +
>>   struct regs regs;
>>     struct regs get_regs(void)
>> @@ -215,6 +226,12 @@ struct svm_test *v2_test;
>>                   "mov regs, %%r15\n\t"           \
>>                   "mov %%r15, 0x1f8(%%rax)\n\t"   \
>>                   LOAD_GPR_C                      \
>> +                "cmpb $0, %[ss_bp]\n\t"         \
>> +                "je 1f\n\t"                     \
>> +                "pushf; pop %%r8\n\t"           \
>> +                "or $0x100, %%r8\n\t"           \
>> +                "push %%r8; popf\n\t"           \
>> +                "1: "                           \
>>                   "vmrun %%rax\n\t"               \
>>                   SAVE_GPR_C                      \
>>                   "mov 0x170(%%rax), %%r15\n\t"   \
>> @@ -234,7 +251,8 @@ int svm_vmrun(void)
>>       asm volatile (
>>           ASM_VMRUN_CMD
>>           :
>> -        : "a" (virt_to_phys(vmcb))
>> +        : "a" (virt_to_phys(vmcb)),
>> +        [ss_bp]"m"(ss_bp_on_vmrun)
>>           : "memory", "r15");
>>         return (vmcb->control.exit_code);
>> @@ -253,6 +271,7 @@ static void test_run(struct svm_test *test)
>>       do {
>>           struct svm_test *the_test = test;
>>           u64 the_vmcb = vmcb_phys;
>> +
>>           asm volatile (
>>               "clgi;\n\t" // semi-colon needed for LLVM compatibility
>>               "sti \n\t"
>> @@ -266,7 +285,8 @@ static void test_run(struct svm_test *test)
>>               "=b" (the_vmcb)             // callee save register!
>>               : [test] "0" (the_test),
>>               [vmcb_phys] "1"(the_vmcb),
>> -            [PREPARE_GIF_CLEAR] "i" (offsetof(struct svm_test, 
>> prepare_gif_clear))
>> +            [PREPARE_GIF_CLEAR] "i" (offsetof(struct svm_test, 
>> prepare_gif_clear)),
>> +            [ss_bp]"m"(ss_bp_on_vmrun)
>>               : "rax", "rcx", "rdx", "rsi",
>>               "r8", "r9", "r10", "r11" , "r12", "r13", "r14", "r15",
>>               "memory");
>> diff --git a/x86/svm.h b/x86/svm.h
>> index a0863b8..d521972 100644
>> --- a/x86/svm.h
>> +++ b/x86/svm.h
>> @@ -391,6 +391,9 @@ void vmcb_ident(struct vmcb *vmcb);
>>   struct regs get_regs(void);
>>   void vmmcall(void);
>>   int svm_vmrun(void);
>> +int svm_vmrun1(void);
>> +void set_ss_bp_on_vmrun(void);
>> +void unset_ss_bp_on_vmrun(void);
>>   void test_set_guest(test_guest_func func);
>>     extern struct vmcb *vmcb;
>> diff --git a/x86/svm_tests.c b/x86/svm_tests.c
>> index 29a0b59..7bf3624 100644
>> --- a/x86/svm_tests.c
>> +++ b/x86/svm_tests.c
>> @@ -2046,7 +2046,6 @@ static void basic_guest_main(struct svm_test 
>> *test)
>>   {
>>   }
>>   -
>>   #define SVM_TEST_REG_RESERVED_BITS(start, end, inc, str_name, reg, 
>> val,    \
>>                      resv_mask)                \
>>   {                                    \
>>
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/3] nVMX: Add helper functions to set/unset host RFLAGS.TF on the VMRUN instruction
  2021-02-05  0:20     ` Krish Sadhukhan
@ 2021-02-05  8:21       ` Paolo Bonzini
  2021-02-23 20:10         ` Krish Sadhukhan
  0 siblings, 1 reply; 8+ messages in thread
From: Paolo Bonzini @ 2021-02-05  8:21 UTC (permalink / raw)
  To: Krish Sadhukhan, kvm; +Cc: jmattson, seanjc

On 05/02/21 01:20, Krish Sadhukhan wrote:
>>
>> I think you can use prepare_gif_clear to set RFLAGS.TF and the 
>> exception handler can:
>>
>> 1) look for VMRUN at the interrupted EIP.  If it is there store the 
>> VMRUN address and set a flag.
>>
>> 2) on the next #DB (flag set), store the EIP and clear the flag
>>
>> The finished callback then checks that the EIP was stored and that the 
>> two EIPs are 3 bytes apart (the length of a VMRUN).
> 
> 
> Thanks for the suggestion. It worked fine and I have sent out v2.
> 
> However, I couldn't use the two RIPs (VMRUN and post-VMRUN) to check the 
> result because the post-VMRUN RIP was more than the length of the VMRUN 
> instruction i.e., when #DB handler got executed following guest exit, 
> the RIP had moved forward a few instructions from VMRUN. So, I have used 
> the same mechanism I used in v1, to check the results.

Where did it move to?  (And could it be a KVM bug?)

Paolo


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/3] nVMX: Add helper functions to set/unset host RFLAGS.TF on the VMRUN instruction
  2021-02-05  8:21       ` Paolo Bonzini
@ 2021-02-23 20:10         ` Krish Sadhukhan
  0 siblings, 0 replies; 8+ messages in thread
From: Krish Sadhukhan @ 2021-02-23 20:10 UTC (permalink / raw)
  To: Paolo Bonzini, kvm; +Cc: jmattson, seanjc


On 2/5/21 12:21 AM, Paolo Bonzini wrote:
> On 05/02/21 01:20, Krish Sadhukhan wrote:
>>>
>>> I think you can use prepare_gif_clear to set RFLAGS.TF and the 
>>> exception handler can:
>>>
>>> 1) look for VMRUN at the interrupted EIP.  If it is there store the 
>>> VMRUN address and set a flag.
>>>
>>> 2) on the next #DB (flag set), store the EIP and clear the flag
>>>
>>> The finished callback then checks that the EIP was stored and that 
>>> the two EIPs are 3 bytes apart (the length of a VMRUN).
>>
>>
>> Thanks for the suggestion. It worked fine and I have sent out v2.
>>
>> However, I couldn't use the two RIPs (VMRUN and post-VMRUN) to check 
>> the result because the post-VMRUN RIP was more than the length of the 
>> VMRUN instruction i.e., when #DB handler got executed following guest 
>> exit, the RIP had moved forward a few instructions from VMRUN. So, I 
>> have used the same mechanism I used in v1, to check the results.
>
> Where did it move to?  (And could it be a KVM bug?)


It moved to the next-to-next instruction and it turned out to be a KVM 
(SVM) bug. I have added a fix to v3 that I have sent out.

>
> Paolo
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-02-23 20:13 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-03  1:28 [PATCH 0/3] nSVM: Test host RFLAGS.TF on VMRUN Krish Sadhukhan
2021-02-03  1:28 ` [PATCH 1/3] KVM: SVM: Replace hard-coded value with #define Krish Sadhukhan
2021-02-03  1:28 ` [PATCH 2/3] nVMX: Add helper functions to set/unset host RFLAGS.TF on the VMRUN instruction Krish Sadhukhan
2021-02-03  8:15   ` Paolo Bonzini
2021-02-05  0:20     ` Krish Sadhukhan
2021-02-05  8:21       ` Paolo Bonzini
2021-02-23 20:10         ` Krish Sadhukhan
2021-02-03  1:28 ` [PATCH 3/3] nSVM: Test effect of host RFLAGS.TF on VMRUN Krish Sadhukhan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.