Re: [PATCH v11 20/23] x86/resctrl: Configure mbm_cntr_assign mode if supported

[Reinette Chatre <reinette.chatre@intel.com>]

Hi Babu,

On 2/24/25 1:18 PM, Moger, Babu wrote:
> Hi Reinette,
> 
> 
> On 2/24/25 11:01, Reinette Chatre wrote:
>> Hi James and Babu,
>>
>> On 2/24/25 7:49 AM, Moger, Babu wrote:
>>> Hi James,
>>>
>>> On 2/21/25 12:06, James Morse wrote:
>>>> Hi Babu,
>>>>
>>>> On 22/01/2025 20:20, Babu Moger wrote:
>>
>>>> This sequence has me confused:
>>>>
>>>>> diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c
>>>>> index 3d748fdbcb5f..a9a5dc626a1e 100644
>>>>> --- a/arch/x86/kernel/cpu/resctrl/monitor.c
>>>>> +++ b/arch/x86/kernel/cpu/resctrl/monitor.c
>>>>> @@ -1233,6 +1233,7 @@ int __init rdt_get_mon_l3_config(struct rdt_resource *r)
>>>>>  			r->mon.mbm_cntr_assignable = true;
>>>>>  			cpuid_count(0x80000020, 5, &eax, &ebx, &ecx, &edx);
>>>>>  			r->mon.num_mbm_cntrs = (ebx & GENMASK(15, 0)) + 1;
>>>>
>>>>> +			hw_res->mbm_cntr_assign_enabled = true;
>>>>
>>>> Here the arch code sets ABMC to be enabled by default at boot.
>>>>
>>>>
>>>>> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>>>>> index 6922173c4f8f..515969c5f64f 100644
>>>>> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>>>>> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>>>>> @@ -4302,9 +4302,13 @@ int resctrl_online_mon_domain(struct rdt_resource *r, struct rdt_mon_domain *d)
>>>>>  
>>>>>  void resctrl_online_cpu(unsigned int cpu)
>>>>>  {
>>>>> +	struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_L3].r_resctrl;
>>>>> +
>>>>>  	mutex_lock(&rdtgroup_mutex);
>>>>>  	/* The CPU is set in default rdtgroup after online. */
>>>>>  	cpumask_set_cpu(cpu, &rdtgroup_default.cpu_mask);
>>>>> +	if (r->mon_capable && r->mon.mbm_cntr_assignable)
>>>>> +		resctrl_arch_mbm_cntr_assign_set_one(r);
>>>>>  	mutex_unlock(&rdtgroup_mutex);
>>>>>  }
>>>>
>>>> But here, resctrl has to call back to the arch code to make sure the hardware is in the
>>>> same state as hw_res->mbm_cntr_assign_enabled.
>>
>> Another scenario needing to be supported by this flow is when CPUs come online later ...
>> after resctrl is mounted and potentially after the user modified the assignable counter
>> mode.
> 
> If the user modifies the assignable counter mode. It is recorded in
> mbm_cntr_assign_enabled already. When the new CPU comes online, the
> hotplug handler(resctrl_arch_online_cpu) is will update the CPU to the new
> mode after checking mbm_cntr_assign_enabled.
> 
> Are you talking about different case here? Please elaborate.

I am talking about the same case. James started with "This sequence has me confused"
with "Here the arch code sets ABMC to be enabled by default at boot." snippet not
seemingly matching with later snippet where "resctrl has to call back to the arch code".
My goal was to highlight why resctrl would need to call back into arch code even though
arch code establishes the default. 

Reinette