Re: [PATCH v9 20/31] x86/resctrl: Find and enable usable telemetry events

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

On Fri, 29 Aug 2025, Tony Luck wrote:

> The INTEL_PMT driver provides telemetry region structures of the
> types requested by resctrl.
> 
> Scan these structures to discover which pass sanity checks:
> 
> 1) They have guid known to resctrl.
> 2) They have a valid package ID.
> 3) The enumerated size of the MMIO region matches the expected
>    value from the XML description file.
> 4) At least one region passes the above checks.
> 
> Enable the active events in resctrl filesystem to make them available to
> user space. Pass a pointer to the pmt_event structure of the event within
> the struct event_group that resctrl stores in mon_evt::arch_priv. resctrl
> passes this pointer back when asking to read the event data which enables
> the data to be found in MMIO.
> 
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> ---
>  arch/x86/kernel/cpu/resctrl/intel_aet.c | 36 +++++++++++++++++++++++--
>  1 file changed, 34 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/intel_aet.c b/arch/x86/kernel/cpu/resctrl/intel_aet.c
> index 565777841f5c..5c5466dc3189 100644
> --- a/arch/x86/kernel/cpu/resctrl/intel_aet.c
> +++ b/arch/x86/kernel/cpu/resctrl/intel_aet.c
> @@ -102,12 +102,44 @@ static struct event_group *known_perf_event_groups[] = {
>  	for (_peg = _grp; _peg < &_grp[ARRAY_SIZE(_grp)]; _peg++)	\
>  		if ((*_peg)->pfg)
>  
> -/* Stub for now */
> -static bool enable_events(struct event_group *e, struct pmt_feature_group *p)
> +static bool skip_this_region(struct telemetry_region *tr, struct event_group *e)

skip_telem_region? It would get rid of vague "this" in the name.

>  {
> +	if (tr->guid != e->guid)
> +		return true;
> +	if (tr->plat_info.package_id >= topology_max_packages()) {
> +		pr_warn_once("Bad package %d in guid 0x%x\n", tr->plat_info.package_id,
> +			     tr->guid);
> +		return true;
> +	}
> +	if (tr->size != e->mmio_size) {
> +		pr_warn_once("MMIO space wrong size (%zu bytes) for guid 0x%x. Expected %zu bytes.\n",
> +			     tr->size, e->guid, e->mmio_size);

Are _once warranted in these two cases? Do we call this function for the 
same guid and region combination more than once?

+ include for pr_*().

> +		return true;
> +	}
> +
>  	return false;
>  }
>  
> +static bool enable_events(struct event_group *e, struct pmt_feature_group *p)
> +{
> +	bool usable_events = false;
> +
> +	for (int i = 0; i < p->count; i++) {
> +		if (skip_this_region(&p->regions[i], e))
> +			continue;
> +		usable_events = true;

Do you need to loop beyond asserting this?

> +	}
> +
> +	if (!usable_events)
> +		return false;
> +
> +	for (int j = 0; j < e->num_events; j++)
> +		resctrl_enable_mon_event(e->evts[j].id, true,
> +					 e->evts[j].bin_bits, &e->evts[j]);
> +
> +	return true;
> +}
> +
>  DEFINE_FREE(intel_pmt_put_feature_group, struct pmt_feature_group *,
>  		if (!IS_ERR_OR_NULL(_T))
>  			intel_pmt_put_feature_group(_T))
> 

-- 
 i.