Re: [PATCH v2 3/3] x86/ucode: Drop MIS_UCODE and microcode_match_result

[Jan Beulich <jbeulich@xxxxxxxx>]

On 12.11.2024 22:19, Andrew Cooper wrote:
> @@ -199,8 +198,8 @@ static bool microcode_fits_cpu(const struct 
> microcode_patch *patch)
>      return equiv.id == patch->processor_rev_id;
>  }
>  
> -static enum microcode_match_result cf_check compare_patch(
> -    const struct microcode_patch *new, const struct microcode_patch *old)
> +static int cf_check compare_patch(
> +    const struct microcode_patch *old, const struct microcode_patch *new)
>  {

Let's hope we won't screw up a backport because of this swapping. I'd
like to ask to at least consider renaming at least the functions,
perhaps also the hook pointer, perhaps simply by switching from singular
to plural. This would then also avoid reviewers like me to go hunt for
all uses of the function/hook, in an attempt to make sure none was left
out when converting.

> @@ -54,11 +47,17 @@ struct microcode_ops {
>                             unsigned int flags);
>  
>      /*
> -     * Given two patches, are they both applicable to the current CPU, and is
> -     * new a higher revision than old?
> +     * Given a current patch, and a proposed new patch, order them based on 
> revision.
> +     *
> +     * This operation is not necessarily symmetrical.  In some cases, a debug
> +     * "new" patch will always considered to be newer, on the expectation 
> that
> +     * whomever is using debug patches knows exactly what they're doing.
>       */
> -    enum microcode_match_result (*compare_patch)(
> -        const struct microcode_patch *new, const struct microcode_patch 
> *old);
> +#define OLD_UCODE  -1

Nit: I'm pretty sure Misra wants parentheses here.

Preferably with both (mechanical) adjustments:
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan