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
