Re: [PATCH 2/2] KVM: nVMX: invvpid handling improvements

[Ladi Prosek <lprosek@redhat.com>]

On Tue, Oct 25, 2016 at 3:40 PM, Radim Krčmář <rkrcmar@redhat.com> wrote:
> 2016-10-25 10:08+0200, Ladi Prosek:
>> On Fri, Oct 21, 2016 at 10:02 PM, Radim Krčmář <rkrcmar@redhat.com> wrote:
>>> 2016-10-19 01:45+0300, Jan Dakinevich:
>>>>  - Expose all invalidation types to the L1
>>>>
>>>>  - Reject invvpid instruction, if L1 passed zero vpid value to single
>>>>    context invalidations
>>>>
>>>> Signed-off-by: Jan Dakinevich <jan.dakinevich@gmail.com>
>>>> ---
>>>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>>>> @@ -132,6 +132,11 @@
>>>>
>>>>  #define VMX_MISC_EMULATED_PREEMPTION_TIMER_RATE 5
>>>>
>>>> +#define VMX_VPID_EXTENT_ALL_MASK (VMX_VPID_EXTENT_INDIVIDUAL_ADDR_BIT |      \
>>>
>>> SUPPORTED instead of ALL would be a better name.
>>>
>>>> +     VMX_VPID_EXTENT_SINGLE_CONTEXT_BIT |                            \
>>>> +     VMX_VPID_EXTENT_GLOBAL_CONTEXT_BIT |                            \
>>>> +     VMX_VPID_EXTENT_SINGLE_CONTEXT_RETAINING_GLOBALS_BIT)
>>>> +
>>>>  /*
>>>>   * These 2 parameters are used to config the controls for Pause-Loop Exiting:
>>>>   * ple_gap:    upper bound on the amount of time between two successive
>>>> @@ -2838,8 +2843,7 @@ static void nested_vmx_setup_ctls_msrs(struct vcpu_vmx *vmx)
>>>>        */
>>>>       if (enable_vpid)
>>>>               vmx->nested.nested_vmx_vpid_caps = VMX_VPID_INVVPID_BIT |
>>>> -                             VMX_VPID_EXTENT_SINGLE_CONTEXT_BIT |
>>>> -                             VMX_VPID_EXTENT_GLOBAL_CONTEXT_BIT;
>>>> +                     VMX_VPID_EXTENT_ALL_MASK;
>>>
>>> I'd still support only type 2, because it is the only one we implement,
>>> and type 1, because of buggy KVMs.
>>>
>>> Are there some OSes that can't use single or all context invalidation,
>>> so supporting more might benefit something?
>>
>> Windows Server 2016 with Hyper-V enabled requires all four
>> invalidation types. The log message is not super clear, just
>> s/allowed/required/ and s/required/available/
>
> Yep, I would have understood it the other way around ...
>
>> "
>> Hypervisor launch failed;
>> Processor does not support the minimum features required to run the hypervisor
>> (MSR index 0x48C, allowed bits 0xF0106104040, required bits 0x60106114041).
>> "
>>
>> I have verified that adding VMX_VPID_EXTENT_INDIVIDUAL_ADDR and
>> VMX_VPID_EXTENT_SINGLE_CONTEXT_RETAINING_GLOBALS solves this,
>
> Great info.  Jan, I take back the conservativeness in review -- we'll
> want to support the currently existing modes.  (Invalidating unnecessary
> entries is sad, but not that bad ... nested is already slow.)
>
>>                                                               although
>> it's not the only issue preventing Hyper-V from running on KVM at the
>> moment.
>
> You mentioned another "allowed" bit that KVM lacks, which one was it?

MSR 48B 'Secondary Processor-Based VM-Execution Controls'
bit 2 'Descriptor-table exiting'

> Thanks.