On 2013-08-24 20:44, root wrote: > This patch contains the following two changes: > 1. Fix the bug in nested preemption timer support. If vmexit L2->L0 > with some reasons not emulated by L1, preemption timer value should > be save in such exits. > 2. Add support of "Save VMX-preemption timer value" VM-Exit controls > to nVMX. > > With this patch, nested VMX preemption timer features are fully > supported. > > Signed-off-by: Arthur Chunqi Li <[email protected]> > --- > arch/x86/kvm/vmx.c | 30 +++++++++++++++++++++++++----- > 1 file changed, 25 insertions(+), 5 deletions(-) > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index 57b4e12..9579409 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -2204,7 +2204,8 @@ static __init void nested_vmx_setup_ctls_msrs(void) > #ifdef CONFIG_X86_64 > VM_EXIT_HOST_ADDR_SPACE_SIZE | > #endif > - VM_EXIT_LOAD_IA32_PAT | VM_EXIT_SAVE_IA32_PAT; > + VM_EXIT_LOAD_IA32_PAT | VM_EXIT_SAVE_IA32_PAT | > + VM_EXIT_SAVE_VMX_PREEMPTION_TIMER; > nested_vmx_exit_ctls_high |= (VM_EXIT_ALWAYSON_WITHOUT_TRUE_MSR | > VM_EXIT_LOAD_IA32_EFER);
In the absence of VM_EXIT_SAVE_VMX_PREEMPTION_TIMER, you need to hide
PIN_BASED_VMX_PREEMPTION_TIMER from the guest as we cannot emulate its
behavior properly in that case.
>
> @@ -7578,9 +7579,14 @@ static void prepare_vmcs02(struct kvm_vcpu *vcpu,
> struct vmcs12 *vmcs12)
> (vmcs_config.pin_based_exec_ctrl |
> vmcs12->pin_based_vm_exec_control));
>
> - if (vmcs12->pin_based_vm_exec_control & PIN_BASED_VMX_PREEMPTION_TIMER)
> - vmcs_write32(VMX_PREEMPTION_TIMER_VALUE,
> - vmcs12->vmx_preemption_timer_value);
> + if (vmcs12->pin_based_vm_exec_control & PIN_BASED_VMX_PREEMPTION_TIMER)
> {
> + if (vmcs12->vm_exit_controls &
> VM_EXIT_SAVE_VMX_PREEMPTION_TIMER)
> + vmcs12->vmx_preemption_timer_value =
> + vmcs_read32(VMX_PREEMPTION_TIMER_VALUE);
> + else
> + vmcs_write32(VMX_PREEMPTION_TIMER_VALUE,
> + vmcs12->vmx_preemption_timer_value);
> + }
This is not correct. We still need to set the vmcs to
vmx_preemption_timer_value. The difference is that, on exit from L2,
vmx_preemption_timer_value has to be updated according to the saved
hardware state. The corresponding code is missing in your patch so far.
>
> /*
> * Whether page-faults are trapped is determined by a combination of
> @@ -7690,7 +7696,11 @@ static void prepare_vmcs02(struct kvm_vcpu *vcpu,
> struct vmcs12 *vmcs12)
> * we should use its exit controls. Note that VM_EXIT_LOAD_IA32_EFER
> * bits are further modified by vmx_set_efer() below.
> */
> - vmcs_write32(VM_EXIT_CONTROLS, vmcs_config.vmexit_ctrl);
> + if (vmcs12->pin_based_vm_exec_control & PIN_BASED_VMX_PREEMPTION_TIMER)
> + vmcs_write32(VM_EXIT_CONTROLS, vmcs_config.vmexit_ctrl |
> + VM_EXIT_SAVE_VMX_PREEMPTION_TIMER);
> + else
> + vmcs_write32(VM_EXIT_CONTROLS, vmcs_config.vmexit_ctrl);
Let's prepare the value for VM_EXIT_CONTROLS in a local variable first,
then write it to the vmcs.
>
> /* vmcs12's VM_ENTRY_LOAD_IA32_EFER and VM_ENTRY_IA32E_MODE are
> * emulated by vmx_set_efer(), below.
> @@ -7912,6 +7922,16 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool
> launch)
> }
>
> /*
> + * If L2 support PIN_BASED_VMX_PREEMPTION_TIMER, L0 must support
> + * VM_EXIT_SAVE_VMX_PREEMPTION_TIMER.
> + */
> + if ((vmcs12->pin_based_vm_exec_control &
> PIN_BASED_VMX_PREEMPTION_TIMER) &&
> + !(nested_vmx_exit_ctls_high &
> VM_EXIT_SAVE_VMX_PREEMPTION_TIMER)) {
> + nested_vmx_failValid(vcpu, VMXERR_ENTRY_INVALID_CONTROL_FIELD);
> + return 1;
> + }
Nope, the guest is free to run the preemption timer without saving on
exits. It may have a valid use case for this, e.g. that it will always
reprogram it on entry.
> +
> + /*
> * We're finally done with prerequisite checking, and can start with
> * the nested entry.
> */
>
Jan
signature.asc
Description: OpenPGP digital signature
