Re: [Xen-devel] [PATCH 7/7] x86/ucode/intel: Fold structures together

[Jan Beulich <jbeulich@xxxxxxxx>]

On 23.03.2020 11:17, Andrew Cooper wrote:
> Currently, we allocate an 8 byte struct microcode_patch to point at a
> separately allocated struct microcode_intel.  This is wasteful.

As indicated elsewhere I'm very much in favor of this, but I think it
wants doing in one of the earlier series, and then for AMD at the same
time. Possibly, to limit code churn there, ...

> --- a/xen/arch/x86/cpu/microcode/intel.c
> +++ b/xen/arch/x86/cpu/microcode/intel.c
> @@ -32,17 +32,12 @@
>  
>  #define pr_debug(x...) ((void)0)
>  
> -struct microcode_header_intel {
> +struct microcode_patch {

... accompanying this with

#define microcode_header_intel microcode_patch

or even ...

> -    union {
> -        struct {
> -            uint16_t year;
> -            uint8_t day;
> -            uint8_t month;
> -        };
> -        unsigned int date;
> -    };
> +    uint16_t year;
> +    uint8_t  day;
> +    uint8_t  month;
>      unsigned int sig;
>      unsigned int cksum;
>      unsigned int ldrver;
> @@ -57,10 +52,7 @@ struct microcode_header_intel {
>      unsigned int _datasize;
>      unsigned int _totalsize;
>      unsigned int reserved[3];
> -};
>  
> -struct microcode_intel {
> -    struct microcode_header_intel hdr;
>      uint8_t data[];
>  };

... keeping the two structures separate until here, which would
make this one what would initially become struct microcode_patch.
This is in particular because ...

>  static void free_patch(struct microcode_patch *patch)
>  {
> -    if ( patch )
> -    {
> -        xfree(patch->mc_intel);
> -        xfree(patch);
> -    }
> +    xfree(patch);
>  }

... in that earlier series you've moved the 2nd xfree() here just
to now delete it again.

Jan