Re: [PATCH v9 24/31] x86/resctrl: Handle number of RMIDs supported by telemetry resources

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

Hi Tony,

On 8/29/25 12:33 PM, Tony Luck wrote:
> There are now three meanings for "number of RMIDs":
> 
> 1) The number for legacy features enumerated by CPUID leaf 0xF. This
> is the maximum number of distinct values that can be loaded into the
> IA32_PQR_ASSOC MSR. Note that systems with Sub-NUMA Cluster mode enabled
> will force scaling down the CPUID enumerated value by the number of SNC
> nodes per L3-cache.
> 
> 2) The number of registers in MMIO space for each event. This
> is enumerated in the XML files and is the value initialized into
> event_group::num_rmids. This will be overwritten with a lower
> value if hardware does not support all these registers at the
> same time (see next case).

Same comment as v8. :(

> 
> 3) The number of "hardware counters" (this isn't a strictly accurate
> description of how things work, but serves as a useful analogy that
> does describe the limitations) feeding to those MMIO registers. This
> is enumerated in telemetry_region::num_rmids returned from the call to
> intel_pmt_get_regions_by_feature()
> 
> Event groups with insufficient "hardware counters" to track all RMIDs
> are difficult for users to use, since the system may reassign "hardware
> counters" at any time. This means that users cannot reliably collect
> two consecutive event counts to compute the rate at which events are
> occurring.
> 
> Use rdt_set_feature_disabled() to mark any under-resourced event groups

Use -> Introduce

> (those with telemetry_region::num_rmids < event_group::num_rmids) as
> unusable.  Note that the rdt_options[] structure must now be writable
> at run-time.  The request to disable will be overridden if the user
> explicitly requests to enable using the "rdt=" Linux boot argument.

Can add snippet here about how values are adjusted in this case.

> 
> Scan all enabled event groups and assign the RDT_RESOURCE_PERF_PKG
> resource "num_rmids" value to the smallest of these values as this value
> will be used later to compare against the number of RMIDs supported by
> other resources.

can append "... to determine how many monitoring resource groups are            
supported."

> 
> N.B. Changed type of rdt_resource::num_rmid to u32 to match type of

Same comment as v8. :(

> event_group::num_rmids so that min(r->num_rmid, e->num_rmids) won't
> complain about mixing signed and unsigned types.  Print r->num_rmid as
> unsigned value in rdt_num_rmids_show().
> 
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> ---

...

> diff --git a/arch/x86/kernel/cpu/resctrl/intel_aet.c b/arch/x86/kernel/cpu/resctrl/intel_aet.c
> index 71aed96f9499..f06f71c06029 100644
> --- a/arch/x86/kernel/cpu/resctrl/intel_aet.c
> +++ b/arch/x86/kernel/cpu/resctrl/intel_aet.c
> @@ -15,6 +15,7 @@
>  #include <linux/cpu.h>
>  #include <linux/intel_vsec.h>
>  #include <linux/io.h>
> +#include <linux/minmax.h>
>  #include <linux/resctrl.h>
>  
>  #include "internal.h"
> @@ -36,20 +37,26 @@ struct pmt_event {
>  
>  /**
>   * struct event_group - All information about a group of telemetry events.
> + * @name:		Name for this group (used by boot rdt= option)
>   * @pfg:		Points to the aggregated telemetry space information
>   *			within the INTEL_PMT_TELEMETRY driver that contains data for all
>   *			telemetry regions.
>   * @guid:		Unique number per XML description file.
> + * @num_rmids:		Number of RMIDs supported by this group. May be djusted downwards

djusted -> adjusted
Also, please stay within 80 columns.

> + *			if enumeration from intel_pmt_get_regions_by_feature() indicates
> + *			fewer RMIDs can be tracked simultaneously.
>   * @mmio_size:		Number of bytes of MMIO registers for this group.
>   * @num_events:		Number of events in this group.
>   * @evts:		Array of event descriptors.
>   */
>  struct event_group {
>  	/* Data fields for additional structures to manage this group. */
> +	char				*name;
>  	struct pmt_feature_group	*pfg;
>  
>  	/* Remaining fields initialized from XML file. */
>  	u32				guid;
> +	u32				num_rmids;
>  	size_t				mmio_size;
>  	unsigned int			num_events;
>  	struct pmt_event		evts[] __counted_by(num_events);

Reinette