Hardware has one or more telemetry event aggregators per package
for each group of telemetry events. Each aggregator provides access
to event counts in an array of 64-bit values in MMIO space. There
is a "guid" (in this case a unique 32-bit integer) which refers to
an XML file published in the https://github.com/intel/Intel-PMT
that provides all the details about each aggregator.

The XML files provide the following information:
1) Which telemetry events are included in the group for this aggregator.
2) The order in which the event counters appear for each RMID.
3) The value type of each event counter (integer or fixed-point).
4) The number of RMIDs supported.
5) Which additional aggregator status registers are included.
6) The total size of the MMIO region for this aggregator.

Add select of X86_PLATFORM_DEVICES, INTEL_VSEC and
INTEL_PMT_TELEMETRY to CONFIG_X86_CPU_RESCTRL to enable use of the
discovery driver that enumerate all aggregators on the system with
intel_pmt_get_regions_by_feature(). Call this for each pmt_feature_id
that indicates per-RMID telemetry.

Save the returned pmt_feature_group pointers with guids that are known
to resctrl for use at run time.

Those pointers are returned to the INTEL_PMT_DISCOVERY driver at
resctrl_arch_exit() time.

Signed-off-by: Tony Luck <tony.luck@intel.com>
---
 arch/x86/kernel/cpu/resctrl/internal.h  |   3 +
 arch/x86/kernel/cpu/resctrl/core.c      |   5 +
 arch/x86/kernel/cpu/resctrl/intel_aet.c | 122 ++++++++++++++++++++++++
 arch/x86/Kconfig                        |   3 +
 arch/x86/kernel/cpu/resctrl/Makefile    |   1 +
 5 files changed, 134 insertions(+)
 create mode 100644 arch/x86/kernel/cpu/resctrl/intel_aet.c

diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index 224b71730cc3..e93b15bf6aab 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -169,4 +169,7 @@ void __init intel_rdt_mbm_apply_quirk(void);
 
 void rdt_domain_reconfigure_cdp(struct rdt_resource *r);
 
+bool intel_aet_get_events(void);
+void __exit intel_aet_exit(void);
+
 #endif /* _ASM_X86_RESCTRL_INTERNAL_H */
diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
index a5f01cac2363..9144766da836 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
@@ -734,6 +734,9 @@ void resctrl_arch_pre_mount(void)
 
 	if (!atomic_try_cmpxchg(&only_once, &old, 1))
 		return;
+
+	if (!intel_aet_get_events())
+		return;
 }
 
 enum {
@@ -1086,6 +1089,8 @@ late_initcall(resctrl_arch_late_init);
 
 static void __exit resctrl_arch_exit(void)
 {
+	intel_aet_exit();
+
 	cpuhp_remove_state(rdt_online);
 
 	resctrl_exit();
diff --git a/arch/x86/kernel/cpu/resctrl/intel_aet.c b/arch/x86/kernel/cpu/resctrl/intel_aet.c
new file mode 100644
index 000000000000..b09044b093dd
--- /dev/null
+++ b/arch/x86/kernel/cpu/resctrl/intel_aet.c
@@ -0,0 +1,122 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Resource Director Technology(RDT)
+ * - Intel Application Energy Telemetry
+ *
+ * Copyright (C) 2025 Intel Corporation
+ *
+ * Author:
+ *    Tony Luck <tony.luck@intel.com>
+ */
+
+#define pr_fmt(fmt)   "resctrl: " fmt
+
+#include <linux/cleanup.h>
+#include <linux/cpu.h>
+#include <linux/intel_vsec.h>
+#include <linux/resctrl.h>
+
+#include "internal.h"
+
+/**
+ * struct event_group - All information about a group of telemetry events.
+ * @pfg:		Points to the aggregated telemetry space information
+ *			within the OOBMSM driver that contains data for all
+ *			telemetry regions.
+ * @guid:		Unique number per XML description file.
+ */
+struct event_group {
+	/* Data fields for additional structures to manage this group. */
+	struct pmt_feature_group	*pfg;
+
+	/* Remaining fields initialized from XML file. */
+	u32				guid;
+};
+
+/*
+ * Link: https://github.com/intel/Intel-PMT
+ * File: xml/CWF/OOBMSM/RMID-ENERGY/cwf_aggregator.xml
+ */
+static struct event_group energy_0x26696143 = {
+	.guid		= 0x26696143,
+};
+
+/*
+ * Link: https://github.com/intel/Intel-PMT
+ * File: xml/CWF/OOBMSM/RMID-PERF/cwf_aggregator.xml
+ */
+static struct event_group perf_0x26557651 = {
+	.guid		= 0x26557651,
+};
+
+static struct event_group *known_event_groups[] = {
+	&energy_0x26696143,
+	&perf_0x26557651,
+};
+
+#define NUM_KNOWN_GROUPS ARRAY_SIZE(known_event_groups)
+
+/* Stub for now */
+static int configure_events(struct event_group *e, struct pmt_feature_group *p)
+{
+	return -EINVAL;
+}
+
+DEFINE_FREE(intel_pmt_put_feature_group, struct pmt_feature_group *,
+		if (!IS_ERR_OR_NULL(_T))
+			intel_pmt_put_feature_group(_T))
+
+/*
+ * Make a request to the INTEL_PMT_DISCOVERY driver for the
+ * pmt_feature_group for a specific feature. If there is
+ * one the returned structure has an array of telemetry_region
+ * structures. Each describes one telemetry aggregator.
+ * Try to configure any with a known matching guid.
+ */
+static bool get_pmt_feature(enum pmt_feature_id feature)
+{
+	struct pmt_feature_group *p __free(intel_pmt_put_feature_group) = NULL;
+	struct event_group **peg;
+	bool ret;
+
+	p = intel_pmt_get_regions_by_feature(feature);
+
+	if (IS_ERR_OR_NULL(p))
+		return false;
+
+	for (peg = &known_event_groups[0]; peg < &known_event_groups[NUM_KNOWN_GROUPS]; peg++) {
+		ret = configure_events(*peg, p);
+		if (!ret) {
+			(*peg)->pfg = no_free_ptr(p);
+			return true;
+		}
+	}
+
+	return false;
+}
+
+/*
+ * Ask OOBMSM discovery driver for all the RMID based telemetry groups
+ * that it supports.
+ */
+bool intel_aet_get_events(void)
+{
+	bool ret1, ret2;
+
+	ret1 = get_pmt_feature(FEATURE_PER_RMID_ENERGY_TELEM);
+	ret2 = get_pmt_feature(FEATURE_PER_RMID_PERF_TELEM);
+
+	return ret1 || ret2;
+}
+
+void __exit intel_aet_exit(void)
+{
+	struct event_group **peg;
+
+	for (peg = &known_event_groups[0]; peg < &known_event_groups[NUM_KNOWN_GROUPS]; peg++) {
+		if ((*peg)->pfg) {
+			intel_pmt_put_feature_group((*peg)->pfg);
+			(*peg)->pfg = NULL;
+		}
+	}
+}
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 71019b3b54ea..8eb68d2230be 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -512,6 +512,9 @@ 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.
 
diff --git a/arch/x86/kernel/cpu/resctrl/Makefile b/arch/x86/kernel/cpu/resctrl/Makefile
index d8a04b195da2..97ceb4e44dfa 100644
--- a/arch/x86/kernel/cpu/resctrl/Makefile
+++ b/arch/x86/kernel/cpu/resctrl/Makefile
@@ -1,6 +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_RESCTRL_FS_PSEUDO_LOCK)	+= pseudo_lock.o
 
 # To allow define_trace.h's recursive include:
-- 
2.49.0

Hi Tony,

On 6/26/25 9:49 AM, Tony Luck wrote:
> Hardware has one or more telemetry event aggregators per package
> for each group of telemetry events. Each aggregator provides access
> to event counts in an array of 64-bit values in MMIO space. There
> is a "guid" (in this case a unique 32-bit integer) which refers to
> an XML file published in the https://github.com/intel/Intel-PMT

"an XML file published in the" -> "an XML file published in"?

> that provides all the details about each aggregator.
> 
> The XML files provide the following information:

"The XML files provide" -> "The XML file provides"
(First paragraph refers to a single XML file so I assume it means
all this information is available from one XML file?)

> 1) Which telemetry events are included in the group for this aggregator.
> 2) The order in which the event counters appear for each RMID.
> 3) The value type of each event counter (integer or fixed-point).
> 4) The number of RMIDs supported.
> 5) Which additional aggregator status registers are included.
> 6) The total size of the MMIO region for this aggregator.
> 
> Add select of X86_PLATFORM_DEVICES, INTEL_VSEC and
> INTEL_PMT_TELEMETRY to CONFIG_X86_CPU_RESCTRL to enable use of the
> discovery driver that enumerate all aggregators on the system with
> intel_pmt_get_regions_by_feature(). Call this for each pmt_feature_id
> that indicates per-RMID telemetry.
> 
> Save the returned pmt_feature_group pointers with guids that are known
> to resctrl for use at run time.
> 
> Those pointers are returned to the INTEL_PMT_DISCOVERY driver at
> resctrl_arch_exit() time.

It is not clear to me why this work needs to use two different terms for
the same thing.
Assuming it is required, up until this point this work has only used the
term "aggregator" and within this patch is where this work starts
using "telemetry region" interchangeably. When reading this patch
"telemetry region" is first used in struct event_group without the term
introduced to the reader.

Could you please add a snippet in changelog to help with this transition?

> 
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> ---
>  arch/x86/kernel/cpu/resctrl/internal.h  |   3 +
>  arch/x86/kernel/cpu/resctrl/core.c      |   5 +
>  arch/x86/kernel/cpu/resctrl/intel_aet.c | 122 ++++++++++++++++++++++++
>  arch/x86/Kconfig                        |   3 +
>  arch/x86/kernel/cpu/resctrl/Makefile    |   1 +
>  5 files changed, 134 insertions(+)
>  create mode 100644 arch/x86/kernel/cpu/resctrl/intel_aet.c
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> index 224b71730cc3..e93b15bf6aab 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -169,4 +169,7 @@ void __init intel_rdt_mbm_apply_quirk(void);
>  
>  void rdt_domain_reconfigure_cdp(struct rdt_resource *r);
>  
> +bool intel_aet_get_events(void);
> +void __exit intel_aet_exit(void);
> +
>  #endif /* _ASM_X86_RESCTRL_INTERNAL_H */
> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> index a5f01cac2363..9144766da836 100644
> --- a/arch/x86/kernel/cpu/resctrl/core.c
> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> @@ -734,6 +734,9 @@ void resctrl_arch_pre_mount(void)
>  
>  	if (!atomic_try_cmpxchg(&only_once, &old, 1))
>  		return;
> +
> +	if (!intel_aet_get_events())
> +		return;
>  }
>  
>  enum {
> @@ -1086,6 +1089,8 @@ late_initcall(resctrl_arch_late_init);
>  
>  static void __exit resctrl_arch_exit(void)
>  {
> +	intel_aet_exit();
> +
>  	cpuhp_remove_state(rdt_online);
>  
>  	resctrl_exit();
> diff --git a/arch/x86/kernel/cpu/resctrl/intel_aet.c b/arch/x86/kernel/cpu/resctrl/intel_aet.c
> new file mode 100644
> index 000000000000..b09044b093dd
> --- /dev/null
> +++ b/arch/x86/kernel/cpu/resctrl/intel_aet.c
> @@ -0,0 +1,122 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Resource Director Technology(RDT)
> + * - Intel Application Energy Telemetry
> + *
> + * Copyright (C) 2025 Intel Corporation
> + *
> + * Author:
> + *    Tony Luck <tony.luck@intel.com>
> + */
> +
> +#define pr_fmt(fmt)   "resctrl: " fmt
> +
> +#include <linux/cleanup.h>
> +#include <linux/cpu.h>
> +#include <linux/intel_vsec.h>
> +#include <linux/resctrl.h>
> +
> +#include "internal.h"
> +
> +/**
> + * struct event_group - All information about a group of telemetry events.
> + * @pfg:		Points to the aggregated telemetry space information
> + *			within the OOBMSM driver that contains data for all
> + *			telemetry regions.
> + * @guid:		Unique number per XML description file.
> + */
> +struct event_group {
> +	/* Data fields for additional structures to manage this group. */
> +	struct pmt_feature_group	*pfg;
> +
> +	/* Remaining fields initialized from XML file. */
> +	u32				guid;
> +};
> +
> +/*
> + * Link: https://github.com/intel/Intel-PMT
> + * File: xml/CWF/OOBMSM/RMID-ENERGY/cwf_aggregator.xml
> + */
> +static struct event_group energy_0x26696143 = {
> +	.guid		= 0x26696143,
> +};
> +
> +/*
> + * Link: https://github.com/intel/Intel-PMT
> + * File: xml/CWF/OOBMSM/RMID-PERF/cwf_aggregator.xml
> + */
> +static struct event_group perf_0x26557651 = {
> +	.guid		= 0x26557651,
> +};
> +
> +static struct event_group *known_event_groups[] = {
> +	&energy_0x26696143,
> +	&perf_0x26557651,
> +};
> +
> +#define NUM_KNOWN_GROUPS ARRAY_SIZE(known_event_groups)
> +
> +/* Stub for now */
> +static int configure_events(struct event_group *e, struct pmt_feature_group *p)
> +{
> +	return -EINVAL;
> +}
> +
> +DEFINE_FREE(intel_pmt_put_feature_group, struct pmt_feature_group *,
> +		if (!IS_ERR_OR_NULL(_T))
> +			intel_pmt_put_feature_group(_T))

As you state in cover this snippet cannot make checkpatch.pl happy.
I would propose that the issues trying to appease checkpatch.pl are
documented in the maintainer notes of this patch.

> +
> +/*
> + * Make a request to the INTEL_PMT_DISCOVERY driver for the
> + * pmt_feature_group for a specific feature. If there is
> + * one the returned structure has an array of telemetry_region
> + * structures. Each describes one telemetry aggregator.
> + * Try to configure any with a known matching guid.

I interpret "configure" to involve "write" activity where, for example,
settings are changed and things are ... configured. The way this is
written it seems that resctrl is configuring (i.e. making changes to)
events known to it. This is not the case though (?), resctrl is just
doing discovery of events here. How about above is instead:

	Try to discover any with a known matching guid

and configure_events() -> discover_events()?

> + */
> +static bool get_pmt_feature(enum pmt_feature_id feature)
> +{
> +	struct pmt_feature_group *p __free(intel_pmt_put_feature_group) = NULL;
> +	struct event_group **peg;
> +	bool ret;
> +
> +	p = intel_pmt_get_regions_by_feature(feature);
> +
> +	if (IS_ERR_OR_NULL(p))
> +		return false;
> +
> +	for (peg = &known_event_groups[0]; peg < &known_event_groups[NUM_KNOWN_GROUPS]; peg++) {
> +		ret = configure_events(*peg, p);
> +		if (!ret) {
> +			(*peg)->pfg = no_free_ptr(p);
> +			return true;
> +		}
> +	}
> +
> +	return false;
> +}
> +
> +/*
> + * Ask OOBMSM discovery driver for all the RMID based telemetry groups
> + * that it supports.
> + */
> +bool intel_aet_get_events(void)
> +{
> +	bool ret1, ret2;
> +
> +	ret1 = get_pmt_feature(FEATURE_PER_RMID_ENERGY_TELEM);
> +	ret2 = get_pmt_feature(FEATURE_PER_RMID_PERF_TELEM);
> +
> +	return ret1 || ret2;
> +}
> +
> +void __exit intel_aet_exit(void)
> +{
> +	struct event_group **peg;
> +
> +	for (peg = &known_event_groups[0]; peg < &known_event_groups[NUM_KNOWN_GROUPS]; peg++) {
> +		if ((*peg)->pfg) {
> +			intel_pmt_put_feature_group((*peg)->pfg);
> +			(*peg)->pfg = NULL;
> +		}
> +	}
> +}
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 71019b3b54ea..8eb68d2230be 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig

Kconfig has its own thread.

Reinette

On Thu, Jun 26, 2025 at 09:49:26AM -0700, Tony Luck wrote:
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 71019b3b54ea..8eb68d2230be 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -512,6 +512,9 @@ 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.
>  

The list of dependencies to "select" keeps growing. "lkp"
just told me that "INTEL_VSEC" depends on "PCI".

An alternative approach is to just add:

	depends on INTEL_PMT_DISCOVERY=y

instead of all the extra "select" lines.

Pro: This describes exactly what is needed. The INTEL_PMT_DISCOVERY
driver must be built-in to the kernel so that resctrl can enumerate the
telemetry features.

Con: "make olddefconfig" will now drop X86_CPU_RESCTRL until the user
hunts down and enables the chain of dependencies to get RESCTRL turned
back on again.

-Tony

Hi Tony,

On 6/27/25 11:06 AM, Luck, Tony wrote:
> On Thu, Jun 26, 2025 at 09:49:26AM -0700, Tony Luck wrote:
>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>> index 71019b3b54ea..8eb68d2230be 100644
>> --- a/arch/x86/Kconfig
>> +++ b/arch/x86/Kconfig
>> @@ -512,6 +512,9 @@ 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.
>>  
> 
> The list of dependencies to "select" keeps growing. "lkp"
> just told me that "INTEL_VSEC" depends on "PCI".
> 
> An alternative approach is to just add:
> 
> 	depends on INTEL_PMT_DISCOVERY=y
> 
> instead of all the extra "select" lines.
> 
> Pro: This describes exactly what is needed. The INTEL_PMT_DISCOVERY
> driver must be built-in to the kernel so that resctrl can enumerate the
> telemetry features.

How will this behave on AMD systems?

> 
> Con: "make olddefconfig" will now drop X86_CPU_RESCTRL until the user
> hunts down and enables the chain of dependencies to get RESCTRL turned
> back on again.

Reinette

On Thu, Jul 03, 2025 at 11:27:19AM -0700, Reinette Chatre wrote:
> Hi Tony,
> 
> On 6/27/25 11:06 AM, Luck, Tony wrote:
> > On Thu, Jun 26, 2025 at 09:49:26AM -0700, Tony Luck wrote:
> >> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> >> index 71019b3b54ea..8eb68d2230be 100644
> >> --- a/arch/x86/Kconfig
> >> +++ b/arch/x86/Kconfig
> >> @@ -512,6 +512,9 @@ 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.
> >>  
> > 
> > The list of dependencies to "select" keeps growing. "lkp"
> > just told me that "INTEL_VSEC" depends on "PCI".
> > 
> > An alternative approach is to just add:
> > 
> > 	depends on INTEL_PMT_DISCOVERY=y
> > 
> > instead of all the extra "select" lines.
> > 
> > Pro: This describes exactly what is needed. The INTEL_PMT_DISCOVERY
> > driver must be built-in to the kernel so that resctrl can enumerate the
> > telemetry features.
> 
> How will this behave on AMD systems?

The call to intel_pmt_get_regions_by_feature() in the INTEL_PMT_DISCOVERY
driver will return that there are no telemetry events.

If it is a problem to force resctrl users building AMD only kernels
to load the INTEL_PMT_DISCOVERY in order to use resctrl, then I can
look at providing stubs for the entry points in intel_aet.c and
create a new CONFIG option to allow resctrl to be built without
Intel telemetry support.

> > 
> > Con: "make olddefconfig" will now drop X86_CPU_RESCTRL until the user
> > hunts down and enables the chain of dependencies to get RESCTRL turned
> > back on again.
> 
> Reinette
> 

-Tony

Hi Tony,

On 7/3/25 1:17 PM, Luck, Tony wrote:
> On Thu, Jul 03, 2025 at 11:27:19AM -0700, Reinette Chatre wrote:
>> On 6/27/25 11:06 AM, Luck, Tony wrote:
>>> On Thu, Jun 26, 2025 at 09:49:26AM -0700, Tony Luck wrote:
>>>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>>>> index 71019b3b54ea..8eb68d2230be 100644
>>>> --- a/arch/x86/Kconfig
>>>> +++ b/arch/x86/Kconfig
>>>> @@ -512,6 +512,9 @@ 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.
>>>>  
>>>
>>> The list of dependencies to "select" keeps growing. "lkp"
>>> just told me that "INTEL_VSEC" depends on "PCI".
>>>
>>> An alternative approach is to just add:
>>>
>>> 	depends on INTEL_PMT_DISCOVERY=y
>>>
>>> instead of all the extra "select" lines.
>>>
>>> Pro: This describes exactly what is needed. The INTEL_PMT_DISCOVERY
>>> driver must be built-in to the kernel so that resctrl can enumerate the
>>> telemetry features.
>>
>> How will this behave on AMD systems?
> 
> The call to intel_pmt_get_regions_by_feature() in the INTEL_PMT_DISCOVERY
> driver will return that there are no telemetry events.
> 
> If it is a problem to force resctrl users building AMD only kernels
> to load the INTEL_PMT_DISCOVERY in order to use resctrl, then I can
> look at providing stubs for the entry points in intel_aet.c and
> create a new CONFIG option to allow resctrl to be built without
> Intel telemetry support.

I do not think resctrl should enforce dependency on a driver that is not
valid for a platform.

Reinette

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.

"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
+	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:
-- 
2.50.0

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

> > +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?

Yes. It will move here. The rest of resctrl is still 32-bit clean (presumably ... I
wonder if anyone regularly builds, runs, and tests resctrl in a 32-bit kernel).

-Tony