Re: [PATCH V2 3/3] x86/microcode: early microcode patch loadingsupport on AMD

From: Jacob Shin
Date: Fri May 24 2013 - 15:51:41 EST

On Fri, May 24, 2013 at 03:33:38PM +0000, Yu, Fenghua wrote:
> > From: Jacob Shin [mailto:jacob.shin@xxxxxxx]
> > Add support for early microcode patch loading on AMD.
> >
> > Signed-off-by: Jacob Shin <jacob.shin@xxxxxxx>
> > ---
> > arch/x86/Kconfig | 16 +-
> > arch/x86/include/asm/microcode.h | 1 -
> > arch/x86/include/asm/microcode_amd.h | 17 ++
> > arch/x86/include/asm/microcode_intel.h | 1 +
> > arch/x86/kernel/microcode_amd.c | 338
> > ++++++++++++++++++++++++++++----
> > arch/x86/kernel/microcode_core_early.c | 7 +
> > 6 files changed, 333 insertions(+), 47 deletions(-)
> > create mode 100644 arch/x86/include/asm/microcode_amd.h
> >
> > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > index 3a5bced..fab72e7 100644
> > --- a/arch/x86/Kconfig
> > +++ b/arch/x86/Kconfig
> > @@ -1090,8 +1090,18 @@ config MICROCODE_INTEL_LIB
> > depends on MICROCODE_INTEL
> >
> > config MICROCODE_INTEL_EARLY
> > - bool "Early load microcode"
> > + def_bool n
> > depends on MICROCODE_INTEL && BLK_DEV_INITRD
> > +
> > +config MICROCODE_AMD_EARLY
> > + def_bool n
> > + depends on MICROCODE_AMD && BLK_DEV_INITRD
> > +
> > +config MICROCODE_EARLY
> > + bool "Early load microcode"
> > + depends on (MICROCODE_INTEL || MICROCODE_AMD) && BLK_DEV_INITRD
> > + select MICROCODE_INTEL_EARLY if MICROCODE_INTEL
> > + select MICROCODE_AMD_EARLY if MICROCODE_AMD
> > default y
> > help
> > This option provides functionality to read additional microcode
> > data
> > @@ -1099,10 +1109,6 @@ config MICROCODE_INTEL_EARLY
> > microcode to CPU's as early as possible. No functional change
> > if no
> > microcode data is glued to the initrd, therefore it's safe to
> > say Y.
> >
> > -config MICROCODE_EARLY
> > - def_bool y
> > - depends on MICROCODE_INTEL_EARLY
> > -
> > config X86_MSR
> > tristate "/dev/cpu/*/msr - Model-specific register support"
> > ---help---
> > diff --git a/arch/x86/include/asm/microcode.h
> > b/arch/x86/include/asm/microcode.h
> > index 6825e2e..f4be4cc 100644
> > --- a/arch/x86/include/asm/microcode.h
> > +++ b/arch/x86/include/asm/microcode.h
> > @@ -58,7 +58,6 @@ static inline void __exit exit_amd_microcode(void) {}
> > #endif
> >
> > #ifdef CONFIG_MICROCODE_EARLY
> > -#define MAX_UCODE_COUNT 128
> > extern void __init load_ucode_bsp(void);
> > extern __init void load_ucode_ap(void);
> > extern int __init save_microcode_in_initrd(void);
> > diff --git a/arch/x86/include/asm/microcode_amd.h
> > b/arch/x86/include/asm/microcode_amd.h
> > new file mode 100644
> > index 0000000..376123c
> > --- /dev/null
> > +++ b/arch/x86/include/asm/microcode_amd.h
> > @@ -0,0 +1,17 @@
> > +#ifndef _ASM_X86_MICROCODE_AMD_H
> > +#define _ASM_X86_MICROCODE_AMD_H
> > +
> > +#ifdef CONFIG_MICROCODE_AMD_EARLY
> > +extern void __init load_ucode_amd_bsp(void);
> > +extern void __cpuinit load_ucode_amd_ap(void);
> > +extern int __init save_microcode_in_initrd_amd(void);
> > +#else
> > +static inline void __init load_ucode_amd_bsp(void) {}
> > +static inline void __cpuinit load_ucode_amd_ap(void) {}
> > +static inline int __init save_microcode_in_initrd_amd(void)
> > +{
> > + return -EINVAL;
> > +}
> > +#endif
> > +
> > +#endif /* _ASM_X86_MICROCODE_AMD_H */
> > diff --git a/arch/x86/include/asm/microcode_intel.h
> > b/arch/x86/include/asm/microcode_intel.h
> > index 538e407..db4a2f9 100644
> > --- a/arch/x86/include/asm/microcode_intel.h
> > +++ b/arch/x86/include/asm/microcode_intel.h
> > @@ -64,6 +64,7 @@ extern int
> > update_match_revision(struct microcode_header_intel *mc_header, int
> > rev);
> >
> > #ifdef CONFIG_MICROCODE_INTEL_EARLY
> > +#define MAX_UCODE_COUNT 128
> > extern void __init load_ucode_intel_bsp(void);
> > extern void __cpuinit load_ucode_intel_ap(void);
> > extern void show_ucode_info_early(void);
> > diff --git a/arch/x86/kernel/microcode_amd.c
> > b/arch/x86/kernel/microcode_amd.c
> > index efdec7c..cda647e 100644
> > --- a/arch/x86/kernel/microcode_amd.c
> > +++ b/arch/x86/kernel/microcode_amd.c
> > @@ -27,10 +27,13 @@
> > #include <linux/kernel.h>
> > #include <linux/module.h>
> > #include <linux/pci.h>
> > +#include <linux/earlycpio.h>
> >
> > #include <asm/microcode.h>
> > +#include <asm/microcode_amd.h>
> > #include <asm/processor.h>
> > #include <asm/msr.h>
> > +#include <asm/setup.h>
> >
> > MODULE_DESCRIPTION("AMD Microcode Update Driver");
> > MODULE_AUTHOR("Peter Oruba");
> > @@ -84,23 +87,28 @@ struct ucode_patch {
> >
> > static LIST_HEAD(pcache);
> >
> > -static u16 find_equiv_id(unsigned int cpu)
> > +static u16 _find_equiv_id(struct equiv_cpu_entry *eq,
> > + struct ucode_cpu_info *uci)
> > {
> > - struct ucode_cpu_info *uci = ucode_cpu_info + cpu;
> > int i = 0;
> >
> > - if (!equiv_cpu_table)
> > + if (!eq)
> > return 0;
> >
> > - while (equiv_cpu_table[i].installed_cpu != 0) {
> > - if (uci->cpu_sig.sig == equiv_cpu_table[i].installed_cpu)
> > - return equiv_cpu_table[i].equiv_cpu;
> > + while (eq[i].installed_cpu != 0) {
> > + if (uci->cpu_sig.sig == eq[i].installed_cpu)
> > + return eq[i].equiv_cpu;
> >
> > i++;
> > }
> > return 0;
> > }
> >
> > +static u16 find_equiv_id(unsigned int cpu)
> > +{
> > + return _find_equiv_id(equiv_cpu_table, ucode_cpu_info + cpu);
> > +}
> > +
> > static u32 find_cpu_family_by_equiv_cpu(u16 equiv_cpu)
> > {
> > int i = 0;
> > @@ -173,9 +181,17 @@ static struct ucode_patch *find_patch(unsigned int
> > cpu)
> > static int collect_cpu_info_amd(int cpu, struct cpu_signature *csig)
> > {
> > struct cpuinfo_x86 *c = &cpu_data(cpu);
> > + struct ucode_cpu_info *uci = ucode_cpu_info + cpu;
> > + struct ucode_patch *p;
> >
> > csig->sig = cpuid_eax(0x00000001);
> > csig->rev = c->microcode;
> > +
> > + /* if a patch was early loaded, tell microcode_core.c about it */
> > + p = find_patch(cpu);
> > + if (p && (p->patch_id == csig->rev))
> > + uci->mc = p->data;
> > +
> > pr_info("CPU%d: patch_level=0x%08x\n", cpu, csig->rev);
> >
> > return 0;
> > @@ -215,24 +231,14 @@ static unsigned int verify_patch_size(int cpu,
> > u32 patch_size,
> > return patch_size;
> > }
> >
> > -static int apply_microcode_amd(int cpu)
> > +static int _apply_microcode_amd(int cpu, void *data, struct
> > cpuinfo_x86 *c,
> > + struct ucode_cpu_info *uci, bool silent)
> > {
> > - struct cpuinfo_x86 *c = &cpu_data(cpu);
> > struct microcode_amd *mc_amd;
> > - struct ucode_cpu_info *uci;
> > - struct ucode_patch *p;
> > u32 rev, dummy;
> >
> > - BUG_ON(raw_smp_processor_id() != cpu);
> > -
> > - uci = ucode_cpu_info + cpu;
> > -
> > - p = find_patch(cpu);
> > - if (!p)
> > - return 0;
> > -
> > - mc_amd = p->data;
> > - uci->mc = p->data;
> > + mc_amd = data;
> > + uci->mc = data;
> >
> > rdmsr(MSR_AMD64_PATCH_LEVEL, rev, dummy);
> >
> > @@ -247,18 +253,37 @@ static int apply_microcode_amd(int cpu)
> > /* verify patch application was successful */
> > rdmsr(MSR_AMD64_PATCH_LEVEL, rev, dummy);
> > if (rev != mc_amd->hdr.patch_id) {
> > - pr_err("CPU%d: update failed for patch_level=0x%08x\n",
> > - cpu, mc_amd->hdr.patch_id);
> > + if (!silent)
> > + pr_err("CPU%d: update failed for
> > patch_level=0x%08x\n",
> > + cpu, mc_amd->hdr.patch_id);
> > return -1;
> > }
> >
> > - pr_info("CPU%d: new patch_level=0x%08x\n", cpu, rev);
> > + if (!silent)
> > + pr_info("CPU%d: new patch_level=0x%08x\n", cpu, rev);
>
> In 32-bit, this info won't be printed. Then microcode is silently updated. This may make it difficult for user to catch new microcode update info. And this is not consistent to 64-bit which prints the info.
>
> You might print this info later when printk is usable like Intel ucode does.

Okay, will do.

>
> > uci->cpu_sig.rev = rev;
> > c->microcode = rev;
> >
> > return 0;
> > }
> >
> > +static int apply_microcode_amd(int cpu)
>
> This apply_microcode_amd(int cpu) can be simplified as apply_microcode_amd(void).

Actually, this function is part of the microcode_ops (non-early
loading called by microcode_core.c) so I cannot change the function
parameters.

Thanks!

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/