Re: [PATCH v9 07/12] x86/resctrl: Add PMT registration API for AET enumeration callbacks

[Luck, Tony]

On Wed, Jul 08, 2026 at 03:51:22PM -0700, Reinette Chatre wrote:
> Hi Tony,
> 
> On 7/1/26 2:35 PM, Tony Luck wrote:
> > resctrl is always built-in; INTEL_PMT_TELEMETRY may be a module.  Add, and
> > export, register/unregister functions so the PMT module can supply/clear
> > enumeration callback functions when loaded/unloaded.
> > 
> > Suggested-by: Christoph Hellwig <hch@xxxxxxxxxxxxx>
> > Signed-off-by: Tony Luck <tony.luck@xxxxxxxxx>
> > ---
> >  arch/x86/include/asm/resctrl.h          | 22 ++++++++++++++++
> >  arch/x86/kernel/cpu/resctrl/intel_aet.c | 34 +++++++++++++++++++++++++
> >  2 files changed, 56 insertions(+)
> > 
> > diff --git a/arch/x86/include/asm/resctrl.h b/arch/x86/include/asm/resctrl.h
> > index a9f481b7a8ed..48ad55bcda66 100644
> > --- a/arch/x86/include/asm/resctrl.h
> > +++ b/arch/x86/include/asm/resctrl.h
> > @@ -4,6 +4,8 @@
> >  
> >  #ifdef CONFIG_X86_CPU_RESCTRL
> >  
> > +#include <linux/intel_pmt_features.h>
> > +#include <linux/intel_vsec.h>
> >  #include <linux/jump_label.h>
> >  #include <linux/percpu.h>
> >  #include <linux/resctrl_types.h>
> > @@ -189,11 +191,31 @@ static inline void resctrl_arch_mon_ctx_free(struct rdt_resource *r,
> >  
> >  void resctrl_cpu_detect(struct cpuinfo_x86 *c);
> >  
> > +#ifdef CONFIG_X86_CPU_RESCTRL_INTEL_AET
> > +void intel_aet_register_enumeration(struct module *module,
> > +				    struct pmt_feature_group *(*get)(enum pmt_feature_id id),
> > +				    void (*put)(struct pmt_feature_group *p));
> > +void intel_aet_unregister_enumeration(void);
> >  #else
> > +static inline void intel_aet_register_enumeration(struct module *module,
> > +						  struct pmt_feature_group *(*get)(enum pmt_feature_id id),
> > +						  void (*put)(struct pmt_feature_group *p)) { }
> > +static inline void intel_aet_unregister_enumeration(void) { }
> > +#endif /* CONFIG_X86_CPU_RESCTRL_INTEL_AET */
> > +
> > +#else
> > +
> > +#include <linux/intel_pmt_features.h>
> > +#include <linux/intel_vsec.h>
> 
> Are we sure we want to include these in arch/x86/kernel/process_32.c and
> arch/x86/kernel/process_64.c when resctrl is not built in?

I'll look at this some more. Maybe this code needs some X86_64 checks?

> >  
> >  static inline void resctrl_arch_sched_in(struct task_struct *tsk) {}
> >  static inline void resctrl_cpu_detect(struct cpuinfo_x86 *c) {}
> >  
> > +static inline void intel_aet_register_enumeration(struct module *module,
> > +						  struct pmt_feature_group *(*get)(enum pmt_feature_id id),
> > +						  void (*put)(struct pmt_feature_group *p)) { }
> > +static inline void intel_aet_unregister_enumeration(void) { }
> > +
> >  #endif /* CONFIG_X86_CPU_RESCTRL */
> >  
> >  #endif /* _ASM_X86_RESCTRL_H */
> > diff --git a/arch/x86/kernel/cpu/resctrl/intel_aet.c b/arch/x86/kernel/cpu/resctrl/intel_aet.c
> > index cd8257c58f84..c26bd813e057 100644
> > --- a/arch/x86/kernel/cpu/resctrl/intel_aet.c
> > +++ b/arch/x86/kernel/cpu/resctrl/intel_aet.c
> > @@ -12,17 +12,21 @@
> >  #define pr_fmt(fmt)   "resctrl: " fmt
> >  
> >  #include <linux/bits.h>
> > +#include <linux/cleanup.h>
> >  #include <linux/compiler_types.h>
> >  #include <linux/container_of.h>
> >  #include <linux/cpumask.h>
> >  #include <linux/err.h>
> >  #include <linux/errno.h>
> > +#include <linux/export.h>
> >  #include <linux/gfp_types.h>
> >  #include <linux/init.h>
> >  #include <linux/intel_pmt_features.h>
> >  #include <linux/intel_vsec.h>
> >  #include <linux/io.h>
> >  #include <linux/minmax.h>
> > +#include <linux/mutex.h>
> > +#include <linux/module.h>
> >  #include <linux/printk.h>
> >  #include <linux/rculist.h>
> >  #include <linux/rcupdate.h>
> > @@ -291,6 +295,10 @@ static enum pmt_feature_id lookup_pfid(const char *pfname)
> >  	return FEATURE_INVALID;
> >  }
> >  
> > +static struct module *pmt_module;
> > +static struct pmt_feature_group *(*get_feature)(enum pmt_feature_id id);
> > +static void (*put_feature)(struct pmt_feature_group *p);
> > +
> >  /*
> >   * Request a copy of struct pmt_feature_group for each event group. If there is
> >   * one, the returned structure has an array of telemetry_region structures,
> > @@ -325,6 +333,32 @@ bool intel_aet_get_events(void)
> >  	return ret;
> >  }
> >  
> > +/*
> > + * Defend against races between pmt_telemetry register/unregister and
> > + * resctrl file system mount/unmount.
> > + */
> 
> Please help developers understand what this mutex protects. I am not able to deduce
> from above when this mutex needs to be taken.
> 
> > +static DEFINE_MUTEX(aet_register_lock);
> > +
> > +void intel_aet_register_enumeration(struct module *module,
> > +				    struct pmt_feature_group *(*get)(enum pmt_feature_id id),
> > +				    void (*put)(struct pmt_feature_group *p))
> > +{
> > +	guard(mutex)(&aet_register_lock);
> > +	get_feature = get;
> > +	put_feature = put;
> > +	pmt_module = module;
> > +}
> > +EXPORT_SYMBOL_NS_GPL(intel_aet_register_enumeration, "INTEL_PMT");
> > +
> > +void intel_aet_unregister_enumeration(void)
> > +{
> > +	guard(mutex)(&aet_register_lock);
> > +	pmt_module = NULL;
> > +	get_feature = NULL;
> > +	put_feature = NULL;
> > +}
> > +EXPORT_SYMBOL_NS_GPL(intel_aet_unregister_enumeration, "INTEL_PMT");
> > +
> >  void __exit intel_aet_exit(void)
> >  {
> >  	struct event_group **peg;
> 
> Reinette

-Tony