[v8,2/8] x86/cpu, kvm: Move open-coded cpuid leaf 0x80000021 EAX bit propagation code

Message ID 20230123225700.2224063-3-kim.phillips@amd.com
State New
Headers
Series x86/cpu, kvm: Support AMD Automatic IBRS |

Commit Message

Kim Phillips Jan. 23, 2023, 10:56 p.m. UTC
  Move code from __do_cpuid_func() to kvm_set_cpu_caps() in preparation
for adding the features in their native leaf.

Also drop the bit description comments as it will be more self-
describing once the individual features are added.

Whilst there, switch to using the more efficient cpu_feature_enabled()
instead of static_cpu_has().

Signed-off-by: Kim Phillips <kim.phillips@amd.com>
---
 arch/x86/kvm/cpuid.c | 30 +++++++++++-------------------
 1 file changed, 11 insertions(+), 19 deletions(-)
  

Comments

Sean Christopherson Jan. 24, 2023, 1:14 a.m. UTC | #1
Nit, shortlog for this should be

  KVM: x86:

since this touches only KVM code.

On Mon, Jan 23, 2023, Kim Phillips wrote:
> Move code from __do_cpuid_func() to kvm_set_cpu_caps() in preparation
> for adding the features in their native leaf.

Huh, this wasn't why I was expecting, but this is better than what I had in mind.
Moving everything all at once wouldn't work well because of the kernel dependencies.

> Also drop the bit description comments as it will be more self-
> describing once the individual features are added.
> 
> Whilst there, switch to using the more efficient cpu_feature_enabled()
> instead of static_cpu_has().

One more nit/request.  Can you add a blurb about the synthetic features?  That
part is easy to miss and will be confusing after the fact.  E.g.

 Note, LFENCE_RDTSC and "NULL selector clears base" are is currently
 synthetic, Linux-defined feature flags as Linux tracking of the features
 predates AMD's definition.  Keep the manual propagation of the flags from
 their synthetic counterparts until the kernel fully converts to AMD's
 definition, otherwise KVM would  stop synthesizing the flags as intended.
 
> Signed-off-by: Kim Phillips <kim.phillips@amd.com>
> ---
>  arch/x86/kvm/cpuid.c | 30 +++++++++++-------------------
>  1 file changed, 11 insertions(+), 19 deletions(-)
> 
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 596061c1610e..3930452bf06e 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -741,6 +741,16 @@ void kvm_set_cpu_caps(void)
>  		0 /* SME */ | F(SEV) | 0 /* VM_PAGE_FLUSH */ | F(SEV_ES) |
>  		F(SME_COHERENT));
>  
> +	kvm_cpu_cap_mask(CPUID_8000_0021_EAX,
> +		BIT(0) /* NO_NESTED_DATA_BP */ | 0 /* SmmPgCfgLock */ |

Uber nit, to make this a bit closer to pure code movement, this should include
BIT(2) as well.  Mainly because BIT(6) is also kept even though it too may be
synthesized by KVM.

> +		BIT(6) /* NULL_SEL_CLR_BASE */ | 0 /* PrefetchCtlMsr */
> +	);
> +	if (cpu_feature_enabled(X86_FEATURE_LFENCE_RDTSC))
> +		kvm_cpu_caps[CPUID_8000_0021_EAX] |= BIT(2) /* LFENCE Always serializing */;
> +	if (!static_cpu_has_bug(X86_BUG_NULL_SEG))
> +		kvm_cpu_caps[CPUID_8000_0021_EAX] |= BIT(6) /* NULL_SEL_CLR_BASE */;
> +	kvm_cpu_caps[CPUID_8000_0021_EAX] |= BIT(9) /* NO_SMM_CTL_MSR */;
> +
>  	kvm_cpu_cap_mask(CPUID_C000_0001_EDX,
>  		F(XSTORE) | F(XSTORE_EN) | F(XCRYPT) | F(XCRYPT_EN) |
>  		F(ACE2) | F(ACE2_EN) | F(PHE) | F(PHE_EN) |
  

Patch

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 596061c1610e..3930452bf06e 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -741,6 +741,16 @@  void kvm_set_cpu_caps(void)
 		0 /* SME */ | F(SEV) | 0 /* VM_PAGE_FLUSH */ | F(SEV_ES) |
 		F(SME_COHERENT));
 
+	kvm_cpu_cap_mask(CPUID_8000_0021_EAX,
+		BIT(0) /* NO_NESTED_DATA_BP */ | 0 /* SmmPgCfgLock */ |
+		BIT(6) /* NULL_SEL_CLR_BASE */ | 0 /* PrefetchCtlMsr */
+	);
+	if (cpu_feature_enabled(X86_FEATURE_LFENCE_RDTSC))
+		kvm_cpu_caps[CPUID_8000_0021_EAX] |= BIT(2) /* LFENCE Always serializing */;
+	if (!static_cpu_has_bug(X86_BUG_NULL_SEG))
+		kvm_cpu_caps[CPUID_8000_0021_EAX] |= BIT(6) /* NULL_SEL_CLR_BASE */;
+	kvm_cpu_caps[CPUID_8000_0021_EAX] |= BIT(9) /* NO_SMM_CTL_MSR */;
+
 	kvm_cpu_cap_mask(CPUID_C000_0001_EDX,
 		F(XSTORE) | F(XSTORE_EN) | F(XCRYPT) | F(XCRYPT_EN) |
 		F(ACE2) | F(ACE2_EN) | F(PHE) | F(PHE_EN) |
@@ -1222,25 +1232,7 @@  static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function)
 		break;
 	case 0x80000021:
 		entry->ebx = entry->ecx = entry->edx = 0;
-		/*
-		 * Pass down these bits:
-		 *    EAX      0      NNDBP, Processor ignores nested data breakpoints
-		 *    EAX      2      LAS, LFENCE always serializing
-		 *    EAX      6      NSCB, Null selector clear base
-		 *
-		 * Other defined bits are for MSRs that KVM does not expose:
-		 *   EAX      3      SPCL, SMM page configuration lock
-		 *   EAX      13     PCMSR, Prefetch control MSR
-		 *
-		 * KVM doesn't support SMM_CTL.
-		 *   EAX       9     SMM_CTL MSR is not supported
-		 */
-		entry->eax &= BIT(0) | BIT(2) | BIT(6);
-		entry->eax |= BIT(9);
-		if (static_cpu_has(X86_FEATURE_LFENCE_RDTSC))
-			entry->eax |= BIT(2);
-		if (!static_cpu_has_bug(X86_BUG_NULL_SEG))
-			entry->eax |= BIT(6);
+		cpuid_entry_override(entry, CPUID_8000_0021_EAX);
 		break;
 	/*Add support for Centaur's CPUID instruction*/
 	case 0xC0000000: