This particular variant has an error that causes RDSEED to always return
0xffffffff, while RDRAND works correctly.

Inspired by Linux commit 5b937a1ed64ebeba8876e398110a5790ad77407c
("x86/rdrand: Disable RDSEED on AMD Cyan Skillfish").

Like for RDRAND, permit a command line override to be used to keep
RDSEED enabled.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Considering how it is described, I didn't think probing RDSEED (like we
do for RDRAND) would be necessary.

Am I going too far in also updating cpuidmask_defaults here, or is us
not doing so for the RDRAND disabling actually an oversight?

Using warning_add() may not be quite appropriate, as we don't really
mean the admin to possibly override this with "cpuid=rdseed" (that's
only a last resort, in case the issue is yet more limited in scope). But
mere printk() would feel like hiding the information in the middle of
lots of other output.

--- a/xen/arch/x86/cpu-policy.c
+++ b/xen/arch/x86/cpu-policy.c
@@ -123,6 +123,10 @@ static void __init cf_check _parse_xen_c
     else if ( feat == X86_FEATURE_RDRAND &&
               (cpuid_ecx(1) & cpufeat_mask(X86_FEATURE_RDRAND)) )
         setup_force_cpu_cap(X86_FEATURE_RDRAND);
+    else if ( feat == X86_FEATURE_RDSEED &&
+              cpuid_eax(0) >= 7 &&
+              (cpuid_count_ebx(7, 0) & cpufeat_mask(X86_FEATURE_RDSEED)) )
+        setup_force_cpu_cap(X86_FEATURE_RDSEED);
 }
 
 static int __init cf_check parse_xen_cpuid(const char *s)
--- a/xen/arch/x86/cpu/amd.c
+++ b/xen/arch/x86/cpu/amd.c
@@ -1220,6 +1220,24 @@ static void cf_check init_amd(struct cpu
 		}
 		break;
 
+	case 0x17:
+		/*
+		 * Fam17 model 47 stepping 0 has an error that causes RDSEED to
+		 * always return 0xffffffff (while RDRAND works correctly).
+		 */
+		if (c == &boot_cpu_data &&
+		    c->family == 0x47 && c->stepping == 0 &&
+		    cpu_has(c, X86_FEATURE_RDSEED) &&
+		    !is_forced_cpu_cap(X86_FEATURE_RDSEED)) {
+			static const char __initconst text[] =
+				"RDSEED is unreliable on this hardware; disabling its exposure\n";
+
+			setup_clear_cpu_cap(X86_FEATURE_RDSEED);
+			cpuidmask_defaults._7ab0 &= ~cpufeat_mask(X86_FEATURE_RDSEED);
+			warning_add(text);
+		}
+		break;
+
 	case 0x19:
 		/*
 		 * Zen3 (Fam19h model < 0x10) parts are not susceptible to

On 24.07.2025 14:56, Jan Beulich wrote:
> --- a/xen/arch/x86/cpu/amd.c
> +++ b/xen/arch/x86/cpu/amd.c
> @@ -1220,6 +1220,24 @@ static void cf_check init_amd(struct cpu
>  		}
>  		break;
>  
> +	case 0x17:
> +		/*
> +		 * Fam17 model 47 stepping 0 has an error that causes RDSEED to
> +		 * always return 0xffffffff (while RDRAND works correctly).
> +		 */
> +		if (c == &boot_cpu_data &&
> +		    c->family == 0x47 && c->stepping == 0 &&

Of course this means to be c->model.

Jan

On 24.07.2025 14:56, Jan Beulich wrote:
> This particular variant has an error that causes RDSEED to always return
> 0xffffffff, while RDRAND works correctly.
> 
> Inspired by Linux commit 5b937a1ed64ebeba8876e398110a5790ad77407c
> ("x86/rdrand: Disable RDSEED on AMD Cyan Skillfish").
> 
> Like for RDRAND, permit a command line override to be used to keep
> RDSEED enabled.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> Considering how it is described, I didn't think probing RDSEED (like we
> do for RDRAND) would be necessary.
> 
> Am I going too far in also updating cpuidmask_defaults here, or is us
> not doing so for the RDRAND disabling actually an oversight?
> 
> Using warning_add() may not be quite appropriate, as we don't really
> mean the admin to possibly override this with "cpuid=rdseed" (that's
> only a last resort, in case the issue is yet more limited in scope). But
> mere printk() would feel like hiding the information in the middle of
> lots of other output.

And, other than presumably intended judging from the RDRAND logic, passing
"cpuid=no-rdseed" doesn't prevent the warning. That's - aiui - because the
clearing of forced-cleared features happens only afterwards. Apparently we
would need yet another predicate is_cleared_cpu_cap(). Or have
is_forced_cpu_cap() check both forced_caps[] and cleared_caps[]. (As per
present uses doing the latter would be okay, but I'm not sure whether
checking both in the same predicate would be a good idea going forward.)

Jan