Message ID | 20181101052159.32695-3-krish.sadhukhan@oracle.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [1/2] nVMX x86: Check VMX-preemption timer controls on vmentry of L2 guests | expand |
On 01/11/18 06:21, Krish Sadhukhan wrote: > According to section "Checks on VMX Controls" in Intel SDM vol 3C, the > following check needs to be enforced on vmentry of L2 guests: > > If the "activate VMX-preemption timer" VM-execution control is 0, the > the "save VMX-preemption timer value" VM-exit control must also be 0. > > Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com> > Reviewed-by: Mihai Carabas <mihai.carabas@oracle.com> > --- > x86/vmx_tests.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 50 insertions(+) > > diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c > index b105b23..7f49048 100644 > --- a/x86/vmx_tests.c > +++ b/x86/vmx_tests.c > @@ -4721,6 +4721,55 @@ static void test_pml(void) > vmcs_write(CPU_EXEC_CTRL1, secondary_saved); > } > > + /* > + * If the "activate VMX-preemption timer" VM-execution control is 0, the > + * the "save VMX-preemption timer value" VM-exit control must also be 0. > + * > + * [Intel SDM] > + */ > +static void test_vmx_preemption_timer(void) > +{ > + u32 saved_pin = vmcs_read(PIN_CONTROLS); > + u32 saved_exit = vmcs_read(EXI_CONTROLS); > + u32 pin = saved_pin; > + u32 exit = saved_exit; > + > + if (!((ctrl_exit_rev.clr & EXI_SAVE_PREEMPT) || > + (ctrl_pin_rev.clr & PIN_PREEMPT))) { > + printf("\"Save-VMX-preemption-timer\" control and/or \"Enable-VMX-preemption-timer\" control is not supported\n"); > + return; > + } > + > + pin |= PIN_PREEMPT; > + vmcs_write(PIN_CONTROLS, pin); > + exit &= ~EXI_SAVE_PREEMPT; > + vmcs_write(EXI_CONTROLS, exit); > + report_prefix_pushf("enable-VMX-preemption-timer enabled, save-VMX-preemption-timer disabled"); > + test_vmx_controls(true, false); > + report_prefix_pop(); > + > + exit |= EXI_SAVE_PREEMPT; > + vmcs_write(EXI_CONTROLS, exit); > + report_prefix_pushf("enable-VMX-preemption-timer enabled, save-VMX-preemption-timer enabled"); > + test_vmx_controls(true, false); > + report_prefix_pop(); > + > + pin &= ~PIN_PREEMPT; > + vmcs_write(PIN_CONTROLS, pin); > + report_prefix_pushf("enable-VMX-preemption-timer disabled, save-VMX-preemption-timer enabled"); > + test_vmx_controls(false, false); > + report_prefix_pop(); > + > + exit &= ~EXI_SAVE_PREEMPT; > + vmcs_write(EXI_CONTROLS, exit); > + report_prefix_pushf("enable-VMX-preemption-timer disabled, save-VMX-preemption-timer disabled"); > + test_vmx_controls(true, false); > + report_prefix_pop(); > + > + vmcs_write(PIN_CONTROLS, saved_pin); > + vmcs_write(EXI_CONTROLS, saved_exit); > +} > + > /* > * Check that the virtual CPU checks all of the VMX controls as > * documented in the Intel SDM. > @@ -4747,6 +4796,7 @@ static void vmx_controls_test(void) > test_invalid_event_injection(); > test_vpid(); > test_eptp(); > + test_vmx_preemption_timer(); > } > > static bool valid_vmcs_for_vmentry(void) > The following is needed too: diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c index fe187c5..7a8d6ce 100644 --- a/x86/vmx_tests.c +++ b/x86/vmx_tests.c @@ -183,6 +183,8 @@ static int preemption_timer_exit_handler(void) vmx_set_test_stage(4); vmcs_write(PIN_CONTROLS, vmcs_read(PIN_CONTROLS) & ~PIN_PREEMPT); + vmcs_write(EXI_CONTROLS, + vmcs_read(EXI_CONTROLS) & ~EXI_SAVE_PREEMPT); vmcs_write(GUEST_ACTV_STATE, ACTV_ACTIVE); return VMX_TEST_RESUME; case 4: Paolo
On 11/26/2018 08:32 AM, Paolo Bonzini wrote: > On 01/11/18 06:21, Krish Sadhukhan wrote: >> According to section "Checks on VMX Controls" in Intel SDM vol 3C, the >> following check needs to be enforced on vmentry of L2 guests: >> >> If the "activate VMX-preemption timer" VM-execution control is 0, the >> the "save VMX-preemption timer value" VM-exit control must also be 0. >> >> Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com> >> Reviewed-by: Mihai Carabas <mihai.carabas@oracle.com> >> --- >> x86/vmx_tests.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 50 insertions(+) >> >> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c >> index b105b23..7f49048 100644 >> --- a/x86/vmx_tests.c >> +++ b/x86/vmx_tests.c >> @@ -4721,6 +4721,55 @@ static void test_pml(void) >> vmcs_write(CPU_EXEC_CTRL1, secondary_saved); >> } >> >> + /* >> + * If the "activate VMX-preemption timer" VM-execution control is 0, the >> + * the "save VMX-preemption timer value" VM-exit control must also be 0. >> + * >> + * [Intel SDM] >> + */ >> +static void test_vmx_preemption_timer(void) >> +{ >> + u32 saved_pin = vmcs_read(PIN_CONTROLS); >> + u32 saved_exit = vmcs_read(EXI_CONTROLS); >> + u32 pin = saved_pin; >> + u32 exit = saved_exit; >> + >> + if (!((ctrl_exit_rev.clr & EXI_SAVE_PREEMPT) || >> + (ctrl_pin_rev.clr & PIN_PREEMPT))) { >> + printf("\"Save-VMX-preemption-timer\" control and/or \"Enable-VMX-preemption-timer\" control is not supported\n"); >> + return; >> + } >> + >> + pin |= PIN_PREEMPT; >> + vmcs_write(PIN_CONTROLS, pin); >> + exit &= ~EXI_SAVE_PREEMPT; >> + vmcs_write(EXI_CONTROLS, exit); >> + report_prefix_pushf("enable-VMX-preemption-timer enabled, save-VMX-preemption-timer disabled"); >> + test_vmx_controls(true, false); >> + report_prefix_pop(); >> + >> + exit |= EXI_SAVE_PREEMPT; >> + vmcs_write(EXI_CONTROLS, exit); >> + report_prefix_pushf("enable-VMX-preemption-timer enabled, save-VMX-preemption-timer enabled"); >> + test_vmx_controls(true, false); >> + report_prefix_pop(); >> + >> + pin &= ~PIN_PREEMPT; >> + vmcs_write(PIN_CONTROLS, pin); >> + report_prefix_pushf("enable-VMX-preemption-timer disabled, save-VMX-preemption-timer enabled"); >> + test_vmx_controls(false, false); >> + report_prefix_pop(); >> + >> + exit &= ~EXI_SAVE_PREEMPT; >> + vmcs_write(EXI_CONTROLS, exit); >> + report_prefix_pushf("enable-VMX-preemption-timer disabled, save-VMX-preemption-timer disabled"); >> + test_vmx_controls(true, false); >> + report_prefix_pop(); >> + >> + vmcs_write(PIN_CONTROLS, saved_pin); >> + vmcs_write(EXI_CONTROLS, saved_exit); >> +} >> + >> /* >> * Check that the virtual CPU checks all of the VMX controls as >> * documented in the Intel SDM. >> @@ -4747,6 +4796,7 @@ static void vmx_controls_test(void) >> test_invalid_event_injection(); >> test_vpid(); >> test_eptp(); >> + test_vmx_preemption_timer(); >> } >> >> static bool valid_vmcs_for_vmentry(void) >> > The following is needed too: > > diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c > index fe187c5..7a8d6ce 100644 > --- a/x86/vmx_tests.c > +++ b/x86/vmx_tests.c > @@ -183,6 +183,8 @@ static int preemption_timer_exit_handler(void) > vmx_set_test_stage(4); > vmcs_write(PIN_CONTROLS, > vmcs_read(PIN_CONTROLS) & ~PIN_PREEMPT); > + vmcs_write(EXI_CONTROLS, > + vmcs_read(EXI_CONTROLS) & ~EXI_SAVE_PREEMPT); > vmcs_write(GUEST_ACTV_STATE, ACTV_ACTIVE); > return VMX_TEST_RESUME; > case 4: > > Paolo I missed that. Thanks ! For adhering to the rule, we should also disable the control at the end of preemption_timer_exit_handler(): ... vmcs_write(PIN_CONTROLS, vmcs_read(PIN_CONTROLS) & ~PIN_PREEMPT); + vmcs_write(EXI_CONTROLS, + vmcs_read(EXI_CONTROLS) & ~EXI_SAVE_PREEMPT); return VMX_TEST_VMEXIT; Should I send out a revised patch or you will apply these changes ?
On 01/12/18 03:12, Krish Sadhukhan wrote: > For adhering to the rule, we should also disable the control at the end > of preemption_timer_exit_handler(): > > ... > vmcs_write(PIN_CONTROLS, vmcs_read(PIN_CONTROLS) & ~PIN_PREEMPT); > + vmcs_write(EXI_CONTROLS, > + vmcs_read(EXI_CONTROLS) & ~EXI_SAVE_PREEMPT); > return VMX_TEST_VMEXIT; > Or also remove the other vmcs_write, since the VMCS is not reused across tests. > Should I send out a revised patch or you will apply these changes ? I have already applied the patch. Paolo
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c index b105b23..7f49048 100644 --- a/x86/vmx_tests.c +++ b/x86/vmx_tests.c @@ -4721,6 +4721,55 @@ static void test_pml(void) vmcs_write(CPU_EXEC_CTRL1, secondary_saved); } + /* + * If the "activate VMX-preemption timer" VM-execution control is 0, the + * the "save VMX-preemption timer value" VM-exit control must also be 0. + * + * [Intel SDM] + */ +static void test_vmx_preemption_timer(void) +{ + u32 saved_pin = vmcs_read(PIN_CONTROLS); + u32 saved_exit = vmcs_read(EXI_CONTROLS); + u32 pin = saved_pin; + u32 exit = saved_exit; + + if (!((ctrl_exit_rev.clr & EXI_SAVE_PREEMPT) || + (ctrl_pin_rev.clr & PIN_PREEMPT))) { + printf("\"Save-VMX-preemption-timer\" control and/or \"Enable-VMX-preemption-timer\" control is not supported\n"); + return; + } + + pin |= PIN_PREEMPT; + vmcs_write(PIN_CONTROLS, pin); + exit &= ~EXI_SAVE_PREEMPT; + vmcs_write(EXI_CONTROLS, exit); + report_prefix_pushf("enable-VMX-preemption-timer enabled, save-VMX-preemption-timer disabled"); + test_vmx_controls(true, false); + report_prefix_pop(); + + exit |= EXI_SAVE_PREEMPT; + vmcs_write(EXI_CONTROLS, exit); + report_prefix_pushf("enable-VMX-preemption-timer enabled, save-VMX-preemption-timer enabled"); + test_vmx_controls(true, false); + report_prefix_pop(); + + pin &= ~PIN_PREEMPT; + vmcs_write(PIN_CONTROLS, pin); + report_prefix_pushf("enable-VMX-preemption-timer disabled, save-VMX-preemption-timer enabled"); + test_vmx_controls(false, false); + report_prefix_pop(); + + exit &= ~EXI_SAVE_PREEMPT; + vmcs_write(EXI_CONTROLS, exit); + report_prefix_pushf("enable-VMX-preemption-timer disabled, save-VMX-preemption-timer disabled"); + test_vmx_controls(true, false); + report_prefix_pop(); + + vmcs_write(PIN_CONTROLS, saved_pin); + vmcs_write(EXI_CONTROLS, saved_exit); +} + /* * Check that the virtual CPU checks all of the VMX controls as * documented in the Intel SDM. @@ -4747,6 +4796,7 @@ static void vmx_controls_test(void) test_invalid_event_injection(); test_vpid(); test_eptp(); + test_vmx_preemption_timer(); } static bool valid_vmcs_for_vmentry(void)