Re: [PATCH 2/4] kvm-unit-tests: VMX: Add test cases for CR0/4 shadowing

[Jan Kiszka <jan.kiszka@web.de>]

[-- Attachment #1: Type: text/plain, Size: 2513 bytes --]

On 2013-08-15 09:59, Arthur Chunqi Li wrote:
> On Thu, Aug 15, 2013 at 3:47 PM, Jan Kiszka <jan.kiszka@web.de> wrote:
>> On 2013-08-15 09:40, Arthur Chunqi Li wrote:
>>> On Thu, Aug 15, 2013 at 3:30 PM, Jan Kiszka <jan.kiszka@web.de> wrote:
>>>> On 2013-08-13 17:56, Arthur Chunqi Li wrote:
>>>>> Add testing for CR0/4 shadowing.
>>>>
>>>> A few sentences on the test strategy would be good.
>>>>
>>>>>
>>>>> Signed-off-by: Arthur Chunqi Li <yzt356@gmail.com>
>>>>> ---
>>>>>  lib/x86/vm.h    |    4 +
>>>>>  x86/vmx_tests.c |  218 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>>  2 files changed, 222 insertions(+)
>>>>>
>>>>> diff --git a/lib/x86/vm.h b/lib/x86/vm.h
>>>>> index eff6f72..6e0ce2b 100644
>>>>> --- a/lib/x86/vm.h
>>>>> +++ b/lib/x86/vm.h
>>>>> @@ -17,9 +17,13 @@
>>>>>  #define PTE_ADDR    (0xffffffffff000ull)
>>>>>
>>>>>  #define X86_CR0_PE      0x00000001
>>>>> +#define X86_CR0_MP      0x00000002
>>>>> +#define X86_CR0_TS      0x00000008
>>>>>  #define X86_CR0_WP      0x00010000
>>>>>  #define X86_CR0_PG      0x80000000
>>>>>  #define X86_CR4_VMXE   0x00000001
>>>>> +#define X86_CR4_TSD     0x00000004
>>>>> +#define X86_CR4_DE      0x00000008
>>>>>  #define X86_CR4_PSE     0x00000010
>>>>>  #define X86_CR4_PAE     0x00000020
>>>>>  #define X86_CR4_PCIDE  0x00020000
>>>>> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
>>>>> index 61b0cef..44be3f4 100644
>>>>> --- a/x86/vmx_tests.c
>>>>> +++ b/x86/vmx_tests.c
>>>>> @@ -5,12 +5,18 @@
>>>>>
>>>>>  u64 ia32_pat;
>>>>>  u64 ia32_efer;
>>>>> +u32 stage;
>>>>>
>>>>>  static inline void vmcall()
>>>>>  {
>>>>>       asm volatile("vmcall");
>>>>>  }
>>>>>
>>>>> +static inline void set_stage(u32 s)
>>>>> +{
>>>>> +     asm volatile("mov %0, stage\n\t"::"r"(s):"memory", "cc");
>>>>> +}
>>>>> +
>>>>
>>>> Why do we need "state = s" as assembler instruction?
>>> This is due to assembler optimization. If we simply use "state = s",
>>> assembler will sometimes optimize it and state may not be set indeed.
>>
>> volatile u32 stage? And we have barrier() to avoid reordering.
> Reordering here is not a big deal here, though it is actually needed
> here. I occurred the following problem:
> 
> stage = 1;
> do something that causes vmexit;
> stage = 2;
> 
> Then the compiler will optimize "stage = 1" and "stage = 2" to one
> instruction "stage =2", since instructions between them don't use
> "stage". Can volatile solve this problem?

Yep.

Jan



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]