> On 1 Nov 2018, at 9:30, Vitaly Kuznetsov <vkuznets@xxxxxxxxxx> wrote: > > Liran Alon <liran.alon@xxxxxxxxxx> writes: > >>> Vitaly Kuznetsov <vkuznets@xxxxxxxxxx> writes: >> >>>> Liran Alon <liran.alon@xxxxxxxxxx> writes: >> >>>> It seems to me from what you describe that Hyper-V has a bug of using revision_id specified in IA32_VMX_BASIC_MSR >>>> instead of using one of the eVMCS supported versions reported in CPUID.4000000A. >>>> >>>> I’m also not sure I understand what you suggest as “change the other side of KVM to make it’s behaviour match Hyper-V”. >>>> Do you mean that we will change KVM usage of eVMCS such that >>>> alloc_vmcs_cpu() will always set vmcs->hdr.revision_id to >>>> vmcs_config.revision_id? >> >>> Yes. My current understanding (correct me if I'm wrong!) is that both >>> KVM-on-Hyper-V and Hyper-V-on-KVM work well with Enlightened >>> VMCS. Surprisingly, KVM-on-KVM doesn't - and this is because of the >>> issue you're trying to address by the patch. >> >> Yes you understand correctly. >> >> Well, I can change alloc_vmcs_cpu() to always set vmcs->hdr.revison_id to vmcs_config.revision_id even when eVMCS is enabled >> (which is a deliberate wrong behaviour of KVM eVMCS client code) and also not apply this patch (which remains a deliberate wrong behaviour of KVM eVMCS server code). >> All of this to overcome Hyper-V wrong behaviour :) > > Alternatively we can just drop the check in > nested_vmx_handle_enlightened_vmptrld() for now: The only supported > eVMCS version - both by Hyper-V and KVM is '1' and we explicitly > advertise that in CPUID.4000000A so even when there's a new Hyper-V > version supporting e.g. eVMCS2 it won't be used. Microsoft will also > have to fix the issue when this happens - or how else would they know > which eVMCS version is in use by the guest if they always copy > IA32_VMX_BASIC_MSR? Agree. I will submit a patch that just checks if given hv_evmcs->version_number is either KVM_EVMCS_VERSION or VMCS12_REVISION and will document properly why we do that… I think it’s better than just dropping the check. > >> >> Paolo, do you think we should insert this wrong behaviour to KVM to support running on top of buggy Hyper-V? I unfortunately believe we should. >> If so, should I insert this weird behaviour behind a kvm-intel module >> parameter? E.g. compact_evmcs_version? > > Not Paolo but I'd say this is an overkill. And, even if we decide to go > down this road, I think that the default KVM configuration should > support real world Hyper-V so the module parameter should rather be > 'correct_evmcs_versioning' :-) I agree this seems unnecessary given your suggestion. Thanks! -Liran > > -- > Vitaly
