Re: [Xen-devel] [PATCH v9 12/15] microcode: reduce memory allocation and copy when creating a patch

[Jan Beulich <jbeulich@xxxxxxxx>]

On 19.08.2019 03:25, Chao Gao wrote:
> @@ -542,29 +505,21 @@ static struct microcode_patch 
> *cpu_request_microcode(const void *buf,
>      while ( (error = get_ucode_from_buffer_amd(mc_amd, buf, bufsize,
>                                                 &offset)) == 0 )
>      {
> -        struct microcode_patch *new_patch = alloc_microcode_patch(mc_amd);
> -
> -        if ( IS_ERR(new_patch) )
> -        {
> -            error = PTR_ERR(new_patch);
> -            break;
> -        }
> -
>          /*
> -         * If the new patch covers current CPU, compare patches and store the
> +         * If the new ucode covers current CPU, compare ucodes and store the
>           * one with higher revision.
>           */
> -        if ( (microcode_fits(new_patch->mc_amd) != MIS_UCODE) &&
> -             (!patch || (compare_patch(new_patch, patch) == NEW_UCODE)) )
> +#define REV_ID(mpb) (((struct microcode_header_amd 
> *)(mpb))->processor_rev_id)
> +        if ( (microcode_fits(mc_amd) != MIS_UCODE) &&
> +             (!saved || (REV_ID(mc_amd->mpb) > REV_ID(saved))) )
> +#undef REV_ID

I'm not happy with this helper #define, the more that "saved" already is
of the correct type. compare_patch() in reality only acts on the header,
so I'd suggest having that function forward to a new compare_header()
(or some other suitable name) and use that new function here as well.

> @@ -379,47 +360,47 @@ static struct microcode_patch 
> *cpu_request_microcode(const void *buf,
>  {
>      long offset = 0;
>      int error = 0;
> -    void *mc;
> +    struct microcode_intel *mc, *saved = NULL;
>      struct microcode_patch *patch = NULL;
>  
> -    while ( (offset = get_next_ucode_from_buffer(&mc, buf, size, offset)) > 
> 0 )
> +    while ( (offset = get_next_ucode_from_buffer((void **)&mc, buf,

Casts like this make me rather nervous. Please see about getting rid of
it (by using a union or a 2nd local variable).

Jan

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