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
