Messages in this thread | | | Date | Mon, 14 Aug 2023 12:43:57 +0800 | Subject | Re: [PATCH v2 07/21] KVM: x86: Add a framework for enabling KVM-governed x86 features | From | Zeng Guang <> |
| |
On 7/29/2023 9:15 AM, Sean Christopherson wrote: > Introduce yet another X86_FEATURE flag framework to manage and cache KVM > governed features (for lack of a better name). "Governed" in this case > means that KVM has some level of involvement and/or vested interest in > whether or not an X86_FEATURE can be used by the guest. The intent of the > framework is twofold: to simplify caching of guest CPUID flags that KVM > needs to frequently query, and to add clarity to such caching, e.g. it > isn't immediately obvious that SVM's bundle of flags for "optional nested > SVM features" track whether or not a flag is exposed to L1. > > Begrudgingly define KVM_MAX_NR_GOVERNED_FEATURES for the size of the > bitmap to avoid exposing governed_features.h in arch/x86/include/asm/, but > add a FIXME to call out that it can and should be cleaned up once > "struct kvm_vcpu_arch" is no longer expose to the kernel at large. > > Signed-off-by: Sean Christopherson <seanjc@google.com> > --- > arch/x86/include/asm/kvm_host.h | 19 +++++++++++++ > arch/x86/kvm/cpuid.c | 4 +++ > arch/x86/kvm/cpuid.h | 46 ++++++++++++++++++++++++++++++++ > arch/x86/kvm/governed_features.h | 9 +++++++ > 4 files changed, 78 insertions(+) > create mode 100644 arch/x86/kvm/governed_features.h > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > index dad9331c5270..007fa8bfd634 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -831,6 +831,25 @@ struct kvm_vcpu_arch { > struct kvm_cpuid_entry2 *cpuid_entries; > struct kvm_hypervisor_cpuid kvm_cpuid; > > + /* > + * FIXME: Drop this macro and use KVM_NR_GOVERNED_FEATURES directly > + * when "struct kvm_vcpu_arch" is no longer defined in an > + * arch/x86/include/asm header. The max is mostly arbitrary, i.e. > + * can be increased as necessary. > + */ > +#define KVM_MAX_NR_GOVERNED_FEATURES BITS_PER_LONG > + > + /* > + * Track whether or not the guest is allowed to use features that are > + * governed by KVM, where "governed" means KVM needs to manage state > + * and/or explicitly enable the feature in hardware. Typically, but > + * not always, governed features can be used by the guest if and only > + * if both KVM and userspace want to expose the feature to the guest. > + */ > + struct { > + DECLARE_BITMAP(enabled, KVM_MAX_NR_GOVERNED_FEATURES); > + } governed_features; > + > u64 reserved_gpa_bits; > int maxphyaddr; > > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c > index 7f4d13383cf2..ef826568c222 100644 > --- a/arch/x86/kvm/cpuid.c > +++ b/arch/x86/kvm/cpuid.c > @@ -313,6 +313,10 @@ static void kvm_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu) > struct kvm_lapic *apic = vcpu->arch.apic; > struct kvm_cpuid_entry2 *best; > > + BUILD_BUG_ON(KVM_NR_GOVERNED_FEATURES > KVM_MAX_NR_GOVERNED_FEATURES); > + bitmap_zero(vcpu->arch.governed_features.enabled, > + KVM_MAX_NR_GOVERNED_FEATURES); > + > best = kvm_find_cpuid_entry(vcpu, 1); > if (best && apic) { > if (cpuid_entry_has(best, X86_FEATURE_TSC_DEADLINE_TIMER)) > diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h > index b1658c0de847..3000fbe97678 100644 > --- a/arch/x86/kvm/cpuid.h > +++ b/arch/x86/kvm/cpuid.h > @@ -232,4 +232,50 @@ static __always_inline bool guest_pv_has(struct kvm_vcpu *vcpu, > return vcpu->arch.pv_cpuid.features & (1u << kvm_feature); > } > > +enum kvm_governed_features { > +#define KVM_GOVERNED_FEATURE(x) KVM_GOVERNED_##x, > +#include "governed_features.h" > + KVM_NR_GOVERNED_FEATURES > +}; > + > +static __always_inline int kvm_governed_feature_index(unsigned int x86_feature) > +{ > + switch (x86_feature) { > +#define KVM_GOVERNED_FEATURE(x) case x: return KVM_GOVERNED_##x; > +#include "governed_features.h" > + default: > + return -1; > + } > +} > + > +static __always_inline int kvm_is_governed_feature(unsigned int x86_feature) > +{ > + return kvm_governed_feature_index(x86_feature) >= 0; > +} > +
I think it proper to return a bool, not "int" instead.
| |