Re: [PATCH v4] KVM: nVMX: Fully support of nested VMX preemption timer

[Jan Kiszka <jan.kiszka@web.de>]

[-- Attachment #1: Type: text/plain, Size: 3814 bytes --]

On 2013-09-16 07:42, Arthur Chunqi Li wrote:
> On Sat, Sep 14, 2013 at 1:15 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Il 06/09/2013 04:04, Arthur Chunqi Li ha scritto:
>>> 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 <yzt356@gmail.com>
>>> ---
>>> ChangeLog to v3:
>>> Move nested_adjust_preemption_timer to the latest place just before vmenter.
>>> Some minor changes.
>>>
>>>  arch/x86/include/uapi/asm/msr-index.h |    1 +
>>>  arch/x86/kvm/vmx.c                    |   49 +++++++++++++++++++++++++++++++--
>>>  2 files changed, 48 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/x86/include/uapi/asm/msr-index.h b/arch/x86/include/uapi/asm/msr-index.h
>>> index bb04650..b93e09a 100644
>>> --- a/arch/x86/include/uapi/asm/msr-index.h
>>> +++ b/arch/x86/include/uapi/asm/msr-index.h
>>> @@ -536,6 +536,7 @@
>>>
>>>  /* MSR_IA32_VMX_MISC bits */
>>>  #define MSR_IA32_VMX_MISC_VMWRITE_SHADOW_RO_FIELDS (1ULL << 29)
>>> +#define MSR_IA32_VMX_MISC_PREEMPTION_TIMER_SCALE   0x1F
>>>  /* AMD-V MSRs */
>>>
>>>  #define MSR_VM_CR                       0xc0010114
>>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>>> index 1f1da43..f364d16 100644
>>> --- a/arch/x86/kvm/vmx.c
>>> +++ b/arch/x86/kvm/vmx.c
>>> @@ -374,6 +374,8 @@ struct nested_vmx {
>>>        */
>>>       struct page *apic_access_page;
>>>       u64 msr_ia32_feature_control;
>>> +     /* Set if vmexit is L2->L1 */
>>> +     bool nested_vmx_exit;
>>>  };
>>>
>>>  #define POSTED_INTR_ON  0
>>> @@ -2204,7 +2206,17 @@ 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;
>>> +     if (!(nested_vmx_pinbased_ctls_high &
>>> +                             PIN_BASED_VMX_PREEMPTION_TIMER) ||
>>> +                     !(nested_vmx_exit_ctls_high &
>>> +                             VM_EXIT_SAVE_VMX_PREEMPTION_TIMER)) {
>>
>> Align this under the other "!".  Also, I prefer to have one long line
>> for the whole "!(... & ...) ||" (and likewise below), but I don't know
>> if Gleb agrees
>>
>>> +             nested_vmx_exit_ctls_high &=
>>> +                     (~VM_EXIT_SAVE_VMX_PREEMPTION_TIMER);
>>
>> Please remove parentheses around ~VM_EXIT_SAVE_VMX_PREEMPTION_TIMER, and
>> likewise elsewhere in the patch.
>>
>>> +             nested_vmx_pinbased_ctls_high &=
>>> +                     (~PIN_BASED_VMX_PREEMPTION_TIMER);
>>> +     }
>>>       nested_vmx_exit_ctls_high |= (VM_EXIT_ALWAYSON_WITHOUT_TRUE_MSR |
>>>                                     VM_EXIT_LOAD_IA32_EFER);
>>>
>>> @@ -6707,6 +6719,24 @@ static void vmx_get_exit_info(struct kvm_vcpu *vcpu, u64 *info1, u64 *info2)
>>>       *info2 = vmcs_read32(VM_EXIT_INTR_INFO);
>>>  }
>>>
>>> +static void nested_adjust_preemption_timer(struct kvm_vcpu *vcpu)
>>> +{
>>> +     u64 delta_tsc_l1;
>>> +     u32 preempt_val_l1, preempt_val_l2, preempt_scale;
>>
>> Should this exit immediately if the preemption timer pin-based control
>> is disabled?
> Hi Paolo,
> How can I get pin-based control here from "struct kvm_vcpu *vcpu"?

You can find get_vmcs12(vcpu)->pin_based_vm_exec_control in existing code.

Jan



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]