Re: [PATCH v6 17/30] x86/resctrl: Discover hardware telemetry events

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

Hi Tony,

On 7/3/25 2:11 PM, Luck, Tony wrote:
> On Thu, Jul 03, 2025 at 01:31:19PM -0700, Reinette Chatre wrote:
>> I do not think resctrl should enforce dependency on a driver that is not
>> valid for a platform.
> 
> Fewer stubs than I thought.  I can merge something along these
> lines back into the series for the next version.
> 
> Suggestions welcome for the name of the config option. Do
> I need a "_CPU" in CONFIG_X86_RESCTRL_INTEL_AET? It's already
> very long.

Looking at other config options in the same file it does not seem
as though this new name is exceedingly long. I'd vote for keeping the
"_CPU" with the motivation that doing so maintains a "namespace" prefix
for the resctrl options.

> 
> "help" text is a placeholder. I can change that up to add more
> details.
> 
> -Tony
> 
> ---
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> index 11f25c225837..56615b1d3fc3 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -175,9 +175,19 @@ bool rdt_is_software_feature_enabled(char *option);
>  
>  bool rdt_is_software_feature_force_enabled(char *name);
>  
> +#ifdef CONFIG_X86_RESCTRL_INTEL_AET
>  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,
>  			 void *arch_priv, u64 *val);
> +#else
> +static inline bool intel_aet_get_events(void) { return false; }
> +static inline void __exit intel_aet_exit(void) { }
> +static inline int intel_aet_read_event(int domid, int rmid, enum resctrl_event_id evtid,
> +				       void *arch_priv, u64 *val)
> +{
> +	return -EINVAL;
> +}
> +#endif
>  
>  #endif /* _ASM_X86_RESCTRL_INTERNAL_H */
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index a6b6ecbd3877..ceb3eb371a3d 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -512,9 +512,6 @@ config X86_CPU_RESCTRL
>  	select ARCH_HAS_CPU_RESCTRL
>  	select RESCTRL_FS
>  	select RESCTRL_FS_PSEUDO_LOCK
> -	select X86_PLATFORM_DEVICES
> -	select INTEL_VSEC
> -	select INTEL_PMT_TELEMETRY
>  	help
>  	  Enable x86 CPU resource control support.
>  
> @@ -531,6 +528,18 @@ config X86_CPU_RESCTRL
>  
>  	  Say N if unsure.
>  
> +config X86_RESCTRL_INTEL_AET
> +	bool "Intel Application Energy Telemetry"
> +	depends on X86_CPU_RESCTRL && CPU_SUP_INTEL && INTEL_PMT_DISCOVERY

Thank you. This pattern looks more appropriate to me. Do you expect that
the X86_64 dependency (added in patch #22) will move here also?

> +	help
> +	  Enable per-RMID telemetry events in resctrl
> +
> +	  Intel feature that collects per-RMID execution data
> +	  including core energy consumed by tasks. Data is aggregated
> +	  per package.
> +
> +	  Say N if unsure.
> +
>  config X86_FRED
>  	bool "Flexible Return and Event Delivery"
>  	depends on X86_64
> diff --git a/arch/x86/kernel/cpu/resctrl/Makefile b/arch/x86/kernel/cpu/resctrl/Makefile
> index 97ceb4e44dfa..26fc957fb3dd 100644
> --- a/arch/x86/kernel/cpu/resctrl/Makefile
> +++ b/arch/x86/kernel/cpu/resctrl/Makefile
> @@ -1,7 +1,7 @@
>  # SPDX-License-Identifier: GPL-2.0
>  obj-$(CONFIG_X86_CPU_RESCTRL)		+= core.o rdtgroup.o monitor.o
>  obj-$(CONFIG_X86_CPU_RESCTRL)		+= ctrlmondata.o
> -obj-$(CONFIG_X86_CPU_RESCTRL)		+= intel_aet.o
> +obj-$(CONFIG_X86_RESCTRL_INTEL_AET)	+= intel_aet.o
>  obj-$(CONFIG_RESCTRL_FS_PSEUDO_LOCK)	+= pseudo_lock.o
>  
>  # To allow define_trace.h's recursive include:

Reinette