[01/13] x86/CPU/AMD: Add ZenX generations flags

  From: "Borislav Petkov (AMD)" <bp@alien8.de>

Add X86_FEATURE flags for each Zen generation. They should be used from
now on instead of checking f/m/s.

Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
---
 arch/x86/include/asm/cpufeatures.h |  6 ++-
 arch/x86/kernel/cpu/amd.c          | 70 +++++++++++++++++++++++++++++-
 2 files changed, 72 insertions(+), 4 deletions(-)
  

On 11/20/23 04:41, Borislav Petkov wrote:
> From: "Borislav Petkov (AMD)" <bp@alien8.de>
> 
> Add X86_FEATURE flags for each Zen generation. They should be used from
> now on instead of checking f/m/s.
> 
> Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
> ---
>   arch/x86/include/asm/cpufeatures.h |  6 ++-
>   arch/x86/kernel/cpu/amd.c          | 70 +++++++++++++++++++++++++++++-
>   2 files changed, 72 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
> index 4af140cf5719..6f6cf49e9891 100644
> --- a/arch/x86/include/asm/cpufeatures.h
> +++ b/arch/x86/include/asm/cpufeatures.h
> @@ -218,7 +218,7 @@
>   #define X86_FEATURE_IBRS		( 7*32+25) /* Indirect Branch Restricted Speculation */
>   #define X86_FEATURE_IBPB		( 7*32+26) /* Indirect Branch Prediction Barrier */
>   #define X86_FEATURE_STIBP		( 7*32+27) /* Single Thread Indirect Branch Predictors */
> -#define X86_FEATURE_ZEN			(7*32+28) /* "" CPU based on Zen microarchitecture */
> +#define X86_FEATURE_ZEN			( 7*32+28) /* "" CPU based on Zen microarchitecture */
>   #define X86_FEATURE_L1TF_PTEINV		( 7*32+29) /* "" L1TF workaround PTE inversion */
>   #define X86_FEATURE_IBRS_ENHANCED	( 7*32+30) /* Enhanced IBRS */
>   #define X86_FEATURE_MSR_IA32_FEAT_CTL	( 7*32+31) /* "" MSR IA32_FEAT_CTL configured */
> @@ -308,10 +308,12 @@
>   #define X86_FEATURE_SMBA		(11*32+21) /* "" Slow Memory Bandwidth Allocation */
>   #define X86_FEATURE_BMEC		(11*32+22) /* "" Bandwidth Monitoring Event Configuration */
>   #define X86_FEATURE_USER_SHSTK		(11*32+23) /* Shadow stack support for user mode applications */
> -
>   #define X86_FEATURE_SRSO		(11*32+24) /* "" AMD BTB untrain RETs */
>   #define X86_FEATURE_SRSO_ALIAS		(11*32+25) /* "" AMD BTB untrain RETs through aliasing */
>   #define X86_FEATURE_IBPB_ON_VMEXIT	(11*32+26) /* "" Issue an IBPB only on VMEXIT */
> +#define X86_FEATURE_ZEN2		(11*32+27) /* "" CPU based on Zen2 microarchitecture */
> +#define X86_FEATURE_ZEN3		(11*32+28) /* "" CPU based on Zen3 microarchitecture */
> +#define X86_FEATURE_ZEN4		(11*32+29) /* "" CPU based on Zen4 microarchitecture */
>   
>   /* Intel-defined CPU features, CPUID level 0x00000007:1 (EAX), word 12 */
>   #define X86_FEATURE_AVX_VNNI		(12*32+ 4) /* AVX VNNI instructions */
> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
> index a7eab05e5f29..fa6ba63ca7e2 100644
> --- a/arch/x86/kernel/cpu/amd.c
> +++ b/arch/x86/kernel/cpu/amd.c
> @@ -616,6 +616,49 @@ static void bsp_init_amd(struct cpuinfo_x86 *c)
>   	}
>   
>   	resctrl_cpu_detect(c);
> +
> +	/* Figure out Zen generations: */
> +	switch (c->x86) {
> +	case 0x17: {
> +		switch (c->x86_model) {
> +		case 0x00 ... 0x2f:
> +		case 0x50 ... 0x5f:
> +			setup_force_cpu_cap(X86_FEATURE_ZEN);
> +			break;
> +		case 0x30 ... 0x4f:
> +		case 0x60 ... 0x7f:
> +		case 0x90 ... 0x91:
> +		case 0xa0 ... 0xaf:
> +			setup_force_cpu_cap(X86_FEATURE_ZEN2);
> +			break;
> +		default:
> +			goto warn;

Previously just being family 17h or 19h would get X86_FEATURE_ZEN set. 
With this, if the model check doesn't match, you won't get any 
X86_FEATURE_ZEN* set. Should you do set X86_FEATURE_ZEN here, e.g. lowest 
common denominator for the family?

> +		}
> +		break;
> +	}
> +	case 0x19: {
> +		switch (c->x86_model) {
> +		case 0x00 ... 0x0f:
> +		case 0x20 ... 0x5f:
> +			setup_force_cpu_cap(X86_FEATURE_ZEN3);
> +			break;
> +		case 0x10 ... 0x1f:
> +		case 0x60 ... 0xaf:
> +			setup_force_cpu_cap(X86_FEATURE_ZEN4);
> +			break;
> +		default:

Ditto here to set X86_FEATURE_ZEN3?

Thanks,
Tom

> +			goto warn;
> +		}
> +		break;
> +	}
> +	default:
> +		break;
> +	}
> +
> +	return;
> +
> +warn:
> +	WARN_ONCE(1, "Family 0x%x, model: 0x%x??\n", c->x86, c->x86_model);
>   }
>   
>   static void early_detect_mem_encrypt(struct cpuinfo_x86 *c)
> @@ -974,8 +1017,6 @@ void init_spectral_chicken(struct cpuinfo_x86 *c)
>   
>   static void init_amd_zn(struct cpuinfo_x86 *c)
>   {
> -	set_cpu_cap(c, X86_FEATURE_ZEN);
> -
>   #ifdef CONFIG_NUMA
>   	node_reclaim_distance = 32;
>   #endif
> @@ -997,6 +1038,22 @@ static void init_amd_zn(struct cpuinfo_x86 *c)
>   	}
>   }
>   
> +static void init_amd_zen(struct cpuinfo_x86 *c)
> +{
> +}
> +
> +static void init_amd_zen2(struct cpuinfo_x86 *c)
> +{
> +}
> +
> +static void init_amd_zen3(struct cpuinfo_x86 *c)
> +{
> +}
> +
> +static void init_amd_zen4(struct cpuinfo_x86 *c)
> +{
> +}
> +
>   static bool cpu_has_zenbleed_microcode(void)
>   {
>   	u32 good_rev = 0;
> @@ -1077,6 +1134,15 @@ static void init_amd(struct cpuinfo_x86 *c)
>   	case 0x19: init_amd_zn(c); break;
>   	}
>   
> +	if (boot_cpu_has(X86_FEATURE_ZEN))
> +		init_amd_zen(c);
> +	else if (boot_cpu_has(X86_FEATURE_ZEN2))
> +		init_amd_zen2(c);
> +	else if (boot_cpu_has(X86_FEATURE_ZEN3))
> +		init_amd_zen3(c);
> +	else if (boot_cpu_has(X86_FEATURE_ZEN4))
> +		init_amd_zen4(c);
> +
>   	/*
>   	 * Enable workaround for FXSAVE leak on CPUs
>   	 * without a XSaveErPtr feature
  

On Thu, Nov 30, 2023 at 11:05:14AM -0600, Tom Lendacky wrote:
> Previously just being family 17h or 19h would get X86_FEATURE_ZEN set. With
> this, if the model check doesn't match, you won't get any X86_FEATURE_ZEN*
> set. Should you do set X86_FEATURE_ZEN here, e.g. lowest common denominator
> for the family?

My assumption/expectation is that those WARNs should never happen
because they will be caught early enough in enablement and I will get
patches.

Besides, X86_FEATURE_ZEN means only Zen1 now.
  

On Thu, Nov 30, 2023 at 12:14 PM Borislav Petkov <bp@alien8.de> wrote:
>
> On Thu, Nov 30, 2023 at 11:05:14AM -0600, Tom Lendacky wrote:
> > Previously just being family 17h or 19h would get X86_FEATURE_ZEN set. With
> > this, if the model check doesn't match, you won't get any X86_FEATURE_ZEN*
> > set. Should you do set X86_FEATURE_ZEN here, e.g. lowest common denominator
> > for the family?
>
> My assumption/expectation is that those WARNs should never happen
> because they will be caught early enough in enablement and I will get
> patches.
>
> Besides, X86_FEATURE_ZEN means only Zen1 now.

It should be renamed to X86_FEATURE_ZEN1 for clarity.

Brian Gerst
  

On 11/30/23 11:13, Borislav Petkov wrote:
> On Thu, Nov 30, 2023 at 11:05:14AM -0600, Tom Lendacky wrote:
>> Previously just being family 17h or 19h would get X86_FEATURE_ZEN set. With
>> this, if the model check doesn't match, you won't get any X86_FEATURE_ZEN*
>> set. Should you do set X86_FEATURE_ZEN here, e.g. lowest common denominator
>> for the family?
> 
> My assumption/expectation is that those WARNs should never happen
> because they will be caught early enough in enablement and I will get
> patches.
> 
> Besides, X86_FEATURE_ZEN means only Zen1 now.

There are references to X86_FEATURE_ZEN in arch/x86/kernel/process.c and 
drivers/acpi/resource.c that should probably be vetted.

Maybe having X86_FEATURE_ZEN mean all ZEN (and set for anything family 17h 
or higher) and a separate per generation, e.g. X86_FEATURE_ZEN1, when you 
need to be specific, would work.

Thanks,
Tom

>
  

On Thu, Nov 30, 2023 at 12:17:02PM -0600, Tom Lendacky wrote:
> There are references to X86_FEATURE_ZEN in arch/x86/kernel/process.c and
> drivers/acpi/resource.c that should probably be vetted.
> 
> Maybe having X86_FEATURE_ZEN mean all ZEN (and set for anything family 17h
> or higher) and a separate per generation, e.g. X86_FEATURE_ZEN1, when you
> need to be specific, would work.

Yap, looks like it. Those process.c and resource.c things mean all Zen
- and perhaps that really should mean that - Zen and newer. This all
falls nicely into place anyway since Zen stopped using a family number
per generation so that's a natural cutoff point where we're going to
start using those synthetic feature flags for the generations and one
for all Zens.

And yap, the Zen1 stuff should probably be behind X86_FEATURE_ZEN1.

Yap, sounds good. Lemme cook up a patch tomorrow.

Thx to you and Brian for the suggestions.