Hi Borislav,
On 5/4/2020 9:19 PM, Reinette Chatre wrote:
> 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()?
>
I should have given this more thought ... even though the function will
be identical between the two vendors it does contain vendor-specific
code, thus needing to be called from the c_bsp_init helpers? I will
resubmit the new series that intends to follow all your suggestions shortly.
Thank you very much for your feedback
Reinette
