[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <fb205a5b-0689-4d53-a272-d196d337a9e5@amd.com>
Date: Thu, 26 Jun 2025 12:41:30 -0500
From: "Moger, Babu" <babu.moger@....com>
To: Reinette Chatre <reinette.chatre@...el.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 Reinette,
On 6/24/25 22:03, Reinette Chatre wrote:
> Hi Babu,
>
> With the new arch API this have "x86,fs/resctrl" prefix.
Sure.
>
> On 6/13/25 2:05 PM, Babu Moger wrote:
>> The ABMC feature allows users to assign a hardware counter ID to an RMID,
>> event pair and monitor bandwidth usage as long as it is assigned. The
>> hardware continues to track the assigned counter until it is explicitly
>> unassigned by the user.
>>
>> Implement an architecture-specific handler to assign and unassign the
>> counter. Configure counters by writing to the L3_QOS_ABMC_CFG MSR,
>> specifying the counter ID, bandwidth source (RMID), and event
>> configuration.
>>
>> The feature details are documented in the APM listed below [1].
>> [1] AMD64 Architecture Programmer's Manual Volume 2: System Programming
>> Publication # 24593 Revision 3.41 section 19.3.3.3 Assignable Bandwidth
>> Monitoring (ABMC).
>>
>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537
>> Signed-off-by: Babu Moger <babu.moger@....com>
>> ---
>
> ...
>
>> ---
>> 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);
You want to do these changes now or later? It requires few other changes
to move around the code.
>
>> +
>> + /*
>> + * 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.
>
>> +}
>> diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
>> index a58dd40b7246..1539d1faa1a1 100644
>> --- a/include/linux/resctrl.h
>> +++ b/include/linux/resctrl.h
>> @@ -594,6 +594,25 @@ void resctrl_arch_reset_rmid_all(struct rdt_resource *r, struct rdt_mon_domain *
>> */
>> void resctrl_arch_reset_all_ctrls(struct rdt_resource *r);
>>
>> +/**
>> + * resctrl_arch_config_cntr() - Configure the counter with its new RMID
>> + * and event details.
>> + * @r: Resource structure.
>> + * @d: The domain in which the counter ID is to be configured.
>
> "The domain in which the counter should be configured." or "The domain in which counter
> with ID @cntr_id should be configured."?
Added
"The domain in which counter with ID @cntr_id should be configured."
>
>> + * @evtid: Monitoring event type (e.g., QOS_L3_MBM_TOTAL_EVENT_ID
>> + * or QOS_L3_MBM_LOCAL_EVENT_ID).
>> + * @rmid: RMID.
>> + * @closid: CLOSID.
>> + * @cntr_id: Counter ID to configure.
>> + * @assign: True to assign the counter, false to unassign
>> + * the counter.
>
> The changelog and comments only mention counter "assignment" but this same call
> is used to update an existing assignment, which on ABMC are done the same.
> This may be ok for now but I think it will be helpful to amend the above to say
> something like:
>
> * @assign: True to assign the counter or update an existing assignment, false to unassign
> * the counter.
Sure.
>
>> + *
>> + * This can be called from any CPU.
>> + */
>> +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);
>> +
>> extern unsigned int resctrl_rmid_realloc_threshold;
>> extern unsigned int resctrl_rmid_realloc_limit;
>>
>
> Reinette
>
--
Thanks
Babu Moger
Powered by blists - more mailing lists