Re: [PATCH v12 20/25] x86/resctrl: Enforce system RMID limit on AET

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

Hi Tony,

On 9/16/26 4:13 PM, Tony Luck wrote:
> AET (Application Energy Telemetry) event groups each support a specific
> number of RMIDs. But that number may be lower than the number supported

Is it really a problem if an event group supports a lower number of RMID
than what the system can support? Was this intended to be "higher" instead?

> by the system. This is especially true on systems with SNC (Sub-NUMA Cluster)
> enabled as that reduces the number of supported RMIDs.

The last sentence seems out of place. Does it really apply here? This is
about adjusting the RMIDs of the RDT_RESOURCE_PERF_PKG resource to what
the *system* supports, it has nothing to do with the L3 resource which is
the only resource where SNC adjustment comes in, no?

> 
> Reduce r->mon.num_rmid if needed to ensure that info/PERF_PKG_MON/num_rmids

Please replace verbatim "Reduce r->mon.num_rmid" with what it represents.

> does not display an unachievable value to users.
> 
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> ---
> v12:
> 	Replacement for v11 patch 14 "Enforce system RMID limit ..."
> ---
>  arch/x86/kernel/cpu/resctrl/core.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> index 3bf4d1a07593..a20439a5099a 100644
> --- a/arch/x86/kernel/cpu/resctrl/core.c
> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> @@ -798,6 +798,12 @@ void resctrl_arch_pre_mount(void)
>  	if (!intel_aet_pre_mount())
>  		return;
>  
> +	/*
> +	 * AET may claim to support more RMIDs than are permitted to be

 Contradiction between "may claim to support *more*" above and 
"that number may be lower" in changelog.

> +	 * assigned to IA32_PQR_ASSOC.RMID. Enforce this limit.

Please use grep friendly MSR names. "IA32_PQR_ASSOC.RMID" -> "MSR_IA32_PQR_ASSOC.RMID"

> +	 */
> +	r->mon.num_rmid = min(r->mon.num_rmid, pqr_assoc_num_rmid);
> +

This is AET specific enumeration code. Could placing it in aet_get_events()
that is closer to AET specific enumeration and original initialization
of r->mon.num_rmid be more appropriate?

>  	/*
>  	 * Late discovery of telemetry events means the domains for the
>  	 * resource were not built. Do that now.

Reinette