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

[Jan Beulich <jbeulich@xxxxxxxx>]

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 <jbeulich@xxxxxxxx>
>>
>> --- 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 <andrew.cooper3@xxxxxxxxxx>

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