Re: [PATCH v11 01/23] x86/resctrl: Give better names to X86_FEATURE flags for monitoring

[Reinette Chatre]

Hi Tony,

Please switch the subject prefix to "x86/cpufeatures:" to highlight the
subsystem changed. Considering that, the short description could mention
resctrl instead, for example:
	x86/cpufeatures: Give better names to flags used by resctrl

Although, I do have a question about one rename and depending on that
outcome the subject could be made more specific.

On 8/31/26 10:43 AM, Tony Luck wrote:
> The feature flags for enumeration of Resource Director Technology (RDT)
> capabilities were chosen when the only feature was LLC cache occupancy
> monitoring and they were given names using the abbreviation CQM for
> Cache Quality of Service Monitoring.
> 
> Additional monitoring features have been added to CPUs and the names
> are now more likely to confuse than inform the purpose of these flags.
> 
> Rename X86_FEATURE_CQM to X86_FEATURE_RDT_M (to match the Intel Software
> Developer's Manual, and for symmetry with X86_FEATURE_RDT_A).
> 
> Rename X86_FEATURE_CQM_LLC to X86_FEATURE_L3_MON since it enumerates
> that some L3 monitoring features may be present.

Considering LLC as synonym for L3 it is not obvious why this rename is needed
(more below). 

> 
> Add missing dependency to cpuid_deps[].

nit: dependency -> dependencies

> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
> index f70ee74b5f92..6a7f0adb123e 100644
> --- a/arch/x86/include/asm/cpufeatures.h
> +++ b/arch/x86/include/asm/cpufeatures.h

...

> @@ -285,7 +285,7 @@
>   *
>   * Reuse free bits when adding new feature flags!
>   */
> -#define X86_FEATURE_CQM_LLC		(11*32+ 0) /* "cqm_llc" LLC QoS if 1 */
> +#define X86_FEATURE_L3_MON		(11*32+ 0) /* "cqm_llc" LLC QoS if 1 */

The original name matched the description and since the description needed no changing it
is not clear why the feature name needed to change? I do see some redundancy in the name with
"LLC" as well "cache" making an appearance, but none of that is inaccurate, is it? The changelog
claims that the name confuses the purpose. How does X86_FEATURE_CQM_LLC confuse the purpose of
the flag?

>  #define X86_FEATURE_CQM_OCCUP_LLC	(11*32+ 1) /* "cqm_occup_llc" LLC occupancy monitoring */
>  #define X86_FEATURE_CQM_MBM_TOTAL	(11*32+ 2) /* "cqm_mbm_total" LLC Total MBM monitoring */
>  #define X86_FEATURE_CQM_MBM_LOCAL	(11*32+ 3) /* "cqm_mbm_local" LLC Local MBM monitoring */
Reinette