x86/AMD: extend Zenbleed check to models "good" ucode isn't known for

Commit Message

Jan Beulich Aug. 22, 2023, 2:22 p.m. UTC

Reportedly the AMD Custom APU 0405 found on SteamDeck, models 0x90 and
0x91, (quoting the respective Linux commit) is similarly affected. Put
another instance of our Zen1 vs Zen2 distinction checks in
amd_check_zenbleed(), forcing use of the chickenbit irrespective of
ucode version (building upon real hardware never surfacing a version of
0xffffffff).

Signed-off-by: Jan Beulich <[e-mail]>

Comments

Andrew Cooper Aug. 22, 2023, 2:53 p.m. UTC | #1

On 22/08/2023 3:22 pm, Jan Beulich wrote:
> Reportedly the AMD Custom APU 0405 found on SteamDeck, models 0x90 and
> 0x91, (quoting the respective Linux commit) is similarly affected. Put
> another instance of our Zen1 vs Zen2 distinction checks in
> amd_check_zenbleed(), forcing use of the chickenbit irrespective of
> ucode version (building upon real hardware never surfacing a version of
> 0xffffffff).
>
> Signed-off-by: Jan Beulich <[e-mail]>
>
> --- a/xen/arch/x86/cpu/amd.c
> +++ b/xen/arch/x86/cpu/amd.c
> @@ -936,10 +936,14 @@ void amd_check_zenbleed(void)
>  	case 0xa0 ... 0xaf: good_rev = 0x08a00008; break;
>  	default:
>  		/*
> -		 * With the Fam17h check above, parts getting here are Zen1.
> -		 * They're not affected.
> +		 * With the Fam17h check above, most parts getting here are
> +		 * Zen1.  They're not affected.  Assume Zen2 ones making it
> +		 * here are affected regardless of microcode version.

It's not really "assume Zen2 are vulnerable".  All Zen2 *are*
vulnerable, but we keep on finding new CPUs that AMD did for special
circumstances and haven't documented in their model lists.

Furthermore, there needs to be another sentence:

"Because we still don't have an correct authoritative list of Zen1 vs
Zen2 by model number, use STIBP as a heuristic to distinguish."

Or something like this.  It is important to state that STIBP is our
model-heuristic here.

With some kind of note explaining what's going on, Reviewed-by: Andrew
Cooper <[e-mail]>

>  		 */
> -		return;
> +		if (!boot_cpu_has(X86_FEATURE_AMD_STIBP))
> +			return;
> +		good_rev = ~0u;

While I hate to review like this, someone is going to come along and
swap this u for U for MISRA reasons.  Probably best to adjust it now.

~Andrew

> +		break;
>  	}
>  
>  	rdmsrl(MSR_AMD64_DE_CFG, val);

Jan Beulich Aug. 22, 2023, 3 p.m. UTC | #2

On 22.08.2023 16:53, Andrew Cooper wrote:
> On 22/08/2023 3:22 pm, Jan Beulich wrote:
>> Reportedly the AMD Custom APU 0405 found on SteamDeck, models 0x90 and
>> 0x91, (quoting the respective Linux commit) is similarly affected. Put
>> another instance of our Zen1 vs Zen2 distinction checks in
>> amd_check_zenbleed(), forcing use of the chickenbit irrespective of
>> ucode version (building upon real hardware never surfacing a version of
>> 0xffffffff).
>>
>> Signed-off-by: Jan Beulich <[e-mail]>
>>
>> --- a/xen/arch/x86/cpu/amd.c
>> +++ b/xen/arch/x86/cpu/amd.c
>> @@ -936,10 +936,14 @@ void amd_check_zenbleed(void)
>>  	case 0xa0 ... 0xaf: good_rev = 0x08a00008; break;
>>  	default:
>>  		/*
>> -		 * With the Fam17h check above, parts getting here are Zen1.
>> -		 * They're not affected.
>> +		 * With the Fam17h check above, most parts getting here are
>> +		 * Zen1.  They're not affected.  Assume Zen2 ones making it
>> +		 * here are affected regardless of microcode version.
> 
> It's not really "assume Zen2 are vulnerable".

But that's also not what the comment says. It says "regardless of
microcode version".

>  All Zen2 *are*
> vulnerable, but we keep on finding new CPUs that AMD did for special
> circumstances and haven't documented in their model lists.
> 
> Furthermore, there needs to be another sentence:
> 
> "Because we still don't have an correct authoritative list of Zen1 vs
> Zen2 by model number, use STIBP as a heuristic to distinguish."
> 
> Or something like this.  It is important to state that STIBP is our
> model-heuristic here.

Will add.

> With some kind of note explaining what's going on, Reviewed-by: Andrew
> Cooper <[e-mail]>

Thanks.

>>  		 */
>> -		return;
>> +		if (!boot_cpu_has(X86_FEATURE_AMD_STIBP))
>> +			return;
>> +		good_rev = ~0u;
> 
> While I hate to review like this, someone is going to come along and
> swap this u for U for MISRA reasons.  Probably best to adjust it now.

Oh, right, will do.

Jan

Patch

--- a/xen/arch/x86/cpu/amd.c
+++ b/xen/arch/x86/cpu/amd.c
@@ -936,10 +936,14 @@  void amd_check_zenbleed(void)
 	case 0xa0 ... 0xaf: good_rev = 0x08a00008; break;
 	default:
 		/*
-		 * With the Fam17h check above, parts getting here are Zen1.
-		 * They're not affected.
+		 * With the Fam17h check above, most parts getting here are
+		 * Zen1.  They're not affected.  Assume Zen2 ones making it
+		 * here are affected regardless of microcode version.
 		 */
-		return;
+		if (!boot_cpu_has(X86_FEATURE_AMD_STIBP))
+			return;
+		good_rev = ~0u;
+		break;
 	}
 
 	rdmsrl(MSR_AMD64_DE_CFG, val);