Re: [Xen-devel] [PATCH v9 04/15] microcode: introduce a global cache of ucode patch

[Jan Beulich <jbeulich@xxxxxxxx>]

On 19.08.2019 03:25, Chao Gao wrote:
> +/* Return true if cache gets updated. Otherwise, return false */
> +bool microcode_update_cache(struct microcode_patch *patch)
> +{
> +
> +    ASSERT(spin_is_locked(&microcode_mutex));

Stray blank line ahead of this one.

> --- a/xen/arch/x86/microcode_intel.c
> +++ b/xen/arch/x86/microcode_intel.c
> @@ -259,6 +259,31 @@ static int microcode_sanity_check(void *mc)
>      return 0;
>  }
>  
> +static bool match_cpu(const struct microcode_patch *patch)
> +{
> +    if ( !patch )
> +        return false;
> +
> +    return microcode_update_match(&patch->mc_intel->hdr,
> +                                  smp_processor_id()) == NEW_UCODE;
> +}
> +
> +static void free_patch(void *mc)
> +{
> +    xfree(mc);
> +}
> +
> +/*
> + * Both patches to compare are supposed to be applicable to local CPU.
> + * Just compare the revision number.
> + */
> +static enum microcode_match_result compare_patch(
> +    const struct microcode_patch *new, const struct microcode_patch *old)
> +{
> +    return (new->mc_intel->hdr.rev > old->mc_intel->hdr.rev) ?  NEW_UCODE :
> +                                                                OLD_UCODE;
> +}

The comment ahead of the function is nice, but please move it
inside the function and accompany it by ASSERT()s checking what
the comment says.

> @@ -19,6 +27,11 @@ struct microcode_ops {
>      int (*collect_cpu_info)(unsigned int cpu, struct cpu_signature *csig);
>      int (*apply_microcode)(unsigned int cpu);
>      int (*start_update)(void);
> +    void (*free_patch)(void *mc);
> +    bool (*match_cpu)(const struct microcode_patch *patch);
> +    enum microcode_match_result (*compare_patch)(
> +            const struct microcode_patch *new,
> +            const struct microcode_patch *old);

I'm afraid indentation here doesn't really match our (sadly still
unwritten) conventions - it should be four more spaces than what
the previous (incomplete) line had.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel