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