Re: [Xen-devel] [PATCH 3/7] x86/ucode/intel: Remove gratuitous memory allocations from cpu_request_microcode()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 23.03.2020 11:17, Andrew Cooper wrote:
>  static struct microcode_patch *cpu_request_microcode(const void *buf,
>                                                       size_t size)
>  {
> -    long offset = 0;
>      int error = 0;
> -    struct microcode_intel *mc, *saved = NULL;
> +    const struct microcode_intel *saved = NULL;
>      struct microcode_patch *patch = NULL;
>  
> -    while ( (offset = get_next_ucode_from_buffer(&mc, buf, size, offset)) > 
> 0 )
> +    while ( size )
>      {
> -        error = microcode_sanity_check(mc);
> -        if ( error )
> +        const struct microcode_intel *mc;
> +        unsigned int blob_size;
> +
> +        if ( size < MC_HEADER_SIZE ||       /* Insufficient space for 
> header? */
> +             (mc = buf)->hdr.hdrver != 1 || /* Unrecognised header version?  
>  */
> +             mc->hdr.ldrver != 1 ||         /* Unrecognised loader version?  
>  */

These two duplicate what microcode_sanity_check() does,
plus the function issues a log message when either check
fails. Since I think ...

> +             size < (blob_size =            /* Insufficient space for patch? 
>  */
> +                     get_totalsize(&mc->hdr)) )

... this wants to come after the two version checks, how
about lifting the printk() here? The other user of
microcode_sanity_check() is an ASSERT(), where it's rather
less relevant to have such a printk().

Jan