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 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 <[email protected]>

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

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

---

Hi Chenyu,

Quoted 9 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.

Quoted 7 lines

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 <[email protected]>

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

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

---

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/[email protected]/

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 <[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/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

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

index 273ddfa30836..2216ee084832 100644

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

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

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

new file mode 100644
index 000000000000..0dfe5eda166c

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

+
+#include <linux/acpi.h>
+#include <linux/overflow.h>
+#include <linux/resctrl.h>
+#include <linux/sizes.h>
+

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

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

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

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

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

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

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

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

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

+
+	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) {

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

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

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

+{
+	return acpi_table_parse(ACPI_SIG_ERDT, enumerate_erdt_table);
+}

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

index e3cfa0c10e92..156206088372 100644
@@ -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

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

+ * @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

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

Quoted 7 lines

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.

Quoted 2 lines

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.

Quoted 4 lines

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.

Quoted 2 lines

Based on original work from Anil S Keshavamurthy.

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

Quoted 13 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/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

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

Quoted 9 lines

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

index 273ddfa30836..2216ee084832 100644
@@ -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.

Quoted 9 lines

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

 obj-$(CONFIG_RESCTRL_FS_PSEUDO_LOCK)	+= pseudo_lock.o
 
 # To allow define_trace.h's recursive include:

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

index 55214d6fdc49..54cfdf12dfbb 100644
@@ -1016,10 +1016,17 @@ static __init void check_quirks(void)
 

arch/x86/kernel/cpu/resctrl/core.c static __init void check_quirks(void) · line 1019

Quoted 3 lines

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

 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().

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

Quoted 2 lines

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

+
+	erdt_init();

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

Quoted 11 lines

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

 __exitcall(resctrl_arch_exit);

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

new file mode 100644
index 000000000000..0dfe5eda166c
@@ -0,0 +1,265 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Enhanced Resource Director Technology (ERDT)
+ *

arch/x86/kernel/cpu/resctrl/erdt.c line 9

Quoted 5 lines

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

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

Quoted 11 lines

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

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

arch/x86/kernel/cpu/resctrl/erdt.c line 25

Quoted 5 lines

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

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

Quoted 5 lines

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

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

arch/x86/kernel/cpu/resctrl/erdt.c line 35

Quoted 5 lines

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

+/* 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".

arch/x86/kernel/cpu/resctrl/erdt.c line 36

Quoted 1 line

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

+ * small rmid accessing an invalid rmid.

Please use upper case for acronyms.

Quoted 19 lines

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

+ */
+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",

arch/x86/kernel/cpu/resctrl/erdt.c line 56

Quoted 1 line

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

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

Quoted 20 lines

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

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

arch/x86/kernel/cpu/resctrl/erdt.c line 81

Quoted 5 lines

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

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

Quoted 8 lines

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

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

arch/x86/kernel/cpu/resctrl/erdt.c line 99

Quoted 10 lines

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

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

Quoted 7 lines

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

+		}
+
+		cpumask_set_cpu(cpu, &domain_info->cpu_mask);
+	}
+
+	return 0;
+}

arch/x86/kernel/cpu/resctrl/erdt.c line 111

Quoted 5 lines

arch/x86/kernel/cpu/resctrl/erdt.c new file +5 -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?

Quoted 16 lines

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

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

arch/x86/kernel/cpu/resctrl/erdt.c line 155

Quoted 5 lines

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

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

Quoted 33 lines

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

+
+	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",

arch/x86/kernel/cpu/resctrl/erdt.c line 194

Quoted 6 lines

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

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

arch/x86/kernel/cpu/resctrl/erdt.c line 262

Quoted 3 lines

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

+}
+
+int __init erdt_init(void)

Please place the storage class attribute before the return type.

Quoted 11 lines

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

+{
+	return acpi_table_parse(ACPI_SIG_ERDT, enumerate_erdt_table);
+}

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

index e3cfa0c10e92..156206088372 100644
@@ -21,6 +21,31 @@
 
 #define RMID_VAL_UNAVAIL		BIT_ULL(62)
 

arch/x86/kernel/cpu/resctrl/internal.h line 36

Quoted 13 lines

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

+/*
+ * 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"

Quoted 11 lines · Reinette Chatre

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.

Quoted 4 lines

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.

Quoted 8 lines

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.

Quoted 6 lines

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.

Quoted 22 lines

Quoted 8 lines

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

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

arch/x86/kernel/cpu/resctrl/erdt.c line 99

Quoted 10 lines

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

+		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

Quoted 3 lines

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.

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 <[email protected]>

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

Tested-by Hongyu Ning <[email protected]>

---
 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(+)

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

index 54cfdf12dfbb..3514d73a8056 100644

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

 }
 
 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);

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

index 0dfe5eda166c..249ba547d7c8 100644

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

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

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

+	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",

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

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

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

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

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

index 156206088372..2e8fb36ad804 100644
@@ -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.

Hi Chenyu,

Quoted 4 lines · 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.

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

Quoted 23 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 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 <[email protected]>

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

Tested-by Hongyu Ning <[email protected]>

---
 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(+)

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

Quoted 8 lines

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

index 54cfdf12dfbb..3514d73a8056 100644
@@ -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.

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

Quoted 3 lines

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

+ *
+ * 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 ...

arch/x86/kernel/cpu/resctrl/core.c static void l3_mon_domain_setup(int cpu, int id, struct rdt_resource *r, struct · line 571

Quoted 9 lines

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

  */
 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.

Quoted 29 lines

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

 }
 
 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);

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

index 0dfe5eda166c..249ba547d7c8 100644
@@ -209,6 +209,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 212

Quoted 1 line

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

+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 ...".

Quoted 17 lines

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

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

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

Quoted 10 lines

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

+	 * 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.

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

Quoted 6 lines

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

+
+	/*
+	 * 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

Quoted 19 lines

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

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

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

Quoted 5 lines

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

+
+/*
+ * 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).

Quoted 7 lines

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

+{
+	struct rdt_hw_l3_mon_domain *hw_dom;
+	struct erdt_domain_info *d;
+
+	if (!erdt_enabled)
+		return;
+

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

Quoted 5 lines

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

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

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

Quoted 1 line

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

+			if (hw_dom->d_info) {

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

Quoted 30 lines

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

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

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

index 156206088372..2e8fb36ad804 100644
@@ -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;
 };
 

arch/x86/kernel/cpu/resctrl/internal.h struct rdt_hw_ctrl_domain { · line 112

Quoted 3 lines

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

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

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

 static inline bool resctrl_arch_alloc_capable(void)
 {
 	return rdt_alloc_capable;

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

index 249ba547d7c8..a8a7417c2f82 100644
@@ -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. */

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

+ * 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);
 }
 

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

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

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

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

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

index 2e8fb36ad804..26c3e0c546ad 100644
@@ -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

Hi Chenyu,

arch/x86/include/asm/resctrl.h DECLARE_STATIC_KEY_FALSE(rdt_enable_key); · line 53

Quoted 10 lines · Chen Yu

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

index 8f6edcdcfd87..9a32ed418c33 100644
@@ -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.

Quoted 19 lines

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

 static inline bool resctrl_arch_alloc_capable(void)
 {
 	return rdt_alloc_capable;

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

index 249ba547d7c8..a8a7417c2f82 100644
@@ -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;
 

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

Quoted 2 lines

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

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

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

Quoted 3 lines

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

+ * 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.

Quoted 3 lines

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

+ * 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.
+ */

arch/x86/kernel/cpu/resctrl/erdt.c static __init int cacd_init(struct acpi_subtbl_hdr_16 *subtbl, · line 126

Quoted 2 lines

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

+static __init int cmrc_init(struct acpi_subtbl_hdr_16 *subtbl,
+			    struct erdt_domain_info *domain_info)

Same comment as for cacd_init().

Quoted 5 lines

static inline struct acpi_subtbl_hdr_16 *rmdd_subtbl(struct acpi_erdt_rmdd *rmdd) {

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

 	return (void *)rmdd + sizeof(*rmdd);

@@ -170,6 +230,19 @@ static __init bool parse_rmdd_table(struct acpi_subtbl_hdr_16 *rmdd_hdr)
 

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

Quoted 7 lines

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

 			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.

From: Tony Luck <[email protected]>

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 <[email protected]>

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

Tested-by Hongyu Ning <[email protected]>

---
 fs/resctrl/monitor.c | 48 +++++++++++++++++++++++++++++++-------------
 1 file changed, 34 insertions(+), 14 deletions(-)

fs/resctrl/monitor.c +0 -0

index 73413cb128ea..3e05b0271c58 100644

fs/resctrl/monitor.c +26 -13

+ * @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) {

Hi Chenyu,

Quoted 9 lines · Chen Yu

From: Tony Luck <[email protected]>

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.

Quoted 4 lines

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.

Quoted 4 lines

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

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 <[email protected]>

Tested-by Hongyu Ning <[email protected]>

---
 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(+)

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

index 9a32ed418c33..bcfcc8ac1fe6 100644

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

 static inline bool resctrl_arch_alloc_capable(void)

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

index 3514d73a8056..ca7e67f976d5 100644
@@ -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)

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

 bool resctrl_arch_is_evt_configurable(enum resctrl_event_id evt)
 {
 	if (!rdt_cpu_has(X86_FEATURE_BMEC))

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

index a8a7417c2f82..400973ebf2b8 100644
@@ -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;

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

index 26c3e0c546ad..ce037e017fc3 100644

Hi Chenyu,

Quoted 2 lines · 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

"is derived from" - needs imperative

Quoted 1 line

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?

Quoted 8 lines

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

index 9a32ed418c33..bcfcc8ac1fe6 100644
@@ -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);
 

arch/x86/include/asm/resctrl.h DECLARE_STATIC_KEY_FALSE(rdt_enable_key); · line 53

Quoted 2 lines

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

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

Quoted 13 lines

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

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

arch/x86/include/asm/resctrl.h DECLARE_STATIC_KEY_FALSE(rdt_enable_key);

Quoted 10 lines

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

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

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 <[email protected]>

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

Tested-by Hongyu Ning <[email protected]>

---
 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(-)

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

index bcfcc8ac1fe6..31c2f81cd06e 100644
@@ -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)
 {

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

 	val /= scale;

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

index ca7e67f976d5..135f2eb2a5a5 100644
@@ -1007,7 +1007,10 @@ static __init bool get_rdt_mon_resources(void)
 	struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_L3].r_resctrl;

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

+#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)
 

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

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

Hi Chenyu,

Quoted 4 lines · 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.

Quoted 4 lines

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?

Quoted 3 lines

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

Quoted 1 line

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?

Quoted 30 lines

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/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(-)

arch/x86/include/asm/resctrl.h +7 -1

index bcfcc8ac1fe6..31c2f81cd06e 100644
@@ -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;
+	}
 

arch/x86/include/asm/resctrl.h static inline void __resctrl_sched_in(struct task_struct *tsk) · line 146

Quoted 1 line

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

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

This comment is no longer accurate.

Quoted 9 lines

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

 	val /= scale;

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

index ca7e67f976d5..135f2eb2a5a5 100644
@@ -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;