Re: [PATCH v8 3/9] x86/resctrl: Parse ACPI ERDT table and save CACD cpumask for RMDD domains

[Reinette Chatre]

Hi Chenyu,

On 9/29/26 3:32 AM, Chen Yu wrote:
> On Mon, Sep 28, 2026 at 02:37:45PM -0700, Reinette Chatre wrote:
>> On 9/17/26 9:49 PM, Chen Yu wrote:


>>
>>   x86/resctrl: Parse ACPI ERDT table and build per-RMD CPU masks
>>
>>   Enhanced RDT (ERDT) exposes per-domain MMIO registers that resctrl needs
>>   in order to read hardware monitoring counters on the upcoming MMIO-based
>>   path. The kernel discovers this hardware through a single per-platform
>>   ERDT ACPI table.
>>
>>   Each Resource Management Domain Description (RMDD) sub-table within the
>>   ERDT table carries the MMIO base of one resource management domain (RMD) and,
>>   when RMDD_FLAG_CPU_L3_DOMAIN is set, identifies the domain as a CPU-scoped
>>   L3 monitoring domain. The set of CPUs in the domain is listed by x2APIC ID
> 
> 
> Maybe remove "monitoring" since RMDD is for a CPU mask, not specific to whether
> the mask is a monitor or control domain.

Sure.

> 
>>   in a nested CPU Agent Collection Description (CACD) sub-table.
>>
>>   Walk the ERDT table's RMDD sub-tables in preparation for attaching each
>>   ERDT domain to a resctrl L3 monitoring domain. For each CPU-based L3 RMDD,
>>   ioremap its control-register region, walk its nested CACD entries,
>>   translate each x2APIC ID to a logical CPU, and record the result on the
>>   ERDT domain's erdt_domain_info. Record the largest RMID that is valid
> 
> I think it is the minimum of the largest RMIDs exposed by each RMD.

This sounds the same to me? Your suggestion is closer to the math and code
while "largest RMID that is valid on every RMD"(*) describes what it actually
means? Having the changelog describe the changes and goals at high level makes
it easier to understand the low level details/code found in the patch.

(*) I just noticed the incorrect use of RMDD vs RMD in my example, highligthing
    that it should not just be copy&pasted.

> 
>>   on every RMDD so a later reader cannot access an RMID that is
>>   unsupported on some domain, and require every RMDD to advertise the
>>   same set of sub-table types so downstream code can rely on a uniform
>>   shape.
>>

...

>>> +			    struct erdt_domain_info *domain_info)
>>> +{
>>> +	struct acpi_erdt_cacd *cacd = (struct acpi_erdt_cacd *)subtbl;
>>> +	unsigned int num_ids;
>>> +	int cpu;
>>> +
>>> +	if (cacd->header.length < struct_size(cacd, X2APICIDS, 1)) {
>>> +		pr_warn(FW_BUG "Invalid x2apicid CACD table\n");
>>> +		return -EIO;
>>> +	}
>>> +
>>> +	num_ids = (cacd->header.length - sizeof(*cacd)) / sizeof(cacd->X2APICIDS[0]);
>>> +
>>> +	for (unsigned int i = 0; i < num_ids; i++) {
>>> +		cpu = topo_lookup_cpuid(cacd->X2APICIDS[i]);
>>> +		if (cpu < 0) {
>>> +			pr_warn(FW_BUG "Unknown x2apicid 0x%x\n", cacd->X2APICIDS[i]);
>>> +			return -EIO;
>>
>> The Sashiko reported issue looks real to me:
>> https://sashiko.dev/#/patchset/cover.1789705667.git.yu.c.chen%40intel.com?part=3
>>
>> Were you able to try the example where system limits the number of processors
>> via maxcpus= and see if ERDT still works?
>>
> 
> Yes, this is a valid case, but maxcpus= should be replaced by nr_cpus=.
> maxcpus= limits the online CPUs during bootup, and after bootup, those
> offline CPUs can still be queried by topo_lookup_cpuid(). While for
> nr_cpus=, those offline CPUs are not in the possible CPU mask, they can
> not be online after bootup, and they can not be found by

ah - thank you for pointing this out.

> topo_lookup_cpuid() either. If there is an inconsistency between the
> possible_cpus_mask and the CPU x2APIC set exposed by CACD, all ERDTs will
> be disabled. This is too strong. Let me skip this CPU if it cannot be found
> in the possible CPU mask.
>  
Reinette