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