Re: [RFC PATCH v4 18/26] KVM: x86: Add support for reverse CPUID lookup of scattered features

[Kai Huang <kai.huang@intel.com>]

Hi Paolo,

Although this series was sent as RFC, would you consider reviewing and merging
this patch from Sean? As more Intel features are coming, KVM reverse lookup
needs to handle scattered x86 feature bits. One example is Robert's KVM
keylocker support:

https://www.spinics.net/lists/kvm/msg233638.html

Thanks!

On Mon, 8 Feb 2021 23:55:28 +1300 Kai Huang wrote:
> From: Sean Christopherson <seanjc@google.com>
> 
> Introduce a scheme that allows KVM's CPUID magic to support features
> that are scattered in the kernel's feature words.  To advertise and/or
> query guest support for CPUID-based features, KVM requires the bit
> number of an X86_FEATURE_* to match the bit number in its associated
> CPUID entry.  For scattered features, this does not hold true.
> 
> Add a framework to allow defining KVM-only words, stored in
> kvm_cpu_caps after the shared kernel caps, that can be used to gather
> the scattered feature bits by translating X86_FEATURE_* flags into their
> KVM-defined feature.
> 
> Note, because reverse_cpuid_check() effectively forces kvm_cpu_caps
> lookups to be resolved at compile time, there is no runtime cost for
> translating from kernel-defined to kvm-defined features.
> 
> More details here:  https://lkml.kernel.org/r/X/jxCOLG+HUO4QlZ@google.com
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Kai Huang <kai.huang@intel.com>
> ---
>  arch/x86/kvm/cpuid.c | 32 +++++++++++++++++++++++++++-----
>  arch/x86/kvm/cpuid.h | 39 ++++++++++++++++++++++++++++++++++-----
>  2 files changed, 61 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 13036cf0b912..f8037fab8950 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -28,7 +28,7 @@
>   * Unlike "struct cpuinfo_x86.x86_capability", kvm_cpu_caps doesn't need to be
>   * aligned to sizeof(unsigned long) because it's not accessed via bitops.
>   */
> -u32 kvm_cpu_caps[NCAPINTS] __read_mostly;
> +u32 kvm_cpu_caps[NR_KVM_CPU_CAPS] __read_mostly;
>  EXPORT_SYMBOL_GPL(kvm_cpu_caps);
>  
>  static u32 xstate_required_size(u64 xstate_bv, bool compacted)
> @@ -53,6 +53,7 @@ static u32 xstate_required_size(u64 xstate_bv, bool compacted)
>  }
>  
>  #define F feature_bit
> +#define SF(name) (boot_cpu_has(X86_FEATURE_##name) ? F(name) : 0)
>  
>  static inline struct kvm_cpuid_entry2 *cpuid_entry2_find(
>  	struct kvm_cpuid_entry2 *entries, int nent, u32 function, u32 index)
> @@ -331,13 +332,13 @@ int kvm_vcpu_ioctl_get_cpuid2(struct kvm_vcpu *vcpu,
>  	return r;
>  }
>  
> -static __always_inline void kvm_cpu_cap_mask(enum cpuid_leafs leaf, u32 mask)
> +/* Mask kvm_cpu_caps for @leaf with the raw CPUID capabilities of this CPU. */
> +static __always_inline void __kvm_cpu_cap_mask(enum cpuid_leafs leaf)
>  {
>  	const struct cpuid_reg cpuid = x86_feature_cpuid(leaf * 32);
>  	struct kvm_cpuid_entry2 entry;
>  
>  	reverse_cpuid_check(leaf);
> -	kvm_cpu_caps[leaf] &= mask;
>  
>  	cpuid_count(cpuid.function, cpuid.index,
>  		    &entry.eax, &entry.ebx, &entry.ecx, &entry.edx);
> @@ -345,6 +346,26 @@ static __always_inline void kvm_cpu_cap_mask(enum cpuid_leafs leaf, u32 mask)
>  	kvm_cpu_caps[leaf] &= *__cpuid_entry_get_reg(&entry, cpuid.reg);
>  }
>  
> +static __always_inline void kvm_cpu_cap_mask(enum cpuid_leafs leaf, u32 mask)
> +{
> +	/* Use the "init" variant for scattered leafs. */
> +	BUILD_BUG_ON(leaf >= NCAPINTS);
> +
> +	kvm_cpu_caps[leaf] &= mask;
> +
> +	__kvm_cpu_cap_mask(leaf);
> +}
> +
> +static __always_inline void kvm_cpu_cap_init(enum cpuid_leafs leaf, u32 mask)
> +{
> +	/* Use the "mask" variant for hardwared-defined leafs. */
> +	BUILD_BUG_ON(leaf < NCAPINTS);
> +
> +	kvm_cpu_caps[leaf] = mask;
> +
> +	__kvm_cpu_cap_mask(leaf);
> +}
> +
>  void kvm_set_cpu_caps(void)
>  {
>  	unsigned int f_nx = is_efer_nx() ? F(NX) : 0;
> @@ -355,12 +376,13 @@ void kvm_set_cpu_caps(void)
>  	unsigned int f_gbpages = 0;
>  	unsigned int f_lm = 0;
>  #endif
> +	memset(kvm_cpu_caps, 0, sizeof(kvm_cpu_caps));
>  
> -	BUILD_BUG_ON(sizeof(kvm_cpu_caps) >
> +	BUILD_BUG_ON(sizeof(kvm_cpu_caps) - (NKVMCAPINTS * sizeof(*kvm_cpu_caps)) >
>  		     sizeof(boot_cpu_data.x86_capability));
>  
>  	memcpy(&kvm_cpu_caps, &boot_cpu_data.x86_capability,
> -	       sizeof(kvm_cpu_caps));
> +	       sizeof(kvm_cpu_caps) - (NKVMCAPINTS * sizeof(*kvm_cpu_caps)));
>  
>  	kvm_cpu_cap_mask(CPUID_1_ECX,
>  		/*
> diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h
> index dc921d76e42e..2041e2f07347 100644
> --- a/arch/x86/kvm/cpuid.h
> +++ b/arch/x86/kvm/cpuid.h
> @@ -7,7 +7,20 @@
>  #include <asm/processor.h>
>  #include <uapi/asm/kvm_para.h>
>  
> -extern u32 kvm_cpu_caps[NCAPINTS] __read_mostly;
> +/*
> + * Hardware-defined CPUID leafs that are scattered in the kernel, but need to
> + * be directly used by KVM.  Note, these word values conflict with the kernel's
> + * "bug" caps, but KVM doesn't use those.
> + */
> +enum kvm_only_cpuid_leafs {
> +	NR_KVM_CPU_CAPS = NCAPINTS,
> +
> +	NKVMCAPINTS = NR_KVM_CPU_CAPS - NCAPINTS,
> +};
> +
> +#define X86_KVM_FEATURE(w, f)		((w)*32 + (f))
> +
> +extern u32 kvm_cpu_caps[NR_KVM_CPU_CAPS] __read_mostly;
>  void kvm_set_cpu_caps(void);
>  
>  void kvm_update_cpuid_runtime(struct kvm_vcpu *vcpu);
> @@ -83,6 +96,20 @@ static __always_inline void reverse_cpuid_check(unsigned int x86_leaf)
>  	BUILD_BUG_ON(reverse_cpuid[x86_leaf].function == 0);
>  }
>  
> +/*
> + * Translate feature bits that are scattered in the kernel's cpufeatures word
> + * into KVM feature words that align with hardware's definitions.
> + */
> +static __always_inline u32 __feature_translate(int x86_feature)
> +{
> +	return x86_feature;
> +}
> +
> +static __always_inline u32 __feature_leaf(int x86_feature)
> +{
> +	return __feature_translate(x86_feature) / 32;
> +}
> +
>  /*
>   * Retrieve the bit mask from an X86_FEATURE_* definition.  Features contain
>   * the hardware defined bit number (stored in bits 4:0) and a software defined
> @@ -91,6 +118,8 @@ static __always_inline void reverse_cpuid_check(unsigned int x86_leaf)
>   */
>  static __always_inline u32 __feature_bit(int x86_feature)
>  {
> +	x86_feature = __feature_translate(x86_feature);
> +
>  	reverse_cpuid_check(x86_feature / 32);
>  	return 1 << (x86_feature & 31);
>  }
> @@ -99,7 +128,7 @@ static __always_inline u32 __feature_bit(int x86_feature)
>  
>  static __always_inline struct cpuid_reg x86_feature_cpuid(unsigned int x86_feature)
>  {
> -	unsigned int x86_leaf = x86_feature / 32;
> +	unsigned int x86_leaf = __feature_leaf(x86_feature);
>  
>  	reverse_cpuid_check(x86_leaf);
>  	return reverse_cpuid[x86_leaf];
> @@ -291,7 +320,7 @@ static inline bool cpuid_fault_enabled(struct kvm_vcpu *vcpu)
>  
>  static __always_inline void kvm_cpu_cap_clear(unsigned int x86_feature)
>  {
> -	unsigned int x86_leaf = x86_feature / 32;
> +	unsigned int x86_leaf = __feature_leaf(x86_feature);
>  
>  	reverse_cpuid_check(x86_leaf);
>  	kvm_cpu_caps[x86_leaf] &= ~__feature_bit(x86_feature);
> @@ -299,7 +328,7 @@ static __always_inline void kvm_cpu_cap_clear(unsigned int x86_feature)
>  
>  static __always_inline void kvm_cpu_cap_set(unsigned int x86_feature)
>  {
> -	unsigned int x86_leaf = x86_feature / 32;
> +	unsigned int x86_leaf = __feature_leaf(x86_feature);
>  
>  	reverse_cpuid_check(x86_leaf);
>  	kvm_cpu_caps[x86_leaf] |= __feature_bit(x86_feature);
> @@ -307,7 +336,7 @@ static __always_inline void kvm_cpu_cap_set(unsigned int x86_feature)
>  
>  static __always_inline u32 kvm_cpu_cap_get(unsigned int x86_feature)
>  {
> -	unsigned int x86_leaf = x86_feature / 32;
> +	unsigned int x86_leaf = __feature_leaf(x86_feature);
>  
>  	reverse_cpuid_check(x86_leaf);
>  	return kvm_cpu_caps[x86_leaf] & __feature_bit(x86_feature);
> -- 
> 2.29.2
>