Introduce MMIO-based CMT access for Enhanced RDT

Enhanced RDT (ERDT) enumerates its monitoring and control resources

through ACPI tables that identify each CPU by its x2APIC ID. To associate this firmware-provided data with the kernel's per-CPU structures, ERDT must translate those x2APIC IDs into logical CPU numbers.

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

Export topo_lookup_cpuid() and add its declaration to asm/apic.h so

that ERDT can resolve x2APIC IDs from ACPI tables to logical CPU numbers.

No functional change intended.

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

Tested-by Hongyu Ning <[email protected]>

---

Hi Chenyu,

Quoted 1 line · Chen Yu

Enhanced RDT (ERDT) enumerates its monitoring and control resources

Please use at least 80 characters per line. The checkpatch warnings about 75 characters can be ignored. For context, please see
https://lore.kernel.org/lkml/20250916105447.GCaMlB976WLxHHeNMD@fat_crate.local/

Please check entire series.

Quoted 8 lines

through ACPI tables that identify each CPU by its x2APIC ID. To associate this firmware-provided data with the kernel's per-CPU structures, ERDT must translate those x2APIC IDs into logical CPU numbers.

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

Quoted 1 line

Export topo_lookup_cpuid() and add its declaration to asm/apic.h so

To support this change you could add a motivation why this particular header file was chosen. For example,
"add its declaration to asm/apic.h" -> "add its declaration to asm/apic.h that already exposes the inverse mapping"

Quoted 8 lines

that ERDT can resolve x2APIC IDs from ACPI tables to logical CPU numbers.

No functional change intended.

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

Tested-by Hongyu Ning <[email protected]>

---

| Reviewed-by: Reinette Chatre <[email protected]>

Reinette

All known hardware that supports Intel RDT or AMD QoS is 64-bit. Future enhancements like MMIO-based Enhanced RDT (ERDT) monitoring require readq()/writeq() which are only available on 64-bit. There is also no realistic use case for users to build a 32-bit kernel on a server and enable resctrl.

Drop 32-bit support by changing the X86_CPU_RESCTRL dependency from X86 to X86_64.

Since X86_CPU_RESCTRL now implies X86_64, remove the redundant X86_64 dependency from X86_CPU_RESCTRL_INTEL_AET.

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

Tested-by Hongyu Ning <[email protected]>

---

Reviewed at:
https://lore.kernel.org/lkml/[email protected]/

Quoted 2 lines

Reviewed at:
https://lore.kernel.org/lkml/[email protected]/

Reinette,

I've written up this draft for next series. Are you looking for something like this?

x86/resctrl: Require 64-bit x86 for resctrl support

All known hardware that supports Intel RDT or AMD QoS is 64-bit.

On x86 the maximum supported memory size on a 32-bit kernel is 64GB (when PAE is enabled). Linux imposes additional constraints since each 4KByte page requires a 64-byte "struct page" which must reside in the always-mapped 3-4GByte range of the 32-bit kernel virtual address space. This means the maximum supported memory size is slightly above 32GB, but at that memory size there are considerable inefficiencies as over half of kernel virtual memory is dedicated to the page structures.

The earliest server processor to support resctrl was Intel Haswell, released in 2014. It supported four memory channels, and production systems were advised to populate all channels to provide sufficient memory bandwidth to keep the cores busy. Best value DIMMs in 2014 were 16GB. So even a single socket Haswell system would have more memory than could be addressed by a 32-bit kernel.

Drop 32-bit support by changing the X86_CPU_RESCTRL dependency from X86 to X86_64.

Since X86_CPU_RESCTRL now implies X86_64, remove the redundant X86_64 dependency from X86_CPU_RESCTRL_INTEL_AET.

To be specific, my comment about RDT not being a "server" feature was made with Apollo Lake [1] in mind. That system showed up in scenarios where servers do not typically venture and per [1] its max memory size (8GB) is significantly less than what servers can support.

Quoted 3 lines

To be specific, my comment about RDT not being a "server" feature was made with Apollo Lake [1] in mind. That system showed up in scenarios where servers do not typically venture and per [1] its max memory size (8GB) is significantly less than what servers can support.

Even with just 8GB you should really use 64-bit.

Quoted 4 lines · Tony

Quoted 3 lines

To be specific, my comment about RDT not being a "server" feature was made with Apollo Lake [1] in mind. That system showed up in scenarios where servers do not typically venture and per [1] its max memory size (8GB) is significantly less than what servers can support.

Even with just 8GB you should really use 64-bit.

Just in practice, 32-bit is very lightly tested. We're generally not even supporting new CPU features on 32-bit even if the hardware supports it. Think of shadow stacks, for instance.

It's actually increasingly hard to even find a modern 32-bit distro.

So it really has nothing to do with high memory. It just practically that the arch/x86 32-bit code is increasingly there for legacy reasons. Its testing goes down every day and the testing of *new* features is for all intents and purposes zero.

Honestly, the only people who complain about 32-bit support going away are folks who "test" it regularly. It's quite rare to see anyone actually "using" a 32-bit system.

My comments were only regarding accuracy of the motivation used in the changelog.

Quoted 2 lines · Reinette Chatre

My comments were only regarding accuracy of the motivation used in the changelog.

Well, what should be the motivation? How about "no one cares about 32-bit and if people wanna use resctrl on it, they're on their own."?

It is clear to me that there is no need for a strong motivation for this change that x86 maintainers already support. Even so, I find no justification for using misinformation in the changelog that makes this change. This was my only feedback to the original commit as I aim to support precise changelogs that are not confusing.

Quoted 4 lines · Borislav Petkov

Quoted 2 lines · Reinette Chatre

My comments were only regarding accuracy of the motivation used in the changelog.

Quoted 2 lines

Well, what should be the motivation? How about "no one cares about 32-bit and if people wanna use resctrl on it, they're on their own."?

It is clear to me that there is no need for a strong motivation for this change that x86 maintainers already support. Even so, I find no justification for using misinformation in the changelog that makes this change. This was my only feedback to the original commit as I aim to support precise changelogs that are not confusing.

"32-bit x86 is maintained for legacy reasons. New CPU features are generally not enabled for 32-bit system, its test coverage keeps shrinking, and modern 32-bit distributions have become rare."

Quoted 3 lines · Chen Yu

"32-bit x86 is maintained for legacy reasons. New CPU features are generally not enabled for 32-bit system, its test coverage keeps shrinking, and modern 32-bit distributions have become rare."

What would folks think about plopping something like the following text in to Documentation/arch/x86/32bit-kernels.rst ?

Would that clear things up?

---

=========================================
Supporting new features on 32-bit kernels
=========================================

While it is possible to compile and run a 32-bit kernel on 64-bit capable hardware, it is impractical and discouraged. Processors capable of running 64-bit kernels are expected to do so.

The last 32-bit processor to come out of Intel or AMD was a Quark CPU in 2013. Before that the most recent were some Atom CPUs in 2008.

Distros supporting 32-bit kernels and userspace are disappearing. For example, in 2026 Ubuntu and Fedora no longer have support, with Debian being the most notable holdout still maintaining 32-bit options.

New Processor Features
======================

When adding support for a new processor feature in 2026 and beyond it should, by default, be:

         config NEW_SNAZZY_CPU_FEATURE
                 depends X86_64

This ensures that the folks developing this feature do not waste resources making it work on 32-bit kernels running on 64-bit hardware.

It is also acceptable to add this restriction to existing processor features if 32-bit kernel support is adding maintenance burden, especially if the feature was only ever available on 64-bit processors.

New Software Features
=====================

Let's say you are enhancing ftrace or adding a debugging feature that has no strict dependency on processor features. It does not need to be restricted to 64-bit only. But you might want to consider doing so if it simplifies your implementation.

For instance, sometimes it adds complexity to support HIGHMEM. You may add a dependency on X86_64 in cases where it is impractical or simply challenging to support 32-bit kernels.

What would folks think about plopping something like the following text in to Documentation/arch/x86/32bit-kernels.rst ?

Honestly, the only people who complain about 32-bit support going away are folks who "test" it regularly. It's quite rare to see anyone actually "using" a 32-bit system.

I've for a long time advocated detecting LM during the 32bit boot and simply aborting with a message: "Go run the x86_64 kernel!".

Quoted 2 lines

I've for a long time advocated detecting LM during the 32bit boot and simply aborting with a message: "Go run the x86_64 kernel!".

Ack to that. Let's do it and see who complains.

Probably everyone running regression tests of 32-bit kernels on hardware, rather than in Qemu.

From: Anil S Keshavamurthy <[email protected]>

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

There is one ERDT table per platform. Each RMDD describes one resource

management domain (RMD), also known as an L3 domain, and carries MMIO base information for later monitoring support.

For each RMDD, parse the associated CACD, map its x2APIC IDs to logical

CPUs, and save the resulting CPU mask. This mask associates each ERDT domain with the CPUs that belong to it and is used later when attaching ERDT data to resctrl monitoring domains.

---
 arch/x86/kernel/cpu/resctrl/Makefile   |   1 +
 arch/x86/kernel/cpu/resctrl/core.c     |  14 +-
 arch/x86/kernel/cpu/resctrl/erdt.c     | 266 +++++++++++++++++++++++++
 arch/x86/kernel/cpu/resctrl/internal.h |  31 +++
 4 files changed, 311 insertions(+), 1 deletion(-)
 create mode 100644 arch/x86/kernel/cpu/resctrl/erdt.c

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

index 9c01d2562b7a..23925bcd71d7 100644

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

@@ -1114,7 +1115,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;

@@ -1157,6 +1158,15 @@ static int __init resctrl_arch_late_init(void)
 	return 0;

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

 }
 
 __exitcall(resctrl_arch_exit);

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

new file mode 100644
index 000000000000..8998cae47090
@@ -0,0 +1,266 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Enhanced Resource Director Technology (ERDT)
+ *
+ * Copyright (C) 2026 Intel Corporation
+ *
+ */
+
+#define pr_fmt(fmt)     "resctrl: " fmt
+
+#include <linux/acpi.h>
+#include <linux/overflow.h>
+#include <linux/resctrl.h>
+#include <linux/sizes.h>
+
+#include <asm/apic.h>
+
+#include "internal.h"
+
+static LIST_HEAD(domain_info_list);
+
+/* True when the ERDT ACPI table describes at least one domain with at least one CPU. */
+static bool erdt_enabled;
+

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

+
+int erdt_get_max_rmid(void)
+{
+	return erdt_max_rmid;
+}

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

+
+	if (cacd->header.length < struct_size(cacd, X2APICIDS, 1)) {
+		pr_warn(FW_BUG "Invalid x2apicid CACD table\n");
+		return -EIO;
+	}
+
+	num_ids = (cacd->header.length - sizeof(*cacd)) / sizeof(cacd->X2APICIDS[0]);
+
+	for (int i = 0; i < num_ids; i++) {
+		cpu = topo_lookup_cpuid(cacd->X2APICIDS[i]);
+		if (cpu < 0) {
+			pr_warn(FW_BUG "Unknown x2apicid 0x%x\n", cacd->X2APICIDS[i]);
+			return -EIO;
+		}
+
+		cpumask_set_cpu(cpu, &domain_info->cpu_mask);
+	}
+
+	return 0;
+}
+
+static inline struct acpi_subtbl_hdr_16 *rmdd_subtbl(struct acpi_erdt_rmdd *rmdd)
+{
+	return (void *)rmdd + sizeof(*rmdd);
+}
+
+static inline struct acpi_subtbl_hdr_16 *next_subtbl(struct acpi_subtbl_hdr_16 *subtbl)
+{
+	return (void *)subtbl + subtbl->length;
+}
+
+static inline bool subtbl_valid(void *end, struct acpi_subtbl_hdr_16 *subtbl)
+{
+	/* Ensure the header is within bounds before dereferencing it. */
+	if ((void *)subtbl + sizeof(*subtbl) > end)
+		return false;
+
+	/* A sub-table must be at least as large as its header. */
+	if (subtbl->length < sizeof(*subtbl))
+		return false;
+
+	/* The entire sub-table (including body) must fit within the parent. */
+	if ((void *)subtbl + subtbl->length > end)
+		return false;
+
+	return true;
+}
+
+static __init bool parse_rmdd_table(struct acpi_subtbl_hdr_16 *rmdd_hdr)
+{
+	struct erdt_domain_info *domain_info;

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

+
+	/* Quietly ignore non-CPU-based L3 domains */
+	if (!(rmdd->flags & RMDD_FLAG_CPU_L3_DOMAIN))
+		return true;
+
+	domain_info = kzalloc_obj(*domain_info, GFP_KERNEL);
+	if (!domain_info)
+		return false;
+

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

+		switch (subtbl->type) {
+		/* An RMDD table has one or more CACD sub-table(s) */
+		case ACPI_ERDT_TYPE_CACD:
+			if (cacd_init(subtbl, domain_info))
+				goto cleanup;
+
+			subtbl_mask |= BIT(ACPI_ERDT_TYPE_CACD);
+			break;
+		default:
+			break;
+		}
+	}
+
+	if (!subtbl_mask)
+		goto cleanup;
+
+	/*
+	 * Require all RMDDs to support same set of sub-tables
+	 */
+	if (!valid_subtbl_mask) {
+		valid_subtbl_mask = subtbl_mask;
+		first_rmdd_domain_id = rmdd->domain_id;
+	} else if (subtbl_mask != valid_subtbl_mask) {
+		pr_warn(FW_BUG "RMDD %u sub-table set does not match the first RMDD %u\n",
+			rmdd->domain_id, first_rmdd_domain_id);
+		goto cleanup;
+	}
+
+	if (!rmdd->max_rmid) {
+		pr_warn(FW_BUG "Unreasonable RMDD max_rmid %u\n", rmdd->max_rmid);
+		goto cleanup;
+	}
+	domain_info->max_rmid = rmdd->max_rmid;
+
+	if (!erdt_max_rmid)
+		erdt_max_rmid = rmdd->max_rmid;
+	else
+		erdt_max_rmid = min_t(int, 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)

arch/x86/kernel/cpu/resctrl/erdt.c new file +10 -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;
+	void *table_end;
+
+	if (erdt->header.revision != ERDT_VALID_VERSION) {
+		pr_info("Unsupported ERDT table revision %u (expected %u)\n",

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

+	table_end = (void *)erdt + erdt->header.length;
+
+	while (subtbl_valid(table_end, subtbl)) {
+		if (subtbl->type == ACPI_ERDT_TYPE_RMDD &&
+		    !parse_rmdd_table(subtbl))
+			goto cleanup;
+
+		subtbl = next_subtbl(subtbl);
+	}
+
+	if (list_empty(&domain_info_list))
+		goto cleanup;
+
+	erdt_enabled = true;
+
+	return 0;
+
+cleanup:
+	erdt_exit();
+	return -EINVAL;
+}
+
+int __init erdt_init(void)
+{
+	return acpi_table_parse(ACPI_SIG_ERDT, enumerate_erdt_table);
+}

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

index e3cfa0c10e92..bdff3ea36e62 100644
@@ -21,6 +21,33 @@
 
 #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 +13 -0

+ * @cpu_mask:	CPUs belonging to this resource management domain
+ * @max_rmid:	Maximum RMID supported by this 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;
+	u32			max_rmid;
+	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,

Quoted 4 lines · Chen Yu

From: Anil S Keshavamurthy <[email protected]>

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

The changelog always needs to start with context. Please see "Changelog" in Documentation/process/maintainer-tip.rst for complete expectations.

Quoted 2 lines

There is one ERDT table per platform. Each RMDD describes one resource

I do not think what "RMDD" stands for has been introduced at this point yet.

Quoted 4 lines

management domain (RMD), also known as an L3 domain, and carries MMIO base information for later monitoring support.

For each RMDD, parse the associated CACD, map its x2APIC IDs to logical

What is "CACD"? Please always expand acronym before its first use.

Quoted 3 lines

CPUs, and save the resulting CPU mask. This mask associates each ERDT domain with the CPUs that belong to it and is used later when attaching ERDT data to resctrl monitoring domains.

Please let each patch description stand on its own without referring to later patches in series. If a patch provides capability in preparation for future changes then you can use language like,
"Associate every ERDT domain with the CPUs that belong to it to prepare
for attaching ERDT data to resctrl monitoring domains."

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

Quoted 6 lines

static __init bool get_rdt_resources(void)
{

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

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

Functions are not expected to leave dangling state when they return failure. get_rdt_resources() returning false is considered a failure and now the caller is left to clean up the dangling state which is not a familiar pattern to use and thus something that can/will trip people.

On top of this this implementation pushes the cleanup very far from even the caller making this unfamiliar pattern even harder to recognize.

Please let get_rdt_resources() clean up after itself on failure to find any resources.

Quoted 13 lines

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

@@ -1114,7 +1115,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;

@@ -1157,6 +1158,15 @@ static int __init resctrl_arch_late_init(void)
 	return 0;
 }
 

arch/x86/kernel/cpu/resctrl/core.c static int __init resctrl_arch_late_init(void) · line 1168

Quoted 8 lines

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

+static int __init resctrl_arch_late_init(void)
+{
+	int ret = __resctrl_arch_late_init();
+
+	if (ret)
+		erdt_exit();
+	return ret;
+}

Related to earlier comment on cleanup I find this cleanup to be asymmentrical and inconsistent with how resctrl usually does cleanup. Why not do cleanup in (original) resctrl_arch_late_init() to be consistent with other cleanup when failures are encountered during initialization, for example, cpuhp_remove_state()? I find that having the cleanup handled where error is encountered is easier to understand.

Quoted 7 lines

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

+
 late_initcall(resctrl_arch_late_init);
 
 static void __exit resctrl_arch_exit(void)

@@ -1166,6 +1176,8 @@ static void __exit resctrl_arch_exit(void)
 	cpuhp_remove_state(rdt_online);
 

arch/x86/kernel/cpu/resctrl/core.c static void __exit resctrl_arch_exit(void) · line 1180

Quoted 3 lines

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

 	resctrl_exit();
+
+	erdt_exit();

Here the cleanup is indeed done in the same place as cleanup of other init work (cpuhp_remove_state()).

Quoted 10 lines

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

Quoted 4 lines

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

+/* Domain ID of the first RMDD that established @valid_subtbl_mask, for diagnostics. */
+static u16 first_rmdd_domain_id;
+
+static int erdt_max_rmid;

Could this ever be negative? Could this instead be of same type as the value it is initialized with? Looks like this will improve type safety with the min_t() usage then using the accurate and consistent type of both parameters?

I also think a comment describing erdt_max_rmid will be helpful, especially considering that it has "max" in its name but then its value is determined using the *minimum* of all domains' RMID?

Quoted 5 lines

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

+
+int erdt_get_max_rmid(void)
+{
+	return erdt_max_rmid;
+}

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

Quoted 10 lines

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

+
+static void __iomem *erdt_ioremap(phys_addr_t base, u32 num_pages, const char *desc)
+{
+	void __iomem *addr;
+	size_t size;
+
+	if (check_mul_overflow(num_pages, SZ_4K, &size))
+		return NULL;
+
+	addr = ioremap(base, size);

The types seem to target a function with prototype ioremap(phys_addr_t base, size_t size) but I find arch/x86/include/asm/io.h to declare:
void __iomem *ioremap(resource_size_t offset, unsigned long size);

Since it is this code that determines the type it could just use matching accurate type from the beginning?

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

Quoted 3 lines

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

+	if (!addr)
+		pr_warn(FW_BUG "ERDT: Failed to map %s at phys addr %pa (size: %u pages)\n",
+		       desc, &base, num_pages);

Please align to open parenthesis.

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

Quoted 5 lines

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

+static __init int cacd_init(struct acpi_subtbl_hdr_16 *subtbl,
+			    struct erdt_domain_info *domain_info)
+{
+	struct acpi_erdt_cacd *cacd = (struct acpi_erdt_cacd *)subtbl;
+	int num_ids, cpu;

Can num_ids ever be negative? If not, please use unsigned type.

Quoted 10 lines

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

+static __init bool parse_rmdd_table(struct acpi_subtbl_hdr_16 *rmdd_hdr)
+{
+	struct erdt_domain_info *domain_info;
+	struct acpi_subtbl_hdr_16 *subtbl;
+	struct acpi_erdt_rmdd *rmdd;
+	u32 subtbl_mask = 0;
+
+	if (rmdd_hdr->length < sizeof(*rmdd)) {
+		pr_warn(FW_BUG "Invalid RMDD length %u bytes\n", rmdd_hdr->length);
+		return false;

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

Quoted 3 lines

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

+	}
+
+	rmdd = (struct acpi_erdt_rmdd *)rmdd_hdr;

Could this initialization be done at time of declaration to be consistent with the other functions parsing tables? The length comparison could then use rmdd->header.length to match how this check is done in all the other places. Being consistent makes the code much easier to understand.

Quoted 16 lines

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

+
+	/* Quietly ignore non-CPU-based L3 domains */
+	if (!(rmdd->flags & RMDD_FLAG_CPU_L3_DOMAIN))
+		return true;
+
+	domain_info = kzalloc_obj(*domain_info, GFP_KERNEL);
+	if (!domain_info)
+		return false;
+
+	domain_info->dom_id = -1;
+
+	domain_info->base[ERDT_MMIO_RMDD_CREG] =
+		erdt_ioremap(rmdd->creg_base, rmdd->creg_size, "RMDD ctrl base");
+	if (!domain_info->base[ERDT_MMIO_RMDD_CREG])
+		goto cleanup;
+

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

Quoted 3 lines

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

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

I find it curious how the implementation varies in how the tables are parsed. For example, here it uses a for () loop to cycle through the tables while enumerate_erdt_table() uses a while() loop for what appears to be the same flow. Are they actually different? Why are the two different patterns needed?

Quoted 5 lines

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

+void erdt_exit(void)
+{
+	struct erdt_domain_info *d, *tmp;
+
+	list_for_each_entry_safe(d, tmp, &domain_info_list, entry) {

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

Quoted 6 lines

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

+		list_del(&d->entry);
+		cleanup_one_domain(d);
+	}
+	erdt_enabled = false;
+	valid_subtbl_mask = 0;
+	first_rmdd_domain_id = 0;

Should erdt_max_rmid be reset also?

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

Quoted 7 lines

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

+
+	if (erdt->header.length < sizeof(*erdt)) {
+		pr_warn(FW_BUG "ERDT: Invalid table length %u bytes\n", erdt->header.length);
+		return -EINVAL;
+	}
+
+	subtbl = (void *)erdt + sizeof(struct acpi_table_erdt);

Please use sizeof(*erdt)

Quoted 4 lines

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

+enum erdt_mmio_type {
+	ERDT_MMIO_RMDD_CREG,
+	ERDT_MMIO_LAST = ERDT_MMIO_RMDD_CREG
+};

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

Quoted 6 lines

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

+
+#define ERDT_MMIO_NUM_TYPES	(ERDT_MMIO_LAST + 1)
+
+/**
+ * struct erdt_domain_info - Per-domain ERDT information
+ * @base:	Array of ioremapped MMIO region base addresses, indexed by ERDT_MMIO_* type

I think "type" can be dropped? The enum is already implicitly an "MMIO type"?

Quoted 14 lines · Reinette Chatre

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

Quoted 6 lines · Chen Yu

static __init bool get_rdt_resources(void)
{

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

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

Functions are not expected to leave dangling state when they return failure. get_rdt_resources() returning false is considered a failure and now the caller is left to clean up the dangling state which is not a familiar pattern to use and thus something that can/will trip people.

Quoted 5 lines

On top of this this implementation pushes the cleanup very far from even the caller making this unfamiliar pattern even harder to recognize.

Please let get_rdt_resources() clean up after itself on failure to find any resources.

The original intent was to encapsulate the cleanup logic as a helper function (erdt_exit()), since it is needed in multiple places across the code. All other call sites invoke this function when necessary. If we place the cleanup call directly inside resctrl_arch_late_init(), it would only execute erdt_exit() once within that function. Alternatively, if we let get_rdt_resources() handle cleanup via erdt_exit(), then we would need to add explicit erdt_exit() calls in __resctrl_arch_late_init() for each failure path - for example, when cpuhp_setup_state() fails or when resctrl_init() fails. Anyway, if code readability is a priority, I'll change the logic as suggested.

Quoted 14 lines

Quoted 14 lines

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

@@ -1114,7 +1115,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;

@@ -1157,6 +1158,15 @@ static int __init resctrl_arch_late_init(void)
 	return 0;
 }
 

Quoted 15 lines

arch/x86/kernel/cpu/resctrl/core.c static int __init resctrl_arch_late_init(void) · line 1168

Quoted 8 lines

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

+static int __init resctrl_arch_late_init(void)
+{
+	int ret = __resctrl_arch_late_init();
+
+	if (ret)
+		erdt_exit();
+	return ret;
+}

Related to earlier comment on cleanup I find this cleanup to be asymmentrical and inconsistent with how resctrl usually does cleanup. Why not do cleanup in (original) resctrl_arch_late_init() to be consistent with other cleanup when failures are encountered during initialization, for example, cpuhp_remove_state()? I find that having the cleanup handled where error is encountered is easier to understand.

Thomas previously suggested this approach to avoid using goto.
https://lore.kernel.org/lkml/871pem5jnh.ffs@fw13/
However, I agree that we can adopt the individual cleanup strategy instead, as it will improve code readability.

Hi Chenyu,

Yes, code readability is a priority. It is not all just about code readability. Please consider all the points I mentioned above. I do not see any justification for get_rdt_resources() leaving dangling state on failure. This is something that will get tripped over in the next inevitable refactor. Apart from that, adding a single "cleanup" function at one location down in the call stack is convenient for *this* implementation based on the *current* state of the code but while doing so it breaks custom, not just of resctrl but of the rest of the kernel also, and because of that makes this code difficult to build on and maintain.

Above is the issue I mentioned earlier where get_rdt_resources() leaves dangling state. If it cleaned up after itself on failure as is the custom in the kernel then resctrl_arch_late_init() does not have to.

If the goal is to avoid goto in __resctrl_arch_late_init() then the failure paths that follow get_rdt_resources() can just call erdt_exit() directly to make obvious where what needs to be cleaned up.

As the suggestion is implemented the cleanup is inconsistent with __resctrl_arch_late_init() doing its own cleanup for code run _after_ erdt_init() and leaving the erdt_init() cleanup to be done further down the stack by __resctrl_arch_late_init()'s caller. While technically correct this inconsistency adds unnecessary complexity and another place that will get tripped over in the next inevitable refactor.

Reading LLC occupancy counters via MMIO requires the per-domain ERDT information, parsed earlier 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 a resctrl monitoring domain. Exclude the CPU from all monitoring 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 later code can read monitoring data 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 +0 -0

 	hdr = resctrl_find_domain(&r->mon_domains, id, &add_pos);
 	if (hdr)
 		cpumask_set_cpu(cpu, &hdr->cpu_mask);

@@ -589,8 +592,14 @@ static void domain_add_cpu_mon(int cpu, struct rdt_resource *r)
 		/* Update the mbm_assign_mode state for the CPU if supported */

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

+
 		break;
 	case RDT_RESOURCE_PERF_PKG:
 		if (!hdr)

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

index 8998cae47090..6257869d0db2 100644
@@ -207,6 +207,72 @@ static __init bool parse_rmdd_table(struct acpi_subtbl_hdr_16 *rmdd_hdr)
 	return false;
 }
 
+bool erdt_cpu_valid(int cpu)

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

+
+	/*
+	 * Find the erdt_domain_info that contains this CPU,
+	 * check if all CPUs in erdt_domain_info's cpumask

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

+		if (cpumask_test_cpu(cpu, &d->cpu_mask)) {
+			if (d->dom_id == -1) {
+				d->dom_id = dom_id;
+			} else if (d->dom_id != dom_id) {
+				pr_warn(FW_BUG "CPU%d's id=%d not equal to CACD domain(%*pbl) id=%d, skip this CPU\n",
+					cpu, dom_id, cpumask_pr_args(&d->cpu_mask), d->dom_id);
+
+				return false;
+			}
+
+			return true;
+		}
+	}
+
+	pr_warn(FW_BUG "Cannot find CACD domain for CPU%d\n", cpu);
+	return false;
+}
+
+/*
+ * Associate ERDT table information with this domain.
+ */
+void erdt_l3_mon_domain_setup(int cpu, struct rdt_domain_hdr *hdr)
+{

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

+			return;
+		}
+	}
+}
+
 void erdt_exit(void)
 {
 	struct erdt_domain_info *d, *tmp;

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

index bdff3ea36e62..bd437c3e5bf0 100644
@@ -99,14 +99,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;

Hi Chenyu,

Quoted 2 lines · Chen Yu

Reading LLC occupancy counters via MMIO requires the per-domain ERDT information, parsed earlier from the ACPI ERDT table, to be reachable

Please avoid using terms about a patch's position in a series. You can just drop "earlier".

Quoted 1 line

so that later code can read monitoring data via ERDT and its sub-tables.

(similar comment as above)
"so that later code can read monitoring data via ERDT and its sub-tables" -> "so that monitoring data can be read via ERDT and its sub-tables"

Quoted 5 lines

Suggested-by Reinette Chatre <[email protected]>

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

Tested-by Hongyu Ning <[email protected]>

---

...

Quoted 8 lines

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

index 23925bcd71d7..c2568b29474e 100644
@@ -580,6 +580,9 @@ static void domain_add_cpu_mon(int cpu, struct rdt_resource *r)
  		return;
  	}
 

arch/x86/kernel/cpu/resctrl/core.c static void domain_add_cpu_mon(int cpu, struct rdt_resource *r) · line 585

Quoted 3 lines

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

+	if (!erdt_cpu_valid(cpu))
+		return;
+

Including this check in domain_add_cpu_mon() means that it is repeated for every monitoring resource. I think this check only needs to be done once? How about moving it to resctrl_arch_online_cpu() where this check can be done before cycling through *any* (monitoring or control) resource?

While domain_info_list is initialized early, this validity check makes concurrent changes to it so locking is required. The current implementation already does this modification with domain_list_lock held but it is not made explicit that this list is now under the protection of this lock. Please add a snippet to the comment above the domain_list_lock to document that it is now also used to protect domain_info_list.

Quoted 10 lines

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

  	hdr = resctrl_find_domain(&r->mon_domains, id, &add_pos);
  	if (hdr)
  		cpumask_set_cpu(cpu, &hdr->cpu_mask);

@@ -589,8 +592,14 @@ static void domain_add_cpu_mon(int cpu, struct rdt_resource *r)
  		/* Update the mbm_assign_mode state for the CPU if supported */
  		if (r->mon.mbm_cntr_assignable)
  			resctrl_arch_mbm_cntr_assign_set_one(r);
-		if (!hdr)
+		if (!hdr) {
  			l3_mon_domain_setup(cpu, id, r, add_pos);

arch/x86/kernel/cpu/resctrl/core.c static void domain_add_cpu_mon(int cpu, struct rdt_resource *r) · line 601

Quoted 5 lines

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

+			hdr = resctrl_find_domain(&r->mon_domains, id, NULL);
+		}
+
+		if (hdr)
+			erdt_l3_mon_domain_setup(cpu, hdr);

The additional search for "hdr" seems unnecessary. Could l3_mon_domain_setup() just call erdt_l3_mon_domain_setup() directly?

Quoted 6 lines

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

+bool erdt_cpu_valid(int cpu)
+{
+	struct erdt_domain_info *d;
+	int dom_id;
+
+	if (!erdt_enabled)

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

Quoted 5 lines

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

+		return true;
+
+	dom_id = get_cpu_cacheinfo_id(cpu, RESCTRL_L3_CACHE);
+	if (dom_id < 0)
+		return true;

Should this be "false"? Perhaps also with a warning similar to domain_add_cpu_mon()'s warning when the domain ID cannot be determined?

Quoted 8 lines

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

+
+	/*
+	 * Find the erdt_domain_info that contains this CPU,
+	 * check if all CPUs in erdt_domain_info's cpumask
+	 * have the same id(L3 id).
+	 *
+	 * For example, erdt_domain_info reports:
+	 * domain0: CPU0, CPU2, domain1: CPU1, CPU3

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

Quoted 6 lines

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

+	 * rdt_domain_hdr reports:
+	 * domain0: CPU0, CPU1, domain1: CPU2, CPU3
+	 * As a result, CPU1, CPU2 should not be covered by resctrl.
+	 */
+	list_for_each_entry(d, &domain_info_list, entry) {
+

(unnecessary empty line)

Quoted 27 lines

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

+		if (cpumask_test_cpu(cpu, &d->cpu_mask)) {
+			if (d->dom_id == -1) {
+				d->dom_id = dom_id;
+			} else if (d->dom_id != dom_id) {
+				pr_warn(FW_BUG "CPU%d's id=%d not equal to CACD domain(%*pbl) id=%d, skip this CPU\n",
+					cpu, dom_id, cpumask_pr_args(&d->cpu_mask), d->dom_id);
+
+				return false;
+			}
+
+			return true;
+		}
+	}
+
+	pr_warn(FW_BUG "Cannot find CACD domain for CPU%d\n", cpu);
+	return false;
+}
+
+/*
+ * Associate ERDT table information with this domain.
+ */
+void erdt_l3_mon_domain_setup(int cpu, struct rdt_domain_hdr *hdr)
+{
+	struct rdt_hw_l3_mon_domain *hw_dom;
+	struct erdt_domain_info *d;
+
+	if (!erdt_enabled)

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

Quoted 6 lines

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

+		return;
+
+	hw_dom = resctrl_to_arch_mon_dom(container_of(hdr, struct rdt_l3_mon_domain, hdr));
+
+	list_for_each_entry(d, &domain_info_list, entry) {
+		if (cpumask_test_cpu(cpu, &d->cpu_mask)) {

Any motivation for why the cpumask is used as a test instead of the domain ID?

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

Quoted 3 lines

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

+			/* Assign the ERDT information to hw_dom */
+			if (!hw_dom->d_info)
+				hw_dom->d_info = d;

This should become obvious if this initialization is done from l3_mon_domain_setup() where hw_mon would have been kzalloc'ed. This means that if hw_dom->d_info is already initialized that there would be *two* ERDT domains that map to an existing resctrl monitoring domain. That looks to be something to complain about?

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

Parse each CMRC sub-table, ioremap its register window, and save a copy of the CMRC table in the corresponding ERDT domain entry so that later monitoring code

can read the counters via MMIO.

Suggested-by Tony Luck <[email protected]>

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

Tested-by Hongyu Ning <[email protected]>

Reviewed-by Thomas Gleixner <[email protected]>

---

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

index 575f8408a9e7..e60c2aea7ebd 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);
 
+int erdt_get_scale(void);
+
 static inline bool resctrl_arch_alloc_capable(void)
 {
 	return rdt_alloc_capable;

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

index 6257869d0db2..422618991927 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 +6 -0

 {
 	return erdt_max_rmid;
 }
 
+int erdt_get_scale(void)
+{
+	return erdt_scale;
+}
+
 static void __iomem *erdt_ioremap(phys_addr_t base, u32 num_pages, const char *desc)
 {
 	void __iomem *addr;

@@ -67,6 +76,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 +16 -0

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

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

+
+	return 0;
+}
+
 static inline struct acpi_subtbl_hdr_16 *rmdd_subtbl(struct acpi_erdt_rmdd *rmdd)
 {
 	return (void *)rmdd + sizeof(*rmdd);

@@ -166,6 +213,16 @@ static __init bool parse_rmdd_table(struct acpi_subtbl_hdr_16 *rmdd_hdr)
 				goto cleanup;
 

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

+
 			break;
 		default:
 			break;

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

index bd437c3e5bf0..27f28b0fead8 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,6 +37,7 @@ enum erdt_mmio_type {
 /**
  * struct erdt_domain_info - Per-domain ERDT information
  * @base:	Array of ioremapped MMIO region base addresses, indexed by ERDT_MMIO_* type
+ * @cmrc:	Copy of the ACPI CMRC sub-table for this domain
  * @cpu_mask:	CPUs belonging to this resource management domain
  * @max_rmid:	Maximum RMID supported by this domain
  * @dom_id:	L3 cache ID shared by all CPUs in this domain (-1 if unset)

Hi Chenyu,

Quoted 3 lines · Chen Yu

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

nit: "an RMD" -> "a monitoring domain"

Quoted 3 lines

Parse each CMRC sub-table, ioremap its register window, and save a copy of the CMRC table in the corresponding ERDT domain entry so that later monitoring code

nit: drop "later"

Quoted 3 lines

static int erdt_max_rmid;

+/* Scale to bytes for the monitoring counters when ERDT is enabled. */

hmmm ... when looking ahead at patch #9 this does not seem to be how this value is used? Instead, when a monitoring counter is read it is scaled using the per-domain acpi_erdt_cmrc::up_scale?

Instead this seems to be the scale used to set/initialize resctrl_rmid_realloc_threshold that is used by the limbo handler?

Quoted 1 line

+static int erdt_scale;

Can the scale ever be negative? Could it be unsigned int?

Actually, looks like the original
MSR based scale obtained via CPUID.(EAX=0FH,ECX=1H) is 32 bits while this new scale value from CMRC is 64 bits. The existing code can thus not accommodate the new values and need to be updated?

Quoted 2 lines

+
int erdt_get_max_rmid(void)

Can this be negative?

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

Quoted 8 lines

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

+static __init int cmrc_init(struct acpi_subtbl_hdr_16 *subtbl,
+			    struct erdt_domain_info *domain_info)
+{
+	struct acpi_erdt_cmrc *cmrc = (struct acpi_erdt_cmrc *)subtbl;
+
+	if (cmrc->header.length < sizeof(*cmrc)) {
+		pr_warn(FW_BUG "Truncated CMRC subtable\n");

Please note there is inconsistency wrt "subtable" vs "sub-table" in error messages.

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

Quoted 8 lines

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

+	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_t(int, erdt_scale, cmrc->up_scale);

Please add a comment to describe why maximum of all domains' scale value is used. This comment may be best placed at global definition of erdt_scale.

Quoted 10 lines

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

+
+	return 0;
+}
+
  static inline struct acpi_subtbl_hdr_16 *rmdd_subtbl(struct acpi_erdt_rmdd *rmdd)
  {
  	return (void *)rmdd + sizeof(*rmdd);

@@ -166,6 +213,16 @@ static __init bool parse_rmdd_table(struct acpi_subtbl_hdr_16 *rmdd_hdr)
  				goto cleanup;
 

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

Quoted 10 lines

arch/x86/kernel/cpu/resctrl/erdt.c +9 -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.
+			 */
+			if (!(subtbl_mask & BIT(ACPI_ERDT_TYPE_CMRC)) &&
+			    !cmrc_init(subtbl, domain_info))
+				subtbl_mask |= BIT(ACPI_ERDT_TYPE_CMRC);

How is cmrc_init() failure handled?

Quoted 7 lines · Reinette Chatre

On 7/25/26 2:23 AM, Chen Yu wrote:

Actually, looks like the original
MSR based scale obtained via CPUID.(EAX=0FH,ECX=1H) is 32 bits while this new scale value from CMRC is 64 bits. The existing code can thus not accommodate the new values and need to be updated?

Yes, the legacy CPUID reports it as 32 bits, while CMRC is declared as 64 bits.

In theory, we should change the scale type from unsigned int to u64 to accommodate both the legacy CPUID and CMRC. However, it seems unlikely that the scale would exceed 32 bits. If the scale were 32 bits, the L3 occupancy would be at least 2^32 − 1, which is about 4 GB. We have not yet seen platform with 4 GB of L3 cache. So perhaps we can keep erdt_scale as unsigned int for now IMO.

Hi Chenyu,

Quoted 9 lines · Yu C

Quoted 7 lines · Reinette Chatre

On 7/25/26 2:23 AM, Chen Yu wrote:

Actually, looks like the original
MSR based scale obtained via CPUID.(EAX=0FH,ECX=1H) is 32 bits while this new scale value from CMRC is 64 bits. The existing code can thus not accommodate the new values and need to be updated?

Quoted 7 lines

Yes, the legacy CPUID reports it as 32 bits, while CMRC is declared as 64 bits.

In theory, we should change the scale type from unsigned int to u64 to accommodate both the legacy CPUID and CMRC. However, it seems unlikely that the scale would exceed 32 bits. If the scale were 32 bits, the L3 occupancy would be at least 2^32 − 1, which is about 4 GB. We have not yet seen platform with 4 GB of L3 cache. So perhaps we can keep erdt_scale as unsigned int for now IMO.

It sounds like you are saying that the spec you are enabling should not be using 64 bits for this value. It is unexpected to me that work on enabling of a spec adds such constraint without clear motivation. If you find that the spec is not correct then I think it would be better to have it changed there instead of building what appears to be unsupported assumptions into resctrl.

Hi Reinette,

Quoted 20 lines · Reinette Chatre

Hi Chenyu,

Quoted 15 lines · Yu C

Quoted 6 lines · Reinette Chatre

On 7/25/26 2:23 AM, Chen Yu wrote:
Actually, looks like the original
MSR based scale obtained via CPUID.(EAX=0FH,ECX=1H) is 32 bits while this new scale value from CMRC is 64 bits. The existing code can thus not accommodate the new values and need to be updated?

Yes, the legacy CPUID reports it as 32 bits, while CMRC is declared as 64 bits.

In theory, we should change the scale type from unsigned int to u64 to accommodate both the legacy CPUID and CMRC. However, it seems unlikely that the scale would exceed 32 bits. If the scale were 32 bits, the L3 occupancy would be at least 2^32 − 1, which is about 4 GB. We have not yet seen platform with 4 GB of L3 cache. So perhaps we can keep erdt_scale as unsigned int for now IMO.

Quoted 5 lines

It sounds like you are saying that the spec you are enabling should not be using 64 bits for this value. It is unexpected to me that work on enabling of a spec adds such constraint without clear motivation. If you find that the spec is not correct then I think it would be better to have it changed there instead of building what appears to be unsupported assumptions into resctrl.

OK, let me check with the team.

thanks,
Chenyu

Quoted 7 lines

Quoted 7 lines

Quoted 5 lines

In theory, we should change the scale type from unsigned int to u64 to accommodate both the legacy CPUID and CMRC. However, it seems unlikely that the scale would exceed 32 bits. If the scale were 32 bits, the L3 occupancy would be at least 2^32 − 1, which is about 4 GB. We have not yet seen platform with 4 GB of L3 cache. So perhaps we can keep erdt_scale as unsigned int for now IMO.

Quoted 7 lines

Quoted 5 lines

It sounds like you are saying that the spec you are enabling should not be using 64 bits for this value. It is unexpected to me that work on enabling of a spec adds such constraint without clear motivation. If you find that the spec is not correct then I think it would be better to have it changed there instead of building what appears to be unsupported assumptions into resctrl.

OK, let me check with the team.

Looking at the RDT architecture spec that defines the CMRC table it looks to me like the "CMT Counter Upscaling Factor" was made 8-bytes just to round the table out to a whole number or quad-words.

This value represents the granularity of cache occupancy reporting. The feature would be worthless if this was >=4GB no matter how large the LLC cache may be in future generations.

If large values are a big concern then perhaps include a sanity check in the code:

	if (cmrc->upscale >= SZ_4G)
		pr_warn(FW_BUG "insane CMRC upscale value 0x%llx\n", cmrc->upscale);

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

index 569894d6e5c8..786828a0a3c2 100644

arch/x86/kernel/cpu/resctrl/monitor.c static u64 get_corrected_val(struct rdt_resource *r, struct rdt_l3_mon_domain *d · line 244

Quoted 6 lines

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

-int resctrl_arch_rmid_read(struct rdt_resource *r, struct rdt_domain_hdr *hdr,
-			   u32 unused, u32 rmid, enum resctrl_event_id eventid,
-			   void *arch_priv, u64 *val, void *ignored)
+static int arch_l3_read_event(struct rdt_domain_hdr *hdr, u32 rmid,
+			      enum resctrl_event_id eventid, u64 *val,
+			      struct rdt_resource *r)

Could you please keep the same parameter order as original resctrl_arch_rmid_read()?

From: Tony Luck <[email protected]>

LLC occupancy can be read on any CPU when the counter is accessed via MMIO, so such an event is read from task context on whatever CPU the caller happens to be running on rather than being bounced to a CPU in the monitoring domain. mon_evt::any_cpu marks these CPU-agnostic events.

__l3_mon_event_count() calls smp_processor_id() to find the CPU to read from. For an any_cpu event that lookup is unsafe:
the code runs in preemptible task context, so smp_processor_id() emits a debug warning.

Skip the current-CPU lookup when an event's any_cpu flag is set, events with this flag do not require execution on a specific CPU. For legacy MSR-based access, update rmid_read::err if the reading of the event was dispatched to a wrong CPU, according to the change at:
https://lore.kernel.org/lkml/6b3c66a49788828bd8c04a6911bd74c91ccd56f3.1782857711.git.reinette.chatre@intel.com/

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

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

Tested-by Hongyu Ning <[email protected]>

---

Hi Chenyu,

Quoted 2 lines · Chen Yu

From: Tony Luck <[email protected]>

Quoted 4 lines

LLC occupancy can be read on any CPU when the counter is accessed via MMIO, so such an event is read from task context on whatever CPU the caller happens to be running on rather than being bounced to a CPU in the monitoring domain. mon_evt::any_cpu marks these CPU-agnostic events.

Please let the context describe what the *current* code does. This changelog describes context that does not exist (yet) and then presents the change in this patch as a bugfix. This is not accurate and misrepresents this change.

Quoted 6 lines

__l3_mon_event_count() calls smp_processor_id() to find the CPU to read from. For an any_cpu event that lookup is unsafe:
the code runs in preemptible task context, so smp_processor_id() emits a debug warning.

Quoted 5 lines

Skip the current-CPU lookup when an event's any_cpu flag is set, events with this flag do not require execution on a specific CPU. For legacy MSR-based access, update rmid_read::err if the reading of the event was dispatched to a wrong CPU, according to the change at:
https://lore.kernel.org/lkml/6b3c66a49788828bd8c04a6911bd74c91ccd56f3.1782857711.git.reinette.chatre@intel.com/

This has been merged now so it should not be necessary to call this out, at least in this format

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 an event, 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 read its monitoring data via MMIO exist, otherwise callers would

take the ERDT path and find no implementation behind it. Features are added to erdt_support() as their MMIO implementations are introduced.

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

Tested-by Hongyu Ning <[email protected]>

---
v5->v6:
New patch. Split out of the v5 "x86/resctrl: Introduce helpers to read L3 occupancy via MMIO" patch, which carried two logical changes. (Reinette Chatre)
Rename erdt_support_features() to erdt_support(). (Reinette Chatre) Reword "expects x86 feature flag as parameters" to "expects an x86 feature flag as parameter". (Reinette Chatre)
Explain in the changelog why erdt_support() returns false here: a feature is only reported as ERDT backed once the MMIO accessors that read its monitoring data exist. (Reinette Chatre)
Drop the unused asm/cpufeatures.h include. (Reinette Chatre)

---
 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 |  2 ++
 4 files changed, 31 insertions(+)

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

index e60c2aea7ebd..5491853113dd 100644

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

 static inline bool resctrl_arch_alloc_capable(void)

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

index c2568b29474e..74087d04f1da 100644
@@ -887,6 +887,29 @@ bool rdt_cpu_has(int flag)
 	return ret;
 }
 
+bool erdt_cpu_has(int flag)
+{
+	struct rdt_options *o;
+	bool ret;
+
+	ret = erdt_support(flag);
+
+	if (!ret)
+		return ret;
+
+	for (o = rdt_options; o < &rdt_options[NUM_RDT_OPTIONS]; o++) {
+		if (flag == o->flag) {
+			if (o->force_off)
+				ret = false;
+			if (o->force_on)
+				ret = true;
+			break;
+		}
+	}
+
+	return ret;
+}
+
 bool resctrl_arch_is_evt_configurable(enum resctrl_event_id evt)
 {
 	if (!rdt_cpu_has(X86_FEATURE_BMEC))

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

index 422618991927..167d2dfc368d 100644
@@ -37,6 +37,11 @@ static int erdt_max_rmid;
 /* Scale to bytes for the monitoring counters when ERDT is enabled. */
 static int erdt_scale;
 
+bool erdt_support(int flag)
+{
+	return false;
+}
+
 int erdt_get_max_rmid(void)
 {
 	return erdt_max_rmid;

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

index 27f28b0fead8..d14a36d65b05 100644

Hi Chenyu,

Quoted 10 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 considers firmware (ERDT table and its sub-tables) support for an event, 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.

Quoted 3 lines

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 read its monitoring data via MMIO exist, otherwise callers would

Are these helpers limited to monitoring ERDT features? First paragraph mentions "support for an event" and above mentions "read its monitoring data" that imply these helpers are limited to monitoring features but I do not think this is the case?

Quoted 2 lines

take the ERDT path and find no implementation behind it. Features are added to erdt_support() as their MMIO implementations are introduced.

nit: "are added" -> "should be added"

Quoted 30 lines

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

Tested-by Hongyu Ning <[email protected]>

---
v5->v6:
New patch. Split out of the v5 "x86/resctrl: Introduce helpers to read L3 occupancy via MMIO" patch, which carried two logical changes. (Reinette Chatre)
Rename erdt_support_features() to erdt_support(). (Reinette Chatre) Reword "expects x86 feature flag as parameters" to "expects an x86 feature flag as parameter". (Reinette Chatre)
Explain in the changelog why erdt_support() returns false here: a feature is only reported as ERDT backed once the MMIO accessors that read its monitoring data exist. (Reinette Chatre)
Drop the unused asm/cpufeatures.h include. (Reinette Chatre)

---
  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 |  2 ++
  4 files changed, 31 insertions(+)

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

index e60c2aea7ebd..5491853113dd 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 54

Quoted 3 lines

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

+bool erdt_cpu_has(int flag);
  int erdt_get_scale(void);
 

Can this declaration be dropped? Below there is a duplicate declaration added to arch/x86/kernel/cpu/resctrl/internal.h that should be sufficient?

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. This CMRC information is stored in the rdt_hw_l3_mon_domain, which could be accessed directly.

Currently, the per-domain limbo handler is still in use. There is no need to switch to a global limbo handler, because even after such a switch, the worker thread would still have to iterate through all domains one by one. The per-domain handler already accomplishes this using a worker thread rather than costly IPIs, so there is no clear benefit to switching to a global handler.

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

+
+	if (erdt_cpu_has(X86_FEATURE_CQM_OCCUP_LLC)) {
+		escale = erdt_get_scale();
+		if (escale)
+			scale = escale;
+	}
 
 	/* h/w works in units of "boot_cpu_data.x86_cache_occ_scale" */
 	val /= scale;

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

index 74087d04f1da..b528023a7bcc 100644
@@ -997,7 +997,10 @@ static __init bool get_rdt_mon_resources(void)
 	struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_L3].r_resctrl;
 	bool ret = false;
 
-	if (rdt_cpu_has(X86_FEATURE_CQM_OCCUP_LLC)) {
+	if (erdt_cpu_has(X86_FEATURE_CQM_OCCUP_LLC)) {
+		resctrl_enable_mon_event(QOS_L3_OCCUP_EVENT_ID, true, 0, NULL);
+		ret = true;
+	} else if (rdt_cpu_has(X86_FEATURE_CQM_OCCUP_LLC)) {
 		resctrl_enable_mon_event(QOS_L3_OCCUP_EVENT_ID, false, 0, NULL);
 		ret = true;
 	}

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

index 167d2dfc368d..5f9377c80be6 100644
@@ -26,6 +26,10 @@ static bool erdt_enabled;
 #define CMRC_SUPPORTED_INDEX_FN		1
 #define RMDD_FLAG_CPU_L3_DOMAIN		BIT(0)
 
+/* Set in a monitoring counter when it holds no valid data to report. */
+#define UNAVAILABLE_COUNTER		BIT_ULL(63)
+#define CMRC_FLAG_UNAVAILABLE_BIT	BIT(0)
+
 /* Bitmask of valid sub-tables found in the first RMDD, used to ensure all RMDDs match. */
 static u32 valid_subtbl_mask;
 

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

 	return false;
 }
 

@@ -52,6 +59,58 @@ int erdt_get_scale(void)
 	return erdt_scale;
 }
 
+static u32 cmrc_index_function_1(struct acpi_erdt_cmrc *cmrc, u32 rmid)
+{
+	/*
+	 * MMIO_offset_for_RMID# =
+	 *   (RMID / ClumpSize) * Stride +
+	 *   (RMID % ClumpSize) * 8
+	 */
+	return (rmid / cmrc->clump_size) * cmrc->clump_stride +
+	       (rmid % cmrc->clump_size) * 8;
+}
+
+static int erdt_read_l3_occupancy(const struct erdt_domain_info *d, u32 rmid, u64 *val)
+{
+	struct acpi_erdt_cmrc *cmrc;
+	u64 l3_cmt_count;
+	u32 offset;
+
+	cmrc = d->cmrc;
+	if (!cmrc)
+		return -EIO;
+
+	offset = cmrc_index_function_1(cmrc, rmid);
+	/* Overflow of cmt_reg_size * SZ_4K already validated in erdt_ioremap(). */
+	if (offset + sizeof(u64) > (u32)cmrc->cmt_reg_size * SZ_4K)
+		return -EINVAL;
+
+	l3_cmt_count = readq(d->base[ERDT_MMIO_CMRC_BASE] + offset);
+	if ((cmrc->flags & CMRC_FLAG_UNAVAILABLE_BIT) &&
+	    (l3_cmt_count & UNAVAILABLE_COUNTER))
+		return -EINVAL;
+
+	*val = l3_cmt_count * cmrc->up_scale;
+
+	return 0;
+}
+
+int erdt_mon_read(struct rdt_domain_hdr *hdr, enum resctrl_event_id evtid, u32 rmid, u64 *val)
+{
+	struct rdt_hw_l3_mon_domain *hw_dom;
+	const struct erdt_domain_info *d;
+
+	hw_dom = resctrl_to_arch_mon_dom(container_of(hdr, struct rdt_l3_mon_domain, hdr));
+	d = hw_dom->d_info;
+	if (!d)
+		return -EIO;
+
+	if (evtid == QOS_L3_OCCUP_EVENT_ID)
+		return erdt_read_l3_occupancy(d, rmid, val);
+
+	return -EIO;
+}
+
 static void __iomem *erdt_ioremap(phys_addr_t base, u32 num_pages, const char *desc)
 {
 	void __iomem *addr;

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

index d14a36d65b05..a3cb32e8fabc 100644
@@ -158,7 +158,11 @@ struct msr_param {
  *			which has been corrected for features like CDP.
  * @msr_base:		Base MSR address for CBMs
  * @msr_update:		Function pointer to update QOS MSRs
- * @mon_scale:		cqm counter * mon_scale = occupancy in bytes
+ * @mon_scale:		Scale factor applied to a raw counter value on the
+ *			MSR-based read path: CMT occupancy counter * mon_scale =
+ *			occupancy in bytes, and MBM chunk count * mon_scale = bytes
+ *			transferred. ERDT reads occupancy via MMIO and applies its
+ *			own firmware-provided scale instead.
  * @mbm_width:		Monitor width, to detect and correct for overflow.
  * @cdp_enabled:	CDP state of this resource
  * @mbm_cntr_assign_enabled:	ABMC feature is enabled

@@ -292,6 +296,7 @@ static inline bool intel_handle_aet_option(bool force_off, char *tok) { return f
 bool erdt_support(int flag);
 bool erdt_cpu_has(int flag);
 int erdt_get_max_rmid(void);
+int erdt_mon_read(struct rdt_domain_hdr *hdr, enum resctrl_event_id evtid, u32 rmid, u64 *val);
 int erdt_init(void);
 void erdt_exit(void);
 

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

index 786828a0a3c2..d33a865e59d4 100644
@@ -284,6 +284,10 @@ int resctrl_arch_rmid_read(struct rdt_resource *r, struct rdt_domain_hdr *hdr,
 
 	switch (r->rid) {
 	case RDT_RESOURCE_L3:
+		if (eventid == QOS_L3_OCCUP_EVENT_ID &&
+		    erdt_cpu_has(X86_FEATURE_CQM_OCCUP_LLC))
+			return erdt_mon_read(hdr, eventid, rmid, val);
+
 		return arch_l3_read_event(hdr, rmid, eventid, val, r);
 	case RDT_RESOURCE_PERF_PKG:
 		return intel_aet_read_event(hdr->id, rmid, arch_priv, val);

Hi Chenyu,

Quoted 5 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. This CMRC information is stored in the rdt_hw_l3_mon_domain, which could be accessed directly.

Please write in imperative tone.

Quoted 6 lines

Currently, the per-domain limbo handler is still in use. There is no need to switch to a global limbo handler, because even after such a switch, the worker thread would still have to iterate through all domains one by one. The per-domain handler already accomplishes this using a worker thread rather than costly IPIs, so there is no clear benefit to switching to a global handler.

Could you please elaborate how a global limbo handler would require IPIs?

Quoted 6 lines

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

index 5491853113dd..0948f64856ef 100644
@@ -132,7 +132,13 @@ static inline void __resctrl_sched_in(struct task_struct *tsk)
 

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

Quoted 4 lines

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

  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;

related to earlier topic, "scale" being unsigned int is ok since x86_cache_occ_scale is initialized from 32bits. As I understand it the ERDT scale value is initialized from 64 bits instead so the existing types do not seem to accommodate?