Re: [PATCH 4/4] kvm-unit-tests: VMX: Add test cases for instruction interception

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Aug 15, 2013 at 4:06 PM, Jan Kiszka <jan.kiszka@xxxxxx> wrote:
> On 2013-08-13 17:56, Arthur Chunqi Li wrote:
>> Add test cases for instruction interception, including three types:
>> 1. Primary Processor-Based VM-Execution Controls (HLT/INVLPG/MWAIT/
>> RDPMC/RDTSC/MONITOR/PAUSE)
>> 2. Secondary Processor-Based VM-Execution Controls (WBINVD)
>> 3. No control flag (CPUID/INVD)
>>
>> Signed-off-by: Arthur Chunqi Li <yzt356@xxxxxxxxx>
>> ---
>>  x86/vmx.c       |    3 +-
>>  x86/vmx.h       |    7 ++++
>>  x86/vmx_tests.c |  117 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 125 insertions(+), 2 deletions(-)
>>
>> diff --git a/x86/vmx.c b/x86/vmx.c
>> index ca36d35..c346070 100644
>> --- a/x86/vmx.c
>> +++ b/x86/vmx.c
>> @@ -336,8 +336,7 @@ static void init_vmx(void)
>>                       : MSR_IA32_VMX_ENTRY_CTLS);
>>       ctrl_cpu_rev[0].val = rdmsr(basic.ctrl ? MSR_IA32_VMX_TRUE_PROC
>>                       : MSR_IA32_VMX_PROCBASED_CTLS);
>> -     if (ctrl_cpu_rev[0].set & CPU_SECONDARY)
>> -             ctrl_cpu_rev[1].val = rdmsr(MSR_IA32_VMX_PROCBASED_CTLS2);
>> +     ctrl_cpu_rev[1].val = rdmsr(MSR_IA32_VMX_PROCBASED_CTLS2);
>>       if (ctrl_cpu_rev[1].set & CPU_EPT || ctrl_cpu_rev[1].set & CPU_VPID)
>>               ept_vpid.val = rdmsr(MSR_IA32_VMX_EPT_VPID_CAP);
>>
>> diff --git a/x86/vmx.h b/x86/vmx.h
>> index dba8b20..d81d25d 100644
>> --- a/x86/vmx.h
>> +++ b/x86/vmx.h
>> @@ -354,6 +354,9 @@ enum Ctrl0 {
>>       CPU_INTR_WINDOW         = 1ul << 2,
>>       CPU_HLT                 = 1ul << 7,
>>       CPU_INVLPG              = 1ul << 9,
>> +     CPU_MWAIT               = 1ul << 10,
>> +     CPU_RDPMC               = 1ul << 11,
>> +     CPU_RDTSC               = 1ul << 12,
>>       CPU_CR3_LOAD            = 1ul << 15,
>>       CPU_CR3_STORE           = 1ul << 16,
>>       CPU_TPR_SHADOW          = 1ul << 21,
>> @@ -361,6 +364,8 @@ enum Ctrl0 {
>>       CPU_IO                  = 1ul << 24,
>>       CPU_IO_BITMAP           = 1ul << 25,
>>       CPU_MSR_BITMAP          = 1ul << 28,
>> +     CPU_MONITOR             = 1ul << 29,
>> +     CPU_PAUSE               = 1ul << 30,
>>       CPU_SECONDARY           = 1ul << 31,
>>  };
>>
>> @@ -368,6 +373,8 @@ enum Ctrl1 {
>>       CPU_EPT                 = 1ul << 1,
>>       CPU_VPID                = 1ul << 5,
>>       CPU_URG                 = 1ul << 7,
>> +     CPU_WBINVD              = 1ul << 6,
>> +     CPU_RDRAND              = 1ul << 11,
>>  };
>>
>>  #define SAVE_GPR                             \
>> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
>> index ad28c4c..66187f4 100644
>> --- a/x86/vmx_tests.c
>> +++ b/x86/vmx_tests.c
>> @@ -20,6 +20,13 @@ static inline void set_stage(u32 s)
>>       asm volatile("mov %0, stage\n\t"::"r"(s):"memory", "cc");
>>  }
>>
>> +static inline u32 get_stage()
>> +{
>> +     u32 s;
>> +     asm volatile("mov stage, %0\n\t":"=r"(s)::"memory", "cc");
>> +     return s;
>> +}
>
> Tagging "stage" volatile will obsolete this special assembly.
>
>> +
>>  void basic_init()
>>  {
>>  }
>> @@ -638,6 +645,114 @@ static int iobmp_exit_handler()
>>       return VMX_TEST_VMEXIT;
>>  }
>>
>> +asm(
>> +     "insn_hlt: hlt;ret\n\t"
>> +     "insn_invlpg: invlpg 0x12345678;ret\n\t"
>> +     "insn_mwait: mwait;ret\n\t"
>> +     "insn_rdpmc: rdpmc;ret\n\t"
>> +     "insn_rdtsc: rdtsc;ret\n\t"
>> +     "insn_monitor: monitor;ret\n\t"
>> +     "insn_pause: pause;ret\n\t"
>> +     "insn_wbinvd: wbinvd;ret\n\t"
>> +     "insn_cpuid: cpuid;ret\n\t"
>> +     "insn_invd: invd;ret\n\t"
>> +);
>> +extern void insn_hlt();
>> +extern void insn_invlpg();
>> +extern void insn_mwait();
>> +extern void insn_rdpmc();
>> +extern void insn_rdtsc();
>> +extern void insn_monitor();
>> +extern void insn_pause();
>> +extern void insn_wbinvd();
>> +extern void insn_cpuid();
>> +extern void insn_invd();
>> +
>> +u32 cur_insn;
>> +
>> +struct insn_table {
>> +     const char *name;
>> +     u32 flag;
>> +     void (*insn_func)();
>> +     u32 type;
>
> What do the "type" values mean?
For intercepted instructions we have three type: controlled by Primary
Processor-Based VM-Execution Controls, controlled by Secondary
Controls and always intercepted. The testing process is different for
different types.
>
>> +     u32 reason;
>> +     ulong exit_qual;
>> +     u32 insn_info;
>
> For none of the instructions you test, EXI_INST_INFO will have valid
> content on exit. So you must not check it anyway.
Actually , "RDRAND" uses EXI_INST_INFO though it is not supported now.
Since for all intercepts these three vmcs fields are enough to
determine everything, I put it here for future use.
>
>> +};
>> +
>> +static struct insn_table insn_table[] = {
>> +     // Flags for Primary Processor-Based VM-Execution Controls
>> +     {"HLT",  CPU_HLT, insn_hlt, 0, 12, 0, 0},
>> +     {"INVLPG", CPU_INVLPG, insn_invlpg, 0, 14, 0x12345678, 0},
>> +     {"MWAIT", CPU_MWAIT, insn_mwait, 0, 36, 0, 0},
>> +     {"RDPMC", CPU_RDPMC, insn_rdpmc, 0, 15, 0, 0},
>> +     {"RDTSC", CPU_RDTSC, insn_rdtsc, 0, 16, 0, 0},
>> +     {"MONITOR", CPU_MONITOR, insn_monitor, 0, 39, 0, 0},
>> +     {"PAUSE", CPU_PAUSE, insn_pause, 0, 40, 0, 0},
>> +     // Flags for Secondary Processor-Based VM-Execution Controls
>> +     {"WBINVD", CPU_WBINVD, insn_wbinvd, 1, 54, 0, 0},
>> +     // Flags for Non-Processor-Based
>> +     {"CPUID", 0, insn_cpuid, 2, 10, 0, 0},
>> +     {"INVD", 0, insn_invd, 2, 13, 0, 0},
>> +     {NULL},
>> +};
>> +
>> +static void insn_intercept_init()
>> +{
>> +     u32 ctrl_cpu[2];
>> +
>> +     ctrl_cpu[0] = vmcs_read(CPU_EXEC_CTRL0);
>> +     ctrl_cpu[0] |= CPU_HLT | CPU_INVLPG | CPU_MWAIT | CPU_RDPMC | CPU_RDTSC |
>> +             CPU_MONITOR | CPU_PAUSE | CPU_SECONDARY;
>> +     ctrl_cpu[0] &= ctrl_cpu_rev[0].clr;
>> +     vmcs_write(CPU_EXEC_CTRL0, ctrl_cpu[0]);
>> +     ctrl_cpu[1] = vmcs_read(CPU_EXEC_CTRL1);
>> +     ctrl_cpu[1] |= CPU_WBINVD | CPU_RDRAND;
>> +     ctrl_cpu[1] &= ctrl_cpu_rev[1].clr;
>> +     vmcs_write(CPU_EXEC_CTRL1, ctrl_cpu[1]);
>> +}
>> +
>> +static void insn_intercept_main()
>> +{
>> +     cur_insn = 0;
>> +     while(insn_table[cur_insn].name != NULL) {
>> +             set_stage(cur_insn);
>> +             if ((insn_table[cur_insn].type == 0 && !(ctrl_cpu_rev[0].clr & insn_table[cur_insn].flag)) ||
>> +                     (insn_table[cur_insn].type == 1 && !(ctrl_cpu_rev[1].clr & insn_table[cur_insn].flag))) {
>> +                     printf("\tCPU_CTRL.CPU_%s is not supported.\n", insn_table[cur_insn].name);
>
>
> Coding style, specifically overlong lines.
>
>> +             } else {
>> +                     insn_table[cur_insn].insn_func();
>> +                     if (stage != cur_insn + 1)
>> +                             report(insn_table[cur_insn].name, 0);
>
> Would be good to test the inverse as well, i.e. the intercept-free
> execution.
Since this is called "insn_intercept", I think we'd better test
intercept-free execution insns in a separate test suite.

Arthur
>
>> +             }
>> +             cur_insn ++;
>> +     }
>> +}
>> +
>> +static int insn_intercept_exit_handler()
>> +{
>> +     u64 guest_rip;
>> +     u32 reason;
>> +     ulong exit_qual;
>> +     u32 insn_len;
>> +     u32 insn_info;
>> +
>> +     guest_rip = vmcs_read(GUEST_RIP);
>> +     reason = vmcs_read(EXI_REASON) & 0xff;
>> +     exit_qual = vmcs_read(EXI_QUALIFICATION);
>> +     insn_len = vmcs_read(EXI_INST_LEN);
>> +     insn_info = vmcs_read(EXI_INST_INFO);
>> +     if (cur_insn == get_stage() &&
>> +                     insn_table[cur_insn].reason == reason &&
>> +                     insn_table[cur_insn].exit_qual == exit_qual &&
>> +                     insn_table[cur_insn].insn_info == insn_info) {
>> +             report(insn_table[cur_insn].name, 1);
>> +             set_stage(stage + 1);
>> +     }
>> +     vmcs_write(GUEST_RIP, guest_rip + insn_len);
>> +     return VMX_TEST_RESUME;
>> +}
>> +
>>  /* name/init/guest_main/exit_handler/syscall_handler/guest_regs
>>     basic_* just implement some basic functions */
>>  struct vmx_test vmx_tests[] = {
>> @@ -653,5 +768,7 @@ struct vmx_test vmx_tests[] = {
>>               cr_shadowing_exit_handler, basic_syscall_handler, {0} },
>>       { "I/O bitmap", iobmp_init, iobmp_main, iobmp_exit_handler,
>>               basic_syscall_handler, {0} },
>> +     { "instruction intercept", insn_intercept_init, insn_intercept_main,
>> +             insn_intercept_exit_handler, basic_syscall_handler, {0} },
>>       { NULL, NULL, NULL, NULL, NULL, {0} },
>>  };
>>
>
> Jan
>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux