Re: [PATCH for-4.15] x86/ucode: Fix microcode payload size for Fam19 processors

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 10/02/2021 08:37, Jan Beulich wrote:
> On 09.02.2021 18:39, Andrew Cooper wrote:
>> On 09/02/2021 17:17, Ian Jackson wrote:
>>> Jan Beulich writes ("Re: [PATCH for-4.15] x86/ucode: Fix microcode payload 
>>> size for Fam19 processors"):
>>>> On 09.02.2021 16:33, Andrew Cooper wrote:
>>>>> The original limit provided wasn't accurate.  Blobs are in fact rather 
>>>>> larger.
>>>>>
>>>>> Fixes: fe36a173d1 ("x86/amd: Initial support for Fam19h processors")
>>>>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>>>> Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
>>>>
>>>>> --- a/xen/arch/x86/cpu/microcode/amd.c
>>>>> +++ b/xen/arch/x86/cpu/microcode/amd.c
>>>>> @@ -111,7 +111,7 @@ static bool verify_patch_size(uint32_t patch_size)
>>>>>  #define F15H_MPB_MAX_SIZE 4096
>>>>>  #define F16H_MPB_MAX_SIZE 3458
>>>>>  #define F17H_MPB_MAX_SIZE 3200
>>>>> -#define F19H_MPB_MAX_SIZE 4800
>>>>> +#define F19H_MPB_MAX_SIZE 5568
>>>> How certain is it that there's not going to be another increase?
>>>> And in comparison, how bad would it be if we pulled this upper
>>>> limit to something that's at least slightly less of an "odd"
>>>> number, e.g. 0x1800, and thus provide some headroom?
>>> 5568 does seem really an excessively magic number...
>> It reads better in hex - 0x15c0.  It is exactly the header,
>> match/patch-ram, and the digital signature of a fixed algorithm.
> And I realize it's less odd than Fam16's 3458 (0xd82).

This particular value is under investigation.  Firmware packages for
Fam16's have the blob size at 0xd60.

>> Its far simpler than Intel's format which contains multiple embedded
>> blobs, and support for minor platform variations within the same blob.
>>
>> There are a lot of problems with how we do patch verification on AMD
>> right now, but its all a consequence of the header not containing a
>> length field.
>>
>> This number wont change now.  Zen3 processors are out in the world.
> Given history on earlier families, where in some cases sizes
> also vary by model, how can this fact guarantee anything?

There is one example where it changed, and it got shorter.  Making it
larger would involve someone re-laying out the core so more silicon area
could be used for patch ram space, which is increasingly unlikely with
newer models in the same family.

When 4.16 comes, I do have plans to try and make us more robust to
changes in debug builds, particularly given the lack of suitable
documentation on the matter.

~Andrew