kvm-unit-tests: VMX: Separate host and guest rflags

PATCH Applied Arthur Chunqi Li 3 participants 4 messages Aug 5, 2013 – Aug 7, 2013 KVM

Share & tools

Arthur Chunqi Li

author

2013-08-05 12:43 UTC #0 lore raw

PATCH 1/1 kvm-unit-tests: VMX: Separate host and guest rflags

Separate host_rflags and guest_rflags (regs.rflags used for guest). Fix bug of set/get guest rflags when vmenter/vmexit.

Signed-off-by Arthur Chunqi Li <[email protected]>

---
 x86/vmx.c |   11 +++++++----
 x86/vmx.h |    4 ++--
 2 files changed, 9 insertions(+), 6 deletions(-)

x86/vmx.c +7 -4

index 7467927..7b28aca 100644
@@ -19,6 +19,7 @@ struct regs regs;
 struct vmx_test *current;
 u64 hypercall_field = 0;
 bool launched;
+u64 host_rflags;
 
 extern u64 gdt64_desc[];
 extern u64 idt_descr[];

@@ -440,12 +441,14 @@ static int exit_handler()
 	int ret;
 
 	current->exits++;
+	regs.rflags = vmcs_read(GUEST_RFLAGS);
 	current->guest_regs = regs;
 	if (is_hypercall())
 		ret = handle_hypercall();
 	else
 		ret = current->exit_handler();
 	regs = current->guest_regs;
+	vmcs_write(GUEST_RFLAGS, regs.rflags);
 	switch (ret) {
 	case VMX_TEST_VMEXIT:
 	case VMX_TEST_RESUME:

@@ -505,15 +508,15 @@ static int vmx_run()
 		return 0;
 	case VMX_TEST_LAUNCH_ERR:
 		printf("%s : vmlaunch failed.\n", __func__);
-		if ((!(regs.rflags & X86_EFLAGS_CF) && !(regs.rflags & X86_EFLAGS_ZF))
-			|| ((regs.rflags & X86_EFLAGS_CF) && (regs.rflags & X86_EFLAGS_ZF)))
+		if ((!(host_rflags & X86_EFLAGS_CF) && !(host_rflags & X86_EFLAGS_ZF))
+			|| ((host_rflags & X86_EFLAGS_CF) && (host_rflags & X86_EFLAGS_ZF)))
 			printf("\tvmlaunch set wrong flags\n");
 		report("test vmlaunch", 0);
 		break;
 	case VMX_TEST_RESUME_ERR:
 		printf("%s : vmresume failed.\n", __func__);
-		if ((!(regs.rflags & X86_EFLAGS_CF) && !(regs.rflags & X86_EFLAGS_ZF))
-			|| ((regs.rflags & X86_EFLAGS_CF) && (regs.rflags & X86_EFLAGS_ZF)))
+		if ((!(host_rflags & X86_EFLAGS_CF) && !(host_rflags & X86_EFLAGS_ZF))
+			|| ((host_rflags & X86_EFLAGS_CF) && (host_rflags & X86_EFLAGS_ZF)))
 			printf("\tvmresume set wrong flags\n");
 		report("test vmresume", 0);
 		break;

x86/vmx.h +2 -2

index 1fb9738..d4f979c 100644
@@ -403,10 +403,10 @@ enum Ctrl1 {
 
 #define SAVE_RFLAGS		\
 	"pushf\n\t"			\
-	"pop regs+0x80\n\t"
+	"pop host_rflags\n\t"
 
 #define LOAD_RFLAGS		\
-	"push regs+0x80\n\t"	\
+	"push host_rflags\n\t"	\
 	"popf\n\t"
 
 #define VMX_IO_SIZE_MASK		0x7

Signature · 1 line

1.7.9.5

Gleb Natapov

2013-08-05 13:18 UTC #1 lore raw

Quoted 14 lines · Arthur Chunqi Li

Separate host_rflags and guest_rflags (regs.rflags used for guest). Fix bug of set/get guest rflags when vmenter/vmexit.
Signed-off-by Arthur Chunqi Li <[email protected]>
---
 x86/vmx.c |   11 +++++++----
 x86/vmx.h |    4 ++--
 2 files changed, 9 insertions(+), 6 deletions(-)
x86/vmx.c +0 -0
index 7467927..7b28aca 100644

x86/vmx.c struct regs regs; · line 23

Quoted 5 lines

x86/vmx.c +1 -0

@@ -19,6 +19,7 @@ struct regs regs;
 struct vmx_test *current;
 u64 hypercall_field = 0;
 bool launched;
+u64 host_rflags;

Can't you define in on stack?

Quoted 56 lines

x86/vmx.c +6 -4

 extern u64 gdt64_desc[];
 extern u64 idt_descr[];

@@ -440,12 +441,14 @@ static int exit_handler()
 	int ret;
 
 	current->exits++;
+	regs.rflags = vmcs_read(GUEST_RFLAGS);
 	current->guest_regs = regs;
 	if (is_hypercall())
 		ret = handle_hypercall();
 	else
 		ret = current->exit_handler();
 	regs = current->guest_regs;
+	vmcs_write(GUEST_RFLAGS, regs.rflags);
 	switch (ret) {
 	case VMX_TEST_VMEXIT:
 	case VMX_TEST_RESUME:

@@ -505,15 +508,15 @@ static int vmx_run()
 		return 0;
 	case VMX_TEST_LAUNCH_ERR:
 		printf("%s : vmlaunch failed.\n", __func__);
-		if ((!(regs.rflags & X86_EFLAGS_CF) && !(regs.rflags & X86_EFLAGS_ZF))
-			|| ((regs.rflags & X86_EFLAGS_CF) && (regs.rflags & X86_EFLAGS_ZF)))
+		if ((!(host_rflags & X86_EFLAGS_CF) && !(host_rflags & X86_EFLAGS_ZF))
+			|| ((host_rflags & X86_EFLAGS_CF) && (host_rflags & X86_EFLAGS_ZF)))
 			printf("\tvmlaunch set wrong flags\n");
 		report("test vmlaunch", 0);
 		break;
 	case VMX_TEST_RESUME_ERR:
 		printf("%s : vmresume failed.\n", __func__);
-		if ((!(regs.rflags & X86_EFLAGS_CF) && !(regs.rflags & X86_EFLAGS_ZF))
-			|| ((regs.rflags & X86_EFLAGS_CF) && (regs.rflags & X86_EFLAGS_ZF)))
+		if ((!(host_rflags & X86_EFLAGS_CF) && !(host_rflags & X86_EFLAGS_ZF))
+			|| ((host_rflags & X86_EFLAGS_CF) && (host_rflags & X86_EFLAGS_ZF)))
 			printf("\tvmresume set wrong flags\n");
 		report("test vmresume", 0);
 		break;

x86/vmx.h +2 -2

index 1fb9738..d4f979c 100644
@@ -403,10 +403,10 @@ enum Ctrl1 {
 
 #define SAVE_RFLAGS		\
 	"pushf\n\t"			\
-	"pop regs+0x80\n\t"
+	"pop host_rflags\n\t"
 
 #define LOAD_RFLAGS		\
-	"push regs+0x80\n\t"	\
+	"push host_rflags\n\t"	\
 	"popf\n\t"
 
 #define VMX_IO_SIZE_MASK		0x7

Signature · 1 line

1.7.9.5

--
Gleb.

Arthur Chunqi Li

author

2013-08-05 13:45 UTC #2 lore raw

Quoted 10 lines · Gleb Natapov

Quoted 9 lines · Arthur Chunqi Li
Separate host_rflags and guest_rflags (regs.rflags used for guest). Fix bug of set/get guest rflags when vmenter/vmexit.
Signed-off-by Arthur Chunqi Li <[email protected]>
---
 x86/vmx.c |   11 +++++++----
 x86/vmx.h |    4 ++--
 2 files changed, 9 insertions(+), 6 deletions(-)

Quoted 11 lines

x86/vmx.c struct regs regs; · line 23
Quoted 10 lines
x86/vmx.c +1 -0
index 7467927..7b28aca 100644
@@ -19,6 +19,7 @@ struct regs regs;
 struct vmx_test *current;
 u64 hypercall_field = 0;
 bool launched;
+u64 host_rflags;
 
Can't you define in on stack?

Currently I don't use it outside vmx_run(), but it may be used in user defined exit_handler in the future, so I put it globally.

Arthur

Quoted 60 lines

Quoted 56 lines

x86/vmx.c +6 -4

 extern u64 gdt64_desc[];
 extern u64 idt_descr[];

@@ -440,12 +441,14 @@ static int exit_handler()
      int ret;
 
      current->exits++;
+     regs.rflags = vmcs_read(GUEST_RFLAGS);
      current->guest_regs = regs;
      if (is_hypercall())
              ret = handle_hypercall();
      else
              ret = current->exit_handler();
      regs = current->guest_regs;
+     vmcs_write(GUEST_RFLAGS, regs.rflags);
      switch (ret) {
      case VMX_TEST_VMEXIT:
      case VMX_TEST_RESUME:

@@ -505,15 +508,15 @@ static int vmx_run()
              return 0;
      case VMX_TEST_LAUNCH_ERR:
              printf("%s : vmlaunch failed.\n", __func__);
-             if ((!(regs.rflags & X86_EFLAGS_CF) && !(regs.rflags & X86_EFLAGS_ZF))
-                     || ((regs.rflags & X86_EFLAGS_CF) && (regs.rflags & X86_EFLAGS_ZF)))
+             if ((!(host_rflags & X86_EFLAGS_CF) && !(host_rflags & X86_EFLAGS_ZF))
+                     || ((host_rflags & X86_EFLAGS_CF) && (host_rflags & X86_EFLAGS_ZF)))
                      printf("\tvmlaunch set wrong flags\n");
              report("test vmlaunch", 0);
              break;
      case VMX_TEST_RESUME_ERR:
              printf("%s : vmresume failed.\n", __func__);
-             if ((!(regs.rflags & X86_EFLAGS_CF) && !(regs.rflags & X86_EFLAGS_ZF))
-                     || ((regs.rflags & X86_EFLAGS_CF) && (regs.rflags & X86_EFLAGS_ZF)))
+             if ((!(host_rflags & X86_EFLAGS_CF) && !(host_rflags & X86_EFLAGS_ZF))
+                     || ((host_rflags & X86_EFLAGS_CF) && (host_rflags & X86_EFLAGS_ZF)))
                      printf("\tvmresume set wrong flags\n");
              report("test vmresume", 0);
              break;

x86/vmx.h +2 -3

index 1fb9738..d4f979c 100644
@@ -403,10 +403,10 @@ enum Ctrl1 {
 
 #define SAVE_RFLAGS          \
      "pushf\n\t"                     \
-     "pop regs+0x80\n\t"
+     "pop host_rflags\n\t"
 
 #define LOAD_RFLAGS          \
-     "push regs+0x80\n\t"    \
+     "push host_rflags\n\t"  \
      "popf\n\t"
 
 #define VMX_IO_SIZE_MASK             0x7
--

Quoted 1 line

1.7.9.5

--
Gleb.

Paolo Bonzini

maintainer Applied

2013-08-07 15:37 UTC #3 lore raw

In reply to Arthur Chunqi Li

Quoted 4 lines · Arthur Chunqi Li

Separate host_rflags and guest_rflags (regs.rflags used for guest). Fix bug of set/get guest rflags when vmenter/vmexit.
Signed-off-by Arthur Chunqi Li <[email protected]>

Applied, thanks.

Not putting it on the stack is fine, but please do a followup patch to define vmx.c-private variables as "static" (as soon as I push the patches to kernel.org).

Paolo

Quoted 70 lines

---
  x86/vmx.c |   11 +++++++----
  x86/vmx.h |    4 ++--
  2 files changed, 9 insertions(+), 6 deletions(-)

x86/vmx.c +7 -4

index 7467927..7b28aca 100644
@@ -19,6 +19,7 @@ struct regs regs;
  struct vmx_test *current;
  u64 hypercall_field = 0;
  bool launched;
+u64 host_rflags;
 
  extern u64 gdt64_desc[];
  extern u64 idt_descr[];

@@ -440,12 +441,14 @@ static int exit_handler()
  	int ret;
 
  	current->exits++;
+	regs.rflags = vmcs_read(GUEST_RFLAGS);
  	current->guest_regs = regs;
  	if (is_hypercall())
  		ret = handle_hypercall();
  	else
  		ret = current->exit_handler();
  	regs = current->guest_regs;
+	vmcs_write(GUEST_RFLAGS, regs.rflags);
  	switch (ret) {
  	case VMX_TEST_VMEXIT:
  	case VMX_TEST_RESUME:

@@ -505,15 +508,15 @@ static int vmx_run()
  		return 0;
  	case VMX_TEST_LAUNCH_ERR:
  		printf("%s : vmlaunch failed.\n", __func__);
-		if ((!(regs.rflags & X86_EFLAGS_CF) && !(regs.rflags & X86_EFLAGS_ZF))
-			|| ((regs.rflags & X86_EFLAGS_CF) && (regs.rflags & X86_EFLAGS_ZF)))
+		if ((!(host_rflags & X86_EFLAGS_CF) && !(host_rflags & X86_EFLAGS_ZF))
+			|| ((host_rflags & X86_EFLAGS_CF) && (host_rflags & X86_EFLAGS_ZF)))
  			printf("\tvmlaunch set wrong flags\n");
  		report("test vmlaunch", 0);
  		break;
  	case VMX_TEST_RESUME_ERR:
  		printf("%s : vmresume failed.\n", __func__);
-		if ((!(regs.rflags & X86_EFLAGS_CF) && !(regs.rflags & X86_EFLAGS_ZF))
-			|| ((regs.rflags & X86_EFLAGS_CF) && (regs.rflags & X86_EFLAGS_ZF)))
+		if ((!(host_rflags & X86_EFLAGS_CF) && !(host_rflags & X86_EFLAGS_ZF))
+			|| ((host_rflags & X86_EFLAGS_CF) && (host_rflags & X86_EFLAGS_ZF)))
  			printf("\tvmresume set wrong flags\n");
  		report("test vmresume", 0);
  		break;

x86/vmx.h +2 -2

index 1fb9738..d4f979c 100644
@@ -403,10 +403,10 @@ enum Ctrl1 {
 
  #define SAVE_RFLAGS		\
  	"pushf\n\t"			\
-	"pop regs+0x80\n\t"
+	"pop host_rflags\n\t"
 
  #define LOAD_RFLAGS		\
-	"push regs+0x80\n\t"	\
+	"push host_rflags\n\t"	\
  	"popf\n\t"
 
  #define VMX_IO_SIZE_MASK		0x7