Re: [PATCH v11 11/23] x86/resctrl: Introduce mbm_cntr_cfg to track assignable counters at domain

[Moger, Babu]

On 2/21/2025 12:35 PM, Reinette Chatre wrote:
> Hi James,
>
> On 2/21/25 10:07 AM, James Morse wrote:
> > Hi Babu,
> >
> > On 22/01/2025 20:20, Babu Moger wrote:
> > > In mbm_cntr_assign mode hardware counters are assigned/unassigned to an
> > > MBM event of a monitor group. Hardware counters are assigned/unassigned
> > > at monitoring domain level.
> > >
> > > Manage a monitoring domain's hardware counters using a per monitoring
> > > domain array of struct mbm_cntr_cfg that is indexed by the hardware
> > > counter	ID. A hardware counter's configuration contains the MBM event
> > > ID and points to the monitoring group that it is assigned to, with a
> > > NULL pointer meaning that the hardware counter is available for assignment.
> > >
> > > There is no direct way to determine which hardware counters are	assigned
> > > to a particular monitoring group. Check every entry of every hardware
> > > counter	configuration array in every monitoring domain to query which
> > > MBM events of a monitoring group is tracked by hardware. Such queries
> > > are acceptable because of a very small number of assignable counters.
> >
> > > diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
> > > index 511cfce8fc21..9a54e307d340 100644
> > > --- a/include/linux/resctrl.h
> > > +++ b/include/linux/resctrl.h
> > > @@ -94,6 +94,18 @@ struct rdt_ctrl_domain {
> > >   	u32				*mbps_val;
> > >   };
> > >   
> > > +/**
> > > + * struct mbm_cntr_cfg - assignable counter configuration
> > > + * @evtid:		 MBM event to which the counter is assigned. Only valid
> > > + *			 if @rdtgroup is not NULL.
> > > + * @rdtgroup:		 resctrl group assigned to the counter. NULL if the
> > > + *			 counter is free.
> > > + */
> > > +struct mbm_cntr_cfg {
> > > +	enum resctrl_event_id	evtid;
> > > +	struct rdtgroup		*rdtgrp;
> > > +};
> >
> > struct rdtgroup here suggests this shouldn't be something the arch code is touching.
> >
> > If its not needed by any arch specific code, (I couldn't find a resctrl_arch helper that
> > takes this) - could it be moved to resctrl's internal.h.
> >
> > (If this does need to be visible to the arch code, one option would be to replace rdtgroup
> > with the closid/rmid, and a valid flag so that memset() continues to reset these entries)
>
> Thank you for catching this!
>
> Reinette

Sure. Will move it to arch/x86/kernel/cpu/resctrl/internal.h.

thanks
Babu