Re: [PATCH v5 4/4] x86/ucode: Utilize ucode_force and remove opt_ucode_allow_same

[Jan Beulich <jbeulich@suse.com>]

On 12.07.2024 15:07, Fouad Hilly wrote:
> --- a/xen/arch/x86/cpu/microcode/core.c
> +++ b/xen/arch/x86/cpu/microcode/core.c
> @@ -90,6 +90,16 @@ struct ucode_mod_blob {
>      size_t size;
>  };
>  
> +struct microcode_patch_with_flags {
> +    unsigned int flags;
> +    struct microcode_patch *patch;
> +};
> +
> +struct microcode_nmi_patch_with_flags {
> +    unsigned int flags;
> +    const struct microcode_patch *patch;
> +};

Why two different structures? I have to admit that I can't spot where the
difference (const or not) would matter?

Also for an internal struct I don't think you need the microcode_ prefixes.

> @@ -284,21 +286,22 @@ static enum microcode_match_result cf_check compare_patch(
>      return compare_revisions(old->rev, new->rev);
>  }
>  
> -static int cf_check apply_microcode(const struct microcode_patch *patch)
> +static int cf_check apply_microcode(const struct microcode_patch *patch,
> +                                    unsigned int flags)
>  {
>      uint64_t msr_content;
>      unsigned int cpu = smp_processor_id();
>      struct cpu_signature *sig = &this_cpu(cpu_sig);
>      uint32_t rev, old_rev = sig->rev;
>      enum microcode_match_result result;
> +    bool ucode_force = flags == XENPF_UCODE_FORCE;

Why == ? The term "flags" usually stands for there being multiple boolean
indicators in a single value. That would demand use of & here.

Jan