Re: [PATCH RFC 2/3] x86/kvm/hyper-v: move VMX controls sanitization out of nested_enable_evmcs()

From: Vitaly Kuznetsov <vkuznets@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: kvm@vger.kernel.org, Jim Mattson <jmattson@google.com>,
	linux-kernel@vger.kernel.org, Liran Alon <liran.alon@oracle.com>,
	Roman Kagan <rkagan@virtuozzo.com>,
	Sean Christopherson <sean.j.christopherson@intel.com>
Subject: Re: [PATCH RFC 2/3] x86/kvm/hyper-v: move VMX controls sanitization out of nested_enable_evmcs()
Date: Thu, 23 Jan 2020 20:09:03 +0100	[thread overview]
Message-ID: <87lfpyq9bk.fsf@vitty.brq.redhat.com> (raw)
In-Reply-To: <87zheer0si.fsf@vitty.brq.redhat.com>

Vitaly Kuznetsov <vkuznets@redhat.com> writes:

> Paolo Bonzini <pbonzini@redhat.com> writes:
>
>> On 22/01/20 17:29, Vitaly Kuznetsov wrote:
>>> Yes, in case we're back to the idea to filter things out in QEMU we can
>>> do this. What I don't like is that every other userspace which decides
>>> to enable eVMCS will have to perform the exact same surgery as in case
>>> it sets allow_unsupported_controls=0 it'll have to know (hardcode) the
>>> filtering (or KVM_SET_MSRS will fail) and in case it opts for
>>> allow_unsupported_controls=1 Windows guests just won't boot without the
>>> filtering.
>>> 
>>> It seems to be 1:1, eVMCSv1 requires the filter.
>>
>> Yes, that's the point.  It *is* a hack in KVM, but it is generally
>> preferrable to have an easier API for userspace, if there's only one way
>> to do it.
>>
>> Though we could be a bit more "surgical" and only remove
>> SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES---thus minimizing the impact on
>> non-eVMCS guests.  Vitaly, can you prepare a v2 that does that and adds
>> a huge "hack alert" comment that explains the discussion?
>
> Yes, sure. I'd like to do more testing to make sure filtering out
> SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES is enough for other Hyper-V
> versions too (who knows how many bugs are there :-)

... and the answer is -- more than one :-)

I've tested Hyper-V 2016/2019 BIOS and UEFI-booted and the minimal
viable set seems to be:

MSR_IA32_VMX_PROCBASED_CTLS2: 
	~SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES

MSR_IA32_VMX_ENTRY_CTLS/MSR_IA32_VMX_TRUE_ENTRY_CTLS:
	~VM_ENTRY_LOAD_IA32_PERF_GLOBAL_CTRL

MSR_IA32_VMX_EXIT_CTLS/MSR_IA32_VMX_TRUE_EXIT_CTLS:
	~VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL

with these filtered out all 4 versions are at least able to boot with >1
vCPU and run a nested guest (different from Windows management
partition).

This still feels a bit fragile as who knows under which circumstances
Hyper-V might want to enable additional (missing) controls.

If there are no objections and if we still think it would be beneficial
to minimize the list of controls we filter out (and not go with the full
set like my RFC suggests), I'll prepare v2. (v1, actually, this was RFC).

-- 
Vitaly