Re: [PATCH v2 11/16] x86/resctrl: Link known events onto RDT_RESOURCE_INTEL_AET.evt_list

["Luck, Tony" <tony.luck@intel.com>]

On Mon, Mar 31, 2025 at 09:23:14AM -0700, Reinette Chatre wrote:
> Hi Tony,
> 
> On 3/21/25 4:16 PM, Tony Luck wrote:
> > Core code uses this list to populate "mon_data" directories.
> > 
> > Signed-off-by: Tony Luck <tony.luck@intel.com>
> > ---
> >  arch/x86/kernel/cpu/resctrl/intel_aet.c | 23 ++++++++++++++++++++++-
> >  1 file changed, 22 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/x86/kernel/cpu/resctrl/intel_aet.c b/arch/x86/kernel/cpu/resctrl/intel_aet.c
> > index e2d8eab997fc..9ac912742ef1 100644
> > --- a/arch/x86/kernel/cpu/resctrl/intel_aet.c
> > +++ b/arch/x86/kernel/cpu/resctrl/intel_aet.c
> > @@ -13,6 +13,7 @@
> >  
> >  #include <linux/cpu.h>
> >  #include <linux/cleanup.h>
> > +#include <linux/minmax.h>
> >  #include <linux/slab.h>
> >  #include "fake_intel_aet_features.h"
> >  #include <linux/intel_vsec.h>
> > @@ -308,14 +309,34 @@ void rdt_get_intel_aet_mount(void)
> >  {
> >  	struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_INTEL_AET].r_resctrl;
> >  	struct rdt_core_mon_domain *d, *tmp;
> > +	struct telem_entry **tentry;
> >  	static int do_one_time;
> > +	struct mon_evt *evt;
> > +	bool ret = false;
> >  
> >  	if (do_one_time)
> >  		return;
> >  
> >  	do_one_time = 1;
> >  
> > -	if (!get_events()) {
> > +	if (!get_events())
> > +		goto done;
> > +
> > +	for (tentry = telem_entry; *tentry; tentry++) {
> > +		if (!(*tentry)->active)
> > +			continue;
> > +		for (int i = 0; (*tentry)->evts[i].evt.name; i++) {
> > +			evt = &(*tentry)->evts[i].evt;
> > +			list_add_tail(&evt->list, &r->evt_list);
> > +			ret = true;
> > +		}
> 
> Architecture code should not be doing this. I expect this will be something
> similar to l3_mon_evt_init() done by fs code after the architecture had
> opportunity to configure which events are supported. 

If we do put all possible events into the architecture code, then it can
do something like:

	for_each_set_bit(evt, &rdt_mon_features, NUM_EVENTS)
		list_add(...);

My speculative patch:
	https://lore.kernel.org/all/20250314202609.5753-1-tony.luck@intel.com/
did this for the existing events, so could build on it.

> I wonder if resctrl_mon_resource_init() could be moved to rdt_get_tree()
> to be called after the new (yet to be named) "resctrl_arch_for_arch_to_do_needed_on_resctrl_mount()"
> resctrl_mon_resource_init() could be enhanced to do any needed resctrl fs
> initialization for this new feature. This will include being able to 
> learn the accurate counts of rmid supported by the system to be able to
> create monitor groups that can be supported by all monitoring resources?
> 
> > +		if (!r->num_rmid)
> > +			r->num_rmid = (*tentry)->num_rmids;
> > +		else
> > +			r->num_rmid = min(r->num_rmid, (*tentry)->num_rmids);
> > +	}
> > +done:
> > +	if (!ret) {
> >  		list_for_each_entry_safe(d, tmp, &r->mon_domains, hdr.list)
> >  			kfree(d);
> >  		r->mon_capable = false;
> 
> Reinette

-Tony