Re: [PATCH v15 16/34] x86,fs/resctrl: Implement resctrl_arch_config_cntr() to assign a counter with ABMC

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

Hi Babu,

On 7/8/25 3:17 PM, Babu Moger wrote:
> The ABMC feature allows users to assign a hardware counter 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.

From above description it is not obvious to me how configuring a counter
is connected to assign/unassign/configure. How about something like:

  Implement an x86 architecture-specific handler to configure a counter. This
  architecture specific handler is called by resctrl fs when a counter
  is assigned or unassigned as well as when an already assigned counter's
  configuration should be updated. Configure counters by writing to the
  L3_QOS_ABMC_CFG MSR, specifying the counter ID, bandwidth source (RMID),
  and event configuration.

(please feel free to improve)

> 
> 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@amd.com>
> ---

...

> +/*
> + * 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);
> +
> +	/*
> +	 * The hardware counter is reset (because cfg_en == 1) so there is no
> +	 * need to record initial non-zero counts.
> +	 */
> +	if (assign) {

We learned from patch #14 that the counter is reset whenever cfg_en = 1. Since this
is always the case it is not clear to me why the architectural state is not always
reset to match what hardware does. Even more, looking how this function is later used in
resctrl_config_cntr(), the caller resctrl_config_cntr() always resets non-architectural
state, why not always reset architectural state here?


> +		am = get_arch_mbm_state(hw_dom, rmid, evtid);
> +		if (am)
> +			memset(am, 0, sizeof(*am));
> +	}
> +}

Reinette