Introduce MMIO-based CMT access for Enhanced RDT

From: Tony Luck <[email protected]>

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 is in line with Dave Hansen's proposal[1] for an addition to Linux documentation that details reduced 32-bit x86 support.

Signed-off-by Tony Luck <[email protected]>

Signed-off-by Chen Yu <[email protected]>

Link https://lore.kernel.org/all/[email protected]/ # [1] open here

---
Changes since v8:
- Pick up the latest version of this patch from the AET series so that both series carry identical versions. Changelog wording only, no code

Hi Chenyu,

Quoted 12 lines · Chen Yu

From: Tony Luck <[email protected]>

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 is in line with Dave Hansen's proposal[1] for an addition to Linux documentation that details reduced 32-bit x86 support.

Quoted 7 lines

Signed-off-by Tony Luck <[email protected]>

Signed-off-by Chen Yu <[email protected]>

Link https://lore.kernel.org/all/[email protected]/ # [1] open here

---
Changes since v8:
- Pick up the latest version of this patch from the AET series so that both series carry identical versions. Changelog wording only, no code

Please do continue to keep a note in patch changelog explaining that this is picked from the AET work to help explain why the changelog is unrelated to rest of the series.

Reviewed-by Reinette Chatre <[email protected]>

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 represents one resource management domain (RMD). When RMDD_FLAG_CPU_L3_DOMAIN is set, it identifies the domain as a CPU-scoped L3 domain. The MMIO base of the domain's CMT counters is carried by the RMDD's Cache Monitoring Registers for CPU Agents Description (CMRC)

sub-table. The set of CPUs in the domain is listed by x2APIC ID in the CPU Agent Collection Description (CACD) sub-table.

The hierarchy of these ACPI tables is illustrated below:

  +---------------------------------------------------------------+
  | ERDT  (one instance per platform)                             |
  |                                                               |
  |   +-------------------------------------------------------+   |
  |   | RMDD #0                                               |   |
  |   |   domain_id, max_rmid                                 |   |
  |   |                                                       |   |
  |   |   +-----------------------------------------------+   |   |
  |   |   | CACD                                          |   |   |
  |   |   |   domain_id, x2APIC ID[0], x2APIC ID[1], ...  |   |   |
  |   |   +-----------------------------------------------+   |   |
  |   +-------------------------------------------------------+   |
  |   +-------------------------------------------------------+   |
  |   | RMDD #1                                               |   |
  |   |   ...                                                 |   |
  |   +-------------------------------------------------------+   |
  |   ...                                                         |
  |   +-------------------------------------------------------+   |
  |   | RMDD #N-1                                             |   |
  |   +-------------------------------------------------------+   |
  +---------------------------------------------------------------+

Walk the ERDT table's RMDD sub-tables in preparation for attaching each ERDT domain to a resctrl L3 domain. For each CPU-based L3 RMDD, ioremap its control-register region, walk its 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 RMD 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.

Originally-by Anil S Keshavamurthy <[email protected]>

Suggested-by Tony Luck <[email protected]>

Suggested-by Reinette Chatre <[email protected]>

Signed-off-by Chen Yu <[email protected]>

Tested-by Hongyu Ning <[email protected]>

---
Changes since v8:
- Rename the subject from "... save CACD cpumask for RMDD domains" to "... save CACD cpumask per RMD". (Reinette Chatre)
- Rewrite the changelog to lead with the motivation, drop the duplicated first paragraph, and add a diagram of the ERDT/RMDD/CACD hierarchy. (Reinette Chatre)

arch/x86/kernel/cpu/resctrl/Makefile +0 -0

 obj-$(CONFIG_X86_CPU_RESCTRL_INTEL_AET)	+= intel_aet.o
 obj-$(CONFIG_RESCTRL_FS_PSEUDO_LOCK)	+= pseudo_lock.o
 

arch/x86/kernel/cpu/resctrl/erdt.c new file +27 -0

new file mode 100644
index 000000000000..eb5e262e243a
@@ -0,0 +1,255 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Enhanced Resource Director Technology (ERDT)
+ *
+ * Copyright (C) 2026 Intel Corporation
+ *
+ */
+
+#define pr_fmt(fmt)     "resctrl: ERDT: " 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_REVISION		1
+#define RMDD_FLAG_CPU_L3_DOMAIN		BIT(0)
+#define ERDT_DOMAIN_ID_UNSET		-1

arch/x86/kernel/cpu/resctrl/erdt.c new file +48 -0

+ */
+static unsigned int 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 "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 IDs to CPUs and save them in a mask.
+ */
+static __init bool 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 x2APIC ID CACD table\n");

arch/x86/kernel/cpu/resctrl/erdt.c new file +56 -0

+
+		cpumask_set_cpu(cpu, &domain_info->cpu_mask);
+	}
+
+	return true;
+}
+
+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 = ERDT_DOMAIN_ID_UNSET;
+
+	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;
+

arch/x86/kernel/cpu/resctrl/erdt.c new file +34 -0

+
+	/*
+	 * 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 0\n");
+		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)
+{

Hi Chenyu,

Quoted 5 lines · Chen Yu

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.

Quoted 5 lines

Each Resource Management Domain Description (RMDD) sub-table within the ERDT table represents one resource management domain (RMD). When RMDD_FLAG_CPU_L3_DOMAIN is set, it identifies the domain as a CPU-scoped L3 domain. The MMIO base of the domain's CMT counters is carried by the RMDD's Cache Monitoring Registers for CPU Agents Description (CMRC)

Please drop this sentency about CMRC table and CMT counters, it is unrelated to this patch and distracting from what it aims to achieve.

Quoted 6 lines

Changes since v8:
- Rename the subject from "... save CACD cpumask for RMDD domains" to "... save CACD cpumask per RMD". (Reinette Chatre)
- Rewrite the changelog to lead with the motivation, drop the duplicated first paragraph, and add a diagram of the ERDT/RMDD/CACD hierarchy. (Reinette Chatre)

Did I really ask for a diagram?

It is nice but I think it highlights
how the various domain IDs can be confusing. The diagram shows domain_id at two different scope which raises this importance of these values but these domain IDs are not actually (apart from some debugging) used in this patch.

arch/x86/kernel/cpu/resctrl/Makefile line 4

Quoted 9 lines

arch/x86/kernel/cpu/resctrl/Makefile +1 -0

index 273ddfa30836..a975c73e273e 100644
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_X86_CPU_RESCTRL)		+= core.o rdtgroup.o monitor.o
 obj-$(CONFIG_X86_CPU_RESCTRL)		+= ctrlmondata.o
+obj-$(CONFIG_X86_CPU_RESCTRL)		+= erdt.o

nit: erdt.o could just be added to the line with ctrlmondata.o?

arch/x86/kernel/cpu/resctrl/Makefile line 7

Quoted 3 lines

arch/x86/kernel/cpu/resctrl/Makefile +0 -0

 obj-$(CONFIG_X86_CPU_RESCTRL_INTEL_AET)	+= intel_aet.o
 obj-$(CONFIG_RESCTRL_FS_PSEUDO_LOCK)	+= pseudo_lock.o
 

...

Quoted 3 lines

+/*
+ * The minimal max_rmid of different domains. Using minimal is to avoid the domain with
+ * small RMID accessing an invalid RMID.

Going back and forth between minimum and maximum is confusing.
Could this comment just be simplified to "Largest RMID that is valid in every ERDT domain."?

Quoted 15 lines

arch/x86/kernel/cpu/resctrl/Makefile +15 -0

+static __init bool 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 x2APIC ID CACD table\n");
+		return false;
+	}
+
+	num_ids = (cacd->header.length - sizeof(*cacd)) / sizeof(cacd->X2APICIDS[0]);
+
+	for (unsigned int i = 0; i < num_ids; i++) {

arch/x86/kernel/cpu/resctrl/Makefile

Quoted 4 lines

arch/x86/kernel/cpu/resctrl/Makefile +4 -0

+		cpu = topo_lookup_cpuid(cacd->X2APICIDS[i]);
+		/* Skip CPUs not in the possible mask, e.g. excluded by nr_cpus= */
+		if (cpu < 0)
+			continue;

With this new change it is possible for cacd_init() to return "true" without finding any matching CPUs. A "true" return would result in an ERDT domain without any CPUs added to the domain_info_list. Should such a domain rather be skipped?

Quoted 13 lines

arch/x86/kernel/cpu/resctrl/Makefile +13 -0

+
+	domain_info->dom_id = ERDT_DOMAIN_ID_UNSET;
+
+	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 = (void *)rmdd + sizeof(*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) */

arch/x86/kernel/cpu/resctrl/Makefile

Quoted 5 lines

arch/x86/kernel/cpu/resctrl/Makefile +5 -0

+		case ACPI_ERDT_TYPE_CACD:
+			if (!cacd_init(subtbl, domain_info))
+				goto cleanup;
+
+			subtbl_mask |= BIT(ACPI_ERDT_TYPE_CACD);

Should this bit be set if no valid CPUs found (in which case cacd_init() returns true)?

Quoted 4 lines

arch/x86/kernel/cpu/resctrl/Makefile +4 -0

+			break;
+		default:
+			break;
+		}

arch/x86/kernel/cpu/resctrl/Makefile

Quoted 4 lines

arch/x86/kernel/cpu/resctrl/Makefile +4 -0

+	}
+
+	if (!subtbl_mask)
+		goto cleanup;

How about a check here to ensure that the domain actually has CPUs before adding it to the list? If so, it may be easier to map ERDT_MMIO_RMDD_CREG *after* the RMDD has been determined to have some valid CPUs?

Quoted 14 lines

arch/x86/kernel/cpu/resctrl/Makefile +14 -0

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

arch/x86/kernel/cpu/resctrl/Makefile

Quoted 1 line

arch/x86/kernel/cpu/resctrl/Makefile +1 -0

+	erdt_max_rmid = 0;

The ERDT API seems to expose these globals via helpers that resctrl can call at any time irrespective of whether erdt_enabled is true or not. For example, patch #7 adds a new global erdt_scale, and makes it available via erdt_get_scale() without any additional checks. If this is the plan then it is very important to ensure all globals are reset here and it looks like erdt_scale is not added to this list when it is introduced.

Also please consider checking if erdt is
enabled to ensure a sane value is return from these helpers. Doing so may be an easier pattern for contributions to follow than to subtly require a reset here that is already missed in this series.

ERDT cannot coexist with SNC(Sub-NUMA-Clustering). The definition of SNC

is to 'divide' an L3 into smaller L3 slices by mapping addresses to different L3 slices. The current platform with ERDT enabled has several L3 per socket, and it will not be divided into smaller slices. Furthermore, ERDT has only L3 scope for the CPU agent, and there are no node-scope domains.

snc_nodes_per_l3_cache is calculated in rdt_get_l3_mon_config(), which runs after erdt_init() has parsed the ACPI ERDT table. Introduce resctrl_arch_update_snc() to do that calculation and call it from get_rdt_resources() before erdt_init(). Leave ERDT disabled if SNC is detected and keep using the legacy MSR based CMT interface. Do this check while parsing the ERDT table so that platforms that do not provide an ERDT table at all stay silent.

Suggested-by Tony Luck <[email protected]>

Suggested-by Reinette Chatre <[email protected]>

Signed-off-by Chen Yu <[email protected]>

Tested-by Hongyu Ning <[email protected]>

---

arch/x86/kernel/cpu/resctrl/core.c +0 -0

index 7fadfebb5312..0ea888807bd0 100644

arch/x86/kernel/cpu/resctrl/core.c +0 -0

 	erdt_init();
 	rdt_alloc_capable = get_rdt_alloc_resources();
 	rdt_mon_capable = get_rdt_mon_resources();

arch/x86/kernel/cpu/resctrl/erdt.c +0 -0

index eb5e262e243a..d94c54617fa1 100644

arch/x86/kernel/cpu/resctrl/erdt.c +3 -0

+		return -EINVAL;
+	}
+
 	for (subtbl = (void *)erdt + sizeof(*erdt);
 	     subtbl_valid((void *)erdt + erdt->header.length, subtbl);
 	     subtbl = next_subtbl(subtbl)) {

arch/x86/kernel/cpu/resctrl/internal.h +3 -0

index e2f3b884847c..34d29f719076 100644
@@ -177,6 +177,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 */

@@ -251,6 +253,7 @@ union l3_qos_abmc_cfg {
 void rdt_ctrl_update(void *arg);
 
 int rdt_get_l3_mon_config(struct rdt_resource *r);
+__init void resctrl_arch_update_snc(void);
 
 bool rdt_cpu_has(int flag);
 

arch/x86/kernel/cpu/resctrl/monitor.c +1 -1

index 3838e0a13d36..6e46f10d8640 100644
@@ -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.

Hi Chenyu,

Quoted 1 line · Chen Yu

ERDT cannot coexist with SNC(Sub-NUMA-Clustering). The definition of SNC

"Sub-NUMA-Clustering" -> "Sub-NUMA-Cluster mode"

Quoted 6 lines

is to 'divide' an L3 into smaller L3 slices by mapping addresses to different L3 slices. The current platform with ERDT enabled has several L3 per socket, and it will not be divided into smaller slices. Furthermore, ERDT has only L3 scope for the CPU agent, and there are no node-scope domains.

Quoted 7 lines

snc_nodes_per_l3_cache is calculated in rdt_get_l3_mon_config(), which runs after erdt_init() has parsed the ACPI ERDT table. Introduce resctrl_arch_update_snc() to do that calculation and call it from get_rdt_resources() before erdt_init(). Leave ERDT disabled if SNC is detected and keep using the legacy MSR based CMT interface. Do this check while parsing the ERDT table so that platforms that do not provide an ERDT table at all stay silent.

Please do not just narrate the code in changelog but instead describe at high level in regular language what the change does.

Quoted 16 lines

Suggested-by Tony Luck <[email protected]>

Suggested-by Reinette Chatre <[email protected]>

Signed-off-by Chen Yu <[email protected]>

Tested-by Hongyu Ning <[email protected]>

---

arch/x86/kernel/cpu/resctrl/core.c +0 -0

index 7fadfebb5312..0ea888807bd0 100644
@@ -1018,6 +1018,7 @@ static __init bool get_rdt_resources(void)
 {
 	bool ret;
 

arch/x86/kernel/cpu/resctrl/core.c static __init bool get_rdt_resources(void) · line 1021

Quoted 1 line

arch/x86/kernel/cpu/resctrl/core.c +1 -0

+	resctrl_arch_update_snc();

"resctrl_arch_..." is the common prefix for helpers found in include/linux/resctrl.h that all architectures are required to implement.
Is a helper really needed here? It is just one line, could it just be open coded?

Quoted 11 lines

arch/x86/kernel/cpu/resctrl/core.c +0 -0

 	erdt_init();
 	rdt_alloc_capable = get_rdt_alloc_resources();
 	rdt_mon_capable = get_rdt_mon_resources();

arch/x86/kernel/cpu/resctrl/erdt.c +0 -0

index eb5e262e243a..d94c54617fa1 100644
@@ -229,6 +229,18 @@ static __init int enumerate_erdt_table(struct acpi_table_header *table_hdr)
 		return -EINVAL;
 	}
 

arch/x86/kernel/cpu/resctrl/erdt.c static __init int enumerate_erdt_table(struct acpi_table_header *table_hdr) · line 236

Quoted 5 lines

arch/x86/kernel/cpu/resctrl/erdt.c +5 -0

+	/*
+	 * ERDT describes monitoring resources per L3 cache domain, while SNC
+	 * splits an L3 cache into NUMA node scoped monitoring domains. The two
+	 * cannot be matched up, so leave ERDT disabled and fall back to the
+	 * MSR based CMT interface. snc_nodes_per_l3_cache has been computed by

"MSR CMT interface" - this restriction is not specific to CMT, is it? Could "CMT" just be dropped?

Last sentence of comment can be dropped. It surely will be a bug if snc_nodes_per_l3_cache is not initialized before it is used here and one can just go find where that happens.

arch/x86/kernel/cpu/resctrl/erdt.c static __init int enumerate_erdt_table(struct acpi_table_header *table_hdr) · line 239

Quoted 3 lines

arch/x86/kernel/cpu/resctrl/erdt.c +3 -0

+	 * resctrl_arch_update_snc() before erdt_init() is called.
+	 */
+	if (snc_nodes_per_l3_cache > 1) {

Looks like this check occurs *after* parsing of ERDT table already started. Could this be moved to be the first check to avoid that unnecessary work?

arch/x86/kernel/cpu/resctrl/erdt.c static __init int enumerate_erdt_table(struct acpi_table_header *table_hdr) · line 240

Quoted 1 line

arch/x86/kernel/cpu/resctrl/erdt.c +1 -0

+		pr_info("Not supported with Sub-NUMA Cluster enabled, using legacy CMT\n");

"using legacy CMT" ERDT is not limited to CMT so this can just be dropped? I understand that the pr_info() adds an "ERDT:" prefix but I think it will be useful to include in this message that ERDT in its entirety is not supported:
"ERDT not supported with Sub-NUMA Cluster enabled".

I think this work could be integrated better by switching the order between this patch and "x86/resctrl: Parse ACPI ERDT table and save CACD cpumask per RMD", separating the change to where snc_nodes_per_l3_cache is initialized and the new dependency by moving just this hunk to "x86/resctrl: Parse ACPI ERDT table and save CACD cpumask per RMD" with a note in the changelog that ERDT and SNC are not compatible.

arch/x86/kernel/cpu/resctrl/erdt.c static __init int enumerate_erdt_table(struct acpi_table_header *table_hdr) · line 245

Quoted 5 lines

arch/x86/kernel/cpu/resctrl/erdt.c +5 -0

+__init void resctrl_arch_update_snc(void)
+{
+	snc_nodes_per_l3_cache = snc_get_config();
+}
+

This looks unnecessary. Could all these references to snc_nodes_per_l3_cache be dropped from monitor.c?
Definition of snc_nodes_per_l3_cache could be moved to core.c where it is initialized now and its initialization could just be open coded in get_rdt_resources().

arch/x86/kernel/cpu/resctrl/erdt.c int __init rdt_get_l3_mon_config(struct rdt_resource *r) · line 433

Quoted 12 lines

arch/x86/kernel/cpu/resctrl/erdt.c +0 -2

 int __init rdt_get_l3_mon_config(struct rdt_resource *r)
 {
 	unsigned int mbm_offset = boot_cpu_data.x86_cache_mbm_width_offset;

@@ -423,8 +428,6 @@ int __init rdt_get_l3_mon_config(struct rdt_resource *r)
 	unsigned int threshold;
 	u32 eax, ebx, ecx, edx;
 
-	snc_nodes_per_l3_cache = snc_get_config();
-
 	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;

Reinette

Quoted 1 line · Reinette Chatre

On 10/13/26 7:58 PM, Chen Yu wrote:

Quoted 11 lines

arch/x86/kernel/cpu/resctrl/monitor.c static __init int snc_get_config(void) · line 423

Quoted 5 lines

arch/x86/kernel/cpu/resctrl/monitor.c +5 -0

+__init void resctrl_arch_update_snc(void)
+{
+	snc_nodes_per_l3_cache = snc_get_config();
+}
+

This looks unnecessary. Could all these references to snc_nodes_per_l3_cache be dropped from monitor.c?
Definition of snc_nodes_per_l3_cache could be moved to core.c where it is initialized now and its initialization could just be open coded in get_rdt_resources().

Yes, I moved the definition of snc_nodes_per_l3_cache to core.c and initialize it there, but we cannot move all references to snc_nodes_per_l3_cache out of monitor.c for now because several places in monitor.c use it. After all, IMHO snc_nodes_per_l3_cache was mainly introduced to deal with MBM.

Hi Chenyu,

Quoted 2 lines · Chen Yu

Quoted 1 line · Reinette Chatre

On 10/13/26 7:58 PM, Chen Yu wrote:

...

Quoted 12 lines

Quoted 11 lines

arch/x86/kernel/cpu/resctrl/monitor.c static __init int snc_get_config(void) · line 423

Quoted 5 lines

arch/x86/kernel/cpu/resctrl/monitor.c +5 -0

+__init void resctrl_arch_update_snc(void)
+{
+	snc_nodes_per_l3_cache = snc_get_config();
+}
+

This looks unnecessary. Could all these references to snc_nodes_per_l3_cache be dropped from monitor.c?
Definition of snc_nodes_per_l3_cache could be moved to core.c where it is initialized now and its initialization could just be open coded in get_rdt_resources().

Quoted 4 lines

Yes, I moved the definition of snc_nodes_per_l3_cache to core.c and initialize it there, but we cannot move all references to snc_nodes_per_l3_cache out of monitor.c for now because several places in monitor.c use it. After all, IMHO snc_nodes_per_l3_cache was mainly introduced to deal with MBM.

Agreed. By "these references" I referred to the references in this patch, not _all_ references. I do expect snc_nodes_per_l3_cache to be added to internal.h and that existing usages remain untouched.

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 and bind the CPU to its ERDT domain before adding it to any resctrl domain.

Fail the CPU online operation on a mismatch for now. Leaving the CPU online but outside all resctrl domains is doable, but it would require a change to cpus_ctrl_write() to reject CPUs that are online but not tracked by any resctrl group. The current approach is stricter, but simpler.

The ID used to match a resctrl monitoring domain against an ERDT domain is always the L3 cache ID. ERDT is only enabled when Sub-NUMA Clustering is disabled, so resctrl keeps the L3 monitoring domains at L3 cache scope instead of node scope.

Suggested-by Reinette Chatre <[email protected]>

Signed-off-by Chen Yu <[email protected]>

Tested-by Hongyu Ning <[email protected]>

---

arch/x86/kernel/cpu/resctrl/core.c +2 -0

  * All writers take this mutex:
  */
 static DEFINE_MUTEX(domain_list_lock);

@@ -558,6 +560,8 @@ static void l3_mon_domain_setup(int cpu, int id, struct rdt_resource *r, struct
 		return;
 	}
 
+	erdt_l3_mon_domain_setup(&d->hdr);
+
 	err = resctrl_online_mon_domain(r, &d->hdr);
 	if (err) {
 		l3_mon_domain_free(hw_dom);

arch/x86/kernel/cpu/resctrl/core.c +2 -0

+	}
+
 	for_each_capable_rdt_resource(r)
 		domain_add_cpu(cpu, r);
 	mutex_unlock(&domain_list_lock);

arch/x86/kernel/cpu/resctrl/erdt.c +0 -0

index d94c54617fa1..d4b630358d6d 100644

arch/x86/kernel/cpu/resctrl/erdt.c +9 -0

+ * ERDT domain containing @cpu to that L3 domain.
+ *
+ * Find the erdt_domain_info whose CACD lists @cpu, then bind that ERDT domain
+ * to @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:

arch/x86/kernel/cpu/resctrl/erdt.c +59 -0

+ */
+bool erdt_try_bind_cpu(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_once(FW_BUG "Can't find L3 id for CPU:%d\n", cpu);
+		return false;
+	}
+
+	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_once(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 @cpu instead (CPU2 in the example above).
+	 */
+	if (cpu_dom->dom_id != ERDT_DOMAIN_ID_UNSET) {
+		pr_warn_once(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 @cpu should not be considered
+	 * by resctrl (CPU1 in the example above).
+	 */
+	list_for_each_entry(d, &domain_info_list, entry) {
+		if (d == cpu_dom)
+			continue;
+
+		if (d->dom_id == dom_id) {
+			pr_warn_once(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;
+		}

arch/x86/kernel/cpu/resctrl/erdt.c +12 -0

+ */
+void erdt_l3_mon_domain_setup(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) {

Hi Chenyu,

Quoted 3 lines · Chen Yu

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.

Please review "Changelog" section of Documentation/process/maintainer-tip.rst I understand other subsystems need a snippet that describes the change at the top of changelog but x86 does not. Unless it is something really simple, which this is not, there has to be some context before the change is described.

Quoted 8 lines

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 and bind the CPU to its ERDT domain before adding it to any resctrl domain.

"bind the CPU to its ERDT domain" is this not what patch #3 did?

"adding it" - what does "it" refer to? the ERDT domain or the CPU?

Quoted 4 lines

Fail the CPU online operation on a mismatch for now. Leaving the CPU online but outside all resctrl domains is doable, but it would require a change to cpus_ctrl_write() to reject CPUs that are online but not tracked by any resctrl group. The current approach is stricter, but simpler.

I do not believe failing the CPU online operation is actually possible.

resctrl_arch_late_init() is a late initcall and all boot CPUs are already online, so the failure during this time will result in resctrl not being initialized at all without impacting online CPUs. A CPU onlined after that will fail to initialize, but that will make the CPU unusable for *everything*, not just unmonitored. This sounds extreme for an ERDT table issue that should never happen.

A CPU that is online is still able to run tasks and the earlier enumeration ensured that MSR_IA32_PQR_ASSOC is accessible and what the valid values for it are. Even this "failure" path demonstrates this by calling clear_closid_rmid() that writes to MSR_IA32_PQR_ASSOC *after* the supposed failure.

resctrl's current behavior when a CPU cannot be onlined, for example when domain creation fails, is to *not* prevent CPU from onlining. If a domain cannot be created then the CPU is still added to the default group's CPU mask (resctrl_online_cpu()) to ensure that tasks can continue to be scheduled on it by writing to MSR_IA32_PQR_ASSOC with values known to be valid.

Such a CPU is *not* added to a domain's CPU mask so resctrl will not attempt to make any configuration changes to it or read any counters from it.

Hi Chenyu,

Quoted 3 lines · Chen Yu

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.

Please review "Changelog" section of Documentation/process/maintainer-tip.rst I understand other subsystems need a snippet that describes the change at the top of changelog but x86 does not. Unless it is something really simple, which this is not, there has to be some context before the change is described.

Quoted 8 lines

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 and bind the CPU to its ERDT domain before adding it to any resctrl domain.

"bind the CPU to its ERDT domain" is this not what patch #3 did?

"adding it" - what does "it" refer to? the ERDT domain or the CPU?

Quoted 4 lines

Fail the CPU online operation on a mismatch for now. Leaving the CPU online but outside all resctrl domains is doable, but it would require a change to cpus_ctrl_write() to reject CPUs that are online but not tracked by any resctrl group. The current approach is stricter, but simpler.

I do not believe failing the CPU online operation is actually possible.

resctrl_arch_late_init() is a late initcall and all boot CPUs are already online, so the failure during this time will result in resctrl not being initialized at all without impacting online CPUs. A CPU onlined after that will fail to initialize, but that will make the CPU unusable for *everything*, not just unmonitored. This sounds extreme for an ERDT table issue that should never happen.

A CPU that is online is still able to run tasks and the earlier enumeration ensured that MSR_IA32_PQR_ASSOC is accessible and what the valid values for it are. Even this "failure" path demonstrates this by calling clear_closid_rmid() that writes to MSR_IA32_PQR_ASSOC *after* the supposed failure.

resctrl's current behavior when a CPU cannot be onlined, for example when domain creation fails, is to *not* prevent CPU from onlining. If a domain cannot be created then the CPU is still added to the default group's CPU mask (resctrl_online_cpu()) to ensure that tasks can continue to be scheduled on it by writing to MSR_IA32_PQR_ASSOC with values known to be valid.

Such a CPU is *not* added to a domain's CPU mask so resctrl will not attempt to make any configuration changes to it or read any counters from it.

Quoted 5 lines

The ID used to match a resctrl monitoring domain against an ERDT domain is always the L3 cache ID. ERDT is only enabled when Sub-NUMA Clustering is disabled, so resctrl keeps the L3 monitoring domains at L3 cache scope instead of node scope.

Please always write with imperative tone.

Quoted 5 lines

Suggested-by Reinette Chatre <[email protected]>

Signed-off-by Chen Yu <[email protected]>

Tested-by Hongyu Ning <[email protected]>

---

...

arch/x86/kernel/cpu/resctrl/core.c line 37

Quoted 10 lines

arch/x86/kernel/cpu/resctrl/core.c +2 -0

index 0ea888807bd0..9a59643f366c 100644
@@ -33,6 +33,8 @@
  * The rdt_resource's domain list is updated when this happens. Readers of
  * the domain list must either take cpus_read_lock(), or rely on an RCU
  * read-side critical section, to avoid observing concurrent modification.
+ * This mutex also protects the ERDT domain_info_list, which is modified when a
+ * CPU comes online.

This places unrelated text in the middle of the description about how reads and writes to existing domain list is managed. Please consider entire paragraph and ensure that the end result is coherent.

Quoted 15 lines

arch/x86/kernel/cpu/resctrl/core.c +2 -0

  * All writers take this mutex:
  */
 static DEFINE_MUTEX(domain_list_lock);

@@ -558,6 +560,8 @@ static void l3_mon_domain_setup(int cpu, int id, struct rdt_resource *r, struct
 		return;
 	}
 
+	erdt_l3_mon_domain_setup(&d->hdr);
+
 	err = resctrl_online_mon_domain(r, &d->hdr);
 	if (err) {
 		l3_mon_domain_free(hw_dom);

@@ -742,6 +746,13 @@ static int resctrl_arch_online_cpu(unsigned int cpu)
 	struct rdt_resource *r;
 

arch/x86/kernel/cpu/resctrl/core.c static int resctrl_arch_online_cpu(unsigned int cpu) · line 753

Quoted 6 lines

arch/x86/kernel/cpu/resctrl/core.c +5 -0

 	mutex_lock(&domain_list_lock);
+	/* Refuse to online a CPU whose ERDT and CPUID L3 domain views disagree. */
+	if (!erdt_try_bind_cpu(cpu)) {
+		mutex_unlock(&domain_list_lock);
+		clear_closid_rmid(cpu);
+		return -EINVAL;

(comments added in response to changelog)

Quoted 13 lines

arch/x86/kernel/cpu/resctrl/core.c +2 -0

+	}
+
 	for_each_capable_rdt_resource(r)
 		domain_add_cpu(cpu, r);
 	mutex_unlock(&domain_list_lock);

arch/x86/kernel/cpu/resctrl/erdt.c +0 -0

index d94c54617fa1..d4b630358d6d 100644
@@ -199,6 +199,115 @@ static __init bool parse_rmdd_table(struct acpi_subtbl_hdr_16 *rmdd_hdr)
 	return false;
 }
 

arch/x86/kernel/cpu/resctrl/erdt.c static __init bool parse_rmdd_table(struct acpi_subtbl_hdr_16 *rmdd_hdr) · line 203

Quoted 2 lines

arch/x86/kernel/cpu/resctrl/erdt.c +2 -0

+/*
+ * erdt_try_bind_cpu() - Validate @cpu's L3 domain against ERDT and bind the

Now that the changelog and descriptions come together I do not find my earlier suggestion for function name to be appropriate. How about erdt_try_bind_domain()?

Quoted 9 lines

arch/x86/kernel/cpu/resctrl/erdt.c +9 -0

+ * ERDT domain containing @cpu to that L3 domain.
+ *
+ * Find the erdt_domain_info whose CACD lists @cpu, then bind that ERDT domain
+ * to @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:

arch/x86/kernel/cpu/resctrl/erdt.c static __init bool parse_rmdd_table(struct acpi_subtbl_hdr_16 *rmdd_hdr) · line 222

Quoted 10 lines

arch/x86/kernel/cpu/resctrl/erdt.c +10 -0

+ * id0: CPU0, CPU1, id1: CPU2, CPU3
+ * With the CPUs coming online in order:
+ * - CPU0 binds domain0 to L3 id0.
+ * - CPU1 is in the unbound domain1, but L3 id0 is already bound to domain0,
+ *   so CPU1 is not covered by resctrl.
+ * - CPU2 is in domain0, which is bound to L3 id0 rather than CPU2's L3 id1,
+ *   so CPU2 is not covered by resctrl.
+ * - CPU3 binds domain1 to L3 id1.
+ *
+ * Return: true if @cpu can be covered by resctrl, false otherwise.

Could "can be covered by resctrl" be more specific?

Quoted 66 lines

arch/x86/kernel/cpu/resctrl/erdt.c +66 -0

+ */
+bool erdt_try_bind_cpu(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_once(FW_BUG "Can't find L3 id for CPU:%d\n", cpu);
+		return false;
+	}
+
+	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_once(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 @cpu instead (CPU2 in the example above).
+	 */
+	if (cpu_dom->dom_id != ERDT_DOMAIN_ID_UNSET) {
+		pr_warn_once(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 @cpu should not be considered
+	 * by resctrl (CPU1 in the example above).
+	 */
+	list_for_each_entry(d, &domain_info_list, entry) {
+		if (d == cpu_dom)
+			continue;
+
+		if (d->dom_id == dom_id) {
+			pr_warn_once(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;
+}

arch/x86/kernel/cpu/resctrl/erdt.c static __init bool parse_rmdd_table(struct acpi_subtbl_hdr_16 *rmdd_hdr) · line 291

Quoted 3 lines

arch/x86/kernel/cpu/resctrl/erdt.c +3 -0

+
+/*
+ * Associate ERDT table information with this domain.

Could this add a note (if it is actually true) that earlier checks ensures that hw_dom->d_info will *always* point to a valid ERDT domain ... or does resctrl need to check for hw_dom->d_info everywhere like erdt_mon_read()? Or are those checks just for robustness?

Quoted 12 lines

arch/x86/kernel/cpu/resctrl/erdt.c +12 -0

+ */
+void erdt_l3_mon_domain_setup(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) {

arch/x86/kernel/cpu/resctrl/erdt.c static __init bool parse_rmdd_table(struct acpi_subtbl_hdr_16 *rmdd_hdr) · line 313

Quoted 10 lines

arch/x86/kernel/cpu/resctrl/erdt.c +7 -0

+		if (d->dom_id == hdr->id) {
+			hw_dom->d_info = d;
+			return;
+		}
+	}
+}
+
 void erdt_exit(void)
 {
 	struct erdt_domain_info *d, *tmp;

Reinette

Quoted 1 line · Reinette Chatre

On 10/13/26 7:58 PM, Chen Yu wrote:

Quoted 10 lines

Quoted 7 lines

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 and bind the CPU to its ERDT domain before adding it to any resctrl domain.

"bind the CPU to its ERDT domain" is this not what patch #3 did?

Yes, this is done in patch #3. Here it actually assigns the L3 ID to the ERDT domain.

Quoted 2 lines

"adding it" - what does "it" refer to? the ERDT domain or the CPU?

"It" here refers to the CPU, but it is a bit confusing, I'll revise it to:
When a CPU comes online, check that its ERDT domain and its CPUID L3 cache agree, and bind the ERDT domain to the CPU's L3 cache ID.

Quoted 26 lines

Quoted 5 lines

Fail the CPU online operation on a mismatch for now. Leaving the CPU online but outside all resctrl domains is doable, but it would require a change to cpus_ctrl_write() to reject CPUs that are online but not tracked by any resctrl group. The current approach is stricter, but simpler.

I do not believe failing the CPU online operation is actually possible.

resctrl_arch_late_init() is a late initcall and all boot CPUs are already online, so the failure during this time will result in resctrl not being initialized at all without impacting online CPUs. A CPU onlined after that will fail to initialize, but that will make the CPU unusable for *everything*, not just unmonitored. This sounds extreme for an ERDT table issue that should never happen.

A CPU that is online is still able to run tasks and the earlier enumeration ensured that MSR_IA32_PQR_ASSOC is accessible and what the valid values for it are. Even this "failure" path demonstrates this by calling clear_closid_rmid() that writes to MSR_IA32_PQR_ASSOC *after* the supposed failure.

resctrl's current behavior when a CPU cannot be onlined, for example when domain creation fails, is to *not* prevent CPU from onlining. If a domain cannot be created then the CPU is still added to the default group's CPU mask (resctrl_online_cpu()) to ensure that tasks can continue to be scheduled on it by writing to MSR_IA32_PQR_ASSOC with values known to be valid.

Quoted 3 lines

Such a CPU is *not* added to a domain's CPU mask so resctrl will not attempt to make any configuration changes to it or read any counters from it.

Thanks for the detailed explanation, got it. I'll change it to:
On a mismatch the CPU stays online and is not added to any monitor domain, but still goes through clear_closid_rmid() and resctrl_online_cpu(), so it joins the default group. How about the following:

        mutex_lock(&domain_list_lock);
        /* Keep a CPU out of all domains if its ERDT and CPUID L3 domains conflict. */
        if (!erdt_try_bind_domain(cpu))
                goto unlock;

        for_each_capable_rdt_resource(r)
                domain_add_cpu(cpu, r);
unlock:
        mutex_unlock(&domain_list_lock);

        clear_closid_rmid(cpu);
        resctrl_online_cpu(cpu);

        return 0;

And you also mentioned that even if CPU1 is not eligible to be added to the monitor domain's cpumask, since the event can be read from any CPU, CPU1 can still read the domain's monitor data via (mmio_base, RMID) without involving any CPU, although the data read from this address is questionable because the ERDT is already broken.

Quoted 6 lines

Quoted 6 lines

arch/x86/kernel/cpu/resctrl/core.c +0 -0

index 0ea888807bd0..9a59643f366c 100644
@@ -33,6 +33,8 @@
  * The rdt_resource's domain list is updated when this happens. Readers of

Quoted 9 lines

arch/x86/kernel/cpu/resctrl/core.c line 37

Quoted 4 lines

arch/x86/kernel/cpu/resctrl/core.c +2 -0

  * the domain list must either take cpus_read_lock(), or rely on an RCU
  * read-side critical section, to avoid observing concurrent modification.
+ * This mutex also protects the ERDT domain_info_list, which is modified when a
+ * CPU comes online.

This places unrelated text in the middle of the description about how reads and writes to existing domain list is managed. Please consider entire paragraph and ensure that the end result is coherent.

How about adding a separate paragraph after the domain list description. It also states more precisely what is protected: the binding:
cpu_dom->dom_id = dom_id, not the list itself, because domain_info_list is only modified during init.

* All writers take this mutex.
*
* This mutex also serializes binding an ERDT domain to an L3 cache ID
* (erdt_domain_info::dom_id) when a CPU comes online.

On second thought, would this race condition really happen? erdt_try_bind_domain() is the function that changes erdt_domain_info::dom_id, and it is invoked by resctrl_arch_online_cpu(). CPU onlining is performed one at a time, not in parallel, so it is unlikely that two CPUs invoke erdt_try_bind_domain() at the same time. Although in the future, it is possible that onlining could be changed to run in parallel.

Quoted 4 lines

Quoted 4 lines

arch/x86/kernel/cpu/resctrl/erdt.c +0 -0

index d94c54617fa1..d4b630358d6d 100644

Quoted 20 lines

Quoted 19 lines

+ * ERDT domain containing @cpu to that L3 domain.
+ *
+ * Find the erdt_domain_info whose CACD lists @cpu, then bind that ERDT domain
+ * to @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.
+ * - CPU1 is in the unbound domain1, but L3 id0 is already bound to domain0,
+ * so CPU1 is not covered by resctrl.
+ * - CPU2 is in domain0, which is bound to L3 id0 rather than CPU2's L3 id1,
+ * so CPU2 is not covered by resctrl.
+ * - CPU3 binds domain1 to L3 id1.
+ *
+ * Return: true if @cpu can be covered by resctrl, false otherwise.

Quoted 2 lines

Could "can be covered by resctrl" be more specific?

OK, how about changing it to:

* Return: true if @cpu can be added to resctrl domains, false if @cpu must
* not be in any resctrl domain.

The other "covered by resctrl" wording in the comment is updated too.

Quoted 66 lines

Quoted 66 lines

+ */

arch/x86/kernel/cpu/resctrl/erdt.c +65 -0

+bool erdt_try_bind_cpu(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_once(FW_BUG "Can't find L3 id for CPU:%d\n", cpu);
+		return false;
+	}
+
+	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_once(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 @cpu instead (CPU2 in the example above).
+	 */
+	if (cpu_dom->dom_id != ERDT_DOMAIN_ID_UNSET) {
+		pr_warn_once(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 @cpu should not be considered
+	 * by resctrl (CPU1 in the example above).
+	 */
+	list_for_each_entry(d, &domain_info_list, entry) {
+		if (d == cpu_dom)
+			continue;
+
+		if (d->dom_id == dom_id) {
+			pr_warn_once(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;
+}

Quoted 9 lines

arch/x86/kernel/cpu/resctrl/erdt.c static __init bool parse_rmdd_table(struct acpi_subtbl_hdr_16 *rmdd_hdr) · line 291

Quoted 3 lines

arch/x86/kernel/cpu/resctrl/erdt.c +3 -0

+
+/*
+ * Associate ERDT table information with this domain.

Could this add a note (if it is actually true) that earlier checks ensures that hw_dom->d_info will *always* point to a valid ERDT domain ... or does resctrl need to check for hw_dom->d_info everywhere like erdt_mon_read()? Or are those checks just for robustness?

It should be always valid if ERDT is enabled. The CPU that creates the L3 monitoring domain has passed erdt_try_bind_domain(), which bound an ERDT domain to that CPU's L3 cache ID. The L3 monitoring domain ID is the L3 cache ID, so the lookup should always find a match.

The check in erdt_mon_read() was only for robustness(I did not find scenario that it could be NULL), just in case I added "if (WARN_ON_ONCE(!d))" in the next version.

Quoted 11 lines

Quoted 11 lines

+ */

arch/x86/kernel/cpu/resctrl/erdt.c +10 -0

+void erdt_l3_mon_domain_setup(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));
+

Quoted 8 lines

arch/x86/kernel/cpu/resctrl/erdt.c static __init bool parse_rmdd_table(struct acpi_subtbl_hdr_16 *rmdd_hdr) · line 310

Quoted 8 lines

arch/x86/kernel/cpu/resctrl/erdt.c +8 -0

+	list_for_each_entry(d, &domain_info_list, entry) {
+		if (d->dom_id == hdr->id) {
+			hw_dom->d_info = d;
+			return;
+		}
+	}
+}
+

Hi Chenyu,

Quoted 5 lines

OK, how about changing it to:

* Return: true if @cpu can be added to resctrl domains, false if @cpu must
* not be in any resctrl domain.

This refers to how caller acts on result of this function as opposed to what this function actually returns. Compare to, for example:

* Return: true if ERDT is unused, or if the ERDT domain containing @cpu is
* bound to @cpu's L3 cache ID, whether that binding already existed or was
* created by this call. false if ERDT and CPUID cannot be reconciled into a
* one-to-one binding, in which case no binding is modified.

Quoted 6 lines

Quoted 5 lines

Could this add a note (if it is actually true) that earlier checks ensures that hw_dom->d_info will *always* point to a valid ERDT domain ... or does resctrl need to check for hw_dom->d_info everywhere like erdt_mon_read()? Or are those checks just for robustness?

Quoted 4 lines

It should be always valid if ERDT is enabled. The CPU that creates the L3 monitoring domain has passed erdt_try_bind_domain(), which bound an ERDT domain to that CPU's L3 cache ID. The L3 monitoring domain ID is the L3 cache ID, so the lookup should always find a match.

Could you please add a this information to the function comment?

resctrl_arch_round_mon_val() is an inline function in the architecture header. It is only called during resctrl initialization and when user space

writes "max_threshold_occupancy", neither of which is a hot path that benefits from inlining.

Upcoming ERDT support needs resctrl_arch_round_mon_val() to consult ERDT-specific state. Keeping it inline would force ERDT prototypes into the architecture header, fragmenting the ERDT code across files.

Move resctrl_arch_round_mon_val() into monitor.c and declare it in include/linux/resctrl.h alongside the other resctrl_arch_*() calls. Move the MPAM stub from arm_mpam.h into mpam_resctrl.c accordingly.

No functional change.

Suggested-by Reinette Chatre <[email protected]>

Signed-off-by Chen Yu <[email protected]>

---

 arch/x86/include/asm/resctrl.h        | 9 ---------
 arch/x86/kernel/cpu/resctrl/monitor.c | 9 +++++++++
 drivers/resctrl/mpam_resctrl.c        | 5 +++++
 include/linux/arm_mpam.h              | 5 -----

 include/linux/resctrl.h               | 1 +
 5 files changed, 15 insertions(+), 14 deletions(-)

Hi Chenyu,

Quoted 2 lines · Chen Yu

resctrl_arch_round_mon_val() is an inline function in the architecture header. It is only called during resctrl initialization and when user space

For emphasis this can be:
"in the architecture header" -> "in the x86 and MPAM architecture headers."

Quoted 6 lines

writes "max_threshold_occupancy", neither of which is a hot path that benefits from inlining.

Upcoming ERDT support needs resctrl_arch_round_mon_val() to consult ERDT-specific state. Keeping it inline would force ERDT prototypes into the architecture header, fragmenting the ERDT code across files.

For emphasis:
"architecture header" -> "x86 asm header"
"fragmenting the ERDT code into the x86 asm header where it does not belong."?

Quoted 11 lines

Move resctrl_arch_round_mon_val() into monitor.c and declare it in include/linux/resctrl.h alongside the other resctrl_arch_*() calls. Move the MPAM stub from arm_mpam.h into mpam_resctrl.c accordingly.

No functional change.

Suggested-by Reinette Chatre <[email protected]>

Signed-off-by Chen Yu <[email protected]>

---

Quoted 4 lines

 arch/x86/include/asm/resctrl.h        | 9 ---------
 arch/x86/kernel/cpu/resctrl/monitor.c | 9 +++++++++
 drivers/resctrl/mpam_resctrl.c        | 5 +++++
 include/linux/arm_mpam.h              | 5 -----

This needs a subject prefix of "arm,x86,fs/resctrl:" to draw attention from MPAM maintainers.

This needs an ack from the MPAM maintainer and at this time it is really buried within this series and unlikely to get attention.

Placing this change in the middle of the series also places unnecessary burden on MPAM maintainers since now they need to review all patches up to this change when only a review of this change is needed.

Please move this patch to beginning of series, this could be the second patch in the series after the "x86/topology" change.

Quoted 2 lines

 include/linux/resctrl.h               | 1 +
 5 files changed, 15 insertions(+), 14 deletions(-)

Patch looks good.

Reinette

resctrl_arch_round_mon_val() is an inline function in the architecture header. It is only called during resctrl initialization and when user space writes "max_threshold_occupancy", neither of which is a hot path that benefits from inlining.

Upcoming ERDT support needs resctrl_arch_round_mon_val() to consult ERDT-specific state. Keeping it inline would force ERDT prototypes into the architecture header, fragmenting the ERDT code across files.

Move resctrl_arch_round_mon_val() into monitor.c and declare it in include/linux/resctrl.h alongside the other resctrl_arch_*() calls. Move the MPAM stub from arm_mpam.h into mpam_resctrl.c accordingly.

No functional change.

Suggested-by Reinette Chatre <[email protected]>

Signed-off-by Chen Yu <[email protected]>

---
Changes since v8:
- New patch. v8 added erdt_get_scale() and erdt_cpu_has() prototypes to asm/resctrl.h only to keep resctrl_arch_round_mon_val() inline, which fragmented the ERDT code. Move the function out of line instead so the ERDT prototypes can stay in the resctrl internal header. (Reinette Chatre)

 arch/x86/include/asm/resctrl.h        | 9 ---------
 arch/x86/kernel/cpu/resctrl/monitor.c | 9 +++++++++
 drivers/resctrl/mpam_resctrl.c        | 5 +++++
 include/linux/arm_mpam.h              | 5 -----
 include/linux/resctrl.h               | 1 +
 5 files changed, 15 insertions(+), 14 deletions(-)

arch/x86/include/asm/resctrl.h +0 -9

index 8f6edcdcfd87..738de74235bd 100644
@@ -130,15 +130,6 @@ 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;
-
-	/* h/w works in units of "boot_cpu_data.x86_cache_occ_scale" */
-	val /= scale;
-	return val * scale;
-}
-
 static inline void resctrl_arch_set_cpu_default_closid_rmid(int cpu, u32 closid,
 							    u32 rmid)
 {

arch/x86/kernel/cpu/resctrl/monitor.c +9 -0

index 6e46f10d8640..e42ee2769a39 100644
@@ -421,6 +421,15 @@ __init void resctrl_arch_update_snc(void)
 	snc_nodes_per_l3_cache = snc_get_config();
 }
 
+unsigned int resctrl_arch_round_mon_val(unsigned int val)
+{
+	unsigned int scale = boot_cpu_data.x86_cache_occ_scale;
+
+	/* h/w works in units of "boot_cpu_data.x86_cache_occ_scale" */
+	val /= scale;
+	return val * scale;
+}
+
 int __init rdt_get_l3_mon_config(struct rdt_resource *r)
 {
 	unsigned int mbm_offset = boot_cpu_data.x86_cache_mbm_width_offset;

drivers/resctrl/mpam_resctrl.c +5 -0

index 9d223057953a..5799c6bd4f98 100644
@@ -143,6 +143,11 @@ void resctrl_arch_pre_mount(void)
 {
 }
 
+unsigned int resctrl_arch_round_mon_val(unsigned int val)
+{
+	return val;
+}
+
 bool resctrl_arch_get_cdp_enabled(enum resctrl_res_level rid)
 {
 	return mpam_resctrl_controls[rid].cdp_enabled;

include/linux/arm_mpam.h +0 -5

index f92a36187a52..4c639e9c8e5b 100644
@@ -76,11 +76,6 @@ static inline void resctrl_arch_disable_mon(void) { }
 static inline void resctrl_arch_enable_alloc(void) { }
 static inline void resctrl_arch_disable_alloc(void) { }
 
-static inline unsigned int resctrl_arch_round_mon_val(unsigned int val)
-{
-	return val;
-}
-
 /**
  * mpam_register_requestor() - Register a requestor with the MPAM driver
  * @partid_max:		The maximum PARTID value the requestor can generate.

include/linux/resctrl.h +1 -0

index dd09c2ce9a0f..c99561b19e7a 100644
@@ -418,6 +418,7 @@ static inline u32 resctrl_get_default_ctrl(struct rdt_resource *r)
 u32 resctrl_arch_get_num_closid(struct rdt_resource *r);
 u32 resctrl_arch_system_num_rmid_idx(void);
 int resctrl_arch_update_domains(struct rdt_resource *r, u32 closid);
+unsigned int resctrl_arch_round_mon_val(unsigned int val);
 
 bool resctrl_enable_mon_event(enum resctrl_event_id eventid, bool any_cpu,
 			      unsigned int binary_bits, void *arch_priv);

Hi Chen,

Quoted 9 lines · Chen Yu

resctrl_arch_round_mon_val() is an inline function in the architecture header. It is only called during resctrl initialization and when user space writes "max_threshold_occupancy", neither of which is a hot path that benefits from inlining.

Upcoming ERDT support needs resctrl_arch_round_mon_val() to consult ERDT-specific state. Keeping it inline would force ERDT prototypes into the architecture header, fragmenting the ERDT code across files.

Quoted 3 lines

Move resctrl_arch_round_mon_val() into monitor.c and declare it in include/linux/resctrl.h alongside the other resctrl_arch_*() calls. Move the MPAM stub from arm_mpam.h into mpam_resctrl.c accordingly.

For the MPAM part:

Acked-by Ben Horgan <[email protected]>

The limbo handler polls the LLC occupancy of stale RMIDs and frees an RMID

once its occupancy drops below max_threshold_occupancy. User space writes

that threshold in bytes, but the hardware counts occupancy in units of the occupancy scale, so the threshold is rounded down to a multiple of that scale. The scale is the number of bytes one counter unit represents, i.e.

the granularity of the occupancy counter.

ERDT may provide per-domain monitoring scale factors, but resctrl rounds max_threshold_occupancy through a single global resctrl_rmid_realloc_threshold value that is visible to user space.

Prepare for that by adding erdt_scale/erdt_get_scale() and teaching

resctrl_arch_round_mon_val() to consult the ERDT scale when firmware has populated one.

When CMRC parsing provides per-domain up_scale values, erdt_scale will be updated with max(). The reason to use max is because 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.

For now erdt_scale is intentionally left at its default value of 0.

Suggested-by Reinette Chatre <[email protected]>

Signed-off-by Chen Yu <[email protected]>

---
Changes since v8:
- New patch. erdt_scale and erdt_get_scale() were introduced inside "x86/resctrl: Parse ACPI CMRC table" without any mention in its changelog. Split them out and describe why a single global scale is used. (Reinette Chatre)
- Reword the erdt_scale comment: it is not used by the limbo handler. (Reinette Chatre)

 arch/x86/kernel/cpu/resctrl/erdt.c     | 16 ++++++++++++++++
 arch/x86/kernel/cpu/resctrl/internal.h |  1 +
 arch/x86/kernel/cpu/resctrl/monitor.c  |  5 ++++-
 3 files changed, 21 insertions(+), 1 deletion(-)

arch/x86/kernel/cpu/resctrl/erdt.c +0 -0

index d4b630358d6d..09072e478bde 100644
@@ -38,6 +38,22 @@ static u16 first_rmdd_domain_id;
  */

arch/x86/kernel/cpu/resctrl/erdt.c +2 -0

+}
+
 static void __iomem *erdt_ioremap(resource_size_t base, u32 num_pages, const char *desc)
 {
 	void __iomem *addr;

arch/x86/kernel/cpu/resctrl/internal.h +1 -0

index 48aa64c1ff96..75dd4736937d 100644
@@ -284,6 +284,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
 
+unsigned int erdt_get_scale(void);
 void erdt_init(void);
 void erdt_exit(void);
 bool erdt_try_bind_cpu(int cpu);

arch/x86/kernel/cpu/resctrl/monitor.c +0 -0

index e42ee2769a39..f1a28a3c81bc 100644

Hi Chenyu,

Quoted 1 line · Chen Yu

The limbo handler polls the LLC occupancy of stale RMIDs and frees an RMID

"stale RMIDs"? This is a new concept unfamiliar to resctrl.

Quoted 1 line

once its occupancy drops below max_threshold_occupancy. User space writes

Please do not interchangeably use the name of the resctrl file and the variable that backs it because the variable that backs the file may not have the value that user space wrote to the file.

Quoted 3 lines

that threshold in bytes, but the hardware counts occupancy in units of the occupancy scale, so the threshold is rounded down to a multiple of that scale. The scale is the number of bytes one counter unit represents, i.e.

This does not sound right, the threshold is explicitly noted as:
"This is the threshold cache occupancy in *bytes* ..."

Quoted 7 lines

the granularity of the occupancy counter.

ERDT may provide per-domain monitoring scale factors, but resctrl rounds max_threshold_occupancy through a single global resctrl_rmid_realloc_threshold value that is visible to user space.

Prepare for that by adding erdt_scale/erdt_get_scale() and teaching

"Prepare for that" - Prepare for what?

Quoted 9 lines

resctrl_arch_round_mon_val() to consult the ERDT scale when firmware has populated one.

When CMRC parsing provides per-domain up_scale values, erdt_scale will be updated with max(). The reason to use max is because 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.

For now erdt_scale is intentionally left at its default value of 0.

This is a clear signal that the patch ordering needs to change.

Quoted 8 lines

+/*
+ * Used only to round the global resctrl_rmid_realloc_threshold for the limbo
+ * handler. resctrl_rmid_realloc_threshold is exposed to user space as a single
+ * global value, so ERDT uses one scale derived from the per-domain
+ * cmrc->up_scale values. max() keeps this single threshold conservative across
+ * domains 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.

This documents changes that do not exist at this point in series. Please reorder this patch to ensure its dependencies land first.

arch/x86/kernel/cpu/resctrl/erdt.c static u16 first_rmdd_domain_id; · line 54

Quoted 6 lines

+ */

arch/x86/kernel/cpu/resctrl/erdt.c +5 -0

+static unsigned int erdt_scale;
+
+unsigned int erdt_get_scale(void)
+{
+	return erdt_scale;

The name and usage implies that this is a global scale. This will become more confusing when trying to land the memory bandwidth monitoring changes that have separate scale values.

Having this change pretend it is a global change and then add a snippet to final patch to constrain it with a "if (!erdt_cpu_has(X86_FEATURE_CQM_OCCUP_LLC))" just makes this work difficult to consume.

If this is specific to the CMT scale only then let the global variable and its accessor make that obvious. Let this change land after the CMRC parsing and erdt_cpu_has() introduction so that it can be introduced as one coherent change.

Quoted 9 lines

unsigned int resctrl_arch_round_mon_val(unsigned int val) {

arch/x86/kernel/cpu/resctrl/monitor.c +4 -1

-	unsigned int scale = boot_cpu_data.x86_cache_occ_scale;
+	unsigned int scale = erdt_get_scale();
+
+	if (!scale)
+		scale = boot_cpu_data.x86_cache_occ_scale;

Quoted 1 line

/* h/w works in units of "boot_cpu_data.x86_cache_occ_scale" */

The fixup of this comment found in patch #12 needs to be squashed here.

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.

Although the occupancy counters can now be read from any CPU via MMIO, keep the per-domain limbo handler. A global handler would still have to iterate over every domain and would save nr_domains - 1

worker threads, which does not justify maintaining two limbo handler models.

Suggested-by Reinette Chatre <[email protected]>

Signed-off-by Chen Yu <[email protected]>

Tested-by Hongyu Ning <[email protected]>

---

arch/x86/kernel/cpu/resctrl/erdt.c +8 -1

index 110e7b9806b4..1a9847a11de8 100644
@@ -27,6 +27,9 @@ static bool erdt_enabled;
 #define RMDD_FLAG_CPU_L3_DOMAIN		BIT(0)
 #define ERDT_DOMAIN_ID_UNSET		-1
 
+#define CMRC_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;
 

@@ -52,14 +55,84 @@ static unsigned int erdt_scale;
 
 bool erdt_support(int flag)
 {
-	return false;
+	if (!erdt_enabled)
+		return false;
+
+	switch (flag) {
+	case X86_FEATURE_CQM_OCCUP_LLC:

arch/x86/kernel/cpu/resctrl/erdt.c +13 -0

 }
 
+/*
+ * Compute the CMRC MMIO offset for CMRC_SUPPORTED_INDEX_FN using the
+ * MMIO_offset_for_RMID# = (RMID / ClumpSize) * Stride + (RMID % ClumpSize) * 8
+ * formula from the Intel RDT Architecture Specification.
+ */
+static u32 cmrc_index_function_1(struct acpi_erdt_cmrc *cmrc, u32 rmid)
+{
+	return (rmid / cmrc->clump_size) * cmrc->clump_stride +
+	       (rmid % cmrc->clump_size) * sizeof(u64);
+}
+
+static int erdt_read_l3_occupancy(const struct erdt_domain_info *d, u32 rmid, u64 *val)
+{

arch/x86/kernel/cpu/resctrl/erdt.c +28 -0

+		return -EINVAL;
+
+	l3_cmt_count = readq(d->base[ERDT_MMIO_CMRC_BASE] + offset);
+	if ((cmrc->flags & CMRC_FLAG_UNAVAILABLE_BIT) &&
+	    (l3_cmt_count & CMRC_UNAVAILABLE_COUNTER))
+		return -EINVAL;
+
+	*val = l3_cmt_count * cmrc->up_scale;
+
+	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)
 {
 	void __iomem *addr;

arch/x86/kernel/cpu/resctrl/erdt.c +0 -0

 	return true;
 }
 

arch/x86/kernel/cpu/resctrl/internal.h +2 -0

index 88fae7a84fc7..3561a7ae8c7e 100644
@@ -291,6 +291,8 @@ static inline bool intel_handle_aet_option(bool force_off, char *tok) { return f
 bool erdt_cpu_has(int flag);
 bool erdt_support(int flag);
 unsigned int erdt_get_scale(void);
+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);
 void erdt_init(void);
 void erdt_exit(void);
 bool erdt_try_bind_cpu(int cpu);

arch/x86/kernel/cpu/resctrl/monitor.c +4 -0

index e66e4c845739..93ee638119d3 100644
@@ -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);

arch/x86/kernel/cpu/resctrl/monitor.c +4 -0

+	erdt_max_rmid = erdt_get_max_rmid();
+	if (erdt_max_rmid)
+		max_rmid = min_t(unsigned int, erdt_max_rmid,
+				 boot_cpu_data.x86_cache_max_rmid);