Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
IronPort-SDR: 8ftTRlp9vlazqKYp6xM0TiWf6U0biTKc1EpilroVsoODo4O7UOx7I+x89DuwmAiJNpN96k22Ix
 srr129ITqvdg==
IronPort-SDR: 19MUW11o3UgrQDTTg4md8G0ybJFc6HfNWwZfHSEpBuwhhB4PIO/eoxyfX1KyrrY6u/xY3AH8Ae
 m2dcTKImE9NQ==
Subject: Re: [PATCH 2/2] x86/resctrl: Support CPUID enumeration of MBM counter
 width
To:     Borislav Petkov <bp@alien8.de>
Cc:     tglx@linutronix.de, fenghua.yu@intel.com, tony.luck@intel.com,
        kuo-lang.tseng@intel.com, mingo@redhat.com, babu.moger@amd.com,
        hpa@zytor.com, x86@kernel.org, linux-kernel@vger.kernel.org
References: <cover.1585763047.git.reinette.chatre@intel.com>
 <76dc65631c373e0c1c9f3e8aaa768f022a2c989c.1585763047.git.reinette.chatre@intel.com>
 <20200429181149.GE16407@zn.tnic>
 <6fb58472-88f2-925b-3a4e-4692957a9582@intel.com>
 <20200430095913.GA3996@zn.tnic>
 <4288b11f-d4da-d311-7112-fa05887f50b4@intel.com>
 <20200504075632.GB15046@zn.tnic>
From:   Reinette Chatre <reinette.chatre@intel.com>
Message-ID: <b357e1f6-bf92-ec50-73f2-bd5987b159fc@intel.com>
Date:   Mon, 4 May 2020 21:19:21 -0700
User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:68.0) Gecko/20100101
 Thunderbird/68.7.0
MIME-Version: 1.0
In-Reply-To: <20200504075632.GB15046@zn.tnic>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Borislav,

On 5/4/2020 12:56 AM, Borislav Petkov wrote:
> Hi,
> 
> On Sun, May 03, 2020 at 11:51:00AM -0700, Reinette Chatre wrote:
>> I am struggling with what should follow ...
> 
> Since a diff is better than a thousand words :-) see below.
> 

Thank you so much for providing the details. Your explanation is clear
to me but I do have one clarification question ...


> @@ -597,6 +598,8 @@ static void bsp_init_amd(struct cpuinfo_x86 *c)
>  			x86_amd_ls_cfg_ssbd_mask = 1ULL << bit;
>  		}
>  	}
> +
> +	resctrl_cpu_detect(c);
>  }
>  

...

> @@ -322,6 +323,11 @@ static void early_init_intel(struct cpuinfo_x86 *c)
>  		detect_ht_early(c);
>  }
>  
> +static void bsp_init_intel(struct cpuinfo_x86 *c)
> +{
> +	resctrl_cpu_detect(c);
> +}
> +
>  #ifdef CONFIG_X86_32
>  /*
>   *	Early probe support logic for ppro memory erratum #50
> @@ -961,6 +967,7 @@ static const struct cpu_dev intel_cpu_dev = {
>  #endif
>  	.c_detect_tlb	= intel_detect_tlb,
>  	.c_early_init   = early_init_intel,
> +	.c_bsp_init	= bsp_init_intel,
>  	.c_init		= init_intel,
>  	.c_x86_vendor	= X86_VENDOR_INTEL,
>  };
> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> index d8cc5223b7ce..5e5955aa6593 100644
> --- a/arch/x86/kernel/cpu/resctrl/core.c
> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> @@ -22,7 +22,7 @@
>  #include <linux/cpuhotplug.h>
>  
>  #include <asm/intel-family.h>
> -#include <asm/resctrl_sched.h>
> +#include <asm/resctrl.h>
>  #include "internal.h"
>  
>  /* Mutex to protect rdtgroup access. */
> @@ -958,6 +958,35 @@ static __init void rdt_init_res_defs(void)
>  
>  static enum cpuhp_state rdt_online;
>  
> +/* Runs once on the BSP during boot. */
> +void resctrl_cpu_detect(struct cpuinfo_x86 *c)
> +{
> +	if (!cpu_has(c, X86_FEATURE_CQM_LLC)) {
> +		c->x86_cache_max_rmid  = -1;
> +		c->x86_cache_occ_scale = -1;
> +		c->x86_cache_mbm_width_offset = -1;
> +		return;
> +	}
> +
> +	/* will be overridden if occupancy monitoring exists */
> +	c->x86_cache_max_rmid = cpuid_ebx(0xf);
> +
> +	if (cpu_has(c, X86_FEATURE_CQM_OCCUP_LLC) ||
> +	    cpu_has(c, X86_FEATURE_CQM_MBM_TOTAL) ||
> +	    cpu_has(c, X86_FEATURE_CQM_MBM_LOCAL)) {
> +		u32 eax, ebx, ecx, edx;
> +
> +		/* QoS sub-leaf, EAX=0Fh, ECX=1 */
> +		cpuid_count(0xf, 1, &eax, &ebx, &ecx, &edx);
> +
> +		c->x86_cache_max_rmid  = ecx;
> +		c->x86_cache_occ_scale = ebx;
> +
> +		if (c->x86_vendor == X86_VENDOR_INTEL)
> +			c->x86_cache_mbm_width_offset = eax & 0xff;
> +	}
> +}
> +

resctrl_cpu_detect() is now identical among vendors. Do we still need
the c_bsp_init helpers? Could we not perhaps call resctrl_cpu_detect()
directly from early_identify_cpu()?

Thank you

Reinette