linux-kernel - Re: [PATCH v6 24/30] x86/resctrl: Add energy/perf choices to rdt boot option

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f95e4868-0f87-4b58-b4a8-4fd18dcbf6ad@intel.com>
Date: Wed, 9 Jul 2025 15:14:52 -0700
From: Reinette Chatre <reinette.chatre@...el.com>
To: Tony Luck <tony.luck@...el.com>, Fenghua Yu <fenghuay@...dia.com>, "Maciej
 Wieczor-Retman" <maciej.wieczor-retman@...el.com>, Peter Newman
	<peternewman@...gle.com>, James Morse <james.morse@....com>, Babu Moger
	<babu.moger@....com>, Drew Fustini <dfustini@...libre.com>, Dave Martin
	<Dave.Martin@....com>, Anil Keshavamurthy <anil.s.keshavamurthy@...el.com>,
	Chen Yu <yu.c.chen@...el.com>
CC: <x86@...nel.org>, <linux-kernel@...r.kernel.org>,
	<patches@...ts.linux.dev>
Subject: Re: [PATCH v6 24/30] x86/resctrl: Add energy/perf choices to rdt boot
 option

Hi Tony,

On 6/26/25 9:49 AM, Tony Luck wrote:
> Hardware backed 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.
> 
> Provide similar functionality for software defined features that do not
> have an X86_FEATURE_* flag.
> 
> Unlike other options that are tied to X86_FEATURE_* flags, these must be
> queried by name. Add rdt_is_software_feature_enabled() to check whether
> quirks or kernel command line have disabled a feature. Just like the
> hardware feature options the command line enable overrides quirk disable.

Referring to "Intel Application Energy Telemetry" as a software feature does
not sound right. It is a hardware feature, no? Perhaps just "a hardware feature
that does not have a X86_FEATURE_* flag"? With this rdt_is_software_feature_enabled()
could perhaps just be rdt_is_feature_enabled() (open to other ideas)?

The idea that a "software feature" may have a "quirk" also sounds wrong ...
to me that sounds like a different way of saying a "software bug" and we do not
handle software bugs with quirks. Referring to it as a "software feature" just
does not sound right.

> 
> Signed-off-by: Tony Luck <tony.luck@...el.com>
> ---
>  .../admin-guide/kernel-parameters.txt         |  2 +-
>  arch/x86/kernel/cpu/resctrl/internal.h        |  2 ++
>  arch/x86/kernel/cpu/resctrl/core.c            | 30 +++++++++++++++++++
>  arch/x86/kernel/cpu/resctrl/intel_aet.c       |  7 +++++
>  4 files changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index f1f2c0874da9..4c12159f3ea0 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -6066,7 +6066,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 e8d2a754bc0c..ee1c6204722e 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -169,6 +169,8 @@ void __init intel_rdt_mbm_apply_quirk(void);
>  
>  void rdt_domain_reconfigure_cdp(struct rdt_resource *r);
>  
> +bool rdt_is_software_feature_enabled(char *option);
> +
>  bool intel_aet_get_events(void);
>  void __exit intel_aet_exit(void);
>  int intel_aet_read_event(int domid, int rmid, enum resctrl_event_id evtid,
> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> index f857f92e7b8b..f9f3bc58290e 100644
> --- a/arch/x86/kernel/cpu/resctrl/core.c
> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> @@ -791,6 +791,8 @@ enum {
>  	RDT_FLAG_MBA,
>  	RDT_FLAG_SMBA,
>  	RDT_FLAG_BMEC,
> +	RDT_FLAG_ENERGY,
> +	RDT_FLAG_PERF,
>  };
>  
>  #define RDT_OPT(idx, n, f)	\
> @@ -816,6 +818,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)
>  
> @@ -865,6 +869,32 @@ bool rdt_cpu_has(int flag)
>  	return ret;
>  }
>  
> +/*
> + * Software options that are not based on X86_FEATURE_* bits.

"Hardware features that do not have X86_FEATURE_* bits"?

> + * There is no "h/w does not support this at all" case.
> + * Assume that the caller has already determined that s/w

(please expand h/w to hardware, s/w to software)

> + * support is present and just needs to check if the option has been

Looking at how this function is used I rather interpret
intel_pmt_get_regions_by_feature() as determining that hardware
support is present.

> + * disabled by a quirk that has not been overridden * by a command

(stray *)

> + * line option.
> + */
> +bool rdt_is_software_feature_enabled(char *name)
> +{

Reinette