Re: [PATCH v2 06/14] x86/microcode/intel: Expose microcode_sanity_check()

Joseph, Jithu
2022-11-11T21:39:43Z

On 11/11/2022 6:33 AM, Borislav Petkov wrote:
> On Mon, Nov 07, 2022 at 02:53:15PM -0800, Jithu Joseph wrote:
>> IFS test image carries the same microcode header as regular Intel
>> microcode blobs. Microcode blobs use header version of 1,
>> whereas IFS test images will use header version of 2.
>>
>> microcode_sanity_check() can be used by IFS driver to perform
>> sanity check of the IFS test images too.
>>
>> Refactor header version as a parameter, move it to cpu/intel.c
>> and expose this function. Qualify the function name with intel.
>
> Same comments as before.

Will drop the last para

>> +#define MICROCODE_HEADER_VER_UCODE 1
>
> "MICROCODE" ... "UCODE" - too much.
>
> And "header version" sounds wrong when all you wanna say is, this header
> is of this or that *type*. So you simply do:
>
> #define MC_HEADER_TYPE_MICROCODE 1
> #define MC_HEADER_TYPE_IFS 2
>
> and that's it.

Will modify

>
>> #define get_totalsize(mc) \
>> (((struct microcode_intel *)mc)->hdr.datasize ? \
>> diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
>> index b6f9210fb31a..f8a5a25ab502 100644
>> --- a/arch/x86/kernel/cpu/intel.c
>> +++ b/arch/x86/kernel/cpu/intel.c
>> @@ -245,6 +245,106 @@ int intel_find_matching_signature(void *mc, unsigned int csig, int cpf)
>> }
>> EXPORT_SYMBOL_GPL(intel_find_matching_signature);
>>
>> +int intel_microcode_sanity_check(void *mc, bool print_err, int hdr_ver)
>
> This is not how this is done:
>
> 1st patch: *only* mechanical code move, no semantic or functional changes
> whatsoever.
>
> 2nd patch: Add semantical/functional changes.
>
> Also, in the second patch, pls put a kernel-doc comment over the
> function to explain what hdr_type - not hdr_ver - means here.

Thanks for the suggestion above, will split to 2 patches

Jithu