| Message ID | 20201007180120.27203-1-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/ucode: Trivial further cleanup | expand |
On Wed, Oct 07, 2020 at 07:01:20PM +0100, Andrew Cooper wrote: > * Drop unused include in private.h. > * Used explicit width integers for Intel header fields. > * Adjust comment to better describe the extended header. > * Drop unnecessary __packed attribute for AMD header. > * Switch mc_patch_data_id to being uint16_t, which is how it is more commonly > referred to. > * Fix types and style. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Roger Pau Monné <roger.pau@citrix.com> > --- > CC: Jan Beulich <JBeulich@suse.com> > CC: Roger Pau Monné <roger.pau@citrix.com> > CC: Wei Liu <wl@xen.org> > --- > xen/arch/x86/cpu/microcode/amd.c | 10 +++++----- > xen/arch/x86/cpu/microcode/intel.c | 34 +++++++++++++++++----------------- > xen/arch/x86/cpu/microcode/private.h | 2 -- > 3 files changed, 22 insertions(+), 24 deletions(-) > > diff --git a/xen/arch/x86/cpu/microcode/amd.c b/xen/arch/x86/cpu/microcode/amd.c > index cd532321e8..e913232067 100644 > --- a/xen/arch/x86/cpu/microcode/amd.c > +++ b/xen/arch/x86/cpu/microcode/amd.c > @@ -24,7 +24,7 @@ > > #define pr_debug(x...) ((void)0) > > -struct __packed equiv_cpu_entry { > +struct equiv_cpu_entry { > uint32_t installed_cpu; > uint32_t fixed_errata_mask; > uint32_t fixed_errata_compare; > @@ -35,7 +35,7 @@ struct __packed equiv_cpu_entry { > struct microcode_patch { > uint32_t data_code; > uint32_t patch_id; > - uint8_t mc_patch_data_id[2]; > + uint16_t mc_patch_data_id; > uint8_t mc_patch_data_len; I think you could also drop the mc_patch_ prefixes from a couple of fields in this structure, since they serve no purpose AFAICT. Thanks, Roger.
On 08/10/2020 08:49, Roger Pau Monné wrote: > On Wed, Oct 07, 2020 at 07:01:20PM +0100, Andrew Cooper wrote: >> * Drop unused include in private.h. >> * Used explicit width integers for Intel header fields. >> * Adjust comment to better describe the extended header. >> * Drop unnecessary __packed attribute for AMD header. >> * Switch mc_patch_data_id to being uint16_t, which is how it is more commonly >> referred to. >> * Fix types and style. >> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > Acked-by: Roger Pau Monné <roger.pau@citrix.com> Thanks, >> --- >> CC: Jan Beulich <JBeulich@suse.com> >> CC: Roger Pau Monné <roger.pau@citrix.com> >> CC: Wei Liu <wl@xen.org> >> --- >> xen/arch/x86/cpu/microcode/amd.c | 10 +++++----- >> xen/arch/x86/cpu/microcode/intel.c | 34 +++++++++++++++++----------------- >> xen/arch/x86/cpu/microcode/private.h | 2 -- >> 3 files changed, 22 insertions(+), 24 deletions(-) >> >> diff --git a/xen/arch/x86/cpu/microcode/amd.c b/xen/arch/x86/cpu/microcode/amd.c >> index cd532321e8..e913232067 100644 >> --- a/xen/arch/x86/cpu/microcode/amd.c >> +++ b/xen/arch/x86/cpu/microcode/amd.c >> @@ -24,7 +24,7 @@ >> >> #define pr_debug(x...) ((void)0) >> >> -struct __packed equiv_cpu_entry { >> +struct equiv_cpu_entry { >> uint32_t installed_cpu; >> uint32_t fixed_errata_mask; >> uint32_t fixed_errata_compare; >> @@ -35,7 +35,7 @@ struct __packed equiv_cpu_entry { >> struct microcode_patch { >> uint32_t data_code; >> uint32_t patch_id; >> - uint8_t mc_patch_data_id[2]; >> + uint16_t mc_patch_data_id; >> uint8_t mc_patch_data_len; > I think you could also drop the mc_patch_ prefixes from a couple of > fields in this structure, since they serve no purpose AFAICT. Actually, I'll drop this change and leave the field names alone. Stripping that prefix will make the field names logically wrong (e.g. data_len isn't the length of the header, or of the entire patch), and I've got other work planned to clean this area up. ~Andrew
diff --git a/xen/arch/x86/cpu/microcode/amd.c b/xen/arch/x86/cpu/microcode/amd.c index cd532321e8..e913232067 100644 --- a/xen/arch/x86/cpu/microcode/amd.c +++ b/xen/arch/x86/cpu/microcode/amd.c @@ -24,7 +24,7 @@ #define pr_debug(x...) ((void)0) -struct __packed equiv_cpu_entry { +struct equiv_cpu_entry { uint32_t installed_cpu; uint32_t fixed_errata_mask; uint32_t fixed_errata_compare; @@ -35,7 +35,7 @@ struct __packed equiv_cpu_entry { struct microcode_patch { uint32_t data_code; uint32_t patch_id; - uint8_t mc_patch_data_id[2]; + uint16_t mc_patch_data_id; uint8_t mc_patch_data_len; uint8_t init_flag; uint32_t mc_patch_data_checksum; @@ -102,7 +102,7 @@ static void collect_cpu_info(void) smp_processor_id(), csig->rev); } -static bool_t verify_patch_size(uint32_t patch_size) +static bool verify_patch_size(uint32_t patch_size) { uint32_t max_size; @@ -113,7 +113,7 @@ static bool_t verify_patch_size(uint32_t patch_size) #define F17H_MPB_MAX_SIZE 3200 #define F19H_MPB_MAX_SIZE 4800 - switch (boot_cpu_data.x86) + switch ( boot_cpu_data.x86 ) { case 0x14: max_size = F14H_MPB_MAX_SIZE; @@ -135,7 +135,7 @@ static bool_t verify_patch_size(uint32_t patch_size) break; } - return (patch_size <= max_size); + return patch_size <= max_size; } static bool check_final_patch_levels(const struct cpu_signature *sig) diff --git a/xen/arch/x86/cpu/microcode/intel.c b/xen/arch/x86/cpu/microcode/intel.c index d031196d4c..d9bb1bc10e 100644 --- a/xen/arch/x86/cpu/microcode/intel.c +++ b/xen/arch/x86/cpu/microcode/intel.c @@ -32,38 +32,38 @@ #define pr_debug(x...) ((void)0) struct microcode_patch { - unsigned int hdrver; - unsigned int rev; + uint32_t hdrver; + uint32_t rev; uint16_t year; uint8_t day; uint8_t month; - unsigned int sig; - unsigned int cksum; - unsigned int ldrver; + uint32_t sig; + uint32_t cksum; + uint32_t ldrver; /* * Microcode for the Pentium Pro and II had all further fields in the * header reserved, had a fixed datasize of 2000 and totalsize of 2048, * and didn't use platform flags despite the availability of the MSR. */ - unsigned int pf; - unsigned int datasize; - unsigned int totalsize; - unsigned int reserved[3]; + uint32_t pf; + uint32_t datasize; + uint32_t totalsize; + uint32_t reserved[3]; /* Microcode payload. Format is propriety and encrypted. */ uint8_t data[]; -}; -/* microcode format is extended from prescott processors */ + /* Extended header (iff totalsize > datasize, P4 Prescott and later) */ +}; struct extended_sigtable { - unsigned int count; - unsigned int cksum; - unsigned int reserved[3]; + uint32_t count; + uint32_t cksum; + uint32_t rsvd[3]; struct { - unsigned int sig; - unsigned int pf; - unsigned int cksum; + uint32_t sig; + uint32_t pf; + uint32_t cksum; } sigs[]; }; diff --git a/xen/arch/x86/cpu/microcode/private.h b/xen/arch/x86/cpu/microcode/private.h index c00ba590d1..9a15cdc879 100644 --- a/xen/arch/x86/cpu/microcode/private.h +++ b/xen/arch/x86/cpu/microcode/private.h @@ -1,8 +1,6 @@ #ifndef ASM_X86_MICROCODE_PRIVATE_H #define ASM_X86_MICROCODE_PRIVATE_H -#include <xen/types.h> - #include <asm/microcode.h> enum microcode_match_result {
* Drop unused include in private.h. * Used explicit width integers for Intel header fields. * Adjust comment to better describe the extended header. * Drop unnecessary __packed attribute for AMD header. * Switch mc_patch_data_id to being uint16_t, which is how it is more commonly referred to. * Fix types and style. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Roger Pau Monné <roger.pau@citrix.com> CC: Wei Liu <wl@xen.org> --- xen/arch/x86/cpu/microcode/amd.c | 10 +++++----- xen/arch/x86/cpu/microcode/intel.c | 34 +++++++++++++++++----------------- xen/arch/x86/cpu/microcode/private.h | 2 -- 3 files changed, 22 insertions(+), 24 deletions(-)