| Message ID | 1378308395-3209-1-git-send-email-yzt356@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Jan, Gleb and Paolo, It suddenly occurred to me that, if guest's PIN_PREEMPT disabled while EXI_SAVE_PREEMPT_VALUE enabled, what will happen? The preempt value in vmcs will not be affected, yes? This cases fails to test in this patch. Arthur On Wed, Sep 4, 2013 at 11:26 PM, Arthur Chunqi Li <yzt356@gmail.com> wrote: > Test cases for preemption timer in nested VMX. Two aspects are tested: > 1. Save preemption timer on VMEXIT if relevant bit set in EXIT_CONTROL > 2. Test a relevant bug of KVM. The bug will not save preemption timer > value if exit L2->L0 for some reason and enter L0->L2. Thus preemption > timer will never trigger if the value is large enough. > > Signed-off-by: Arthur Chunqi Li <yzt356@gmail.com> > --- > x86/vmx.h | 3 ++ > x86/vmx_tests.c | 117 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 120 insertions(+) > > diff --git a/x86/vmx.h b/x86/vmx.h > index 28595d8..ebc8cfd 100644 > --- a/x86/vmx.h > +++ b/x86/vmx.h > @@ -210,6 +210,7 @@ enum Encoding { > GUEST_ACTV_STATE = 0x4826ul, > GUEST_SMBASE = 0x4828ul, > GUEST_SYSENTER_CS = 0x482aul, > + PREEMPT_TIMER_VALUE = 0x482eul, > > /* 32-Bit Host State Fields */ > HOST_SYSENTER_CS = 0x4c00ul, > @@ -331,6 +332,7 @@ enum Ctrl_exi { > EXI_LOAD_PERF = 1UL << 12, > EXI_INTA = 1UL << 15, > EXI_LOAD_EFER = 1UL << 21, > + EXI_SAVE_PREEMPT = 1UL << 22, > }; > > enum Ctrl_ent { > @@ -342,6 +344,7 @@ enum Ctrl_pin { > PIN_EXTINT = 1ul << 0, > PIN_NMI = 1ul << 3, > PIN_VIRT_NMI = 1ul << 5, > + PIN_PREEMPT = 1ul << 6, > }; > > enum Ctrl0 { > diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c > index c1b39f4..d358148 100644 > --- a/x86/vmx_tests.c > +++ b/x86/vmx_tests.c > @@ -1,4 +1,30 @@ > #include "vmx.h" > +#include "msr.h" > +#include "processor.h" > + > +volatile u32 stage; > + > +static inline void vmcall() > +{ > + asm volatile("vmcall"); > +} > + > +static inline void set_stage(u32 s) > +{ > + barrier(); > + stage = s; > + barrier(); > +} > + > +static inline u32 get_stage() > +{ > + u32 s; > + > + barrier(); > + s = stage; > + barrier(); > + return s; > +} > > void basic_init() > { > @@ -76,6 +102,95 @@ int vmenter_exit_handler() > return VMX_TEST_VMEXIT; > } > > +u32 preempt_scale; > +volatile unsigned long long tsc_val; > +volatile u32 preempt_val; > + > +void preemption_timer_init() > +{ > + u32 ctrl_pin; > + > + ctrl_pin = vmcs_read(PIN_CONTROLS) | PIN_PREEMPT; > + ctrl_pin &= ctrl_pin_rev.clr; > + vmcs_write(PIN_CONTROLS, ctrl_pin); > + preempt_val = 10000000; > + vmcs_write(PREEMPT_TIMER_VALUE, preempt_val); > + preempt_scale = rdmsr(MSR_IA32_VMX_MISC) & 0x1F; > +} > + > +void preemption_timer_main() > +{ > + tsc_val = rdtsc(); > + if (!(ctrl_pin_rev.clr & PIN_PREEMPT)) { > + printf("\tPreemption timer is not supported\n"); > + return; > + } > + if (!(ctrl_exit_rev.clr & EXI_SAVE_PREEMPT)) > + printf("\tSave preemption value is not supported\n"); > + else { > + set_stage(0); > + vmcall(); > + if (get_stage() == 1) > + vmcall(); > + } > + while (1) { > + if (((rdtsc() - tsc_val) >> preempt_scale) > + > 10 * preempt_val) { > + report("Preemption timer", 0); > + break; > + } > + } > +} > + > +int preemption_timer_exit_handler() > +{ > + u64 guest_rip; > + ulong reason; > + u32 insn_len; > + u32 ctrl_exit; > + > + guest_rip = vmcs_read(GUEST_RIP); > + reason = vmcs_read(EXI_REASON) & 0xff; > + insn_len = vmcs_read(EXI_INST_LEN); > + switch (reason) { > + case VMX_PREEMPT: > + if (((rdtsc() - tsc_val) >> preempt_scale) < preempt_val) > + report("Preemption timer", 0); > + else > + report("Preemption timer", 1); > + return VMX_TEST_VMEXIT; > + case VMX_VMCALL: > + switch (get_stage()) { > + case 0: > + if (vmcs_read(PREEMPT_TIMER_VALUE) != preempt_val) > + report("Save preemption value", 0); > + else { > + set_stage(get_stage() + 1); > + ctrl_exit = (vmcs_read(EXI_CONTROLS) | > + EXI_SAVE_PREEMPT) & ctrl_exit_rev.clr; > + vmcs_write(EXI_CONTROLS, ctrl_exit); > + } > + break; > + case 1: > + if (vmcs_read(PREEMPT_TIMER_VALUE) >= preempt_val) > + report("Save preemption value", 0); > + else > + report("Save preemption value", 1); > + break; > + default: > + printf("Invalid stage.\n"); > + print_vmexit_info(); > + return VMX_TEST_VMEXIT; > + } > + vmcs_write(GUEST_RIP, guest_rip + insn_len); > + return VMX_TEST_RESUME; > + default: > + printf("Unknown exit reason, %d\n", reason); > + print_vmexit_info(); > + } > + return VMX_TEST_VMEXIT; > +} > + > /* name/init/guest_main/exit_handler/syscall_handler/guest_regs > basic_* just implement some basic functions */ > struct vmx_test vmx_tests[] = { > @@ -83,5 +198,7 @@ struct vmx_test vmx_tests[] = { > basic_syscall_handler, {0} }, > { "vmenter", basic_init, vmenter_main, vmenter_exit_handler, > basic_syscall_handler, {0} }, > + { "preemption timer", preemption_timer_init, preemption_timer_main, > + preemption_timer_exit_handler, basic_syscall_handler, {0} }, > { NULL, NULL, NULL, NULL, NULL, {0} }, > }; > -- > 1.7.9.5 >
Il 04/09/2013 17:26, Arthur Chunqi Li ha scritto: > Test cases for preemption timer in nested VMX. Two aspects are tested: > 1. Save preemption timer on VMEXIT if relevant bit set in EXIT_CONTROL > 2. Test a relevant bug of KVM. The bug will not save preemption timer > value if exit L2->L0 for some reason and enter L0->L2. Thus preemption > timer will never trigger if the value is large enough. > > Signed-off-by: Arthur Chunqi Li <yzt356@gmail.com> > --- > x86/vmx.h | 3 ++ > x86/vmx_tests.c | 117 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 120 insertions(+) Please rebase your other four patches on top of this one, since this one is good to go! Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Paolo > +void preemption_timer_init() > +{ > + u32 ctrl_pin; > + > + ctrl_pin = vmcs_read(PIN_CONTROLS) | PIN_PREEMPT; > + ctrl_pin &= ctrl_pin_rev.clr; > + vmcs_write(PIN_CONTROLS, ctrl_pin); > + preempt_val = 10000000; > + vmcs_write(PREEMPT_TIMER_VALUE, preempt_val); > + preempt_scale = rdmsr(MSR_IA32_VMX_MISC) & 0x1F; > +} > + > +void preemption_timer_main() > +{ > + tsc_val = rdtsc(); > + if (!(ctrl_pin_rev.clr & PIN_PREEMPT)) { > + printf("\tPreemption timer is not supported\n"); > + return; > + } > + if (!(ctrl_exit_rev.clr & EXI_SAVE_PREEMPT)) > + printf("\tSave preemption value is not supported\n"); > + else { > + set_stage(0); > + vmcall(); > + if (get_stage() == 1) > + vmcall(); > + } > + while (1) { > + if (((rdtsc() - tsc_val) >> preempt_scale) > + > 10 * preempt_val) { > + report("Preemption timer", 0); > + break; > + } > + } > +} > + > +int preemption_timer_exit_handler() > +{ > + u64 guest_rip; > + ulong reason; > + u32 insn_len; > + u32 ctrl_exit; > + > + guest_rip = vmcs_read(GUEST_RIP); > + reason = vmcs_read(EXI_REASON) & 0xff; > + insn_len = vmcs_read(EXI_INST_LEN); > + switch (reason) { > + case VMX_PREEMPT: > + if (((rdtsc() - tsc_val) >> preempt_scale) < preempt_val) > + report("Preemption timer", 0); > + else > + report("Preemption timer", 1); > + return VMX_TEST_VMEXIT; > + case VMX_VMCALL: > + switch (get_stage()) { > + case 0: > + if (vmcs_read(PREEMPT_TIMER_VALUE) != preempt_val) > + report("Save preemption value", 0); > + else { > + set_stage(get_stage() + 1); > + ctrl_exit = (vmcs_read(EXI_CONTROLS) | > + EXI_SAVE_PREEMPT) & ctrl_exit_rev.clr; > + vmcs_write(EXI_CONTROLS, ctrl_exit); > + } > + break; > + case 1: > + if (vmcs_read(PREEMPT_TIMER_VALUE) >= preempt_val) > + report("Save preemption value", 0); > + else > + report("Save preemption value", 1); > + break; > + default: > + printf("Invalid stage.\n"); > + print_vmexit_info(); > + return VMX_TEST_VMEXIT; > + } > + vmcs_write(GUEST_RIP, guest_rip + insn_len); > + return VMX_TEST_RESUME; > + default: > + printf("Unknown exit reason, %d\n", reason); > + print_vmexit_info(); > + } > + return VMX_TEST_VMEXIT; > +} > + > /* name/init/guest_main/exit_handler/syscall_handler/guest_regs > basic_* just implement some basic functions */ > struct vmx_test vmx_tests[] = { > @@ -83,5 +198,7 @@ struct vmx_test vmx_tests[] = { > basic_syscall_handler, {0} }, > { "vmenter", basic_init, vmenter_main, vmenter_exit_handler, > basic_syscall_handler, {0} }, > + { "preemption timer", preemption_timer_init, preemption_timer_main, > + preemption_timer_exit_handler, basic_syscall_handler, {0} }, > { NULL, NULL, NULL, NULL, NULL, {0} }, > }; > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Il 05/09/2013 11:22, Arthur Chunqi Li ha scritto: > Hi Jan, Gleb and Paolo, > > It suddenly occurred to me that, if guest's PIN_PREEMPT disabled while > EXI_SAVE_PREEMPT_VALUE enabled, what will happen? The preempt value in > vmcs will not be affected, yes? Indeed. The VMX preemption timer will not count down, so it will remain set to the same value. Saving it on exit will actually do nothing. > This cases fails to test in this patch. You can add it as a follow up. Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Sep 9, 2013 at 8:51 PM, Paolo Bonzini <pbonzini@redhat.com> wrote: > Il 05/09/2013 11:22, Arthur Chunqi Li ha scritto: >> Hi Jan, Gleb and Paolo, >> >> It suddenly occurred to me that, if guest's PIN_PREEMPT disabled while >> EXI_SAVE_PREEMPT_VALUE enabled, what will happen? The preempt value in >> vmcs will not be affected, yes? > > Indeed. The VMX preemption timer will not count down, so it will remain > set to the same value. Saving it on exit will actually do nothing. > >> This cases fails to test in this patch. > > You can add it as a follow up. Yep. Because this patch was committed several weeks ago, I have committed a second version, you can just review that patch. Arthur > > Paolo >
diff --git a/x86/vmx.h b/x86/vmx.h index 28595d8..ebc8cfd 100644 --- a/x86/vmx.h +++ b/x86/vmx.h @@ -210,6 +210,7 @@ enum Encoding { GUEST_ACTV_STATE = 0x4826ul, GUEST_SMBASE = 0x4828ul, GUEST_SYSENTER_CS = 0x482aul, + PREEMPT_TIMER_VALUE = 0x482eul, /* 32-Bit Host State Fields */ HOST_SYSENTER_CS = 0x4c00ul, @@ -331,6 +332,7 @@ enum Ctrl_exi { EXI_LOAD_PERF = 1UL << 12, EXI_INTA = 1UL << 15, EXI_LOAD_EFER = 1UL << 21, + EXI_SAVE_PREEMPT = 1UL << 22, }; enum Ctrl_ent { @@ -342,6 +344,7 @@ enum Ctrl_pin { PIN_EXTINT = 1ul << 0, PIN_NMI = 1ul << 3, PIN_VIRT_NMI = 1ul << 5, + PIN_PREEMPT = 1ul << 6, }; enum Ctrl0 { diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c index c1b39f4..d358148 100644 --- a/x86/vmx_tests.c +++ b/x86/vmx_tests.c @@ -1,4 +1,30 @@ #include "vmx.h" +#include "msr.h" +#include "processor.h" + +volatile u32 stage; + +static inline void vmcall() +{ + asm volatile("vmcall"); +} + +static inline void set_stage(u32 s) +{ + barrier(); + stage = s; + barrier(); +} + +static inline u32 get_stage() +{ + u32 s; + + barrier(); + s = stage; + barrier(); + return s; +} void basic_init() { @@ -76,6 +102,95 @@ int vmenter_exit_handler() return VMX_TEST_VMEXIT; } +u32 preempt_scale; +volatile unsigned long long tsc_val; +volatile u32 preempt_val; + +void preemption_timer_init() +{ + u32 ctrl_pin; + + ctrl_pin = vmcs_read(PIN_CONTROLS) | PIN_PREEMPT; + ctrl_pin &= ctrl_pin_rev.clr; + vmcs_write(PIN_CONTROLS, ctrl_pin); + preempt_val = 10000000; + vmcs_write(PREEMPT_TIMER_VALUE, preempt_val); + preempt_scale = rdmsr(MSR_IA32_VMX_MISC) & 0x1F; +} + +void preemption_timer_main() +{ + tsc_val = rdtsc(); + if (!(ctrl_pin_rev.clr & PIN_PREEMPT)) { + printf("\tPreemption timer is not supported\n"); + return; + } + if (!(ctrl_exit_rev.clr & EXI_SAVE_PREEMPT)) + printf("\tSave preemption value is not supported\n"); + else { + set_stage(0); + vmcall(); + if (get_stage() == 1) + vmcall(); + } + while (1) { + if (((rdtsc() - tsc_val) >> preempt_scale) + > 10 * preempt_val) { + report("Preemption timer", 0); + break; + } + } +} + +int preemption_timer_exit_handler() +{ + u64 guest_rip; + ulong reason; + u32 insn_len; + u32 ctrl_exit; + + guest_rip = vmcs_read(GUEST_RIP); + reason = vmcs_read(EXI_REASON) & 0xff; + insn_len = vmcs_read(EXI_INST_LEN); + switch (reason) { + case VMX_PREEMPT: + if (((rdtsc() - tsc_val) >> preempt_scale) < preempt_val) + report("Preemption timer", 0); + else + report("Preemption timer", 1); + return VMX_TEST_VMEXIT; + case VMX_VMCALL: + switch (get_stage()) { + case 0: + if (vmcs_read(PREEMPT_TIMER_VALUE) != preempt_val) + report("Save preemption value", 0); + else { + set_stage(get_stage() + 1); + ctrl_exit = (vmcs_read(EXI_CONTROLS) | + EXI_SAVE_PREEMPT) & ctrl_exit_rev.clr; + vmcs_write(EXI_CONTROLS, ctrl_exit); + } + break; + case 1: + if (vmcs_read(PREEMPT_TIMER_VALUE) >= preempt_val) + report("Save preemption value", 0); + else + report("Save preemption value", 1); + break; + default: + printf("Invalid stage.\n"); + print_vmexit_info(); + return VMX_TEST_VMEXIT; + } + vmcs_write(GUEST_RIP, guest_rip + insn_len); + return VMX_TEST_RESUME; + default: + printf("Unknown exit reason, %d\n", reason); + print_vmexit_info(); + } + return VMX_TEST_VMEXIT; +} + /* name/init/guest_main/exit_handler/syscall_handler/guest_regs basic_* just implement some basic functions */ struct vmx_test vmx_tests[] = { @@ -83,5 +198,7 @@ struct vmx_test vmx_tests[] = { basic_syscall_handler, {0} }, { "vmenter", basic_init, vmenter_main, vmenter_exit_handler, basic_syscall_handler, {0} }, + { "preemption timer", preemption_timer_init, preemption_timer_main, + preemption_timer_exit_handler, basic_syscall_handler, {0} }, { NULL, NULL, NULL, NULL, NULL, {0} }, };
Test cases for preemption timer in nested VMX. Two aspects are tested: 1. Save preemption timer on VMEXIT if relevant bit set in EXIT_CONTROL 2. Test a relevant bug of KVM. The bug will not save preemption timer value if exit L2->L0 for some reason and enter L0->L2. Thus preemption timer will never trigger if the value is large enough. Signed-off-by: Arthur Chunqi Li <yzt356@gmail.com> --- x86/vmx.h | 3 ++ x86/vmx_tests.c | 117 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 120 insertions(+)