Re: [PATCH v2 5/5] x86/ucode: Relax digest check when Entrysign is fixed in firmware

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 28/10/2025 9:47 am, Jan Beulich wrote:
> On 27.10.2025 23:17, Andrew Cooper wrote:
>> When Entrysign has been mitigated in firwmare, it is believed to be safe to
>> rely on the CPU patchloader again.  This avoids us needing to maintain the
>> digest table for all new microcode indefinitely.
>>
>> Relax the digest check when firmware looks to be up to date, and leave behind
>> a clear message when not.
>>
>> This is best-effort only.  If a malicious microcode has been loaded prior to
>> Xen running, then all bets are off.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Like for patch 4, adjustments for Zen6 are then going to be needed here too,
> aiui. May be worth repeating that statement here.

Ok.

>
>> @@ -603,3 +604,82 @@ static void __init __constructor 
>> test_digests_sorted(void)
>>      }
>>  }
>>  #endif /* CONFIG_SELF_TESTS */
>> +
>> +/*
>> + * The Entrysign vulnerability affects all Zen1 thru Zen5 CPUs.  Firmware
>> + * fixes were produced from Nov 2024.  Zen3 thru Zen5 can continue to take
>> + * OS-loadable microcode updates using a new signature scheme, as long as
>> + * firmware has been updated first.
>> + */
>> +void __init amd_check_entrysign(void)
>> +{
>> +    unsigned int curr_rev;
>> +    uint8_t fixed_rev;
>> +
>> +    if ( boot_cpu_data.vendor != X86_VENDOR_AMD ||
> Given the function name, might this check better live at the call site?

Possibly, but I really don't want to split the vendor check away from
the family ranges.

A family check without a vendor check in eyeshot is
suspicious-going-on-buggy, and this is called once at init.

>
>> +         boot_cpu_data.family < 0x17 ||
>> +         boot_cpu_data.family > 0x1a )
>> +        return;
>> +
>> +    /*
>> +     * Table taken from Linux, which is the only known source of information
>> +     * about client revisions.  Note, Linux expresses "last-vulnerable-rev"
>> +     * while Xen wants "first-fixed-rev".
>> +     */
>> +    curr_rev = this_cpu(cpu_sig).rev;
>> +    switch ( curr_rev >> 8 )
>> +    {
>> +    case 0x080012: fixed_rev = 0x78; break;
>> +    case 0x080082: fixed_rev = 0x10; break;
>> +    case 0x083010: fixed_rev = 0x7d; break;
>> +    case 0x086001: fixed_rev = 0x0f; break;
>> +    case 0x086081: fixed_rev = 0x09; break;
>> +    case 0x087010: fixed_rev = 0x35; break;
>> +    case 0x08a000: fixed_rev = 0x0b; break;
>> +    case 0x0a0010: fixed_rev = 0x7b; break;
>> +    case 0x0a0011: fixed_rev = 0xdb; break;
>> +    case 0x0a0012: fixed_rev = 0x44; break;
>> +    case 0x0a0082: fixed_rev = 0x0f; break;
>> +    case 0x0a1011: fixed_rev = 0x54; break;
>> +    case 0x0a1012: fixed_rev = 0x4f; break;
>> +    case 0x0a1081: fixed_rev = 0x0a; break;
>> +    case 0x0a2010: fixed_rev = 0x30; break;
>> +    case 0x0a2012: fixed_rev = 0x13; break;
>> +    case 0x0a4041: fixed_rev = 0x0a; break;
>> +    case 0x0a5000: fixed_rev = 0x14; break;
>> +    case 0x0a6012: fixed_rev = 0x0b; break;
>> +    case 0x0a7041: fixed_rev = 0x0a; break;
>> +    case 0x0a7052: fixed_rev = 0x09; break;
>> +    case 0x0a7080: fixed_rev = 0x0a; break;
>> +    case 0x0a70c0: fixed_rev = 0x0a; break;
>> +    case 0x0aa001: fixed_rev = 0x17; break;
>> +    case 0x0aa002: fixed_rev = 0x19; break;
>> +    case 0x0b0021: fixed_rev = 0x47; break;
>> +    case 0x0b1010: fixed_rev = 0x47; break;
>> +    case 0x0b2040: fixed_rev = 0x32; break;
>> +    case 0x0b4040: fixed_rev = 0x32; break;
>> +    case 0x0b6000: fixed_rev = 0x32; break;
>> +    case 0x0b7000: fixed_rev = 0x32; break;
> Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
> (after cross checking with up-to-date Linux, i.e. including your recent
> correction there)

Thanks.

~Andrew