Re: [PATCH v14 17/32] x86/resctrl: Implement resctrl_arch_config_cntr() to assign a counter with ABMC

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

Hi Babu,

On 6/26/25 11:35 AM, Moger, Babu wrote:
> On 6/26/25 13:02, Reinette Chatre wrote:
>> On 6/26/25 10:41 AM, Moger, Babu wrote:
>>> On 6/24/25 22:03, Reinette Chatre wrote:
>>>> On 6/13/25 2:05 PM, Babu Moger wrote:
>>
>> ..
>>
>>>>> ---
>>>>>  arch/x86/kernel/cpu/resctrl/monitor.c | 38 +++++++++++++++++++++++++++
>>>>>  include/linux/resctrl.h               | 19 ++++++++++++++
>>>>>  2 files changed, 57 insertions(+)
>>>>>
>>>>> diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c
>>>>> index 0ad9c731c13e..6b0ea4b17c7a 100644
>>>>> --- a/arch/x86/kernel/cpu/resctrl/monitor.c
>>>>> +++ b/arch/x86/kernel/cpu/resctrl/monitor.c
>>>>> @@ -444,3 +444,41 @@ bool resctrl_arch_mbm_cntr_assign_enabled(struct rdt_resource *r)
>>>>>  {
>>>>>  	return resctrl_to_arch_res(r)->mbm_cntr_assign_enabled;
>>>>>  }
>>>>> +
>>>>> +static void resctrl_abmc_config_one_amd(void *info)
>>>>> +{
>>>>> +	union l3_qos_abmc_cfg *abmc_cfg = info;
>>>>> +
>>>>> +	wrmsrl(MSR_IA32_L3_QOS_ABMC_CFG, abmc_cfg->full);
>>>>> +}
>>>>> +
>>>>> +/*
>>>>> + * Send an IPI to the domain to assign the counter to RMID, event pair.
>>>>> + */
>>>>> +void resctrl_arch_config_cntr(struct rdt_resource *r, struct rdt_mon_domain *d,
>>>>> +			      enum resctrl_event_id evtid, u32 rmid, u32 closid,
>>>>> +			      u32 cntr_id, bool assign)
>>>>> +{
>>>>> +	struct rdt_hw_mon_domain *hw_dom = resctrl_to_arch_mon_dom(d);
>>>>> +	union l3_qos_abmc_cfg abmc_cfg = { 0 };
>>>>> +	struct arch_mbm_state *am;
>>>>> +
>>>>> +	abmc_cfg.split.cfg_en = 1;
>>>>> +	abmc_cfg.split.cntr_en = assign ? 1 : 0;
>>>>> +	abmc_cfg.split.cntr_id = cntr_id;
>>>>> +	abmc_cfg.split.bw_src = rmid;
>>>>> +	if (assign)
>>>>> +		abmc_cfg.split.bw_type = resctrl_get_mon_evt_cfg(evtid);
>>>>> +
>>>>> +	smp_call_function_any(&d->hdr.cpu_mask, resctrl_abmc_config_one_amd, &abmc_cfg, 1);
>>>>
>>>> An optimization to consider is to direct the IPI to a housekeeping CPU.
>>>> If one exist, a further optimization could be to queue it on that CPU
>>>> instead of IPI. Your call since I am not familiar with the use cases here.
>>>> Looks like all paths leading to this is triggered by a user space action
>>>> (mount, mkdir, or write to update event config). This would require exposing
>>>> the housekeeping CPU code to arch.
>>>
>>> Do you mean something like this?
>>>
>>> cpu = cpumask_any_housekeeping(&d->hdr.cpu_mask, RESCTRL_PICK_ANY_CPU);
>>>
>>> smp_call_on_cpu(cpu, resctrl_abmc_config_one_amd, &abmc_cfg, false);
>>
>> Please note the returned "cpu" may be nohz_full and if it is it would need
>> an IPI. Similar to mon_event_read().
>>
> 
> Yes. Got that.
> 
>>>
>>>
>>> You want to do these changes now or later?  It requires few other changes
>>> to move around the code.
>>
>> I'll leave this up to you. I think it would be ideal if cpumask_any_housekeeping()
>> can be hosted in include/linux/cpumask.h instead of moving it around within
>> resctrl.
>>
> 
> ok. It will be couple more patches to re-arrange all the related code. I
> would prefer its done separately.
> 

This is ok with me. Thank you.

Reinette