On Fri, Oct 21, 2016 at 10:02 PM, Radim Krčmář <rkrcmar@xxxxxxxxxx> 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@xxxxxxxxx>
>> ---
>> 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/
"
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, although
it's not the only issue preventing Hyper-V from running on KVM at the
moment.
I'll be happy to test the next version of this patch.
Thanks!
Ladi
>> else
>> vmx->nested.nested_vmx_vpid_caps = 0;
>> @@ -7720,7 +7724,7 @@ static int handle_invvpid(struct kvm_vcpu *vcpu)
>> vmx_instruction_info = vmcs_read32(VMX_INSTRUCTION_INFO);
>> type = kvm_register_readl(vcpu, (vmx_instruction_info >> 28) & 0xf);
>>
>> - types = (vmx->nested.nested_vmx_vpid_caps >> 8) & 0x7;
>> + types = (vmx->nested.nested_vmx_vpid_caps & VMX_VPID_EXTENT_ALL_MASK) >> 8;
>>
>> if (!(types & (1UL << type))) {
>> nested_vmx_failValid(vcpu,
>> @@ -7742,17 +7746,24 @@ static int handle_invvpid(struct kvm_vcpu *vcpu)
>> }
>>
>> switch (type) {
>> + case VMX_VPID_EXTENT_INDIVIDUAL_ADDR:
>> case VMX_VPID_EXTENT_SINGLE_CONTEXT:
>> + case VMX_VPID_EXTENT_SINGLE_CONTEXT_RETAINING_GLOBALS:
>> + if (!vpid) {
>> + nested_vmx_failValid(vcpu,
>> + VMXERR_INVALID_OPERAND_TO_INVEPT_INVVPID);
>> + skip_emulated_instruction(vcpu);
>> + return 1;
>
> (Just break and share the code.)
>
>> + }
>> + /* fall through */
>> + case VMX_VPID_EXTENT_GLOBAL_CONTEXT:
>> /*
>> - * Old versions of KVM use the single-context version so we
>> - * have to support it; just treat it the same as all-context.
>> + * Treat any invvpid type as all-context invalidation
>> */
>> - case VMX_VPID_EXTENT_GLOBAL_CONTEXT:
>> __vmx_flush_tlb(vcpu, to_vmx(vcpu)->nested.vpid02);
>> nested_vmx_succeed(vcpu);
>> break;
>> default:
>> - /* Trap individual address invalidation invvpid calls */
>> BUG_ON(1);
>
> Yeah, this was wrong when this type was not actually supported.
> BUG_ON() is not a good thing to do in any case, please change it into
> WARN_ON_ONCE(), because we should never get into the default branch.
>
> Thanks.
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
