Date: Mon, 15 Jan 2018 19:46:16 +0100
From: Borislav Petkov <bp@alien8.de>
To: Jia Zhang <zhang.jia@linux.alibaba.com>
Cc: hmh@hmh.eng.br, tony.luck@intel.com, mingo@redhat.com,
        hpa@zytor.com, tglx@linutronix.de, x86@kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] x86/microcode/intel: Extend BDW late-loading with LLC
 size check
Message-ID: <20180115184616.r6pypjegywyd7ncm@pd.tnic>
References: <1516021917-48335-1-git-send-email-zhang.jia@linux.alibaba.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <1516021917-48335-1-git-send-email-zhang.jia@linux.alibaba.com>
User-Agent: NeoMutt/20170609 (1.8.3)
Sender: linux-kernel-owner@vger.kernel.org

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.

> +
> +	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.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.