kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Vitaly Kuznetsov <vkuznets@redhat.com>
To: Liran Alon <liran.alon@oracle.com>
Cc: kvm list <kvm@vger.kernel.org>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Sean Christopherson <sean.j.christopherson@intel.com>,
	Jim Mattson <jmattson@google.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Roman Kagan <rkagan@virtuozzo.com>
Subject: Re: [PATCH RFC 2/3] x86/kvm/hyper-v: move VMX controls sanitization out of nested_enable_evmcs()
Date: Mon, 03 Feb 2020 16:11:06 +0100	[thread overview]
Message-ID: <87h807ogdx.fsf@vitty.brq.redhat.com> (raw)
In-Reply-To: <87a76niypi.fsf@vitty.brq.redhat.com>

Vitaly Kuznetsov <vkuznets@redhat.com> writes:

> Liran Alon <liran.alon@oracle.com> writes:
>
>>> On 15 Jan 2020, at 19:10, Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
>>> 
>>> With fine grained VMX feature enablement QEMU>=4.2 tries to do KVM_SET_MSRS
>>> with default (matching CPU model) values and in case eVMCS is also enabled,
>>> fails.
>>> 
>>> It would be possible to drop VMX feature filtering completely and make
>>> this a guest's responsibility: if it decides to use eVMCS it should know
>>> which fields are available and which are not. Hyper-V mostly complies to
>>> this, however, there is at least one problematic control:
>>> SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES
>>> which Hyper-V enables. As there is no 'apic_addr_field' in eVMCS, we
>>> fail to handle this properly in KVM. It is unclear how this is supposed
>>> to work, genuine Hyper-V doesn't expose the control so it is possible that
>>> this is just a bug (in Hyper-V).
>>
>> Have you tried contacted someone at Hyper-V team about this?
>>
>
> Yes, I have.

I heard back from MS, they admited the bug and suggested us ... to hide
the control from L1 when eVMCS is enabled. No surprises here. They also
said the bug is unlikely to get fixed in the existing Hyper-V versions
(2016 and 2019) so it seems we're stuck with the hack for awhile :-(

...

>>> 
>>> #endif /* __KVM_X86_VMX_EVMCS_H */
>>> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
>>> index e3394c839dea..8eb74618b8d8 100644
>>> --- a/arch/x86/kvm/vmx/vmx.c
>>> +++ b/arch/x86/kvm/vmx/vmx.c
>>> @@ -1849,8 +1849,14 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>>> 	case MSR_IA32_VMX_BASIC ... MSR_IA32_VMX_VMFUNC:
>>> 		if (!nested_vmx_allowed(vcpu))
>>> 			return 1;
>>> -		return vmx_get_vmx_msr(&vmx->nested.msrs, msr_info->index,
>>> -				       &msr_info->data);
>>> +		if (vmx_get_vmx_msr(&vmx->nested.msrs, msr_info->index,
>>> +				    &msr_info->data))
>>> +			return 1;
>>> +		if (!msr_info->host_initiated &&
>>> +		    vmx->nested.enlightened_vmcs_enabled)
>>> +			nested_evmcs_filter_control_msr(msr_info->index,
>>> +							&msr_info->data);
>>> +		break;
>>
>> Nit: It seems more elegant to me to put the call to nested_evmcs_filter_control_msr() inside vmx_get_vmx_msr().
>>
>
> Sure, will move it there (in case we actually decide to merge this)
>

Thinking more about it, we can't check host_initiated in
vmx_get_vmx_msr() as it's not passed there, this can probably get
changed but I don't see a big difference so I'll probably keep the hack
where it is now.

-- 
Vitaly


  reply	other threads:[~2020-02-03 15:11 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-15 17:10 [PATCH RFC 0/3] x86/kvm/hyper-v: fix enlightened VMCS & QEMU4.2 Vitaly Kuznetsov
2020-01-15 17:10 ` [PATCH RFC 1/3] x86/kvm/hyper-v: remove stale evmcs_already_enabled check from nested_enable_evmcs() Vitaly Kuznetsov
2020-01-15 22:50   ` Liran Alon
2020-01-15 17:10 ` [PATCH RFC 2/3] x86/kvm/hyper-v: move VMX controls sanitization out of nested_enable_evmcs() Vitaly Kuznetsov
2020-01-15 22:49   ` Liran Alon
2020-01-16  8:37     ` Vitaly Kuznetsov
2020-02-03 15:11       ` Vitaly Kuznetsov [this message]
2020-01-15 23:27   ` Sean Christopherson
2020-01-15 23:30     ` Liran Alon
2020-01-16  8:51       ` Vitaly Kuznetsov
2020-01-16 16:19         ` Sean Christopherson
2020-01-16 16:57           ` Vitaly Kuznetsov
2020-01-17  6:31             ` Sean Christopherson
2020-01-18 21:42           ` Paolo Bonzini
2020-01-19  8:54   ` Paolo Bonzini
2020-01-22  5:47     ` Sean Christopherson
2020-01-22  9:37       ` Vitaly Kuznetsov
2020-01-22 14:33       ` Paolo Bonzini
2020-01-22 15:08         ` Vitaly Kuznetsov
2020-01-22 15:51           ` Sean Christopherson
2020-01-22 16:29             ` Vitaly Kuznetsov
2020-01-22 16:40               ` Paolo Bonzini
2020-01-23  9:15                 ` Vitaly Kuznetsov
2020-01-23 19:09                   ` Vitaly Kuznetsov
2020-01-24 17:25                     ` Sean Christopherson
2020-01-27 15:38                       ` Vitaly Kuznetsov
2020-01-27 17:53                         ` Paolo Bonzini
2020-01-27 21:52                           ` Vitaly Kuznetsov
2020-01-27 18:17                         ` Sean Christopherson
2020-01-15 17:10 ` [PATCH RFC 3/3] x86/kvm/hyper-v: don't allow to turn on unsupported VMX controls for nested guests Vitaly Kuznetsov
2020-01-15 22:59   ` Liran Alon
2020-01-16  8:55     ` Vitaly Kuznetsov
2020-01-16 16:21       ` Sean Christopherson
2020-01-19  8:57         ` Paolo Bonzini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87h807ogdx.fsf@vitty.brq.redhat.com \
    --to=vkuznets@redhat.com \
    --cc=jmattson@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=liran.alon@oracle.com \
    --cc=pbonzini@redhat.com \
    --cc=rkagan@virtuozzo.com \
    --cc=sean.j.christopherson@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).