Configure mbm_cntr_assign mode on AMD platforms. On AMD platforms, it
is recommended to use mbm_cntr_assign mode if supported, because
reading "mbm_total_bytes" or "mbm_local_bytes" will report 'Unavailable'
if there is no counter associated with that event.

The mbm_cntr_assign mode, referred to as ABMC (Assignable Bandwidth
Monitoring Counters) on AMD, is enabled by default when supported by the
system.

Update ABMC across all logical processors within the resctrl domain to
ensure proper functionality.

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
v11: Commit text in imperative tone. Added few more details.
     Moved resctrl_arch_mbm_cntr_assign_set_one() to monitor.c.

v10: Commit text in imperative tone.

v9: Minor code change due to merge. Actual code did not change.

v8: Renamed resctrl_arch_mbm_cntr_assign_configure to
	resctrl_arch_mbm_cntr_assign_set_one.
    Adde r->mon_capable check.
    Commit message update.

v7: Introduced resctrl_arch_mbm_cntr_assign_configure() to configure.
    Moved the default settings to rdt_get_mon_l3_config(). It should be
    done before the hotplug handler is called. It cannot be done at
    rdtgroup_init().

v6: Keeping the default enablement in arch init code for now.
     This may need some discussion.
     Renamed resctrl_arch_configure_abmc to resctrl_arch_mbm_cntr_assign_configure.

v5: New patch to enable ABMC by default.
---
 arch/x86/kernel/cpu/resctrl/internal.h | 1 +
 arch/x86/kernel/cpu/resctrl/monitor.c  | 8 ++++++++
 arch/x86/kernel/cpu/resctrl/rdtgroup.c | 4 ++++
 3 files changed, 13 insertions(+)

diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index c006c4d8d6ff..2480698b643d 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -734,4 +734,5 @@ int resctrl_unassign_cntr_event(struct rdt_resource *r, struct rdt_mon_domain *d
 void mbm_cntr_reset(struct rdt_resource *r);
 int mbm_cntr_get(struct rdt_resource *r, struct rdt_mon_domain *d,
 		 struct rdtgroup *rdtgrp, enum resctrl_event_id evtid);
+void resctrl_arch_mbm_cntr_assign_set_one(struct rdt_resource *r);
 #endif /* _ASM_X86_RESCTRL_INTERNAL_H */
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;
 			resctrl_file_fflags_init("num_mbm_cntrs", RFTYPE_MON_INFO);
 			resctrl_file_fflags_init("available_mbm_cntrs", RFTYPE_MON_INFO);
 		}
@@ -1313,6 +1314,13 @@ static void _resctrl_abmc_enable(struct rdt_resource *r, bool enable)
 				 resctrl_abmc_set_one_amd, &enable, 1);
 }
 
+void resctrl_arch_mbm_cntr_assign_set_one(struct rdt_resource *r)
+{
+	struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
+
+	resctrl_abmc_set_one_amd(&hw_res->mbm_cntr_assign_enabled);
+}
+
 int resctrl_arch_mbm_cntr_assign_set(struct rdt_resource *r, bool enable)
 {
 	struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
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);
 }
 
-- 
2.34.1

Hi Babu,

On 22/01/2025 20:20, Babu Moger wrote:
> Configure mbm_cntr_assign mode on AMD platforms. On AMD platforms, it
> is recommended to use mbm_cntr_assign mode if supported, because
> reading "mbm_total_bytes" or "mbm_local_bytes" will report 'Unavailable'
> if there is no counter associated with that event.

(If you agree with my comment on patch 7, it would be good to update this
wording to match.)


> The mbm_cntr_assign mode, referred to as ABMC (Assignable Bandwidth
> Monitoring Counters) on AMD, is enabled by default when supported by the
> system.
> 
> Update ABMC across all logical processors within the resctrl domain to
> ensure proper functionality.
> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> index c006c4d8d6ff..2480698b643d 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -734,4 +734,5 @@ int resctrl_unassign_cntr_event(struct rdt_resource *r, struct rdt_mon_domain *d
>  void mbm_cntr_reset(struct rdt_resource *r);
>  int mbm_cntr_get(struct rdt_resource *r, struct rdt_mon_domain *d,
>  		 struct rdtgroup *rdtgrp, enum resctrl_event_id evtid);
> +void resctrl_arch_mbm_cntr_assign_set_one(struct rdt_resource *r);
>  #endif /* _ASM_X86_RESCTRL_INTERNAL_H */

Could this be put in include/linux/resctrl.h, its where it needs to end up eventually.



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.

Could this be done in resctrl_arch_online_cpu() instead? That way resctrl doesn't get CPUs
in an inconsistent state that it has to fix up...


Thanks,

James

Hi James,

On 2/21/25 12:06, James Morse wrote:
> Hi Babu,
> 
> On 22/01/2025 20:20, Babu Moger wrote:
>> Configure mbm_cntr_assign mode on AMD platforms. On AMD platforms, it
>> is recommended to use mbm_cntr_assign mode if supported, because
>> reading "mbm_total_bytes" or "mbm_local_bytes" will report 'Unavailable'
>> if there is no counter associated with that event.
> 
> (If you agree with my comment on patch 7, it would be good to update this
> wording to match.)

Sure.

> 
> 
>> The mbm_cntr_assign mode, referred to as ABMC (Assignable Bandwidth
>> Monitoring Counters) on AMD, is enabled by default when supported by the
>> system.
>>
>> Update ABMC across all logical processors within the resctrl domain to
>> ensure proper functionality.
>> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
>> index c006c4d8d6ff..2480698b643d 100644
>> --- a/arch/x86/kernel/cpu/resctrl/internal.h
>> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
>> @@ -734,4 +734,5 @@ int resctrl_unassign_cntr_event(struct rdt_resource *r, struct rdt_mon_domain *d
>>  void mbm_cntr_reset(struct rdt_resource *r);
>>  int mbm_cntr_get(struct rdt_resource *r, struct rdt_mon_domain *d,
>>  		 struct rdtgroup *rdtgrp, enum resctrl_event_id evtid);
>> +void resctrl_arch_mbm_cntr_assign_set_one(struct rdt_resource *r);
>>  #endif /* _ASM_X86_RESCTRL_INTERNAL_H */
> 
> Could this be put in include/linux/resctrl.h, its where it needs to end up eventually.
> 

As Reinette already mentioned in  [1], Boris wanted this moved when
arch/fs code separation integrated. Lets keep it in resctrl/internal.h
for now.

[1]
https://lore.kernel.org/lkml/e524c376-9ef8-488e-8053-b49ccafd306d@intel.com/

> 
> 
> 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.
> 
> Could this be done in resctrl_arch_online_cpu() instead? That way resctrl doesn't get CPUs
> in an inconsistent state that it has to fix up...
> 

Sure. Here is the diff.

diff --git a/arch/x86/kernel/cpu/resctrl/core.c
b/arch/x86/kernel/cpu/resctrl/core.c
index 22399f19810f..f48b298413bc 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
@@ -771,6 +771,12 @@ static int resctrl_arch_online_cpu(unsigned int cpu)
                domain_add_cpu(cpu, r);
        mutex_unlock(&domain_list_lock);

+       r = &rdt_resources_all[RDT_RESOURCE_L3].r_resctrl;
+       mutex_lock(&rdtgroup_mutex);
+       if (r->mon_capable && r->mon.mbm_cntr_assignable)
+               resctrl_arch_mbm_cntr_assign_set_one(r);
+       mutex_unlock(&rdtgroup_mutex);
+
        clear_closid_rmid(cpu);
        resctrl_online_cpu(cpu);


-- 
Thanks
Babu Moger

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.

>>
>> Could this be done in resctrl_arch_online_cpu() instead? That way resctrl doesn't get CPUs
>> in an inconsistent state that it has to fix up...

Could you please elaborate the inconsistent state that would need to be fixed up?

>>
> 
> Sure. Here is the diff.
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/core.c
> b/arch/x86/kernel/cpu/resctrl/core.c
> index 22399f19810f..f48b298413bc 100644
> --- a/arch/x86/kernel/cpu/resctrl/core.c
> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> @@ -771,6 +771,12 @@ static int resctrl_arch_online_cpu(unsigned int cpu)
>                 domain_add_cpu(cpu, r);
>         mutex_unlock(&domain_list_lock);
> 
> +       r = &rdt_resources_all[RDT_RESOURCE_L3].r_resctrl;
> +       mutex_lock(&rdtgroup_mutex);
> +       if (r->mon_capable && r->mon.mbm_cntr_assignable)
> +               resctrl_arch_mbm_cntr_assign_set_one(r);
> +       mutex_unlock(&rdtgroup_mutex);
> +
>         clear_closid_rmid(cpu);
>         resctrl_online_cpu(cpu);

This would require every architecture to duplicate the above, no?

Also, please note there is more appropriate domain_add_cpu_mon().

Reinette

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.

> 
>>>
>>> Could this be done in resctrl_arch_online_cpu() instead? That way resctrl doesn't get CPUs
>>> in an inconsistent state that it has to fix up...
> 
> Could you please elaborate the inconsistent state that would need to be fixed up?
> 
>>>
>>
>> Sure. Here is the diff.
>>
>> diff --git a/arch/x86/kernel/cpu/resctrl/core.c
>> b/arch/x86/kernel/cpu/resctrl/core.c
>> index 22399f19810f..f48b298413bc 100644
>> --- a/arch/x86/kernel/cpu/resctrl/core.c
>> +++ b/arch/x86/kernel/cpu/resctrl/core.c
>> @@ -771,6 +771,12 @@ static int resctrl_arch_online_cpu(unsigned int cpu)
>>                 domain_add_cpu(cpu, r);
>>         mutex_unlock(&domain_list_lock);
>>
>> +       r = &rdt_resources_all[RDT_RESOURCE_L3].r_resctrl;
>> +       mutex_lock(&rdtgroup_mutex);
>> +       if (r->mon_capable && r->mon.mbm_cntr_assignable)
>> +               resctrl_arch_mbm_cntr_assign_set_one(r);
>> +       mutex_unlock(&rdtgroup_mutex);
>> +
>>         clear_closid_rmid(cpu);
>>         resctrl_online_cpu(cpu);
> 
> This would require every architecture to duplicate the above, no?
> 
> Also, please note there is more appropriate domain_add_cpu_mon().

Yes. This may be better place to add this code. Will wait once James
clarifies on "inconsistent state".
-- 
Thanks
Babu Moger

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