[PATCH v9 00/12] Allow AET to use PMT as loadable module

[PATCH v9 00/12] Allow AET to use PMT as loadable module

[Tony Luck]

Requiring INTEL_PMT_TELEMETRY=y to enable AET is a functional workaround
to enable enumeration of Application Energy Telemetry (AET) events, but
unacceptable to many users. It results in increased configuration complexity,
increased kernel memory footprint and inability to patch problems by unloading
a module and loading an updated version.

Add a registration function to the AET code that can be used by
INTEL_PMT_TELEMETRY to provide the enumeration functions.

INTEL_PMT_TELEMETRY can be loaded/unloaded independently of
resctrl file system mount/unmount. Perform enumeration on
every mount and cleanup on every unmount.

Patch series based on v7.2-rc1 plus Reinette's "Fix long-standing issues"[1]
and "Improve resctrl quality and consistency"[2] series.

Signed-off-by: Tony Luck <tony.luck@intel.com>

---

Changes since v8[3]:

0001:	Suppress_bind_attrs for intel_vsec as well as intel_telemetry
0008:	PROBE_FORCE_SYNCHRONOUS and register/unregister in init/exit
0009:	Remove broken atomic ops introduced in v8. Handle repeat mount
	attempts in intel_aet.c - See Sashiko v8 review[4].
0012:	Add example udev and systemd config files to mount resctrl

Link: https://lore.kernel.org/lkml/cover.1781029125.git.reinette.chatre@intel.com/ [1]
Link: https://lore.kernel.org/all/cover.1782857711.git.reinette.chatre@intel.com/ [2]
Link: https://lore.kernel.org/lkml/20260615182457.14725-1-tony.luck@intel.com/ [3]
Link: https://sashiko.dev/#/patchset/20260615182457.14725-1-tony.luck%40intel.com [4]

Tony Luck (12):
  platform/x86/intel/{pmt,vsec}: Prevent unbind via sysfs
  fs/resctrl: Remove redundant calls to resctrl_arch_mon_capable()
  x86/resctrl: Honor rdt=perf option to force enable AET perf events
  fs/resctrl: Add interface to disable a monitor event
  x86/resctrl: Drop global 'rdt_mon_capable' flag
  arm,x86,fs/resctrl: Handle change in number of RMIDs on each mount
  x86/resctrl: Add PMT registration API for AET enumeration callbacks
  platform/x86/intel/pmt: Register enumeration functions with resctrl
  arm,x86/resctrl: Resolve INTEL_PMT_TELEMETRY symbols at runtime
  fs/resctrl: Call architecture hooks for every mount/unmount
  x86/resctrl: Simplify Kconfig options for resctrl
  Documentation/filesystems/resctrl: Document telemetry mount timing
    caveat

 Documentation/filesystems/resctrl.rst      | 60 +++++++++++++-
 include/linux/resctrl.h                    | 47 ++++++++++-
 arch/x86/include/asm/resctrl.h             | 21 +++--
 arch/x86/kernel/cpu/resctrl/internal.h     | 24 +++---
 arch/x86/kernel/cpu/resctrl/core.c         | 57 +++++++++++--
 arch/x86/kernel/cpu/resctrl/intel_aet.c    | 96 +++++++++++++++++++---
 arch/x86/kernel/cpu/resctrl/monitor.c      |  6 --
 drivers/platform/x86/intel/pmt/telemetry.c | 13 ++-
 drivers/platform/x86/intel/vsec.c          |  3 +
 drivers/resctrl/mpam_resctrl.c             |  9 ++
 fs/resctrl/monitor.c                       | 93 ++++++++++++++-------
 fs/resctrl/rdtgroup.c                      | 16 ++--
 arch/x86/Kconfig                           | 15 +---
 arch/x86/kernel/cpu/resctrl/Makefile       |  2 +-
 14 files changed, 371 insertions(+), 91 deletions(-)

-- 
2.54.0

[PATCH v9 01/12] platform/x86/intel/{pmt,vsec}: Prevent unbind via sysfs

[Tony Luck]

The resctrl AET code uses intel_pmt_get_regions_by_feature() (exported by the
pmt_telemetry module) to obtain pmt_feature_group instances that are populated
using data from the pmt_discovery module. The returned telemetry_region::addr
values are MMIO mappings created in the telemetry driver's probe path,
and resctrl reads from them while a mount is active.

However, a user can unbind the intel_pmt driver or its parent intel_vsec
from their devices via sysfs "unbind" attributes. Unbinding either of these
drivers would tear down the MMIO mappings, leaving resctrl with stale pointers
and causing a fault on the next access.

It is safe for the discovery driver to be unbound. It does all its work at
initial probe time and saves a copy of the features and RMID count.

Set device_driver::suppress_bind_attrs in each driver so that the "bind"
and "unbind" sysfs attributes are not exposed and the user cannot detach
the driver from its devices while resctrl (or any other in-kernel consumer)
may be using the resources.

Fixes: 1fb2daa60de6 ("x86/resctrl: Discover hardware telemetry events")
Assisted-by: Claude:Opus-4.7
Signed-off-by: Tony Luck <tony.luck@intel.com>
--
v9:
	Set .suppress_bind_attrs for intel_vsec as well as intel_telemetry
---
 drivers/platform/x86/intel/pmt/telemetry.c | 3 +++
 drivers/platform/x86/intel/vsec.c          | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/drivers/platform/x86/intel/pmt/telemetry.c b/drivers/platform/x86/intel/pmt/telemetry.c
index 953f35b6daec..f243ccf3185b 100644
--- a/drivers/platform/x86/intel/pmt/telemetry.c
+++ b/drivers/platform/x86/intel/pmt/telemetry.c
@@ -427,6 +427,9 @@ static struct auxiliary_driver pmt_telem_aux_driver = {
 	.id_table	= pmt_telem_id_table,
 	.remove		= pmt_telem_remove,
 	.probe		= pmt_telem_probe,
+	.driver = {
+		.suppress_bind_attrs = true,
+	},
 };
 
 static int __init pmt_telem_init(void)
diff --git a/drivers/platform/x86/intel/vsec.c b/drivers/platform/x86/intel/vsec.c
index 3ae4557b32b4..50e9e122866c 100644
--- a/drivers/platform/x86/intel/vsec.c
+++ b/drivers/platform/x86/intel/vsec.c
@@ -893,6 +893,9 @@ static struct pci_driver intel_vsec_pci_driver = {
 	.id_table = intel_vsec_pci_ids,
 	.probe = intel_vsec_pci_probe,
 	.err_handler = &intel_vsec_pci_err_handlers,
+	.driver = {
+		.suppress_bind_attrs = true,
+	},
 };
 module_pci_driver(intel_vsec_pci_driver);
 
-- 
2.54.0

[PATCH v9 02/12] fs/resctrl: Remove redundant calls to resctrl_arch_mon_capable()

[Tony Luck]

Architecture code provides resctrl_arch_mon_capable() so that file
system code knows whether any monitor features are supported. This
is used to decide whether to create "mon_data" directories etc.

Initially resctrl_arch_mon_capable() was an inline function providing
the value of the x86 architecture variable "rdt_mon_capable". I.e.
extremely low overhead and so it was used liberally as a sanity check.

The ARM implementation of resctrl_arch_mon_capable() is not inline and
chases a couple of pointers. Future changes to x86 implementation will
also make resctrl_arch_mon_capable() more expensive.

Redundant calls to resctrl_arch_mon_capable() in hot code paths should
be eliminated.

1) mbm_handle_overflow()
This function is called once per second from worker threads running on
each L3 domain. The call is clearly redundant because worker threads are
only created if the MBM monitoring feature is enabled.

2) is_rmid_match()
This function is called when a user reads a "tasks" file of a MON_GROUP.
The call is redundant because the function also checks "r->type ==
RDTMON_GROUP" and this type of group can only be created on systems that
support monitoring.

Remove the call to resctrl_arch_mon_capable() from both of these
functions.

Signed-off-by: Tony Luck <tony.luck@intel.com>
---
 fs/resctrl/monitor.c  | 2 +-
 fs/resctrl/rdtgroup.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/resctrl/monitor.c b/fs/resctrl/monitor.c
index 73413cb128ea..6acf6106e6f1 100644
--- a/fs/resctrl/monitor.c
+++ b/fs/resctrl/monitor.c
@@ -888,7 +888,7 @@ void mbm_handle_overflow(struct work_struct *work)
 	 * If the filesystem has been unmounted this work no longer needs to
 	 * run.
 	 */
-	if (!resctrl_mounted || !resctrl_arch_mon_capable())
+	if (!resctrl_mounted)
 		goto out_unlock;
 
 	/*
diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
index efadadfe4ed5..ac1a5af00bc5 100644
--- a/fs/resctrl/rdtgroup.c
+++ b/fs/resctrl/rdtgroup.c
@@ -691,7 +691,7 @@ static bool is_closid_match(struct task_struct *t, struct rdtgroup *r)
 
 static bool is_rmid_match(struct task_struct *t, struct rdtgroup *r)
 {
-	return (resctrl_arch_mon_capable() && (r->type == RDTMON_GROUP) &&
+	return ((r->type == RDTMON_GROUP) &&
 		resctrl_arch_match_rmid(t, r->mon.parent->closid,
 					r->mon.rmid));
 }
-- 
2.54.0

[PATCH v9 03/12] x86/resctrl: Honor rdt=perf option to force enable AET perf events

[Tony Luck]

The kernel command line option "rdt=" is used to force enable or disable
individual resctrl features.

Linux only enumerates AET (Application Energy Telemetry) once on first
mount. But that is going to change to enumerate on each mount to allow the
pmt_telemetry driver to be configured as a module, and allow unloading when
not in use.

The following scenario will be a problem:

1) User mounts the resctrl file system.  all_regions_have_sufficient_rmid()
notes that a perf telemetry region supports fewer RMIDs than expected for
the event_group and sets e->force_off = true. But the user had specified
"rdt=perf" on the kernel command line so perf events are enabled for
this first mount.

2) Resctrl file system is unmounted and later remounted.

3) During this new mount cycle enable_events() sees that e->force_off is
set and stops enumeration for this event group. So on this second, and
all subsequent, mounts perf events are not enabled.

Fix by checking the state of e->force_on at the start of enable_events().

Signed-off-by: Tony Luck <tony.luck@intel.com>
---
 arch/x86/kernel/cpu/resctrl/intel_aet.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/cpu/resctrl/intel_aet.c b/arch/x86/kernel/cpu/resctrl/intel_aet.c
index c22c3cf5167d..cd8257c58f84 100644
--- a/arch/x86/kernel/cpu/resctrl/intel_aet.c
+++ b/arch/x86/kernel/cpu/resctrl/intel_aet.c
@@ -60,8 +60,8 @@ struct pmt_event {
  *			data for all telemetry regions of type @pfname.
  *			Valid if the system supports the event group,
  *			NULL otherwise.
- * @force_off:		True when "rdt" command line or architecture code disables
- *			this event group due to insufficient RMIDs.
+ * @force_off:		True when "rdt" command line disables this event group
+ *			to avoid system limitations due to insufficient RMIDs.
  * @force_on:		True when "rdt" command line overrides disable of this
  *			event group.
  * @guid:		Unique number per XML description file.
@@ -228,7 +228,7 @@ static bool enable_events(struct event_group *e, struct pmt_feature_group *p)
 	struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_PERF_PKG].r_resctrl;
 	int skipped_events = 0;
 
-	if (e->force_off)
+	if (e->force_off && !e->force_on)
 		return false;
 
 	if (!group_has_usable_regions(e, p))
-- 
2.54.0

[PATCH v9 04/12] fs/resctrl: Add interface to disable a monitor event

[Tony Luck]

resctrl currently assumes all monitor events are enabled before any domain
is created, because per-domain state is allocated by the architecture's CPU
hotplug callbacks. There is no way to disable an event once registered.

AET events are enumerated by the INTEL_PMT_TELEMETRY driver. To allow that
driver to be a loadable module, resctrl must tolerate AET events appearing
and disappearing, which requires the ability to disable an event when the
driver is unloaded.

Add resctrl_disable_mon_event(). The architecture owns domain lifetime
and knows mount state, so it is responsible for calling this only while
resctrl is unmounted and for cleaning up any per-domain state. Document
those requirements in the kerneldoc since they are not enforced in code.

Signed-off-by: Tony Luck <tony.luck@intel.com>
---
 include/linux/resctrl.h | 34 ++++++++++++++++++++++++++++++++++
 fs/resctrl/monitor.c    | 15 +++++++++++++++
 2 files changed, 49 insertions(+)

diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
index dd09c2ce9a0f..a184500745f8 100644
--- a/include/linux/resctrl.h
+++ b/include/linux/resctrl.h
@@ -419,9 +419,43 @@ u32 resctrl_arch_get_num_closid(struct rdt_resource *r);
 u32 resctrl_arch_system_num_rmid_idx(void);
 int resctrl_arch_update_domains(struct rdt_resource *r, u32 closid);
 
+/**
+ * resctrl_enable_mon_event() - Enable monitoring event
+ * @eventid:	ID of the event
+ * @any_cpu:	True if event data can be read from any CPU.
+ * @binary_bits: Number of binary places of the fixed-point value expected to
+ *		back a floating point event. Can only be set for floating point
+ *		events.
+ * @arch_priv:  Architecture private data associated with event. Passed back to
+ *		architecture when reading the event via resctrl_arch_rmid_read().
+ *
+ * The file system must not be mounted when enabling an event.
+ *
+ * Events that require per-domain (architectural and/or filesystem) state must
+ * be enabled before the domain structures are allocated. For example before
+ * CPU hotplug callbacks that allocate domain structures are registered. If the
+ * architecture discovers a resource after initialization it should enable
+ * events needing per-domain state before any domain structure allocation which
+ * should be coordinated with the CPU hotplug callbacks.
+ *
+ * Return:
+ * true if event was successfully enabled, false otherwise.
+ */
 bool resctrl_enable_mon_event(enum resctrl_event_id eventid, bool any_cpu,
 			      unsigned int binary_bits, void *arch_priv);
 
+/**
+ * resctrl_disable_mon_event() - Disable monitoring event
+ * @eventid:	ID of the event
+ *
+ * The file system must not be mounted when disabling an event.
+ *
+ * Events that require per-domain (architectural and/or filesystem) state
+ * will require additional cleanup which should be coordinated with the CPU
+ * hotplug callbacks.
+ */
+void resctrl_disable_mon_event(enum resctrl_event_id eventid);
+
 bool resctrl_is_mon_event_enabled(enum resctrl_event_id eventid);
 
 bool resctrl_arch_is_evt_configurable(enum resctrl_event_id evt);
diff --git a/fs/resctrl/monitor.c b/fs/resctrl/monitor.c
index 6acf6106e6f1..5d5625e4e3b4 100644
--- a/fs/resctrl/monitor.c
+++ b/fs/resctrl/monitor.c
@@ -1066,6 +1066,21 @@ bool resctrl_enable_mon_event(enum resctrl_event_id eventid, bool any_cpu,
 	return true;
 }
 
+void resctrl_disable_mon_event(enum resctrl_event_id eventid)
+{
+	if (WARN_ON_ONCE(eventid < QOS_FIRST_EVENT || eventid >= QOS_NUM_EVENTS))
+		return;
+	if (!mon_event_all[eventid].enabled) {
+		pr_warn("Repeat disable for event %d\n", eventid);
+		return;
+	}
+
+	mon_event_all[eventid].any_cpu = false;
+	mon_event_all[eventid].binary_bits = 0;
+	mon_event_all[eventid].arch_priv = NULL;
+	mon_event_all[eventid].enabled = false;
+}
+
 bool resctrl_is_mon_event_enabled(enum resctrl_event_id eventid)
 {
 	return eventid >= QOS_FIRST_EVENT && eventid < QOS_NUM_EVENTS &&
-- 
2.54.0

[PATCH v9 05/12] x86/resctrl: Drop global 'rdt_mon_capable' flag

[Tony Luck]

File system code calls resctrl_arch_mon_capable() to determine if the
system supports any monitoring features.

x86 architecture code sets a global flag to support implementation of
resctrl_arch_mon_capable() but the upcoming change to enumerate AET
(Application Energy Telemetry) features on each mount introduces a new
corner case.

Specifically when AET is the only monitoring feature, loading and
unloading the pmt_telemetry module between resctrl mounts may result
in monitoring support enabled on some mounts, but not on others.

Replace the global flag with a scan of resources to check if any are
marked mon_capable.

Signed-off-by: Tony Luck <tony.luck@intel.com>
---
 arch/x86/include/asm/resctrl.h        |  6 +-----
 arch/x86/kernel/cpu/resctrl/core.c    | 18 +++++++++++++++---
 arch/x86/kernel/cpu/resctrl/monitor.c |  6 ------
 3 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/arch/x86/include/asm/resctrl.h b/arch/x86/include/asm/resctrl.h
index 575f8408a9e7..a9f481b7a8ed 100644
--- a/arch/x86/include/asm/resctrl.h
+++ b/arch/x86/include/asm/resctrl.h
@@ -43,7 +43,6 @@ struct resctrl_pqr_state {
 DECLARE_PER_CPU(struct resctrl_pqr_state, pqr_state);
 
 extern bool rdt_alloc_capable;
-extern bool rdt_mon_capable;
 
 DECLARE_STATIC_KEY_FALSE(rdt_enable_key);
 DECLARE_STATIC_KEY_FALSE(rdt_alloc_enable_key);
@@ -66,10 +65,7 @@ static inline void resctrl_arch_disable_alloc(void)
 	static_branch_dec_cpuslocked(&rdt_enable_key);
 }
 
-static inline bool resctrl_arch_mon_capable(void)
-{
-	return rdt_mon_capable;
-}
+bool resctrl_arch_mon_capable(void);
 
 static inline void resctrl_arch_enable_mon(void)
 {
diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
index 9b9495174041..60d50ac79e7b 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
@@ -51,6 +51,20 @@ DEFINE_PER_CPU(struct resctrl_pqr_state, pqr_state);
  */
 bool rdt_alloc_capable;
 
+/*
+ * Need to re-evaluate on each mount whether any mon_capable resources
+ * are enabled.
+ */
+bool resctrl_arch_mon_capable(void)
+{
+	struct rdt_resource *r;
+
+	for_each_mon_capable_rdt_resource(r)
+		return true;
+
+	return false;
+}
+
 static void mba_wrmsr_intel(struct msr_param *m);
 static void cat_wrmsr(struct msr_param *m);
 static void mba_wrmsr_amd(struct msr_param *m);
@@ -780,7 +794,6 @@ void resctrl_arch_pre_mount(void)
 	cpus_read_lock();
 	mutex_lock(&domain_list_lock);
 	r->mon_capable = true;
-	rdt_mon_capable = true;
 	for_each_online_cpu(cpu)
 		domain_add_cpu_mon(cpu, r);
 	mutex_unlock(&domain_list_lock);
@@ -1014,9 +1027,8 @@ static __init void check_quirks(void)
 static __init bool get_rdt_resources(void)
 {
 	rdt_alloc_capable = get_rdt_alloc_resources();
-	rdt_mon_capable = get_rdt_mon_resources();
 
-	return (rdt_mon_capable || rdt_alloc_capable);
+	return get_rdt_mon_resources() || rdt_alloc_capable;
 }
 
 static __init void rdt_init_res_defs_intel(void)
diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c
index 430b8fae0b77..1fea893f2cc1 100644
--- a/arch/x86/kernel/cpu/resctrl/monitor.c
+++ b/arch/x86/kernel/cpu/resctrl/monitor.c
@@ -26,12 +26,6 @@
 
 #include "internal.h"
 
-/*
- * Global boolean for rdt_monitor which is true if any
- * resource monitoring is enabled.
- */
-bool rdt_mon_capable;
-
 #define CF(cf)	((unsigned long)(1048576 * (cf) + 0.5))
 
 static int snc_nodes_per_l3_cache = 1;
-- 
2.54.0

[PATCH v9 06/12] arm,x86,fs/resctrl: Handle change in number of RMIDs on each mount

[Tony Luck]

Application Energy Telemetry (AET) event enumeration takes place
asynchronously. Linux builds the pmt_telemetry module into the kernel to
kick off enumeration early enough that it completes before first mount of
the resctrl file system.

Allowing pmt_telemetry to be a loadable module means that it is possible
for different numbers of RMIDs to be supported on each mount, depending
on whether pmt_telemetry module is loaded.

For simplicity, calculate the maximum possible number of RMIDs and use
that value to allocate the rmid_ptrs[] array just once. Also use this
maximum RMID value when allocating rdt_l3_mon_domain::rmid_busy_llc
bitmap and rdt_l3_mon_domain::mbm_states.

The limbo code must deal with changes in the number of RMIDs from one
mount to the next because some RMIDs may still be "busy" when the file
system is unmounted, but be above resctrl_arch_system_num_rmid_idx()
for the remount. In this case RMIDs that can be released are not put
onto the rmid_free_lru list.

Signed-off-by: Tony Luck <tony.luck@intel.com>
---
 include/linux/resctrl.h            |  5 +-
 arch/x86/kernel/cpu/resctrl/core.c | 14 ++++++
 drivers/resctrl/mpam_resctrl.c     |  5 ++
 fs/resctrl/monitor.c               | 76 +++++++++++++++++++-----------
 fs/resctrl/rdtgroup.c              |  6 +--
 5 files changed, 75 insertions(+), 31 deletions(-)

diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
index a184500745f8..e1501ee72d28 100644
--- a/include/linux/resctrl.h
+++ b/include/linux/resctrl.h
@@ -183,10 +183,12 @@ struct mbm_cntr_cfg {
  * struct rdt_l3_mon_domain - group of CPUs sharing RDT_RESOURCE_L3 monitoring
  * @hdr:		common header for different domain types
  * @ci_id:		cache info id for this domain
- * @rmid_busy_llc:	bitmap of which limbo RMIDs are above threshold
+ * @rmid_busy_llc:	bitmap of which limbo RMIDs are above threshold. Sized for
+ *			resctrl_arch_system_max_rmid_idx() RMIDs
  * @mbm_states:		Per-event pointer to the MBM event's saved state.
  *			An MBM event's state is an array of struct mbm_state
  *			indexed by RMID on x86 or combined CLOSID, RMID on Arm.
+ *			Also sized for resctrl_arch_system_max_rmid_idx() RMIDs
  * @mbm_over:		worker to periodically read MBM h/w counters
  * @cqm_limbo:		worker to periodically read CQM h/w counters
  * @mbm_work_cpu:	worker CPU for MBM h/w counters
@@ -417,6 +419,7 @@ static inline u32 resctrl_get_default_ctrl(struct rdt_resource *r)
 /* The number of closid supported by this resource regardless of CDP */
 u32 resctrl_arch_get_num_closid(struct rdt_resource *r);
 u32 resctrl_arch_system_num_rmid_idx(void);
+u32 resctrl_arch_system_max_rmid_idx(void);
 int resctrl_arch_update_domains(struct rdt_resource *r, u32 closid);
 
 /**
diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
index 60d50ac79e7b..3169441a2d40 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
@@ -144,6 +144,20 @@ u32 resctrl_arch_system_num_rmid_idx(void)
 	return num_rmids == U32_MAX ? 0 : num_rmids;
 }
 
+/**
+ * resctrl_arch_system_max_rmid_idx - Largest possible number of RMIDs
+ *
+ * Return: If L3 monitoring is supported, largest possible comes from L3 based
+ * on CPUID(0xf,0x0).EBX (scaled down on Sub-NUMA Cluster systems). Otherwise
+ * maximum from any other mon_capable resources.
+ */
+u32 resctrl_arch_system_max_rmid_idx(void)
+{
+	struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_L3].r_resctrl;
+
+	return r->mon_capable ? r->mon.num_rmid : resctrl_arch_system_num_rmid_idx();
+}
+
 struct rdt_resource *resctrl_arch_get_resource(enum resctrl_res_level l)
 {
 	if (l >= RDT_NUM_RESOURCES)
diff --git a/drivers/resctrl/mpam_resctrl.c b/drivers/resctrl/mpam_resctrl.c
index 226ff6f532fa..7079870ca894 100644
--- a/drivers/resctrl/mpam_resctrl.c
+++ b/drivers/resctrl/mpam_resctrl.c
@@ -272,6 +272,11 @@ u32 resctrl_arch_system_num_rmid_idx(void)
 	return (mpam_pmg_max + 1) * (mpam_partid_max + 1);
 }
 
+u32 resctrl_arch_system_max_rmid_idx(void)
+{
+	return resctrl_arch_system_num_rmid_idx();
+}
+
 u32 resctrl_arch_rmid_idx_encode(u32 closid, u32 rmid)
 {
 	return closid * (mpam_pmg_max + 1) + rmid;
diff --git a/fs/resctrl/monitor.c b/fs/resctrl/monitor.c
index 5d5625e4e3b4..72b1a628488c 100644
--- a/fs/resctrl/monitor.c
+++ b/fs/resctrl/monitor.c
@@ -115,10 +115,18 @@ static inline struct rmid_entry *__rmid_entry(u32 idx)
 
 static void limbo_release_entry(struct rmid_entry *entry)
 {
+	u32 min_idx_limit = resctrl_arch_system_num_rmid_idx();
+
 	lockdep_assert_held(&rdtgroup_mutex);
 
 	rmid_limbo_count--;
-	list_add_tail(&entry->list, &rmid_free_lru);
+
+	/*
+	 * Limbo may be freeing an RMID from a previous mount where there
+	 * were more RMIDs available.
+	 */
+	if (resctrl_arch_rmid_idx_encode(entry->closid, entry->rmid) < min_idx_limit)
+		list_add_tail(&entry->list, &rmid_free_lru);
 
 	if (IS_ENABLED(CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID))
 		closid_num_dirty_rmid[entry->closid]--;
@@ -133,14 +141,20 @@ static void limbo_release_entry(struct rmid_entry *entry)
 void __check_limbo(struct rdt_l3_mon_domain *d, bool force_free)
 {
 	struct rdt_resource *r = resctrl_arch_get_resource(RDT_RESOURCE_L3);
-	u32 idx_limit = resctrl_arch_system_num_rmid_idx();
 	struct rmid_entry *entry;
 	bool rmid_dirty = true;
 	u32 idx, cur_idx = 1;
 	void *arch_mon_ctx;
+	u32 max_idx_limit;
 	void *arch_priv;
 	u64 val = 0;
 
+	/*
+	 * Need to check all possible RMIDs, not just the range available
+	 * in this mount cycle.
+	 */
+	max_idx_limit = resctrl_arch_system_max_rmid_idx();
+
 	arch_priv = mon_event_all[QOS_L3_OCCUP_EVENT_ID].arch_priv;
 	arch_mon_ctx = resctrl_arch_mon_ctx_alloc(r, QOS_L3_OCCUP_EVENT_ID);
 	if (IS_ERR(arch_mon_ctx)) {
@@ -156,8 +170,8 @@ void __check_limbo(struct rdt_l3_mon_domain *d, bool force_free)
 	 * RMID and move it to the free list when the counter reaches 0.
 	 */
 	for (;;) {
-		idx = find_next_bit(d->rmid_busy_llc, idx_limit, cur_idx);
-		if (idx >= idx_limit)
+		idx = find_next_bit(d->rmid_busy_llc, max_idx_limit, cur_idx);
+		if (idx >= max_idx_limit)
 			break;
 
 		entry = __rmid_entry(idx);
@@ -197,9 +211,9 @@ void __check_limbo(struct rdt_l3_mon_domain *d, bool force_free)
 
 bool has_busy_rmid(struct rdt_l3_mon_domain *d)
 {
-	u32 idx_limit = resctrl_arch_system_num_rmid_idx();
+	u32 max_idx_limit = resctrl_arch_system_max_rmid_idx();
 
-	return find_first_bit(d->rmid_busy_llc, idx_limit) != idx_limit;
+	return find_first_bit(d->rmid_busy_llc, max_idx_limit) != max_idx_limit;
 }
 
 static struct rmid_entry *resctrl_find_free_rmid(u32 closid)
@@ -961,8 +975,8 @@ void mbm_setup_overflow_handler(struct rdt_l3_mon_domain *dom, unsigned long del
 
 int setup_rmid_lru_list(void)
 {
+	u32 max_idx_limit, min_idx_limit;
 	struct rmid_entry *entry = NULL;
-	u32 idx_limit;
 	u32 idx;
 	int i;
 
@@ -970,27 +984,29 @@ int setup_rmid_lru_list(void)
 		return 0;
 
 	/*
-	 * Called on every mount, but the number of RMIDs cannot change
-	 * after the first mount, so keep using the same set of rmid_ptrs[]
-	 * until resctrl_exit(). Note that the limbo handler continues to
-	 * access rmid_ptrs[] after resctrl is unmounted.
+	 * Allocate the largest number of RMIDs that this system will ever
+	 * need. These cannot be freed until resctrl_exit() because the limbo
+	 * handler continues to access rmid_ptrs[] after resctrl is unmounted.
 	 */
-	if (rmid_ptrs)
-		return 0;
-
-	idx_limit = resctrl_arch_system_num_rmid_idx();
-	rmid_ptrs = kzalloc_objs(struct rmid_entry, idx_limit);
-	if (!rmid_ptrs)
-		return -ENOMEM;
+	if (!rmid_ptrs) {
+		max_idx_limit = resctrl_arch_system_max_rmid_idx();
+		rmid_ptrs = kzalloc_objs(struct rmid_entry, max_idx_limit);
+		if (!rmid_ptrs)
+			return -ENOMEM;
 
-	for (i = 0; i < idx_limit; i++) {
-		entry = &rmid_ptrs[i];
-		INIT_LIST_HEAD(&entry->list);
+		for (i = 0; i < max_idx_limit; i++) {
+			entry = &rmid_ptrs[i];
+			INIT_LIST_HEAD(&entry->list);
 
-		resctrl_arch_rmid_idx_decode(i, &entry->closid, &entry->rmid);
-		list_add_tail(&entry->list, &rmid_free_lru);
+			resctrl_arch_rmid_idx_decode(i, &entry->closid, &entry->rmid);
+		}
 	}
 
+	/* Find how many RMIDs are needed for this mount */
+	min_idx_limit = resctrl_arch_system_num_rmid_idx();
+
+	INIT_LIST_HEAD(&rmid_free_lru);
+
 	/*
 	 * RESCTRL_RESERVED_CLOSID and RESCTRL_RESERVED_RMID are special and
 	 * are always allocated. These are used for the rdtgroup_default
@@ -998,8 +1014,14 @@ int setup_rmid_lru_list(void)
 	 */
 	idx = resctrl_arch_rmid_idx_encode(RESCTRL_RESERVED_CLOSID,
 					   RESCTRL_RESERVED_RMID);
-	entry = __rmid_entry(idx);
-	list_del(&entry->list);
+
+	for (i = 0; i < min_idx_limit; i++) {
+		entry = &rmid_ptrs[i];
+		/* Don't add reserved or busy entries to free list */
+		if (i == idx || entry->busy)
+			continue;
+		list_add_tail(&entry->list, &rmid_free_lru);
+	}
 
 	return 0;
 }
@@ -1218,7 +1240,7 @@ static void mbm_cntr_free_all(struct rdt_resource *r, struct rdt_l3_mon_domain *
  */
 static void resctrl_reset_rmid_all(struct rdt_resource *r, struct rdt_l3_mon_domain *d)
 {
-	u32 idx_limit = resctrl_arch_system_num_rmid_idx();
+	u32 max_idx_limit = resctrl_arch_system_max_rmid_idx();
 	enum resctrl_event_id evt;
 	int idx;
 
@@ -1226,7 +1248,7 @@ static void resctrl_reset_rmid_all(struct rdt_resource *r, struct rdt_l3_mon_dom
 		if (!resctrl_is_mon_event_enabled(evt))
 			continue;
 		idx = MBM_STATE_IDX(evt);
-		memset(d->mbm_states[idx], 0, sizeof(*d->mbm_states[0]) * idx_limit);
+		memset(d->mbm_states[idx], 0, sizeof(*d->mbm_states[0]) * max_idx_limit);
 	}
 }
 
diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
index ac1a5af00bc5..763e4191d717 100644
--- a/fs/resctrl/rdtgroup.c
+++ b/fs/resctrl/rdtgroup.c
@@ -4623,13 +4623,13 @@ void resctrl_offline_mon_domain(struct rdt_resource *r, struct rdt_domain_hdr *h
  */
 static int domain_setup_l3_mon_state(struct rdt_resource *r, struct rdt_l3_mon_domain *d)
 {
-	u32 idx_limit = resctrl_arch_system_num_rmid_idx();
+	u32 max_idx_limit = resctrl_arch_system_max_rmid_idx();
 	size_t tsize = sizeof(*d->mbm_states[0]);
 	enum resctrl_event_id eventid;
 	int idx;
 
 	if (resctrl_is_mon_event_enabled(QOS_L3_OCCUP_EVENT_ID)) {
-		d->rmid_busy_llc = bitmap_zalloc(idx_limit, GFP_KERNEL);
+		d->rmid_busy_llc = bitmap_zalloc(max_idx_limit, GFP_KERNEL);
 		if (!d->rmid_busy_llc)
 			return -ENOMEM;
 	}
@@ -4638,7 +4638,7 @@ static int domain_setup_l3_mon_state(struct rdt_resource *r, struct rdt_l3_mon_d
 		if (!resctrl_is_mon_event_enabled(eventid))
 			continue;
 		idx = MBM_STATE_IDX(eventid);
-		d->mbm_states[idx] = kcalloc(idx_limit, tsize, GFP_KERNEL);
+		d->mbm_states[idx] = kcalloc(max_idx_limit, tsize, GFP_KERNEL);
 		if (!d->mbm_states[idx])
 			goto cleanup;
 	}
-- 
2.54.0

[PATCH v9 07/12] x86/resctrl: Add PMT registration API for AET enumeration callbacks

[Tony Luck]

resctrl is always built-in; INTEL_PMT_TELEMETRY may be a module.  Add, and
export, register/unregister functions so the PMT module can supply/clear
enumeration callback functions when loaded/unloaded.

Suggested-by: Christoph Hellwig <hch@infradead.org>
Signed-off-by: Tony Luck <tony.luck@intel.com>
---
 arch/x86/include/asm/resctrl.h          | 22 ++++++++++++++++
 arch/x86/kernel/cpu/resctrl/intel_aet.c | 34 +++++++++++++++++++++++++
 2 files changed, 56 insertions(+)

diff --git a/arch/x86/include/asm/resctrl.h b/arch/x86/include/asm/resctrl.h
index a9f481b7a8ed..48ad55bcda66 100644
--- a/arch/x86/include/asm/resctrl.h
+++ b/arch/x86/include/asm/resctrl.h
@@ -4,6 +4,8 @@
 
 #ifdef CONFIG_X86_CPU_RESCTRL
 
+#include <linux/intel_pmt_features.h>
+#include <linux/intel_vsec.h>
 #include <linux/jump_label.h>
 #include <linux/percpu.h>
 #include <linux/resctrl_types.h>
@@ -189,11 +191,31 @@ static inline void resctrl_arch_mon_ctx_free(struct rdt_resource *r,
 
 void resctrl_cpu_detect(struct cpuinfo_x86 *c);
 
+#ifdef CONFIG_X86_CPU_RESCTRL_INTEL_AET
+void intel_aet_register_enumeration(struct module *module,
+				    struct pmt_feature_group *(*get)(enum pmt_feature_id id),
+				    void (*put)(struct pmt_feature_group *p));
+void intel_aet_unregister_enumeration(void);
 #else
+static inline void intel_aet_register_enumeration(struct module *module,
+						  struct pmt_feature_group *(*get)(enum pmt_feature_id id),
+						  void (*put)(struct pmt_feature_group *p)) { }
+static inline void intel_aet_unregister_enumeration(void) { }
+#endif /* CONFIG_X86_CPU_RESCTRL_INTEL_AET */
+
+#else
+
+#include <linux/intel_pmt_features.h>
+#include <linux/intel_vsec.h>
 
 static inline void resctrl_arch_sched_in(struct task_struct *tsk) {}
 static inline void resctrl_cpu_detect(struct cpuinfo_x86 *c) {}
 
+static inline void intel_aet_register_enumeration(struct module *module,
+						  struct pmt_feature_group *(*get)(enum pmt_feature_id id),
+						  void (*put)(struct pmt_feature_group *p)) { }
+static inline void intel_aet_unregister_enumeration(void) { }
+
 #endif /* CONFIG_X86_CPU_RESCTRL */
 
 #endif /* _ASM_X86_RESCTRL_H */
diff --git a/arch/x86/kernel/cpu/resctrl/intel_aet.c b/arch/x86/kernel/cpu/resctrl/intel_aet.c
index cd8257c58f84..c26bd813e057 100644
--- a/arch/x86/kernel/cpu/resctrl/intel_aet.c
+++ b/arch/x86/kernel/cpu/resctrl/intel_aet.c
@@ -12,17 +12,21 @@
 #define pr_fmt(fmt)   "resctrl: " fmt
 
 #include <linux/bits.h>
+#include <linux/cleanup.h>
 #include <linux/compiler_types.h>
 #include <linux/container_of.h>
 #include <linux/cpumask.h>
 #include <linux/err.h>
 #include <linux/errno.h>
+#include <linux/export.h>
 #include <linux/gfp_types.h>
 #include <linux/init.h>
 #include <linux/intel_pmt_features.h>
 #include <linux/intel_vsec.h>
 #include <linux/io.h>
 #include <linux/minmax.h>
+#include <linux/mutex.h>
+#include <linux/module.h>
 #include <linux/printk.h>
 #include <linux/rculist.h>
 #include <linux/rcupdate.h>
@@ -291,6 +295,10 @@ static enum pmt_feature_id lookup_pfid(const char *pfname)
 	return FEATURE_INVALID;
 }
 
+static struct module *pmt_module;
+static struct pmt_feature_group *(*get_feature)(enum pmt_feature_id id);
+static void (*put_feature)(struct pmt_feature_group *p);
+
 /*
  * Request a copy of struct pmt_feature_group for each event group. If there is
  * one, the returned structure has an array of telemetry_region structures,
@@ -325,6 +333,32 @@ bool intel_aet_get_events(void)
 	return ret;
 }
 
+/*
+ * Defend against races between pmt_telemetry register/unregister and
+ * resctrl file system mount/unmount.
+ */
+static DEFINE_MUTEX(aet_register_lock);
+
+void intel_aet_register_enumeration(struct module *module,
+				    struct pmt_feature_group *(*get)(enum pmt_feature_id id),
+				    void (*put)(struct pmt_feature_group *p))
+{
+	guard(mutex)(&aet_register_lock);
+	get_feature = get;
+	put_feature = put;
+	pmt_module = module;
+}
+EXPORT_SYMBOL_NS_GPL(intel_aet_register_enumeration, "INTEL_PMT");
+
+void intel_aet_unregister_enumeration(void)
+{
+	guard(mutex)(&aet_register_lock);
+	pmt_module = NULL;
+	get_feature = NULL;
+	put_feature = NULL;
+}
+EXPORT_SYMBOL_NS_GPL(intel_aet_unregister_enumeration, "INTEL_PMT");
+
 void __exit intel_aet_exit(void)
 {
 	struct event_group **peg;
-- 
2.54.0

[PATCH v9 08/12] platform/x86/intel/pmt: Register enumeration functions with resctrl

[Tony Luck]

INTEL_PMT_TELEMETRY is a loadable module, but resctrl is built-in and cannot
call PMT functions directly.

Use ".probe_type = PROBE_FORCE_SYNCHRONOUS" to ensure sequential, synchronous
calls to per-device .probe() functions.  Register the telemetry enumeration
function pointers at the end of pmt_telem_init() when all .probe() functions
have run and enumeration is complete.

Unregister at the start of pmt_telem_exit() before teardown of the
auxiliary drivers.

Note that checkpatch complains about the #include of <asm/resctrl.h>.
This is needed rather than <linux/resctrl.h> to get the function stub
definitions when CONFIG_X86_CPU_RESCTRL=n.

Suggested-by: Christoph Hellwig <hch@infradead.org>
Signed-off-by: Tony Luck <tony.luck@intel.com>
--
v9:
	Back to register in .init() and unregister in .exti()
	Add PROBE_FORCE_SYNCHRONOUS
---
 drivers/platform/x86/intel/pmt/telemetry.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/intel/pmt/telemetry.c b/drivers/platform/x86/intel/pmt/telemetry.c
index f243ccf3185b..fba0b3912c06 100644
--- a/drivers/platform/x86/intel/pmt/telemetry.c
+++ b/drivers/platform/x86/intel/pmt/telemetry.c
@@ -25,6 +25,8 @@
 #include <linux/uaccess.h>
 #include <linux/xarray.h>
 
+#include <asm/resctrl.h>
+
 #include "class.h"
 
 #define TELEM_SIZE_OFFSET	0x0
@@ -429,17 +431,23 @@ static struct auxiliary_driver pmt_telem_aux_driver = {
 	.probe		= pmt_telem_probe,
 	.driver = {
 		.suppress_bind_attrs = true,
+		.probe_type = PROBE_FORCE_SYNCHRONOUS,
 	},
 };
 
 static int __init pmt_telem_init(void)
 {
-	return auxiliary_driver_register(&pmt_telem_aux_driver);
+	int ret = auxiliary_driver_register(&pmt_telem_aux_driver);
+
+	intel_aet_register_enumeration(THIS_MODULE, intel_pmt_get_regions_by_feature,
+				       intel_pmt_put_feature_group);
+	return ret;
 }
 module_init(pmt_telem_init);
 
 static void __exit pmt_telem_exit(void)
 {
+	intel_aet_unregister_enumeration();
 	auxiliary_driver_unregister(&pmt_telem_aux_driver);
 	xa_destroy(&telem_array);
 }
-- 
2.54.0

[PATCH v9 09/12] arm,x86/resctrl: Resolve INTEL_PMT_TELEMETRY symbols at runtime

[Tony Luck]

resctrl is always built-in, but INTEL_PMT_TELEMETRY and INTEL_TPMI are
logically independent and should be loadable modules.  Switch AET to use the
function-pointer registration API instead of direct link-time references to
PMT symbols.

Prepare for the file system to call resctrl_arch_pre_mount() on every mount
by moving AET enumeration into resctrl_arch_pre_mount() and cleanup into
resctrl_arch_unmount(). This allows the PMT module to be unloaded whenever
the filesystem is not mounted.

Remove intel_aet_exit because all cleanup now happens in the unmount path.

Note that the Linux file system code does not serialize calls to
fs_context_operations::get_tree(), so there may be arbitrarily many parallel
calls if users invoke mount(2) multiple times.

Use "pmt_in_use" to detect nested mount attempts.

event_group::num_rmid may be reset (reduced) during enumeration. This is
not worth resetting on unmount because the same reduction would occur on
each subsequent mount.

Signed-off-by: Tony Luck <tony.luck@intel.com>
--
v9:
	Drop broken atomic ops introduced in v8
	Check pmt_in_use in intel_aet_pre_mount() to detect nested
	mount flow.
---
 include/linux/resctrl.h                 |  6 +++
 arch/x86/kernel/cpu/resctrl/internal.h  | 23 ++++++++--
 arch/x86/kernel/cpu/resctrl/core.c      | 25 +++++++++--
 arch/x86/kernel/cpu/resctrl/intel_aet.c | 58 +++++++++++++++++++++----
 drivers/resctrl/mpam_resctrl.c          |  4 ++
 5 files changed, 101 insertions(+), 15 deletions(-)

diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
index e1501ee72d28..4dc41dc99dc2 100644
--- a/include/linux/resctrl.h
+++ b/include/linux/resctrl.h
@@ -564,6 +564,12 @@ void resctrl_offline_cpu(unsigned int cpu);
  */
 void resctrl_arch_pre_mount(void);
 
+/*
+ * Architecture hook called when mount fails, or on unmount.
+ * No locks are held.
+ */
+void resctrl_arch_unmount(void);
+
 /**
  * resctrl_arch_rmid_read() - Read the eventid counter corresponding to rmid
  *			      for this resource and domain.
diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index e3cfa0c10e92..a891017f69ec 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -234,15 +234,15 @@ void rdt_domain_reconfigure_cdp(struct rdt_resource *r);
 void resctrl_arch_mbm_cntr_assign_set_one(struct rdt_resource *r);
 
 #ifdef CONFIG_X86_CPU_RESCTRL_INTEL_AET
-bool intel_aet_get_events(void);
-void __exit intel_aet_exit(void);
+bool intel_aet_pre_mount(void);
+void intel_aet_unmount(void);
 int intel_aet_read_event(int domid, u32 rmid, void *arch_priv, u64 *val);
 void intel_aet_mon_domain_setup(int cpu, int id, struct rdt_resource *r,
 				struct list_head *add_pos);
 bool intel_handle_aet_option(bool force_off, char *tok);
 #else
-static inline bool intel_aet_get_events(void) { return false; }
-static inline void __exit intel_aet_exit(void) { }
+static inline bool intel_aet_pre_mount(void) { return false; }
+static inline void intel_aet_unmount(void) { }
 static inline int intel_aet_read_event(int domid, u32 rmid, void *arch_priv, u64 *val)
 {
 	return -EINVAL;
@@ -253,4 +253,19 @@ static inline void intel_aet_mon_domain_setup(int cpu, int id, struct rdt_resour
 static inline bool intel_handle_aet_option(bool force_off, char *tok) { return false; }
 #endif
 
+#ifdef CONFIG_INTEL_PMT_TELEMETRY_MODULE
+static inline bool intel_aet_try_module_get(struct module *mod)
+{
+	return try_module_get(mod);
+}
+
+static inline void intel_aet_module_put(struct module *mod)
+{
+	module_put(mod);
+}
+#else
+static inline bool intel_aet_try_module_get(struct module *mod) { return true; }
+static inline void intel_aet_module_put(struct module *mod) { }
+#endif
+
 #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 3169441a2d40..3ef620d8e0c8 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
@@ -16,10 +16,12 @@
 
 #define pr_fmt(fmt)	"resctrl: " fmt
 
+#include <linux/cleanup.h>
 #include <linux/cpu.h>
 #include <linux/slab.h>
 #include <linux/err.h>
 #include <linux/cpuhotplug.h>
+#include <linux/mutex.h>
 
 #include <asm/cpu_device_id.h>
 #include <asm/cpuid/api.h>
@@ -798,7 +800,7 @@ void resctrl_arch_pre_mount(void)
 	struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_PERF_PKG].r_resctrl;
 	int cpu;
 
-	if (!intel_aet_get_events())
+	if (!intel_aet_pre_mount())
 		return;
 
 	/*
@@ -814,6 +816,25 @@ void resctrl_arch_pre_mount(void)
 	cpus_read_unlock();
 }
 
+void resctrl_arch_unmount(void)
+{
+	struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_PERF_PKG].r_resctrl;
+	int cpu;
+
+	intel_aet_unmount();
+
+	if (!r->mon_capable)
+		return;
+
+	cpus_read_lock();
+	mutex_lock(&domain_list_lock);
+	for_each_online_cpu(cpu)
+		domain_remove_cpu_mon(cpu, r);
+	r->mon_capable = false;
+	mutex_unlock(&domain_list_lock);
+	cpus_read_unlock();
+}
+
 enum {
 	RDT_FLAG_CMT,
 	RDT_FLAG_MBM_TOTAL,
@@ -1187,8 +1208,6 @@ 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
index c26bd813e057..c2e27ce52248 100644
--- a/arch/x86/kernel/cpu/resctrl/intel_aet.c
+++ b/arch/x86/kernel/cpu/resctrl/intel_aet.c
@@ -25,8 +25,8 @@
 #include <linux/intel_vsec.h>
 #include <linux/io.h>
 #include <linux/minmax.h>
-#include <linux/mutex.h>
 #include <linux/module.h>
+#include <linux/mutex.h>
 #include <linux/printk.h>
 #include <linux/rculist.h>
 #include <linux/rcupdate.h>
@@ -310,7 +310,7 @@ static void (*put_feature)(struct pmt_feature_group *p);
  * struct pmt_feature_group to indicate that its events are successfully
  * enabled.
  */
-bool intel_aet_get_events(void)
+static bool aet_get_events(void)
 {
 	struct pmt_feature_group *p;
 	enum pmt_feature_id pfid;
@@ -319,14 +319,14 @@ bool intel_aet_get_events(void)
 
 	for_each_event_group(peg) {
 		pfid = lookup_pfid((*peg)->pfname);
-		p = intel_pmt_get_regions_by_feature(pfid);
+		p = get_feature(pfid);
 		if (IS_ERR_OR_NULL(p))
 			continue;
 		if (enable_events(*peg, p)) {
 			(*peg)->pfg = p;
 			ret = true;
 		} else {
-			intel_pmt_put_feature_group(p);
+			put_feature(p);
 		}
 	}
 
@@ -359,16 +359,58 @@ void intel_aet_unregister_enumeration(void)
 }
 EXPORT_SYMBOL_NS_GPL(intel_aet_unregister_enumeration, "INTEL_PMT");
 
-void __exit intel_aet_exit(void)
+/*
+ * Track whether pmt_telemetry enumeration succeeded during mount to
+ * detect nested mount attempts and for use during unmount.
+ */
+static bool pmt_in_use;
+
+bool intel_aet_pre_mount(void)
+{
+	bool ret;
+
+	guard(mutex)(&aet_register_lock);
+
+	/* Nested mount attempt. File system code will return -EBUSY */
+	if (pmt_in_use)
+		return false;
+
+	if (!get_feature || !put_feature)
+		return false;
+
+	if (!intel_aet_try_module_get(pmt_module))
+		return false;
+
+	ret = aet_get_events();
+
+	if (!ret)
+		intel_aet_module_put(pmt_module);
+	else
+		pmt_in_use = true;
+
+	return ret;
+}
+
+void intel_aet_unmount(void)
 {
 	struct event_group **peg;
 
+	guard(mutex)(&aet_register_lock);
+	if (!pmt_in_use)
+		return;
+
 	for_each_event_group(peg) {
-		if ((*peg)->pfg) {
-			intel_pmt_put_feature_group((*peg)->pfg);
-			(*peg)->pfg = NULL;
+		struct event_group *e = *peg;
+
+		if (e->pfg) {
+			for (int j = 0; j < e->num_events; j++)
+				resctrl_disable_mon_event(e->evts[j].id);
+			put_feature(e->pfg);
+			e->pfg = NULL;
 		}
 	}
+	intel_aet_module_put(pmt_module);
+	pmt_in_use = false;
 }
 
 #define DATA_VALID	BIT_ULL(63)
diff --git a/drivers/resctrl/mpam_resctrl.c b/drivers/resctrl/mpam_resctrl.c
index 7079870ca894..5859cc0f5e37 100644
--- a/drivers/resctrl/mpam_resctrl.c
+++ b/drivers/resctrl/mpam_resctrl.c
@@ -162,6 +162,10 @@ void resctrl_arch_pre_mount(void)
 {
 }
 
+void resctrl_arch_unmount(void)
+{
+}
+
 bool resctrl_arch_get_cdp_enabled(enum resctrl_res_level rid)
 {
 	return mpam_resctrl_controls[rid].cdp_enabled;
-- 
2.54.0

[PATCH v9 10/12] fs/resctrl: Call architecture hooks for every mount/unmount

[Tony Luck]

The intel_pmt module should only be pinned while the resctrl file system
is mounted.

Add hooks for every mount/unmount of the resctrl file system so that
architecture code can increment the intel_pmt module reference count on
mount and decrement on unmount.

Signed-off-by: Tony Luck <tony.luck@intel.com>
---
 include/linux/resctrl.h | 2 +-
 fs/resctrl/rdtgroup.c   | 8 ++++++--
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
index 4dc41dc99dc2..483a50df7ae6 100644
--- a/include/linux/resctrl.h
+++ b/include/linux/resctrl.h
@@ -559,7 +559,7 @@ void resctrl_online_cpu(unsigned int cpu);
 void resctrl_offline_cpu(unsigned int cpu);
 
 /*
- * Architecture hook called at beginning of first file system mount attempt.
+ * Architecture hook called before attempting to mount the file system.
  * No locks are held.
  */
 void resctrl_arch_pre_mount(void);
diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
index 763e4191d717..96a337156f3c 100644
--- a/fs/resctrl/rdtgroup.c
+++ b/fs/resctrl/rdtgroup.c
@@ -18,7 +18,6 @@
 #include <linux/fs_parser.h>
 #include <linux/sysfs.h>
 #include <linux/kernfs.h>
-#include <linux/once.h>
 #include <linux/resctrl.h>
 #include <linux/seq_buf.h>
 #include <linux/seq_file.h>
@@ -3168,6 +3167,8 @@ static void resctrl_unmount(void)
 	resctrl_mounted = false;
 	mutex_unlock(&rdtgroup_mutex);
 	cpus_read_unlock();
+
+	resctrl_arch_unmount();
 }
 
 static int rdt_get_tree(struct fs_context *fc)
@@ -3179,7 +3180,7 @@ static int rdt_get_tree(struct fs_context *fc)
 	struct rdt_resource *r;
 	int ret;
 
-	DO_ONCE_SLEEPABLE(resctrl_arch_pre_mount);
+	resctrl_arch_pre_mount();
 
 	cpus_read_lock();
 	mutex_lock(&rdtgroup_mutex);
@@ -3318,6 +3319,9 @@ static int rdt_get_tree(struct fs_context *fc)
 out:
 	mutex_unlock(&rdtgroup_mutex);
 	cpus_read_unlock();
+
+	resctrl_arch_unmount();
+
 	return ret;
 }
 
-- 
2.54.0

[PATCH v9 11/12] x86/resctrl: Simplify Kconfig options for resctrl

[Tony Luck]

Linus Torvalds complained[1] about Kconfig complexity making it too
hard for "random people to build their own kernels".

CONFIG_X86_CPU_RESCTRL_INTEL_AET has been causing problems since it was
first added as it required other config options to be set to "built-in".

AET now resolves PMT symbols at runtime via the registration API,
so INTEL_PMT_TELEMETRY no longer needs to be built-in. This means
that AET can be included unconditionally as part of X86_CPU_RESCTRL.

CONFIG_X86_CPU_RESCTRL now depends on X86_64 because reading telemetry
data needs readq().

Signed-off-by: Tony Luck <tony.luck@intel.com>
Link: https://lore.kernel.org/all/CAHk-=whigg3hvOy7c1j1MXFy6o6CHp0g4Tc3Y-MAk+XDssHU0A@mail.gmail.com # 1
---
 arch/x86/include/asm/resctrl.h         |  7 -------
 arch/x86/kernel/cpu/resctrl/internal.h | 13 -------------
 arch/x86/Kconfig                       | 15 +--------------
 arch/x86/kernel/cpu/resctrl/Makefile   |  2 +-
 4 files changed, 2 insertions(+), 35 deletions(-)

diff --git a/arch/x86/include/asm/resctrl.h b/arch/x86/include/asm/resctrl.h
index 48ad55bcda66..abb0790e4a4e 100644
--- a/arch/x86/include/asm/resctrl.h
+++ b/arch/x86/include/asm/resctrl.h
@@ -191,17 +191,10 @@ static inline void resctrl_arch_mon_ctx_free(struct rdt_resource *r,
 
 void resctrl_cpu_detect(struct cpuinfo_x86 *c);
 
-#ifdef CONFIG_X86_CPU_RESCTRL_INTEL_AET
 void intel_aet_register_enumeration(struct module *module,
 				    struct pmt_feature_group *(*get)(enum pmt_feature_id id),
 				    void (*put)(struct pmt_feature_group *p));
 void intel_aet_unregister_enumeration(void);
-#else
-static inline void intel_aet_register_enumeration(struct module *module,
-						  struct pmt_feature_group *(*get)(enum pmt_feature_id id),
-						  void (*put)(struct pmt_feature_group *p)) { }
-static inline void intel_aet_unregister_enumeration(void) { }
-#endif /* CONFIG_X86_CPU_RESCTRL_INTEL_AET */
 
 #else
 
diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index a891017f69ec..b50faedede47 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -233,25 +233,12 @@ void __init intel_rdt_mbm_apply_quirk(void);
 void rdt_domain_reconfigure_cdp(struct rdt_resource *r);
 void resctrl_arch_mbm_cntr_assign_set_one(struct rdt_resource *r);
 
-#ifdef CONFIG_X86_CPU_RESCTRL_INTEL_AET
 bool intel_aet_pre_mount(void);
 void intel_aet_unmount(void);
 int intel_aet_read_event(int domid, u32 rmid, void *arch_priv, u64 *val);
 void intel_aet_mon_domain_setup(int cpu, int id, struct rdt_resource *r,
 				struct list_head *add_pos);
 bool intel_handle_aet_option(bool force_off, char *tok);
-#else
-static inline bool intel_aet_pre_mount(void) { return false; }
-static inline void intel_aet_unmount(void) { }
-static inline int intel_aet_read_event(int domid, u32 rmid, void *arch_priv, u64 *val)
-{
-	return -EINVAL;
-}
-
-static inline void intel_aet_mon_domain_setup(int cpu, int id, struct rdt_resource *r,
-					      struct list_head *add_pos) { }
-static inline bool intel_handle_aet_option(bool force_off, char *tok) { return false; }
-#endif
 
 #ifdef CONFIG_INTEL_PMT_TELEMETRY_MODULE
 static inline bool intel_aet_try_module_get(struct module *mod)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index bdad90f210e4..ae6fb01e8865 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -514,7 +514,7 @@ config X86_MPPARSE
 
 config X86_CPU_RESCTRL
 	bool "x86 CPU resource control support"
-	depends on X86 && (CPU_SUP_INTEL || CPU_SUP_AMD)
+	depends on X86_64 && (CPU_SUP_INTEL || CPU_SUP_AMD)
 	depends on MISC_FILESYSTEMS
 	select ARCH_HAS_CPU_RESCTRL
 	select RESCTRL_FS
@@ -535,19 +535,6 @@ config X86_CPU_RESCTRL
 
 	  Say N if unsure.
 
-config X86_CPU_RESCTRL_INTEL_AET
-	bool "Intel Application Energy Telemetry"
-	depends on X86_64 && X86_CPU_RESCTRL && CPU_SUP_INTEL && INTEL_PMT_TELEMETRY=y && INTEL_TPMI=y
-	help
-	  Enable per-RMID telemetry events in resctrl.
-
-	  Intel feature that collects per-RMID execution data
-	  about energy consumption, measure of frequency independent
-	  activity and other performance metrics. 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 273ddfa30836..97ceb4e44dfa 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)	+= intel_aet.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.54.0

[PATCH v9 12/12] Documentation/filesystems/resctrl: Document telemetry mount timing caveat

[Tony Luck]

The PMT driver enumerates telemetry features asynchronously, so an
automatic mount of resctrl from /etc/fstab early in boot may occur before
those features are available, resulting in them not being enabled in the
mounted instance.

Add a footnote to the 'If telemetry monitoring is enabled' sentence
pointing readers to a new "Mounting resctrl with telemetry" section. That
section explains the race and provides example systemd service and udev
rule snippets that defer the mount until after the pmt_telemetry module
has loaded.

Assisted-by: Claude:Opus_4.7
Signed-off-by: Tony Luck <tony.luck@intel.com>
--
v9:
	Added section with example udev/systemd configuration
---
 Documentation/filesystems/resctrl.rst | 60 ++++++++++++++++++++++++++-
 1 file changed, 59 insertions(+), 1 deletion(-)

diff --git a/Documentation/filesystems/resctrl.rst b/Documentation/filesystems/resctrl.rst
index e4b66af55ffb..260a5eef675f 100644
--- a/Documentation/filesystems/resctrl.rst
+++ b/Documentation/filesystems/resctrl.rst
@@ -630,7 +630,7 @@ When monitoring is enabled all MON groups will also contain:
 	each instance of an L3 cache. Each directory contains files for the enabled
 	L3 events (e.g. "llc_occupancy", "mbm_total_bytes", and "mbm_local_bytes").
 
-	If telemetry monitoring is enabled, there will be a "mon_PERF_PKG_YY"
+	If telemetry monitoring is enabled [#]_, there will be a "mon_PERF_PKG_YY"
 	directory for each physical processor package. Each directory contains
 	files for the enabled telemetry events (e.g. "core_energy". "activity",
 	"uops_retired", etc.)
@@ -669,6 +669,10 @@ When monitoring is enabled all MON groups will also contain:
 	returned if the MBM event does not have an assigned counter in the
 	CTRL_MON group nor in any of its associated MON groups.
 
+.. [#] Telemetry features are enumerated asynchronously by the PMT driver, so
+   an automatic mount of resctrl from ``/etc/fstab`` at boot may not enable
+   them. See `Mounting resctrl with telemetry`_ below.
+
 "mon_hw_id":
 	Available only with debug option. The identifier used by hardware
 	for the monitor group. On x86 this is the RMID.
@@ -1898,6 +1902,60 @@ m. Unmount the resctrl filesystem.
 
   # umount /sys/fs/resctrl/
 
+Mounting resctrl with telemetry
+===============================
+
+Telemetry features (e.g. the ``mon_PERF_PKG_YY`` events) are enumerated
+asynchronously by the PMT driver. If resctrl is mounted before that
+enumeration completes - for example, when mounted automatically from
+``/etc/fstab`` early in boot - the telemetry features will not be available
+at mount time and will therefore not be enabled in the mounted instance.
+
+To avoid this race, defer the mount until after the ``pmt_telemetry`` module
+has loaded. One way to do this is with a udev rule that triggers a systemd
+service when the module appears. A tmpfs mount onto /sys/fs/resctrl is
+needed to prevent daemon tasks from obtaining references to the resctrl
+file system in the other name spaces that systemd uses during startup.
+
+Example systemd tmpfs mount service (``/etc/systemd/system/sys-fs-resctrl.mount``)::
+
+  [Unit]
+  Description=Early Resctrl Namespace Firewall
+  DefaultDependencies=no
+  Before=basic.target local-fs.target
+
+  [Mount]
+  What=tmpfs
+  Where=/sys/fs/resctrl
+  Type=tmpfs
+  Options=private,nosuid,nodev,noexec,mode=755
+
+  [Install]
+  WantedBy=sysinit.target
+
+Example systemd service (``/etc/systemd/system/mount-resctrl.service``)::
+
+  [Unit]
+  Description=Mount real resctrl pseudo-filesystem natively
+  DefaultDependencies=no
+  Requires=sys-fs-resctrl.mount
+  After=sys-fs-resctrl.mount
+
+  [Service]
+  Type=oneshot
+  # Pause to let systemd start daemons
+  ExecStart=/usr/bin/sleep 10
+  # Mount the real resctrl file system
+  ExecStart=/usr/bin/mount -t resctrl resctrl /sys/fs/resctrl
+  RemainAfterExit=no
+
+  [Install]
+  WantedBy=multi-user.target
+
+Example udev rule (``/etc/udev/rules.d/99-rmid-telemetry.rules``)::
+
+  SUBSYSTEM=="module", KERNEL=="pmt_telemetry", ACTION=="add", RUN+="/usr/bin/systemctl start mount-resctrl.service"
+
 Intel RDT Errata
 ================
 
-- 
2.54.0

Re: [PATCH v9 01/12] platform/x86/intel/{pmt,vsec}: Prevent unbind via sysfs

[Reinette Chatre]

Hi Tony,

On 7/1/26 2:35 PM, Tony Luck wrote:
> The resctrl AET code uses intel_pmt_get_regions_by_feature() (exported by the
> pmt_telemetry module) to obtain pmt_feature_group instances that are populated
> using data from the pmt_discovery module. The returned telemetry_region::addr
> values are MMIO mappings created in the telemetry driver's probe path,
> and resctrl reads from them while a mount is active.
> 
> However, a user can unbind the intel_pmt driver or its parent intel_vsec
> from their devices via sysfs "unbind" attributes. Unbinding either of these
> drivers would tear down the MMIO mappings, leaving resctrl with stale pointers
> and causing a fault on the next access.
> 
> It is safe for the discovery driver to be unbound. It does all its work at
> initial probe time and saves a copy of the features and RMID count.
> 
> Set device_driver::suppress_bind_attrs in each driver so that the "bind"
> and "unbind" sysfs attributes are not exposed and the user cannot detach
> the driver from its devices while resctrl (or any other in-kernel consumer)
> may be using the resources.

Unbinding via sysfs does not seem to be the only way in which resources that
resctrl depend on could disappear.  The "/sys/bus/pci/devices/.../remove"
interface looks to be another way? I am not aware of a way to prevent this since
this represents how devices could just be yanked from a system. I wonder if
resctrl should instead protect itself against telemetry resources disappearing
after mount with another helper that telemetry driver can call when the device
is being removed or the module is being unloaded. When telemetry driver informs AET
that resources are going away after resctrl is mounted then resctrl/AET can return
EIO on event read attempts? resctrl could possibly recover when the device/module
re-appears but if that is complicated then user space could just be forced to remount
resctrl.

With this there is no need to prevent unbind here since it creates a false
guarantee to resctrl.

> 
> Fixes: 1fb2daa60de6 ("x86/resctrl: Discover hardware telemetry events")
> Assisted-by: Claude:Opus-4.7
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> --

Please note the separator typo ("--" -> "---") above that exists in every patch in
this series that has a changelog. Please check all patches for this, I only highlight this
instance.

Reinette

Re: [PATCH v9 03/12] x86/resctrl: Honor rdt=perf option to force enable AET perf events

[Reinette Chatre]

Hi Tony,

On 7/1/26 2:35 PM, Tony Luck wrote:
> The kernel command line option "rdt=" is used to force enable or disable
> individual resctrl features.
> 
> Linux only enumerates AET (Application Energy Telemetry) once on first
> mount. But that is going to change to enumerate on each mount to allow the
> pmt_telemetry driver to be configured as a module, and allow unloading when
> not in use.
> 
> The following scenario will be a problem:
> 
> 1) User mounts the resctrl file system.  all_regions_have_sufficient_rmid()
> notes that a perf telemetry region supports fewer RMIDs than expected for
> the event_group and sets e->force_off = true. But the user had specified
> "rdt=perf" on the kernel command line so perf events are enabled for
> this first mount.
> 
> 2) Resctrl file system is unmounted and later remounted.
> 
> 3) During this new mount cycle enable_events() sees that e->force_off is
> set and stops enumeration for this event group. So on this second, and
> all subsequent, mounts perf events are not enabled.
> 
> Fix by checking the state of e->force_on at the start of enable_events().

Above just writes in words what the patch does. Please reduce repeating code
segments in the changelog and replace with higher level descriptions to help understand
"why". 
I think a more detailed context will help with this. Consider, for example,
something like below (a suggestion to start, please do not copy&paste):
	resctrl can disable ("force_off") a feature supported by the platform if,       
	for example, there is an erratum related to the feature or, in the case of      
	AET (Application Energy Telemetry), when the event group has insufficient       
	RMIDs. The kernel command line option "rdt=" can be used to override            
	("force_on") when a feature is disabled in such case and also let the user      
	disable ("force_off") individual supported resctrl features if they are not     
	needed.                                

	Linux enumerates AET once on first mount and skips this enumeration if the
	associated event group is supported but disabled. Since AET is only enumerated
	once, this check is guaranteed to only consider disabling via the rdt= kernel
	parameter. However, when transitioning to enumerate AET on every mount, it is
	no longer correct to skip enumeration simply because an event group is marked
	as disabled. It is then possible for resctrl to have force-disabled the event
	group during an earlier enumeration due to insufficient RMIDs, which would
	improperly cause the kernel to ignore a user's explicit rdt= override on
	subsequent mounts.

	Ensure the user's command line choices take precedence over system-level limits.

> 
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> ---
>  arch/x86/kernel/cpu/resctrl/intel_aet.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/intel_aet.c b/arch/x86/kernel/cpu/resctrl/intel_aet.c
> index c22c3cf5167d..cd8257c58f84 100644
> --- a/arch/x86/kernel/cpu/resctrl/intel_aet.c
> +++ b/arch/x86/kernel/cpu/resctrl/intel_aet.c
> @@ -60,8 +60,8 @@ struct pmt_event {
>   *			data for all telemetry regions of type @pfname.
>   *			Valid if the system supports the event group,
>   *			NULL otherwise.
> - * @force_off:		True when "rdt" command line or architecture code disables
> - *			this event group due to insufficient RMIDs.
> + * @force_off:		True when "rdt" command line disables this event group
> + *			to avoid system limitations due to insufficient RMIDs.
>   * @force_on:		True when "rdt" command line overrides disable of this
>   *			event group.
>   * @guid:		Unique number per XML description file.

This is unexpected. The original comment is more accurate and I understood from 
https://lore.kernel.org/lkml/ainCnu4xmHbgh91r@agluck-desk3/ that you were not going to
make changes here.

> @@ -228,7 +228,7 @@ static bool enable_events(struct event_group *e, struct pmt_feature_group *p)
>  	struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_PERF_PKG].r_resctrl;
>  	int skipped_events = 0;
>  
> -	if (e->force_off)
> +	if (e->force_off && !e->force_on)
>  		return false;
>  
>  	if (!group_has_usable_regions(e, p))

Reinette

Re: [PATCH v9 05/12] x86/resctrl: Drop global 'rdt_mon_capable' flag

[Reinette Chatre]

Hi Tony,

On 7/1/26 2:35 PM, Tony Luck wrote:
> File system code calls resctrl_arch_mon_capable() to determine if the
> system supports any monitoring features.
> 
> x86 architecture code sets a global flag to support implementation of
> resctrl_arch_mon_capable() but the upcoming change to enumerate AET
> (Application Energy Telemetry) features on each mount introduces a new
> corner case.
> 
> Specifically when AET is the only monitoring feature, loading and
> unloading the pmt_telemetry module between resctrl mounts may result
> in monitoring support enabled on some mounts, but not on others.
> 
> Replace the global flag with a scan of resources to check if any are
> marked mon_capable.
> 
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> ---

...

> diff --git a/arch/x86/include/asm/resctrl.h b/arch/x86/include/asm/resctrl.h
> index 575f8408a9e7..a9f481b7a8ed 100644
> --- a/arch/x86/include/asm/resctrl.h
> +++ b/arch/x86/include/asm/resctrl.h

...

> @@ -66,10 +65,7 @@ static inline void resctrl_arch_disable_alloc(void)
>  	static_branch_dec_cpuslocked(&rdt_enable_key);
>  }
>  
> -static inline bool resctrl_arch_mon_capable(void)
> -{
> -	return rdt_mon_capable;
> -}
> +bool resctrl_arch_mon_capable(void);
>  
>  static inline void resctrl_arch_enable_mon(void)
>  {
> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> index 9b9495174041..60d50ac79e7b 100644
> --- a/arch/x86/kernel/cpu/resctrl/core.c
> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> @@ -51,6 +51,20 @@ DEFINE_PER_CPU(struct resctrl_pqr_state, pqr_state);
>   */
>  bool rdt_alloc_capable;
>  
> +/*
> + * Need to re-evaluate on each mount whether any mon_capable resources
> + * are enabled.
> + */

This series introduces a significant change in behavior of this call and I
find that having the declaration in asm header and this significant comment in
arch code to essentially bury very important information on this
call's usage. Could the declaration be elevated to include/linux/resctrl.h
where its function comments can provide the details on how resctrl fs
uses this call and what its expectations from the architecture are? 
Basically this series causes resctrl_arch_mon_capable() to possibly return
different value depending on when it is called so there needs to be
a contract between resctrl fs and architecture on when resctrl fs can safely
call this and expect consistent results.

> +bool resctrl_arch_mon_capable(void)
> +{
> +	struct rdt_resource *r;
> +
> +	for_each_mon_capable_rdt_resource(r)
> +		return true;
> +
> +	return false;
> +}
> +

Reinette

Re: [PATCH v9 06/12] arm,x86,fs/resctrl: Handle change in number of RMIDs on each mount

[Reinette Chatre]

Hi Tony,

On 7/1/26 2:35 PM, Tony Luck wrote:
> Application Energy Telemetry (AET) event enumeration takes place
> asynchronously. Linux builds the pmt_telemetry module into the kernel to
> kick off enumeration early enough that it completes before first mount of
> the resctrl file system.
> 
> Allowing pmt_telemetry to be a loadable module means that it is possible
> for different numbers of RMIDs to be supported on each mount, depending
> on whether pmt_telemetry module is loaded.
> 
> For simplicity, calculate the maximum possible number of RMIDs and use
> that value to allocate the rmid_ptrs[] array just once. Also use this
> maximum RMID value when allocating rdt_l3_mon_domain::rmid_busy_llc
> bitmap and rdt_l3_mon_domain::mbm_states.

This does not sound right. Why use the maximum RMID for L3 monitoring state?
It can be guaranteed that L3 monitoring state is only accessed when that
monitoring is enabled and when it is enabled it can be guaranteed to never
use more RMID than what L3 itself supports. Why would it ever be required
to allocate more than that? Could this not instead be limited to
rdt_resource::resctrl_mon::num_rmid? From what I can tell such transition
will make it explicit and consistent (since arch code already allocates
this state based on this) how the L3 monitoring state is sized.

> 
> The limbo code must deal with changes in the number of RMIDs from one
> mount to the next because some RMIDs may still be "busy" when the file
> system is unmounted, but be above resctrl_arch_system_num_rmid_idx()
> for the remount. In this case RMIDs that can be released are not put
> onto the rmid_free_lru list.
> 
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> ---

...

> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> index 60d50ac79e7b..3169441a2d40 100644
> --- a/arch/x86/kernel/cpu/resctrl/core.c
> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> @@ -144,6 +144,20 @@ u32 resctrl_arch_system_num_rmid_idx(void)
>  	return num_rmids == U32_MAX ? 0 : num_rmids;
>  }
>  
> +/**
> + * resctrl_arch_system_max_rmid_idx - Largest possible number of RMIDs
> + *
> + * Return: If L3 monitoring is supported, largest possible comes from L3 based
> + * on CPUID(0xf,0x0).EBX (scaled down on Sub-NUMA Cluster systems). Otherwise
> + * maximum from any other mon_capable resources.

Above reads "Otherwise maximum from any other ..." but the code is actually
"Otherwise minimum from any other ..." which contradicts the "max" intention of
this function?

> + */
> +u32 resctrl_arch_system_max_rmid_idx(void)
> +{
> +	struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_L3].r_resctrl;
> +
> +	return r->mon_capable ? r->mon.num_rmid : resctrl_arch_system_num_rmid_idx();

This smells like a function that pretends to be generic but created to provide
correct results on specific x86 hardware. 
If this needs to provide max RMID why not cycle through all monitoring resources
and determine the max RMID, why special case L3?

...

> @@ -961,8 +975,8 @@ void mbm_setup_overflow_handler(struct rdt_l3_mon_domain *dom, unsigned long del
>  
>  int setup_rmid_lru_list(void)
>  {
> +	u32 max_idx_limit, min_idx_limit;
>  	struct rmid_entry *entry = NULL;
> -	u32 idx_limit;
>  	u32 idx;
>  	int i;
>  
> @@ -970,27 +984,29 @@ int setup_rmid_lru_list(void)
>  		return 0;
>  
>  	/*
> -	 * Called on every mount, but the number of RMIDs cannot change
> -	 * after the first mount, so keep using the same set of rmid_ptrs[]
> -	 * until resctrl_exit(). Note that the limbo handler continues to
> -	 * access rmid_ptrs[] after resctrl is unmounted.
> +	 * Allocate the largest number of RMIDs that this system will ever
> +	 * need. These cannot be freed until resctrl_exit() because the limbo
> +	 * handler continues to access rmid_ptrs[] after resctrl is unmounted.
>  	 */
> -	if (rmid_ptrs)
> -		return 0;
> -
> -	idx_limit = resctrl_arch_system_num_rmid_idx();
> -	rmid_ptrs = kzalloc_objs(struct rmid_entry, idx_limit);
> -	if (!rmid_ptrs)
> -		return -ENOMEM;
> +	if (!rmid_ptrs) {
> +		max_idx_limit = resctrl_arch_system_max_rmid_idx();
> +		rmid_ptrs = kzalloc_objs(struct rmid_entry, max_idx_limit);
> +		if (!rmid_ptrs)
> +			return -ENOMEM;
>  
> -	for (i = 0; i < idx_limit; i++) {
> -		entry = &rmid_ptrs[i];
> -		INIT_LIST_HEAD(&entry->list);
> +		for (i = 0; i < max_idx_limit; i++) {
> +			entry = &rmid_ptrs[i];
> +			INIT_LIST_HEAD(&entry->list);
>  
> -		resctrl_arch_rmid_idx_decode(i, &entry->closid, &entry->rmid);
> -		list_add_tail(&entry->list, &rmid_free_lru);
> +			resctrl_arch_rmid_idx_decode(i, &entry->closid, &entry->rmid);
> +		}
>  	}
>  
> +	/* Find how many RMIDs are needed for this mount */
> +	min_idx_limit = resctrl_arch_system_num_rmid_idx();

Since mon_capable resources can now come and go from mount to mount there are scenarios where
rmid_ptrs[] may be smaller than min_idx_limit so above needs extra checks to protect against
overrun below.

> +
> +	INIT_LIST_HEAD(&rmid_free_lru);
> +
>  	/*
>  	 * RESCTRL_RESERVED_CLOSID and RESCTRL_RESERVED_RMID are special and
>  	 * are always allocated. These are used for the rdtgroup_default
> @@ -998,8 +1014,14 @@ int setup_rmid_lru_list(void)
>  	 */
>  	idx = resctrl_arch_rmid_idx_encode(RESCTRL_RESERVED_CLOSID,
>  					   RESCTRL_RESERVED_RMID);
> -	entry = __rmid_entry(idx);
> -	list_del(&entry->list);
> +
> +	for (i = 0; i < min_idx_limit; i++) {
> +		entry = &rmid_ptrs[i];
> +		/* Don't add reserved or busy entries to free list */
> +		if (i == idx || entry->busy)
> +			continue;
> +		list_add_tail(&entry->list, &rmid_free_lru);
> +	}
>  
>  	return 0;
>  }

Reinette

Re: [PATCH v9 07/12] x86/resctrl: Add PMT registration API for AET enumeration callbacks

[Reinette Chatre]

Hi Tony,

On 7/1/26 2:35 PM, Tony Luck wrote:
> resctrl is always built-in; INTEL_PMT_TELEMETRY may be a module.  Add, and
> export, register/unregister functions so the PMT module can supply/clear
> enumeration callback functions when loaded/unloaded.
> 
> Suggested-by: Christoph Hellwig <hch@infradead.org>
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> ---
>  arch/x86/include/asm/resctrl.h          | 22 ++++++++++++++++
>  arch/x86/kernel/cpu/resctrl/intel_aet.c | 34 +++++++++++++++++++++++++
>  2 files changed, 56 insertions(+)
> 
> diff --git a/arch/x86/include/asm/resctrl.h b/arch/x86/include/asm/resctrl.h
> index a9f481b7a8ed..48ad55bcda66 100644
> --- a/arch/x86/include/asm/resctrl.h
> +++ b/arch/x86/include/asm/resctrl.h
> @@ -4,6 +4,8 @@
>  
>  #ifdef CONFIG_X86_CPU_RESCTRL
>  
> +#include <linux/intel_pmt_features.h>
> +#include <linux/intel_vsec.h>
>  #include <linux/jump_label.h>
>  #include <linux/percpu.h>
>  #include <linux/resctrl_types.h>
> @@ -189,11 +191,31 @@ static inline void resctrl_arch_mon_ctx_free(struct rdt_resource *r,
>  
>  void resctrl_cpu_detect(struct cpuinfo_x86 *c);
>  
> +#ifdef CONFIG_X86_CPU_RESCTRL_INTEL_AET
> +void intel_aet_register_enumeration(struct module *module,
> +				    struct pmt_feature_group *(*get)(enum pmt_feature_id id),
> +				    void (*put)(struct pmt_feature_group *p));
> +void intel_aet_unregister_enumeration(void);
>  #else
> +static inline void intel_aet_register_enumeration(struct module *module,
> +						  struct pmt_feature_group *(*get)(enum pmt_feature_id id),
> +						  void (*put)(struct pmt_feature_group *p)) { }
> +static inline void intel_aet_unregister_enumeration(void) { }
> +#endif /* CONFIG_X86_CPU_RESCTRL_INTEL_AET */
> +
> +#else
> +
> +#include <linux/intel_pmt_features.h>
> +#include <linux/intel_vsec.h>

Are we sure we want to include these in arch/x86/kernel/process_32.c and
arch/x86/kernel/process_64.c when resctrl is not built in?

>  
>  static inline void resctrl_arch_sched_in(struct task_struct *tsk) {}
>  static inline void resctrl_cpu_detect(struct cpuinfo_x86 *c) {}
>  
> +static inline void intel_aet_register_enumeration(struct module *module,
> +						  struct pmt_feature_group *(*get)(enum pmt_feature_id id),
> +						  void (*put)(struct pmt_feature_group *p)) { }
> +static inline void intel_aet_unregister_enumeration(void) { }
> +
>  #endif /* CONFIG_X86_CPU_RESCTRL */
>  
>  #endif /* _ASM_X86_RESCTRL_H */
> diff --git a/arch/x86/kernel/cpu/resctrl/intel_aet.c b/arch/x86/kernel/cpu/resctrl/intel_aet.c
> index cd8257c58f84..c26bd813e057 100644
> --- a/arch/x86/kernel/cpu/resctrl/intel_aet.c
> +++ b/arch/x86/kernel/cpu/resctrl/intel_aet.c
> @@ -12,17 +12,21 @@
>  #define pr_fmt(fmt)   "resctrl: " fmt
>  
>  #include <linux/bits.h>
> +#include <linux/cleanup.h>
>  #include <linux/compiler_types.h>
>  #include <linux/container_of.h>
>  #include <linux/cpumask.h>
>  #include <linux/err.h>
>  #include <linux/errno.h>
> +#include <linux/export.h>
>  #include <linux/gfp_types.h>
>  #include <linux/init.h>
>  #include <linux/intel_pmt_features.h>
>  #include <linux/intel_vsec.h>
>  #include <linux/io.h>
>  #include <linux/minmax.h>
> +#include <linux/mutex.h>
> +#include <linux/module.h>
>  #include <linux/printk.h>
>  #include <linux/rculist.h>
>  #include <linux/rcupdate.h>
> @@ -291,6 +295,10 @@ static enum pmt_feature_id lookup_pfid(const char *pfname)
>  	return FEATURE_INVALID;
>  }
>  
> +static struct module *pmt_module;
> +static struct pmt_feature_group *(*get_feature)(enum pmt_feature_id id);
> +static void (*put_feature)(struct pmt_feature_group *p);
> +
>  /*
>   * Request a copy of struct pmt_feature_group for each event group. If there is
>   * one, the returned structure has an array of telemetry_region structures,
> @@ -325,6 +333,32 @@ bool intel_aet_get_events(void)
>  	return ret;
>  }
>  
> +/*
> + * Defend against races between pmt_telemetry register/unregister and
> + * resctrl file system mount/unmount.
> + */

Please help developers understand what this mutex protects. I am not able to deduce
from above when this mutex needs to be taken.

> +static DEFINE_MUTEX(aet_register_lock);
> +
> +void intel_aet_register_enumeration(struct module *module,
> +				    struct pmt_feature_group *(*get)(enum pmt_feature_id id),
> +				    void (*put)(struct pmt_feature_group *p))
> +{
> +	guard(mutex)(&aet_register_lock);
> +	get_feature = get;
> +	put_feature = put;
> +	pmt_module = module;
> +}
> +EXPORT_SYMBOL_NS_GPL(intel_aet_register_enumeration, "INTEL_PMT");
> +
> +void intel_aet_unregister_enumeration(void)
> +{
> +	guard(mutex)(&aet_register_lock);
> +	pmt_module = NULL;
> +	get_feature = NULL;
> +	put_feature = NULL;
> +}
> +EXPORT_SYMBOL_NS_GPL(intel_aet_unregister_enumeration, "INTEL_PMT");
> +
>  void __exit intel_aet_exit(void)
>  {
>  	struct event_group **peg;

Reinette

Re: [PATCH v9 09/12] arm,x86/resctrl: Resolve INTEL_PMT_TELEMETRY symbols at runtime

[Reinette Chatre]

Hi Tony,

On 7/1/26 2:35 PM, Tony Luck wrote:
> resctrl is always built-in, but INTEL_PMT_TELEMETRY and INTEL_TPMI are
> logically independent and should be loadable modules.  Switch AET to use the
> function-pointer registration API instead of direct link-time references to
> PMT symbols.
> 
> Prepare for the file system to call resctrl_arch_pre_mount() on every mount
> by moving AET enumeration into resctrl_arch_pre_mount() and cleanup into
> resctrl_arch_unmount(). This allows the PMT module to be unloaded whenever
> the filesystem is not mounted.
> 
> Remove intel_aet_exit because all cleanup now happens in the unmount path.

intel_aet_exit -> intel_aet_exit()

At first glance this change does not seem symmetrical since it introduces
the new enumeration and cleanup calls but does not call the cleanup call that
appears as though it keeps a reference to the module. I think it will help to
highlight that the previous exit code was not called anyway and at this point
the module is still required to be built in so there is no actual reference taken.

> 
> Note that the Linux file system code does not serialize calls to
> fs_context_operations::get_tree(), so there may be arbitrarily many parallel
> calls if users invoke mount(2) multiple times.
> 
> Use "pmt_in_use" to detect nested mount attempts.
> 
> event_group::num_rmid may be reset (reduced) during enumeration. This is
> not worth resetting on unmount because the same reduction would occur on
> each subsequent mount.

If event_group::num_rmid gets a mention, should rdt_resource::resctrl_mon::num_rmid also?

> 
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> --
> v9:
> 	Drop broken atomic ops introduced in v8
> 	Check pmt_in_use in intel_aet_pre_mount() to detect nested
> 	mount flow.
> ---
>  include/linux/resctrl.h                 |  6 +++
>  arch/x86/kernel/cpu/resctrl/internal.h  | 23 ++++++++--
>  arch/x86/kernel/cpu/resctrl/core.c      | 25 +++++++++--
>  arch/x86/kernel/cpu/resctrl/intel_aet.c | 58 +++++++++++++++++++++----
>  drivers/resctrl/mpam_resctrl.c          |  4 ++
>  5 files changed, 101 insertions(+), 15 deletions(-)
> 
> diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
> index e1501ee72d28..4dc41dc99dc2 100644
> --- a/include/linux/resctrl.h
> +++ b/include/linux/resctrl.h
> @@ -564,6 +564,12 @@ void resctrl_offline_cpu(unsigned int cpu);
>   */
>  void resctrl_arch_pre_mount(void);
>  
> +/*
> + * Architecture hook called when mount fails, or on unmount.
> + * No locks are held.
> + */
> +void resctrl_arch_unmount(void);
> +
>  /**
>   * resctrl_arch_rmid_read() - Read the eventid counter corresponding to rmid
>   *			      for this resource and domain.
> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> index e3cfa0c10e92..a891017f69ec 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -234,15 +234,15 @@ void rdt_domain_reconfigure_cdp(struct rdt_resource *r);
>  void resctrl_arch_mbm_cntr_assign_set_one(struct rdt_resource *r);
>  
>  #ifdef CONFIG_X86_CPU_RESCTRL_INTEL_AET
> -bool intel_aet_get_events(void);
> -void __exit intel_aet_exit(void);
> +bool intel_aet_pre_mount(void);
> +void intel_aet_unmount(void);
>  int intel_aet_read_event(int domid, u32 rmid, void *arch_priv, u64 *val);
>  void intel_aet_mon_domain_setup(int cpu, int id, struct rdt_resource *r,
>  				struct list_head *add_pos);
>  bool intel_handle_aet_option(bool force_off, char *tok);
>  #else
> -static inline bool intel_aet_get_events(void) { return false; }
> -static inline void __exit intel_aet_exit(void) { }
> +static inline bool intel_aet_pre_mount(void) { return false; }
> +static inline void intel_aet_unmount(void) { }
>  static inline int intel_aet_read_event(int domid, u32 rmid, void *arch_priv, u64 *val)
>  {
>  	return -EINVAL;
> @@ -253,4 +253,19 @@ static inline void intel_aet_mon_domain_setup(int cpu, int id, struct rdt_resour
>  static inline bool intel_handle_aet_option(bool force_off, char *tok) { return false; }
>  #endif
>  
> +#ifdef CONFIG_INTEL_PMT_TELEMETRY_MODULE
> +static inline bool intel_aet_try_module_get(struct module *mod)
> +{
> +	return try_module_get(mod);
> +}
> +
> +static inline void intel_aet_module_put(struct module *mod)
> +{
> +	module_put(mod);
> +}
> +#else
> +static inline bool intel_aet_try_module_get(struct module *mod) { return true; }
> +static inline void intel_aet_module_put(struct module *mod) { }
> +#endif


Trying to look back to how the above ended up like this I find:
https://lore.kernel.org/lkml/SJ1PR11MB60838C247727AE7CC324BB25FC1A2@SJ1PR11MB6083.namprd11.prod.outlook.com/
that states:
	pmt_module is NULL if CONFIG_INTEL_PMT_TELEMETRY=y ... i.e. built-in to the kernel.

	In that case it obviously can't go away, and doesn't need module_get()/module_put().
	There's no special case for this. try_module_get() takes a fault on NULL dereference.
	...

Taking a closer look at try_module_get() I do not see it attempting to dereference
a NULL parameter but instead it returns true in this case which makes the above unnecessary?
What am I missing?

> +
>  #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 3169441a2d40..3ef620d8e0c8 100644
> --- a/arch/x86/kernel/cpu/resctrl/core.c
> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> @@ -16,10 +16,12 @@
>  
>  #define pr_fmt(fmt)	"resctrl: " fmt
>  
> +#include <linux/cleanup.h>
>  #include <linux/cpu.h>
>  #include <linux/slab.h>
>  #include <linux/err.h>
>  #include <linux/cpuhotplug.h>
> +#include <linux/mutex.h>
>  
>  #include <asm/cpu_device_id.h>
>  #include <asm/cpuid/api.h>
> @@ -798,7 +800,7 @@ void resctrl_arch_pre_mount(void)
>  	struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_PERF_PKG].r_resctrl;
>  	int cpu;
>  
> -	if (!intel_aet_get_events())
> +	if (!intel_aet_pre_mount())
>  		return;
>  
>  	/*
> @@ -814,6 +816,25 @@ void resctrl_arch_pre_mount(void)
>  	cpus_read_unlock();
>  }
>  
> +void resctrl_arch_unmount(void)
> +{
> +	struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_PERF_PKG].r_resctrl;
> +	int cpu;
> +
> +	intel_aet_unmount();
> +
> +	if (!r->mon_capable)
> +		return;

Could the mon_capable check be moved before intel_aet_unmount()?

> +
> +	cpus_read_lock();
> +	mutex_lock(&domain_list_lock);
> +	for_each_online_cpu(cpu)
> +		domain_remove_cpu_mon(cpu, r);
> +	r->mon_capable = false;
> +	mutex_unlock(&domain_list_lock);
> +	cpus_read_unlock();
> +}
> +
>  enum {
>  	RDT_FLAG_CMT,
>  	RDT_FLAG_MBM_TOTAL,
> @@ -1187,8 +1208,6 @@ 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
> index c26bd813e057..c2e27ce52248 100644
> --- a/arch/x86/kernel/cpu/resctrl/intel_aet.c
> +++ b/arch/x86/kernel/cpu/resctrl/intel_aet.c
> @@ -25,8 +25,8 @@
>  #include <linux/intel_vsec.h>
>  #include <linux/io.h>
>  #include <linux/minmax.h>
> -#include <linux/mutex.h>
>  #include <linux/module.h>
> +#include <linux/mutex.h>
>  #include <linux/printk.h>
>  #include <linux/rculist.h>
>  #include <linux/rcupdate.h>

This can be squashed with earlier patch.

> @@ -310,7 +310,7 @@ static void (*put_feature)(struct pmt_feature_group *p);
>   * struct pmt_feature_group to indicate that its events are successfully
>   * enabled.
>   */
> -bool intel_aet_get_events(void)
> +static bool aet_get_events(void)
>  {
>  	struct pmt_feature_group *p;
>  	enum pmt_feature_id pfid;
> @@ -319,14 +319,14 @@ bool intel_aet_get_events(void)
>  
>  	for_each_event_group(peg) {
>  		pfid = lookup_pfid((*peg)->pfname);
> -		p = intel_pmt_get_regions_by_feature(pfid);
> +		p = get_feature(pfid);
>  		if (IS_ERR_OR_NULL(p))
>  			continue;
>  		if (enable_events(*peg, p)) {
>  			(*peg)->pfg = p;
>  			ret = true;
>  		} else {
> -			intel_pmt_put_feature_group(p);
> +			put_feature(p);
>  		}
>  	}
>  
> @@ -359,16 +359,58 @@ void intel_aet_unregister_enumeration(void)
>  }
>  EXPORT_SYMBOL_NS_GPL(intel_aet_unregister_enumeration, "INTEL_PMT");
>  
> -void __exit intel_aet_exit(void)
> +/*
> + * Track whether pmt_telemetry enumeration succeeded during mount to
> + * detect nested mount attempts and for use during unmount.
> + */
> +static bool pmt_in_use;
> +
> +bool intel_aet_pre_mount(void)
> +{
> +	bool ret;
> +
> +	guard(mutex)(&aet_register_lock);
> +
> +	/* Nested mount attempt. File system code will return -EBUSY */
> +	if (pmt_in_use)
> +		return false;
> +
> +	if (!get_feature || !put_feature)
> +		return false;
> +
> +	if (!intel_aet_try_module_get(pmt_module))
> +		return false;
> +
> +	ret = aet_get_events();
> +
> +	if (!ret)
> +		intel_aet_module_put(pmt_module);
> +	else
> +		pmt_in_use = true;
> +
> +	return ret;
> +}
> +
> +void intel_aet_unmount(void)
>  {
>  	struct event_group **peg;
>  
> +	guard(mutex)(&aet_register_lock);
> +	if (!pmt_in_use)
> +		return;
> +
>  	for_each_event_group(peg) {
> -		if ((*peg)->pfg) {
> -			intel_pmt_put_feature_group((*peg)->pfg);
> -			(*peg)->pfg = NULL;
> +		struct event_group *e = *peg;
> +
> +		if (e->pfg) {
> +			for (int j = 0; j < e->num_events; j++)
> +				resctrl_disable_mon_event(e->evts[j].id);
> +			put_feature(e->pfg);
> +			e->pfg = NULL;
>  		}
>  	}
> +	intel_aet_module_put(pmt_module);
> +	pmt_in_use = false;
>  }
>  
>  #define DATA_VALID	BIT_ULL(63)
> diff --git a/drivers/resctrl/mpam_resctrl.c b/drivers/resctrl/mpam_resctrl.c
> index 7079870ca894..5859cc0f5e37 100644
> --- a/drivers/resctrl/mpam_resctrl.c
> +++ b/drivers/resctrl/mpam_resctrl.c
> @@ -162,6 +162,10 @@ void resctrl_arch_pre_mount(void)
>  {
>  }
>  
> +void resctrl_arch_unmount(void)
> +{
> +}
> +
>  bool resctrl_arch_get_cdp_enabled(enum resctrl_res_level rid)
>  {
>  	return mpam_resctrl_controls[rid].cdp_enabled;

Reinette

Re: [PATCH v9 10/12] fs/resctrl: Call architecture hooks for every mount/unmount

[Reinette Chatre]

Hi Tony,

On 7/1/26 2:35 PM, Tony Luck wrote:
> The intel_pmt module should only be pinned while the resctrl file system
> is mounted.
> 
> Add hooks for every mount/unmount of the resctrl file system so that
> architecture code can increment the intel_pmt module reference count on
> mount and decrement on unmount.
> 
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> ---

> @@ -3168,6 +3167,8 @@ static void resctrl_unmount(void)
>  	resctrl_mounted = false;
>  	mutex_unlock(&rdtgroup_mutex);
>  	cpus_read_unlock();
> +
> +	resctrl_arch_unmount();
>  }
>  
>  static int rdt_get_tree(struct fs_context *fc)
> @@ -3179,7 +3180,7 @@ static int rdt_get_tree(struct fs_context *fc)
>  	struct rdt_resource *r;
>  	int ret;
>  
> -	DO_ONCE_SLEEPABLE(resctrl_arch_pre_mount);
> +	resctrl_arch_pre_mount();
>  
>  	cpus_read_lock();
>  	mutex_lock(&rdtgroup_mutex);
> @@ -3318,6 +3319,9 @@ static int rdt_get_tree(struct fs_context *fc)
>  out:
>  	mutex_unlock(&rdtgroup_mutex);
>  	cpus_read_unlock();
> +
> +	resctrl_arch_unmount();
> +
>  	return ret;
>  }
>  

rdt_get_tree() jumps to "out" if resctrl is already mounted - in this scenario
resctrl fs is mounted and the telemetry events exposed to user space. Above would
cause telemetry to be yanked from underneath an existing mount?

Reinette

Re: [PATCH v9 11/12] x86/resctrl: Simplify Kconfig options for resctrl

[Reinette Chatre]

Hi Tony,

On 7/1/26 2:35 PM, Tony Luck wrote:
> Linus Torvalds complained[1] about Kconfig complexity making it too
> hard for "random people to build their own kernels".
> 
> CONFIG_X86_CPU_RESCTRL_INTEL_AET has been causing problems since it was
> first added as it required other config options to be set to "built-in".
> 
> AET now resolves PMT symbols at runtime via the registration API,
> so INTEL_PMT_TELEMETRY no longer needs to be built-in. This means
> that AET can be included unconditionally as part of X86_CPU_RESCTRL.
> 
> CONFIG_X86_CPU_RESCTRL now depends on X86_64 because reading telemetry
> data needs readq().

Above reads as though it is a new dependency as opposed to an existing dependency
that moves one level up. How about something like:
	Move the X86_64 dependency of the removed AET config up to CONFIG_X86_CPU_RESCTRL.

Reinette

Re: [PATCH v9 01/12] platform/x86/intel/{pmt,vsec}: Prevent unbind via sysfs

[Luck, Tony]

On Wed, Jul 08, 2026 at 03:45:03PM -0700, Reinette Chatre wrote:
> Hi Tony,
> 
> On 7/1/26 2:35 PM, Tony Luck wrote:
> > The resctrl AET code uses intel_pmt_get_regions_by_feature() (exported by the
> > pmt_telemetry module) to obtain pmt_feature_group instances that are populated
> > using data from the pmt_discovery module. The returned telemetry_region::addr
> > values are MMIO mappings created in the telemetry driver's probe path,
> > and resctrl reads from them while a mount is active.
> > 
> > However, a user can unbind the intel_pmt driver or its parent intel_vsec
> > from their devices via sysfs "unbind" attributes. Unbinding either of these
> > drivers would tear down the MMIO mappings, leaving resctrl with stale pointers
> > and causing a fault on the next access.
> > 
> > It is safe for the discovery driver to be unbound. It does all its work at
> > initial probe time and saves a copy of the features and RMID count.
> > 
> > Set device_driver::suppress_bind_attrs in each driver so that the "bind"
> > and "unbind" sysfs attributes are not exposed and the user cannot detach
> > the driver from its devices while resctrl (or any other in-kernel consumer)
> > may be using the resources.
> 
> Unbinding via sysfs does not seem to be the only way in which resources that
> resctrl depend on could disappear.  The "/sys/bus/pci/devices/.../remove"
> interface looks to be another way? I am not aware of a way to prevent this since
> this represents how devices could just be yanked from a system. I wonder if
> resctrl should instead protect itself against telemetry resources disappearing
> after mount with another helper that telemetry driver can call when the device
> is being removed or the module is being unloaded. When telemetry driver informs AET
> that resources are going away after resctrl is mounted then resctrl/AET can return
> EIO on event read attempts? resctrl could possibly recover when the device/module
> re-appears but if that is complicated then user space could just be forced to remount
> resctrl.
> 
> With this there is no need to prevent unbind here since it creates a false
> guarantee to resctrl.

This sounds like a complexity road that I'd prefer not to travel. The AET
enumeration is handled with a contract between resctrl and the pmt_telemetry
module. But the mapping of MMIO space is handled by the parent intel_vsec
driver. So to get notifications of impending unmap of MMIO space the intel_vsec
would somehow need to be aware that pmt_telemetry had handed out some pointers.

The "remove" interface might push this trail up additional levels (to the
PCIe bus driver level?).

All this complexity to avoid crashing when system operators poke at sysfs
files without understanding dependencies.

I propose a simpler solution. Just have the AET code defensively read from
the MMIO registers with a new function that turns any page fault due to the
MMIO space being unmapped into an error return.

I'm terrible as inline ASM code. So I had Gemini write this for me. I tested it
by removing the code that disables unbind, and then unbinding the intel_vsec
device for socket 0 on my system. All the AET files for socket 0 reported
"unavailable", the socket 1 files continued to work normally.

------------------------------
/**
 * readq_safe - Safely read a 64-bit MMIO register, catching page faults.
 * @ptr: The MMIO address to read from.
 * @val: Pointer to where the fetched value should be stored.
 *
 * Returns 0 on success, or -EFAULT if a page fault occurred.
 */
static int readq_safe(const volatile u64 __iomem *ptr, u64 *val)
{
        int err = 0;
        u64 ret_val;

        asm volatile (
                "1: movq %[addr], %[val]\n"
                _ASM_EXTABLE(1b, 2f)
                "   jmp 3f\n"
                "2: mov %[efault], %[err]\n"
                "   xorq %[val], %[val]\n"
                "3:\n"
                : [val] "=r" (ret_val), [err] "+r" (err)
                : [addr] "m" (*ptr), [efault] "i" (-EFAULT)
                : "memory"
        );

        *val = err ? 0 : ret_val;

        return err;
}
------------------------------
> 
> > 
> > Fixes: 1fb2daa60de6 ("x86/resctrl: Discover hardware telemetry events")
> > Assisted-by: Claude:Opus-4.7
> > Signed-off-by: Tony Luck <tony.luck@intel.com>
> > --
> 
> Please note the separator typo ("--" -> "---") above that exists in every patch in
> this series that has a changelog. Please check all patches for this, I only highlight this
> instance.

Oops. I was using those so keep my change list intact when updating,
shuffling, re-basing patches. I forgot to tweak the "--" back to "---"
when posting.
> 
> Reinette
> 
-Tony

Re: [PATCH v9 01/12] platform/x86/intel/{pmt,vsec}: Prevent unbind via sysfs

[Reinette Chatre]

Hi Tony,

On 7/9/26 10:41 AM, Luck, Tony wrote:
> On Wed, Jul 08, 2026 at 03:45:03PM -0700, Reinette Chatre wrote:
>> Hi Tony,
>>
>> On 7/1/26 2:35 PM, Tony Luck wrote:
>>> The resctrl AET code uses intel_pmt_get_regions_by_feature() (exported by the
>>> pmt_telemetry module) to obtain pmt_feature_group instances that are populated
>>> using data from the pmt_discovery module. The returned telemetry_region::addr
>>> values are MMIO mappings created in the telemetry driver's probe path,
>>> and resctrl reads from them while a mount is active.
>>>
>>> However, a user can unbind the intel_pmt driver or its parent intel_vsec
>>> from their devices via sysfs "unbind" attributes. Unbinding either of these
>>> drivers would tear down the MMIO mappings, leaving resctrl with stale pointers
>>> and causing a fault on the next access.
>>>
>>> It is safe for the discovery driver to be unbound. It does all its work at
>>> initial probe time and saves a copy of the features and RMID count.
>>>
>>> Set device_driver::suppress_bind_attrs in each driver so that the "bind"
>>> and "unbind" sysfs attributes are not exposed and the user cannot detach
>>> the driver from its devices while resctrl (or any other in-kernel consumer)
>>> may be using the resources.
>>
>> Unbinding via sysfs does not seem to be the only way in which resources that
>> resctrl depend on could disappear.  The "/sys/bus/pci/devices/.../remove"
>> interface looks to be another way? I am not aware of a way to prevent this since
>> this represents how devices could just be yanked from a system. I wonder if
>> resctrl should instead protect itself against telemetry resources disappearing
>> after mount with another helper that telemetry driver can call when the device
>> is being removed or the module is being unloaded. When telemetry driver informs AET
>> that resources are going away after resctrl is mounted then resctrl/AET can return
>> EIO on event read attempts? resctrl could possibly recover when the device/module
>> re-appears but if that is complicated then user space could just be forced to remount
>> resctrl.
>>
>> With this there is no need to prevent unbind here since it creates a false
>> guarantee to resctrl.
> 
> This sounds like a complexity road that I'd prefer not to travel. The AET
> enumeration is handled with a contract between resctrl and the pmt_telemetry
> module. But the mapping of MMIO space is handled by the parent intel_vsec
> driver. So to get notifications of impending unmap of MMIO space the intel_vsec
> would somehow need to be aware that pmt_telemetry had handed out some pointers.
> 
> The "remove" interface might push this trail up additional levels (to the
> PCIe bus driver level?).
> 
> All this complexity to avoid crashing when system operators poke at sysfs
> files without understanding dependencies.
> 
> I propose a simpler solution. Just have the AET code defensively read from
> the MMIO registers with a new function that turns any page fault due to the
> MMIO space being unmapped into an error return.

A simpler solution would be ideal. It is not clear to me that there would be
a page fault though. I looked around at what other drivers do and it seems that
readq() may return all 1s in this scenario? Consider for example, how
IXGBE_FAILED_READ_REG, PCI_ERROR_RESPONSE (note the comments above its definition),
and PCI_POSSIBLE_ERROR are used. Some drivers just straight compare against -1 like
the "if (readl(dev->bar + NVME_REG_CSTS) == -1)" in nvme_pci_enable().
There is also this related snippet in Documentation/PCI/pci.rst: "Most x86 platforms
will allow MMIO reads to master abort (a.k.a. "Soft Fail") and return garbage
(e.g. ~0)." Handling ~0 may be even simpler?

Reinette

Re: [PATCH v9 01/12] platform/x86/intel/{pmt,vsec}: Prevent unbind via sysfs

[Luck, Tony]

On Thu, Jul 09, 2026 at 01:48:12PM -0700, Reinette Chatre wrote:
> Hi Tony,
> 
> On 7/9/26 10:41 AM, Luck, Tony wrote:
> > On Wed, Jul 08, 2026 at 03:45:03PM -0700, Reinette Chatre wrote:
> >> Hi Tony,
> >>
> >> On 7/1/26 2:35 PM, Tony Luck wrote:
> >>> The resctrl AET code uses intel_pmt_get_regions_by_feature() (exported by the
> >>> pmt_telemetry module) to obtain pmt_feature_group instances that are populated
> >>> using data from the pmt_discovery module. The returned telemetry_region::addr
> >>> values are MMIO mappings created in the telemetry driver's probe path,
> >>> and resctrl reads from them while a mount is active.
> >>>
> >>> However, a user can unbind the intel_pmt driver or its parent intel_vsec
> >>> from their devices via sysfs "unbind" attributes. Unbinding either of these
> >>> drivers would tear down the MMIO mappings, leaving resctrl with stale pointers
> >>> and causing a fault on the next access.
> >>>
> >>> It is safe for the discovery driver to be unbound. It does all its work at
> >>> initial probe time and saves a copy of the features and RMID count.
> >>>
> >>> Set device_driver::suppress_bind_attrs in each driver so that the "bind"
> >>> and "unbind" sysfs attributes are not exposed and the user cannot detach
> >>> the driver from its devices while resctrl (or any other in-kernel consumer)
> >>> may be using the resources.
> >>
> >> Unbinding via sysfs does not seem to be the only way in which resources that
> >> resctrl depend on could disappear.  The "/sys/bus/pci/devices/.../remove"
> >> interface looks to be another way? I am not aware of a way to prevent this since
> >> this represents how devices could just be yanked from a system. I wonder if
> >> resctrl should instead protect itself against telemetry resources disappearing
> >> after mount with another helper that telemetry driver can call when the device
> >> is being removed or the module is being unloaded. When telemetry driver informs AET
> >> that resources are going away after resctrl is mounted then resctrl/AET can return
> >> EIO on event read attempts? resctrl could possibly recover when the device/module
> >> re-appears but if that is complicated then user space could just be forced to remount
> >> resctrl.
> >>
> >> With this there is no need to prevent unbind here since it creates a false
> >> guarantee to resctrl.
> > 
> > This sounds like a complexity road that I'd prefer not to travel. The AET
> > enumeration is handled with a contract between resctrl and the pmt_telemetry
> > module. But the mapping of MMIO space is handled by the parent intel_vsec
> > driver. So to get notifications of impending unmap of MMIO space the intel_vsec
> > would somehow need to be aware that pmt_telemetry had handed out some pointers.
> > 
> > The "remove" interface might push this trail up additional levels (to the
> > PCIe bus driver level?).
> > 
> > All this complexity to avoid crashing when system operators poke at sysfs
> > files without understanding dependencies.
> > 
> > I propose a simpler solution. Just have the AET code defensively read from
> > the MMIO registers with a new function that turns any page fault due to the
> > MMIO space being unmapped into an error return.
> 
> A simpler solution would be ideal. It is not clear to me that there would be
> a page fault though. I looked around at what other drivers do and it seems that
> readq() may return all 1s in this scenario? Consider for example, how
> IXGBE_FAILED_READ_REG, PCI_ERROR_RESPONSE (note the comments above its definition),
> and PCI_POSSIBLE_ERROR are used. Some drivers just straight compare against -1 like
> the "if (readl(dev->bar + NVME_REG_CSTS) == -1)" in nvme_pci_enable().
> There is also this related snippet in Documentation/PCI/pci.rst: "Most x86 platforms
> will allow MMIO reads to master abort (a.k.a. "Soft Fail") and return garbage
> (e.g. ~0)." Handling ~0 may be even simpler?

The return of all 1s happens in the case that the underlying h/w has gone,
so the PCIe request to read the data is aborted. That's not what happens
here. When Linux uses ioremap() to set up a virtual address to access an
MMIO range the virtual address is allocated from the same virtual space
that vmalloc() uses. When that range is iounmap()'d the page tables are
invalidated. So any subsequent access gets a page fault (confirmed by my test).

But ... I ran my patch past an AI review and it pointed out the flaw.  The old
virtual address may be reallocated by a subsequent ioremap() or vmalloc()
call, at which point AET would begin accessing whatever memory/device was
now mapped to the virtual address.

> 
> Reinette

-Tony

Re: [PATCH v9 01/12] platform/x86/intel/{pmt,vsec}: Prevent unbind via sysfs

[Luck, Tony]

On Thu, Jul 09, 2026 at 02:12:58PM -0700, Luck, Tony wrote:
> On Thu, Jul 09, 2026 at 01:48:12PM -0700, Reinette Chatre wrote:
> > Hi Tony,
> > 
> > On 7/9/26 10:41 AM, Luck, Tony wrote:
> > > On Wed, Jul 08, 2026 at 03:45:03PM -0700, Reinette Chatre wrote:
> > >> Hi Tony,
> > >>
> > >> On 7/1/26 2:35 PM, Tony Luck wrote:
...
> > 
> > A simpler solution would be ideal. It is not clear to me that there would be
> > a page fault though. I looked around at what other drivers do and it seems that
> > readq() may return all 1s in this scenario? Consider for example, how
> > IXGBE_FAILED_READ_REG, PCI_ERROR_RESPONSE (note the comments above its definition),
> > and PCI_POSSIBLE_ERROR are used. Some drivers just straight compare against -1 like
> > the "if (readl(dev->bar + NVME_REG_CSTS) == -1)" in nvme_pci_enable().
> > There is also this related snippet in Documentation/PCI/pci.rst: "Most x86 platforms
> > will allow MMIO reads to master abort (a.k.a. "Soft Fail") and return garbage
> > (e.g. ~0)." Handling ~0 may be even simpler?
> 
> The return of all 1s happens in the case that the underlying h/w has gone,
> so the PCIe request to read the data is aborted. That's not what happens
> here. When Linux uses ioremap() to set up a virtual address to access an
> MMIO range the virtual address is allocated from the same virtual space
> that vmalloc() uses. When that range is iounmap()'d the page tables are
> invalidated. So any subsequent access gets a page fault (confirmed by my test).
> 
> But ... I ran my patch past an AI review and it pointed out the flaw.  The old
> virtual address may be reallocated by a subsequent ioremap() or vmalloc()
> call, at which point AET would begin accessing whatever memory/device was
> now mapped to the virtual address.

New scheme. Change intel_pmt_get_regions_by_feature() enumeration to
pass the physical address of the MMIO region instead of the virtual
address mapping created by intel_pmt_dev_register().

Then intel_aet.c can use ioremap() to create an independent virtual
mapping to the AET register region in MMIO space.

Then it doesn't matter if some unbind or remove operation tears down
the virtual mapping. Resctrl can keep accessing the MMIO registers.

The MMIO space is "on package", so can't be physically removed in a
hot plug operation. The PCIe device could be disabled, but that would
only result in the 0xfffffffffffffff return value when trying to read
the registers.

There's a flaw in every approach. The problem in this one is if Linux
reprograms the PCIe device BAR registers to move to a different physical
address, resctrl would be left using mappings to a stale physical
address. I don't know how to judge if this is a real problem on
production servers.
> 
> > 
> > Reinette
> 
> -Tony

-Tony

Re: [PATCH v9 01/12] platform/x86/intel/{pmt,vsec}: Prevent unbind via sysfs

[Luck, Tony]

On Fri, Jul 10, 2026 at 10:01:34AM -0700, Luck, Tony wrote:
> On Thu, Jul 09, 2026 at 02:12:58PM -0700, Luck, Tony wrote:
> > On Thu, Jul 09, 2026 at 01:48:12PM -0700, Reinette Chatre wrote:
> > > Hi Tony,
> > > 
> > > On 7/9/26 10:41 AM, Luck, Tony wrote:
> > > > On Wed, Jul 08, 2026 at 03:45:03PM -0700, Reinette Chatre wrote:
> > > >> Hi Tony,
> > > >>
> > > >> On 7/1/26 2:35 PM, Tony Luck wrote:
> ...
> > > 
> > > A simpler solution would be ideal. It is not clear to me that there would be
> > > a page fault though. I looked around at what other drivers do and it seems that
> > > readq() may return all 1s in this scenario? Consider for example, how
> > > IXGBE_FAILED_READ_REG, PCI_ERROR_RESPONSE (note the comments above its definition),
> > > and PCI_POSSIBLE_ERROR are used. Some drivers just straight compare against -1 like
> > > the "if (readl(dev->bar + NVME_REG_CSTS) == -1)" in nvme_pci_enable().
> > > There is also this related snippet in Documentation/PCI/pci.rst: "Most x86 platforms
> > > will allow MMIO reads to master abort (a.k.a. "Soft Fail") and return garbage
> > > (e.g. ~0)." Handling ~0 may be even simpler?
> > 
> > The return of all 1s happens in the case that the underlying h/w has gone,
> > so the PCIe request to read the data is aborted. That's not what happens
> > here. When Linux uses ioremap() to set up a virtual address to access an
> > MMIO range the virtual address is allocated from the same virtual space
> > that vmalloc() uses. When that range is iounmap()'d the page tables are
> > invalidated. So any subsequent access gets a page fault (confirmed by my test).
> > 
> > But ... I ran my patch past an AI review and it pointed out the flaw.  The old
> > virtual address may be reallocated by a subsequent ioremap() or vmalloc()
> > call, at which point AET would begin accessing whatever memory/device was
> > now mapped to the virtual address.
> 
> New scheme. Change intel_pmt_get_regions_by_feature() enumeration to
> pass the physical address of the MMIO region instead of the virtual
> address mapping created by intel_pmt_dev_register().
> 
> Then intel_aet.c can use ioremap() to create an independent virtual
> mapping to the AET register region in MMIO space.
> 
> Then it doesn't matter if some unbind or remove operation tears down
> the virtual mapping. Resctrl can keep accessing the MMIO registers.
> 
> The MMIO space is "on package", so can't be physically removed in a
> hot plug operation. The PCIe device could be disabled, but that would
> only result in the 0xfffffffffffffff return value when trying to read
> the registers.
> 
> There's a flaw in every approach. The problem in this one is if Linux
> reprograms the PCIe device BAR registers to move to a different physical
> address, resctrl would be left using mappings to a stale physical
> address. I don't know how to judge if this is a real problem on
> production servers.

Digging back at my earlier assertion about complexity of handling
unbind from the parent driver of pmt_telemetry, some experimentation
showed that when unbind happens at that level, cleanup includes calling
pmt_telem_remove().

So I plan to add a new exported function from the AET code:

	intel_aet_pmt_notify_remove();

The pmt_telemetry driver will call this to let AET know that all the
MMIO virtual pointers are stale. When this happens reading any of the
AET event files will return -EINVAL (and the user will see "unavailable").

Initial PoC code for this seems to work. I'll clean it up and drop the
patches that set .suppress_bind_attrs = true;

> > 
> > > 
> > > Reinette
> > 
> > -Tony
> 
> -Tony

-Tony

Re: [PATCH v9 03/12] x86/resctrl: Honor rdt=perf option to force enable AET perf events

[Luck, Tony]

On Wed, Jul 08, 2026 at 03:46:45PM -0700, Reinette Chatre wrote:
> Hi Tony,
> 
> On 7/1/26 2:35 PM, Tony Luck wrote:
> > The kernel command line option "rdt=" is used to force enable or disable
> > individual resctrl features.
> > 
> > Linux only enumerates AET (Application Energy Telemetry) once on first
> > mount. But that is going to change to enumerate on each mount to allow the
> > pmt_telemetry driver to be configured as a module, and allow unloading when
> > not in use.
> > 
> > The following scenario will be a problem:
> > 
> > 1) User mounts the resctrl file system.  all_regions_have_sufficient_rmid()
> > notes that a perf telemetry region supports fewer RMIDs than expected for
> > the event_group and sets e->force_off = true. But the user had specified
> > "rdt=perf" on the kernel command line so perf events are enabled for
> > this first mount.
> > 
> > 2) Resctrl file system is unmounted and later remounted.
> > 
> > 3) During this new mount cycle enable_events() sees that e->force_off is
> > set and stops enumeration for this event group. So on this second, and
> > all subsequent, mounts perf events are not enabled.
> > 
> > Fix by checking the state of e->force_on at the start of enable_events().
> 
> Above just writes in words what the patch does. Please reduce repeating code
> segments in the changelog and replace with higher level descriptions to help understand
> "why". 
> I think a more detailed context will help with this. Consider, for example,
> something like below (a suggestion to start, please do not copy&paste):
> 	resctrl can disable ("force_off") a feature supported by the platform if,       
> 	for example, there is an erratum related to the feature or, in the case of      
> 	AET (Application Energy Telemetry), when the event group has insufficient       
> 	RMIDs. The kernel command line option "rdt=" can be used to override            
> 	("force_on") when a feature is disabled in such case and also let the user      
> 	disable ("force_off") individual supported resctrl features if they are not     
> 	needed.                                
> 
> 	Linux enumerates AET once on first mount and skips this enumeration if the
> 	associated event group is supported but disabled. Since AET is only enumerated
> 	once, this check is guaranteed to only consider disabling via the rdt= kernel
> 	parameter. However, when transitioning to enumerate AET on every mount, it is
> 	no longer correct to skip enumeration simply because an event group is marked
> 	as disabled. It is then possible for resctrl to have force-disabled the event
> 	group during an earlier enumeration due to insufficient RMIDs, which would
> 	improperly cause the kernel to ignore a user's explicit rdt= override on
> 	subsequent mounts.
> 
> 	Ensure the user's command line choices take precedence over system-level limits.

This looks good. I'll dig through it to see if I can improve any parts.

> > 
> > Signed-off-by: Tony Luck <tony.luck@intel.com>
> > ---
> >  arch/x86/kernel/cpu/resctrl/intel_aet.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/x86/kernel/cpu/resctrl/intel_aet.c b/arch/x86/kernel/cpu/resctrl/intel_aet.c
> > index c22c3cf5167d..cd8257c58f84 100644
> > --- a/arch/x86/kernel/cpu/resctrl/intel_aet.c
> > +++ b/arch/x86/kernel/cpu/resctrl/intel_aet.c
> > @@ -60,8 +60,8 @@ struct pmt_event {
> >   *			data for all telemetry regions of type @pfname.
> >   *			Valid if the system supports the event group,
> >   *			NULL otherwise.
> > - * @force_off:		True when "rdt" command line or architecture code disables
> > - *			this event group due to insufficient RMIDs.
> > + * @force_off:		True when "rdt" command line disables this event group
> > + *			to avoid system limitations due to insufficient RMIDs.
> >   * @force_on:		True when "rdt" command line overrides disable of this
> >   *			event group.
> >   * @guid:		Unique number per XML description file.
> 
> This is unexpected. The original comment is more accurate and I understood from 
> https://lore.kernel.org/lkml/ainCnu4xmHbgh91r@agluck-desk3/ that you were not going to
> make changes here.

Oops. I'll drop this for next time.

> 
> > @@ -228,7 +228,7 @@ static bool enable_events(struct event_group *e, struct pmt_feature_group *p)
> >  	struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_PERF_PKG].r_resctrl;
> >  	int skipped_events = 0;
> >  
> > -	if (e->force_off)
> > +	if (e->force_off && !e->force_on)
> >  		return false;
> >  
> >  	if (!group_has_usable_regions(e, p))
> 
> Reinette

-Tony

Re: [PATCH v9 05/12] x86/resctrl: Drop global 'rdt_mon_capable' flag

[Luck, Tony]

On Wed, Jul 08, 2026 at 03:47:51PM -0700, Reinette Chatre wrote:
> Hi Tony,
> 
> On 7/1/26 2:35 PM, Tony Luck wrote:
> > File system code calls resctrl_arch_mon_capable() to determine if the
> > system supports any monitoring features.
> > 
> > x86 architecture code sets a global flag to support implementation of
> > resctrl_arch_mon_capable() but the upcoming change to enumerate AET
> > (Application Energy Telemetry) features on each mount introduces a new
> > corner case.
> > 
> > Specifically when AET is the only monitoring feature, loading and
> > unloading the pmt_telemetry module between resctrl mounts may result
> > in monitoring support enabled on some mounts, but not on others.
> > 
> > Replace the global flag with a scan of resources to check if any are
> > marked mon_capable.
> > 
> > Signed-off-by: Tony Luck <tony.luck@intel.com>
> > ---
> 
> ...
> 
> > diff --git a/arch/x86/include/asm/resctrl.h b/arch/x86/include/asm/resctrl.h
> > index 575f8408a9e7..a9f481b7a8ed 100644
> > --- a/arch/x86/include/asm/resctrl.h
> > +++ b/arch/x86/include/asm/resctrl.h
> 
> ...
> 
> > @@ -66,10 +65,7 @@ static inline void resctrl_arch_disable_alloc(void)
> >  	static_branch_dec_cpuslocked(&rdt_enable_key);
> >  }
> >  
> > -static inline bool resctrl_arch_mon_capable(void)
> > -{
> > -	return rdt_mon_capable;
> > -}
> > +bool resctrl_arch_mon_capable(void);
> >  
> >  static inline void resctrl_arch_enable_mon(void)
> >  {
> > diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> > index 9b9495174041..60d50ac79e7b 100644
> > --- a/arch/x86/kernel/cpu/resctrl/core.c
> > +++ b/arch/x86/kernel/cpu/resctrl/core.c
> > @@ -51,6 +51,20 @@ DEFINE_PER_CPU(struct resctrl_pqr_state, pqr_state);
> >   */
> >  bool rdt_alloc_capable;
> >  
> > +/*
> > + * Need to re-evaluate on each mount whether any mon_capable resources
> > + * are enabled.
> > + */
> 
> This series introduces a significant change in behavior of this call and I
> find that having the declaration in asm header and this significant comment in
> arch code to essentially bury very important information on this
> call's usage. Could the declaration be elevated to include/linux/resctrl.h
> where its function comments can provide the details on how resctrl fs
> uses this call and what its expectations from the architecture are? 
> Basically this series causes resctrl_arch_mon_capable() to possibly return
> different value depending on when it is called so there needs to be
> a contract between resctrl fs and architecture on when resctrl fs can safely
> call this and expect consistent results.

Ok. I'll move declaration to <linux/resctrl.h> and add comments on usage
and expectations.

> > +bool resctrl_arch_mon_capable(void)
> > +{
> > +	struct rdt_resource *r;
> > +
> > +	for_each_mon_capable_rdt_resource(r)
> > +		return true;
> > +
> > +	return false;
> > +}
> > +
> 
> Reinette

-Tony

Re: [PATCH v9 06/12] arm,x86,fs/resctrl: Handle change in number of RMIDs on each mount

[Luck, Tony]

On Wed, Jul 08, 2026 at 03:50:45PM -0700, Reinette Chatre wrote:
> Hi Tony,
> 
> On 7/1/26 2:35 PM, Tony Luck wrote:
> > Application Energy Telemetry (AET) event enumeration takes place
> > asynchronously. Linux builds the pmt_telemetry module into the kernel to
> > kick off enumeration early enough that it completes before first mount of
> > the resctrl file system.
> > 
> > Allowing pmt_telemetry to be a loadable module means that it is possible
> > for different numbers of RMIDs to be supported on each mount, depending
> > on whether pmt_telemetry module is loaded.
> > 
> > For simplicity, calculate the maximum possible number of RMIDs and use
> > that value to allocate the rmid_ptrs[] array just once. Also use this
> > maximum RMID value when allocating rdt_l3_mon_domain::rmid_busy_llc
> > bitmap and rdt_l3_mon_domain::mbm_states.
> 
> This does not sound right. Why use the maximum RMID for L3 monitoring state?
> It can be guaranteed that L3 monitoring state is only accessed when that
> monitoring is enabled and when it is enabled it can be guaranteed to never
> use more RMID than what L3 itself supports. Why would it ever be required
> to allocate more than that? Could this not instead be limited to
> rdt_resource::resctrl_mon::num_rmid? From what I can tell such transition
> will make it explicit and consistent (since arch code already allocates
> this state based on this) how the L3 monitoring state is sized.

L3 monitoring on X86 happens to provide the overall maximum possible RMID
value (any larger value with #GP fault when written to IA32_PQR_ASSOC MSR).

AET could potentially claim to support a larger number (and this could
happen on low core count SKUs where the number of RMIDs supported scales
down with the number of cores).

resctrl_arch_system_max_rmid_idx() needs better comments, and
implementation. On x86 even if the L3 monitor functions have been
disabled, the limit is still bounded by CPUID. Not by what might
be found in the AET->mon.num_rmid.

Though perhaps intel_aet.c should look at CPUID to avoid believing any
larger numbers seen in AET enumeration.

> > 
> > The limbo code must deal with changes in the number of RMIDs from one
> > mount to the next because some RMIDs may still be "busy" when the file
> > system is unmounted, but be above resctrl_arch_system_num_rmid_idx()
> > for the remount. In this case RMIDs that can be released are not put
> > onto the rmid_free_lru list.
> > 
> > Signed-off-by: Tony Luck <tony.luck@intel.com>
> > ---
> 
> ...
> 
> > diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> > index 60d50ac79e7b..3169441a2d40 100644
> > --- a/arch/x86/kernel/cpu/resctrl/core.c
> > +++ b/arch/x86/kernel/cpu/resctrl/core.c
> > @@ -144,6 +144,20 @@ u32 resctrl_arch_system_num_rmid_idx(void)
> >  	return num_rmids == U32_MAX ? 0 : num_rmids;
> >  }
> >  
> > +/**
> > + * resctrl_arch_system_max_rmid_idx - Largest possible number of RMIDs
> > + *
> > + * Return: If L3 monitoring is supported, largest possible comes from L3 based
> > + * on CPUID(0xf,0x0).EBX (scaled down on Sub-NUMA Cluster systems). Otherwise
> > + * maximum from any other mon_capable resources.
> 
> Above reads "Otherwise maximum from any other ..." but the code is actually
> "Otherwise minimum from any other ..." which contradicts the "max" intention of
> this function?
> 
> > + */
> > +u32 resctrl_arch_system_max_rmid_idx(void)
> > +{
> > +	struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_L3].r_resctrl;
> > +
> > +	return r->mon_capable ? r->mon.num_rmid : resctrl_arch_system_num_rmid_idx();
> 
> This smells like a function that pretends to be generic but created to provide
> correct results on specific x86 hardware. 
> If this needs to provide max RMID why not cycle through all monitoring resources
> and determine the max RMID, why special case L3?
> 
> ...
> 
> > @@ -961,8 +975,8 @@ void mbm_setup_overflow_handler(struct rdt_l3_mon_domain *dom, unsigned long del
> >  
> >  int setup_rmid_lru_list(void)
> >  {
> > +	u32 max_idx_limit, min_idx_limit;
> >  	struct rmid_entry *entry = NULL;
> > -	u32 idx_limit;
> >  	u32 idx;
> >  	int i;
> >  
> > @@ -970,27 +984,29 @@ int setup_rmid_lru_list(void)
> >  		return 0;
> >  
> >  	/*
> > -	 * Called on every mount, but the number of RMIDs cannot change
> > -	 * after the first mount, so keep using the same set of rmid_ptrs[]
> > -	 * until resctrl_exit(). Note that the limbo handler continues to
> > -	 * access rmid_ptrs[] after resctrl is unmounted.
> > +	 * Allocate the largest number of RMIDs that this system will ever
> > +	 * need. These cannot be freed until resctrl_exit() because the limbo
> > +	 * handler continues to access rmid_ptrs[] after resctrl is unmounted.
> >  	 */
> > -	if (rmid_ptrs)
> > -		return 0;
> > -
> > -	idx_limit = resctrl_arch_system_num_rmid_idx();
> > -	rmid_ptrs = kzalloc_objs(struct rmid_entry, idx_limit);
> > -	if (!rmid_ptrs)
> > -		return -ENOMEM;
> > +	if (!rmid_ptrs) {
> > +		max_idx_limit = resctrl_arch_system_max_rmid_idx();
> > +		rmid_ptrs = kzalloc_objs(struct rmid_entry, max_idx_limit);
> > +		if (!rmid_ptrs)
> > +			return -ENOMEM;
> >  
> > -	for (i = 0; i < idx_limit; i++) {
> > -		entry = &rmid_ptrs[i];
> > -		INIT_LIST_HEAD(&entry->list);
> > +		for (i = 0; i < max_idx_limit; i++) {
> > +			entry = &rmid_ptrs[i];
> > +			INIT_LIST_HEAD(&entry->list);
> >  
> > -		resctrl_arch_rmid_idx_decode(i, &entry->closid, &entry->rmid);
> > -		list_add_tail(&entry->list, &rmid_free_lru);
> > +			resctrl_arch_rmid_idx_decode(i, &entry->closid, &entry->rmid);
> > +		}
> >  	}
> >  
> > +	/* Find how many RMIDs are needed for this mount */
> > +	min_idx_limit = resctrl_arch_system_num_rmid_idx();
> 
> Since mon_capable resources can now come and go from mount to mount there are scenarios where
> rmid_ptrs[] may be smaller than min_idx_limit so above needs extra checks to protect against
> overrun below.

I don't intend for that to happen. The initial allocation of rmid_ptrs[]
is supposed to be large enough that no subsequent mount would need more.

I.e. resctrl_arch_system_max_rmid_idx() must return a value >= than
any subsequent resctrl_arch_system_num_rmid_idx() call.

Perhaps I should add this to resctrl_arch_system_num_rmid_idx()?

	num_rmids = min(num_rmids, resctrl_arch_system_max_rmid_idx());

> > +
> > +	INIT_LIST_HEAD(&rmid_free_lru);
> > +
> >  	/*
> >  	 * RESCTRL_RESERVED_CLOSID and RESCTRL_RESERVED_RMID are special and
> >  	 * are always allocated. These are used for the rdtgroup_default
> > @@ -998,8 +1014,14 @@ int setup_rmid_lru_list(void)
> >  	 */
> >  	idx = resctrl_arch_rmid_idx_encode(RESCTRL_RESERVED_CLOSID,
> >  					   RESCTRL_RESERVED_RMID);
> > -	entry = __rmid_entry(idx);
> > -	list_del(&entry->list);
> > +
> > +	for (i = 0; i < min_idx_limit; i++) {
> > +		entry = &rmid_ptrs[i];
> > +		/* Don't add reserved or busy entries to free list */
> > +		if (i == idx || entry->busy)
> > +			continue;
> > +		list_add_tail(&entry->list, &rmid_free_lru);
> > +	}
> >  
> >  	return 0;
> >  }
> 
> Reinette

-Tony

Re: [PATCH v9 07/12] x86/resctrl: Add PMT registration API for AET enumeration callbacks

[Luck, Tony]

On Wed, Jul 08, 2026 at 03:51:22PM -0700, Reinette Chatre wrote:
> Hi Tony,
> 
> On 7/1/26 2:35 PM, Tony Luck wrote:
> > resctrl is always built-in; INTEL_PMT_TELEMETRY may be a module.  Add, and
> > export, register/unregister functions so the PMT module can supply/clear
> > enumeration callback functions when loaded/unloaded.
> > 
> > Suggested-by: Christoph Hellwig <hch@infradead.org>
> > Signed-off-by: Tony Luck <tony.luck@intel.com>
> > ---
> >  arch/x86/include/asm/resctrl.h          | 22 ++++++++++++++++
> >  arch/x86/kernel/cpu/resctrl/intel_aet.c | 34 +++++++++++++++++++++++++
> >  2 files changed, 56 insertions(+)
> > 
> > diff --git a/arch/x86/include/asm/resctrl.h b/arch/x86/include/asm/resctrl.h
> > index a9f481b7a8ed..48ad55bcda66 100644
> > --- a/arch/x86/include/asm/resctrl.h
> > +++ b/arch/x86/include/asm/resctrl.h
> > @@ -4,6 +4,8 @@
> >  
> >  #ifdef CONFIG_X86_CPU_RESCTRL
> >  
> > +#include <linux/intel_pmt_features.h>
> > +#include <linux/intel_vsec.h>
> >  #include <linux/jump_label.h>
> >  #include <linux/percpu.h>
> >  #include <linux/resctrl_types.h>
> > @@ -189,11 +191,31 @@ static inline void resctrl_arch_mon_ctx_free(struct rdt_resource *r,
> >  
> >  void resctrl_cpu_detect(struct cpuinfo_x86 *c);
> >  
> > +#ifdef CONFIG_X86_CPU_RESCTRL_INTEL_AET
> > +void intel_aet_register_enumeration(struct module *module,
> > +				    struct pmt_feature_group *(*get)(enum pmt_feature_id id),
> > +				    void (*put)(struct pmt_feature_group *p));
> > +void intel_aet_unregister_enumeration(void);
> >  #else
> > +static inline void intel_aet_register_enumeration(struct module *module,
> > +						  struct pmt_feature_group *(*get)(enum pmt_feature_id id),
> > +						  void (*put)(struct pmt_feature_group *p)) { }
> > +static inline void intel_aet_unregister_enumeration(void) { }
> > +#endif /* CONFIG_X86_CPU_RESCTRL_INTEL_AET */
> > +
> > +#else
> > +
> > +#include <linux/intel_pmt_features.h>
> > +#include <linux/intel_vsec.h>
> 
> Are we sure we want to include these in arch/x86/kernel/process_32.c and
> arch/x86/kernel/process_64.c when resctrl is not built in?

I'll look at this some more. Maybe this code needs some X86_64 checks?

> >  
> >  static inline void resctrl_arch_sched_in(struct task_struct *tsk) {}
> >  static inline void resctrl_cpu_detect(struct cpuinfo_x86 *c) {}
> >  
> > +static inline void intel_aet_register_enumeration(struct module *module,
> > +						  struct pmt_feature_group *(*get)(enum pmt_feature_id id),
> > +						  void (*put)(struct pmt_feature_group *p)) { }
> > +static inline void intel_aet_unregister_enumeration(void) { }
> > +
> >  #endif /* CONFIG_X86_CPU_RESCTRL */
> >  
> >  #endif /* _ASM_X86_RESCTRL_H */
> > diff --git a/arch/x86/kernel/cpu/resctrl/intel_aet.c b/arch/x86/kernel/cpu/resctrl/intel_aet.c
> > index cd8257c58f84..c26bd813e057 100644
> > --- a/arch/x86/kernel/cpu/resctrl/intel_aet.c
> > +++ b/arch/x86/kernel/cpu/resctrl/intel_aet.c
> > @@ -12,17 +12,21 @@
> >  #define pr_fmt(fmt)   "resctrl: " fmt
> >  
> >  #include <linux/bits.h>
> > +#include <linux/cleanup.h>
> >  #include <linux/compiler_types.h>
> >  #include <linux/container_of.h>
> >  #include <linux/cpumask.h>
> >  #include <linux/err.h>
> >  #include <linux/errno.h>
> > +#include <linux/export.h>
> >  #include <linux/gfp_types.h>
> >  #include <linux/init.h>
> >  #include <linux/intel_pmt_features.h>
> >  #include <linux/intel_vsec.h>
> >  #include <linux/io.h>
> >  #include <linux/minmax.h>
> > +#include <linux/mutex.h>
> > +#include <linux/module.h>
> >  #include <linux/printk.h>
> >  #include <linux/rculist.h>
> >  #include <linux/rcupdate.h>
> > @@ -291,6 +295,10 @@ static enum pmt_feature_id lookup_pfid(const char *pfname)
> >  	return FEATURE_INVALID;
> >  }
> >  
> > +static struct module *pmt_module;
> > +static struct pmt_feature_group *(*get_feature)(enum pmt_feature_id id);
> > +static void (*put_feature)(struct pmt_feature_group *p);
> > +
> >  /*
> >   * Request a copy of struct pmt_feature_group for each event group. If there is
> >   * one, the returned structure has an array of telemetry_region structures,
> > @@ -325,6 +333,32 @@ bool intel_aet_get_events(void)
> >  	return ret;
> >  }
> >  
> > +/*
> > + * Defend against races between pmt_telemetry register/unregister and
> > + * resctrl file system mount/unmount.
> > + */
> 
> Please help developers understand what this mutex protects. I am not able to deduce
> from above when this mutex needs to be taken.
> 
> > +static DEFINE_MUTEX(aet_register_lock);
> > +
> > +void intel_aet_register_enumeration(struct module *module,
> > +				    struct pmt_feature_group *(*get)(enum pmt_feature_id id),
> > +				    void (*put)(struct pmt_feature_group *p))
> > +{
> > +	guard(mutex)(&aet_register_lock);
> > +	get_feature = get;
> > +	put_feature = put;
> > +	pmt_module = module;
> > +}
> > +EXPORT_SYMBOL_NS_GPL(intel_aet_register_enumeration, "INTEL_PMT");
> > +
> > +void intel_aet_unregister_enumeration(void)
> > +{
> > +	guard(mutex)(&aet_register_lock);
> > +	pmt_module = NULL;
> > +	get_feature = NULL;
> > +	put_feature = NULL;
> > +}
> > +EXPORT_SYMBOL_NS_GPL(intel_aet_unregister_enumeration, "INTEL_PMT");
> > +
> >  void __exit intel_aet_exit(void)
> >  {
> >  	struct event_group **peg;
> 
> Reinette

-Tony

Re: [PATCH v9 09/12] arm,x86/resctrl: Resolve INTEL_PMT_TELEMETRY symbols at runtime

[Luck, Tony]

On Wed, Jul 08, 2026 at 03:52:36PM -0700, Reinette Chatre wrote:
> Hi Tony,
> 
> On 7/1/26 2:35 PM, Tony Luck wrote:
> > resctrl is always built-in, but INTEL_PMT_TELEMETRY and INTEL_TPMI are
> > logically independent and should be loadable modules.  Switch AET to use the
> > function-pointer registration API instead of direct link-time references to
> > PMT symbols.
> > 
> > Prepare for the file system to call resctrl_arch_pre_mount() on every mount
> > by moving AET enumeration into resctrl_arch_pre_mount() and cleanup into
> > resctrl_arch_unmount(). This allows the PMT module to be unloaded whenever
> > the filesystem is not mounted.
> > 
> > Remove intel_aet_exit because all cleanup now happens in the unmount path.
> 
> intel_aet_exit -> intel_aet_exit()
> 
> At first glance this change does not seem symmetrical since it introduces
> the new enumeration and cleanup calls but does not call the cleanup call that
> appears as though it keeps a reference to the module. I think it will help to
> highlight that the previous exit code was not called anyway and at this point
> the module is still required to be built in so there is no actual reference taken.

Sure, will update.

> > 
> > Note that the Linux file system code does not serialize calls to
> > fs_context_operations::get_tree(), so there may be arbitrarily many parallel
> > calls if users invoke mount(2) multiple times.
> > 
> > Use "pmt_in_use" to detect nested mount attempts.
> > 
> > event_group::num_rmid may be reset (reduced) during enumeration. This is
> > not worth resetting on unmount because the same reduction would occur on
> > each subsequent mount.
> 
> If event_group::num_rmid gets a mention, should rdt_resource::resctrl_mon::num_rmid also?

Ok.
> 
> > 
> > Signed-off-by: Tony Luck <tony.luck@intel.com>
> > --
> > v9:
> > 	Drop broken atomic ops introduced in v8
> > 	Check pmt_in_use in intel_aet_pre_mount() to detect nested
> > 	mount flow.
> > ---
> >  include/linux/resctrl.h                 |  6 +++
> >  arch/x86/kernel/cpu/resctrl/internal.h  | 23 ++++++++--
> >  arch/x86/kernel/cpu/resctrl/core.c      | 25 +++++++++--
> >  arch/x86/kernel/cpu/resctrl/intel_aet.c | 58 +++++++++++++++++++++----
> >  drivers/resctrl/mpam_resctrl.c          |  4 ++
> >  5 files changed, 101 insertions(+), 15 deletions(-)
> > 
> > diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
> > index e1501ee72d28..4dc41dc99dc2 100644
> > --- a/include/linux/resctrl.h
> > +++ b/include/linux/resctrl.h
> > @@ -564,6 +564,12 @@ void resctrl_offline_cpu(unsigned int cpu);
> >   */
> >  void resctrl_arch_pre_mount(void);
> >  
> > +/*
> > + * Architecture hook called when mount fails, or on unmount.
> > + * No locks are held.
> > + */
> > +void resctrl_arch_unmount(void);
> > +
> >  /**
> >   * resctrl_arch_rmid_read() - Read the eventid counter corresponding to rmid
> >   *			      for this resource and domain.
> > diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> > index e3cfa0c10e92..a891017f69ec 100644
> > --- a/arch/x86/kernel/cpu/resctrl/internal.h
> > +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> > @@ -234,15 +234,15 @@ void rdt_domain_reconfigure_cdp(struct rdt_resource *r);
> >  void resctrl_arch_mbm_cntr_assign_set_one(struct rdt_resource *r);
> >  
> >  #ifdef CONFIG_X86_CPU_RESCTRL_INTEL_AET
> > -bool intel_aet_get_events(void);
> > -void __exit intel_aet_exit(void);
> > +bool intel_aet_pre_mount(void);
> > +void intel_aet_unmount(void);
> >  int intel_aet_read_event(int domid, u32 rmid, void *arch_priv, u64 *val);
> >  void intel_aet_mon_domain_setup(int cpu, int id, struct rdt_resource *r,
> >  				struct list_head *add_pos);
> >  bool intel_handle_aet_option(bool force_off, char *tok);
> >  #else
> > -static inline bool intel_aet_get_events(void) { return false; }
> > -static inline void __exit intel_aet_exit(void) { }
> > +static inline bool intel_aet_pre_mount(void) { return false; }
> > +static inline void intel_aet_unmount(void) { }
> >  static inline int intel_aet_read_event(int domid, u32 rmid, void *arch_priv, u64 *val)
> >  {
> >  	return -EINVAL;
> > @@ -253,4 +253,19 @@ static inline void intel_aet_mon_domain_setup(int cpu, int id, struct rdt_resour
> >  static inline bool intel_handle_aet_option(bool force_off, char *tok) { return false; }
> >  #endif
> >  
> > +#ifdef CONFIG_INTEL_PMT_TELEMETRY_MODULE
> > +static inline bool intel_aet_try_module_get(struct module *mod)
> > +{
> > +	return try_module_get(mod);
> > +}
> > +
> > +static inline void intel_aet_module_put(struct module *mod)
> > +{
> > +	module_put(mod);
> > +}
> > +#else
> > +static inline bool intel_aet_try_module_get(struct module *mod) { return true; }
> > +static inline void intel_aet_module_put(struct module *mod) { }
> > +#endif
> 
> 
> Trying to look back to how the above ended up like this I find:
> https://lore.kernel.org/lkml/SJ1PR11MB60838C247727AE7CC324BB25FC1A2@SJ1PR11MB6083.namprd11.prod.outlook.com/
> that states:
> 	pmt_module is NULL if CONFIG_INTEL_PMT_TELEMETRY=y ... i.e. built-in to the kernel.
> 
> 	In that case it obviously can't go away, and doesn't need module_get()/module_put().
> 	There's no special case for this. try_module_get() takes a fault on NULL dereference.
> 	...
> 
> Taking a closer look at try_module_get() I do not see it attempting to dereference
> a NULL parameter but instead it returns true in this case which makes the above unnecessary?
> What am I missing?

I don't know. I definitely got a NULL dereference when I did a test with
the PMT code built-in. But it looks like I mis-diagnosed the issue. You
are right the try_module_get() and module_put() have special cases to
handle a NULL argument.

I will revisit.

> > +
> >  #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 3169441a2d40..3ef620d8e0c8 100644
> > --- a/arch/x86/kernel/cpu/resctrl/core.c
> > +++ b/arch/x86/kernel/cpu/resctrl/core.c
> > @@ -16,10 +16,12 @@
> >  
> >  #define pr_fmt(fmt)	"resctrl: " fmt
> >  
> > +#include <linux/cleanup.h>
> >  #include <linux/cpu.h>
> >  #include <linux/slab.h>
> >  #include <linux/err.h>
> >  #include <linux/cpuhotplug.h>
> > +#include <linux/mutex.h>
> >  
> >  #include <asm/cpu_device_id.h>
> >  #include <asm/cpuid/api.h>
> > @@ -798,7 +800,7 @@ void resctrl_arch_pre_mount(void)
> >  	struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_PERF_PKG].r_resctrl;
> >  	int cpu;
> >  
> > -	if (!intel_aet_get_events())
> > +	if (!intel_aet_pre_mount())
> >  		return;
> >  
> >  	/*
> > @@ -814,6 +816,25 @@ void resctrl_arch_pre_mount(void)
> >  	cpus_read_unlock();
> >  }
> >  
> > +void resctrl_arch_unmount(void)
> > +{
> > +	struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_PERF_PKG].r_resctrl;
> > +	int cpu;
> > +
> > +	intel_aet_unmount();
> > +
> > +	if (!r->mon_capable)
> > +		return;
> 
> Could the mon_capable check be moved before intel_aet_unmount()?

Yes. Will move.

> > +
> > +	cpus_read_lock();
> > +	mutex_lock(&domain_list_lock);
> > +	for_each_online_cpu(cpu)
> > +		domain_remove_cpu_mon(cpu, r);
> > +	r->mon_capable = false;
> > +	mutex_unlock(&domain_list_lock);
> > +	cpus_read_unlock();
> > +}
> > +
> >  enum {
> >  	RDT_FLAG_CMT,
> >  	RDT_FLAG_MBM_TOTAL,
> > @@ -1187,8 +1208,6 @@ 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
> > index c26bd813e057..c2e27ce52248 100644
> > --- a/arch/x86/kernel/cpu/resctrl/intel_aet.c
> > +++ b/arch/x86/kernel/cpu/resctrl/intel_aet.c
> > @@ -25,8 +25,8 @@
> >  #include <linux/intel_vsec.h>
> >  #include <linux/io.h>
> >  #include <linux/minmax.h>
> > -#include <linux/mutex.h>
> >  #include <linux/module.h>
> > +#include <linux/mutex.h>
> >  #include <linux/printk.h>
> >  #include <linux/rculist.h>
> >  #include <linux/rcupdate.h>
> 
> This can be squashed with earlier patch.

OK. Will do.

> > @@ -310,7 +310,7 @@ static void (*put_feature)(struct pmt_feature_group *p);
> >   * struct pmt_feature_group to indicate that its events are successfully
> >   * enabled.
> >   */
> > -bool intel_aet_get_events(void)
> > +static bool aet_get_events(void)
> >  {
> >  	struct pmt_feature_group *p;
> >  	enum pmt_feature_id pfid;
> > @@ -319,14 +319,14 @@ bool intel_aet_get_events(void)
> >  
> >  	for_each_event_group(peg) {
> >  		pfid = lookup_pfid((*peg)->pfname);
> > -		p = intel_pmt_get_regions_by_feature(pfid);
> > +		p = get_feature(pfid);
> >  		if (IS_ERR_OR_NULL(p))
> >  			continue;
> >  		if (enable_events(*peg, p)) {
> >  			(*peg)->pfg = p;
> >  			ret = true;
> >  		} else {
> > -			intel_pmt_put_feature_group(p);
> > +			put_feature(p);
> >  		}
> >  	}
> >  
> > @@ -359,16 +359,58 @@ void intel_aet_unregister_enumeration(void)
> >  }
> >  EXPORT_SYMBOL_NS_GPL(intel_aet_unregister_enumeration, "INTEL_PMT");
> >  
> > -void __exit intel_aet_exit(void)
> > +/*
> > + * Track whether pmt_telemetry enumeration succeeded during mount to
> > + * detect nested mount attempts and for use during unmount.
> > + */
> > +static bool pmt_in_use;
> > +
> > +bool intel_aet_pre_mount(void)
> > +{
> > +	bool ret;
> > +
> > +	guard(mutex)(&aet_register_lock);
> > +
> > +	/* Nested mount attempt. File system code will return -EBUSY */
> > +	if (pmt_in_use)
> > +		return false;
> > +
> > +	if (!get_feature || !put_feature)
> > +		return false;
> > +
> > +	if (!intel_aet_try_module_get(pmt_module))
> > +		return false;
> > +
> > +	ret = aet_get_events();
> > +
> > +	if (!ret)
> > +		intel_aet_module_put(pmt_module);
> > +	else
> > +		pmt_in_use = true;
> > +
> > +	return ret;
> > +}
> > +
> > +void intel_aet_unmount(void)
> >  {
> >  	struct event_group **peg;
> >  
> > +	guard(mutex)(&aet_register_lock);
> > +	if (!pmt_in_use)
> > +		return;
> > +
> >  	for_each_event_group(peg) {
> > -		if ((*peg)->pfg) {
> > -			intel_pmt_put_feature_group((*peg)->pfg);
> > -			(*peg)->pfg = NULL;
> > +		struct event_group *e = *peg;
> > +
> > +		if (e->pfg) {
> > +			for (int j = 0; j < e->num_events; j++)
> > +				resctrl_disable_mon_event(e->evts[j].id);
> > +			put_feature(e->pfg);
> > +			e->pfg = NULL;
> >  		}
> >  	}
> > +	intel_aet_module_put(pmt_module);
> > +	pmt_in_use = false;
> >  }
> >  
> >  #define DATA_VALID	BIT_ULL(63)
> > diff --git a/drivers/resctrl/mpam_resctrl.c b/drivers/resctrl/mpam_resctrl.c
> > index 7079870ca894..5859cc0f5e37 100644
> > --- a/drivers/resctrl/mpam_resctrl.c
> > +++ b/drivers/resctrl/mpam_resctrl.c
> > @@ -162,6 +162,10 @@ void resctrl_arch_pre_mount(void)
> >  {
> >  }
> >  
> > +void resctrl_arch_unmount(void)
> > +{
> > +}
> > +
> >  bool resctrl_arch_get_cdp_enabled(enum resctrl_res_level rid)
> >  {
> >  	return mpam_resctrl_controls[rid].cdp_enabled;
> 
> Reinette

-Tony

Re: [PATCH v9 10/12] fs/resctrl: Call architecture hooks for every mount/unmount

[Luck, Tony]

On Wed, Jul 08, 2026 at 03:53:20PM -0700, Reinette Chatre wrote:
> Hi Tony,
> 
> On 7/1/26 2:35 PM, Tony Luck wrote:
> > The intel_pmt module should only be pinned while the resctrl file system
> > is mounted.
> > 
> > Add hooks for every mount/unmount of the resctrl file system so that
> > architecture code can increment the intel_pmt module reference count on
> > mount and decrement on unmount.
> > 
> > Signed-off-by: Tony Luck <tony.luck@intel.com>
> > ---
> 
> > @@ -3168,6 +3167,8 @@ static void resctrl_unmount(void)
> >  	resctrl_mounted = false;
> >  	mutex_unlock(&rdtgroup_mutex);
> >  	cpus_read_unlock();
> > +
> > +	resctrl_arch_unmount();
> >  }
> >  
> >  static int rdt_get_tree(struct fs_context *fc)
> > @@ -3179,7 +3180,7 @@ static int rdt_get_tree(struct fs_context *fc)
> >  	struct rdt_resource *r;
> >  	int ret;
> >  
> > -	DO_ONCE_SLEEPABLE(resctrl_arch_pre_mount);
> > +	resctrl_arch_pre_mount();
> >  
> >  	cpus_read_lock();
> >  	mutex_lock(&rdtgroup_mutex);
> > @@ -3318,6 +3319,9 @@ static int rdt_get_tree(struct fs_context *fc)
> >  out:
> >  	mutex_unlock(&rdtgroup_mutex);
> >  	cpus_read_unlock();
> > +
> > +	resctrl_arch_unmount();
> > +
> >  	return ret;
> >  }
> >  
> 
> rdt_get_tree() jumps to "out" if resctrl is already mounted - in this scenario
> resctrl fs is mounted and the telemetry events exposed to user space. Above would
> cause telemetry to be yanked from underneath an existing mount?

Correct. I think I just need to make intel_aet_unmount() return a "bool"
so that the early return when resctrl is mounted knows to skip tearing
down the domains etc.
> 
> Reinette

-Tony

Re: [PATCH v9 11/12] x86/resctrl: Simplify Kconfig options for resctrl

[Luck, Tony]

On Wed, Jul 08, 2026 at 04:01:00PM -0700, Reinette Chatre wrote:
> Hi Tony,
> 
> On 7/1/26 2:35 PM, Tony Luck wrote:
> > Linus Torvalds complained[1] about Kconfig complexity making it too
> > hard for "random people to build their own kernels".
> > 
> > CONFIG_X86_CPU_RESCTRL_INTEL_AET has been causing problems since it was
> > first added as it required other config options to be set to "built-in".
> > 
> > AET now resolves PMT symbols at runtime via the registration API,
> > so INTEL_PMT_TELEMETRY no longer needs to be built-in. This means
> > that AET can be included unconditionally as part of X86_CPU_RESCTRL.
> > 
> > CONFIG_X86_CPU_RESCTRL now depends on X86_64 because reading telemetry
> > data needs readq().
> 
> Above reads as though it is a new dependency as opposed to an existing dependency
> that moves one level up. How about something like:
> 	Move the X86_64 dependency of the removed AET config up to CONFIG_X86_CPU_RESCTRL.

Needs more than that. Before this change RESCTRL (sans AET) could be
built as 32-bit. After this change it is 64-bit only.

I'll clone Chen Yu's patch[1] that makes resctrl 64-bit only and
include that before my patch to remove CONFIG_X86_CPU_RESCTRL_INTEL_AET 

> Reinette

-Tony

[1] https://lore.kernel.org/all/cbd33bf5d99b27e98ddf899b36384d9a41f14201.1782866200.git.yu.c.chen@intel.com/
>