Re: [PATCH v6 4/9] x86/resctrl: Attach ACPI ERDT information to L3 mon domain on CPU online
From: Chen, Yu C
Date: Tue Aug 25 2026 - 01:54:42 EST
Hi Reinette,
On 8/20/2026 7:04 AM, Reinette Chatre wrote:
Hi Chenyu,
On 7/25/26 2:23 AM, Chen Yu wrote:
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".
Got it, will do.
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"
OK, will do.
Suggested-by: Reinette Chatre <reinette.chatre@xxxxxxxxx>
Signed-off-by: Chen Yu <yu.c.chen@xxxxxxxxx>
Tested-by: Hongyu Ning <hongyu.ning@xxxxxxxxxxxxxxx>
---
...
diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
index 23925bcd71d7..c2568b29474e 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
@@ -580,6 +580,9 @@ static void domain_add_cpu_mon(int cpu, struct rdt_resource *r)
return;
}
+ 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?
Yes, moving it to resctrl_arch_online_cpu() is more reasonable, will adjust the code.
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.
OK, will add lockdep_assert_held(&domain_list_lock) in erdt_cpu_valid().
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);
+ 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?
OK, l3_mon_domain_setup() can leverage erdt_l3_mon_domain_setup() to attach
the ERDT domain to the corresponding newly-created hw domain. And later when
other CPUs of the same domain are onlined, they share the same hw_dom, so there
is no need to re-attach the ERDT domain again. I'll adjust the code accordingly.
+bool erdt_cpu_valid(int cpu)
+{
+ struct erdt_domain_info *d;
+ int dom_id;
+
+ if (!erdt_enabled)
+ 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?
Right, will fix this and also !erdt_enabled case, and add a warning here.
+
+ /*
+ * 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
+ * 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)
OK, will remove this.
+ 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)
+ 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?
Let me switch to to compare the erdt_domain.id and the llc_id directly.
+ /* 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?
Yes, this should be a firmware bug and let me add a warning here.
thanks,
Chenyu