On Wed, 2024-11-27 at 17:33 -0800, Sean Christopherson wrote:
> The super short TL;DR: snapshot all X86_FEATURE_* flags that KVM cares
> about so that all queries against guest capabilities are "fast", e.g. don't
> require manual enabling or judgment calls as to where a feature needs to be
> fast.
>
> The guest_cpu_cap_* nomenclature follows the existing kvm_cpu_cap_*
> except for a few (maybe just one?) cases where guest cpu_caps need APIs
> that kvm_cpu_caps don't. In theory, the similar names will make this
> approach more intuitive.
>
> This series also adds more hardening, e.g. to assert at compile-time if a
> feature flag is passed to the wrong word. It also sets the stage for even
> more hardening in the future, as tracking all KVM-supported features allows
> shoving known vs. used features into arrays at compile time, which can then
> be checked for consistency irrespective of hardware support. E.g. allows
> detecting if KVM is checking a feature without advertising it to userspace.
> This extra hardening is future work; I have it mostly working, but it's ugly
> and requires a runtime check to process the generated arrays.
>
> There are *multiple* potentially breaking changes in this series (in for a
> penny, in for a pound). However, I don't expect any fallout for real world
> VMMs because the ABI changes either disallow things that couldn't possibly
> have worked in the first place, or are following in the footsteps of other
> behaviors, e.g. KVM advertises x2APIC, which is 100% dependent on an in-kernel
> local APIC.
>
> * Disallow stuffing CPUID-dependent guest CR4 features before setting guest
> CPUID.
> * Disallow KVM_CAP_X86_DISABLE_EXITS after vCPU creation
> * Reject disabling of MWAIT/HLT interception when not allowed
> * Advertise TSC_DEADLINE_TIMER in KVM_GET_SUPPORTED_CPUID.
> * Advertise HYPERVISOR in KVM_GET_SUPPORTED_CPUID
>
> Validated the flag rework by comparing the output of KVM_GET_SUPPORTED_CPUID
> (and the emulated version) at the beginning and end of the series, on AMD
> and Intel hosts that should support almost every feature known to KVM.
>
> Maxim, I did my best to incorporate all of your feedback, and when we
> disagreed, I tried to find an approach that I we can hopefully both live
> with, at least until someone comes up with a better idea.
>
> I _think_ the only suggestion that I "rejected" entirely is the existence
> of ALIASED_1_EDX_F. I responded to the previous thread, definitely feel
> free to continue the conversation there (or here).
>
> If I missed something you care strongly about, please holler!
Hi,
I did go over this patch series, I don't think I have anything to add,
there are still things I disagree, especially the F* macros, IMHO this
makes the code less readable.
So if you want to merge this, I won't object.
Thanks,
Best regards,
Maxim Levitsky
>
> v3:
> - Collect more reviews.
> - Too many to list.
>
> v2:
> - Collect a few reviews (though I dropped several due to the patches changing
> significantly).
> - Incorporate KVM's support into the vCPU's cpu_caps. [Maxim]
> - A massive pile of new patches.
>
> Sean Christopherson (57):
> KVM: x86: Use feature_bit() to clear CONSTANT_TSC when emulating CPUID
> KVM: x86: Limit use of F() and SF() to
> kvm_cpu_cap_{mask,init_kvm_defined}()
> KVM: x86: Do all post-set CPUID processing during vCPU creation
> KVM: x86: Explicitly do runtime CPUID updates "after" initial setup
> KVM: x86: Account for KVM-reserved CR4 bits when passing through CR4
> on VMX
> KVM: selftests: Update x86's set_sregs_test to match KVM's CPUID
> enforcement
> KVM: selftests: Assert that vcpu->cpuid is non-NULL when getting CPUID
> entries
> KVM: selftests: Refresh vCPU CPUID cache in __vcpu_get_cpuid_entry()
> KVM: selftests: Verify KVM stuffs runtime CPUID OS bits on CR4 writes
> KVM: x86: Move __kvm_is_valid_cr4() definition to x86.h
> KVM: x86/pmu: Drop now-redundant refresh() during init()
> KVM: x86: Drop now-redundant MAXPHYADDR and GPA rsvd bits from vCPU
> creation
> KVM: x86: Disallow KVM_CAP_X86_DISABLE_EXITS after vCPU creation
> KVM: x86: Reject disabling of MWAIT/HLT interception when not allowed
> KVM: x86: Drop the now unused KVM_X86_DISABLE_VALID_EXITS
> KVM: selftests: Fix a bad TEST_REQUIRE() in x86's KVM PV test
> KVM: selftests: Update x86's KVM PV test to match KVM's disabling
> exits behavior
> KVM: x86: Zero out PV features cache when the CPUID leaf is not
> present
> KVM: x86: Don't update PV features caches when enabling enforcement
> capability
> KVM: x86: Do reverse CPUID sanity checks in __feature_leaf()
> KVM: x86: Account for max supported CPUID leaf when getting raw host
> CPUID
> KVM: x86: Unpack F() CPUID feature flag macros to one flag per line of
> code
> KVM: x86: Rename kvm_cpu_cap_mask() to kvm_cpu_cap_init()
> KVM: x86: Add a macro to init CPUID features that are 64-bit only
> KVM: x86: Add a macro to precisely handle aliased 0x1.EDX CPUID
> features
> KVM: x86: Handle kernel- and KVM-defined CPUID words in a single
> helper
> KVM: x86: #undef SPEC_CTRL_SSBD in cpuid.c to avoid macro collisions
> KVM: x86: Harden CPU capabilities processing against out-of-scope
> features
> KVM: x86: Add a macro to init CPUID features that ignore host kernel
> support
> KVM: x86: Add a macro to init CPUID features that KVM emulates in
> software
> KVM: x86: Swap incoming guest CPUID into vCPU before massaging in
> KVM_SET_CPUID2
> KVM: x86: Clear PV_UNHALT for !HLT-exiting only when userspace sets
> CPUID
> KVM: x86: Remove unnecessary caching of KVM's PV CPUID base
> KVM: x86: Always operate on kvm_vcpu data in cpuid_entry2_find()
> KVM: x86: Move kvm_find_cpuid_entry{,_index}() up near
> cpuid_entry2_find()
> KVM: x86: Remove all direct usage of cpuid_entry2_find()
> KVM: x86: Advertise TSC_DEADLINE_TIMER in KVM_GET_SUPPORTED_CPUID
> KVM: x86: Advertise HYPERVISOR in KVM_GET_SUPPORTED_CPUID
> KVM: x86: Rename "governed features" helpers to use "guest_cpu_cap"
> KVM: x86: Replace guts of "governed" features with comprehensive
> cpu_caps
> KVM: x86: Initialize guest cpu_caps based on guest CPUID
> KVM: x86: Extract code for generating per-entry emulated CPUID
> information
> KVM: x86: Treat MONTIOR/MWAIT as a "partially emulated" feature
> KVM: x86: Initialize guest cpu_caps based on KVM support
> KVM: x86: Avoid double CPUID lookup when updating MWAIT at runtime
> KVM: x86: Drop unnecessary check that cpuid_entry2_find() returns
> right leaf
> KVM: x86: Update OS{XSAVE,PKE} bits in guest CPUID irrespective of
> host support
> KVM: x86: Update guest cpu_caps at runtime for dynamic CPUID-based
> features
> KVM: x86: Shuffle code to prepare for dropping guest_cpuid_has()
> KVM: x86: Replace (almost) all guest CPUID feature queries with
> cpu_caps
> KVM: x86: Drop superfluous host XSAVE check when adjusting guest
> XSAVES caps
> KVM: x86: Add a macro for features that are synthesized into
> boot_cpu_data
> KVM: x86: Pull CPUID capabilities from boot_cpu_data only as needed
> KVM: x86: Rename "SF" macro to "SCATTERED_F"
> KVM: x86: Explicitly track feature flags that require vendor enabling
> KVM: x86: Explicitly track feature flags that are enabled at runtime
> KVM: x86: Use only local variables (no bitmask) to init kvm_cpu_caps
>
> Documentation/virt/kvm/api.rst | 10 +-
> arch/x86/include/asm/kvm_host.h | 47 +-
> arch/x86/kvm/cpuid.c | 967 ++++++++++++------
> arch/x86/kvm/cpuid.h | 128 +--
> arch/x86/kvm/governed_features.h | 22 -
> arch/x86/kvm/hyperv.c | 2 +-
> arch/x86/kvm/lapic.c | 4 +-
> arch/x86/kvm/mmu.h | 2 +-
> arch/x86/kvm/mmu/mmu.c | 4 +-
> arch/x86/kvm/pmu.c | 1 -
> arch/x86/kvm/reverse_cpuid.h | 23 +-
> arch/x86/kvm/smm.c | 10 +-
> arch/x86/kvm/svm/nested.c | 22 +-
> arch/x86/kvm/svm/pmu.c | 8 +-
> arch/x86/kvm/svm/sev.c | 21 +-
> arch/x86/kvm/svm/svm.c | 46 +-
> arch/x86/kvm/svm/svm.h | 4 +-
> arch/x86/kvm/vmx/hyperv.h | 2 +-
> arch/x86/kvm/vmx/nested.c | 18 +-
> arch/x86/kvm/vmx/pmu_intel.c | 4 +-
> arch/x86/kvm/vmx/sgx.c | 14 +-
> arch/x86/kvm/vmx/vmx.c | 61 +-
> arch/x86/kvm/x86.c | 153 ++-
> arch/x86/kvm/x86.h | 6 +-
> include/uapi/linux/kvm.h | 4 -
> .../selftests/kvm/include/x86_64/processor.h | 18 +-
> .../selftests/kvm/x86_64/kvm_pv_test.c | 38 +-
> .../selftests/kvm/x86_64/set_sregs_test.c | 63 +-
> 28 files changed, 1017 insertions(+), 685 deletions(-)
> delete mode 100644 arch/x86/kvm/governed_features.h
>
>
> base-commit: 4d911c7abee56771b0219a9fbf0120d06bdc9c14
