Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Vitaly Kuznetsov <vkuznets@redhat.com>
To:     Sean Christopherson <seanjc@google.com>
Cc:     kvm@vger.kernel.org, Paolo Bonzini <pbonzini@redhat.com>,
        Anirudh Rayabharam <anrayabh@linux.microsoft.com>,
        Wanpeng Li <wanpengli@tencent.com>,
        Jim Mattson <jmattson@google.com>,
        Maxim Levitsky <mlevitsk@redhat.com>,
        linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 06/28] KVM: nVMX: Introduce
 KVM_CAP_HYPERV_ENLIGHTENED_VMCS2
In-Reply-To: <YsYAPL1UUKJB3/MJ@google.com>
References: <20220629150625.238286-1-vkuznets@redhat.com>
 <20220629150625.238286-7-vkuznets@redhat.com>
 <YsYAPL1UUKJB3/MJ@google.com>
Date:   Thu, 07 Jul 2022 11:58:38 +0200
Message-ID: <87o7y1qm5t.fsf@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

Sean Christopherson <seanjc@google.com> writes:

> On Wed, Jun 29, 2022, Vitaly Kuznetsov wrote:
>> Turns out Enlightened VMCS can gain new fields without version change
>> and KVM_CAP_HYPERV_ENLIGHTENED_VMCS which KVM currently has cant's
>> handle this reliably. In particular, just updating the current definition
>> of eVMCSv1 with the new fields and adjusting the VMX MSR filtering will
>> inevitably break live migration to older KVMs. Note: enabling eVMCS and
>> setting VMX feature MSR can happen in any order.
>> 
>> Introduce a notion of KVM internal "Enlightened VMCS revision" and add
>> a new capability allowing to add fields to Enlightened VMCS while keeping
>> its version.
>
> Bumping a "minor" version number in KVM is going to be a nightmare.  KVM is going
> to be stuck "supporting" old revisions in perpetuity, and userspace will be forced
> to keep track of which features are available with which arbitrary revision (is
> that information even communicated to userspace?).

My brain is certainly tainted with how we enable this in QEMU but why
would userspace be interested in which features are actually filtered
out?

Currently (again, by QEMU), eVMCS is treated as a purely software
feature. When enabled, certain controls are filtered out "under the
hood" as VMX MSRs reported to VMM remain unfiltered (see
'!msr_info->host_initiated' in vmx_get_msr()). Same stays true with any
new revision: VMM's job is just to check that a) all hardware features
are supported on both source and destination and b) the requested 'eVMCS
revision' is supported by both. No need to know what's filtered out and
what isn't.

>
> I think a more maintainable approach would be to expose the "filtered" VMX MSRs to
> userspace, e.g. add KVM_GET_EVMCS_VMX_MSRS.  Then KVM just needs to document what
> the "filters" are for KVM versions that don't support KVM_GET_EVMCS_VMX_MSRS.
> KVM itself doesn't need to maintain version information because it's userspace's
> responsibility to ensure that userspace doesn't try to migrate to a KVM that doesn't
> support the desired feature set.

That would be a reasonable (but complex for VMM) approach too but I
don't think we need this (and this patch introducing 'eVMCS revisions'
to this matter): luckily, Microsoft added a new PV CPUID feature bit
inidicating the support for the new features in eVMCSv1 so KVM can just
observe whether the bit was set by VMM or not and filter accordingly.

>
> That also avoids messes like unnecessarily blocking migration from "incompatible"
> revisions when running on hardware that doesn't even support the control.

Well yea, in case the difference between 'eVMCS revisions' is void
because the hardware doesn't support these, it would still be possible
to migrate to an older KVM which doesn't support the new revision but
I'd stay strict: if a newer revision was requested it must be supported,
no matter the hardware.

-- 
Vitaly