Introduce MMIO-based CMT access for Enhanced RDT

From: Anil S Keshavamurthy <[email protected]>

ERDT(Enhanced RDT) introduces a new top-level ACPI structure

(the ERDT) that the kernel must parse before any enhanced RDT feature can be used. The ERDT improves the existing RDT by switching low-level register access from MSR-based to MMIO-based, which is more efficient.

The ERDT structure may include several sub ACPI tables:

- Resource Management Domain Description Structure (RMDD)
- CPU Agent Collection Description Structure (CACD)
- Cache Monitoring Registers for CPU Agents Description Structure

    (CMRC)

There is one ERDT table per platform.
Each RMDD substructure in ERDT represents one resource management domain (RMD), also known as an L3 domain. Thus, the total number of RMDDs equals the number of L3 domains on the platform. Each RMDD contains information such as MMIO addresses. This address is used to retrieve RDT metrics like L3 occupancy.

Add basic ERDT ACPI table and sub-table parsing, and store the relevant tables for later processing.

Among these sub-tables, RMDD requires special handling. There is one RMDD per domain, and the domain ID reuses the L3 cache ID. Many code paths need to retrieve an RMDD efficiently by domain ID (L3 cache ID). Because L3 cache IDs are derived from x2APIC IDs and are not contiguous, using a plain array indexed by domain ID would waste memory. As a trade-off, an xarray is used to store these tables, with

the L3 cache ID as the key.

Suggested-by Tony Luck <[email protected]>

Tested-by Hongyu Ning <[email protected]>

Signed-off-by Anil S Keshavamurthy <[email protected]>

---

---
 arch/x86/Kconfig                       |   4 +-
 arch/x86/include/asm/apic.h            |   1 +
 arch/x86/include/asm/resctrl.h         |   2 +
 arch/x86/kernel/cpu/resctrl/Makefile   |   1 +
 arch/x86/kernel/cpu/resctrl/core.c     |  14 +-
 arch/x86/kernel/cpu/resctrl/erdt.c     | 296 +++++++++++++++++++++++++
 arch/x86/kernel/cpu/resctrl/internal.h |   3 +
 arch/x86/kernel/cpu/topology.c         |   2 +-

 8 files changed, 319 insertions(+), 4 deletions(-)
 create mode 100644 arch/x86/kernel/cpu/resctrl/erdt.c

arch/x86/Kconfig +1 -1

index f3f7cb01d69d..97d210bd9bb5 100644
@@ -515,7 +515,7 @@ config X86_MPPARSE
 
 config X86_CPU_RESCTRL
 	bool "x86 CPU resource control support"
-	depends on X86 && (CPU_SUP_INTEL || CPU_SUP_AMD)
+	depends on X86_64 && (CPU_SUP_INTEL || CPU_SUP_AMD)
 	depends on MISC_FILESYSTEMS
 	select ARCH_HAS_CPU_RESCTRL
 	select RESCTRL_FS

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

index 9cd493d467d4..bb84651b14bd 100644
@@ -54,6 +54,7 @@ static inline void x86_32_probe_apic(void) { }
 #endif
 
 extern u32 cpuid_to_apicid[];
+int topo_lookup_cpuid(u32 apic_id);
 
 #define CPU_ACPIID_INVALID	U32_MAX
 

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

index 575f8408a9e7..97c2f6bc7a5f 100644

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

 DECLARE_PER_CPU(struct resctrl_pqr_state, pqr_state);
 
 extern bool rdt_alloc_capable;

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
 obj-$(CONFIG_RESCTRL_FS_PSEUDO_LOCK)	+= pseudo_lock.o
 
 # To allow define_trace.h's recursive include:

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

index 7667cf7c4e94..90730f0851fa 100644
@@ -1012,6 +1012,7 @@ static __init void check_quirks(void)
 
 static __init bool get_rdt_resources(void)
 {
+	erdt_init();
 	rdt_alloc_capable = get_rdt_alloc_resources();
 	rdt_mon_capable = get_rdt_mon_resources();
 

@@ -1113,7 +1114,7 @@ void resctrl_cpu_detect(struct cpuinfo_x86 *c)
 	}
 }
 
-static int __init resctrl_arch_late_init(void)
+static int __init __resctrl_arch_late_init(void)
 {
 	struct rdt_resource *r;
 	int state, ret, i;

@@ -1156,6 +1157,15 @@ static int __init resctrl_arch_late_init(void)
 	return 0;
 }
 
+static int __init resctrl_arch_late_init(void)
+{
+	int ret = __resctrl_arch_late_init();
+
+	if (ret)
+		erdt_exit();
+	return ret;
+}
+
 late_initcall(resctrl_arch_late_init);
 
 static void __exit resctrl_arch_exit(void)

@@ -1165,6 +1175,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..b43dcc06f009

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

+		if (tmp == -1) {
+			pr_warn(FW_BUG "Can not find L3 cache id for CPU%d\n", cpu);
+			return -1;
+		}
+
+		if (cache_id == -1)
+			cache_id = tmp;
+
+		if (tmp != cache_id) {
+			pr_warn(FW_BUG "CACD references multiple L3 cache instances\n");
+			return -1;
+		}
+		online_cpu = cpu;
+	}
+
+	if (online_cpu == -1)
+		return -1;
+
+	/*
+	 * Check if CACD lists all CPUs in the LLC domain.
+	 */

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

+		       desc, (unsigned long long)base, size);
+	}
+	return addr;
+}
+
+static void erdt_iounmap_domain(struct erdt_domain_info *domain)
+{
+	for (int i = 0; i < ERDT_MMIO_MAX; 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);
+}
+
+static __init bool cacd_init(struct acpi_subtbl_hdr_16 *subtbl, int *l3_cache_id)
+{
+	*l3_cache_id = get_l3_cache_id_from_cacd((struct acpi_erdt_cacd *)subtbl);
+
+	return *l3_cache_id != -1;
+}
+
+static inline struct acpi_subtbl_hdr_16 *rmdd_subtbl(struct acpi_erdt_rmdd *rmdd)
+{

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

+	if (subtbl->length < sizeof(*subtbl))
+		return false;
+
+	if ((void *)subtbl + subtbl->length > end)
+		return false;
+
+	return true;
+}
+
+static __init bool parse_rmdd_entry(struct acpi_subtbl_hdr_16 *rmdd_hdr)
+{
+	struct erdt_domain_info *domain_info;
+	struct acpi_subtbl_hdr_16 *subtbl;
+	struct acpi_erdt_rmdd *rmdd;
+	int l3_cache_id = -1;
+	u32 subtbl_mask = 0;
+
+	if (rmdd_hdr->length < sizeof(*rmdd)) {
+		pr_info(FW_BUG "Invalid RMDD length %u\n", rmdd_hdr->length);

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

+	     subtbl = next_subtbl(subtbl)) {
+		switch (subtbl->type) {
+		case ACPI_ERDT_TYPE_CACD:
+			if (cacd_init(subtbl, &l3_cache_id))
+				subtbl_mask |= BIT(ACPI_ERDT_TYPE_CACD);
+			break;
+		default:
+			break;
+		}
+	}
+
+	if (l3_cache_id == -1) {
+		pr_info("ERDT: Failed to resolve L3 cache ID for RMDD domain %d\n",
+			rmdd->domain_id);
+
+		goto cleanup;
+	}
+
+	/* Require all RMDDs to support same set of sub-tables */
+	if (!valid_subtbl_mask) {
+		valid_subtbl_mask = subtbl_mask;
+	} else if (subtbl_mask != valid_subtbl_mask) {
+		pr_info(FW_BUG "Mismatch domain\n");
+		goto cleanup;
+	}
+
+	if (xa_insert(&erdt_domain_xa, l3_cache_id, domain_info, GFP_KERNEL)) {
+		pr_info("ERDT: Failed to store domain info for RMDD domain %d\n",
+			rmdd->domain_id);
+		goto cleanup;
+	}
+
+	return true;
+

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

+		return -EINVAL;
+	}
+
+	subtbl = (void *)erdt + sizeof(struct acpi_table_erdt);
+	table_end = (void *)erdt + erdt->header.length;
+
+	while (subtbl_valid(table_end, subtbl)) {
+		if (subtbl->type == ACPI_ERDT_TYPE_RMDD)
+			if (!parse_rmdd_entry(subtbl))
+				goto cleanup;
+
+		subtbl = next_subtbl(subtbl);
+	}
+
+	if (xa_empty(&erdt_domain_xa))
+		goto cleanup;
+
+	erdt_enabled_flag = true;
+
+	return 0;
+
+cleanup:
+	erdt_cleanup();
+	return -EINVAL;
+}

Hi Chenyu,

Quoted 3 lines · Chen Yu

From: Anil S Keshavamurthy <[email protected]>

ERDT(Enhanced RDT) introduces a new top-level ACPI structure

(nit: please add space between ERDT and the "(")

Quoted 4 lines

(the ERDT) that the kernel must parse before any enhanced RDT feature can be used. The ERDT improves the existing RDT by switching low-level register access from MSR-based to MMIO-based, which is more efficient.

(nit: please use full line length available for changelog)

Above states clearly "switching low-level register access from MSR-based to MMIO-based". This implementation clearly does so ... but the new ACPI tables supporting the MMIO-based access also include, for example, the maximum RMIDs that determines how many MMIO registers are supported. Even so, from what I can tell, the "maximum RMID" that the ACPI tables report is ignored and this implementation instead continues to rely on the maximum RMID reported via CPUID that indicates the maximum RMIDs supported by the *MSR* to also guide the access of the MMIO registers.

This implementation thus looks to straddle some middle ground between ACPI and CPUID enumeration, using return of one to guide access to the other that does not seem robust and requires platforms to forever support both.

Quoted 1 line

memory. As a trade-off, an xarray is used to store these tables, with

(nit: needs imperative eg. "use an xarray ...")

Quoted 9 lines

the L3 cache ID as the key.

  arch/x86/Kconfig                       |   4 +-
  arch/x86/include/asm/apic.h            |   1 +
  arch/x86/include/asm/resctrl.h         |   2 +
  arch/x86/kernel/cpu/resctrl/Makefile   |   1 +
  arch/x86/kernel/cpu/resctrl/core.c     |  14 +-
  arch/x86/kernel/cpu/resctrl/erdt.c     | 296 +++++++++++++++++++++++++
  arch/x86/kernel/cpu/resctrl/internal.h |   3 +
  arch/x86/kernel/cpu/topology.c         |   2 +-

Please split out the non-resctrl changes into separate patch with sybsystem-specific prefix to highlight these changes to not appear to sneak changes into other subsystems via resctrl. They can stil form part of this series as preparatory patches.

Quoted 9 lines

config X86_CPU_RESCTRL
bool "x86 CPU resource control support"

arch/x86/Kconfig +1 -1

-	depends on X86 && (CPU_SUP_INTEL || CPU_SUP_AMD)
+	depends on X86_64 && (CPU_SUP_INTEL || CPU_SUP_AMD)
  	depends on MISC_FILESYSTEMS
  	select ARCH_HAS_CPU_RESCTRL
  	select RESCTRL_FS

@@ -538,7 +538,7 @@ config X86_CPU_RESCTRL
 

arch/x86/Kconfig config X86_CPU_RESCTRL · line 544

Quoted 7 lines

arch/x86/Kconfig +1 -1

  config X86_CPU_RESCTRL_INTEL_AET
  	bool "Intel Application Energy Telemetry"
-	depends on X86_64 && X86_CPU_RESCTRL && CPU_SUP_INTEL && INTEL_PMT_TELEMETRY=y && INTEL_TPMI=y
+	depends on X86_CPU_RESCTRL && CPU_SUP_INTEL && INTEL_PMT_TELEMETRY=y && INTEL_TPMI=y
  	help
  	  Enable per-RMID telemetry events in resctrl.
 

Removing 32bit support is a significant change worth a mention and motivation in the changelog.

Quoted 8 lines

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

index 575f8408a9e7..97c2f6bc7a5f 100644
@@ -40,6 +40,8 @@ struct resctrl_pqr_state {
  	u32			default_closid;
  };
 

arch/x86/include/asm/resctrl.h struct resctrl_pqr_state { · line 44

Quoted 2 lines

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

+bool erdt_enabled(void);
+

I cannot see why this needs to be in asm header ... why not arch/x86/kernel/cpu/resctrl/internal.h?

arch/x86/include/asm/resctrl.h struct resctrl_pqr_state { · line 45

Quoted 1 line

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

+ * Enhanced Resource Director Technology(ERDT)

(nit: please add space)

arch/x86/include/asm/resctrl.h struct resctrl_pqr_state {

Quoted 6 lines

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

+#include <linux/cleanup.h>
+#include <linux/cpu.h>
+#include <linux/err.h>
+#include <linux/xarray.h>
+#include <linux/resctrl.h>
+#include <linux/acpi.h>

Please sort the includes alphabetically and add empty lines between the groups of headers.

arch/x86/include/asm/resctrl.h

Quoted 2 lines

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

+#include <asm/apic.h>
+#include <asm/cpu_device_id.h>

Why is above #include needed?

arch/x86/include/asm/resctrl.h

Quoted 2 lines

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

+#include "internal.h"
+

Please add a comment for the enum below to describe what the different members represent.

arch/x86/include/asm/resctrl.h

Quoted 5 lines

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

+enum erdt_mmio_type {
+	ERDT_MMIO_RMDD_CREG,
+	ERDT_MMIO_CMRC_BASE,
+	ERDT_MMIO_MAX
+};

Depending on how this enum ends up being used, could it benefit from a pattern like
https://lore.kernel.org/lkml/4a304e6c9ed3f5d92156b8045f5dd4ddba359dfc.1777419024.git.reinette.chatre@intel ?

arch/x86/include/asm/resctrl.h

Quoted 7 lines

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

+
+struct erdt_domain_info {
+	void __iomem		*base[ERDT_MMIO_MAX];
+	struct acpi_erdt_cmrc	*cmrc;
+};
+
+static bool erdt_enabled_flag;

What is significance of "flag"? Can it just be "erdt_enabled"? Since it is a bool this seems more intuitive?

arch/x86/include/asm/resctrl.h

Quoted 2 lines

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

+
+static DEFINE_XARRAY(erdt_domain_xa);

Please add comment to describe what is stored in erdt_domain_xa and what the index represents.

arch/x86/include/asm/resctrl.h

Quoted 3 lines

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

+
+#define ERDT_VALID_VERSION		1
+#define RMDD_FLAG_CPU_DOMAIN		BIT(0)

I think this name can be more accurate, how about, for example "RMDD_FLAG_L3_DOMAIN"?

arch/x86/include/asm/resctrl.h

Quoted 2 lines

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

+
+static u32 valid_subtbl_mask;

Please add comment to describe what above global represents.

arch/x86/include/asm/resctrl.h

Quoted 8 lines

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

+
+bool erdt_enabled(void)
+{
+	return erdt_enabled_flag;
+}
+
+/**
+ * get_l3_cache_id_from_cacd - Resolve L3 cache ID from CACD subtable

As I interpret it ... above does not describe to be what this function does.

Quoted 5 lines

+ * @cacd: Pointer to the ACPI ERDT CACD structure
+ *
+ * Parses the X2APIC ID list in the given CACD subtable to
+ * identify an online logical CPU and uses it to query the associated
+ * L3 cache ID. The first valid CPU found is used for this lookup.

This function is __init and requires at least one CPU per domain to be online. What will happen if a system is booted with all CPUs of a domain offline and then later a CPU is online'd?

Quoted 5 lines

+ *
+ * The L3 cache ID is used as a unique domain key for ERDT domain
+ * registration and lookup.
+ *
+ * Return: L3 cache ID for the first matching CPU, or -1 on failure.

Could you please use consistent terminology? Above comments switches between "online logical CPU" and "valid CPU" and "matching CPU" that all seem to refer to the same idea.

arch/x86/include/asm/resctrl.h

Quoted 7 lines

+ */

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

+static __init int get_l3_cache_id_from_cacd(struct acpi_erdt_cacd *cacd)
+{
+	int num_ids, cpu, online_cpu = -1, cache_id = -1, tmp;
+	struct cacheinfo *ci;
+
+	if (cacd->header.length < sizeof(*cacd) + sizeof(cacd->X2APICIDS[0])) {

Is above an open code of struct_size()?

arch/x86/include/asm/resctrl.h

Quoted 1 line

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

+		pr_warn(FW_BUG "Invalid x2apicid CACD table\n");

This patch displays several "FW_BUG" messages but they are sometimes printed with pr_info() and sometimes with pr_warn(). Is this intentional?

Quoted 5 lines

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

+		return -1;
+	}
+
+	num_ids = (cacd->header.length - sizeof(*cacd)) / sizeof(cacd->X2APICIDS[0]);
+

arch/x86/include/asm/resctrl.h

Quoted 7 lines

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

+	guard(cpus_read_lock)();
+
+	for (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]);
+

(nit: unnecessary empty line)

arch/x86/include/asm/resctrl.h

Quoted 7 lines

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

+			return -1;
+		}
+
+		if (!cpu_online(cpu))
+			continue;
+
+		tmp = get_cpu_cacheinfo_id(cpu, RESCTRL_L3_CACHE);

From what I can tell it is tmp that is essentially returned by this function and it
is the result of get_cpu_cacheinfo_id() that returns the cache ID initialized from CPUID leaf 0x4 ... the function name ("get_l3_cache_id_from_cacd()") and function comment "Resolve L3 cache ID from CACD subtable" implies differently and the way this is done makes me wonder why CACD needs to be parsed at all.

resctrl already has a hotplug handler and it dynamically determines the domain ID based on the cache ID returned from get_cpu_cacheinfo_id(). If the domain ID used by the new ERDT tables can be, as it appears is done here, trusted to what Linux already treating as domain ID then why is it needed to parse CACD at all? Why not just pick the domain ID from RMDD directly and use it directly as domain ID?

Later the domain ID from RMDD is just used in error messages ... never actually used as a domain ID to guide the implementation so the correlation appears to be implicit with this CACD parsing not intuitive.

Quoted 1 line

+static void __iomem *erdt_ioremap_checked(phys_addr_t base, u32 size, const char *desc)

"size" -> "num_pages"?

arch/x86/include/asm/resctrl.h

Quoted 2 lines

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

+{
+	void __iomem *addr = ioremap(base, size << 12);

The 12 magic constant is not ideal. Seems like PAGE_SHIFT is avoided since this *has* to be 4K? Would "num_pages * SZ_4K" be more accurate? To be most robust please consider check_mul_overflow() or check_shl_overflow().

It is not clear to me what the "_checked" of the function name implies but I do not see the parameters checked. For example, base could be checked for 4KB alignment?

arch/x86/include/asm/resctrl.h

Quoted 3 lines

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

+
+	if (!addr) {
+		pr_err("ERDT: Failed to map %s at phys addr %#llx (size: %u pages)\n",

phys_addt_t is printed with %pa

arch/x86/include/asm/resctrl.h

Quoted 1 line

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

+	domain_info = kzalloc(sizeof(*domain_info), GFP_KERNEL);

Please use kzalloc_obj().
commit 2932ba8d9c99 ("slab: Introduce kmalloc_obj() and family") contains details about benefits.

Quoted 6 lines

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

+	if (!domain_info)
+		return false;
+
+	domain_info->base[ERDT_MMIO_RMDD_CREG] =
+		erdt_ioremap_checked(rmdd->creg_base, rmdd->creg_size, "RMDD ctrl base");
+	if (!domain_info->base[ERDT_MMIO_RMDD_CREG])

arch/x86/include/asm/resctrl.h

Quoted 4 lines

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

+		goto cleanup;
+
+	for (subtbl = rmdd_subtbl(rmdd);
+	     subtbl_valid((void *)rmdd + rmdd->header.length, subtbl);

This does not look safe to me. What will happen if RMDD is empty? Would subtbl not point to end of the table and then subtbl_valid() would dereference pointer beyond the end of the table to do its checks?

arch/x86/include/asm/resctrl.h

Quoted 5 lines

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

+static void erdt_cleanup(void)
+{
+	struct erdt_domain_info *d;
+	unsigned long index;
+

Looks like the initialization via get_rdt_resources() does not do any error checking so there may be nothing to clean up and certainly there will be nothing to clean up if ERDT is not supported. Can the cleanup have a short circuit here to only clean up if the array is not empty or perhaps skip cleanup if !erdt_enabled?

Quoted 6 lines

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

+	xa_for_each(&erdt_domain_xa, index, d)
+		cleanup_one_domain(d);
+	xa_destroy(&erdt_domain_xa);
+}
+
+static __init int enumerate_erdt_table(struct acpi_table_header *table_hdr)

arch/x86/include/asm/resctrl.h

Quoted 7 lines

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

+{
+	struct acpi_table_erdt *erdt = (struct acpi_table_erdt *)table_hdr;
+	struct acpi_subtbl_hdr_16 *subtbl;
+	void *table_end;
+
+	if (erdt->header.revision != ERDT_VALID_VERSION) {
+		pr_info("Unknown ERDT table revision %d\n", erdt->header.revision);

(nit: "Unknown" -> "Unsupported"?)

arch/x86/include/asm/resctrl.h

Quoted 5 lines

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

+		return -EINVAL;
+	}
+
+	if (erdt->header.length < sizeof(*erdt)) {
+		pr_info(FW_BUG "ERDT: Invalid table length %u\n", erdt->header.length);

Here is an example of a "pr_info()" FW_BUG. It may help to add a unit and not just report "length"

arch/x86/include/asm/resctrl.h

Quoted 4 lines

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

+void erdt_exit(void)
+{
+	erdt_cleanup();
+}

Why is erdt_exit() and erdt_cleanup() both needed?

Quoted 1 line · Reinette Chatre

On 6/13/26 12:56 AM, Chen Yu wrote:

Quoted 9 lines

Quoted 8 lines

  arch/x86/Kconfig                       |   4 +- arch/x86/include/asm/apic.h            |   1 + arch/x86/include/asm/resctrl.h         |   2 + arch/x86/kernel/cpu/resctrl/Makefile   |   1 + arch/x86/kernel/cpu/resctrl/core.c     |  14 +- arch/x86/kernel/cpu/resctrl/erdt.c     | 296 +++++++++++++++++++++++++ arch/x86/kernel/cpu/resctrl/internal.h |   3 + arch/x86/kernel/cpu/topology.c         |   2 +-

Quoted 4 lines

Please split out the non-resctrl changes into separate patch with sybsystem-specific prefix to highlight these changes to not appear to sneak changes into other subsystems via resctrl. They can stil form part of this series as preparatory patches.

I see, let me split two of them out:
x86/topology: Export topo_lookup_cpuid for module use x86/apic: Add declaration for topo_lookup_cpuid

Quoted 11 lines

Quoted 11 lines

  config X86_CPU_RESCTRL
      bool "x86 CPU resource control support"
-    depends on X86 && (CPU_SUP_INTEL || CPU_SUP_AMD) +    depends on X86_64 && (CPU_SUP_INTEL || CPU_SUP_AMD) depends on MISC_FILESYSTEMS
      select ARCH_HAS_CPU_RESCTRL
      select RESCTRL_FS

(quoted diff) +0 -0

@@ -538,7 +538,7 @@ config X86_CPU_RESCTRL

    config X86_CPU_RESCTRL_INTEL_AET

Quoted 9 lines

Quoted 6 lines

      bool "Intel Application Energy Telemetry"
-    depends on X86_64 && X86_CPU_RESCTRL && CPU_SUP_INTEL && INTEL_PMT_TELEMETRY=y && INTEL_TPMI=y +    depends on X86_CPU_RESCTRL && CPU_SUP_INTEL && INTEL_PMT_TELEMETRY=y && INTEL_TPMI=y help
        Enable per-RMID telemetry events in resctrl.

Removing 32bit support is a significant change worth a mention and motivation in the changelog.

The original idea was that MMIO-based access (including CMT/MBM/MBA) uses readq/writeq, which depend on 64-bit. Since MMIO CMT relies on CONFIG_X86_CPU_RESCTRL, making CONFIG_X86_CPU_RESCTRL depend on X86_64 would simplify the code.
If I understand correctly, 32-bit x86 lacks practical hardware support for RDT/QoS. The Kconfig entry currently depends on X86, which technically permits 32-bit builds. However, I am unsure whether there exists any real-world use case where a user compiles a 32-bit kernel on a server and enables resctrl?
If we wish to retain 32-bit RDT support, I can introduce a new config option to gate ERDT:
config X86_CPU_RESCTRL_ERDT
    bool
    depends on X86_CPU_RESCTRL && X86_64 && ACPI
    default y
Could you advise which approach is preferable?

Quoted 12 lines

arch/x86/include/asm/resctrl.h struct resctrl_pqr_state { · line 43

Quoted 9 lines

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

index 575f8408a9e7..97c2f6bc7a5f 100644
@@ -40,6 +40,8 @@ struct resctrl_pqr_state {
       u32            default_closid;
   };
   +bool erdt_enabled(void);
+

I cannot see why this needs to be in asm header ... why not arch/x86/kernel/cpu/resctrl/internal.h?

OK, let me move it into internal.h. By the way, I believe fs/resctrl also needs to call erdt_enabled() for region-aware MBA/MBM. Would it be acceptable to expose erdt_enabled() in include/linux/resctrl.h?

Quoted 11 lines

arch/x86/include/asm/resctrl.h struct resctrl_pqr_state {

Quoted 7 lines

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

+
+struct erdt_domain_info {
+    void __iomem        *base[ERDT_MMIO_MAX];
+    struct acpi_erdt_cmrc    *cmrc;
+};
+
+static bool erdt_enabled_flag;

What is significance of "flag"? Can it just be "erdt_enabled"? Since it is a bool this seems more intuitive?

erdt_enabled() was already used as a function name. Let me change it to static bool is_enabled

Quoted 6 lines

arch/x86/include/asm/resctrl.h

Quoted 3 lines

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

+
+#define ERDT_VALID_VERSION        1
+#define RMDD_FLAG_CPU_DOMAIN        BIT(0)

I think this name can be more accurate, how about, for example "RMDD_FLAG_L3_DOMAIN"?

According to the spec, this flag indicates: CPU L3 cache and I/O L3 Domain, maybe using RMDD_FLAG_CPU_L3_DOMAIN?

Quoted 20 lines

arch/x86/include/asm/resctrl.h

Quoted 7 lines

+            return -1;
+        }

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

+
+        if (!cpu_online(cpu))
+            continue;
+
+        tmp = get_cpu_cacheinfo_id(cpu, RESCTRL_L3_CACHE);

 From what I can tell it is tmp that is essentially returned by this function and it is the result of get_cpu_cacheinfo_id() that returns the cache ID initialized from CPUID leaf 0x4 ... the function name ("get_l3_cache_id_from_cacd()") and function comment "Resolve L3 cache ID from CACD subtable" implies differently and the way this is done makes me wonder why CACD needs to be parsed at all.

resctrl already has a hotplug handler and it dynamically determines the domain ID based on the cache ID returned from get_cpu_cacheinfo_id(). If the domain ID used by the new ERDT tables can be, as it appears is done here, trusted to what Linux already treating as domain ID then why is it needed to parse CACD at all? Why not just pick the domain ID from RMDD directly and use it directly as domain ID?

Quoted 4 lines

Later the domain ID from RMDD is just used in error messages ... never actually used as a domain ID to guide the implementation so the correlation appears to be implicit with this CACD parsing not intuitive.

You are right that the cache ID ultimately comes from get_cpu_cacheinfo_id(), not from CACD directly - the function name is misleading and should be improved.

However, my understanding is that CACD parsing acts as the bridge between ACPI's RMDD domain namespace and the kernel's L3 domain ID: CACD enumerates which CPUs belong to each RMDD, and from those CPUs we derive the kernel-side L3 cache ID. The specification defines an RMDD DomainID as an identifier unique within the ERDT table, yet it does not guarantee that this ID matches the kernel’s cache ID obtained via CPUID leaf 0x4. For this reason, we omitted the RMDD domain ID in the current release.

Hi Chenyu,

Quoted 2 lines · Yu C

Quoted 1 line · Reinette Chatre

On 6/13/26 12:56 AM, Chen Yu wrote:

...

Quoted 14 lines

Quoted 13 lines

Quoted 8 lines

   arch/x86/Kconfig                       |   4 +-
   arch/x86/include/asm/apic.h            |   1 +
   arch/x86/include/asm/resctrl.h         |   2 +
   arch/x86/kernel/cpu/resctrl/Makefile   |   1 +
   arch/x86/kernel/cpu/resctrl/core.c     |  14 +-
   arch/x86/kernel/cpu/resctrl/erdt.c     | 296 +++++++++++++++++++++++++
   arch/x86/kernel/cpu/resctrl/internal.h |   3 +
   arch/x86/kernel/cpu/topology.c         |   2 +-

Please split out the non-resctrl changes into separate patch with sybsystem-specific prefix to highlight these changes to not appear to sneak changes into other subsystems via resctrl. They can stil form part of this series as preparatory patches.

Quoted 3 lines

I see, let me split two of them out:
x86/topology: Export topo_lookup_cpuid for module use x86/apic: Add declaration for topo_lookup_cpuid

I think a single patch would be ok. Interestingly the subject prefix to this area is not consistent ... looks like either "x86/topology" or "x86/cpu/topology" would be ok.

Quoted 21 lines

Quoted 11 lines

Quoted 11 lines

  config X86_CPU_RESCTRL
      bool "x86 CPU resource control support"
-    depends on X86 && (CPU_SUP_INTEL || CPU_SUP_AMD) +    depends on X86_64 && (CPU_SUP_INTEL || CPU_SUP_AMD)
      depends on MISC_FILESYSTEMS
      select ARCH_HAS_CPU_RESCTRL
      select RESCTRL_FS

arch/x86/Kconfig +0 -0

@@ -538,7 +538,7 @@ config X86_CPU_RESCTRL
     config X86_CPU_RESCTRL_INTEL_AET

Quoted 9 lines

arch/x86/Kconfig config X86_CPU_RESCTRL · line 544

Quoted 6 lines

arch/x86/Kconfig +1 -1

       bool "Intel Application Energy Telemetry"
-    depends on X86_64 && X86_CPU_RESCTRL && CPU_SUP_INTEL && INTEL_PMT_TELEMETRY=y && INTEL_TPMI=y
+    depends on X86_CPU_RESCTRL && CPU_SUP_INTEL && INTEL_PMT_TELEMETRY=y && INTEL_TPMI=y
       help
         Enable per-RMID telemetry events in resctrl.
 

Removing 32bit support is a significant change worth a mention and motivation in the changelog.

Quoted 13 lines

The original idea was that MMIO-based access (including CMT/MBM/MBA) uses readq/writeq, which depend on 64-bit. Since MMIO CMT relies on CONFIG_X86_CPU_RESCTRL, making CONFIG_X86_CPU_RESCTRL depend on X86_64 would simplify the code.
If I understand correctly, 32-bit x86 lacks practical hardware support for RDT/QoS. The Kconfig entry currently depends on X86, which technically permits 32-bit builds. However, I am unsure whether there exists any real-world use case where a user compiles a 32-bit kernel on a server and enables resctrl?
If we wish to retain 32-bit RDT support, I can introduce a new config option to gate ERDT:
config X86_CPU_RESCTRL_ERDT
    bool
    depends on X86_CPU_RESCTRL && X86_64 && ACPI
    default y
Could you advise which approach is preferable?

x86 resctrl will soon depend on 64bit, this dependency will arrive via this series or the AET reworks. Whether x86 resctrl depends on 64bit is not the question here.

My question is more specific though:
What in *this* patch requires dropping 32bit support?
You highlight above that 64bit support is needed for readq/writeq. That is understood. Even so, this patch does not have any instances of readq/writeq. This patch thus turns off 32bit support without any mention and without any reason to do so. This is a very significant change to just sneak into a patch that does not appear to require such change.

Quoted 13 lines

Quoted 12 lines

arch/x86/include/asm/resctrl.h struct resctrl_pqr_state { · line 43

Quoted 9 lines

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

index 575f8408a9e7..97c2f6bc7a5f 100644
@@ -40,6 +40,8 @@ struct resctrl_pqr_state {
       u32            default_closid;
   };
   +bool erdt_enabled(void);
+

I cannot see why this needs to be in asm header ... why not arch/x86/kernel/cpu/resctrl/internal.h?

Quoted 3 lines

OK, let me move it into internal.h. By the way, I believe fs/resctrl also needs to call erdt_enabled() for region-aware MBA/MBM. Would it be acceptable to expose erdt_enabled() in include/linux/resctrl.h?

I do not see any calls to erdt_enabled() from resctrl fs in this series. I also do not think such calls would be appropriate. Whether x86's RDT is backed with CPUID or ACPI is an architectural detail of no concern to resctrl fs.

Quoted 12 lines

Quoted 11 lines

arch/x86/include/asm/resctrl.h struct resctrl_pqr_state {

Quoted 7 lines

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

+
+struct erdt_domain_info {
+    void __iomem        *base[ERDT_MMIO_MAX];
+    struct acpi_erdt_cmrc    *cmrc;
+};
+
+static bool erdt_enabled_flag;

What is significance of "flag"? Can it just be "erdt_enabled"? Since it is a bool this seems more intuitive?

Quoted 1 line

erdt_enabled() was already used as a function name. Let me change it to static bool is_enabled

This could also be named "__erdt_enabled" to highlight that it is internal.

Quoted 20 lines

Quoted 12 lines

 From what I can tell it is tmp that is essentially returned by this function and it
is the result of get_cpu_cacheinfo_id() that returns the cache ID initialized from CPUID leaf 0x4 ... the function name ("get_l3_cache_id_from_cacd()") and function comment "Resolve L3 cache ID from CACD subtable" implies differently and the way this is done makes me wonder why CACD needs to be parsed at all.

resctrl already has a hotplug handler and it dynamically determines the domain ID based on the cache ID returned from get_cpu_cacheinfo_id(). If the domain ID used by the new ERDT tables can be, as it appears is done here, trusted to what Linux already treating as domain ID then why is it needed to parse CACD at all? Why not just pick the domain ID from RMDD directly and use it directly as domain ID?

Quoted 4 lines

Later the domain ID from RMDD is just used in error messages ... never actually used as a domain ID to guide the implementation so the correlation appears to be implicit with this CACD parsing not intuitive.

You are right that the cache ID ultimately comes from get_cpu_cacheinfo_id(), not from CACD directly - the function name is misleading and should be improved.

Quoted 7 lines

However, my understanding is that CACD parsing acts as the bridge between ACPI's RMDD domain namespace and the kernel's L3 domain ID: CACD enumerates which CPUs belong to each RMDD, and from those CPUs we derive the kernel-side L3 cache ID. The specification defines an RMDD DomainID as an identifier unique within the ERDT table, yet it does not guarantee that this ID matches the kernel’s cache ID obtained via CPUID leaf 0x4. For this reason, we omitted the RMDD domain ID in the current release.

get_l3_cache_id_from_cacd() implies a straightforward query and the function comments supports this. At the same time the function includes what appears to be sanity checks but from what you say turns out to be essential enforcement of the RMDD to CPUID leaf 0x4 mapping. Is this correct? As sanity checks these additional checks are ok but as RMDD to CPUID leaf 0x4 mapping enforcement I find them inadequate because its correctness requires all CPUs of all domains to be online which cannot be guaranteed when this initialization runs.

Consider for example a scenario where RMDD and CPUID do not agree on which CPUs form part of a domain. If only one CPU of this RMDD domain is online at the time this initialization is done then the mismatch will never be noticed and resctrl will just silently use the CPUID mapping, no?

Quoted 36 lines

Quoted 20 lines

Quoted 12 lines

  From what I can tell it is tmp that is essentially returned by this function and it is the result of get_cpu_cacheinfo_id() that returns the cache ID initialized from CPUID leaf 0x4 ... the function name ("get_l3_cache_id_from_cacd()") and function comment "Resolve L3 cache ID from CACD subtable" implies differently and the way this is done makes me wonder why CACD needs to be parsed at all.

resctrl already has a hotplug handler and it dynamically determines the domain ID based on the cache ID returned from get_cpu_cacheinfo_id(). If the domain ID used by the new ERDT tables can be, as it appears is done here, trusted to what Linux already treating as domain ID then why is it needed to parse CACD at all? Why not just pick the domain ID from RMDD directly and use it directly as domain ID?

Quoted 4 lines

Later the domain ID from RMDD is just used in error messages ... never actually used as a domain ID to guide the implementation so the correlation appears to be implicit with this CACD parsing not intuitive.

You are right that the cache ID ultimately comes from get_cpu_cacheinfo_id(), not from CACD directly - the function name is misleading and should be improved.

Quoted 7 lines

However, my understanding is that CACD parsing acts as the bridge between ACPI's RMDD domain namespace and the kernel's L3 domain ID: CACD enumerates which CPUs belong to each RMDD, and from those CPUs we derive the kernel-side L3 cache ID. The specification defines an RMDD DomainID as an identifier unique within the ERDT table, yet it does not guarantee that this ID matches the kernel’s cache ID obtained via CPUID leaf 0x4. For this reason, we omitted the RMDD domain ID in the current release.

get_l3_cache_id_from_cacd() implies a straightforward query and the function comments supports this. At the same time the function includes what appears to be sanity checks but from what you say turns out to be essential enforcement of the RMDD to CPUID leaf 0x4 mapping. Is this correct? As sanity checks these additional checks are ok but as RMDD to CPUID leaf 0x4 mapping enforcement I find them inadequate because its correctness requires all CPUs of all domains to be online which cannot be guaranteed when this initialization runs.

Quoted 5 lines

Consider for example a scenario where RMDD and CPUID do not agree on which CPUs form part of a domain. If only one CPU of this RMDD domain is online at the time this initialization is done then the mismatch will never be noticed and resctrl will just silently use the CPUID mapping, no?

I suppose you are referring to the following scenario:
CACD says CPUs {0,1,2,3} belong to RMDD-A, and CPUID says CPUs {0,1} share L3-X while CPUs {2,3} share L3-Y (a mismatch). If only CPU 0 is brought online:

get_l3_cache_id_from_cacd() iterates CACD's X2APIC list: {0,1,2,3} CPU 1, 2, 3 are offline -> skipped
CPU 0 is online -> cache_id = get_cpu_cacheinfo_id(0) = L3-X, all passed.

Then later CPU2 is online, resctrl's hotplug creates domain L3-Y. But the xarray has no entry for L3-Y, so erdt_mon_read() will return -EIO for that domain.

If this is the case, I wonder if we could run the sanity check during CPU hotplug to reparse the CACD table whenever a CPU comes online. My thought would be to call get_l3_cache_id_from_cacd() within resctrl_arch_online_cpu().

Quoted 47 lines · Yu C

Quoted 36 lines

Quoted 20 lines

Quoted 12 lines

  From what I can tell it is tmp that is essentially returned by this function and it is the result of get_cpu_cacheinfo_id() that returns the cache ID initialized from CPUID leaf 0x4 ... the function name ("get_l3_cache_id_from_cacd()") and function comment "Resolve L3 cache ID from CACD subtable" implies differently and the way this is done makes me wonder why CACD needs to be parsed at all.

resctrl already has a hotplug handler and it dynamically determines the domain ID based on the cache ID returned from get_cpu_cacheinfo_id(). If the domain ID used by the new ERDT tables can be, as it appears is done here, trusted to what Linux already treating as domain ID then why is it needed to parse CACD at all? Why not just pick the domain ID from RMDD directly and use it directly as domain ID?

Quoted 4 lines

Later the domain ID from RMDD is just used in error messages ... never actually used as a domain ID to guide the implementation so the correlation appears to be implicit with this CACD parsing not intuitive.

You are right that the cache ID ultimately comes from get_cpu_cacheinfo_id(), not from CACD directly - the function name is misleading and should be improved.

Quoted 7 lines

However, my understanding is that CACD parsing acts as the bridge between ACPI's RMDD domain namespace and the kernel's L3 domain ID: CACD enumerates which CPUs belong to each RMDD, and from those CPUs we derive the kernel-side L3 cache ID. The specification defines an RMDD DomainID as an identifier unique within the ERDT table, yet it does not guarantee that this ID matches the kernel’s cache ID obtained via CPUID leaf 0x4. For this reason, we omitted the RMDD domain ID in the current release.

get_l3_cache_id_from_cacd() implies a straightforward query and the function comments supports this. At the same time the function includes what appears to be sanity checks but from what you say turns out to be essential enforcement of the RMDD to CPUID leaf 0x4 mapping. Is this correct? As sanity checks these additional checks are ok but as RMDD to CPUID leaf 0x4 mapping enforcement I find them inadequate because its correctness requires all CPUs of all domains to be online which cannot be guaranteed when this initialization runs.

Quoted 5 lines

Consider for example a scenario where RMDD and CPUID do not agree on which CPUs form part of a domain. If only one CPU of this RMDD domain is online at the time this initialization is done then the mismatch will never be noticed and resctrl will just silently use the CPUID mapping, no?

I suppose you are referring to the following scenario:
CACD says CPUs {0,1,2,3} belong to RMDD-A, and CPUID says CPUs {0,1} share L3-X while CPUs {2,3} share L3-Y (a mismatch). If only CPU 0 is brought online:

Quoted 8 lines

get_l3_cache_id_from_cacd() iterates CACD's X2APIC list: {0,1,2,3} CPU 1, 2, 3 are offline -> skipped
CPU 0 is online -> cache_id = get_cpu_cacheinfo_id(0) = L3-X, all passed.

Then later CPU2 is online, resctrl's hotplug creates domain L3-Y. But the xarray has no entry for L3-Y, so erdt_mon_read() will return -EIO for that domain.

Quoted 6 lines

If this is the case, I wonder if we could run the sanity check during CPU hotplug to reparse the CACD table whenever a CPU comes online. My thought would be to call get_l3_cache_id_from_cacd() within resctrl_arch_online_cpu().

In fact, parsing the CACD table during CPU hotplug may not be feasible, as the ACPI tables could have already been released. We could instead perform the check directly against the erdt_domain_xa xarray. If a mismatch is detected, disable ERDT and fall back to legacy MSR-based access:

/* Called from resctrl_arch_online_cpu() */
void erdt_verify_cpu_domain(int cpu)
{
    int cache_id;

    if (!__erdt_enabled)
        return;

    cache_id = get_cpu_cacheinfo_id(cpu, RESCTRL_L3_CACHE); if (cache_id < 0)
        return;

    if (!xa_load(&erdt_domain_xa, cache_id)) {
        pr_warn(FW_BUG "CPU%d L3 domain %d not described by ERDT, disabling ERDT\n", cpu, cache_id);
        __erdt_enabled = false;
    }
}

Hi Chenyu,

Quoted 48 lines · Yu C

Quoted 47 lines · Yu C

Quoted 36 lines

Quoted 20 lines

Quoted 12 lines

  From what I can tell it is tmp that is essentially returned by this function and it
is the result of get_cpu_cacheinfo_id() that returns the cache ID initialized from CPUID leaf 0x4 ... the function name ("get_l3_cache_id_from_cacd()") and function comment "Resolve L3 cache ID from CACD subtable" implies differently and the way this is done makes me wonder why CACD needs to be parsed at all.

resctrl already has a hotplug handler and it dynamically determines the domain ID based on the cache ID returned from get_cpu_cacheinfo_id(). If the domain ID used by the new ERDT tables can be, as it appears is done here, trusted to what Linux already treating as domain ID then why is it needed to parse CACD at all? Why not just pick the domain ID from RMDD directly and use it directly as domain ID?

Quoted 4 lines

Later the domain ID from RMDD is just used in error messages ... never actually used as a domain ID to guide the implementation so the correlation appears to be implicit with this CACD parsing not intuitive.

You are right that the cache ID ultimately comes from get_cpu_cacheinfo_id(), not from CACD directly - the function name is misleading and should be improved.

Quoted 7 lines

However, my understanding is that CACD parsing acts as the bridge between ACPI's RMDD domain namespace and the kernel's L3 domain ID: CACD enumerates which CPUs belong to each RMDD, and from those CPUs we derive the kernel-side L3 cache ID. The specification defines an RMDD DomainID as an identifier unique within the ERDT table, yet it does not guarantee that this ID matches the kernel’s cache ID obtained via CPUID leaf 0x4. For this reason, we omitted the RMDD domain ID in the current release.

get_l3_cache_id_from_cacd() implies a straightforward query and the function comments supports this. At the same time the function includes what appears to be sanity checks but from what you say turns out to be essential enforcement of the RMDD to CPUID leaf 0x4 mapping. Is this correct? As sanity checks these additional checks are ok but as RMDD to CPUID leaf 0x4 mapping enforcement I find them inadequate because its correctness requires all CPUs of all domains to be online which cannot be guaranteed when this initialization runs.

Quoted 5 lines

Consider for example a scenario where RMDD and CPUID do not agree on which CPUs form part of a domain. If only one CPU of this RMDD domain is online at the time this initialization is done then the mismatch will never be noticed and resctrl will just silently use the CPUID mapping, no?

I suppose you are referring to the following scenario:
CACD says CPUs {0,1,2,3} belong to RMDD-A, and CPUID says CPUs {0,1} share L3-X while CPUs {2,3} share L3-Y (a mismatch). If only CPU 0 is brought online:

Quoted 8 lines

Quoted 8 lines

get_l3_cache_id_from_cacd() iterates CACD's X2APIC list: {0,1,2,3} CPU 1, 2, 3 are offline -> skipped
CPU 0 is online -> cache_id = get_cpu_cacheinfo_id(0) = L3-X, all passed.

Then later CPU2 is online, resctrl's hotplug creates domain L3-Y. But the xarray has no entry for L3-Y, so erdt_mon_read() will return -EIO for that domain.

An error would at least give indication of a problem but I also see a possible scenario where the data will happily be read from the wrong register. Consider the following change to the scenario you present:
CACD: RMDD-A = {0, 2}; RMDD-B = {1, 3}
CPUID: L3-X = {0, 1}; L3-Y = {2, 3}

If ACPI enumeration is done with CPUs 1, 2, and 3 offline then they are skipped. CPU 0 is online -> cache_id = get_cpu_cacheinfo_id(0) = L3-X

Later when CPU 1 comes online resctrl will consider it to be in L3-X and also find an xarray entry for L3-X which will result in erdt_mon_read() to read the RMID data from RMDD-A instead of RMDD-B?

Quoted 25 lines

Quoted 6 lines

If this is the case, I wonder if we could run the sanity check during CPU hotplug to reparse the CACD table whenever a CPU comes online. My thought would be to call get_l3_cache_id_from_cacd() within resctrl_arch_online_cpu().

In fact, parsing the CACD table during CPU hotplug may not be feasible, as the ACPI tables could have already been released. We could instead perform the check directly against the erdt_domain_xa xarray. If a mismatch is detected, disable ERDT and fall back to legacy MSR-based access:

/* Called from resctrl_arch_online_cpu() */
void erdt_verify_cpu_domain(int cpu)
{
    int cache_id;

    if (!__erdt_enabled)
        return;

    cache_id = get_cpu_cacheinfo_id(cpu, RESCTRL_L3_CACHE); if (cache_id < 0)
        return;

Quoted 6 lines

    if (!xa_load(&erdt_domain_xa, cache_id)) {
        pr_warn(FW_BUG "CPU%d L3 domain %d not described by ERDT, disabling ERDT\n", cpu, cache_id);
        __erdt_enabled = false;
    }
}

When considering the example scenario above I do not believe that this implements a robust transition from ACPI to CPUID.

Additionally, disabling ERDT during a CPU online has broader implications: if ERDT was enabled earlier then the associated events will be enabled at this point and have mov_evt::any_cpu set. From what I can tell this would just implicitly expect that the events could still be read via MSR but not actually route the read to the correct CPU?

I think it would be simpler to separate the ACPI enumeration from the CPUID enumeration instead of wedging in these snippets at various touch points. The ACPI enumeration could, for example, just record the ERDT RMDD domain <-> cpu_mask mapping without taking online CPUs into account. Depending on which tables are found in ACPI, if ERDT is the intended mechanism then any duplicate CPUID enumeration should be avoided. This means that there will be two unique flows to enabling resctrl events, one for ACPI and one for CPUID. The x86 online handler can perform the sanity check when a CPU comes online and since RMDD and CPUID are expected to correlate the CPU could not be added to resctrl if it is not in the expected domain. To me this seems simpler and more robust.

Quoted 69 lines · Reinette Chatre

Hi Chenyu,

Quoted 48 lines · Yu C

Quoted 47 lines · Yu C

Quoted 36 lines

Quoted 20 lines

Quoted 12 lines

   From what I can tell it is tmp that is essentially returned by this function and it is the result of get_cpu_cacheinfo_id() that returns the cache ID initialized from CPUID leaf 0x4 ... the function name ("get_l3_cache_id_from_cacd()") and function comment "Resolve L3 cache ID from CACD subtable" implies differently and the way this is done makes me wonder why CACD needs to be parsed at all.

resctrl already has a hotplug handler and it dynamically determines the domain ID based on the cache ID returned from get_cpu_cacheinfo_id(). If the domain ID used by the new ERDT tables can be, as it appears is done here, trusted to what Linux already treating as domain ID then why is it needed to parse CACD at all? Why not just pick the domain ID from RMDD directly and use it directly as domain ID?

Quoted 4 lines

Later the domain ID from RMDD is just used in error messages ... never actually used as a domain ID to guide the implementation so the correlation appears to be implicit with this CACD parsing not intuitive.

You are right that the cache ID ultimately comes from get_cpu_cacheinfo_id(), not from CACD directly - the function name is misleading and should be improved.

Quoted 7 lines

However, my understanding is that CACD parsing acts as the bridge between ACPI's RMDD domain namespace and the kernel's L3 domain ID: CACD enumerates which CPUs belong to each RMDD, and from those CPUs we derive the kernel-side L3 cache ID. The specification defines an RMDD DomainID as an identifier unique within the ERDT table, yet it does not guarantee that this ID matches the kernel’s cache ID obtained via CPUID leaf 0x4. For this reason, we omitted the RMDD domain ID in the current release.

get_l3_cache_id_from_cacd() implies a straightforward query and the function comments supports this. At the same time the function includes what appears to be sanity checks but from what you say turns out to be essential enforcement of the RMDD to CPUID leaf 0x4 mapping. Is this correct? As sanity checks these additional checks are ok but as RMDD to CPUID leaf 0x4 mapping enforcement I find them inadequate because its correctness requires all CPUs of all domains to be online which cannot be guaranteed when this initialization runs.

Quoted 5 lines

Consider for example a scenario where RMDD and CPUID do not agree on which CPUs form part of a domain. If only one CPU of this RMDD domain is online at the time this initialization is done then the mismatch will never be noticed and resctrl will just silently use the CPUID mapping, no?

I suppose you are referring to the following scenario:
CACD says CPUs {0,1,2,3} belong to RMDD-A, and CPUID says CPUs {0,1} share L3-X while CPUs {2,3} share L3-Y (a mismatch). If only CPU 0 is brought online:

Quoted 8 lines

Quoted 8 lines

get_l3_cache_id_from_cacd() iterates CACD's X2APIC list: {0,1,2,3} CPU 1, 2, 3 are offline -> skipped
CPU 0 is online -> cache_id = get_cpu_cacheinfo_id(0) = L3-X, all passed.

Then later CPU2 is online, resctrl's hotplug creates domain L3-Y. But the xarray has no entry for L3-Y, so erdt_mon_read() will return -EIO for that domain.

An error would at least give indication of a problem but I also see a possible scenario where the data will happily be read from the wrong register. Consider the following change to the scenario you present:
CACD: RMDD-A = {0, 2}; RMDD-B = {1, 3}
CPUID: L3-X = {0, 1}; L3-Y = {2, 3}

If ACPI enumeration is done with CPUs 1, 2, and 3 offline then they are skipped. CPU 0 is online -> cache_id = get_cpu_cacheinfo_id(0) = L3-X

Quoted 4 lines

Later when CPU 1 comes online resctrl will consider it to be in L3-X and also find an xarray entry for L3-X which will result in erdt_mon_read() to read the RMID data from RMDD-A instead of RMDD-B?

Yes, this approach is feasible. However, I am uncertain whether RDT should be enabled. The underlying issue is that the firmware(BIOS) and silicon(CPUID) will be out of sync, leaving RDT in an unstable state.

Quoted 75 lines · Yu C

Quoted 73 lines · Reinette Chatre

Hi Chenyu,

Quoted 48 lines · Yu C

Quoted 47 lines · Yu C

Quoted 36 lines

Quoted 20 lines

Quoted 12 lines

   From what I can tell it is tmp that is essentially returned by this function and it is the result of get_cpu_cacheinfo_id() that returns the cache ID initialized from CPUID leaf 0x4 ... the function name ("get_l3_cache_id_from_cacd()") and function comment "Resolve L3 cache ID from CACD subtable" implies differently and the way this is done makes me wonder why CACD needs to be parsed at all.

resctrl already has a hotplug handler and it dynamically determines the domain ID based on the cache ID returned from get_cpu_cacheinfo_id(). If the domain ID used by the new ERDT tables can be, as it appears is done here, trusted to what Linux already treating as domain ID then why is it needed to parse CACD at all? Why not just pick the domain ID from RMDD directly and use it directly as domain ID?

Quoted 4 lines

Later the domain ID from RMDD is just used in error messages ... never actually used as a domain ID to guide the implementation so the correlation appears to be implicit with this CACD parsing not intuitive.

You are right that the cache ID ultimately comes from get_cpu_cacheinfo_id(), not from CACD directly - the function name is misleading and should be improved.

Quoted 7 lines

However, my understanding is that CACD parsing acts as the bridge between ACPI's RMDD domain namespace and the kernel's L3 domain ID: CACD enumerates which CPUs belong to each RMDD, and from those CPUs we derive the kernel-side L3 cache ID. The specification defines an RMDD DomainID as an identifier unique within the ERDT table, yet it does not guarantee that this ID matches the kernel’s cache ID obtained via CPUID leaf 0x4. For this reason, we omitted the RMDD domain ID in the current release.

get_l3_cache_id_from_cacd() implies a straightforward query and the function comments supports this. At the same time the function includes what appears to be sanity checks but from what you say turns out to be essential enforcement of the RMDD to CPUID leaf 0x4 mapping. Is this correct? As sanity checks these additional checks are ok but as RMDD to CPUID leaf 0x4 mapping enforcement I find them inadequate because its correctness requires all CPUs of all domains to be online which cannot be guaranteed when this initialization runs.

Quoted 5 lines

Consider for example a scenario where RMDD and CPUID do not agree on which CPUs form part of a domain. If only one CPU of this RMDD domain is online at the time this initialization is done then the mismatch will never be noticed and resctrl will just silently use the CPUID mapping, no?

I suppose you are referring to the following scenario:
CACD says CPUs {0,1,2,3} belong to RMDD-A, and CPUID says CPUs {0,1} share L3-X while CPUs {2,3} share L3-Y (a mismatch). If only CPU 0 is brought online:

Quoted 8 lines

Quoted 8 lines

get_l3_cache_id_from_cacd() iterates CACD's X2APIC list: {0,1,2,3} CPU 1, 2, 3 are offline -> skipped
CPU 0 is online -> cache_id = get_cpu_cacheinfo_id(0) = L3-X, all passed.

Then later CPU2 is online, resctrl's hotplug creates domain L3-Y. But the xarray has no entry for L3-Y, so erdt_mon_read() will return -EIO for that domain.

An error would at least give indication of a problem but I also see a possible scenario where the data will happily be read from the wrong register. Consider the following change to the scenario you present:
CACD: RMDD-A = {0, 2}; RMDD-B = {1, 3}
CPUID: L3-X = {0, 1}; L3-Y = {2, 3}

If ACPI enumeration is done with CPUs 1, 2, and 3 offline then they are skipped. CPU 0 is online -> cache_id = get_cpu_cacheinfo_id(0) = L3-X

Later when CPU 1 comes online resctrl will consider it to be in L3-X and also find an xarray entry for L3-X which will result in erdt_mon_read() to read the RMID data from RMDD-A instead of RMDD-B?

Quoted 4 lines

Yes, this approach is feasible. However, I am uncertain whether RDT should be enabled. The underlying issue is that the firmware(BIOS) and silicon(CPUID) will be out of sync, leaving RDT in an unstable state.

That said, there remains an open question as to whether we ought to enable RDT on platforms where the domain information returned by firmware (BIOS) is inconsistent with the CPUID values exposed by the silicon.

Thanks for the detailed feedback. If I understand correctly, separating ACPI enumeration from CPUID enumeration would look something like the following. Could you let me know if this approach is feasible?

----------
1. At ACPI parse time , record the full CPU mask from CACD for each RMDD domain without checking online state or resolving L3 cache IDs.

Store a per-CPU pointer (erdt_cpu_domain[cpu]) for O(1) lookup.

2. At CPU online time (resctrl_arch_online_cpu), validate that the CPU's CPUID-derived L3 ID is consistent with other CPUs in the same RMDD:
- First online CPU in an RMDD establishes the L3 ID for that domain and inserts the domain into erdt_domain_xa keyed by L3 cache ID.
- Subsequent CPUs must match. On mismatch, the CPU is refused from resctrl.

3. erdt_mon_read() looks up erdt_domain_xa by hdr->id (L3 cache ID) as before, but the xarray is now populated lazily at CPU online time.

I do not see the need for an xarray. As new domains are created during CPU online their initialization can just point to the needed data, no?

Quoted 2 lines

I do not see the need for an xarray. As new domains are created during CPU online their initialization can just point to the needed data, no?

Reinette,

Historical note. The xarray was added during early development on a simulator where the L3 domain numbers were wildly sparse. That turned out to be an artifact of the simulator CPUID and BIOS interactions. On real h/w the L3 domains show up as 0, 1, 2, 3, ...

But the xarray does still serve some purpose. Without it you’d need two passes through the ACPI ERDT structure. One to count how many RMDD substructures are present to allocate space, then second pass to parse things.

Hi Tony,

Quoted 8 lines · Tony

Quoted 2 lines

I do not see the need for an xarray. As new domains are created during CPU online their initialization can just point to the needed data, no?

Reinette,

Historical note. The xarray was added during early development on a simulator where the L3 domain numbers were wildly sparse. That turned out to be an artifact of the simulator CPUID and BIOS interactions. On real h/w the L3 domains show up as 0, 1, 2, 3, ...

resctrl dynamically allocates monitoring domains as the CPUs come online and the monitoring domains are assigned the domain ID discovered from the system. This should not be impacted by sparse domain numbers and works without an xarray, no? What am I missing?

Quoted 3 lines

But the xarray does still serve some purpose. Without it you’d need two passes through the ACPI ERDT structure. One to count how many RMDD substructures are present to allocate space, then second pass to parse things.

The data structures containing the domain data discovered from ERDT could just be dynamically allocated with a single pass as the domains are discovered, similar to how rdt_resource::mon_domains is populated, no? The difference is that the "ERDT domains" are discovered during initialization.

I do not see a need for a separate xarray indexed by domain ID during "runtime" since we already have the architecture specific domain structures. struct rdt_l3_mon_domain could just point to its associated ERDT domain when it (struct rdt_l3_mon_domain) is initialized, no?

You could use an xarray during initial RMDD parsing but from what I understand its index is of no use since it was established early in this discussion that RMDD domain ID cannot be assumed to be the same as enumerated via CPUID leaf 0x4. So even if you use an xarray the CPU online handler needs to iterate over the xarray instead of indexing directly into it to find CPU being onlined and through that determine the "RMDD domain ID" associated with the CPUID leaf 0x4 cache ID of CPU being onlined. What is the benefit of an xarray when never indexing into it?

Hi Chenyu,

Quoted 2 lines · Yu C

1. At ACPI parse time , record the full CPU mask from CACD for each RMDD domain without checking online state or resolving L3 cache IDs.

ack.

Quoted 1 line

Store a per-CPU pointer (erdt_cpu_domain[cpu]) for O(1) lookup.

Why is such optimization needed? From what I can tell this pointer is only accessed *once*. I do not think onlining a CPU is a hot path? Could this be simplified to just be a list of ERDT domains?

Quoted 5 lines

2. At CPU online time (resctrl_arch_online_cpu), validate that the CPU's CPUID-derived L3 ID is consistent with other CPUs in the same RMDD:
- First online CPU in an RMDD establishes the L3 ID for that domain and inserts the domain into erdt_domain_xa keyed by L3 cache ID.
- Subsequent CPUs must match. On mismatch, the CPU is refused from resctrl.

Considering that this now establishes 1:1 between RMDD and resctrl domain and that resctrl already maintains a list of domains, could the x86 arch specific monitoring domain (struct rdt_hw_l3_mon_domain) perhaps just point to the ERDT monitoring data for that domain instead of maintaining a separate array of domains? So, when x86 is requested to read monitoring data for a domain there would be no need to query an array for the ERDT domain info since the containing structure already has a pointer to it.

This may need more care to organize the ERDT per-domain monitoring data structure when considering the references to upcoming additions that are not clear to me at this time.

Quoted 3 lines

3. erdt_mon_read() looks up erdt_domain_xa by hdr->id (L3 cache ID) as before, but the xarray is now populated lazily at CPU online time.

I do not see the need for an xarray. As new domains are created during CPU online their initialization can just point to the needed data, no?

Quoted 1 line · Reinette Chatre

On 6/23/26 9:05 PM, Chen, Yu C wrote:

Quoted 8 lines

Considering that this now establishes 1:1 between RMDD and resctrl domain and that resctrl already maintains a list of domains, could the x86 arch specific monitoring domain (struct rdt_hw_l3_mon_domain) perhaps just point to the ERDT monitoring data for that domain instead of maintaining a separate array of domains? So, when x86 is requested to read monitoring data for a domain there would be no need to query an array for the ERDT domain info since the containing structure already has a pointer to it.

Quoted 3 lines

This may need more care to organize the ERDT per-domain monitoring data structure when considering the references to upcoming additions that are not clear to me at this time.

I see. We could add a void *priv pointer to struct rdt_hw_l3_mon_domain.

During l3_mon_domain_setup(), we can assign this pointer to the corresponding erdt_domain_info object from the previously created list.
Since each rdt_hw_l3_mon_domain instance is freed when the last CPU in its domain goes offline, we need to keep the global list of struct erdt_domain_info objects

persistent at all times. Going forward, we may also need to add the same pointer assignment to rdt_hw_ctrl_domain for the upcoming region-aware MBA changes.

Quoted 4 lines

Quoted 3 lines

3. erdt_mon_read() looks up erdt_domain_xa by hdr->id (L3 cache ID) as before, but the xarray is now populated lazily at CPU online time.

Quoted 3 lines

I do not see the need for an xarray. As new domains are created during CPU online their initialization can just point to the needed data, no?

If we add the struct erdt_domain_info pointer to rdt_hw_l3_mon_domain, then there is no need to maintain the xarray.

Rename the prev_msr field in struct arch_mbm_state to prev_mon_val.

With ERDT, the previous monitor value may come from an MMIO register rather than from an MSR, so the "msr" suffix is no longer accurate. The new name describes the field by its meaning (the previous monitor value) instead of by the access method.

This is preparation for ERDT support, which reads monitoring counters via MMIO.

No functional change.

Hi Chenyu,

Quoted 1 line · Chen Yu

Rename the prev_msr field in struct arch_mbm_state to prev_mon_val.

This change as described seems out of place when considering the following from cover letter:
This patch set focuses on the first part: enabling MMIO-based access for Cache Monitoring Technology (CMT), *while CAT/MBM/MBA are still using MSR*.

It is thus confusing to find a change related to switching MBM to use MSR in a series that claims that it does not do this.

The patch looks fine but reading the changelog and the first few hunks makes this patch look out of place. Could you please update the changelog to be accurate about what this patch does?

Hi Reinette,

Quoted 10 lines · Reinette Chatre

Hi Chenyu,

Quoted 1 line · Chen Yu

Rename the prev_msr field in struct arch_mbm_state to prev_mon_val.

This change as described seems out of place when considering the following from cover letter:
  This patch set focuses on the first part: enabling MMIO-based access for Cache Monitoring Technology (CMT), *while CAT/MBM/MBA are still using MSR*.

Quoted 7 lines

It is thus confusing to find a change related to switching MBM to use MSR in a series that claims that it does not do this.

The patch looks fine but reading the changelog and the first few hunks makes this patch look out of place. Could you please update the changelog to be accurate about what this patch does?

OK, let me change it to:

x86/resctrl: Rename prev_msr to prev_mon_val

Rename the prev_msr field in struct arch_mbm_state to prev_mon_val to decouple the field name from the specific access method.

Use a generic name for the stored previous monitoring counter value so that the field remains accurate regardless of whether the value is read through an MSR or another interface in the future.

No functional change.

thanks,
Chenyu

Hi Chenyu,

Quoted 17 lines

Quoted 9 lines · Chen Yu

Quoted 1 line

Rename the prev_msr field in struct arch_mbm_state to prev_mon_val.

This change as described seems out of place when considering the following from cover letter:
  This patch set focuses on the first part: enabling MMIO-based access for Cache Monitoring Technology (CMT), *while CAT/MBM/MBA are still using MSR*.

Quoted 7 lines

It is thus confusing to find a change related to switching MBM to use MSR in a series that claims that it does not do this.

The patch looks fine but reading the changelog and the first few hunks makes this patch look out of place. Could you please update the changelog to be accurate about what this patch does?

Quoted 6 lines

OK, let me change it to:

x86/resctrl: Rename prev_msr to prev_mon_val

Rename the prev_msr field in struct arch_mbm_state to prev_mon_val to decouple the field name from the specific access method.

This patch changes many more names to support this switch to MMIO yet the changelog only highlights one of the changes which is the struct member specific to MBM while the work explicitly states that MBM monitoring is not impacted by this series.

Consider for comparison a more generic (just a draft to present an idea, please do not just copy&paste):
x86/resctrl: Replace "msr" in monitoring data identifiers

Monitoring data is only consumed via MSR and many identifiers handling the monitoring data contains "msr" as part of their names, for example "msr_val".

Replace "msr" in monitoring data identifiers as appropriate to support their use for monitoring data accessed via MMIO.

Quoted 7 lines

Use a generic name for the stored previous monitoring counter value so that the field remains accurate regardless of whether the value is read through an MSR or another interface in the future.

No functional change.

On 6/23/2026 5:29 AM, Reinette Chatre wrote:
Would the following be appropriate?

x86/resctrl: Replace "msr" in monitoring data identifiers

Monitoring counter values are currently obtained via MSR and many identifiers involved in processing these values contain "msr" as part of their names, for example "prev_msr" and "msr_val".

The ERDT ACPI table describes MMIO registers for monitoring access. A subsequent patch adds MMIO-based L3 occupancy reads. Rename "msr"

based identifiers to interface-agnostic names so they remain accurate as monitoring values may no longer originate exclusively from MSR.

No functional change.

Hi Chenyu,

Quoted 11 lines · Yu C

On 6/23/2026 5:29 AM, Reinette Chatre wrote:
Would the following be appropriate?

x86/resctrl: Replace "msr" in monitoring data identifiers

Monitoring counter values are currently obtained via MSR and many identifiers involved in processing these values contain "msr" as part of their names, for example "prev_msr" and "msr_val".

The ERDT ACPI table describes MMIO registers for monitoring access. A subsequent patch adds MMIO-based L3 occupancy reads. Rename "msr"

Please drop the "A subsequent patch ...". Apart from it resembling the dreaded "this patch", once it is merged the intended meaning is lost. As an alternative the second paragraph can be:
The ERDT ACPI table describes MMIO registers for monitoring data access. Rename "msr"-based identifiers to be interface-agnostic to support their upcoming use for MMIO-read values.

No functional change.

Quoted 4 lines

based identifiers to interface-agnostic names so they remain accurate as monitoring values may no longer originate exclusively from MSR.

No functional change.

The CMRC (Cache Monitoring Registers for CPU Agents Description) ACPI sub-table provides the MMIO address used to read the LLC occupancy counter for each RMID. When ERDT is enabled on the platform, use this MMIO interface instead of the legacy MSR read to obtain the L3 occupancy value.

Introduce erdt_mon_read(), a helper that retrieves monitoring data for a given RMID and event ID from an ERDT domain. Initial support is added for the L3 occupancy monitoring event (QOS_L3_OCCUP_EVENT_ID).

If the platform supports ERDT, CMRC-based MMIO access is used by default. If ERDT is unavailable, the implementation is to use MSR-based operations.

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

index 97c2f6bc7a5f..9b3b03279dd8 100644
@@ -41,6 +41,8 @@ struct resctrl_pqr_state {
 };
 
 bool erdt_enabled(void);
+struct rdt_domain_hdr;
+int erdt_mon_read(struct rdt_domain_hdr *hdr, int ev_id, int rmid, u64 *val);
 
 DECLARE_PER_CPU(struct resctrl_pqr_state, pqr_state);
 

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

index 90730f0851fa..fe812f7190fc 100644

Hi Chenyu,

Quoted 11 lines · Chen Yu

The CMRC (Cache Monitoring Registers for CPU Agents Description) ACPI sub-table provides the MMIO address used to read the LLC occupancy counter for each RMID. When ERDT is enabled on the platform, use this MMIO interface instead of the legacy MSR read to obtain the L3 occupancy value.

Introduce erdt_mon_read(), a helper that retrieves monitoring data for a given RMID and event ID from an ERDT domain. Initial support is added for the L3 occupancy monitoring event (QOS_L3_OCCUP_EVENT_ID).

Quoted 3 lines

If the platform supports ERDT, CMRC-based MMIO access is used by default. If ERDT is unavailable, the implementation is to use MSR-based operations.

(nit: please write in imperative tone and use entire line length available)

...

Quoted 20 lines

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

index 97c2f6bc7a5f..9b3b03279dd8 100644
@@ -41,6 +41,8 @@ struct resctrl_pqr_state {
  };
 
  bool erdt_enabled(void);
+struct rdt_domain_hdr;
+int erdt_mon_read(struct rdt_domain_hdr *hdr, int ev_id, int rmid, u64 *val);
 
  DECLARE_PER_CPU(struct resctrl_pqr_state, pqr_state);
 

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

index 90730f0851fa..fe812f7190fc 100644
@@ -965,7 +965,7 @@ static __init bool get_rdt_mon_resources(void)
  	bool ret = false;
 

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

Quoted 6 lines

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

  	if (rdt_cpu_has(X86_FEATURE_CQM_OCCUP_LLC)) {
-		resctrl_enable_mon_event(QOS_L3_OCCUP_EVENT_ID, false, 0, NULL);
+		resctrl_enable_mon_event(QOS_L3_OCCUP_EVENT_ID, erdt_enabled(), 0, NULL);
  		ret = true;
  	}
  	if (rdt_cpu_has(X86_FEATURE_CQM_MBM_TOTAL)) {

As mentioned in patch #1, when erdt_enabled() is true the enumeration still proceeds to enumerate the monitoring properties via CPUID to discover the number of RMIDs that the *MSR* supports and use it as the maximum RMID (and thus the maximum number of registers) that MMIO supports?

Quoted 6 lines

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

index 9209927f88a2..1491f96b57c3 100644
@@ -278,6 +278,13 @@ int resctrl_arch_rmid_read(struct rdt_resource *r, struct rdt_domain_hdr *hdr,
 

arch/x86/kernel/cpu/resctrl/monitor.c int resctrl_arch_rmid_read(struct rdt_resource *r, struct rdt_domain_hdr *hdr, · line 282

Quoted 4 lines

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

  	switch (r->rid) {
  	case RDT_RESOURCE_L3:
+		/*
+		 * No SNC for mmio based L3 occupancy, so there is no need

"No SNC for mmio based L3 occupancy" is too significant to be buried like this. Could you please elaborate on this claim and enforce it with the implementation? For example, could rdt_get_l3_mon_config() WARN and *not* set r->mon_capable if erdt_enable() and snc_nodes_per_l3_cache > 1?

arch/x86/kernel/cpu/resctrl/monitor.c int resctrl_arch_rmid_read(struct rdt_resource *r, struct rdt_domain_hdr *hdr, · line 290

Quoted 8 lines

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

+		 * to convert logical RMID to a physical RMID via
+		 * logical_rmid_to_physical_rmid().
+		 */
+		if (erdt_enabled() && eventid == QOS_L3_OCCUP_EVENT_ID)
+			return erdt_mon_read(hdr, eventid, rmid, val);
  		return arch_l3_read_event(hdr, rmid, eventid, val, r);
  	case RDT_RESOURCE_PERF_PKG:
  		return intel_aet_read_event(hdr->id, rmid, arch_priv, val);

Reinette

Quoted 1 line · Reinette Chatre

On 6/13/26 12:57 AM, Chen Yu wrote:

Quoted 19 lines

Quoted 19 lines

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

index 97c2f6bc7a5f..9b3b03279dd8 100644
@@ -41,6 +41,8 @@ struct resctrl_pqr_state {
   };
     bool erdt_enabled(void);
+struct rdt_domain_hdr;
+int erdt_mon_read(struct rdt_domain_hdr *hdr, int ev_id, int rmid, u64 *val);
     DECLARE_PER_CPU(struct resctrl_pqr_state, pqr_state);

  diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c index 90730f0851fa..fe812f7190fc 100644

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

@@ -965,7 +965,7 @@ static __init bool get_rdt_mon_resources(void)
       bool ret = false;
         if (rdt_cpu_has(X86_FEATURE_CQM_OCCUP_LLC)) {
-        resctrl_enable_mon_event(QOS_L3_OCCUP_EVENT_ID, false, 0, NULL);
+        resctrl_enable_mon_event(QOS_L3_OCCUP_EVENT_ID, erdt_enabled(), 0, NULL);

Quoted 9 lines

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

Quoted 3 lines

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

           ret = true;
       }
       if (rdt_cpu_has(X86_FEATURE_CQM_MBM_TOTAL)) {

As mentioned in patch #1, when erdt_enabled() is true the enumeration still proceeds to enumerate the monitoring properties via CPUID to discover the number of RMIDs that the *MSR* supports and use it as the maximum RMID (and thus the maximum number of registers) that MMIO supports?

OK, will switch to the maximum RMID exposed by ACPI table, if erdt_enabled() is true.

Hi Chenyu,

Quoted 29 lines

Quoted 19 lines

Quoted 19 lines

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

index 97c2f6bc7a5f..9b3b03279dd8 100644
@@ -41,6 +41,8 @@ struct resctrl_pqr_state {
   };
     bool erdt_enabled(void);
+struct rdt_domain_hdr;
+int erdt_mon_read(struct rdt_domain_hdr *hdr, int ev_id, int rmid, u64 *val);
     DECLARE_PER_CPU(struct resctrl_pqr_state, pqr_state);
   diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c

index 90730f0851fa..fe812f7190fc 100644

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

@@ -965,7 +965,7 @@ static __init bool get_rdt_mon_resources(void)
       bool ret = false;
         if (rdt_cpu_has(X86_FEATURE_CQM_OCCUP_LLC)) {
-        resctrl_enable_mon_event(QOS_L3_OCCUP_EVENT_ID, false, 0, NULL);
+        resctrl_enable_mon_event(QOS_L3_OCCUP_EVENT_ID, erdt_enabled(), 0, NULL);

Quoted 9 lines

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

Quoted 3 lines

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

           ret = true;
       }
       if (rdt_cpu_has(X86_FEATURE_CQM_MBM_TOTAL)) {

As mentioned in patch #1, when erdt_enabled() is true the enumeration still proceeds to enumerate the monitoring properties via CPUID to discover the number of RMIDs that the *MSR* supports and use it as the maximum RMID (and thus the maximum number of registers) that MMIO supports?

Quoted 1 line

OK, will switch to the maximum RMID exposed by ACPI table, if erdt_enabled() is true.

I believe the issue is larger than just the RMID enumeration. The CPUID and ACPI enumeration appears to be fully intertwined. Taking a closer look at what above code does:
it checks *CPUID* whether CMT is enabled and then enables the LLC occupancy event to blindly use MMIO if ERDT is enabled, irrespective of whether the ERDT tables include a cache monitoring table or not. How is it guaranteed that if ERDT is enabled that there is a cache monitoring table? Should it not be the existence of the ACPI cache monitoring table and its properties that determines whether the LLC occupancy counter using MMIO registers should be enabled?

Hi Reinette,

Quoted 32 lines · Reinette Chatre

Hi Chenyu,

Quoted 29 lines

Quoted 19 lines

Quoted 19 lines

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

index 97c2f6bc7a5f..9b3b03279dd8 100644
@@ -41,6 +41,8 @@ struct resctrl_pqr_state {
    };
      bool erdt_enabled(void);
+struct rdt_domain_hdr;
+int erdt_mon_read(struct rdt_domain_hdr *hdr, int ev_id, int rmid, u64 *val);
      DECLARE_PER_CPU(struct resctrl_pqr_state, pqr_state);

   diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c index 90730f0851fa..fe812f7190fc 100644

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

@@ -965,7 +965,7 @@ static __init bool get_rdt_mon_resources(void)
        bool ret = false;
          if (rdt_cpu_has(X86_FEATURE_CQM_OCCUP_LLC)) {
-        resctrl_enable_mon_event(QOS_L3_OCCUP_EVENT_ID, false, 0, NULL);
+        resctrl_enable_mon_event(QOS_L3_OCCUP_EVENT_ID, erdt_enabled(), 0, NULL);

Quoted 9 lines

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

Quoted 3 lines

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

            ret = true;
        }
        if (rdt_cpu_has(X86_FEATURE_CQM_MBM_TOTAL)) {

As mentioned in patch #1, when erdt_enabled() is true the enumeration still proceeds to enumerate the monitoring properties via CPUID to discover the number of RMIDs that the *MSR* supports and use it as the maximum RMID (and thus the maximum number of registers) that MMIO supports?

Quoted 1 line

OK, will switch to the maximum RMID exposed by ACPI table, if erdt_enabled() is true.

Quoted 8 lines

I believe the issue is larger than just the RMID enumeration. The CPUID and ACPI enumeration appears to be fully intertwined. Taking a closer look at what above code does:
it checks *CPUID* whether CMT is enabled and then enables the LLC occupancy event to blindly use MMIO if ERDT is enabled, irrespective of whether the ERDT tables include a cache monitoring table or not. How is it guaranteed that if ERDT is enabled that there is a cache monitoring table? Should it not be the existence of the ACPI cache monitoring table and its properties that determines whether the LLC occupancy counter using MMIO registers should be enabled?

I see. How about replacing erdt_enabled() with fine-grained helper functions such as erdt_has_cmrc(), erdt_has_mmrc(), and erdt_has_marc()? The latter two will be added later for region-aware MBM/MBA. CMRC, MMRC and MARC are not guaranteed to coexist, so splitting them into separate helpers would offer finer control.

Hi Chenyu,

Quoted 44 lines · Yu C

Hi Reinette,

Quoted 40 lines · Reinette Chatre

Hi Chenyu,

Quoted 29 lines

Quoted 19 lines

Quoted 19 lines

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

index 97c2f6bc7a5f..9b3b03279dd8 100644
@@ -41,6 +41,8 @@ struct resctrl_pqr_state {
    };
      bool erdt_enabled(void);
+struct rdt_domain_hdr;
+int erdt_mon_read(struct rdt_domain_hdr *hdr, int ev_id, int rmid, u64 *val);
      DECLARE_PER_CPU(struct resctrl_pqr_state, pqr_state);
    diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c

index 90730f0851fa..fe812f7190fc 100644

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

@@ -965,7 +965,7 @@ static __init bool get_rdt_mon_resources(void)
        bool ret = false;
          if (rdt_cpu_has(X86_FEATURE_CQM_OCCUP_LLC)) {
-        resctrl_enable_mon_event(QOS_L3_OCCUP_EVENT_ID, false, 0, NULL);
+        resctrl_enable_mon_event(QOS_L3_OCCUP_EVENT_ID, erdt_enabled(), 0, NULL);

Quoted 9 lines

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

Quoted 3 lines

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

            ret = true;
        }
        if (rdt_cpu_has(X86_FEATURE_CQM_MBM_TOTAL)) {

As mentioned in patch #1, when erdt_enabled() is true the enumeration still proceeds to enumerate the monitoring properties via CPUID to discover the number of RMIDs that the *MSR* supports and use it as the maximum RMID (and thus the maximum number of registers) that MMIO supports?

Quoted 1 line

OK, will switch to the maximum RMID exposed by ACPI table, if erdt_enabled() is true.

I believe the issue is larger than just the RMID enumeration. The CPUID and ACPI enumeration appears to be fully intertwined. Taking a closer look at what above code does:
it checks *CPUID* whether CMT is enabled and then enables the LLC occupancy event to blindly use MMIO if ERDT is enabled, irrespective of whether the ERDT tables include a cache monitoring table or not. How is it guaranteed that if ERDT is enabled that there is a cache monitoring table? Should it not be the existence of the ACPI cache monitoring table and its properties that determines whether the LLC occupancy counter using MMIO registers should be enabled?

Quoted 4 lines

I see. How about replacing erdt_enabled() with fine-grained helper functions such as erdt_has_cmrc(), erdt_has_mmrc(), and erdt_has_marc()? The latter two will be added later for region-aware MBM/MBA. CMRC, MMRC and MARC are not guaranteed to coexist, so splitting them into separate helpers would offer finer control.

I am concerned where this is headed since it looks to me as though the plan is to sprinkle these finer grained checks throughout resctrl.

To me this sounds complicated
and error prone. Consider that resctrl_enable_mon_event() has an arch_priv parameter. To me this seems to be the appropriate place for the architecture to give itself the needed information about how to read the event.

Quoted 43 lines · Reinette Chatre

Quoted 42 lines · Yu C

Quoted 40 lines · Reinette Chatre

Hi Chenyu,

Quoted 29 lines

Quoted 19 lines

Quoted 19 lines

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

index 97c2f6bc7a5f..9b3b03279dd8 100644
@@ -41,6 +41,8 @@ struct resctrl_pqr_state {
     };
       bool erdt_enabled(void);
+struct rdt_domain_hdr;
+int erdt_mon_read(struct rdt_domain_hdr *hdr, int ev_id, int rmid, u64 *val);
       DECLARE_PER_CPU(struct resctrl_pqr_state, pqr_state);

    diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c index 90730f0851fa..fe812f7190fc 100644

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

@@ -965,7 +965,7 @@ static __init bool get_rdt_mon_resources(void)
         bool ret = false;
           if (rdt_cpu_has(X86_FEATURE_CQM_OCCUP_LLC)) {
-        resctrl_enable_mon_event(QOS_L3_OCCUP_EVENT_ID, false, 0, NULL);
+        resctrl_enable_mon_event(QOS_L3_OCCUP_EVENT_ID, erdt_enabled(), 0, NULL);

Quoted 9 lines

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

Quoted 3 lines

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

             ret = true;
         }
         if (rdt_cpu_has(X86_FEATURE_CQM_MBM_TOTAL)) {

As mentioned in patch #1, when erdt_enabled() is true the enumeration still proceeds to enumerate the monitoring properties via CPUID to discover the number of RMIDs that the *MSR* supports and use it as the maximum RMID (and thus the maximum number of registers) that MMIO supports?

Quoted 1 line

OK, will switch to the maximum RMID exposed by ACPI table, if erdt_enabled() is true.

I believe the issue is larger than just the RMID enumeration. The CPUID and ACPI enumeration appears to be fully intertwined. Taking a closer look at what above code does:
it checks *CPUID* whether CMT is enabled and then enables the LLC occupancy event to blindly use MMIO if ERDT is enabled, irrespective of whether the ERDT tables include a cache monitoring table or not. How is it guaranteed that if ERDT is enabled that there is a cache monitoring table? Should it not be the existence of the ACPI cache monitoring table and its properties that determines whether the LLC occupancy counter using MMIO registers should be enabled?

Quoted 6 lines

Quoted 4 lines

I see. How about replacing erdt_enabled() with fine-grained helper functions such as erdt_has_cmrc(), erdt_has_mmrc(), and erdt_has_marc()? The latter two will be added later for region-aware MBM/MBA. CMRC, MMRC and MARC are not guaranteed to coexist, so splitting them into separate helpers would offer finer control.

I am concerned where this is headed since it looks to me as though the plan is to sprinkle these finer grained checks throughout resctrl.

The erdt_has_* helper functions would ideally live under arch/x86/kernel/cpu/resctrl/ rather than the generic fs/resctrl directory, as architecture-specific code in the former path is allow to see erdt logic I suppose? That said, since there are a large number of such routines, introduce a dedicated helper to handle this uniformly would be better(using arch_priv)

Quoted 5 lines

To me this sounds complicated
and error prone. Consider that resctrl_enable_mon_event() has an arch_priv parameter. To me this seems to be the appropriate place for the architecture to give itself the needed information about how to read the event.

OK, if I understand correctly, we can use the following logic to hide erdt from monitor read:

/*
 * helper to get the erdt's monitor arch_priv,
 * defined in erdt.c, NULL in other place.
 *
 * caller doesn't know about CMRC
 */
void *get_evt_priv(enum resctrl_event_id eventid)
{
        if (!erdt_enabled())
                return NULL;

        switch (eventid) {
        case QOS_L3_OCCUP_EVENT_ID:
                return cmrc_priv_valid ? &cmrc_priv : NULL; default:
                return NULL;
        }
}

arch/x86/kernel/cpu/resctrl/core.c
get_rdt_mon_resources():
if (rdt_cpu_has(X86_FEATURE_CQM_OCCUP_LLC)) {

        void *priv = get_evt_priv(QOS_L3_OCCUP_EVENT_ID);

        resctrl_enable_mon_event(QOS_L3_OCCUP_EVENT_ID, priv != NULL, 0, priv); }

arch/x86/kernel/cpu/resctrl/monitor.c resctrl_arch_rmid_read():
        if (arch_priv)
                return erdt_mon_read(hdr, eventid, rmid, val); return arch_l3_read_event(hdr, rmid, eventid, val, r);

So in this way, there is no event-type checks in the read path. Adding MMRC later only extends the switch in get_evt_priv().