Re: [PATCH v9 23/31] x86/resctrl: Add energy/perf choices to rdt boot option

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

Hi Tony,

On 8/29/25 12:33 PM, Tony Luck wrote:
> Legacy resctrl features are enumerated by X86_FEATURE_* flags. These
> may be overridden by quirks to disable features in the case of errata.
> 
> Users can use kernel command line options to either disable a feature,
> or to force enable a feature that was disabled by a quirk.

Above two sentences can be merged into a paragraph.

> 
> Provide similar functionality for hardware features that do not have an
> X86_FEATURE_* flag.
> 
> Unlike other features that are tied to X86_FEATURE_* flags, these must
> be queried by name. Add rdt_is_feature_enabled() to check whether quirks
> or kernel command line have disabled a feature.

Above two sentences can be merged into a paragraph.

> 
> Users may force a feature to be disabled. E.g. "rdt=!perf" will ensure
> that none of the perf telemetry events are enabled.
> 
> Resctrl architecture code may disable a feature that does not provide
> full functionality. Users may override that decision.  E.g. "rdt=energy"
> will enable any available energy telemetry events even if they do not
> provide full functionality.
> 
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> 
> starting # with '#' will be ignored, and an empty message
> aborts the commit.  # # Date:	   Mon Aug 11 09:36:45 2025
> -0700 # # On branch rdt-aet-v9-wip # Changes to be committed:
> modified:   arch/x86/kernel/cpu/resctrl/core.c #       modified:
> arch/x86/kernel/cpu/resctrl/internal.h # # Untracked files: #
> 20250811181709.6241-1-tony.luck@intel.com.mbx #       cscope-x86.out #
> cscope-x86.out.in #	  cscope-x86.out.po #	    v8/ #

Mangled changelog.

> ---
>  .../admin-guide/kernel-parameters.txt         |  2 +-
>  arch/x86/kernel/cpu/resctrl/internal.h        |  2 ++
>  arch/x86/kernel/cpu/resctrl/core.c            | 30 +++++++++++++++++++
>  3 files changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 747a55abf494..b600e1b47b0c 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -6154,7 +6154,7 @@
>  	rdt=		[HW,X86,RDT]
>  			Turn on/off individual RDT features. List is:
>  			cmt, mbmtotal, mbmlocal, l3cat, l3cdp, l2cat, l2cdp,
> -			mba, smba, bmec.
> +			mba, smba, bmec, energy, perf.
>  			E.g. to turn on cmt and turn off mba use:
>  				rdt=cmt,!mba
>  
> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> index b054c7cd13f1..6948803721e4 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -177,6 +177,8 @@ void __init intel_rdt_mbm_apply_quirk(void);
>  
>  void rdt_domain_reconfigure_cdp(struct rdt_resource *r);
>  
> +bool rdt_is_feature_enabled(char *name);
> +
>  #ifdef CONFIG_X86_CPU_RESCTRL_INTEL_AET
>  bool intel_aet_get_events(void);
>  void __exit intel_aet_exit(void);
> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> index 5745c6979293..d6e7a9125a10 100644
> --- a/arch/x86/kernel/cpu/resctrl/core.c
> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> @@ -763,6 +763,8 @@ enum {
>  	RDT_FLAG_MBA,
>  	RDT_FLAG_SMBA,
>  	RDT_FLAG_BMEC,
> +	RDT_FLAG_ENERGY,
> +	RDT_FLAG_PERF,
>  };
>  
>  #define RDT_OPT(idx, n, f)	\
> @@ -788,6 +790,8 @@ static struct rdt_options rdt_options[]  __ro_after_init = {
>  	RDT_OPT(RDT_FLAG_MBA,	    "mba",	X86_FEATURE_MBA),
>  	RDT_OPT(RDT_FLAG_SMBA,	    "smba",	X86_FEATURE_SMBA),
>  	RDT_OPT(RDT_FLAG_BMEC,	    "bmec",	X86_FEATURE_BMEC),
> +	RDT_OPT(RDT_FLAG_ENERGY,    "energy",	0),
> +	RDT_OPT(RDT_FLAG_PERF,	    "perf",	0),
>  };
>  #define NUM_RDT_OPTIONS ARRAY_SIZE(rdt_options)
>  
> @@ -837,6 +841,32 @@ bool rdt_cpu_has(int flag)
>  	return ret;
>  }
>  
> +/*
> + * Hardware features that do not have X86_FEATURE_* bits.
> + * There is no "hardware does not support this at all" case.
> + * Assume that the caller has already determined that hardware
> + * support is present and just needs to check if the feature has been
> + * disabled by a quirk that has not been overridden by a command
> + * line option.

Another recurring topic. Please do not unnecessarily trim comments to be
so far under 80 columns.

> + */
> +bool rdt_is_feature_enabled(char *name)
> +{
> +	struct rdt_options *o;
> +	bool ret = true;
> +
> +	for (o = rdt_options; o < &rdt_options[NUM_RDT_OPTIONS]; o++) {
> +		if (!strcmp(name, o->name)) {
> +			if (o->force_off)
> +				ret = false;
> +			if (o->force_on)
> +				ret = true;
> +			break;
> +		}
> +	}
> +
> +	return ret;
> +}
> +
>  bool resctrl_arch_is_evt_configurable(enum resctrl_event_id evt)
>  {
>  	if (!rdt_cpu_has(X86_FEATURE_BMEC))

Reinette