linux-kernel - Re: [PATCH v14 17/32] x86/resctrl: Implement resctrl_arch_config

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d12ea8e3-a7f0-4962-808c-cfed0bd72674@intel.com>
Date: Thu, 26 Jun 2025 11:02:05 -0700
From: Reinette Chatre <reinette.chatre@...el.com>
To: <babu.moger@....com>, <corbet@....net>, <tony.luck@...el.com>,
	<Dave.Martin@....com>, <james.morse@....com>, <tglx@...utronix.de>,
	<mingo@...hat.com>, <bp@...en8.de>, <dave.hansen@...ux.intel.com>
CC: <x86@...nel.org>, <hpa@...or.com>, <akpm@...ux-foundation.org>,
	<rostedt@...dmis.org>, <paulmck@...nel.org>, <thuth@...hat.com>,
	<ardb@...nel.org>, <gregkh@...uxfoundation.org>, <seanjc@...gle.com>,
	<thomas.lendacky@....com>, <pawan.kumar.gupta@...ux.intel.com>,
	<manali.shukla@....com>, <perry.yuan@....com>, <kai.huang@...el.com>,
	<peterz@...radead.org>, <xiaoyao.li@...el.com>, <kan.liang@...ux.intel.com>,
	<mario.limonciello@....com>, <xin3.li@...el.com>, <gautham.shenoy@....com>,
	<xin@...or.com>, <chang.seok.bae@...el.com>, <fenghuay@...dia.com>,
	<peternewman@...gle.com>, <maciej.wieczor-retman@...el.com>,
	<eranian@...gle.com>, <linux-doc@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v14 17/32] x86/resctrl: Implement
 resctrl_arch_config_cntr() to assign a counter with ABMC

Hi Babu,

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().

> 
> 
> 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.

> 
>>
>>> +
>>> +	/*
>>> +	 * The hardware counter is reset (because cfg_en == 1) so there is no
>>> +	 * need to record initial non-zero counts.
>>> +	 */
>>> +	if (assign) {
>>> +		am = get_arch_mbm_state(hw_dom, rmid, evtid);
>>> +		if (am)
>>> +			memset(am, 0, sizeof(*am));
>>> +	}
>>
>> I am not able to recognize how the struct rdt_resource parameter is used. What am I missing?
> 
> No. It is not used here. It is kept as other arch's can use it.  I think
> James commented about it earlier.

I see, thank you.

Reinette