Re: [PATCH v11 03/23] x86/resctrl: Enumerate monitor features in rdt_get_l3_mon_config()

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

Hi Tony,

On 8/31/26 10:44 AM, Tony Luck wrote:
> The original implementation of Intel Cache QoS Monitoring (CQM) planned
> to integrate with the "perf" and "cgroup" subsystems. With that plan it
> made sense for parameters from CPUID to be stored in fields of the
> cpuinfo_x86 structure. But that plan was abandoned and the resctrl file
> system user interface replaced it.
> 
> Enumerate the L3 monitoring features in rdt_get_l3_mon_config().

It may help to append something like "... in preparation for removing resctrl
fields from cpuinfo_x86." Otherwise it looks like this patch leaves dead code
around.

The last sentence is actually confusing since rdt_get_l3_mon_config() already
enumerates the L3 monitoring features, no?
> @@ -418,16 +423,37 @@ static __init int snc_get_config(void)
>  
>  int __init rdt_get_l3_mon_config(struct rdt_resource *r)
>  {
> -	unsigned int mbm_offset = boot_cpu_data.x86_cache_mbm_width_offset;
>  	struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
> +	unsigned int mbm_offset;
>  	unsigned int threshold;
>  	u32 eax, ebx, ecx, edx;
> +	u32 num_rmid;
> +
> +	/* QoS sub-leaf, EAX=0Fh, ECX=1 */

I know you just copy&pasted ... but above is not useful. How about something like:
	/* L3 (ECX=1) monitoring details via resource monitoring (EAX=0xf) leaf. */

> +	cpuid_count(0xf, 1, &eax, &ebx, &ecx, &edx);
> +	mbm_offset = eax & 0xff;

I find GENMASK(7, 0) easier to read and matches similar enumeration in this function.
I am not aware of an x86 requirement to use one or the other though.

Reinette