Re: [patch V3 30/30] x86/microcode/intel: Add a minimum required revision for late-loads

[Qiuxu Zhuo]

> ...
> From: Ashok Raj <ashok.raj@xxxxxxxxx>
> 
> In general users don't have the necessary information to determine whether
> late loading of a new microcode version is safe and does not modify
> anything which the currently running kernel uses already, e.g. removal of
> CPUID bits or behavioural changes of MSRs.
> ...
> 
> The check is always enabled, but by default not enforced. It can be
> enforced via Kconfig or kernel command line.
> 
> If enforced, the kernel refuses to late load microcode with a minium

s/minium/minimum/

> required version field which is zero or when the currently loaded microcode
> revision is smaller than the minimum required revision.
> 
> ...
> --- a/arch/x86/kernel/cpu/microcode/intel.c
> +++ b/arch/x86/kernel/cpu/microcode/intel.c
> @@ -463,16 +463,40 @@ static enum ucode_state apply_microcode_
>  	return ret;
>  }
>  
> +static bool ucode_validate_minrev(struct microcode_header_intel *mc_header)
> +{
> +	int cur_rev = boot_cpu_data.microcode;
> +
> +	/*
> +	 * When late-loading, ensure the header declares a minimum revision
> +	 * required to perform a late-load. The previously reserved field
> +	 * is 0 in older microcode blobs.
> +	 */
> +	if (!mc_header->min_req_ver) {
> +		pr_info("Unsafe microcode update: Microcode header does not specify a required min version\n");
> +		return false;
> +	}
> +
> +	/*
> +	 * Check whether the minimum revision specified in the header is either
> +	 * greater or equal to the current revision.
> +	 */

Seems like the above comment doesn't match the following 'if' check.
Perhaps the comment is:

   "Check whether the current revision is either greater or
    equal to the minimum revision specified in the header."

> +	if (cur_rev < mc_header->min_req_ver) {
> +		pr_info("Unsafe microcode update: Current revision 0x%x too old\n", cur_rev);
> +		pr_info("Current should be at 0x%x or higher. Use early loading instead\n", mc_header->min_req_ver);
> +		return false;
> +	}
> +	return true;
> +}
> ...