Hi Babu, On 5/15/25 3:51 PM, Babu Moger wrote: > Add the functionality to enable/disable AMD ABMC feature. > > AMD ABMC feature is enabled by setting enabled bit(0) in MSR > L3_QOS_EXT_CFG. When the state of ABMC is changed, the MSR needs > to be updated on all the logical processors in the QOS Domain. > > Hardware counters will reset when ABMC state is changed. > > The ABMC feature details are documented in 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/include/asm/msr-index.h | 1 + > arch/x86/kernel/cpu/resctrl/internal.h | 5 +++ > arch/x86/kernel/cpu/resctrl/monitor.c | 43 ++++++++++++++++++++++++++ > include/linux/resctrl.h | 3 ++ > 4 files changed, 52 insertions(+) > > diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h > index e6134ef2263d..3970e0b16e47 100644 > --- a/arch/x86/include/asm/msr-index.h > +++ b/arch/x86/include/asm/msr-index.h > @@ -1203,6 +1203,7 @@ > /* - AMD: */ > #define MSR_IA32_MBA_BW_BASE 0xc0000200 > #define MSR_IA32_SMBA_BW_BASE 0xc0000280 > +#define MSR_IA32_L3_QOS_EXT_CFG 0xc00003ff > #define MSR_IA32_EVT_CFG_BASE 0xc0000400 > > /* AMD-V MSRs */ > diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h > index 5e3c41b36437..fcc9d23686a1 100644 > --- a/arch/x86/kernel/cpu/resctrl/internal.h > +++ b/arch/x86/kernel/cpu/resctrl/internal.h > @@ -37,6 +37,9 @@ struct arch_mbm_state { > u64 prev_msr; > }; > > +/* Setting bit 0 in L3_QOS_EXT_CFG enables the ABMC feature. */ > +#define ABMC_ENABLE_BIT 0 > + > /** > * struct rdt_hw_ctrl_domain - Arch private attributes of a set of CPUs that share > * a resource for a control function > @@ -102,6 +105,7 @@ struct msr_param { > * @mon_scale: cqm counter * mon_scale = occupancy in bytes > * @mbm_width: Monitor width, to detect and correct for overflow. > * @cdp_enabled: CDP state of this resource > + * @mbm_cntr_assign_enabled: ABMC feature is enabled > * > * Members of this structure are either private to the architecture > * e.g. mbm_width, or accessed via helpers that provide abstraction. e.g. > @@ -115,6 +119,7 @@ struct rdt_hw_resource { > unsigned int mon_scale; > unsigned int mbm_width; > bool cdp_enabled; > + bool mbm_cntr_assign_enabled; > }; > > static inline struct rdt_hw_resource *resctrl_to_arch_res(struct rdt_resource *r) > diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c > index fd2761d9f3f7..ff4b2abfa044 100644 > --- a/arch/x86/kernel/cpu/resctrl/monitor.c > +++ b/arch/x86/kernel/cpu/resctrl/monitor.c > @@ -405,3 +405,46 @@ void __init intel_rdt_mbm_apply_quirk(void) > mbm_cf_rmidthreshold = mbm_cf_table[cf_index].rmidthreshold; > mbm_cf = mbm_cf_table[cf_index].cf; > } > + > +static void resctrl_abmc_set_one_amd(void *arg) > +{ > + bool *enable = arg; > + > + if (*enable) > + msr_set_bit(MSR_IA32_L3_QOS_EXT_CFG, ABMC_ENABLE_BIT); > + else > + msr_clear_bit(MSR_IA32_L3_QOS_EXT_CFG, ABMC_ENABLE_BIT); > +} > + > +/* > + * ABMC enable/disable requires update of L3_QOS_EXT_CFG MSR on all the CPUs > + * associated with all monitor domains. > + */ > +static void _resctrl_abmc_enable(struct rdt_resource *r, bool enable) > +{ > + struct rdt_mon_domain *d; > + It remains a challenge to consider these building blocks without insight into how/when they will be used. To help out, please add guardrails to help with review. For example, this could benefit from a: lockdep_assert_cpus_held(); > + list_for_each_entry(d, &r->mon_domains, hdr.list) { > + on_each_cpu_mask(&d->hdr.cpu_mask, > + resctrl_abmc_set_one_amd, &enable, 1); > + resctrl_arch_reset_rmid_all(r, d); > + } > +} > + > +int resctrl_arch_mbm_cntr_assign_set(struct rdt_resource *r, bool enable) > +{ > + struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r); > + > + if (r->mon.mbm_cntr_assignable && > + hw_res->mbm_cntr_assign_enabled != enable) { > + _resctrl_abmc_enable(r, enable); > + hw_res->mbm_cntr_assign_enabled = enable; > + } > + > + return 0; > +} > + > +inline bool resctrl_arch_mbm_cntr_assign_enabled(struct rdt_resource *r) This "inline" in the .c file is unexpected. Why is this needed? > +{ > + return resctrl_to_arch_res(r)->mbm_cntr_assign_enabled; > +} > diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h > index 065fb6e38933..bdb264875ef6 100644 > --- a/include/linux/resctrl.h > +++ b/include/linux/resctrl.h > @@ -428,6 +428,9 @@ static inline u32 resctrl_get_config_index(u32 closid, > bool resctrl_arch_get_cdp_enabled(enum resctrl_res_level l); > int resctrl_arch_set_cdp_enabled(enum resctrl_res_level l, bool enable); > > +bool resctrl_arch_mbm_cntr_assign_enabled(struct rdt_resource *r); > +int resctrl_arch_mbm_cntr_assign_set(struct rdt_resource *r, bool enable); > + > /* > * Update the ctrl_val and apply this config right now. > * Must be called on one of the domain's CPUs. Reinette
