Messages in this thread Patch in this message | | | Subject | Re: [PATCH v2] x86/microcode/intel: Extend BDW late-loading with LLC size check | From | Jia Zhang <> | Date | Tue, 16 Jan 2018 09:14:44 +0800 |
| |
在 2018/1/16 上午2:46, Borislav Petkov 写道: > On Mon, Jan 15, 2018 at 09:11:57PM +0800, Jia Zhang wrote: >> The commit b94b73733171 >> ("x86/microcode/intel: Extend BDW late-loading with a revision check") >> reduces the impact of erratum BDF90 for Broadwell process model. >> Actually, the impact can be reduced further through adding the checks >> for the size of LLC per core. >> >> For more details, see erratum BDF90 in document #334165 (Intel Xeon >> Processor E7-8800/4800 v4 Product Family Specification Update) from >> September 2017. >> >> Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com> >> --- >> arch/x86/kernel/cpu/microcode/intel.c | 15 +++++++++++++-- >> 1 file changed, 13 insertions(+), 2 deletions(-) >> >> diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c >> index d9e460f..9143cf2 100644 >> --- a/arch/x86/kernel/cpu/microcode/intel.c >> +++ b/arch/x86/kernel/cpu/microcode/intel.c >> @@ -906,18 +906,29 @@ static int get_ucode_fw(void *to, const void *from, size_t n) >> return 0; >> } >> >> +static int llc_size_per_core(struct cpuinfo_x86 *c) >> +{ >> + u64 llc_size = c->x86_cache_size * 1024; >> + >> + do_div(llc_size, c->x86_max_cores); > > This is done per-CPU - I don't want it to do the same division for each > core. Do it once at driver init only for that model and cache it.
How about this?
--- a/arch/x86/kernel/cpu/microcode/intel.c +++ b/arch/x86/kernel/cpu/microcode/intel.c @@ -908,10 +908,13 @@ static int get_ucode_fw(void *to, const void *from, size_t n)
static int llc_size_per_core(struct cpuinfo_x86 *c) { - u64 llc_size = c->x86_cache_size * 1024; - - do_div(llc_size, c->x86_max_cores); + static u64 llc_size;
+ if (unlikely(!llc_size)) { + llc_size = c->x86_cache_size * 1024; + do_div(llc_size, c->x86_max_cores); + } + return (int)llc_size; }
or driver init style?
@@ -996,5 +999,7 @@ struct microcode_ops * __init init_intel_microcode(void) return NULL; } + llc_size_per_core = calc_llc_size_per_core(c); + return µcode_intel_ops; } or more generic?
@@ -984,6 +987,7 @@ static int get_ucode_user(void *to, const void *from, size_t n) .request_microcode_fw = request_microcode_fw, .collect_cpu_info = collect_cpu_info, .apply_microcode = apply_microcode_intel, + .is_blacklisted = is_blacklisted, }; > >> + >> + return (int)llc_size; >> +} >> + >> static bool is_blacklisted(unsigned int cpu) >> { >> struct cpuinfo_x86 *c = &cpu_data(cpu); >> >> /* >> * Late loading on model 79 with microcode revision less than 0x0b000021 >> - * may result in a system hang. This behavior is documented in item >> - * BDF90, #334165 (Intel Xeon Processor E7-8800/4800 v4 Product Family). >> + * and LLC size per core bigger than 2.5MB may result in a system hang. >> + * This behavior is documented in item BDF90, #334165 (Intel Xeon >> + * Processor E7-8800/4800 v4 Product Family). >> */ >> if (c->x86 == 6 && >> c->x86_model == INTEL_FAM6_BROADWELL_X && >> c->x86_mask == 0x01 && >> + llc_size_per_core(c) > 2621440 && > > I'm not taking this: this looks like a bunch of voodoo magic numbers. > Please get someone from Intel to explain first.
Tony, could you clarify this?
Thanks, Jia
>
| |