x86/CPU/AMD: Perform function calls post ZEN feature check regardless

[PATCH] x86/CPU/AMD: Perform function calls post ZEN feature check regardless

Posted by Tom Lendacky 2 months, 2 weeks ago

Currently, if a ZEN family/model is not recognized in bsp_init_amd(), then
function calls after the family/model check are skipped. This can prevent
older kernels from enabling features on newer hardware, e.g., unrecognized
new hardware won't enable SEV-SNP because bsp_determine_snp() is skipped.

Rearrange the way the WARN_ONCE() is issued so that functions called after
the family/model check are still called.

Fixes: 216d106c7ff7 ("x86/sev: Add SEV-SNP host initialization support")
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
 arch/x86/kernel/cpu/amd.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index a5ece6ebe8a7..79ea6a0fcf8a 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -420,6 +420,8 @@ static void tsa_init(struct cpuinfo_x86 *c)
 
 static void bsp_init_amd(struct cpuinfo_x86 *c)
 {
+	bool model_warn = false;
+
 	if (cpu_has(c, X86_FEATURE_CONSTANT_TSC)) {
 
 		if (c->x86 > 0x10 ||
@@ -489,7 +491,7 @@ static void bsp_init_amd(struct cpuinfo_x86 *c)
 			setup_force_cpu_cap(X86_FEATURE_ZEN2);
 			break;
 		default:
-			goto warn;
+			model_warn = true;
 		}
 		break;
 
@@ -504,7 +506,7 @@ static void bsp_init_amd(struct cpuinfo_x86 *c)
 			setup_force_cpu_cap(X86_FEATURE_ZEN4);
 			break;
 		default:
-			goto warn;
+			model_warn = true;
 		}
 		break;
 
@@ -521,7 +523,7 @@ static void bsp_init_amd(struct cpuinfo_x86 *c)
 			setup_force_cpu_cap(X86_FEATURE_ZEN6);
 			break;
 		default:
-			goto warn;
+			model_warn = true;
 		}
 		break;
 
@@ -529,16 +531,13 @@ static void bsp_init_amd(struct cpuinfo_x86 *c)
 		break;
 	}
 
+	WARN_ONCE(model_warn, "Family 0x%x, model: 0x%x??\n", c->x86, c->x86_model);
+
 	bsp_determine_snp(c);
 	tsa_init(c);
 
 	if (cpu_has(c, X86_FEATURE_GP_ON_USER_CPUID))
 		setup_force_cpu_cap(X86_FEATURE_CPUID_FAULT);
-
-	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)

base-commit: 804fa10c3a3c8a33ce6ffdb999f94d3a7a6c20b8
-- 
2.46.2

Re: [PATCH] x86/CPU/AMD: Perform function calls post ZEN feature check regardless

Posted by Borislav Petkov 1 month, 3 weeks ago

On Wed, Jul 23, 2025 at 12:01:06PM -0500, Tom Lendacky wrote:
> Currently, if a ZEN family/model is not recognized in bsp_init_amd(), then
> function calls after the family/model check are skipped. This can prevent
> older kernels from enabling features on newer hardware, e.g., unrecognized
> new hardware won't enable SEV-SNP because bsp_determine_snp() is skipped.

bsp_determine_snp() relies partly on the family detection.

If all you care is calling it even without family detection, why don't you
pull it up, check c->x86 and be done with it?

But even then: we don't really care about older kernels on newer hardware
- that's why they get backports. We don't really think about backporting when
working upstream. Frankly...

So I'm sceptical about this.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] x86/CPU/AMD: Perform function calls post ZEN feature check regardless

Posted by Tom Lendacky 1 month, 3 weeks ago

On 8/12/25 16:37, Borislav Petkov wrote:
> On Wed, Jul 23, 2025 at 12:01:06PM -0500, Tom Lendacky wrote:
>> Currently, if a ZEN family/model is not recognized in bsp_init_amd(), then
>> function calls after the family/model check are skipped. This can prevent
>> older kernels from enabling features on newer hardware, e.g., unrecognized
>> new hardware won't enable SEV-SNP because bsp_determine_snp() is skipped.
> 
> bsp_determine_snp() relies partly on the family detection.
> 
> If all you care is calling it even without family detection, why don't you
> pull it up, check c->x86 and be done with it?

ZEN3 and ZEN4 won't be set if we do that and since those families don't
have RMPREAD support, SNP won't get enabled on ZEN3 or ZEN4, so we can't
do that.

And, additionally, now there is tsa_init() and the CPUID_FAULT checks
that will be skipped, too.

> 
> But even then: we don't really care about older kernels on newer hardware
> - that's why they get backports. We don't really think about backporting when
> working upstream. Frankly...

Hmmm... i guess? Not sure I 100% agree with that.

> 
> So I'm sceptical about this.

Your call.

Thanks,
Tom

>

Re: [PATCH] x86/CPU/AMD: Perform function calls post ZEN feature check regardless

Posted by Borislav Petkov 1 month, 3 weeks ago

On Tue, Aug 12, 2025 at 04:56:29PM -0500, Tom Lendacky wrote:
> ZEN3 and ZEN4 won't be set if we do that and since those families don't
> have RMPREAD support, SNP won't get enabled on ZEN3 or ZEN4, so we can't
> do that.

That's why I said partly.

> Hmmm... i guess? Not sure I 100% agree with that.

Look at it this way: you can't really run old kernels on new hardware without
doing at least *some* backporting. So might as well do the family extension
patch too.

Let's worry about this when it really happens. But for a decade at SUSE I've
never even once thought: oh, this new machine support won't need backports.
Because it does every time and this is business as usual.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] x86/CPU/AMD: Perform function calls post ZEN feature check regardless

Posted by Tom Lendacky 2 months, 2 weeks ago

On 7/23/25 12:01, Tom Lendacky wrote:
> Currently, if a ZEN family/model is not recognized in bsp_init_amd(), then
> function calls after the family/model check are skipped. This can prevent
> older kernels from enabling features on newer hardware, e.g., unrecognized
> new hardware won't enable SEV-SNP because bsp_determine_snp() is skipped.
> 
> Rearrange the way the WARN_ONCE() is issued so that functions called after
> the family/model check are still called.
> 
> Fixes: 216d106c7ff7 ("x86/sev: Add SEV-SNP host initialization support")

I put a Fixes: tag, but I'm not sure if this should go to stable. Given
that tsa_init() has been backported, it might be?

Thanks,
Tom

> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
>  arch/x86/kernel/cpu/amd.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
> index a5ece6ebe8a7..79ea6a0fcf8a 100644
> --- a/arch/x86/kernel/cpu/amd.c
> +++ b/arch/x86/kernel/cpu/amd.c
> @@ -420,6 +420,8 @@ static void tsa_init(struct cpuinfo_x86 *c)
>  
>  static void bsp_init_amd(struct cpuinfo_x86 *c)
>  {
> +	bool model_warn = false;
> +
>  	if (cpu_has(c, X86_FEATURE_CONSTANT_TSC)) {
>  
>  		if (c->x86 > 0x10 ||
> @@ -489,7 +491,7 @@ static void bsp_init_amd(struct cpuinfo_x86 *c)
>  			setup_force_cpu_cap(X86_FEATURE_ZEN2);
>  			break;
>  		default:
> -			goto warn;
> +			model_warn = true;
>  		}
>  		break;
>  
> @@ -504,7 +506,7 @@ static void bsp_init_amd(struct cpuinfo_x86 *c)
>  			setup_force_cpu_cap(X86_FEATURE_ZEN4);
>  			break;
>  		default:
> -			goto warn;
> +			model_warn = true;
>  		}
>  		break;
>  
> @@ -521,7 +523,7 @@ static void bsp_init_amd(struct cpuinfo_x86 *c)
>  			setup_force_cpu_cap(X86_FEATURE_ZEN6);
>  			break;
>  		default:
> -			goto warn;
> +			model_warn = true;
>  		}
>  		break;
>  
> @@ -529,16 +531,13 @@ static void bsp_init_amd(struct cpuinfo_x86 *c)
>  		break;
>  	}
>  
> +	WARN_ONCE(model_warn, "Family 0x%x, model: 0x%x??\n", c->x86, c->x86_model);
> +
>  	bsp_determine_snp(c);
>  	tsa_init(c);
>  
>  	if (cpu_has(c, X86_FEATURE_GP_ON_USER_CPUID))
>  		setup_force_cpu_cap(X86_FEATURE_CPUID_FAULT);
> -
> -	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)
> 
> base-commit: 804fa10c3a3c8a33ce6ffdb999f94d3a7a6c20b8