* [PATCH] kvm-unit-tests: VMX: Fix confused definition of rflags
@ 2013-07-30 15:41 Arthur Chunqi Li
2013-07-30 15:43 ` Arthur Chunqi Li
2013-08-05 8:29 ` Gleb Natapov
0 siblings, 2 replies; 6+ messages in thread
From: Arthur Chunqi Li @ 2013-07-30 15:41 UTC (permalink / raw)
To: kvm; +Cc: jan.kiszka, gleb, pbonzini, Arthur Chunqi Li
Change "rflags" in "struct regs" to "host_rflags". Remove settings
to GUEST_RFLAGS since GUEST_RFLAGS can be set by vmwrite. Treat
host_rflags as host rflags before and after vmenter.
Besides, add checks to flags after vmenter.
Signed-off-by: Arthur Chunqi Li <yzt356@gmail.com>
---
x86/vmx.c | 11 ++++++-----
x86/vmx.h | 2 +-
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/x86/vmx.c b/x86/vmx.c
index 7467927..082c3bb 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -481,6 +481,8 @@ static int vmx_run()
"vmresume\n\t"
"2: "
"setbe %0\n\t"
+ "jbe vmx_return\n\t"
+ "ud2\n\t"
"vmx_return:\n\t"
SAVE_GPR_C
SAVE_RFLAGS
@@ -505,15 +507,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 ((!(regs.host_rflags & X86_EFLAGS_CF) && !(regs.host_rflags & X86_EFLAGS_ZF))
+ || ((regs.host_rflags & X86_EFLAGS_CF) && (regs.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 ((!(regs.host_rflags & X86_EFLAGS_CF) && !(regs.host_rflags & X86_EFLAGS_ZF))
+ || ((regs.host_rflags & X86_EFLAGS_CF) && (regs.host_rflags & X86_EFLAGS_ZF)))
printf("\tvmresume set wrong flags\n");
report("test vmresume", 0);
break;
@@ -540,7 +542,6 @@ static int test_run(struct vmx_test *test)
test->exits = 0;
current = test;
regs = test->guest_regs;
- vmcs_write(GUEST_RFLAGS, regs.rflags | 0x2);
launched = 0;
printf("\nTest suite : %s\n", test->name);
vmx_run();
diff --git a/x86/vmx.h b/x86/vmx.h
index 1fb9738..d80e000 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -27,7 +27,7 @@ struct regs {
u64 r13;
u64 r14;
u64 r15;
- u64 rflags;
+ u64 host_rflags;
};
struct vmx_test {
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] kvm-unit-tests: VMX: Fix confused definition of rflags
2013-07-30 15:41 [PATCH] kvm-unit-tests: VMX: Fix confused definition of rflags Arthur Chunqi Li
@ 2013-07-30 15:43 ` Arthur Chunqi Li
2013-08-05 8:29 ` Gleb Natapov
1 sibling, 0 replies; 6+ messages in thread
From: Arthur Chunqi Li @ 2013-07-30 15:43 UTC (permalink / raw)
To: kvm; +Cc: Jan Kiszka, Gleb Natapov, Paolo Bonzini, Arthur Chunqi Li
Since Paolo's advice is a small change and this is only a test suite,
so I don't split these two and commit in one patch.
On Tue, Jul 30, 2013 at 11:41 PM, Arthur Chunqi Li <yzt356@gmail.com> wrote:
> Change "rflags" in "struct regs" to "host_rflags". Remove settings
> to GUEST_RFLAGS since GUEST_RFLAGS can be set by vmwrite. Treat
> host_rflags as host rflags before and after vmenter.
>
> Besides, add checks to flags after vmenter.
>
> Signed-off-by: Arthur Chunqi Li <yzt356@gmail.com>
> ---
> x86/vmx.c | 11 ++++++-----
> x86/vmx.h | 2 +-
> 2 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/x86/vmx.c b/x86/vmx.c
> index 7467927..082c3bb 100644
> --- a/x86/vmx.c
> +++ b/x86/vmx.c
> @@ -481,6 +481,8 @@ static int vmx_run()
> "vmresume\n\t"
> "2: "
> "setbe %0\n\t"
> + "jbe vmx_return\n\t"
> + "ud2\n\t"
> "vmx_return:\n\t"
> SAVE_GPR_C
> SAVE_RFLAGS
> @@ -505,15 +507,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 ((!(regs.host_rflags & X86_EFLAGS_CF) && !(regs.host_rflags & X86_EFLAGS_ZF))
> + || ((regs.host_rflags & X86_EFLAGS_CF) && (regs.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 ((!(regs.host_rflags & X86_EFLAGS_CF) && !(regs.host_rflags & X86_EFLAGS_ZF))
> + || ((regs.host_rflags & X86_EFLAGS_CF) && (regs.host_rflags & X86_EFLAGS_ZF)))
> printf("\tvmresume set wrong flags\n");
> report("test vmresume", 0);
> break;
> @@ -540,7 +542,6 @@ static int test_run(struct vmx_test *test)
> test->exits = 0;
> current = test;
> regs = test->guest_regs;
> - vmcs_write(GUEST_RFLAGS, regs.rflags | 0x2);
> launched = 0;
> printf("\nTest suite : %s\n", test->name);
> vmx_run();
> diff --git a/x86/vmx.h b/x86/vmx.h
> index 1fb9738..d80e000 100644
> --- a/x86/vmx.h
> +++ b/x86/vmx.h
> @@ -27,7 +27,7 @@ struct regs {
> u64 r13;
> u64 r14;
> u64 r15;
> - u64 rflags;
> + u64 host_rflags;
> };
>
> struct vmx_test {
> --
> 1.7.9.5
>
--
Arthur Chunqi Li
Department of Computer Science
School of EECS
Peking University
Beijing, China
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] kvm-unit-tests: VMX: Fix confused definition of rflags
2013-07-30 15:41 [PATCH] kvm-unit-tests: VMX: Fix confused definition of rflags Arthur Chunqi Li
2013-07-30 15:43 ` Arthur Chunqi Li
@ 2013-08-05 8:29 ` Gleb Natapov
2013-08-05 8:48 ` Arthur Chunqi Li
1 sibling, 1 reply; 6+ messages in thread
From: Gleb Natapov @ 2013-08-05 8:29 UTC (permalink / raw)
To: Arthur Chunqi Li; +Cc: kvm, jan.kiszka, pbonzini
On Tue, Jul 30, 2013 at 11:41:00PM +0800, Arthur Chunqi Li wrote:
> Change "rflags" in "struct regs" to "host_rflags". Remove settings
> to GUEST_RFLAGS since GUEST_RFLAGS can be set by vmwrite. Treat
> host_rflags as host rflags before and after vmenter.
>
I am not sure the change is for the better. Before the change one could
set up rflags for guest environment by setting regs.rflags, no special
init function had to be written. I do not see any problem with correct
code, except that rflags is not correct on a guest entry, but this
should be easy to fix.
> Besides, add checks to flags after vmenter.
>
> Signed-off-by: Arthur Chunqi Li <yzt356@gmail.com>
> ---
> x86/vmx.c | 11 ++++++-----
> x86/vmx.h | 2 +-
> 2 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/x86/vmx.c b/x86/vmx.c
> index 7467927..082c3bb 100644
> --- a/x86/vmx.c
> +++ b/x86/vmx.c
> @@ -481,6 +481,8 @@ static int vmx_run()
> "vmresume\n\t"
> "2: "
> "setbe %0\n\t"
> + "jbe vmx_return\n\t"
> + "ud2\n\t"
> "vmx_return:\n\t"
> SAVE_GPR_C
> SAVE_RFLAGS
> @@ -505,15 +507,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 ((!(regs.host_rflags & X86_EFLAGS_CF) && !(regs.host_rflags & X86_EFLAGS_ZF))
> + || ((regs.host_rflags & X86_EFLAGS_CF) && (regs.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 ((!(regs.host_rflags & X86_EFLAGS_CF) && !(regs.host_rflags & X86_EFLAGS_ZF))
> + || ((regs.host_rflags & X86_EFLAGS_CF) && (regs.host_rflags & X86_EFLAGS_ZF)))
> printf("\tvmresume set wrong flags\n");
> report("test vmresume", 0);
> break;
> @@ -540,7 +542,6 @@ static int test_run(struct vmx_test *test)
> test->exits = 0;
> current = test;
> regs = test->guest_regs;
> - vmcs_write(GUEST_RFLAGS, regs.rflags | 0x2);
> launched = 0;
> printf("\nTest suite : %s\n", test->name);
> vmx_run();
> diff --git a/x86/vmx.h b/x86/vmx.h
> index 1fb9738..d80e000 100644
> --- a/x86/vmx.h
> +++ b/x86/vmx.h
> @@ -27,7 +27,7 @@ struct regs {
> u64 r13;
> u64 r14;
> u64 r15;
> - u64 rflags;
> + u64 host_rflags;
> };
>
> struct vmx_test {
> --
> 1.7.9.5
--
Gleb.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] kvm-unit-tests: VMX: Fix confused definition of rflags
2013-08-05 8:29 ` Gleb Natapov
@ 2013-08-05 8:48 ` Arthur Chunqi Li
2013-08-05 9:10 ` Gleb Natapov
0 siblings, 1 reply; 6+ messages in thread
From: Arthur Chunqi Li @ 2013-08-05 8:48 UTC (permalink / raw)
To: Gleb Natapov; +Cc: kvm, Jan Kiszka, Paolo Bonzini
On Mon, Aug 5, 2013 at 4:29 PM, Gleb Natapov <gleb@redhat.com> wrote:
> On Tue, Jul 30, 2013 at 11:41:00PM +0800, Arthur Chunqi Li wrote:
>> Change "rflags" in "struct regs" to "host_rflags". Remove settings
>> to GUEST_RFLAGS since GUEST_RFLAGS can be set by vmwrite. Treat
>> host_rflags as host rflags before and after vmenter.
>>
> I am not sure the change is for the better. Before the change one could
> set up rflags for guest environment by setting regs.rflags, no special
> init function had to be written. I do not see any problem with correct
> code, except that rflags is not correct on a guest entry, but this
> should be easy to fix.
regs.rflags are designed to set guest rflags, but the current
implementation just use it as host_rflags. For every VM entry, it will
load value set by vmcs_write(GUEST_RFLAGS). Set regs.flags as host
rflags and then enter VM cannot affect VM's rflags, which is the
current implementation.
Besides, if host want to set/get guest's rflags, it just use
vmcs_write/read(GUEST_RFLAGS).
Arthur
>
>> Besides, add checks to flags after vmenter.
>>
>> Signed-off-by: Arthur Chunqi Li <yzt356@gmail.com>
>> ---
>> x86/vmx.c | 11 ++++++-----
>> x86/vmx.h | 2 +-
>> 2 files changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/x86/vmx.c b/x86/vmx.c
>> index 7467927..082c3bb 100644
>> --- a/x86/vmx.c
>> +++ b/x86/vmx.c
>> @@ -481,6 +481,8 @@ static int vmx_run()
>> "vmresume\n\t"
>> "2: "
>> "setbe %0\n\t"
>> + "jbe vmx_return\n\t"
>> + "ud2\n\t"
>> "vmx_return:\n\t"
>> SAVE_GPR_C
>> SAVE_RFLAGS
>> @@ -505,15 +507,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 ((!(regs.host_rflags & X86_EFLAGS_CF) && !(regs.host_rflags & X86_EFLAGS_ZF))
>> + || ((regs.host_rflags & X86_EFLAGS_CF) && (regs.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 ((!(regs.host_rflags & X86_EFLAGS_CF) && !(regs.host_rflags & X86_EFLAGS_ZF))
>> + || ((regs.host_rflags & X86_EFLAGS_CF) && (regs.host_rflags & X86_EFLAGS_ZF)))
>> printf("\tvmresume set wrong flags\n");
>> report("test vmresume", 0);
>> break;
>> @@ -540,7 +542,6 @@ static int test_run(struct vmx_test *test)
>> test->exits = 0;
>> current = test;
>> regs = test->guest_regs;
>> - vmcs_write(GUEST_RFLAGS, regs.rflags | 0x2);
>> launched = 0;
>> printf("\nTest suite : %s\n", test->name);
>> vmx_run();
>> diff --git a/x86/vmx.h b/x86/vmx.h
>> index 1fb9738..d80e000 100644
>> --- a/x86/vmx.h
>> +++ b/x86/vmx.h
>> @@ -27,7 +27,7 @@ struct regs {
>> u64 r13;
>> u64 r14;
>> u64 r15;
>> - u64 rflags;
>> + u64 host_rflags;
>> };
>>
>> struct vmx_test {
>> --
>> 1.7.9.5
>
> --
> Gleb.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] kvm-unit-tests: VMX: Fix confused definition of rflags
2013-08-05 8:48 ` Arthur Chunqi Li
@ 2013-08-05 9:10 ` Gleb Natapov
2013-08-05 9:27 ` Gmail
0 siblings, 1 reply; 6+ messages in thread
From: Gleb Natapov @ 2013-08-05 9:10 UTC (permalink / raw)
To: Arthur Chunqi Li; +Cc: kvm, Jan Kiszka, Paolo Bonzini
On Mon, Aug 05, 2013 at 04:48:47PM +0800, Arthur Chunqi Li wrote:
> On Mon, Aug 5, 2013 at 4:29 PM, Gleb Natapov <gleb@redhat.com> wrote:
> > On Tue, Jul 30, 2013 at 11:41:00PM +0800, Arthur Chunqi Li wrote:
> >> Change "rflags" in "struct regs" to "host_rflags". Remove settings
> >> to GUEST_RFLAGS since GUEST_RFLAGS can be set by vmwrite. Treat
> >> host_rflags as host rflags before and after vmenter.
> >>
> > I am not sure the change is for the better. Before the change one could
> > set up rflags for guest environment by setting regs.rflags, no special
> > init function had to be written. I do not see any problem with correct
> > code, except that rflags is not correct on a guest entry, but this
> > should be easy to fix.
> regs.rflags are designed to set guest rflags, but the current
> implementation just use it as host_rflags.
Current implementation uses it to set guest flags at the beginning of
a test.
> For every VM entry, it will
> load value set by vmcs_write(GUEST_RFLAGS).
Not for every vmentry, only for the first one. Doing it for every
vmentry without saving it first with vmcs_read would been incorrect.
> Set regs.flags as host
> rflags and then enter VM cannot affect VM's rflags, which is the
> current implementation.
>
It can, on the first launch. It is easy to add code to exit_handler() to
set current->guest_regs.rflags correctly before calling test's exit handler
and write GUEST_RFLAGS according to current->guest_regs.rflags after
exit handler returns, but as you say below test can do it by itself in
exit handler if it wishes so.
> Besides, if host want to set/get guest's rflags, it just use
> vmcs_write/read(GUEST_RFLAGS).
>
True, but to do it at the beginning of the test it will require to write
special init function even if non are needed otherwise. So you are
removing this functionality without clear benefit.
> Arthur
> >
> >> Besides, add checks to flags after vmenter.
> >>
> >> Signed-off-by: Arthur Chunqi Li <yzt356@gmail.com>
> >> ---
> >> x86/vmx.c | 11 ++++++-----
> >> x86/vmx.h | 2 +-
> >> 2 files changed, 7 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/x86/vmx.c b/x86/vmx.c
> >> index 7467927..082c3bb 100644
> >> --- a/x86/vmx.c
> >> +++ b/x86/vmx.c
> >> @@ -481,6 +481,8 @@ static int vmx_run()
> >> "vmresume\n\t"
> >> "2: "
> >> "setbe %0\n\t"
> >> + "jbe vmx_return\n\t"
> >> + "ud2\n\t"
> >> "vmx_return:\n\t"
> >> SAVE_GPR_C
> >> SAVE_RFLAGS
> >> @@ -505,15 +507,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 ((!(regs.host_rflags & X86_EFLAGS_CF) && !(regs.host_rflags & X86_EFLAGS_ZF))
> >> + || ((regs.host_rflags & X86_EFLAGS_CF) && (regs.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 ((!(regs.host_rflags & X86_EFLAGS_CF) && !(regs.host_rflags & X86_EFLAGS_ZF))
> >> + || ((regs.host_rflags & X86_EFLAGS_CF) && (regs.host_rflags & X86_EFLAGS_ZF)))
> >> printf("\tvmresume set wrong flags\n");
> >> report("test vmresume", 0);
> >> break;
> >> @@ -540,7 +542,6 @@ static int test_run(struct vmx_test *test)
> >> test->exits = 0;
> >> current = test;
> >> regs = test->guest_regs;
> >> - vmcs_write(GUEST_RFLAGS, regs.rflags | 0x2);
> >> launched = 0;
> >> printf("\nTest suite : %s\n", test->name);
> >> vmx_run();
> >> diff --git a/x86/vmx.h b/x86/vmx.h
> >> index 1fb9738..d80e000 100644
> >> --- a/x86/vmx.h
> >> +++ b/x86/vmx.h
> >> @@ -27,7 +27,7 @@ struct regs {
> >> u64 r13;
> >> u64 r14;
> >> u64 r15;
> >> - u64 rflags;
> >> + u64 host_rflags;
> >> };
> >>
> >> struct vmx_test {
> >> --
> >> 1.7.9.5
> >
> > --
> > Gleb.
--
Gleb.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] kvm-unit-tests: VMX: Fix confused definition of rflags
2013-08-05 9:10 ` Gleb Natapov
@ 2013-08-05 9:27 ` Gmail
0 siblings, 0 replies; 6+ messages in thread
From: Gmail @ 2013-08-05 9:27 UTC (permalink / raw)
To: Gleb Natapov; +Cc: kvm, Jan Kiszka, Paolo Bonzini
在 2013-8-5，17:10，Gleb Natapov <gleb@redhat.com> 写道：
> On Mon, Aug 05, 2013 at 04:48:47PM +0800, Arthur Chunqi Li wrote:
>> On Mon, Aug 5, 2013 at 4:29 PM, Gleb Natapov <gleb@redhat.com> wrote:
>>> On Tue, Jul 30, 2013 at 11:41:00PM +0800, Arthur Chunqi Li wrote:
>>>> Change "rflags" in "struct regs" to "host_rflags". Remove settings
>>>> to GUEST_RFLAGS since GUEST_RFLAGS can be set by vmwrite. Treat
>>>> host_rflags as host rflags before and after vmenter.
>>> I am not sure the change is for the better. Before the change one could
>>> set up rflags for guest environment by setting regs.rflags, no special
>>> init function had to be written. I do not see any problem with correct
>>> code, except that rflags is not correct on a guest entry, but this
>>> should be easy to fix.
>> regs.rflags are designed to set guest rflags, but the current
>> implementation just use it as host_rflags.
> Current implementation uses it to set guest flags at the beginning of
> a test.
>
>> For every VM entry, it will
>> load value set by vmcs_write(GUEST_RFLAGS).
> Not for every vmentry, only for the first one. Doing it for every
> vmentry without saving it first with vmcs_read would been incorrect.
>
>> Set regs.flags as host
>> rflags and then enter VM cannot affect VM's rflags, which is the
>> current implementation.
> It can, on the first launch. It is easy to add code to exit_handler() to
> set current->guest_regs.rflags correctly before calling test's exit handler
> and write GUEST_RFLAGS according to current->guest_regs.rflags after
> exit handler returns, but as you say below test can do it by itself in
> exit handler if it wishes so.
>
>> Besides, if host want to set/get guest's rflags, it just use
>> vmcs_write/read(GUEST_RFLAGS).
> True, but to do it at the beginning of the test it will require to write
> special init function even if non are needed otherwise. So you are
> removing this functionality without clear benefit.
True, so it seems better to keep it. However, now it is confused with host_rflags and not correctly set/get when vmentry/vmexit. I will commit another patch to fix both bugs.
Arthur
>
>> Arthur
>>>
>>>> Besides, add checks to flags after vmenter.
>>>>
>>>> Signed-off-by: Arthur Chunqi Li <yzt356@gmail.com>
>>>> ---
>>>> x86/vmx.c | 11 ++++++-----
>>>> x86/vmx.h | 2 +-
>>>> 2 files changed, 7 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/x86/vmx.c b/x86/vmx.c
>>>> index 7467927..082c3bb 100644
>>>> --- a/x86/vmx.c
>>>> +++ b/x86/vmx.c
>>>> @@ -481,6 +481,8 @@ static int vmx_run()
>>>> "vmresume\n\t"
>>>> "2: "
>>>> "setbe %0\n\t"
>>>> + "jbe vmx_return\n\t"
>>>> + "ud2\n\t"
>>>> "vmx_return:\n\t"
>>>> SAVE_GPR_C
>>>> SAVE_RFLAGS
>>>> @@ -505,15 +507,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 ((!(regs.host_rflags & X86_EFLAGS_CF) && !(regs.host_rflags & X86_EFLAGS_ZF))
>>>> + || ((regs.host_rflags & X86_EFLAGS_CF) && (regs.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 ((!(regs.host_rflags & X86_EFLAGS_CF) && !(regs.host_rflags & X86_EFLAGS_ZF))
>>>> + || ((regs.host_rflags & X86_EFLAGS_CF) && (regs.host_rflags & X86_EFLAGS_ZF)))
>>>> printf("\tvmresume set wrong flags\n");
>>>> report("test vmresume", 0);
>>>> break;
>>>> @@ -540,7 +542,6 @@ static int test_run(struct vmx_test *test)
>>>> test->exits = 0;
>>>> current = test;
>>>> regs = test->guest_regs;
>>>> - vmcs_write(GUEST_RFLAGS, regs.rflags | 0x2);
>>>> launched = 0;
>>>> printf("\nTest suite : %s\n", test->name);
>>>> vmx_run();
>>>> diff --git a/x86/vmx.h b/x86/vmx.h
>>>> index 1fb9738..d80e000 100644
>>>> --- a/x86/vmx.h
>>>> +++ b/x86/vmx.h
>>>> @@ -27,7 +27,7 @@ struct regs {
>>>> u64 r13;
>>>> u64 r14;
>>>> u64 r15;
>>>> - u64 rflags;
>>>> + u64 host_rflags;
>>>> };
>>>>
>>>> struct vmx_test {
>>>> --
>>>> 1.7.9.5
>>>
>>> --
>>> Gleb.
>
> --
> Gleb.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-08-05 9:27 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-07-30 15:41 [PATCH] kvm-unit-tests: VMX: Fix confused definition of rflags Arthur Chunqi Li
2013-07-30 15:43 ` Arthur Chunqi Li
2013-08-05 8:29 ` Gleb Natapov
2013-08-05 8:48 ` Arthur Chunqi Li
2013-08-05 9:10 ` Gleb Natapov
2013-08-05 9:27 ` Gmail
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.