[PATCH v8 0/9] Introduce MMIO-based CMT access for Enhanced RDT

[PATCH v8 0/9] Introduce MMIO-based CMT access for Enhanced RDT

[Chen Yu]

v7: https://lore.kernel.org/lkml/cover.1787976868.git.yu.c.chen@intel.com/
v6: https://lore.kernel.org/lkml/cover.1784968626.git.yu.c.chen@intel.com/
v5: https://lore.kernel.org/lkml/cover.1782866200.git.yu.c.chen@intel.com/
v4: https://lore.kernel.org/lkml/cover.1781332698.git.yu.c.chen@intel.com/
v3: https://lore.kernel.org/lkml/cover.1780710620.git.yu.c.chen@intel.com/
v2: https://lore.kernel.org/lkml/cover.1780587063.git.yu.c.chen@intel.com/
v1: https://lore.kernel.org/lkml/cover.1779872016.git.yu.c.chen@intel.com/

Intel Enhanced Resource Director Technology (ERDT) extends the existing
RDT framework with two major capabilities:

  1. MMIO-based access to monitoring and allocation registers, replacing
     the legacy MSR-based interface.
  2. Region-aware RDT for fine-grained control over different tiers of
     memory (e.g., CXL.mem, DDR).

This is described in the Intel RDT Architecture Specification:
https://cdrdv2-public.intel.com/789566/356688-intel-rdt-arch-spec.pdf

This patch set focuses on the first part: enabling MMIO-based access for
Cache Monitoring Technology (CMT), while CAT/MBM/MBA are still using MSR.
The platform advertises the MMIO register layout through the ACPI ERDT
(Enhanced Resource Director Technology) table, which contains sub-tables
describing per-domain register regions for monitoring and allocation.

With ERDT, L3 cache occupancy counters are read via MMIO rather than
MSR, allowing the reads to be performed from any CPU without requiring
cross-CPU IPIs. This series parses the relevant ACPI sub-tables (RMDD,
CMRC), prepares the resctrl monitor infrastructure for MMIO-based reads,
and adds initial support for reading L3 occupancy via the CMRC interface.

kselftest of CMT and L3_CAT has passed with minor adjustment at
https://lore.kernel.org/lkml/20260523101715.3964456-1-yu.c.chen@intel.com/.

Thanks Tony, Reinette, Thomas, Dave, Boris, Peter, Hongyu for your time to
look at this patch set.

V8 has gone through local sashiko review, with false positives left
unaddressed. Also used AI to confirm each review feedback has been
addressed by v6 and v7.

Changes from V7 to V8:
- Rebased on latest x86/cache branch
  git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git
- git commit log revisement
- pick Tony's x86_64 dependency patch from AET patch set and replaced
  mine.
- No other code logic changes

Changes from V6 to V7:
- Let get_rdt_resources() clean up its own state on failure and drop the
  asymmetric __resctrl_arch_late_init() wrapper. (Reinette Chatre)
- Make erdt_max_rmid and num_ids unsigned and use a plain min(), match
  erdt_ioremap() argument types to ioremap(), and reset erdt_max_rmid in
  erdt_exit(). (Reinette Chatre)
- Run erdt_cpu_valid() once in resctrl_arch_online_cpu(), and match an
  ERDT domain to a resctrl monitoring domain by domain id rather than by
  cpumask. Warn on an invalid dom_id and on a duplicate ERDT-to-resctrl
  domain mapping. (Reinette Chatre)
- Fix the silent truncation of the 64-bit CMRC up_scale: reject an
  out-of-range value with a FW_BUG warning and fail closed instead of
  using max_t(int, ...). (Reinette Chatre, Tony Luck)
- Keep the original resctrl_arch_rmid_read() parameter order (resource
  first) and drop the unused closid/arch_priv arguments. (Reinette Chatre)
- Keep a single erdt_cpu_has() declaration in asm/resctrl.h, required by
  the inline resctrl_arch_round_mon_val(). (Reinette Chatre)
- Use switch() in erdt_support(), clamp max_rmid with min_t() so MSR-based
  MBM events are not handed the ERDT RMID, and divide the ERDT scale by
  snc_nodes_per_l3_cache under SNC. (Reinette Chatre)

Changes from V5 to V6:
- Reorder the series so that the x86/topology change, which touches a
  different subsystem, comes first. (Reinette Chatre)
- Drop the v5 "x86/resctrl: Replace 'msr' in monitoring data identifiers"
  patch. None of the renamed identifiers are used by the MMIO code.
  (Reinette Chatre)
- Split the domain setup into erdt_cpu_valid() and
  erdt_l3_mon_domain_setup(). Validation now happens in
  domain_add_cpu_mon() before the CPU is added to the domain cpumask,
  and attaching ERDT data to a freshly created domain (Reinette Chatre)
- Drop the SNC special case and the new resctrl_disable_mon_event().
  (Reinette Chatre)
- resctrl_arch_round_mon_val() now rounds to the ERDT scale instead of
  returning the value unchanged (Reinette Chatre)
- Make erdt_get_max_rmid() a global value instead of a per-CPU lookup.
  (Reinette Chatre)
- Use struct cpumask instead of cpumask_var_t, list_for_each_entry()
  instead of open coded container_of(), and document every member of
  struct erdt_domain_info. (Reinette Chatre)

Thanks Tony, Reinette, Thomas, Hongyu for your time to look at this patch set.

Changes from V4 to V5:
There are some major changes since v4:
- (biggest change) Eliminate the xarray for runtime lookups; embed
  struct erdt_domain_info directly in rdt_hw_l3_mon_domain and assign
  during l3_mon_domain_setup().
- Separate CPUID and ACPI enumeration cleanly. Do not use CPUID feature
  flags to gate MMIO-based monitoring. Use ACPI table presence (e.g., CMRC table)
  to determine event enablement.
- Use ACPI RMDD's own "Max RMID" field for MMIO access instead of relying
  on CPUID's max RMID (which applies to MSR).
- Enforce the SNC constraint in code rather than burying it behind a comment
  WARN. Disable mon_capable in rdt_get_l3_mon_config() when ERDT is
  enabled and snc_nodes_per_l3_cache > 1.
- Split non-resctrl changes (topology.c, apic.h) into a separate preparatory
  patch prefixed with x86/topology.
- Move the "depends on X86" to "X86_64" adjustment to a separate patch with explicit
  justification in its changelog.

Changes from V3 to V4:
- Remove the redundant table length check in subtbl_valid() (Thomas Gleixner)
- Reuse subtbl_valid() for all the table iteration (Thomas Gleixner)
- Refine the commit log of [PATCH 5/6] to state that this change is a
  preparation for [PATCH 6/6] rather than fixing an existing issue
  (Thomas Gleixner, Reinette Chatre, Tony Luck)
- Fix if CACD lists all CPUs in the LLC domain (sashiko)
- Deal with a corner case that if there is no valid RMDD tables,
  the erdt_enabled_flag should remain false.(sashiko)
- Add Thomas's Reviewed-by and Hongyu's Tested-by.

Changes from V2 to V3:
- Wrap __resctrl_arch_late_init() to avoid the goto logic. (Thomas Gleixner)
- Make the variables in struct erdt_domain_info tabular format (Thomas Gleixner)
- Remove tail comments (Thomas Gleixner)
- Make the name of erdt_enabled() and variable in it consistent and
  comprehensible. (Thomas Gleixner)
- Use topo_lookup_cpuid() to search the CPU id according to the x2apic id
  (Thomas Gleixner)
- Fix kernel doc comment format (Thomas Gleixner)
- Use brackets for multiple lines "if" case. (Thomas Gleixner)
- Let the parameter for cacd_init() to fully utilize 100 characters.
  (Thomas Gleixner)
- Variables are reordered in reverse fir-tree.(Thomas Gleixner)
- Added a named constant and use it in the rmdd->flags check.
  (Thomas Gleixner)
- Introduce helper functions to make the code readable when iterating
  the RMDD tables. (Thomas Gleixner)
- Make the macros tabular format. (Thomas Gleixner)

Changes from V1 to V2:
- Add #include <linux/cleanup.h> to follow the "include-what-you-use" best
  practice (Tony Luck)
- Fix 3 issues reported by:
  https://sashiko.dev/#/patchset/cover.1779872016.git.yu.c.chen%40intel.com
  Remove the variable of cacd in struct erdt_domain_info as it will
  never be used after initialization.
  Invoke erdt_exit() to avoid resource leak if rdt_alloc_capable and
  rdt_mon_capable are both false.
  Adjust the comments suggested by sashiko.

Chen Yu (7):
  x86/topology: Export topo_lookup_cpuid() for resctrl use
  x86/resctrl: Parse ACPI ERDT table and save CACD cpumask for RMDD
    domains
  x86/resctrl: Attach ACPI ERDT information to L3 mon domain on CPU
    online
  x86/resctrl: Parse ACPI CMRC table
  x86/resctrl: Refactor the monitor read function
  x86/resctrl: Introduce erdt_cpu_has() and erdt_support()
  x86/resctrl: Add MMIO-based LLC occupancy monitoring support

Tony Luck (2):
  x86/resctrl: Require 64-bit x86 for resctrl support
  fs/resctrl: Do not invoke smp_processor_id() in preemptible context

 arch/x86/Kconfig                       |   4 +-
 arch/x86/include/asm/apic.h            |   1 +
 arch/x86/include/asm/resctrl.h         |  11 +-
 arch/x86/kernel/cpu/resctrl/Makefile   |   1 +
 arch/x86/kernel/cpu/resctrl/core.c     |  60 ++-
 arch/x86/kernel/cpu/resctrl/erdt.c     | 522 +++++++++++++++++++++++++
 arch/x86/kernel/cpu/resctrl/internal.h |  48 ++-
 arch/x86/kernel/cpu/resctrl/monitor.c  |  40 +-
 arch/x86/kernel/cpu/topology.c         |   2 +-
 fs/resctrl/monitor.c                   |  48 ++-
 10 files changed, 704 insertions(+), 33 deletions(-)
 create mode 100644 arch/x86/kernel/cpu/resctrl/erdt.c

-- 
2.25.1

[PATCH v8 1/9] x86/topology: Export topo_lookup_cpuid() for resctrl use

[Chen Yu]

Enhanced RDT (ERDT) enumerates its monitoring and control resources through
ACPI tables that identify each CPU by its x2APIC ID. To associate this
firmware-provided data with the kernel's per-CPU structures, ERDT must
translate those x2APIC IDs into logical CPU numbers.

topo_lookup_cpuid() already performs this translation, but it is static to
topology.c and therefore not reachable from resctrl.

Export topo_lookup_cpuid() and add its declaration to asm/apic.h that already
exposes the inverse mapping via cpuid_to_apicid[], so that ERDT can resolve
x2APIC IDs from ACPI tables to logical CPU numbers.

No functional change intended.

Signed-off-by: Chen Yu <yu.c.chen@intel.com>
Tested-by: Hongyu Ning <hongyu.ning@linux.intel.com>
Reviewed-by: Reinette Chatre <reinette.chatre@intel.com>
---
 arch/x86/include/asm/apic.h    | 1 +
 arch/x86/kernel/cpu/topology.c | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index 9cd493d467d4..bb84651b14bd 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -54,6 +54,7 @@ static inline void x86_32_probe_apic(void) { }
 #endif
 
 extern u32 cpuid_to_apicid[];
+int topo_lookup_cpuid(u32 apic_id);
 
 #define CPU_ACPIID_INVALID	U32_MAX
 
diff --git a/arch/x86/kernel/cpu/topology.c b/arch/x86/kernel/cpu/topology.c
index 4913b64ec592..bcee70fb9277 100644
--- a/arch/x86/kernel/cpu/topology.c
+++ b/arch/x86/kernel/cpu/topology.c
@@ -92,7 +92,7 @@ static inline u32 topo_apicid(u32 apicid, enum x86_topology_domains dom)
 	return apicid & (UINT_MAX << x86_topo_system.dom_shifts[dom - 1]);
 }
 
-static int topo_lookup_cpuid(u32 apic_id)
+int topo_lookup_cpuid(u32 apic_id)
 {
 	int i;
 
-- 
2.25.1

[PATCH v8 2/9] x86/resctrl: Require 64-bit x86 for resctrl support

[Chen Yu]

From: Tony Luck <tony.luck@intel.com>

Intel Application Energy Telemetry (AET) provides event counts in 64-bit
registers in MMIO space. For accurate results these should be read with a
single 64-bit load operation from the CPU.

In preparation for the removal of the separate CONFIG_X86_CPU_RESCTRL_INTEL_AET
configuration option, move the X86_64 dependency to X86_CPU_RESCTRL.

This in line with Dave Hansen's proposal[1] for an addition to Linux
documentation that details the state of 32-bit x86 support.

Signed-off-by: Tony Luck <tony.luck@intel.com>
Signed-off-by: Chen Yu <yu.c.chen@intel.com>
Link: https://lore.kernel.org/all/c6ab0c24-8931-4f0d-9be6-23498ab4c38b@intel.com/ # [1]
---
 arch/x86/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 15fd9ec5ecac..06b5d0c4633e 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -518,7 +518,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
@@ -541,7 +541,7 @@ config X86_CPU_RESCTRL
 
 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
+	depends on X86_CPU_RESCTRL && CPU_SUP_INTEL && INTEL_PMT_TELEMETRY=y && INTEL_TPMI=y
 	help
 	  Enable per-RMID telemetry events in resctrl.
 
-- 
2.25.1

[PATCH v8 3/9] x86/resctrl: Parse ACPI ERDT table and save CACD cpumask for RMDD domains

[Chen Yu]

There is one Enhanced RDT (ERDT) ACPI table per platform. Each Resource
Management Domain Description (RMDD) sub-table within it describes one resource
management domain (RMD), also known as an L3 domain, and carries MMIO base
information for monitoring support. The CPU agents within the scope of an RMDD
are enumerated by their x2APIC IDs in a nested CPU Agent Collection Description
(CACD) sub-table.

Parse the RMDD sub-tables within the ERDT ACPI table and their nested CACD
entries to construct per-domain CPU masks.

For each RMDD, parse the associated CACD, map its x2APIC IDs to logical CPUs,
and save the resulting CPU mask. Associate every ERDT domain with the CPUs that
belong to it to prepare for attaching ERDT data to resctrl monitoring domains.

Based on original work from Anil S Keshavamurthy.

Suggested-by: Tony Luck <tony.luck@intel.com>
Suggested-by: Reinette Chatre <reinette.chatre@intel.com>
Signed-off-by: Chen Yu <yu.c.chen@intel.com>
Tested-by: Hongyu Ning <hongyu.ning@linux.intel.com>
---
 arch/x86/kernel/cpu/resctrl/Makefile   |   1 +
 arch/x86/kernel/cpu/resctrl/core.c     |  16 +-
 arch/x86/kernel/cpu/resctrl/erdt.c     | 265 +++++++++++++++++++++++++
 arch/x86/kernel/cpu/resctrl/internal.h |  29 +++
 4 files changed, 309 insertions(+), 2 deletions(-)
 create mode 100644 arch/x86/kernel/cpu/resctrl/erdt.c

diff --git a/arch/x86/kernel/cpu/resctrl/Makefile b/arch/x86/kernel/cpu/resctrl/Makefile
index 273ddfa30836..2216ee084832 100644
--- a/arch/x86/kernel/cpu/resctrl/Makefile
+++ b/arch/x86/kernel/cpu/resctrl/Makefile
@@ -2,6 +2,7 @@
 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)		+= erdt.o
 obj-$(CONFIG_RESCTRL_FS_PSEUDO_LOCK)	+= pseudo_lock.o
 
 # To allow define_trace.h's recursive include:
diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
index 55214d6fdc49..54cfdf12dfbb 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
@@ -1016,10 +1016,17 @@ static __init void check_quirks(void)
 
 static __init bool get_rdt_resources(void)
 {
+	bool succeed;
+
+	erdt_init();
 	rdt_alloc_capable = get_rdt_alloc_resources();
 	rdt_mon_capable = get_rdt_mon_resources();
 
-	return (rdt_mon_capable || rdt_alloc_capable);
+	succeed = (rdt_mon_capable || rdt_alloc_capable);
+	if (!succeed)
+		erdt_exit();
+
+	return succeed;
 }
 
 static __init void rdt_init_res_defs_intel(void)
@@ -1141,12 +1148,15 @@ static int __init resctrl_arch_late_init(void)
 				  "x86/resctrl/cat:online:",
 				  resctrl_arch_online_cpu,
 				  resctrl_arch_offline_cpu);
-	if (state < 0)
+	if (state < 0) {
+		erdt_exit();
 		return state;
+	}
 
 	ret = resctrl_init();
 	if (ret) {
 		cpuhp_remove_state(state);
+		erdt_exit();
 		return ret;
 	}
 	rdt_online = state;
@@ -1169,6 +1179,8 @@ static void __exit resctrl_arch_exit(void)
 	cpuhp_remove_state(rdt_online);
 
 	resctrl_exit();
+
+	erdt_exit();
 }
 
 __exitcall(resctrl_arch_exit);
diff --git a/arch/x86/kernel/cpu/resctrl/erdt.c b/arch/x86/kernel/cpu/resctrl/erdt.c
new file mode 100644
index 000000000000..0dfe5eda166c
--- /dev/null
+++ b/arch/x86/kernel/cpu/resctrl/erdt.c
@@ -0,0 +1,265 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Enhanced Resource Director Technology (ERDT)
+ *
+ * Copyright (C) 2026 Intel Corporation
+ *
+ */
+
+#define pr_fmt(fmt)     "resctrl: " fmt
+
+#include <linux/acpi.h>
+#include <linux/overflow.h>
+#include <linux/resctrl.h>
+#include <linux/sizes.h>
+
+#include <asm/apic.h>
+
+#include "internal.h"
+
+static LIST_HEAD(domain_info_list);
+
+/* True when the ERDT ACPI table describes at least one domain with at least one CPU. */
+static bool erdt_enabled;
+
+#define ERDT_VALID_VERSION		1
+#define RMDD_FLAG_CPU_L3_DOMAIN		BIT(0)
+
+/* Bitmask of valid sub-tables found in the first RMDD, used to ensure all RMDDs match. */
+static u32 valid_subtbl_mask;
+
+/* Domain ID of the first RMDD that established @valid_subtbl_mask, for diagnostics. */
+static u16 first_rmdd_domain_id;
+
+/*
+ * The minimal max-rmid of different domains. Using minimal is to avoid the domain with
+ * small rmid accessing an invalid rmid.
+ */
+static unsigned int erdt_max_rmid;
+
+unsigned int erdt_get_max_rmid(void)
+{
+	return erdt_max_rmid;
+}
+
+static void __iomem *erdt_ioremap(resource_size_t base, u32 num_pages, const char *desc)
+{
+	void __iomem *addr;
+	unsigned long size;
+
+	if (check_mul_overflow(num_pages, SZ_4K, &size))
+		return NULL;
+
+	addr = ioremap(base, size);
+	if (!addr)
+		pr_warn(FW_BUG "ERDT: Failed to map %s at phys addr %pa (size: %u pages)\n",
+			desc, &base, num_pages);
+
+	return addr;
+}
+
+static void erdt_iounmap_domain(struct erdt_domain_info *domain)
+{
+	for (int i = 0; i < ERDT_MMIO_NUM_TYPES; i++) {
+		if (domain->base[i]) {
+			iounmap(domain->base[i]);
+			domain->base[i] = NULL;
+		}
+	}
+}
+
+static void cleanup_one_domain(struct erdt_domain_info *d)
+{
+	erdt_iounmap_domain(d);
+	kfree(d);
+}
+
+/*
+ * Save CACD information for this RMDD:
+ * convert the X2APIC to CPU and save them in a mask.
+ */
+static __init int cacd_init(struct acpi_subtbl_hdr_16 *subtbl,
+			    struct erdt_domain_info *domain_info)
+{
+	struct acpi_erdt_cacd *cacd = (struct acpi_erdt_cacd *)subtbl;
+	unsigned int num_ids;
+	int cpu;
+
+	if (cacd->header.length < struct_size(cacd, X2APICIDS, 1)) {
+		pr_warn(FW_BUG "Invalid x2apicid CACD table\n");
+		return -EIO;
+	}
+
+	num_ids = (cacd->header.length - sizeof(*cacd)) / sizeof(cacd->X2APICIDS[0]);
+
+	for (unsigned int i = 0; i < num_ids; i++) {
+		cpu = topo_lookup_cpuid(cacd->X2APICIDS[i]);
+		if (cpu < 0) {
+			pr_warn(FW_BUG "Unknown x2apicid 0x%x\n", cacd->X2APICIDS[i]);
+			return -EIO;
+		}
+
+		cpumask_set_cpu(cpu, &domain_info->cpu_mask);
+	}
+
+	return 0;
+}
+
+static inline struct acpi_subtbl_hdr_16 *rmdd_subtbl(struct acpi_erdt_rmdd *rmdd)
+{
+	return (void *)rmdd + sizeof(*rmdd);
+}
+
+static inline struct acpi_subtbl_hdr_16 *next_subtbl(struct acpi_subtbl_hdr_16 *subtbl)
+{
+	return (void *)subtbl + subtbl->length;
+}
+
+static inline bool subtbl_valid(void *end, struct acpi_subtbl_hdr_16 *subtbl)
+{
+	/* Ensure the header is within bounds before dereferencing it. */
+	if ((void *)subtbl + sizeof(*subtbl) > end)
+		return false;
+
+	/* A sub-table must be at least as large as its header. */
+	if (subtbl->length < sizeof(*subtbl))
+		return false;
+
+	/* The entire sub-table (including body) must fit within the parent. */
+	if ((void *)subtbl + subtbl->length > end)
+		return false;
+
+	return true;
+}
+
+static __init bool parse_rmdd_table(struct acpi_subtbl_hdr_16 *rmdd_hdr)
+{
+	struct acpi_erdt_rmdd *rmdd = (struct acpi_erdt_rmdd *)rmdd_hdr;
+	struct erdt_domain_info *domain_info;
+	struct acpi_subtbl_hdr_16 *subtbl;
+	u32 subtbl_mask = 0;
+
+	if (rmdd->header.length < sizeof(*rmdd)) {
+		pr_warn(FW_BUG "Invalid RMDD length %u bytes\n", rmdd->header.length);
+		return false;
+	}
+
+	/* Quietly ignore non-CPU-based L3 domains */
+	if (!(rmdd->flags & RMDD_FLAG_CPU_L3_DOMAIN))
+		return true;
+
+	domain_info = kzalloc_obj(*domain_info, GFP_KERNEL);
+	if (!domain_info)
+		return false;
+
+	domain_info->dom_id = -1;
+
+	domain_info->base[ERDT_MMIO_RMDD_CREG] =
+		erdt_ioremap(rmdd->creg_base, rmdd->creg_size, "RMDD ctrl base");
+	if (!domain_info->base[ERDT_MMIO_RMDD_CREG])
+		goto cleanup;
+
+	for (subtbl = rmdd_subtbl(rmdd);
+	     subtbl_valid((void *)rmdd + rmdd->header.length, subtbl);
+	     subtbl = next_subtbl(subtbl)) {
+		switch (subtbl->type) {
+		/* An RMDD table has one or more CACD sub-table(s) */
+		case ACPI_ERDT_TYPE_CACD:
+			if (cacd_init(subtbl, domain_info))
+				goto cleanup;
+
+			subtbl_mask |= BIT(ACPI_ERDT_TYPE_CACD);
+			break;
+		default:
+			break;
+		}
+	}
+
+	if (!subtbl_mask)
+		goto cleanup;
+
+	/*
+	 * Require all RMDDs to support same set of sub-tables
+	 */
+	if (!valid_subtbl_mask) {
+		valid_subtbl_mask = subtbl_mask;
+		first_rmdd_domain_id = rmdd->domain_id;
+	} else if (subtbl_mask != valid_subtbl_mask) {
+		pr_warn(FW_BUG "RMDD %u sub-table set does not match the first RMDD %u\n",
+			rmdd->domain_id, first_rmdd_domain_id);
+		goto cleanup;
+	}
+
+	if (!rmdd->max_rmid) {
+		pr_warn(FW_BUG "Unreasonable RMDD max_rmid %u\n", rmdd->max_rmid);
+		goto cleanup;
+	}
+
+	if (!erdt_max_rmid)
+		erdt_max_rmid = rmdd->max_rmid;
+	else
+		erdt_max_rmid = min(erdt_max_rmid, rmdd->max_rmid);
+
+	list_add(&domain_info->entry, &domain_info_list);
+
+	return true;
+
+cleanup:
+	cleanup_one_domain(domain_info);
+	return false;
+}
+
+void erdt_exit(void)
+{
+	struct erdt_domain_info *d, *tmp;
+
+	list_for_each_entry_safe(d, tmp, &domain_info_list, entry) {
+		list_del(&d->entry);
+		cleanup_one_domain(d);
+	}
+	erdt_enabled = false;
+	valid_subtbl_mask = 0;
+	first_rmdd_domain_id = 0;
+	erdt_max_rmid = 0;
+}
+
+static __init int enumerate_erdt_table(struct acpi_table_header *table_hdr)
+{
+	struct acpi_table_erdt *erdt = (struct acpi_table_erdt *)table_hdr;
+	struct acpi_subtbl_hdr_16 *subtbl;
+
+	if (erdt->header.revision != ERDT_VALID_VERSION) {
+		pr_info("Unsupported ERDT table revision %u (expected %u)\n",
+			erdt->header.revision, ERDT_VALID_VERSION);
+		return -EINVAL;
+	}
+
+	if (erdt->header.length < sizeof(*erdt)) {
+		pr_warn(FW_BUG "ERDT: Invalid table length %u bytes\n", erdt->header.length);
+		return -EINVAL;
+	}
+
+	for (subtbl = (void *)erdt + sizeof(*erdt);
+	     subtbl_valid((void *)erdt + erdt->header.length, subtbl);
+	     subtbl = next_subtbl(subtbl)) {
+		if (subtbl->type == ACPI_ERDT_TYPE_RMDD &&
+		    !parse_rmdd_table(subtbl))
+			goto cleanup;
+	}
+
+	if (list_empty(&domain_info_list))
+		goto cleanup;
+
+	erdt_enabled = true;
+
+	return 0;
+
+cleanup:
+	erdt_exit();
+	return -EINVAL;
+}
+
+int __init erdt_init(void)
+{
+	return acpi_table_parse(ACPI_SIG_ERDT, enumerate_erdt_table);
+}
diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index e3cfa0c10e92..156206088372 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -21,6 +21,31 @@
 
 #define RMID_VAL_UNAVAIL		BIT_ULL(62)
 
+/*
+ * Index into erdt_domain_info::base[] for each MMIO region.
+ * @ERDT_MMIO_RMDD_CREG: RMDD control register base address
+ */
+enum erdt_mmio_type {
+	ERDT_MMIO_RMDD_CREG,
+	ERDT_MMIO_LAST = ERDT_MMIO_RMDD_CREG
+};
+
+#define ERDT_MMIO_NUM_TYPES	(ERDT_MMIO_LAST + 1)
+
+/**
+ * struct erdt_domain_info - Per-domain ERDT information
+ * @base:	Array of ioremapped MMIO region base addresses, indexed by ERDT_MMIO_*
+ * @cpu_mask:	CPUs belonging to this resource management domain
+ * @dom_id:	L3 cache ID shared by all CPUs in this domain (-1 if unset)
+ * @entry:	Links into the global domain_info_list
+ */
+struct erdt_domain_info {
+	void __iomem		*base[ERDT_MMIO_NUM_TYPES];
+	struct cpumask		cpu_mask;
+	int			dom_id;
+	struct list_head	entry;
+};
+
 /*
  * With the above fields in use 62 bits remain in MSR_IA32_QM_CTR for
  * data to be returned. The counter width is discovered from the hardware
@@ -253,4 +278,8 @@ 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
 
+unsigned int erdt_get_max_rmid(void);
+int erdt_init(void);
+void erdt_exit(void);
+
 #endif /* _ASM_X86_RESCTRL_INTERNAL_H */
-- 
2.25.1

[PATCH v8 4/9] x86/resctrl: Attach ACPI ERDT information to L3 mon domain on CPU online

[Chen Yu]

Reading LLC occupancy counters via MMIO requires the per-domain ERDT
information, parsed from the ACPI ERDT table, to be reachable from the resctrl
L3 monitoring domain. Nothing links the two yet, so the monitoring code cannot
locate the MMIO registers of a domain.

ERDT and CPUID enumerate CPU-to-L3-domain membership independently: CPUID leaf 4
describes the L3 cache topology, while the firmware CACD sub-table lists the
CPUs of each ERDT domain. Both views must agree on a CPU's L3 domain for that
CPU to be monitored safely.

When a CPU comes online, validate that firmware and CPUID agree on its L3 domain
before adding it to any resctrl domain. Exclude the CPU from all resctrl domains
on a mismatch because a topology inconsistency between ERDT and CPUID indicates
a firmware defect that makes the CPU's domain placement unreliable for any
resource. Otherwise attach the matching ERDT domain information to the L3
monitoring domain so that monitoring data can be read via ERDT and its
sub-tables.

Suggested-by: Reinette Chatre <reinette.chatre@intel.com>
Signed-off-by: Chen Yu <yu.c.chen@intel.com>
Tested-by: Hongyu Ning <hongyu.ning@linux.intel.com>
---
 arch/x86/kernel/cpu/resctrl/core.c     |  16 ++++
 arch/x86/kernel/cpu/resctrl/erdt.c     | 109 +++++++++++++++++++++++++
 arch/x86/kernel/cpu/resctrl/internal.h |   5 ++
 3 files changed, 130 insertions(+)

diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
index 54cfdf12dfbb..3514d73a8056 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
@@ -34,6 +34,9 @@
  * the domain list must either take cpus_read_lock(), or rely on an RCU
  * read-side critical section, to avoid observing concurrent modification.
  * All writers take this mutex:
+ *
+ * This mutex also protects the ERDT domain_info_list, which is modified when a
+ * CPU comes online.
  */
 static DEFINE_MUTEX(domain_list_lock);
 
@@ -564,6 +567,8 @@ static void l3_mon_domain_setup(int cpu, int id, struct rdt_resource *r, struct
 		return;
 	}
 	list_add_tail_rcu(&d->hdr.list, add_pos);
+
+	erdt_l3_mon_domain_setup(id, &d->hdr);
 }
 
 static void domain_add_cpu_mon(int cpu, struct rdt_resource *r)
@@ -742,6 +747,17 @@ static int resctrl_arch_online_cpu(unsigned int cpu)
 	struct rdt_resource *r;
 
 	mutex_lock(&domain_list_lock);
+	/*
+	 * A CPU whose ERDT and CPUID L3 domain views disagree is not added to
+	 * any domain. resctrl_arch_offline_cpu() still tries to remove it when
+	 * it goes offline and warns that no domain contains it. That warning is
+	 * expected.
+	 */
+	if (!erdt_cpu_valid(cpu)) {
+		mutex_unlock(&domain_list_lock);
+		return 0;
+	}
+
 	for_each_capable_rdt_resource(r)
 		domain_add_cpu(cpu, r);
 	mutex_unlock(&domain_list_lock);
diff --git a/arch/x86/kernel/cpu/resctrl/erdt.c b/arch/x86/kernel/cpu/resctrl/erdt.c
index 0dfe5eda166c..249ba547d7c8 100644
--- a/arch/x86/kernel/cpu/resctrl/erdt.c
+++ b/arch/x86/kernel/cpu/resctrl/erdt.c
@@ -209,6 +209,115 @@ static __init bool parse_rmdd_table(struct acpi_subtbl_hdr_16 *rmdd_hdr)
 	return false;
 }
 
+bool erdt_cpu_valid(int cpu)
+{
+	struct erdt_domain_info *d, *cpu_dom = NULL;
+	int dom_id;
+
+	/* Without ERDT there is no firmware topology to disagree with. */
+	if (!erdt_enabled)
+		return true;
+
+	dom_id = get_cpu_cacheinfo_id(cpu, RESCTRL_L3_CACHE);
+	if (dom_id < 0) {
+		pr_warn(FW_BUG "Can't find L3 id for CPU:%d\n", cpu);
+		return false;
+	}
+
+	/*
+	 * Find the erdt_domain_info that contains this CPU, then bind that ERDT
+	 * domain to this CPU's L3 id. A CPU whose L3 id does not match the binding
+	 * of its ERDT domain cannot be covered by resctrl.
+	 *
+	 * For example, the CACD sub-tables report:
+	 * domain0: CPU0, CPU2, domain1: CPU1, CPU3
+	 * while CPUID/cacheinfo reports the L3 cache is shared by:
+	 * id0: CPU0, CPU1, id1: CPU2, CPU3
+	 * With the CPUs coming online in order, CPU0 binds domain0 to L3 id0 and
+	 * CPU3 binds domain1 to L3 id1, so CPU1 and CPU2 are not covered by
+	 * resctrl.
+	 */
+	list_for_each_entry(d, &domain_info_list, entry) {
+		if (cpumask_test_cpu(cpu, &d->cpu_mask)) {
+			cpu_dom = d;
+			break;
+		}
+	}
+
+	if (!cpu_dom) {
+		pr_warn(FW_BUG "Cannot find the ERDT domain which has CPU%d\n", cpu);
+		return false;
+	}
+
+	/* This ERDT domain is already bound to this CPU's L3 domain. */
+	if (cpu_dom->dom_id == dom_id)
+		return true;
+
+	/*
+	 * This ERDT domain is already bound to a different L3 domain. Rebinding it
+	 * would leave two L3 domains reading the counters of one ERDT domain, so
+	 * skip this CPU instead:
+	 * When CPU2 is brought online, domain0 is found. But then it found that
+	 * domain0's ID is 0, which is not -1(new domain), so CPU2 is ineligible.
+	 */
+	if (cpu_dom->dom_id != -1) {
+		pr_warn(FW_BUG "CPU%d's id=%d not equal to CACD domain(%*pbl) id=%d, skip this CPU\n",
+			cpu, dom_id, cpumask_pr_args(&cpu_dom->cpu_mask), cpu_dom->dom_id);
+
+		return false;
+	}
+
+	/*
+	 * A possible new binding. Check if another ERDT domain shares the same
+	 * L3 id. If yes, this is a conflict and this CPU should not be considered
+	 * by resctrl:
+	 * When CPU1 is brought online, a new domain1 is found. But then it found that
+	 * domain0's ID is 0, which is the same as CPU1's dom_id, so CPU1 is ineligible.
+	 */
+	list_for_each_entry(d, &domain_info_list, entry) {
+		if (d == cpu_dom)
+			continue;
+
+		if (d->dom_id == dom_id) {
+			pr_warn(FW_BUG "CPU%d's id=%d is already used by CACD domain(%*pbl), skip this CPU\n",
+				cpu, dom_id, cpumask_pr_args(&d->cpu_mask));
+
+			return false;
+		}
+	}
+
+	/* Eligible new binding, assign the L3 id. */
+	cpu_dom->dom_id = dom_id;
+
+	return true;
+}
+
+/*
+ * Associate ERDT table information with this domain.
+ */
+void erdt_l3_mon_domain_setup(int id, struct rdt_domain_hdr *hdr)
+{
+	struct rdt_hw_l3_mon_domain *hw_dom;
+	struct erdt_domain_info *d;
+
+	if (!erdt_enabled)
+		return;
+
+	hw_dom = resctrl_to_arch_mon_dom(container_of(hdr, struct rdt_l3_mon_domain, hdr));
+
+	list_for_each_entry(d, &domain_info_list, entry) {
+		if (d->dom_id == id) {
+			/* Assign the ERDT information to hw_dom */
+			if (hw_dom->d_info) {
+				pr_warn(FW_BUG "Duplicated ERDT domains are mapped to an existing L3 domain\n");
+				return;
+			}
+			hw_dom->d_info = d;
+			return;
+		}
+	}
+}
+
 void erdt_exit(void)
 {
 	struct erdt_domain_info *d, *tmp;
diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index 156206088372..2e8fb36ad804 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -97,14 +97,19 @@ struct rdt_hw_ctrl_domain {
  * @arch_mbm_states:	Per-event pointer to the MBM event's saved state.
  *			An MBM event's state is an array of struct arch_mbm_state
  *			indexed by RMID on x86.
+ * @d_info:		ERDT table information of this domain
  *
  * Members of this structure are accessed via helpers that provide abstraction.
  */
 struct rdt_hw_l3_mon_domain {
 	struct rdt_l3_mon_domain	d_resctrl;
 	struct arch_mbm_state		*arch_mbm_states[QOS_NUM_L3_MBM_EVENTS];
+	const struct erdt_domain_info	*d_info;
 };
 
+bool erdt_cpu_valid(int cpu);
+void erdt_l3_mon_domain_setup(int id, struct rdt_domain_hdr *hdr);
+
 static inline struct rdt_hw_ctrl_domain *resctrl_to_arch_ctrl_dom(struct rdt_ctrl_domain *r)
 {
 	return container_of(r, struct rdt_hw_ctrl_domain, d_resctrl);
-- 
2.25.1

[PATCH v8 5/9] x86/resctrl: Parse ACPI CMRC table

[Chen Yu]

The CMRC (Cache Monitoring Registers for CPU Agents Description) sub-table of
ERDT describes the MMIO registers used to read cache monitoring counters (e.g.
LLC occupancy) for a monitoring domain.

Parse each CMRC sub-table, ioremap its register window, and save a copy of the
CMRC table in the corresponding ERDT domain entry so that monitoring code can
read the counters via MMIO.

Suggested-by: Tony Luck <tony.luck@intel.com>
Signed-off-by: Chen Yu <yu.c.chen@intel.com>
Tested-by: Hongyu Ning <hongyu.ning@linux.intel.com>
---
 arch/x86/include/asm/resctrl.h         |  2 +
 arch/x86/kernel/cpu/resctrl/erdt.c     | 73 ++++++++++++++++++++++++++
 arch/x86/kernel/cpu/resctrl/internal.h |  6 ++-
 3 files changed, 80 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/resctrl.h b/arch/x86/include/asm/resctrl.h
index 8f6edcdcfd87..9a32ed418c33 100644
--- a/arch/x86/include/asm/resctrl.h
+++ b/arch/x86/include/asm/resctrl.h
@@ -49,6 +49,8 @@ DECLARE_STATIC_KEY_FALSE(rdt_enable_key);
 DECLARE_STATIC_KEY_FALSE(rdt_alloc_enable_key);
 DECLARE_STATIC_KEY_FALSE(rdt_mon_enable_key);
 
+unsigned int erdt_get_scale(void);
+
 static inline bool resctrl_arch_alloc_capable(void)
 {
 	return rdt_alloc_capable;
diff --git a/arch/x86/kernel/cpu/resctrl/erdt.c b/arch/x86/kernel/cpu/resctrl/erdt.c
index 249ba547d7c8..a8a7417c2f82 100644
--- a/arch/x86/kernel/cpu/resctrl/erdt.c
+++ b/arch/x86/kernel/cpu/resctrl/erdt.c
@@ -23,6 +23,7 @@ static LIST_HEAD(domain_info_list);
 static bool erdt_enabled;
 
 #define ERDT_VALID_VERSION		1
+#define CMRC_SUPPORTED_INDEX_FN		1
 #define RMDD_FLAG_CPU_L3_DOMAIN		BIT(0)
 
 /* Bitmask of valid sub-tables found in the first RMDD, used to ensure all RMDDs match. */
@@ -37,11 +38,26 @@ static u16 first_rmdd_domain_id;
  */
 static unsigned int erdt_max_rmid;
 
+/*
+ * Used only by the limbo handler to round resctrl_rmid_realloc_threshold.
+ * resctrl_rmid_realloc_threshold is a single global value, and
+ * resctrl_arch_round_mon_val() takes no domain argument, so a single scale has
+ * to be derived from the per-domain cmrc->up_scale. max() is chosen because the
+ * rounding is a floor: a larger scale yields a slightly lower threshold, i.e. an
+ * RMID has to drop to a slightly lower occupancy before it is reused.
+ */
+static unsigned int erdt_scale;
+
 unsigned int erdt_get_max_rmid(void)
 {
 	return erdt_max_rmid;
 }
 
+unsigned int erdt_get_scale(void)
+{
+	return erdt_scale;
+}
+
 static void __iomem *erdt_ioremap(resource_size_t base, u32 num_pages, const char *desc)
 {
 	void __iomem *addr;
@@ -71,6 +87,7 @@ static void erdt_iounmap_domain(struct erdt_domain_info *domain)
 static void cleanup_one_domain(struct erdt_domain_info *d)
 {
 	erdt_iounmap_domain(d);
+	kfree(d->cmrc);
 	kfree(d);
 }
 
@@ -105,6 +122,49 @@ static __init int cacd_init(struct acpi_subtbl_hdr_16 *subtbl,
 	return 0;
 }
 
+static __init int cmrc_init(struct acpi_subtbl_hdr_16 *subtbl,
+			    struct erdt_domain_info *domain_info)
+{
+	struct acpi_erdt_cmrc *cmrc = (struct acpi_erdt_cmrc *)subtbl;
+
+	if (cmrc->header.length < sizeof(*cmrc)) {
+		pr_warn(FW_BUG "Truncated CMRC sub-table\n");
+		return -EIO;
+	}
+
+	if (cmrc->index_fn != CMRC_SUPPORTED_INDEX_FN) {
+		pr_info("Unsupported CMRC index function %u\n", cmrc->index_fn);
+		return -EIO;
+	}
+
+	if (!cmrc->clump_size) {
+		pr_warn(FW_BUG "CMRC clump_size is zero\n");
+		return -EIO;
+	}
+
+	/* resctrl scales monitoring values with an unsigned int. */
+	if (cmrc->up_scale > UINT_MAX) {
+		pr_warn(FW_BUG "Insane CMRC up_scale value 0x%llx\n", cmrc->up_scale);
+		return -EIO;
+	}
+
+	domain_info->base[ERDT_MMIO_CMRC_BASE] =
+		erdt_ioremap(cmrc->cmt_reg_base, cmrc->cmt_reg_size, "CMRC base");
+	if (!domain_info->base[ERDT_MMIO_CMRC_BASE])
+		return -EIO;
+
+	domain_info->cmrc = kmemdup(cmrc, cmrc->header.length, GFP_KERNEL);
+	if (!domain_info->cmrc) {
+		iounmap(domain_info->base[ERDT_MMIO_CMRC_BASE]);
+		domain_info->base[ERDT_MMIO_CMRC_BASE] = NULL;
+		return -ENOMEM;
+	}
+
+	erdt_scale = max(erdt_scale, cmrc->up_scale);
+
+	return 0;
+}
+
 static inline struct acpi_subtbl_hdr_16 *rmdd_subtbl(struct acpi_erdt_rmdd *rmdd)
 {
 	return (void *)rmdd + sizeof(*rmdd);
@@ -170,6 +230,19 @@ static __init bool parse_rmdd_table(struct acpi_subtbl_hdr_16 *rmdd_hdr)
 
 			subtbl_mask |= BIT(ACPI_ERDT_TYPE_CACD);
 			break;
+		case ACPI_ERDT_TYPE_CMRC:
+			/*
+			 * Only one CMRC is supported per domain as there is no
+			 * method to distinguish different CMRCs within a domain.
+			 */
+			if (subtbl_mask & BIT(ACPI_ERDT_TYPE_CMRC))
+				break;
+
+			if (cmrc_init(subtbl, domain_info))
+				goto cleanup;
+
+			subtbl_mask |= BIT(ACPI_ERDT_TYPE_CMRC);
+			break;
 		default:
 			break;
 		}
diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index 2e8fb36ad804..26c3e0c546ad 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -24,10 +24,12 @@
 /*
  * Index into erdt_domain_info::base[] for each MMIO region.
  * @ERDT_MMIO_RMDD_CREG: RMDD control register base address
+ * @ERDT_MMIO_CMRC_BASE: CMRC monitoring register base address
  */
 enum erdt_mmio_type {
 	ERDT_MMIO_RMDD_CREG,
-	ERDT_MMIO_LAST = ERDT_MMIO_RMDD_CREG
+	ERDT_MMIO_CMRC_BASE,
+	ERDT_MMIO_LAST = ERDT_MMIO_CMRC_BASE
 };
 
 #define ERDT_MMIO_NUM_TYPES	(ERDT_MMIO_LAST + 1)
@@ -35,12 +37,14 @@ enum erdt_mmio_type {
 /**
  * struct erdt_domain_info - Per-domain ERDT information
  * @base:	Array of ioremapped MMIO region base addresses, indexed by ERDT_MMIO_*
+ * @cmrc:	Copy of the ACPI CMRC sub-table for this domain
  * @cpu_mask:	CPUs belonging to this resource management domain
  * @dom_id:	L3 cache ID shared by all CPUs in this domain (-1 if unset)
  * @entry:	Links into the global domain_info_list
  */
 struct erdt_domain_info {
 	void __iomem		*base[ERDT_MMIO_NUM_TYPES];
+	struct acpi_erdt_cmrc	*cmrc;
 	struct cpumask		cpu_mask;
 	int			dom_id;
 	struct list_head	entry;
-- 
2.25.1

[PATCH v8 6/9] x86/resctrl: Refactor the monitor read function

[Chen Yu]

Split the monitor read helper into an L3 read path and an AET (Intel Application
Energy Telemetry) read path. This makes the two distinct monitoring sources
easier to extend independently and prepares the L3 path for ERDT-based MMIO
reads.

No functional change.

Signed-off-by: Chen Yu <yu.c.chen@intel.com>
Tested-by: Hongyu Ning <hongyu.ning@linux.intel.com>
---
 arch/x86/kernel/cpu/resctrl/monitor.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c
index 3838e0a13d36..138fd420131b 100644
--- a/arch/x86/kernel/cpu/resctrl/monitor.c
+++ b/arch/x86/kernel/cpu/resctrl/monitor.c
@@ -239,9 +239,8 @@ static u64 get_corrected_val(struct rdt_resource *r, struct rdt_l3_mon_domain *d
 	return chunks * hw_res->mon_scale;
 }
 
-int resctrl_arch_rmid_read(struct rdt_resource *r, struct rdt_domain_hdr *hdr,
-			   u32 unused, u32 rmid, enum resctrl_event_id eventid,
-			   void *arch_priv, u64 *val, void *ignored)
+static int arch_l3_read_event(struct rdt_resource *r, struct rdt_domain_hdr *hdr,
+			      u32 rmid, enum resctrl_event_id eventid, u64 *val)
 {
 	struct rdt_hw_l3_mon_domain *hw_dom;
 	struct rdt_l3_mon_domain *d;
@@ -251,11 +250,6 @@ int resctrl_arch_rmid_read(struct rdt_resource *r, struct rdt_domain_hdr *hdr,
 	int cpu;
 	int ret;
 
-	resctrl_arch_rmid_read_context_check();
-
-	if (r->rid == RDT_RESOURCE_PERF_PKG)
-		return intel_aet_read_event(hdr->id, rmid, arch_priv, val);
-
 	if (!domain_header_is_valid(hdr, RESCTRL_MON_DOMAIN, RDT_RESOURCE_L3))
 		return -EINVAL;
 
@@ -281,6 +275,22 @@ int resctrl_arch_rmid_read(struct rdt_resource *r, struct rdt_domain_hdr *hdr,
 	return ret;
 }
 
+int resctrl_arch_rmid_read(struct rdt_resource *r, struct rdt_domain_hdr *hdr,
+			   u32 unused, u32 rmid, enum resctrl_event_id eventid,
+			   void *arch_priv, u64 *val, void *ignored)
+{
+	resctrl_arch_rmid_read_context_check();
+
+	switch (r->rid) {
+	case RDT_RESOURCE_L3:
+		return arch_l3_read_event(r, hdr, rmid, eventid, val);
+	case RDT_RESOURCE_PERF_PKG:
+		return intel_aet_read_event(hdr->id, rmid, arch_priv, val);
+	default:
+		return -EINVAL;
+	}
+}
+
 static int __cntr_id_read(u32 cntr_id, u64 *val)
 {
 	struct msr msr_val = {
-- 
2.25.1

[PATCH v8 7/9] fs/resctrl: Do not invoke smp_processor_id() in preemptible context

[Chen Yu]

From: Tony Luck <tony.luck@intel.com>

An event flagged with mon_evt::any_cpu is read directly in preemptible task
context instead of being dispatched to a CPU of the monitoring domain. Only
Intel AET events set this flag, so __mon_event_count() rejects an any_cpu L3
event with WARN_ON_ONCE(). The L3 read paths can therefore call
smp_processor_id() unconditionally to check that the current CPU belongs to the
domain being read.

MMIO-based LLC occupancy counters can be read from any CPU, so the L3 read
paths have to tolerate an any_cpu event. Move that check into a helper which
returns early for any_cpu events, avoiding smp_processor_id() in preemptible
context, and report a wrong CPU via rmid_read::err.

Signed-off-by: Tony Luck <tony.luck@intel.com>
Signed-off-by: Chen Yu <yu.c.chen@intel.com>
Tested-by: Hongyu Ning <hongyu.ning@linux.intel.com>
---
 fs/resctrl/monitor.c | 48 +++++++++++++++++++++++++++++++-------------
 1 file changed, 34 insertions(+), 14 deletions(-)

diff --git a/fs/resctrl/monitor.c b/fs/resctrl/monitor.c
index 73413cb128ea..3e05b0271c58 100644
--- a/fs/resctrl/monitor.c
+++ b/fs/resctrl/monitor.c
@@ -422,9 +422,37 @@ static void mbm_cntr_free(struct rdt_l3_mon_domain *d, int cntr_id)
 	memset(&d->cntr_cfg[cntr_id], 0, sizeof(*d->cntr_cfg));
 }
 
+/**
+ * cpu_on_correct_domain() - Check if current CPU is in the correct
+ *			      domain for the event.
+ * @rr: The rmid_read structure containing event and domain information.
+ *
+ * Context: Preemptible process context when @rr->evt->any_cpu is set.
+ *          Non-migratable process context (via smp_call_on_cpu()) or
+ *          non-preemptible context (via smp_call_function_any()) when
+ *          the event must be read on a specific CPU.
+ * Return: true if the current CPU can read this event, false otherwise.
+ */
+static bool cpu_on_correct_domain(struct rmid_read *rr)
+{
+	int cpu;
+
+	/* Any CPU is OK for this event */
+	if (rr->evt->any_cpu)
+		return true;
+
+	cpu = smp_processor_id();
+
+	/* Single domain. Must be on a CPU in that domain. */
+	if (rr->hdr)
+		return cpumask_test_cpu(cpu, &rr->hdr->cpu_mask);
+
+	/* Summing domains that share a cache, must be on a CPU for that cache. */
+	return cpumask_test_cpu(cpu, &rr->ci->shared_cpu_map);
+}
+
 static int __l3_mon_event_count(struct rdtgroup *rdtgrp, struct rmid_read *rr)
 {
-	int cpu = smp_processor_id();
 	u32 closid = rdtgrp->closid;
 	u32 rmid = rdtgrp->mon.rmid;
 	struct rdt_l3_mon_domain *d;
@@ -457,11 +485,6 @@ static int __l3_mon_event_count(struct rdtgroup *rdtgrp, struct rmid_read *rr)
 		return 0;
 	}
 
-	/* Reading a single domain, must be on a CPU in that domain. */
-	if (!cpumask_test_cpu(cpu, &d->hdr.cpu_mask)) {
-		rr->err = -EIO;
-		return -EINVAL;
-	}
 	if (rr->is_mbm_cntr)
 		rr->err = resctrl_arch_cntr_read(rr->r, d, closid, rmid, cntr_id,
 						 rr->evt->evtid, &tval);
@@ -479,7 +502,6 @@ static int __l3_mon_event_count(struct rdtgroup *rdtgrp, struct rmid_read *rr)
 
 static int __l3_mon_event_count_sum(struct rdtgroup *rdtgrp, struct rmid_read *rr)
 {
-	int cpu = smp_processor_id();
 	u32 closid = rdtgrp->closid;
 	u32 rmid = rdtgrp->mon.rmid;
 	struct rdt_l3_mon_domain *d;
@@ -497,12 +519,6 @@ static int __l3_mon_event_count_sum(struct rdtgroup *rdtgrp, struct rmid_read *r
 		return -EINVAL;
 	}
 
-	/* Summing domains that share a cache, must be on a CPU for that cache. */
-	if (!cpumask_test_cpu(cpu, &rr->ci->shared_cpu_map)) {
-		rr->err = -EIO;
-		return -EINVAL;
-	}
-
 	/*
 	 * Legacy files must report the sum of an event across all
 	 * domains that share the same L3 cache instance.
@@ -538,7 +554,11 @@ static int __mon_event_count(struct rdtgroup *rdtgrp, struct rmid_read *rr)
 {
 	switch (rr->r->rid) {
 	case RDT_RESOURCE_L3:
-		WARN_ON_ONCE(rr->evt->any_cpu);
+		if (!cpu_on_correct_domain(rr)) {
+			rr->err = -EIO;
+			return -EINVAL;
+		}
+
 		if (rr->hdr)
 			return __l3_mon_event_count(rdtgrp, rr);
 		else
-- 
2.25.1

[PATCH v8 8/9] x86/resctrl: Introduce erdt_cpu_has() and erdt_support()

[Chen Yu]

Introduce erdt_cpu_has() to verify if a specific RDT feature is backed by an
ERDT table. erdt_cpu_has() is derived from rdt_cpu_has(), which not only
considers firmware (ERDT table and its sub-tables) support for a feature, but
also considers userspace input like "rdt=!cmt". Similar to rdt_cpu_has(),
erdt_cpu_has() expects an x86 feature flag as parameter.

Introduce erdt_support() as a helper called by erdt_cpu_has() to check whether
the ERDT firmware supports a given feature.

erdt_support() deliberately claims no feature yet and returns false. A feature
may only be reported as ERDT backed once the low level accessors that access
its data via MMIO exist, otherwise callers would take the ERDT path and find no
implementation behind it. Features should be added to erdt_support() as their
MMIO implementations are introduced.

Signed-off-by: Chen Yu <yu.c.chen@intel.com>
Tested-by: Hongyu Ning <hongyu.ning@linux.intel.com>
---
 arch/x86/include/asm/resctrl.h         |  1 +
 arch/x86/kernel/cpu/resctrl/core.c     | 23 +++++++++++++++++++++++
 arch/x86/kernel/cpu/resctrl/erdt.c     |  5 +++++
 arch/x86/kernel/cpu/resctrl/internal.h |  1 +
 4 files changed, 30 insertions(+)

diff --git a/arch/x86/include/asm/resctrl.h b/arch/x86/include/asm/resctrl.h
index 9a32ed418c33..bcfcc8ac1fe6 100644
--- a/arch/x86/include/asm/resctrl.h
+++ b/arch/x86/include/asm/resctrl.h
@@ -49,6 +49,7 @@ DECLARE_STATIC_KEY_FALSE(rdt_enable_key);
 DECLARE_STATIC_KEY_FALSE(rdt_alloc_enable_key);
 DECLARE_STATIC_KEY_FALSE(rdt_mon_enable_key);
 
+bool erdt_cpu_has(int flag);
 unsigned int erdt_get_scale(void);
 
 static inline bool resctrl_arch_alloc_capable(void)
diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
index 3514d73a8056..ca7e67f976d5 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
@@ -897,6 +897,29 @@ bool rdt_cpu_has(int flag)
 	return ret;
 }
 
+bool erdt_cpu_has(int flag)
+{
+	struct rdt_options *o;
+	bool ret;
+
+	ret = erdt_support(flag);
+
+	if (!ret)
+		return ret;
+
+	for (o = rdt_options; o < &rdt_options[NUM_RDT_OPTIONS]; o++) {
+		if (flag == o->flag) {
+			if (o->force_off)
+				ret = false;
+			if (o->force_on)
+				ret = true;
+			break;
+		}
+	}
+
+	return ret;
+}
+
 bool resctrl_arch_is_evt_configurable(enum resctrl_event_id evt)
 {
 	if (!rdt_cpu_has(X86_FEATURE_BMEC))
diff --git a/arch/x86/kernel/cpu/resctrl/erdt.c b/arch/x86/kernel/cpu/resctrl/erdt.c
index a8a7417c2f82..400973ebf2b8 100644
--- a/arch/x86/kernel/cpu/resctrl/erdt.c
+++ b/arch/x86/kernel/cpu/resctrl/erdt.c
@@ -48,6 +48,11 @@ static unsigned int erdt_max_rmid;
  */
 static unsigned int erdt_scale;
 
+bool erdt_support(int flag)
+{
+	return false;
+}
+
 unsigned int erdt_get_max_rmid(void)
 {
 	return erdt_max_rmid;
diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index 26c3e0c546ad..ce037e017fc3 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -287,6 +287,7 @@ 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
 
+bool erdt_support(int flag);
 unsigned int erdt_get_max_rmid(void);
 int erdt_init(void);
 void erdt_exit(void);
-- 
2.25.1

[PATCH v8 9/9] x86/resctrl: Add MMIO-based LLC occupancy monitoring support

[Chen Yu]

Add erdt_mon_read() to read LLC occupancy via MMIO and use it when the platform
supports ERDT. Register the L3 occupancy event with ERDT enabled when available,
falling back to the MSR-based path otherwise.

Use the CMRC (Cache Monitoring Registers for CPU Agents Description) ACPI
sub-table to read LLC occupancy counters for each RMID via MMIO when ERDT is
enabled. Store the CMRC information in the rdt_hw_l3_mon_domain, which could be
accessed directly.

Although the occupancy counters can now be read from any CPU via MMIO, the
per-domain limbo handler is kept. A global handler would still have to iterate
over every domain and would only save one worker thread, which does not justify
maintaining two limbo handler models.

Suggested-by: Reinette Chatre <reinette.chatre@intel.com>
Signed-off-by: Chen Yu <yu.c.chen@intel.com>
Tested-by: Hongyu Ning <hongyu.ning@linux.intel.com>
---
 arch/x86/include/asm/resctrl.h         |  8 ++-
 arch/x86/kernel/cpu/resctrl/core.c     |  5 +-
 arch/x86/kernel/cpu/resctrl/erdt.c     | 74 +++++++++++++++++++++++++-
 arch/x86/kernel/cpu/resctrl/internal.h |  9 +++-
 arch/x86/kernel/cpu/resctrl/monitor.c  | 14 +++--
 5 files changed, 102 insertions(+), 8 deletions(-)

diff --git a/arch/x86/include/asm/resctrl.h b/arch/x86/include/asm/resctrl.h
index bcfcc8ac1fe6..31c2f81cd06e 100644
--- a/arch/x86/include/asm/resctrl.h
+++ b/arch/x86/include/asm/resctrl.h
@@ -135,7 +135,13 @@ static inline void __resctrl_sched_in(struct task_struct *tsk)
 
 static inline unsigned int resctrl_arch_round_mon_val(unsigned int val)
 {
-	unsigned int scale = boot_cpu_data.x86_cache_occ_scale;
+	unsigned int scale = boot_cpu_data.x86_cache_occ_scale, escale;
+
+	if (erdt_cpu_has(X86_FEATURE_CQM_OCCUP_LLC)) {
+		escale = erdt_get_scale();
+		if (escale)
+			scale = escale;
+	}
 
 	/* h/w works in units of "boot_cpu_data.x86_cache_occ_scale" */
 	val /= scale;
diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
index ca7e67f976d5..135f2eb2a5a5 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
@@ -1007,7 +1007,10 @@ static __init bool get_rdt_mon_resources(void)
 	struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_L3].r_resctrl;
 	bool ret = false;
 
-	if (rdt_cpu_has(X86_FEATURE_CQM_OCCUP_LLC)) {
+	if (erdt_cpu_has(X86_FEATURE_CQM_OCCUP_LLC)) {
+		resctrl_enable_mon_event(QOS_L3_OCCUP_EVENT_ID, true, 0, NULL);
+		ret = true;
+	} else if (rdt_cpu_has(X86_FEATURE_CQM_OCCUP_LLC)) {
 		resctrl_enable_mon_event(QOS_L3_OCCUP_EVENT_ID, false, 0, NULL);
 		ret = true;
 	}
diff --git a/arch/x86/kernel/cpu/resctrl/erdt.c b/arch/x86/kernel/cpu/resctrl/erdt.c
index 400973ebf2b8..f350a516d3d9 100644
--- a/arch/x86/kernel/cpu/resctrl/erdt.c
+++ b/arch/x86/kernel/cpu/resctrl/erdt.c
@@ -26,6 +26,10 @@ static bool erdt_enabled;
 #define CMRC_SUPPORTED_INDEX_FN		1
 #define RMDD_FLAG_CPU_L3_DOMAIN		BIT(0)
 
+/* Set in a monitoring counter when it holds no valid data to report. */
+#define UNAVAILABLE_COUNTER		BIT_ULL(63)
+#define CMRC_FLAG_UNAVAILABLE_BIT	BIT(0)
+
 /* Bitmask of valid sub-tables found in the first RMDD, used to ensure all RMDDs match. */
 static u32 valid_subtbl_mask;
 
@@ -50,7 +54,12 @@ static unsigned int erdt_scale;
 
 bool erdt_support(int flag)
 {
-	return false;
+	switch (flag) {
+	case X86_FEATURE_CQM_OCCUP_LLC:
+		return valid_subtbl_mask & BIT(ACPI_ERDT_TYPE_CMRC);
+	default:
+		return false;
+	}
 }
 
 unsigned int erdt_get_max_rmid(void)
@@ -60,7 +69,68 @@ unsigned int erdt_get_max_rmid(void)
 
 unsigned int erdt_get_scale(void)
 {
-	return erdt_scale;
+	/* Divided by snc_nodes_per_l3_cache, see erdt_read_l3_occupancy(). */
+	return erdt_scale / snc_nodes_per_l3_cache;
+}
+
+static u32 cmrc_index_function_1(struct acpi_erdt_cmrc *cmrc, u32 rmid)
+{
+	/*
+	 * MMIO_offset_for_RMID# =
+	 *   (RMID / ClumpSize) * Stride +
+	 *   (RMID % ClumpSize) * 8
+	 */
+	return (rmid / cmrc->clump_size) * cmrc->clump_stride +
+	       (rmid % cmrc->clump_size) * 8;
+}
+
+static int erdt_read_l3_occupancy(const struct erdt_domain_info *d, u32 rmid, u64 *val)
+{
+	struct acpi_erdt_cmrc *cmrc;
+	u64 l3_cmt_count;
+	u32 offset;
+
+	cmrc = d->cmrc;
+	if (!cmrc)
+		return -EIO;
+
+	offset = cmrc_index_function_1(cmrc, rmid);
+	/* Overflow of cmt_reg_size * SZ_4K already validated in erdt_ioremap(). */
+	if (offset + sizeof(u64) > (u32)cmrc->cmt_reg_size * SZ_4K)
+		return -EINVAL;
+
+	l3_cmt_count = readq(d->base[ERDT_MMIO_CMRC_BASE] + offset);
+	if ((cmrc->flags & CMRC_FLAG_UNAVAILABLE_BIT) &&
+	    (l3_cmt_count & UNAVAILABLE_COUNTER))
+		return -EINVAL;
+
+	/*
+	 * In legacy mode, scale is divided by snc_nodes_per_l3_cache to
+	 * prevent over-calculation of aggregated monitor data, do it
+	 * the same for MMIO based access.
+	 * This scaling factor might need to be revisited/tuned for future
+	 * platforms that support both SNC and MMIO-based monitoring
+	 * simultaneously.
+	 */
+	*val = l3_cmt_count * cmrc->up_scale / snc_nodes_per_l3_cache;
+
+	return 0;
+}
+
+int erdt_mon_read(struct rdt_domain_hdr *hdr, enum resctrl_event_id evtid, u32 rmid, u64 *val)
+{
+	struct rdt_hw_l3_mon_domain *hw_dom;
+	const struct erdt_domain_info *d;
+
+	hw_dom = resctrl_to_arch_mon_dom(container_of(hdr, struct rdt_l3_mon_domain, hdr));
+	d = hw_dom->d_info;
+	if (!d)
+		return -EIO;
+
+	if (evtid == QOS_L3_OCCUP_EVENT_ID)
+		return erdt_read_l3_occupancy(d, rmid, val);
+
+	return -EIO;
 }
 
 static void __iomem *erdt_ioremap(resource_size_t base, u32 num_pages, const char *desc)
diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index ce037e017fc3..e2d0d6c76b20 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -156,7 +156,11 @@ struct msr_param {
  *			which has been corrected for features like CDP.
  * @msr_base:		Base MSR address for CBMs
  * @msr_update:		Function pointer to update QOS MSRs
- * @mon_scale:		cqm counter * mon_scale = occupancy in bytes
+ * @mon_scale:		Scale factor applied to a raw counter value on the
+ *			MSR-based read path: CMT occupancy counter * mon_scale =
+ *			occupancy in bytes, and MBM chunk count * mon_scale = bytes
+ *			transferred. ERDT reads occupancy via MMIO and applies its
+ *			own firmware-provided scale instead.
  * @mbm_width:		Monitor width, to detect and correct for overflow.
  * @cdp_enabled:	CDP state of this resource
  * @mbm_cntr_assign_enabled:	ABMC feature is enabled
@@ -185,6 +189,8 @@ static inline struct rdt_hw_resource *resctrl_to_arch_res(struct rdt_resource *r
 
 extern struct rdt_hw_resource rdt_resources_all[];
 
+extern int snc_nodes_per_l3_cache;
+
 void arch_mon_domain_online(struct rdt_resource *r, struct rdt_l3_mon_domain *d);
 
 /* CPUID.(EAX=10H, ECX=ResID=1).EAX */
@@ -289,6 +295,7 @@ static inline bool intel_handle_aet_option(bool force_off, char *tok) { return f
 
 bool erdt_support(int flag);
 unsigned int erdt_get_max_rmid(void);
+int erdt_mon_read(struct rdt_domain_hdr *hdr, enum resctrl_event_id evtid, u32 rmid, u64 *val);
 int erdt_init(void);
 void erdt_exit(void);
 
diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c
index 138fd420131b..1fc02020dbc9 100644
--- a/arch/x86/kernel/cpu/resctrl/monitor.c
+++ b/arch/x86/kernel/cpu/resctrl/monitor.c
@@ -34,7 +34,7 @@ bool rdt_mon_capable;
 
 #define CF(cf)	((unsigned long)(1048576 * (cf) + 0.5))
 
-static int snc_nodes_per_l3_cache = 1;
+int snc_nodes_per_l3_cache = 1;
 
 /*
  * The correction factor table is documented in Documentation/filesystems/resctrl.rst.
@@ -283,6 +283,10 @@ int resctrl_arch_rmid_read(struct rdt_resource *r, struct rdt_domain_hdr *hdr,
 
 	switch (r->rid) {
 	case RDT_RESOURCE_L3:
+		if (eventid == QOS_L3_OCCUP_EVENT_ID &&
+		    erdt_cpu_has(X86_FEATURE_CQM_OCCUP_LLC))
+			return erdt_mon_read(hdr, eventid, rmid, val);
+
 		return arch_l3_read_event(r, hdr, rmid, eventid, val);
 	case RDT_RESOURCE_PERF_PKG:
 		return intel_aet_read_event(hdr->id, rmid, arch_priv, val);
@@ -432,12 +436,16 @@ int __init rdt_get_l3_mon_config(struct rdt_resource *r)
 	struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
 	unsigned int threshold;
 	u32 eax, ebx, ecx, edx;
+	int max_rmid;
 
 	snc_nodes_per_l3_cache = snc_get_config();
 
+	max_rmid = erdt_cpu_has(X86_FEATURE_CQM_OCCUP_LLC) ?
+			min_t(int, erdt_get_max_rmid(), boot_cpu_data.x86_cache_max_rmid) :
+			boot_cpu_data.x86_cache_max_rmid;
 	resctrl_rmid_realloc_limit = boot_cpu_data.x86_cache_size * 1024;
 	hw_res->mon_scale = boot_cpu_data.x86_cache_occ_scale / snc_nodes_per_l3_cache;
-	r->mon.num_rmid = (boot_cpu_data.x86_cache_max_rmid + 1) / snc_nodes_per_l3_cache;
+	r->mon.num_rmid = (max_rmid + 1) / snc_nodes_per_l3_cache;
 	hw_res->mbm_width = MBM_CNTR_WIDTH_BASE;
 
 	if (mbm_offset > 0 && mbm_offset <= MBM_CNTR_WIDTH_OFFSET_MAX)
@@ -456,7 +464,7 @@ int __init rdt_get_l3_mon_config(struct rdt_resource *r)
 
 	/*
 	 * Because num_rmid may not be a power of two, round the value
-	 * to the nearest multiple of hw_res->mon_scale so it matches a
+	 * to the nearest multiple of hw_res->mon_scale or mmio scale so it matches a
 	 * value the hardware will measure. mon_scale may not be a power of 2.
 	 */
 	resctrl_rmid_realloc_threshold = resctrl_arch_round_mon_val(threshold);
-- 
2.25.1

Re: [PATCH v8 2/9] x86/resctrl: Require 64-bit x86 for resctrl support

[Reinette Chatre]

Hi Chenyu,

On 9/17/26 9:48 PM, Chen Yu wrote:
> From: Tony Luck <tony.luck@intel.com>
> 
> Intel Application Energy Telemetry (AET) provides event counts in 64-bit
> registers in MMIO space. For accurate results these should be read with a
> single 64-bit load operation from the CPU.
> 
> In preparation for the removal of the separate CONFIG_X86_CPU_RESCTRL_INTEL_AET
> configuration option, move the X86_64 dependency to X86_CPU_RESCTRL.
> 
> This in line with Dave Hansen's proposal[1] for an addition to Linux
> documentation that details the state of 32-bit x86 support.
> 
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> Link: https://lore.kernel.org/all/c6ab0c24-8931-4f0d-9be6-23498ab4c38b@intel.com/ # [1]
> ---

This patch is being submitted as part of two different series. That is ok
but the versions need to be consistent. To help with this I'll not add any
comments here but instead just point to most recent review:

https://lore.kernel.org/lkml/15de2e2f-fc2e-4d06-9179-100398865624@intel.com/

Reinette

Re: [PATCH v8 3/9] x86/resctrl: Parse ACPI ERDT table and save CACD cpumask for RMDD domains

[Reinette Chatre]

Hi Chenyu,

In subject, "RMDD" stands for "Resource Management Domain Description" so "RMDD domains"
reads as "Domain Description domains". How about (another suggestion later):
	x86/resctrl: Parse ACPI ERDT table and save CACD cpumask per RMD
	
On 9/17/26 9:49 PM, Chen Yu wrote:
> There is one Enhanced RDT (ERDT) ACPI table per platform. Each Resource
> Management Domain Description (RMDD) sub-table within it describes one resource
> management domain (RMD), also known as an L3 domain, and carries MMIO base
> information for monitoring support. The CPU agents within the scope of an RMDD
> are enumerated by their x2APIC IDs in a nested CPU Agent Collection Description
> (CACD) sub-table.
> 
> Parse the RMDD sub-tables within the ERDT ACPI table and their nested CACD
> entries to construct per-domain CPU masks.

Above paragraph can be dropped since it is just duplicate of what follows.

> 
> For each RMDD, parse the associated CACD, map its x2APIC IDs to logical CPUs,
> and save the resulting CPU mask. Associate every ERDT domain with the CPUs that
> belong to it to prepare for attaching ERDT data to resctrl monitoring domains.

The last sentence is the primary motivation for this patch but it feels buried at
the end. The changelog also understates what this patch does with reader learning
about many other changes from comments in the patch. Consider an alternative
changelog below. Please do not just copy and paste but consider how it motivates
(the "why") this work and how it describes the change on a high level with the
patch providing the code details.


  x86/resctrl: Parse ACPI ERDT table and build per-RMD CPU masks

  Enhanced RDT (ERDT) exposes per-domain MMIO registers that resctrl needs
  in order to read hardware monitoring counters on the upcoming MMIO-based
  path. The kernel discovers this hardware through a single per-platform
  ERDT ACPI table.

  Each Resource Management Domain Description (RMDD) sub-table within the
  ERDT table carries the MMIO base of one resource management domain (RMD) and,
  when RMDD_FLAG_CPU_L3_DOMAIN is set, identifies the domain as a CPU-scoped
  L3 monitoring domain. The set of CPUs in the domain is listed by x2APIC ID
  in a nested CPU Agent Collection Description (CACD) sub-table.

  Walk the ERDT table's RMDD sub-tables in preparation for attaching each
  ERDT domain to a resctrl L3 monitoring domain. For each CPU-based L3 RMDD,
  ioremap its control-register region, walk its nested CACD entries,
  translate each x2APIC ID to a logical CPU, and record the result on the
  ERDT domain's erdt_domain_info. Record the largest RMID that is valid
  on every RMDD so a later reader cannot access an RMID that is
  unsupported on some domain, and require every RMDD to advertise the
  same set of sub-table types so downstream code can rely on a uniform
  shape.

> 
> Based on original work from Anil S Keshavamurthy.

This sounds like "Originally-by:" tag per Documentation/process/maintainer-tip.rst ?

> 
> Suggested-by: Tony Luck <tony.luck@intel.com>
> Suggested-by: Reinette Chatre <reinette.chatre@intel.com>
> Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> Tested-by: Hongyu Ning <hongyu.ning@linux.intel.com>
> ---
>  arch/x86/kernel/cpu/resctrl/Makefile   |   1 +
>  arch/x86/kernel/cpu/resctrl/core.c     |  16 +-
>  arch/x86/kernel/cpu/resctrl/erdt.c     | 265 +++++++++++++++++++++++++
>  arch/x86/kernel/cpu/resctrl/internal.h |  29 +++
>  4 files changed, 309 insertions(+), 2 deletions(-)
>  create mode 100644 arch/x86/kernel/cpu/resctrl/erdt.c
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/Makefile b/arch/x86/kernel/cpu/resctrl/Makefile
> index 273ddfa30836..2216ee084832 100644
> --- a/arch/x86/kernel/cpu/resctrl/Makefile
> +++ b/arch/x86/kernel/cpu/resctrl/Makefile
> @@ -2,6 +2,7 @@
>  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)		+= erdt.o

Please keep the CONFIG_X86_CPU_RESCTRL entries grouped together.

>  obj-$(CONFIG_RESCTRL_FS_PSEUDO_LOCK)	+= pseudo_lock.o
>  
>  # To allow define_trace.h's recursive include:
> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> index 55214d6fdc49..54cfdf12dfbb 100644
> --- a/arch/x86/kernel/cpu/resctrl/core.c
> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> @@ -1016,10 +1016,17 @@ static __init void check_quirks(void)
>  
>  static __init bool get_rdt_resources(void)
>  {
> +	bool succeed;

Please use "ret" to match style of get_rdt_alloc_resources() and 
get_rdt_mon_resources().

> +
> +	erdt_init();

Why does erdt_init() have a return value when it is deliberately ignored?

>  	rdt_alloc_capable = get_rdt_alloc_resources();
>  	rdt_mon_capable = get_rdt_mon_resources();
>  
> -	return (rdt_mon_capable || rdt_alloc_capable);
> +	succeed = (rdt_mon_capable || rdt_alloc_capable);
> +	if (!succeed)
> +		erdt_exit();
> +
> +	return succeed;
>  }
>  
>  static __init void rdt_init_res_defs_intel(void)
> @@ -1141,12 +1148,15 @@ static int __init resctrl_arch_late_init(void)
>  				  "x86/resctrl/cat:online:",
>  				  resctrl_arch_online_cpu,
>  				  resctrl_arch_offline_cpu);
> -	if (state < 0)
> +	if (state < 0) {
> +		erdt_exit();
>  		return state;
> +	}
>  
>  	ret = resctrl_init();
>  	if (ret) {
>  		cpuhp_remove_state(state);
> +		erdt_exit();
>  		return ret;
>  	}
>  	rdt_online = state;
> @@ -1169,6 +1179,8 @@ static void __exit resctrl_arch_exit(void)
>  	cpuhp_remove_state(rdt_online);
>  
>  	resctrl_exit();
> +
> +	erdt_exit();
>  }
>  
>  __exitcall(resctrl_arch_exit);
> diff --git a/arch/x86/kernel/cpu/resctrl/erdt.c b/arch/x86/kernel/cpu/resctrl/erdt.c
> new file mode 100644
> index 000000000000..0dfe5eda166c
> --- /dev/null
> +++ b/arch/x86/kernel/cpu/resctrl/erdt.c
> @@ -0,0 +1,265 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Enhanced Resource Director Technology (ERDT)
> + *
> + * Copyright (C) 2026 Intel Corporation
> + *
> + */
> +
> +#define pr_fmt(fmt)     "resctrl: " fmt

The pr_... messages inconsistently also add "ERDT:" prefix. Would adding "ERDT:"
prefix here help?

> +
> +#include <linux/acpi.h>
> +#include <linux/overflow.h>
> +#include <linux/resctrl.h>
> +#include <linux/sizes.h>
> +
> +#include <asm/apic.h>
> +
> +#include "internal.h"
> +
> +static LIST_HEAD(domain_info_list);
> +
> +/* True when the ERDT ACPI table describes at least one domain with at least one CPU. */
> +static bool erdt_enabled;
> +
> +#define ERDT_VALID_VERSION		1

How is "version" different from "revision", which is term used for variable this
is compared against as well as debug message?

> +#define RMDD_FLAG_CPU_L3_DOMAIN		BIT(0)
> +
> +/* Bitmask of valid sub-tables found in the first RMDD, used to ensure all RMDDs match. */
> +static u32 valid_subtbl_mask;
> +
> +/* Domain ID of the first RMDD that established @valid_subtbl_mask, for diagnostics. */
> +static u16 first_rmdd_domain_id;
> +
> +/*
> + * The minimal max-rmid of different domains. Using minimal is to avoid the domain with

If "max-rmid" is intended to refer to the struct member then it should
be grep friendly "max_rmid".

> + * small rmid accessing an invalid rmid.

Please use upper case for acronyms.

> + */
> +static unsigned int erdt_max_rmid;
> +
> +unsigned int erdt_get_max_rmid(void)
> +{
> +	return erdt_max_rmid;
> +}
> +
> +static void __iomem *erdt_ioremap(resource_size_t base, u32 num_pages, const char *desc)
> +{
> +	void __iomem *addr;
> +	unsigned long size;
> +
> +	if (check_mul_overflow(num_pages, SZ_4K, &size))
> +		return NULL;
> +
> +	addr = ioremap(base, size);
> +	if (!addr)
> +		pr_warn(FW_BUG "ERDT: Failed to map %s at phys addr %pa (size: %u pages)\n",
> +			desc, &base, num_pages);

This series introduces a lot ot pr_warn() messages and it looks to me as though some of them
could be triggered multiple times? Could you please consider all these instances and use
appropriate variant based on how frequently it can be triggered? For example, pr_warn_once()
when the same identical message can be triggered by user, pr_warn_ratelimited() when there
are different scenarios needing visibility, leaving pr_warn() for when it is really a one-off
message.

> +
> +	return addr;
> +}
> +
> +static void erdt_iounmap_domain(struct erdt_domain_info *domain)
> +{
> +	for (int i = 0; i < ERDT_MMIO_NUM_TYPES; i++) {
> +		if (domain->base[i]) {
> +			iounmap(domain->base[i]);
> +			domain->base[i] = NULL;
> +		}
> +	}
> +}
> +
> +static void cleanup_one_domain(struct erdt_domain_info *d)
> +{
> +	erdt_iounmap_domain(d);
> +	kfree(d);
> +}
> +
> +/*
> + * Save CACD information for this RMDD:
> + * convert the X2APIC to CPU and save them in a mask.
> + */
> +static __init int cacd_init(struct acpi_subtbl_hdr_16 *subtbl,

What is the use of cacd_init() making an effort to return errno values
when caller does not use return code and all related code use bool?

> +			    struct erdt_domain_info *domain_info)
> +{
> +	struct acpi_erdt_cacd *cacd = (struct acpi_erdt_cacd *)subtbl;
> +	unsigned int num_ids;
> +	int cpu;
> +
> +	if (cacd->header.length < struct_size(cacd, X2APICIDS, 1)) {
> +		pr_warn(FW_BUG "Invalid x2apicid CACD table\n");
> +		return -EIO;
> +	}
> +
> +	num_ids = (cacd->header.length - sizeof(*cacd)) / sizeof(cacd->X2APICIDS[0]);
> +
> +	for (unsigned int i = 0; i < num_ids; i++) {
> +		cpu = topo_lookup_cpuid(cacd->X2APICIDS[i]);
> +		if (cpu < 0) {
> +			pr_warn(FW_BUG "Unknown x2apicid 0x%x\n", cacd->X2APICIDS[i]);
> +			return -EIO;

The Sashiko reported issue looks real to me:
https://sashiko.dev/#/patchset/cover.1789705667.git.yu.c.chen%40intel.com?part=3

Were you able to try the example where system limits the number of processors
via maxcpus= and see if ERDT still works?

> +		}
> +
> +		cpumask_set_cpu(cpu, &domain_info->cpu_mask);
> +	}
> +
> +	return 0;
> +}
> +
> +static inline struct acpi_subtbl_hdr_16 *rmdd_subtbl(struct acpi_erdt_rmdd *rmdd)
> +{
> +	return (void *)rmdd + sizeof(*rmdd);
> +}

Could you please drop this function and instead open code this in parse_rmdd_table() to
match the style of enumerate_erdt_table() that makes the parsing easier to follow?

> +
> +static inline struct acpi_subtbl_hdr_16 *next_subtbl(struct acpi_subtbl_hdr_16 *subtbl)
> +{
> +	return (void *)subtbl + subtbl->length;
> +}
> +
> +static inline bool subtbl_valid(void *end, struct acpi_subtbl_hdr_16 *subtbl)
> +{
> +	/* Ensure the header is within bounds before dereferencing it. */
> +	if ((void *)subtbl + sizeof(*subtbl) > end)
> +		return false;
> +
> +	/* A sub-table must be at least as large as its header. */
> +	if (subtbl->length < sizeof(*subtbl))
> +		return false;
> +
> +	/* The entire sub-table (including body) must fit within the parent. */
> +	if ((void *)subtbl + subtbl->length > end)
> +		return false;
> +
> +	return true;
> +}
> +
> +static __init bool parse_rmdd_table(struct acpi_subtbl_hdr_16 *rmdd_hdr)
> +{
> +	struct acpi_erdt_rmdd *rmdd = (struct acpi_erdt_rmdd *)rmdd_hdr;
> +	struct erdt_domain_info *domain_info;
> +	struct acpi_subtbl_hdr_16 *subtbl;
> +	u32 subtbl_mask = 0;
> +
> +	if (rmdd->header.length < sizeof(*rmdd)) {
> +		pr_warn(FW_BUG "Invalid RMDD length %u bytes\n", rmdd->header.length);
> +		return false;
> +	}
> +
> +	/* Quietly ignore non-CPU-based L3 domains */
> +	if (!(rmdd->flags & RMDD_FLAG_CPU_L3_DOMAIN))
> +		return true;
> +
> +	domain_info = kzalloc_obj(*domain_info, GFP_KERNEL);
> +	if (!domain_info)
> +		return false;
> +
> +	domain_info->dom_id = -1;

Is this "-1" handling contained in this ERDT file? Could it be a named constant to
make its usage easier to find?

> +
> +	domain_info->base[ERDT_MMIO_RMDD_CREG] =
> +		erdt_ioremap(rmdd->creg_base, rmdd->creg_size, "RMDD ctrl base");
> +	if (!domain_info->base[ERDT_MMIO_RMDD_CREG])
> +		goto cleanup;
> +
> +	for (subtbl = rmdd_subtbl(rmdd);
> +	     subtbl_valid((void *)rmdd + rmdd->header.length, subtbl);
> +	     subtbl = next_subtbl(subtbl)) {
> +		switch (subtbl->type) {
> +		/* An RMDD table has one or more CACD sub-table(s) */
> +		case ACPI_ERDT_TYPE_CACD:
> +			if (cacd_init(subtbl, domain_info))
> +				goto cleanup;
> +
> +			subtbl_mask |= BIT(ACPI_ERDT_TYPE_CACD);
> +			break;
> +		default:
> +			break;
> +		}
> +	}
> +
> +	if (!subtbl_mask)
> +		goto cleanup;
> +
> +	/*
> +	 * Require all RMDDs to support same set of sub-tables
> +	 */
> +	if (!valid_subtbl_mask) {
> +		valid_subtbl_mask = subtbl_mask;
> +		first_rmdd_domain_id = rmdd->domain_id;
> +	} else if (subtbl_mask != valid_subtbl_mask) {
> +		pr_warn(FW_BUG "RMDD %u sub-table set does not match the first RMDD %u\n",
> +			rmdd->domain_id, first_rmdd_domain_id);
> +		goto cleanup;
> +	}
> +
> +	if (!rmdd->max_rmid) {
> +		pr_warn(FW_BUG "Unreasonable RMDD max_rmid %u\n", rmdd->max_rmid);

Is argument needed when the value can only be zero?

> +		goto cleanup;
> +	}
> +
> +	if (!erdt_max_rmid)
> +		erdt_max_rmid = rmdd->max_rmid;
> +	else
> +		erdt_max_rmid = min(erdt_max_rmid, rmdd->max_rmid);
> +
> +	list_add(&domain_info->entry, &domain_info_list);
> +
> +	return true;
> +
> +cleanup:
> +	cleanup_one_domain(domain_info);
> +	return false;
> +}
> +
> +void erdt_exit(void)
> +{
> +	struct erdt_domain_info *d, *tmp;
> +
> +	list_for_each_entry_safe(d, tmp, &domain_info_list, entry) {
> +		list_del(&d->entry);
> +		cleanup_one_domain(d);
> +	}
> +	erdt_enabled = false;
> +	valid_subtbl_mask = 0;
> +	first_rmdd_domain_id = 0;
> +	erdt_max_rmid = 0;
> +}
> +
> +static __init int enumerate_erdt_table(struct acpi_table_header *table_hdr)
> +{
> +	struct acpi_table_erdt *erdt = (struct acpi_table_erdt *)table_hdr;
> +	struct acpi_subtbl_hdr_16 *subtbl;
> +
> +	if (erdt->header.revision != ERDT_VALID_VERSION) {
> +		pr_info("Unsupported ERDT table revision %u (expected %u)\n",
> +			erdt->header.revision, ERDT_VALID_VERSION);
> +		return -EINVAL;
> +	}
> +
> +	if (erdt->header.length < sizeof(*erdt)) {
> +		pr_warn(FW_BUG "ERDT: Invalid table length %u bytes\n", erdt->header.length);
> +		return -EINVAL;
> +	}
> +
> +	for (subtbl = (void *)erdt + sizeof(*erdt);
> +	     subtbl_valid((void *)erdt + erdt->header.length, subtbl);
> +	     subtbl = next_subtbl(subtbl)) {
> +		if (subtbl->type == ACPI_ERDT_TYPE_RMDD &&
> +		    !parse_rmdd_table(subtbl))
> +			goto cleanup;
> +	}
> +
> +	if (list_empty(&domain_info_list))
> +		goto cleanup;
> +
> +	erdt_enabled = true;
> +
> +	return 0;
> +
> +cleanup:
> +	erdt_exit();
> +	return -EINVAL;
> +}
> +
> +int __init erdt_init(void)

Please place the storage class attribute before the return type.

> +{
> +	return acpi_table_parse(ACPI_SIG_ERDT, enumerate_erdt_table);
> +}
> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> index e3cfa0c10e92..156206088372 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -21,6 +21,31 @@
>  
>  #define RMID_VAL_UNAVAIL		BIT_ULL(62)
>  
> +/*
> + * Index into erdt_domain_info::base[] for each MMIO region.
> + * @ERDT_MMIO_RMDD_CREG: RMDD control register base address
> + */
> +enum erdt_mmio_type {
> +	ERDT_MMIO_RMDD_CREG,
> +	ERDT_MMIO_LAST = ERDT_MMIO_RMDD_CREG
> +};
> +
> +#define ERDT_MMIO_NUM_TYPES	(ERDT_MMIO_LAST + 1)
> +
> +/**
> + * struct erdt_domain_info - Per-domain ERDT information

Since resctrl domains can be of different scope, could this highlight that
it only supports L3 scope? For example, "L3 domain ERDT information"

> + * @base:	Array of ioremapped MMIO region base addresses, indexed by ERDT_MMIO_*
> + * @cpu_mask:	CPUs belonging to this resource management domain
> + * @dom_id:	L3 cache ID shared by all CPUs in this domain (-1 if unset)
> + * @entry:	Links into the global domain_info_list
> + */
> +struct erdt_domain_info {
> +	void __iomem		*base[ERDT_MMIO_NUM_TYPES];
> +	struct cpumask		cpu_mask;
> +	int			dom_id;
> +	struct list_head	entry;
> +};
> +
>  /*
>   * With the above fields in use 62 bits remain in MSR_IA32_QM_CTR for
>   * data to be returned. The counter width is discovered from the hardware
> @@ -253,4 +278,8 @@ 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
>  
> +unsigned int erdt_get_max_rmid(void);
> +int erdt_init(void);
> +void erdt_exit(void);
> +
>  #endif /* _ASM_X86_RESCTRL_INTERNAL_H */

Reinette

Re: [PATCH v8 4/9] x86/resctrl: Attach ACPI ERDT information to L3 mon domain on CPU online

[Reinette Chatre]

Hi Chenyu,

On 9/17/26 9:50 PM, Chen Yu wrote:
> Reading LLC occupancy counters via MMIO requires the per-domain ERDT
> information, parsed from the ACPI ERDT table, to be reachable from the resctrl
> L3 monitoring domain. Nothing links the two yet, so the monitoring code cannot
> locate the MMIO registers of a domain.

Last sentence sets this change up as a bugfix when it is actually a preparatory patch.

> 
> ERDT and CPUID enumerate CPU-to-L3-domain membership independently: CPUID leaf 4
> describes the L3 cache topology, while the firmware CACD sub-table lists the
> CPUs of each ERDT domain. Both views must agree on a CPU's L3 domain for that
> CPU to be monitored safely.
> 
> When a CPU comes online, validate that firmware and CPUID agree on its L3 domain
> before adding it to any resctrl domain. Exclude the CPU from all resctrl domains
> on a mismatch because a topology inconsistency between ERDT and CPUID indicates
> a firmware defect that makes the CPU's domain placement unreliable for any
> resource. Otherwise attach the matching ERDT domain information to the L3
> monitoring domain so that monitoring data can be read via ERDT and its
> sub-tables.
> 
> Suggested-by: Reinette Chatre <reinette.chatre@intel.com>
> Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> Tested-by: Hongyu Ning <hongyu.ning@linux.intel.com>
> ---
>  arch/x86/kernel/cpu/resctrl/core.c     |  16 ++++
>  arch/x86/kernel/cpu/resctrl/erdt.c     | 109 +++++++++++++++++++++++++
>  arch/x86/kernel/cpu/resctrl/internal.h |   5 ++
>  3 files changed, 130 insertions(+)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> index 54cfdf12dfbb..3514d73a8056 100644
> --- a/arch/x86/kernel/cpu/resctrl/core.c
> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> @@ -34,6 +34,9 @@
>   * the domain list must either take cpus_read_lock(), or rely on an RCU
>   * read-side critical section, to avoid observing concurrent modification.
>   * All writers take this mutex:

Above sentence ends with ":" since the definition used to follow it, adding
text below it breaks this reference. 

> + *
> + * This mutex also protects the ERDT domain_info_list, which is modified when a
> + * CPU comes online.

Please read the comment that is above this added line ...

>   */
>  static DEFINE_MUTEX(domain_list_lock);
>  
> @@ -564,6 +567,8 @@ static void l3_mon_domain_setup(int cpu, int id, struct rdt_resource *r, struct
>  		return;
>  	}
>  	list_add_tail_rcu(&d->hdr.list, add_pos);
> +
> +	erdt_l3_mon_domain_setup(id, &d->hdr);

... the comment above domain_list_lock's definition explains how the domain list
is managed between resctrl fs and the architecture. Even though this change is made
with domain_list_lock held the above setup _after_ adding the domain to the RCU list
changes the resctrl monitoring domain _after_ it is made available to resctrl filesystem
via the RCU list.

This issue was also flagged by sashiko:
https://sashiko.dev/#/patchset/cover.1789705667.git.yu.c.chen%40intel.com?part=9


Apart from above I think this is the first hint that SNC and ERDT is not quite
integrated. Sashiko also found a couple of SNC vs ERDT sticky points.
For above, please consider that when SNC is enabled then resctrl sets the scope
of the L3 resource's monitoring domains to be RESCTRL_L3_NODE. That means that
@id parameter of l3_mon_domain_setup() could be the NUMA node ID, not L3 cache ID.

erdt_l3_mon_domain_setup() seems to assume an L3 cache ID and just searches for
a matching ERDT domain.

>  }
>  
>  static void domain_add_cpu_mon(int cpu, struct rdt_resource *r)
> @@ -742,6 +747,17 @@ static int resctrl_arch_online_cpu(unsigned int cpu)
>  	struct rdt_resource *r;
>  
>  	mutex_lock(&domain_list_lock);
> +	/*
> +	 * A CPU whose ERDT and CPUID L3 domain views disagree is not added to
> +	 * any domain. resctrl_arch_offline_cpu() still tries to remove it when
> +	 * it goes offline and warns that no domain contains it. That warning is
> +	 * expected.
> +	 */
> +	if (!erdt_cpu_valid(cpu)) {
> +		mutex_unlock(&domain_list_lock);
> +		return 0;
> +	}
> +
>  	for_each_capable_rdt_resource(r)
>  		domain_add_cpu(cpu, r);
>  	mutex_unlock(&domain_list_lock);
> diff --git a/arch/x86/kernel/cpu/resctrl/erdt.c b/arch/x86/kernel/cpu/resctrl/erdt.c
> index 0dfe5eda166c..249ba547d7c8 100644
> --- a/arch/x86/kernel/cpu/resctrl/erdt.c
> +++ b/arch/x86/kernel/cpu/resctrl/erdt.c
> @@ -209,6 +209,115 @@ static __init bool parse_rmdd_table(struct acpi_subtbl_hdr_16 *rmdd_hdr)
>  	return false;
>  }
>  
> +bool erdt_cpu_valid(int cpu)

The function name "erdt_cpu_valid()" that returns a bool creates impression that it
just does a validity check without changing any state. This function does more than this.
How about something like "erdt_try_bind_cpu(int cpu)" and then the supporting text in changelog
can be "... validate and bind the CPU to its ERDT domain ...".

> +{
> +	struct erdt_domain_info *d, *cpu_dom = NULL;
> +	int dom_id;
> +
> +	/* Without ERDT there is no firmware topology to disagree with. */
> +	if (!erdt_enabled)
> +		return true;
> +
> +	dom_id = get_cpu_cacheinfo_id(cpu, RESCTRL_L3_CACHE);
> +	if (dom_id < 0) {
> +		pr_warn(FW_BUG "Can't find L3 id for CPU:%d\n", cpu);
> +		return false;
> +	}
> +
> +	/*
> +	 * Find the erdt_domain_info that contains this CPU, then bind that ERDT
> +	 * domain to this CPU's L3 id. A CPU whose L3 id does not match the binding
> +	 * of its ERDT domain cannot be covered by resctrl.
> +	 *
> +	 * For example, the CACD sub-tables report:
> +	 * domain0: CPU0, CPU2, domain1: CPU1, CPU3
> +	 * while CPUID/cacheinfo reports the L3 cache is shared by:
> +	 * id0: CPU0, CPU1, id1: CPU2, CPU3
> +	 * With the CPUs coming online in order, CPU0 binds domain0 to L3 id0 and
> +	 * CPU3 binds domain1 to L3 id1, so CPU1 and CPU2 are not covered by
> +	 * resctrl.
> +	 */

Please move this comment block to be the comment of the entire function. It does not
have to be kernel-doc but it could borrow some of the style, for example, "this CPU"
can be @cpu to make it clear it refers to the function parameter.

The "With the CPUs coming online in order" also seems to describe whole function
and not just the snippet below it. With the function comment describing the example
entirely the smaller snippets within functions can refer to it more coherently.

> +	list_for_each_entry(d, &domain_info_list, entry) {
> +		if (cpumask_test_cpu(cpu, &d->cpu_mask)) {
> +			cpu_dom = d;
> +			break;
> +		}
> +	}
> +
> +	if (!cpu_dom) {
> +		pr_warn(FW_BUG "Cannot find the ERDT domain which has CPU%d\n", cpu);
> +		return false;
> +	}
> +
> +	/* This ERDT domain is already bound to this CPU's L3 domain. */
> +	if (cpu_dom->dom_id == dom_id)
> +		return true;
> +
> +	/*
> +	 * This ERDT domain is already bound to a different L3 domain. Rebinding it
> +	 * would leave two L3 domains reading the counters of one ERDT domain, so
> +	 * skip this CPU instead:
> +	 * When CPU2 is brought online, domain0 is found. But then it found that
> +	 * domain0's ID is 0, which is not -1(new domain), so CPU2 is ineligible.
> +	 */
> +	if (cpu_dom->dom_id != -1) {
> +		pr_warn(FW_BUG "CPU%d's id=%d not equal to CACD domain(%*pbl) id=%d, skip this CPU\n",
> +			cpu, dom_id, cpumask_pr_args(&cpu_dom->cpu_mask), cpu_dom->dom_id);
> +
> +		return false;
> +	}
> +
> +	/*
> +	 * A possible new binding. Check if another ERDT domain shares the same
> +	 * L3 id. If yes, this is a conflict and this CPU should not be considered
> +	 * by resctrl:
> +	 * When CPU1 is brought online, a new domain1 is found. But then it found that

Above is mixing tense

> +	 * domain0's ID is 0, which is the same as CPU1's dom_id, so CPU1 is ineligible.
> +	 */
> +	list_for_each_entry(d, &domain_info_list, entry) {
> +		if (d == cpu_dom)
> +			continue;
> +
> +		if (d->dom_id == dom_id) {
> +			pr_warn(FW_BUG "CPU%d's id=%d is already used by CACD domain(%*pbl), skip this CPU\n",
> +				cpu, dom_id, cpumask_pr_args(&d->cpu_mask));
> +
> +			return false;
> +		}
> +	}
> +
> +	/* Eligible new binding, assign the L3 id. */
> +	cpu_dom->dom_id = dom_id;
> +
> +	return true;
> +}
> +
> +/*
> + * Associate ERDT table information with this domain.
> + */
> +void erdt_l3_mon_domain_setup(int id, struct rdt_domain_hdr *hdr)

@id is unnecessary, function can just use hdr->id (but keep above comment about
@id not always being an L3 cache ID in mind).

> +{
> +	struct rdt_hw_l3_mon_domain *hw_dom;
> +	struct erdt_domain_info *d;
> +
> +	if (!erdt_enabled)
> +		return;
> +
> +	hw_dom = resctrl_to_arch_mon_dom(container_of(hdr, struct rdt_l3_mon_domain, hdr));
> +
> +	list_for_each_entry(d, &domain_info_list, entry) {
> +		if (d->dom_id == id) {
> +			/* Assign the ERDT information to hw_dom */

This comment is just duplicate of function comment and does not add any information
to the code it aims to describe.

> +			if (hw_dom->d_info) {

Is this necessary? hw_dom has just been kzalloc'ed so it cannot have any value here 

> +				pr_warn(FW_BUG "Duplicated ERDT domains are mapped to an existing L3 domain\n");
> +				return;
> +			}
> +			hw_dom->d_info = d;
> +			return;
> +		}
> +	}
> +}
> +
>  void erdt_exit(void)
>  {
>  	struct erdt_domain_info *d, *tmp;
> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> index 156206088372..2e8fb36ad804 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -97,14 +97,19 @@ struct rdt_hw_ctrl_domain {
>   * @arch_mbm_states:	Per-event pointer to the MBM event's saved state.
>   *			An MBM event's state is an array of struct arch_mbm_state
>   *			indexed by RMID on x86.
> + * @d_info:		ERDT table information of this domain
>   *
>   * Members of this structure are accessed via helpers that provide abstraction.
>   */
>  struct rdt_hw_l3_mon_domain {
>  	struct rdt_l3_mon_domain	d_resctrl;
>  	struct arch_mbm_state		*arch_mbm_states[QOS_NUM_L3_MBM_EVENTS];
> +	const struct erdt_domain_info	*d_info;
>  };
>  
> +bool erdt_cpu_valid(int cpu);
> +void erdt_l3_mon_domain_setup(int id, struct rdt_domain_hdr *hdr);
> +

Why did these two erdt related prototypes land here instead of with the
other erdt related prototypes?

>  static inline struct rdt_hw_ctrl_domain *resctrl_to_arch_ctrl_dom(struct rdt_ctrl_domain *r)
>  {
>  	return container_of(r, struct rdt_hw_ctrl_domain, d_resctrl);

Reinette

Re: [PATCH v8 5/9] x86/resctrl: Parse ACPI CMRC table

[Reinette Chatre]

Hi Chenyu,

On 9/17/26 9:50 PM, Chen Yu wrote:
> diff --git a/arch/x86/include/asm/resctrl.h b/arch/x86/include/asm/resctrl.h
> index 8f6edcdcfd87..9a32ed418c33 100644
> --- a/arch/x86/include/asm/resctrl.h
> +++ b/arch/x86/include/asm/resctrl.h
> @@ -49,6 +49,8 @@ DECLARE_STATIC_KEY_FALSE(rdt_enable_key);
>  DECLARE_STATIC_KEY_FALSE(rdt_alloc_enable_key);
>  DECLARE_STATIC_KEY_FALSE(rdt_mon_enable_key);
>  
> +unsigned int erdt_get_scale(void);
> +

Adding this prototype to asm header file seems out of place. One needs to look
at later patches to learn this is because of upcoming use in
resctrl_arch_round_mon_val(). Beyond that, resctrl_arch_round_mon_val() also
later needs erdt_cpu_has() that even more looks like the wrong thing to do
when it comes to the asm header file.

resctrl_arch_round_mon_val() is used in two places, during system initialization
and when user space updates resctrl_rmid_realloc_threshold via a write to
"max_threshold_occupancy". Neither is a hot path requiring this to be inline
code. 

Aiming to keep resctrl_arch_round_mon_val() as an inline function is causing this
ERDT support to be unnecessarily fragmented. Could you please add a preparatory
patch that moves resctrl_arch_round_mon_val() to a c file and add its prototype
to include/linux/resctrl.h? This means that a change to MPAM driver is also needed
that I do not expect objection against. To make this easier it would help to
place the stub among the more stable resctrl_arch_* calls in MPAM driver.

>  static inline bool resctrl_arch_alloc_capable(void)
>  {
>  	return rdt_alloc_capable;
> diff --git a/arch/x86/kernel/cpu/resctrl/erdt.c b/arch/x86/kernel/cpu/resctrl/erdt.c
> index 249ba547d7c8..a8a7417c2f82 100644
> --- a/arch/x86/kernel/cpu/resctrl/erdt.c
> +++ b/arch/x86/kernel/cpu/resctrl/erdt.c
> @@ -23,6 +23,7 @@ static LIST_HEAD(domain_info_list);
>  static bool erdt_enabled;
>  
>  #define ERDT_VALID_VERSION		1
> +#define CMRC_SUPPORTED_INDEX_FN		1
>  #define RMDD_FLAG_CPU_L3_DOMAIN		BIT(0)
>  
>  /* Bitmask of valid sub-tables found in the first RMDD, used to ensure all RMDDs match. */
> @@ -37,11 +38,26 @@ static u16 first_rmdd_domain_id;
>   */
>  static unsigned int erdt_max_rmid;
>  
> +/*
> + * Used only by the limbo handler to round resctrl_rmid_realloc_threshold.

This implies erdt_scale is used by limbo handler but limbo handler only uses
resctrl_rmid_realloc_threshold directly, no?

> + * resctrl_rmid_realloc_threshold is a single global value, and
> + * resctrl_arch_round_mon_val() takes no domain argument, so a single scale has
> + * to be derived from the per-domain cmrc->up_scale. max() is chosen because the

This does not sound right. Using the fact that a function does not take an argument as
a motivation just makes one wonder why the function cannot just be changed?

"resctrl_rmid_realloc_threshold is a single global value" is accurate and the reason
why it needs to stay that way is because it is exposed to user space as such. That
was done before RDT introduced per domain scaling. If keeping it a global is ok for
ERDT then please highlight this, otherwise resctrl needs an enhancement.

Apart from above it looks like introduction of erdt_scale and erdt_get_scale() would
benefit from a separate commit. The comment above clearly notes its complexity but
there is no mention of it in changelog. 

> + * rounding is a floor: a larger scale yields a slightly lower threshold, i.e. an
> + * RMID has to drop to a slightly lower occupancy before it is reused.
> + */
> +static unsigned int erdt_scale;
> +
>  unsigned int erdt_get_max_rmid(void)
>  {
>  	return erdt_max_rmid;
>  }
>  
> +unsigned int erdt_get_scale(void)
> +{
> +	return erdt_scale;
> +}
> +
>  static void __iomem *erdt_ioremap(resource_size_t base, u32 num_pages, const char *desc)
>  {
>  	void __iomem *addr;
> @@ -71,6 +87,7 @@ static void erdt_iounmap_domain(struct erdt_domain_info *domain)
>  static void cleanup_one_domain(struct erdt_domain_info *d)
>  {
>  	erdt_iounmap_domain(d);
> +	kfree(d->cmrc);
>  	kfree(d);
>  }
>  
> @@ -105,6 +122,49 @@ static __init int cacd_init(struct acpi_subtbl_hdr_16 *subtbl,
>  	return 0;
>  }
>  
> +static __init int cmrc_init(struct acpi_subtbl_hdr_16 *subtbl,
> +			    struct erdt_domain_info *domain_info)

Same comment as for cacd_init().

> +{
> +	struct acpi_erdt_cmrc *cmrc = (struct acpi_erdt_cmrc *)subtbl;
> +
> +	if (cmrc->header.length < sizeof(*cmrc)) {
> +		pr_warn(FW_BUG "Truncated CMRC sub-table\n");
> +		return -EIO;
> +	}
> +
> +	if (cmrc->index_fn != CMRC_SUPPORTED_INDEX_FN) {
> +		pr_info("Unsupported CMRC index function %u\n", cmrc->index_fn);
> +		return -EIO;
> +	}
> +
> +	if (!cmrc->clump_size) {
> +		pr_warn(FW_BUG "CMRC clump_size is zero\n");
> +		return -EIO;
> +	}
> +
> +	/* resctrl scales monitoring values with an unsigned int. */
> +	if (cmrc->up_scale > UINT_MAX) {
> +		pr_warn(FW_BUG "Insane CMRC up_scale value 0x%llx\n", cmrc->up_scale);
> +		return -EIO;
> +	}
> +
> +	domain_info->base[ERDT_MMIO_CMRC_BASE] =
> +		erdt_ioremap(cmrc->cmt_reg_base, cmrc->cmt_reg_size, "CMRC base");
> +	if (!domain_info->base[ERDT_MMIO_CMRC_BASE])
> +		return -EIO;
> +
> +	domain_info->cmrc = kmemdup(cmrc, cmrc->header.length, GFP_KERNEL);
> +	if (!domain_info->cmrc) {
> +		iounmap(domain_info->base[ERDT_MMIO_CMRC_BASE]);
> +		domain_info->base[ERDT_MMIO_CMRC_BASE] = NULL;
> +		return -ENOMEM;
> +	}
> +
> +	erdt_scale = max(erdt_scale, cmrc->up_scale);
> +
> +	return 0;
> +}
> +
>  static inline struct acpi_subtbl_hdr_16 *rmdd_subtbl(struct acpi_erdt_rmdd *rmdd)
>  {
>  	return (void *)rmdd + sizeof(*rmdd);
> @@ -170,6 +230,19 @@ static __init bool parse_rmdd_table(struct acpi_subtbl_hdr_16 *rmdd_hdr)
>  
>  			subtbl_mask |= BIT(ACPI_ERDT_TYPE_CACD);
>  			break;
> +		case ACPI_ERDT_TYPE_CMRC:
> +			/*
> +			 * Only one CMRC is supported per domain as there is no
> +			 * method to distinguish different CMRCs within a domain.
> +			 */

Please note how this comment style is different from comment used to describe parsing
of other RMDD sub-tables (before or after "case"). Please stick one style and use it
consistently.

> +			if (subtbl_mask & BIT(ACPI_ERDT_TYPE_CMRC))
> +				break;
> +
> +			if (cmrc_init(subtbl, domain_info))
> +				goto cleanup;
> +
> +			subtbl_mask |= BIT(ACPI_ERDT_TYPE_CMRC);
> +			break;
>  		default:
>  			break;
>  		}
> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> index 2e8fb36ad804..26c3e0c546ad 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -24,10 +24,12 @@
>  /*
>   * Index into erdt_domain_info::base[] for each MMIO region.
>   * @ERDT_MMIO_RMDD_CREG: RMDD control register base address
> + * @ERDT_MMIO_CMRC_BASE: CMRC monitoring register base address
>   */
>  enum erdt_mmio_type {
>  	ERDT_MMIO_RMDD_CREG,
> -	ERDT_MMIO_LAST = ERDT_MMIO_RMDD_CREG
> +	ERDT_MMIO_CMRC_BASE,
> +	ERDT_MMIO_LAST = ERDT_MMIO_CMRC_BASE
>  };
>  
>  #define ERDT_MMIO_NUM_TYPES	(ERDT_MMIO_LAST + 1)
> @@ -35,12 +37,14 @@ enum erdt_mmio_type {
>  /**
>   * struct erdt_domain_info - Per-domain ERDT information
>   * @base:	Array of ioremapped MMIO region base addresses, indexed by ERDT_MMIO_*
> + * @cmrc:	Copy of the ACPI CMRC sub-table for this domain
>   * @cpu_mask:	CPUs belonging to this resource management domain
>   * @dom_id:	L3 cache ID shared by all CPUs in this domain (-1 if unset)
>   * @entry:	Links into the global domain_info_list
>   */
>  struct erdt_domain_info {
>  	void __iomem		*base[ERDT_MMIO_NUM_TYPES];
> +	struct acpi_erdt_cmrc	*cmrc;
>  	struct cpumask		cpu_mask;
>  	int			dom_id;
>  	struct list_head	entry;

Reinette

Re: [PATCH v8 7/9] fs/resctrl: Do not invoke smp_processor_id() in preemptible context

[Reinette Chatre]

Hi Chenyu,

On 9/17/26 9:50 PM, Chen Yu wrote:
> From: Tony Luck <tony.luck@intel.com>
> 
> An event flagged with mon_evt::any_cpu is read directly in preemptible task
> context instead of being dispatched to a CPU of the monitoring domain. Only
> Intel AET events set this flag, so __mon_event_count() rejects an any_cpu L3
> event with WARN_ON_ONCE(). The L3 read paths can therefore call
> smp_processor_id() unconditionally to check that the current CPU belongs to the
> domain being read.
> 
> MMIO-based LLC occupancy counters can be read from any CPU, so the L3 read
> paths have to tolerate an any_cpu event. Move that check into a helper which
> returns early for any_cpu events, avoiding smp_processor_id() in preemptible
> context, and report a wrong CPU via rmid_read::err.

"report a wrong CPU via rmid_read::err" is obvious from the patch.

> 
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> Tested-by: Hongyu Ning <hongyu.ning@linux.intel.com>
> ---
>  fs/resctrl/monitor.c | 48 +++++++++++++++++++++++++++++++-------------
>  1 file changed, 34 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/resctrl/monitor.c b/fs/resctrl/monitor.c
> index 73413cb128ea..3e05b0271c58 100644
> --- a/fs/resctrl/monitor.c
> +++ b/fs/resctrl/monitor.c
> @@ -422,9 +422,37 @@ static void mbm_cntr_free(struct rdt_l3_mon_domain *d, int cntr_id)
>  	memset(&d->cntr_cfg[cntr_id], 0, sizeof(*d->cntr_cfg));
>  }
>  
> +/**
> + * cpu_on_correct_domain() - Check if current CPU is in the correct
> + *			      domain for the event.

The domain is not always relevant. When considering "Return: true if the
current CPU can read this event" ... could something like "cpu_can_read_event()"
be more descriptive? 

> + * @rr: The rmid_read structure containing event and domain information.
> + *
> + * Context: Preemptible process context when @rr->evt->any_cpu is set.
> + *          Non-migratable process context (via smp_call_on_cpu()) or
> + *          non-preemptible context (via smp_call_function_any()) when
> + *          the event must be read on a specific CPU.
> + * Return: true if the current CPU can read this event, false otherwise.
> + */
> +static bool cpu_on_correct_domain(struct rmid_read *rr)
> +{
> +	int cpu;
> +
> +	/* Any CPU is OK for this event */
> +	if (rr->evt->any_cpu)
> +		return true;
> +
> +	cpu = smp_processor_id();
> +
> +	/* Single domain. Must be on a CPU in that domain. */
> +	if (rr->hdr)
> +		return cpumask_test_cpu(cpu, &rr->hdr->cpu_mask);
> +
> +	/* Summing domains that share a cache, must be on a CPU for that cache. */
> +	return cpumask_test_cpu(cpu, &rr->ci->shared_cpu_map);
> +}
> +
>  static int __l3_mon_event_count(struct rdtgroup *rdtgrp, struct rmid_read *rr)
>  {
> -	int cpu = smp_processor_id();
>  	u32 closid = rdtgrp->closid;
>  	u32 rmid = rdtgrp->mon.rmid;
>  	struct rdt_l3_mon_domain *d;
> @@ -457,11 +485,6 @@ static int __l3_mon_event_count(struct rdtgroup *rdtgrp, struct rmid_read *rr)
>  		return 0;
>  	}
>  
> -	/* Reading a single domain, must be on a CPU in that domain. */
> -	if (!cpumask_test_cpu(cpu, &d->hdr.cpu_mask)) {
> -		rr->err = -EIO;
> -		return -EINVAL;
> -	}
>  	if (rr->is_mbm_cntr)
>  		rr->err = resctrl_arch_cntr_read(rr->r, d, closid, rmid, cntr_id,
>  						 rr->evt->evtid, &tval);
> @@ -479,7 +502,6 @@ static int __l3_mon_event_count(struct rdtgroup *rdtgrp, struct rmid_read *rr)
>  
>  static int __l3_mon_event_count_sum(struct rdtgroup *rdtgrp, struct rmid_read *rr)
>  {
> -	int cpu = smp_processor_id();
>  	u32 closid = rdtgrp->closid;
>  	u32 rmid = rdtgrp->mon.rmid;
>  	struct rdt_l3_mon_domain *d;
> @@ -497,12 +519,6 @@ static int __l3_mon_event_count_sum(struct rdtgroup *rdtgrp, struct rmid_read *r
>  		return -EINVAL;
>  	}
>  
> -	/* Summing domains that share a cache, must be on a CPU for that cache. */
> -	if (!cpumask_test_cpu(cpu, &rr->ci->shared_cpu_map)) {
> -		rr->err = -EIO;
> -		return -EINVAL;
> -	}
> -
>  	/*
>  	 * Legacy files must report the sum of an event across all
>  	 * domains that share the same L3 cache instance.
> @@ -538,7 +554,11 @@ static int __mon_event_count(struct rdtgroup *rdtgrp, struct rmid_read *rr)
>  {
>  	switch (rr->r->rid) {
>  	case RDT_RESOURCE_L3:
> -		WARN_ON_ONCE(rr->evt->any_cpu);
> +		if (!cpu_on_correct_domain(rr)) {
> +			rr->err = -EIO;
> +			return -EINVAL;
> +		}
> +
>  		if (rr->hdr)
>  			return __l3_mon_event_count(rdtgrp, rr);
>  		else

Reinette

Re: [PATCH v8 8/9] x86/resctrl: Introduce erdt_cpu_has() and erdt_support()

[Reinette Chatre]

Hi Chenyu,

On 9/17/26 9:51 PM, Chen Yu wrote:
> Introduce erdt_cpu_has() to verify if a specific RDT feature is backed by an
> ERDT table. erdt_cpu_has() is derived from rdt_cpu_has(), which not only

"is derived from" - needs imperative

> considers firmware (ERDT table and its sub-tables) support for a feature, but

"rdt_cpu_has(), which not only considers firmware (ERDT table ..." reads as though
rdt_cpu_has() considers ERDT, but it does not?

> also considers userspace input like "rdt=!cmt". Similar to rdt_cpu_has(),
> erdt_cpu_has() expects an x86 feature flag as parameter.
> 
> Introduce erdt_support() as a helper called by erdt_cpu_has() to check whether
> the ERDT firmware supports a given feature.
> 
> erdt_support() deliberately claims no feature yet and returns false. A feature
> may only be reported as ERDT backed once the low level accessors that access
> its data via MMIO exist, otherwise callers would take the ERDT path and find no
> implementation behind it. Features should be added to erdt_support() as their
> MMIO implementations are introduced.
> 
> Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> Tested-by: Hongyu Ning <hongyu.ning@linux.intel.com>
> ---
>  arch/x86/include/asm/resctrl.h         |  1 +
>  arch/x86/kernel/cpu/resctrl/core.c     | 23 +++++++++++++++++++++++
>  arch/x86/kernel/cpu/resctrl/erdt.c     |  5 +++++
>  arch/x86/kernel/cpu/resctrl/internal.h |  1 +
>  4 files changed, 30 insertions(+)
> 
> diff --git a/arch/x86/include/asm/resctrl.h b/arch/x86/include/asm/resctrl.h
> index 9a32ed418c33..bcfcc8ac1fe6 100644
> --- a/arch/x86/include/asm/resctrl.h
> +++ b/arch/x86/include/asm/resctrl.h
> @@ -49,6 +49,7 @@ DECLARE_STATIC_KEY_FALSE(rdt_enable_key);
>  DECLARE_STATIC_KEY_FALSE(rdt_alloc_enable_key);
>  DECLARE_STATIC_KEY_FALSE(rdt_mon_enable_key);
>  
> +bool erdt_cpu_has(int flag);
>  unsigned int erdt_get_scale(void);

Same comment as earlier in response to the erdt_get_scale() prototype
addition.

>  
>  static inline bool resctrl_arch_alloc_capable(void)
> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> index 3514d73a8056..ca7e67f976d5 100644
> --- a/arch/x86/kernel/cpu/resctrl/core.c
> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> @@ -897,6 +897,29 @@ bool rdt_cpu_has(int flag)
>  	return ret;
>  }
>  
> +bool erdt_cpu_has(int flag)
> +{
> +	struct rdt_options *o;
> +	bool ret;
> +
> +	ret = erdt_support(flag);
> +
> +	if (!ret)
> +		return ret;
> +
> +	for (o = rdt_options; o < &rdt_options[NUM_RDT_OPTIONS]; o++) {
> +		if (flag == o->flag) {
> +			if (o->force_off)
> +				ret = false;
> +			if (o->force_on)
> +				ret = true;
> +			break;
> +		}
> +	}
> +
> +	return ret;
> +}
> +

erdt_cpu_has() is practically identical to rdt_cpu_has() just above it. how about
something like:

	static bool apply_rdt_options(int flag, bool ret)
	{
		struct rdt_options *o;

		for (o = rdt_options; o < &rdt_options[NUM_RDT_OPTIONS]; o++) {
			...
	}

	bool rdt_cpu_has(int flag)
	{
		return apply_rdt_options(flag, boot_cpu_has(flag));
	}

	bool erdt_cpu_has(int flag)
	{
		return apply_rdt_options(flag, erdt_support(flag));
	}

>  bool resctrl_arch_is_evt_configurable(enum resctrl_event_id evt)
>  {
>  	if (!rdt_cpu_has(X86_FEATURE_BMEC))
> diff --git a/arch/x86/kernel/cpu/resctrl/erdt.c b/arch/x86/kernel/cpu/resctrl/erdt.c
> index a8a7417c2f82..400973ebf2b8 100644
> --- a/arch/x86/kernel/cpu/resctrl/erdt.c
> +++ b/arch/x86/kernel/cpu/resctrl/erdt.c
> @@ -48,6 +48,11 @@ static unsigned int erdt_max_rmid;
>   */
>  static unsigned int erdt_scale;
>  
> +bool erdt_support(int flag)
> +{
> +	return false;
> +}
> +
>  unsigned int erdt_get_max_rmid(void)
>  {
>  	return erdt_max_rmid;
> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> index 26c3e0c546ad..ce037e017fc3 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -287,6 +287,7 @@ 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
>  
> +bool erdt_support(int flag);
>  unsigned int erdt_get_max_rmid(void);
>  int erdt_init(void);
>  void erdt_exit(void);

Reinette

Re: [PATCH v8 9/9] x86/resctrl: Add MMIO-based LLC occupancy monitoring support

[Reinette Chatre]

Hi Chenyu,

On 9/17/26 9:51 PM, Chen Yu wrote:
> Add erdt_mon_read() to read LLC occupancy via MMIO and use it when the platform
> supports ERDT. Register the L3 occupancy event with ERDT enabled when available,
> falling back to the MSR-based path otherwise.
> 
> Use the CMRC (Cache Monitoring Registers for CPU Agents Description) ACPI
> sub-table to read LLC occupancy counters for each RMID via MMIO when ERDT is
> enabled. Store the CMRC information in the rdt_hw_l3_mon_domain, which could be
> accessed directly.

Regarding "Store the CMRC ..." - wasn't that done in patch #5?

> 
> Although the occupancy counters can now be read from any CPU via MMIO, the
> per-domain limbo handler is kept. A global handler would still have to iterate

"per-domain limbo handler is kept" - needs imperative

> over every domain and would only save one worker thread, which does not justify

"would only save one worker thread"? this assumes that all systems will have two domains?

> maintaining two limbo handler models.
> 
> Suggested-by: Reinette Chatre <reinette.chatre@intel.com>
> Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> Tested-by: Hongyu Ning <hongyu.ning@linux.intel.com>
> ---
>  arch/x86/include/asm/resctrl.h         |  8 ++-
>  arch/x86/kernel/cpu/resctrl/core.c     |  5 +-
>  arch/x86/kernel/cpu/resctrl/erdt.c     | 74 +++++++++++++++++++++++++-
>  arch/x86/kernel/cpu/resctrl/internal.h |  9 +++-
>  arch/x86/kernel/cpu/resctrl/monitor.c  | 14 +++--
>  5 files changed, 102 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/x86/include/asm/resctrl.h b/arch/x86/include/asm/resctrl.h
> index bcfcc8ac1fe6..31c2f81cd06e 100644
> --- a/arch/x86/include/asm/resctrl.h
> +++ b/arch/x86/include/asm/resctrl.h
> @@ -135,7 +135,13 @@ static inline void __resctrl_sched_in(struct task_struct *tsk)
>  
>  static inline unsigned int resctrl_arch_round_mon_val(unsigned int val)
>  {
> -	unsigned int scale = boot_cpu_data.x86_cache_occ_scale;
> +	unsigned int scale = boot_cpu_data.x86_cache_occ_scale, escale;
> +
> +	if (erdt_cpu_has(X86_FEATURE_CQM_OCCUP_LLC)) {
> +		escale = erdt_get_scale();
> +		if (escale)
> +			scale = escale;
> +	}
>  
>  	/* h/w works in units of "boot_cpu_data.x86_cache_occ_scale" */

This comment is no longer accurate.

>  	val /= scale;
> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> index ca7e67f976d5..135f2eb2a5a5 100644
> --- a/arch/x86/kernel/cpu/resctrl/core.c
> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> @@ -1007,7 +1007,10 @@ static __init bool get_rdt_mon_resources(void)
>  	struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_L3].r_resctrl;
>  	bool ret = false;
>  
> -	if (rdt_cpu_has(X86_FEATURE_CQM_OCCUP_LLC)) {
> +	if (erdt_cpu_has(X86_FEATURE_CQM_OCCUP_LLC)) {
> +		resctrl_enable_mon_event(QOS_L3_OCCUP_EVENT_ID, true, 0, NULL);
> +		ret = true;
> +	} else if (rdt_cpu_has(X86_FEATURE_CQM_OCCUP_LLC)) {
>  		resctrl_enable_mon_event(QOS_L3_OCCUP_EVENT_ID, false, 0, NULL);
>  		ret = true;
>  	}

Would https://lore.kernel.org/lkml/20260916231320.14502-3-tony.luck@intel.com/
break this?

> diff --git a/arch/x86/kernel/cpu/resctrl/erdt.c b/arch/x86/kernel/cpu/resctrl/erdt.c
> index 400973ebf2b8..f350a516d3d9 100644
> --- a/arch/x86/kernel/cpu/resctrl/erdt.c
> +++ b/arch/x86/kernel/cpu/resctrl/erdt.c
> @@ -26,6 +26,10 @@ static bool erdt_enabled;
>  #define CMRC_SUPPORTED_INDEX_FN		1
>  #define RMDD_FLAG_CPU_L3_DOMAIN		BIT(0)
>  
> +/* Set in a monitoring counter when it holds no valid data to report. */
> +#define UNAVAILABLE_COUNTER		BIT_ULL(63)

Can this be CMRC_UNAVAILABLE_COUNTER? (or some other appropriate prefix that makes
it more specific)

> +#define CMRC_FLAG_UNAVAILABLE_BIT	BIT(0)
> +
>  /* Bitmask of valid sub-tables found in the first RMDD, used to ensure all RMDDs match. */
>  static u32 valid_subtbl_mask;
>  
> @@ -50,7 +54,12 @@ static unsigned int erdt_scale;
>  
>  bool erdt_support(int flag)
>  {
> -	return false;
> +	switch (flag) {
> +	case X86_FEATURE_CQM_OCCUP_LLC:
> +		return valid_subtbl_mask & BIT(ACPI_ERDT_TYPE_CMRC);
> +	default:
> +		return false;
> +	}
>  }
>  
>  unsigned int erdt_get_max_rmid(void)
> @@ -60,7 +69,68 @@ unsigned int erdt_get_max_rmid(void)
>  
>  unsigned int erdt_get_scale(void)
>  {
> -	return erdt_scale;
> +	/* Divided by snc_nodes_per_l3_cache, see erdt_read_l3_occupancy(). */
> +	return erdt_scale / snc_nodes_per_l3_cache;
> +}
> +


> +static u32 cmrc_index_function_1(struct acpi_erdt_cmrc *cmrc, u32 rmid)
Please add a function comment connecting cmrc_index_function_1() to CMRC_SUPPORTED_INDEX_FN
and also documents where this function comes from (eg. RDT architecture spec).

> +{
> +	/*
> +	 * MMIO_offset_for_RMID# =
> +	 *   (RMID / ClumpSize) * Stride +
> +	 *   (RMID % ClumpSize) * 8

Looks like above can just be on one line? When adding function comment about origin of
algorithm it may be more fitting to move this comment there.

> +	 */
> +	return (rmid / cmrc->clump_size) * cmrc->clump_stride +
> +	       (rmid % cmrc->clump_size) * 8;
> +}
> +
> +static int erdt_read_l3_occupancy(const struct erdt_domain_info *d, u32 rmid, u64 *val)
> +{
> +	struct acpi_erdt_cmrc *cmrc;
> +	u64 l3_cmt_count;
> +	u32 offset;
> +
> +	cmrc = d->cmrc;
> +	if (!cmrc)
> +		return -EIO;
> +
> +	offset = cmrc_index_function_1(cmrc, rmid);
> +	/* Overflow of cmt_reg_size * SZ_4K already validated in erdt_ioremap(). */
> +	if (offset + sizeof(u64) > (u32)cmrc->cmt_reg_size * SZ_4K)
> +		return -EINVAL;
> +
> +	l3_cmt_count = readq(d->base[ERDT_MMIO_CMRC_BASE] + offset);
> +	if ((cmrc->flags & CMRC_FLAG_UNAVAILABLE_BIT) &&
> +	    (l3_cmt_count & UNAVAILABLE_COUNTER))
> +		return -EINVAL;
> +
> +	/*
> +	 * In legacy mode, scale is divided by snc_nodes_per_l3_cache to
> +	 * prevent over-calculation of aggregated monitor data, do it
> +	 * the same for MMIO based access.
> +	 * This scaling factor might need to be revisited/tuned for future
> +	 * platforms that support both SNC and MMIO-based monitoring
> +	 * simultaneously.
> +	 */
> +	*val = l3_cmt_count * cmrc->up_scale / snc_nodes_per_l3_cache;

Please consider all sashiko's comments about SNC systems - from what I can tell
the comments are accurate and the SNC support needs a second look.

Reinette

Re: [PATCH v8 3/9] x86/resctrl: Parse ACPI ERDT table and save CACD cpumask for RMDD domains

[Chen Yu]

Hi Reinette,

On Mon, Sep 28, 2026 at 02:37:45PM -0700, Reinette Chatre wrote:
> 
> Hi Chenyu,
> 
> In subject, "RMDD" stands for "Resource Management Domain Description" so "RMDD domains"
> reads as "Domain Description domains". How about (another suggestion later):
> 	x86/resctrl: Parse ACPI ERDT table and save CACD cpumask per RMD
>

OK, this is more accurate with the duplicated "domains" removed. I will change it.
 	
> On 9/17/26 9:49 PM, Chen Yu wrote:
> > There is one Enhanced RDT (ERDT) ACPI table per platform. Each Resource
> > Management Domain Description (RMDD) sub-table within it describes one resource
> > management domain (RMD), also known as an L3 domain, and carries MMIO base
> > information for monitoring support. The CPU agents within the scope of an RMDD
> > are enumerated by their x2APIC IDs in a nested CPU Agent Collection Description
> > (CACD) sub-table.
> > 
> > Parse the RMDD sub-tables within the ERDT ACPI table and their nested CACD
> > entries to construct per-domain CPU masks.
> 
> Above paragraph can be dropped since it is just duplicate of what follows.
> 

OK.

> > 
> > For each RMDD, parse the associated CACD, map its x2APIC IDs to logical CPUs,
> > and save the resulting CPU mask. Associate every ERDT domain with the CPUs that
> > belong to it to prepare for attaching ERDT data to resctrl monitoring domains.
> 
> The last sentence is the primary motivation for this patch but it feels buried at
> the end. The changelog also understates what this patch does with reader learning
> about many other changes from comments in the patch. Consider an alternative
> changelog below. Please do not just copy and paste but consider how it motivates
> (the "why") this work and how it describes the change on a high level with the
> patch providing the code details.
>

OK, will rewrite this.
 
> 
>   x86/resctrl: Parse ACPI ERDT table and build per-RMD CPU masks
> 
>   Enhanced RDT (ERDT) exposes per-domain MMIO registers that resctrl needs
>   in order to read hardware monitoring counters on the upcoming MMIO-based
>   path. The kernel discovers this hardware through a single per-platform
>   ERDT ACPI table.
> 
>   Each Resource Management Domain Description (RMDD) sub-table within the
>   ERDT table carries the MMIO base of one resource management domain (RMD) and,
>   when RMDD_FLAG_CPU_L3_DOMAIN is set, identifies the domain as a CPU-scoped
>   L3 monitoring domain. The set of CPUs in the domain is listed by x2APIC ID


Maybe remove "monitoring" since RMDD is for a CPU mask, not specific to whether
the mask is a monitor or control domain.

>   in a nested CPU Agent Collection Description (CACD) sub-table.
> 
>   Walk the ERDT table's RMDD sub-tables in preparation for attaching each
>   ERDT domain to a resctrl L3 monitoring domain. For each CPU-based L3 RMDD,
>   ioremap its control-register region, walk its nested CACD entries,
>   translate each x2APIC ID to a logical CPU, and record the result on the
>   ERDT domain's erdt_domain_info. Record the largest RMID that is valid

I think it is the minimum of the largest RMIDs exposed by each RMD.

>   on every RMDD so a later reader cannot access an RMID that is
>   unsupported on some domain, and require every RMDD to advertise the
>   same set of sub-table types so downstream code can rely on a uniform
>   shape.
> 
> > 
> > Based on original work from Anil S Keshavamurthy.
> 
> This sounds like "Originally-by:" tag per Documentation/process/maintainer-tip.rst ?
>

OK, will add this tag.
 
> > 
> > Suggested-by: Tony Luck <tony.luck@intel.com>
> > Suggested-by: Reinette Chatre <reinette.chatre@intel.com>
> > Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> > Tested-by: Hongyu Ning <hongyu.ning@linux.intel.com>
> > ---
> >  arch/x86/kernel/cpu/resctrl/Makefile   |   1 +
> >  arch/x86/kernel/cpu/resctrl/core.c     |  16 +-
> >  arch/x86/kernel/cpu/resctrl/erdt.c     | 265 +++++++++++++++++++++++++
> >  arch/x86/kernel/cpu/resctrl/internal.h |  29 +++
> >  4 files changed, 309 insertions(+), 2 deletions(-)
> >  create mode 100644 arch/x86/kernel/cpu/resctrl/erdt.c
> > 
> > diff --git a/arch/x86/kernel/cpu/resctrl/Makefile b/arch/x86/kernel/cpu/resctrl/Makefile
> > index 273ddfa30836..2216ee084832 100644
> > --- a/arch/x86/kernel/cpu/resctrl/Makefile
> > +++ b/arch/x86/kernel/cpu/resctrl/Makefile
> > @@ -2,6 +2,7 @@
> >  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)		+= erdt.o
> 
> Please keep the CONFIG_X86_CPU_RESCTRL entries grouped together.
>

OK, will do.
 
> >  obj-$(CONFIG_RESCTRL_FS_PSEUDO_LOCK)	+= pseudo_lock.o
> >  
> >  # To allow define_trace.h's recursive include:
> > diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> > index 55214d6fdc49..54cfdf12dfbb 100644
> > --- a/arch/x86/kernel/cpu/resctrl/core.c
> > +++ b/arch/x86/kernel/cpu/resctrl/core.c
> > @@ -1016,10 +1016,17 @@ static __init void check_quirks(void)
> >  
> >  static __init bool get_rdt_resources(void)
> >  {
> > +	bool succeed;
> 
> Please use "ret" to match style of get_rdt_alloc_resources() and 
> get_rdt_mon_resources().
>

OK, will rename it.
 
> > +
> > +	erdt_init();
> 
> Why does erdt_init() have a return value when it is deliberately ignored?
>

Let me remove its return value and define erdt_init() as void.
 
> >  __exitcall(resctrl_arch_exit);
> > diff --git a/arch/x86/kernel/cpu/resctrl/erdt.c b/arch/x86/kernel/cpu/resctrl/erdt.c
> > new file mode 100644
> > index 000000000000..0dfe5eda166c
> > --- /dev/null
> > +++ b/arch/x86/kernel/cpu/resctrl/erdt.c
> > @@ -0,0 +1,265 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Enhanced Resource Director Technology (ERDT)
> > + *
> > + * Copyright (C) 2026 Intel Corporation
> > + *
> > + */
> > +
> > +#define pr_fmt(fmt)     "resctrl: " fmt
> 
> The pr_... messages inconsistently also add "ERDT:" prefix. Would adding "ERDT:"
> prefix here help?
>

Yes, this will remove duplicated string in each pr_...
 
> > +
> > +#include <linux/acpi.h>
> > +#include <linux/overflow.h>
> > +#include <linux/resctrl.h>
> > +#include <linux/sizes.h>
> > +
> > +#include <asm/apic.h>
> > +
> > +#include "internal.h"
> > +
> > +static LIST_HEAD(domain_info_list);
> > +
> > +/* True when the ERDT ACPI table describes at least one domain with at least one CPU. */
> > +static bool erdt_enabled;
> > +
> > +#define ERDT_VALID_VERSION		1
> 
> How is "version" different from "revision", which is term used for variable this
> is compared against as well as debug message?
>

Let me rename it to ERDT_VALID_REVISION.
 
> > +#define RMDD_FLAG_CPU_L3_DOMAIN		BIT(0)
> > +
> > +/* Bitmask of valid sub-tables found in the first RMDD, used to ensure all RMDDs match. */
> > +static u32 valid_subtbl_mask;
> > +
> > +/* Domain ID of the first RMDD that established @valid_subtbl_mask, for diagnostics. */
> > +static u16 first_rmdd_domain_id;
> > +
> > +/*
> > + * The minimal max-rmid of different domains. Using minimal is to avoid the domain with
> 
> If "max-rmid" is intended to refer to the struct member then it should
> be grep friendly "max_rmid".
>

OK, will do.
 
> > + * small rmid accessing an invalid rmid.
> 
> Please use upper case for acronyms.
>

OK, will do.
 
> > + */
> > +static unsigned int erdt_max_rmid;
> > +
> > +unsigned int erdt_get_max_rmid(void)
> > +{
> > +	return erdt_max_rmid;
> > +}
> > +
> > +static void __iomem *erdt_ioremap(resource_size_t base, u32 num_pages, const char *desc)
> > +{
> > +	void __iomem *addr;
> > +	unsigned long size;
> > +
> > +	if (check_mul_overflow(num_pages, SZ_4K, &size))
> > +		return NULL;
> > +
> > +	addr = ioremap(base, size);
> > +	if (!addr)
> > +		pr_warn(FW_BUG "ERDT: Failed to map %s at phys addr %pa (size: %u pages)\n",
> > +			desc, &base, num_pages);
> 
> This series introduces a lot ot pr_warn() messages and it looks to me as though some of them
> could be triggered multiple times? Could you please consider all these instances and use
> appropriate variant based on how frequently it can be triggered? For example, pr_warn_once()
> when the same identical message can be triggered by user, pr_warn_ratelimited() when there
> are different scenarios needing visibility, leaving pr_warn() for when it is really a one-off
> message.
>

OK, will use pr_warn() for the logic during system bootup/initialization, and use pr_warn_once()
for erdt_cpu_valid(), which could be invoked during CPU hotplug triggered by user, and
use pr_warn_once() in erdt_l3_mon_domain_setup(), because it is identical message.
 
> > +
> > +	return addr;
> > +}
> > +
> > +static void erdt_iounmap_domain(struct erdt_domain_info *domain)
> > +{
> > +	for (int i = 0; i < ERDT_MMIO_NUM_TYPES; i++) {
> > +		if (domain->base[i]) {
> > +			iounmap(domain->base[i]);
> > +			domain->base[i] = NULL;
> > +		}
> > +	}
> > +}
> > +
> > +static void cleanup_one_domain(struct erdt_domain_info *d)
> > +{
> > +	erdt_iounmap_domain(d);
> > +	kfree(d);
> > +}
> > +
> > +/*
> > + * Save CACD information for this RMDD:
> > + * convert the X2APIC to CPU and save them in a mask.
> > + */
> > +static __init int cacd_init(struct acpi_subtbl_hdr_16 *subtbl,
> 
> What is the use of cacd_init() making an effort to return errno values
> when caller does not use return code and all related code use bool?
> 

I was thinking of passing the error code up, but we don't need it. I'll change it
to return a bool.

> > +			    struct erdt_domain_info *domain_info)
> > +{
> > +	struct acpi_erdt_cacd *cacd = (struct acpi_erdt_cacd *)subtbl;
> > +	unsigned int num_ids;
> > +	int cpu;
> > +
> > +	if (cacd->header.length < struct_size(cacd, X2APICIDS, 1)) {
> > +		pr_warn(FW_BUG "Invalid x2apicid CACD table\n");
> > +		return -EIO;
> > +	}
> > +
> > +	num_ids = (cacd->header.length - sizeof(*cacd)) / sizeof(cacd->X2APICIDS[0]);
> > +
> > +	for (unsigned int i = 0; i < num_ids; i++) {
> > +		cpu = topo_lookup_cpuid(cacd->X2APICIDS[i]);
> > +		if (cpu < 0) {
> > +			pr_warn(FW_BUG "Unknown x2apicid 0x%x\n", cacd->X2APICIDS[i]);
> > +			return -EIO;
> 
> The Sashiko reported issue looks real to me:
> https://sashiko.dev/#/patchset/cover.1789705667.git.yu.c.chen%40intel.com?part=3
> 
> Were you able to try the example where system limits the number of processors
> via maxcpus= and see if ERDT still works?
>

Yes, this is a valid case, but maxcpus= should be replaced by nr_cpus=.
maxcpus= limits the online CPUs during bootup, and after bootup, those
offline CPUs can still be queried by topo_lookup_cpuid(). While for
nr_cpus=, those offline CPUs are not in the possible CPU mask, they can
not be online after bootup, and they can not be found by
topo_lookup_cpuid() either. If there is an inconsistency between the
possible_cpus_mask and the CPU x2APIC set exposed by CACD, all ERDTs will
be disabled. This is too strong. Let me skip this CPU if it cannot be found
in the possible CPU mask.
 
> > +		}
> > +
> > +		cpumask_set_cpu(cpu, &domain_info->cpu_mask);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static inline struct acpi_subtbl_hdr_16 *rmdd_subtbl(struct acpi_erdt_rmdd *rmdd)
> > +{
> > +	return (void *)rmdd + sizeof(*rmdd);
> > +}
> 
> Could you please drop this function and instead open code this in parse_rmdd_table() to
> match the style of enumerate_erdt_table() that makes the parsing easier to follow?
>

OK, will adjust it.
 
> > +static __init bool parse_rmdd_table(struct acpi_subtbl_hdr_16 *rmdd_hdr)
> > +{
> > +	struct acpi_erdt_rmdd *rmdd = (struct acpi_erdt_rmdd *)rmdd_hdr;
> > +	struct erdt_domain_info *domain_info;
> > +	struct acpi_subtbl_hdr_16 *subtbl;
> > +	u32 subtbl_mask = 0;
> > +
> > +	if (rmdd->header.length < sizeof(*rmdd)) {
> > +		pr_warn(FW_BUG "Invalid RMDD length %u bytes\n", rmdd->header.length);
> > +		return false;
> > +	}
> > +
> > +	/* Quietly ignore non-CPU-based L3 domains */
> > +	if (!(rmdd->flags & RMDD_FLAG_CPU_L3_DOMAIN))
> > +		return true;
> > +
> > +	domain_info = kzalloc_obj(*domain_info, GFP_KERNEL);
> > +	if (!domain_info)
> > +		return false;
> > +
> > +	domain_info->dom_id = -1;
> 
> Is this "-1" handling contained in this ERDT file? Could it be a named constant to
> make its usage easier to find?
>

It is only used internally in erdt.c, let me define it as
#define ERDT_DOMIAN_ID_UNSET -1

> > +
> > +	domain_info->base[ERDT_MMIO_RMDD_CREG] =
> > +		erdt_ioremap(rmdd->creg_base, rmdd->creg_size, "RMDD ctrl base");
> > +	if (!domain_info->base[ERDT_MMIO_RMDD_CREG])
> > +		goto cleanup;
> > +
> > +	for (subtbl = rmdd_subtbl(rmdd);
> > +	     subtbl_valid((void *)rmdd + rmdd->header.length, subtbl);
> > +	     subtbl = next_subtbl(subtbl)) {
> > +		switch (subtbl->type) {
> > +		/* An RMDD table has one or more CACD sub-table(s) */
> > +		case ACPI_ERDT_TYPE_CACD:
> > +			if (cacd_init(subtbl, domain_info))
> > +				goto cleanup;
> > +
> > +			subtbl_mask |= BIT(ACPI_ERDT_TYPE_CACD);
> > +			break;
> > +		default:
> > +			break;
> > +		}
> > +	}
> > +
> > +	if (!subtbl_mask)
> > +		goto cleanup;
> > +
> > +	/*
> > +	 * Require all RMDDs to support same set of sub-tables
> > +	 */
> > +	if (!valid_subtbl_mask) {
> > +		valid_subtbl_mask = subtbl_mask;
> > +		first_rmdd_domain_id = rmdd->domain_id;
> > +	} else if (subtbl_mask != valid_subtbl_mask) {
> > +		pr_warn(FW_BUG "RMDD %u sub-table set does not match the first RMDD %u\n",
> > +			rmdd->domain_id, first_rmdd_domain_id);
> > +		goto cleanup;
> > +	}
> > +
> > +	if (!rmdd->max_rmid) {
> > +		pr_warn(FW_BUG "Unreasonable RMDD max_rmid %u\n", rmdd->max_rmid);
> 
> Is argument needed when the value can only be zero?
>

Not needed, let me remove the argument and print 0 directly.
 
> > +}
> > +
> > +int __init erdt_init(void)
> 
> Please place the storage class attribute before the return type.
>

Got it, will do.
 
> > +{
> > +	return acpi_table_parse(ACPI_SIG_ERDT, enumerate_erdt_table);
> > +}
> > diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> > index e3cfa0c10e92..156206088372 100644
> > --- a/arch/x86/kernel/cpu/resctrl/internal.h
> > +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> > @@ -21,6 +21,31 @@
> >  
> >  #define RMID_VAL_UNAVAIL		BIT_ULL(62)
> >  
> > +/*
> > + * Index into erdt_domain_info::base[] for each MMIO region.
> > + * @ERDT_MMIO_RMDD_CREG: RMDD control register base address
> > + */
> > +enum erdt_mmio_type {
> > +	ERDT_MMIO_RMDD_CREG,
> > +	ERDT_MMIO_LAST = ERDT_MMIO_RMDD_CREG
> > +};
> > +
> > +#define ERDT_MMIO_NUM_TYPES	(ERDT_MMIO_LAST + 1)
> > +
> > +/**
> > + * struct erdt_domain_info - Per-domain ERDT information
> 
> Since resctrl domains can be of different scope, could this highlight that
> it only supports L3 scope? For example, "L3 domain ERDT information"
>

OK, ERDT domain only focus on L3 scope for now, let me use "L3 domain ERDT information"
in the comment.

thanks,
Chenyu 

Re: [PATCH v8 4/9] x86/resctrl: Attach ACPI ERDT information to L3 mon domain on CPU online

[Chen Yu]

Hi Reinette,

On Mon, Sep 28, 2026 at 02:44:19PM -0700, Reinette Chatre wrote:
> Hi Chenyu,
> 
> On 9/17/26 9:50 PM, Chen Yu wrote:
> > Reading LLC occupancy counters via MMIO requires the per-domain ERDT
> > information, parsed from the ACPI ERDT table, to be reachable from the resctrl
> > L3 monitoring domain. Nothing links the two yet, so the monitoring code cannot
> > locate the MMIO registers of a domain.
> 
> Last sentence sets this change up as a bugfix when it is actually a preparatory patch.
>

OK, let me rephase it to:
In preparation for MMIO-based LLC occupancy monitoring, link the per-domain ERDT information
to the resctrl L3 monitoring domain, so the MMIO read path can locate a domain's MMIO registers.
 
> > 
> > ERDT and CPUID enumerate CPU-to-L3-domain membership independently: CPUID leaf 4
> > describes the L3 cache topology, while the firmware CACD sub-table lists the
> > CPUs of each ERDT domain. Both views must agree on a CPU's L3 domain for that
> > CPU to be monitored safely.
> > 
> > When a CPU comes online, validate that firmware and CPUID agree on its L3 domain
> > before adding it to any resctrl domain. Exclude the CPU from all resctrl domains
> > on a mismatch because a topology inconsistency between ERDT and CPUID indicates
> > a firmware defect that makes the CPU's domain placement unreliable for any
> > resource. Otherwise attach the matching ERDT domain information to the L3
> > monitoring domain so that monitoring data can be read via ERDT and its
> > sub-tables.
> > 
> > Suggested-by: Reinette Chatre <reinette.chatre@intel.com>
> > Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> > Tested-by: Hongyu Ning <hongyu.ning@linux.intel.com>
> > ---
> >  arch/x86/kernel/cpu/resctrl/core.c     |  16 ++++
> >  arch/x86/kernel/cpu/resctrl/erdt.c     | 109 +++++++++++++++++++++++++
> >  arch/x86/kernel/cpu/resctrl/internal.h |   5 ++
> >  3 files changed, 130 insertions(+)
> > 
> > diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> > index 54cfdf12dfbb..3514d73a8056 100644
> > --- a/arch/x86/kernel/cpu/resctrl/core.c
> > +++ b/arch/x86/kernel/cpu/resctrl/core.c
> > @@ -34,6 +34,9 @@
> >   * the domain list must either take cpus_read_lock(), or rely on an RCU
> >   * read-side critical section, to avoid observing concurrent modification.
> >   * All writers take this mutex:
> 
> Above sentence ends with ":" since the definition used to follow it, adding
> text below it breaks this reference. 
>

OK, let me move above sentence above the "All writers take this mutex".
 
> > + *
> > + * This mutex also protects the ERDT domain_info_list, which is modified when a
> > + * CPU comes online.
> 
> Please read the comment that is above this added line ...
> 
> >   */
> >  static DEFINE_MUTEX(domain_list_lock);
> >  
> > @@ -564,6 +567,8 @@ static void l3_mon_domain_setup(int cpu, int id, struct rdt_resource *r, struct
> >  		return;
> >  	}
> >  	list_add_tail_rcu(&d->hdr.list, add_pos);
> > +
> > +	erdt_l3_mon_domain_setup(id, &d->hdr);
> 
> ... the comment above domain_list_lock's definition explains how the domain list
> is managed between resctrl fs and the architecture. Even though this change is made
> with domain_list_lock held the above setup _after_ adding the domain to the RCU list
> changes the resctrl monitoring domain _after_ it is made available to resctrl filesystem
> via the RCU list.
> 
> This issue was also flagged by sashiko:
> https://sashiko.dev/#/patchset/cover.1789705667.git.yu.c.chen%40intel.com?part=9
>

The writer l3_mon_domain_setup() not only holds domain_list_lock but also the cpus_write,
because l3_mon_domain_setup() is invoked in cpu hotplug path. And the reader  mon_event_read()
which access the monitoring domain hold cpus_read_lock(), so the race might
not be triggered. But for safety reason in case in the future there is other reader
uses list_for_each_rcu() to access the domain without the lock, we should move erdt_l3_mon_domain_setup()
before the rcu publish.

> 
> Apart from above I think this is the first hint that SNC and ERDT is not quite
> integrated. Sashiko also found a couple of SNC vs ERDT sticky points.
> For above, please consider that when SNC is enabled then resctrl sets the scope
> of the L3 resource's monitoring domains to be RESCTRL_L3_NODE. That means that
> @id parameter of l3_mon_domain_setup() could be the NUMA node ID, not L3 cache ID.
> 
> erdt_l3_mon_domain_setup() seems to assume an L3 cache ID and just searches for
> a matching ERDT domain.
>

Yes, I did not deal with SNC much in this version because it seems that SNC and the
current ERDT cannot co-exist. According to the RDT spec, Table 0-1 (Glossary, Acronym):
"RMD - Resource Management Domain: A set of features defined within a particular cache
domain, such as an L3 cache supporting a number of logical processors."
This implies that the smallest domain in ERDT is cache scope, it can not be node scope,
so dividing a single LLC into several SNC nodes is not supported by the current RMD.

If ERDT's support for SNC scope is added in the future, we can update erdt.c to handle SNC 
(hopefully the ERDT tables will contain SNC ID information).

> >  }
> >  
> >  static void domain_add_cpu_mon(int cpu, struct rdt_resource *r)
> > @@ -742,6 +747,17 @@ static int resctrl_arch_online_cpu(unsigned int cpu)
> >  	struct rdt_resource *r;
> >  
> >  	mutex_lock(&domain_list_lock);
> > +	/*
> > +	 * A CPU whose ERDT and CPUID L3 domain views disagree is not added to
> > +	 * any domain. resctrl_arch_offline_cpu() still tries to remove it when
> > +	 * it goes offline and warns that no domain contains it. That warning is
> > +	 * expected.
> > +	 */
> > +	if (!erdt_cpu_valid(cpu)) {
> > +		mutex_unlock(&domain_list_lock);
> > +		return 0;
> > +	}
> > +
> >  	for_each_capable_rdt_resource(r)
> >  		domain_add_cpu(cpu, r);
> >  	mutex_unlock(&domain_list_lock);
> > diff --git a/arch/x86/kernel/cpu/resctrl/erdt.c b/arch/x86/kernel/cpu/resctrl/erdt.c
> > index 0dfe5eda166c..249ba547d7c8 100644
> > --- a/arch/x86/kernel/cpu/resctrl/erdt.c
> > +++ b/arch/x86/kernel/cpu/resctrl/erdt.c
> > @@ -209,6 +209,115 @@ static __init bool parse_rmdd_table(struct acpi_subtbl_hdr_16 *rmdd_hdr)
> >  	return false;
> >  }
> >  
> > +bool erdt_cpu_valid(int cpu)
> 
> The function name "erdt_cpu_valid()" that returns a bool creates impression that it
> just does a validity check without changing any state. This function does more than this.
> How about something like "erdt_try_bind_cpu(int cpu)" and then the supporting text in changelog
> can be "... validate and bind the CPU to its ERDT domain ...".
>

OK, will rename the function and update the changelog.
 
> > +{
> > +	struct erdt_domain_info *d, *cpu_dom = NULL;
> > +	int dom_id;
> > +
> > +	/* Without ERDT there is no firmware topology to disagree with. */
> > +	if (!erdt_enabled)
> > +		return true;
> > +
> > +	dom_id = get_cpu_cacheinfo_id(cpu, RESCTRL_L3_CACHE);
> > +	if (dom_id < 0) {
> > +		pr_warn(FW_BUG "Can't find L3 id for CPU:%d\n", cpu);
> > +		return false;
> > +	}
> > +
> > +	/*
> > +	 * Find the erdt_domain_info that contains this CPU, then bind that ERDT
> > +	 * domain to this CPU's L3 id. A CPU whose L3 id does not match the binding
> > +	 * of its ERDT domain cannot be covered by resctrl.
> > +	 *
> > +	 * For example, the CACD sub-tables report:
> > +	 * domain0: CPU0, CPU2, domain1: CPU1, CPU3
> > +	 * while CPUID/cacheinfo reports the L3 cache is shared by:
> > +	 * id0: CPU0, CPU1, id1: CPU2, CPU3
> > +	 * With the CPUs coming online in order, CPU0 binds domain0 to L3 id0 and
> > +	 * CPU3 binds domain1 to L3 id1, so CPU1 and CPU2 are not covered by
> > +	 * resctrl.
> > +	 */
> 
> Please move this comment block to be the comment of the entire function. It does not
> have to be kernel-doc but it could borrow some of the style, for example, "this CPU"
> can be @cpu to make it clear it refers to the function parameter.
>
> The "With the CPUs coming online in order" also seems to describe whole function
> and not just the snippet below it. With the function comment describing the example
> entirely the smaller snippets within functions can refer to it more coherently.
> 

OK, will move them to be the comment of the function.

> > +
> > +	/*
> > +	 * A possible new binding. Check if another ERDT domain shares the same
> > +	 * L3 id. If yes, this is a conflict and this CPU should not be considered
> > +	 * by resctrl:
> > +	 * When CPU1 is brought online, a new domain1 is found. But then it found that
> 
> Above is mixing tense
>

OK, will fix all of them.
 
> > +	 * domain0's ID is 0, which is the same as CPU1's dom_id, so CPU1 is ineligible.
> > +	 */
> > +	list_for_each_entry(d, &domain_info_list, entry) {
> > +		if (d == cpu_dom)
> > +			continue;
> > +
> > +		if (d->dom_id == dom_id) {
> > +			pr_warn(FW_BUG "CPU%d's id=%d is already used by CACD domain(%*pbl), skip this CPU\n",
> > +				cpu, dom_id, cpumask_pr_args(&d->cpu_mask));
> > +
> > +			return false;
> > +		}
> > +	}
> > +
> > +	/* Eligible new binding, assign the L3 id. */
> > +	cpu_dom->dom_id = dom_id;
> > +
> > +	return true;
> > +}
> > +
> > +/*
> > + * Associate ERDT table information with this domain.
> > + */
> > +void erdt_l3_mon_domain_setup(int id, struct rdt_domain_hdr *hdr)
> 
> @id is unnecessary, function can just use hdr->id (but keep above comment about
> @id not always being an L3 cache ID in mind).
>

OK, will remove the @id parameter.
 
> > +{
> > +	struct rdt_hw_l3_mon_domain *hw_dom;
> > +	struct erdt_domain_info *d;
> > +
> > +	if (!erdt_enabled)
> > +		return;
> > +
> > +	hw_dom = resctrl_to_arch_mon_dom(container_of(hdr, struct rdt_l3_mon_domain, hdr));
> > +
> > +	list_for_each_entry(d, &domain_info_list, entry) {
> > +		if (d->dom_id == id) {
> > +			/* Assign the ERDT information to hw_dom */
> 
> This comment is just duplicate of function comment and does not add any information
> to the code it aims to describe.
>

OK, will remove it.
 
> > +			if (hw_dom->d_info) {
> 
> Is this necessary? hw_dom has just been kzalloc'ed so it cannot have any value here 
>

Right, this is not needed, the duplicated case has been covered in erdt_cpu_valid().
 
> > +				pr_warn(FW_BUG "Duplicated ERDT domains are mapped to an existing L3 domain\n");
> > +				return;
> > +			}
> > +			hw_dom->d_info = d;
> > +			return;
> > +		}
> > +	}
> > +}
> > +
> >  void erdt_exit(void)
> >  {
> >  	struct erdt_domain_info *d, *tmp;
> > diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> > index 156206088372..2e8fb36ad804 100644
> > --- a/arch/x86/kernel/cpu/resctrl/internal.h
> > +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> > @@ -97,14 +97,19 @@ struct rdt_hw_ctrl_domain {
> >   * @arch_mbm_states:	Per-event pointer to the MBM event's saved state.
> >   *			An MBM event's state is an array of struct arch_mbm_state
> >   *			indexed by RMID on x86.
> > + * @d_info:		ERDT table information of this domain
> >   *
> >   * Members of this structure are accessed via helpers that provide abstraction.
> >   */
> >  struct rdt_hw_l3_mon_domain {
> >  	struct rdt_l3_mon_domain	d_resctrl;
> >  	struct arch_mbm_state		*arch_mbm_states[QOS_NUM_L3_MBM_EVENTS];
> > +	const struct erdt_domain_info	*d_info;
> >  };
> >  
> > +bool erdt_cpu_valid(int cpu);
> > +void erdt_l3_mon_domain_setup(int id, struct rdt_domain_hdr *hdr);
> > +
> 
> Why did these two erdt related prototypes land here instead of with the
> other erdt related prototypes?
>

They were added near the introduction of const struct erdt_domain_info *d_info, let me move
them near other erdt_* helpers.

thanks,
Chenyu

Re: [PATCH v8 2/9] x86/resctrl: Require 64-bit x86 for resctrl support

[Chen Yu]

Hi Reinette,

On Mon, Sep 28, 2026 at 02:21:40PM -0700, Reinette Chatre wrote:
> Hi Chenyu,
> 
> On 9/17/26 9:48 PM, Chen Yu wrote:
> > From: Tony Luck <tony.luck@intel.com>
> > 
> > Intel Application Energy Telemetry (AET) provides event counts in 64-bit
> > registers in MMIO space. For accurate results these should be read with a
> > single 64-bit load operation from the CPU.
> > 
> > In preparation for the removal of the separate CONFIG_X86_CPU_RESCTRL_INTEL_AET
> > configuration option, move the X86_64 dependency to X86_CPU_RESCTRL.
> > 
> > This in line with Dave Hansen's proposal[1] for an addition to Linux
> > documentation that details the state of 32-bit x86 support.
> > 
> > Signed-off-by: Tony Luck <tony.luck@intel.com>
> > Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> > Link: https://lore.kernel.org/all/c6ab0c24-8931-4f0d-9be6-23498ab4c38b@intel.com/ # [1]
> > ---
> 
> This patch is being submitted as part of two different series. That is ok
> but the versions need to be consistent. To help with this I'll not add any
> comments here but instead just point to most recent review:
> 
> https://lore.kernel.org/lkml/15de2e2f-fc2e-4d06-9179-100398865624@intel.com/
>

OK, I'll pick up the latest one from Tony in the next version.

Thanks,
Chenyu 

Re: [PATCH v8 3/9] x86/resctrl: Parse ACPI ERDT table and save CACD cpumask for RMDD domains

[Reinette Chatre]

Hi Chenyu,

On 9/29/26 3:32 AM, Chen Yu wrote:
> On Mon, Sep 28, 2026 at 02:37:45PM -0700, Reinette Chatre wrote:
>> On 9/17/26 9:49 PM, Chen Yu wrote:


>>
>>   x86/resctrl: Parse ACPI ERDT table and build per-RMD CPU masks
>>
>>   Enhanced RDT (ERDT) exposes per-domain MMIO registers that resctrl needs
>>   in order to read hardware monitoring counters on the upcoming MMIO-based
>>   path. The kernel discovers this hardware through a single per-platform
>>   ERDT ACPI table.
>>
>>   Each Resource Management Domain Description (RMDD) sub-table within the
>>   ERDT table carries the MMIO base of one resource management domain (RMD) and,
>>   when RMDD_FLAG_CPU_L3_DOMAIN is set, identifies the domain as a CPU-scoped
>>   L3 monitoring domain. The set of CPUs in the domain is listed by x2APIC ID
> 
> 
> Maybe remove "monitoring" since RMDD is for a CPU mask, not specific to whether
> the mask is a monitor or control domain.

Sure.

> 
>>   in a nested CPU Agent Collection Description (CACD) sub-table.
>>
>>   Walk the ERDT table's RMDD sub-tables in preparation for attaching each
>>   ERDT domain to a resctrl L3 monitoring domain. For each CPU-based L3 RMDD,
>>   ioremap its control-register region, walk its nested CACD entries,
>>   translate each x2APIC ID to a logical CPU, and record the result on the
>>   ERDT domain's erdt_domain_info. Record the largest RMID that is valid
> 
> I think it is the minimum of the largest RMIDs exposed by each RMD.

This sounds the same to me? Your suggestion is closer to the math and code
while "largest RMID that is valid on every RMD"(*) describes what it actually
means? Having the changelog describe the changes and goals at high level makes
it easier to understand the low level details/code found in the patch.

(*) I just noticed the incorrect use of RMDD vs RMD in my example, highligthing
    that it should not just be copy&pasted.

> 
>>   on every RMDD so a later reader cannot access an RMID that is
>>   unsupported on some domain, and require every RMDD to advertise the
>>   same set of sub-table types so downstream code can rely on a uniform
>>   shape.
>>

...

>>> +			    struct erdt_domain_info *domain_info)
>>> +{
>>> +	struct acpi_erdt_cacd *cacd = (struct acpi_erdt_cacd *)subtbl;
>>> +	unsigned int num_ids;
>>> +	int cpu;
>>> +
>>> +	if (cacd->header.length < struct_size(cacd, X2APICIDS, 1)) {
>>> +		pr_warn(FW_BUG "Invalid x2apicid CACD table\n");
>>> +		return -EIO;
>>> +	}
>>> +
>>> +	num_ids = (cacd->header.length - sizeof(*cacd)) / sizeof(cacd->X2APICIDS[0]);
>>> +
>>> +	for (unsigned int i = 0; i < num_ids; i++) {
>>> +		cpu = topo_lookup_cpuid(cacd->X2APICIDS[i]);
>>> +		if (cpu < 0) {
>>> +			pr_warn(FW_BUG "Unknown x2apicid 0x%x\n", cacd->X2APICIDS[i]);
>>> +			return -EIO;
>>
>> The Sashiko reported issue looks real to me:
>> https://sashiko.dev/#/patchset/cover.1789705667.git.yu.c.chen%40intel.com?part=3
>>
>> Were you able to try the example where system limits the number of processors
>> via maxcpus= and see if ERDT still works?
>>
> 
> Yes, this is a valid case, but maxcpus= should be replaced by nr_cpus=.
> maxcpus= limits the online CPUs during bootup, and after bootup, those
> offline CPUs can still be queried by topo_lookup_cpuid(). While for
> nr_cpus=, those offline CPUs are not in the possible CPU mask, they can
> not be online after bootup, and they can not be found by

ah - thank you for pointing this out.

> topo_lookup_cpuid() either. If there is an inconsistency between the
> possible_cpus_mask and the CPU x2APIC set exposed by CACD, all ERDTs will
> be disabled. This is too strong. Let me skip this CPU if it cannot be found
> in the possible CPU mask.
>  
Reinette