Re: [PATCH v4 1/6] x86/resctrl: Parse ACPI ERDT table and map RMDD domains by L3 cache ID

["Chen, Yu C" <yu.c.chen@intel.com>]

Hi Reinette,

On 6/23/2026 5:28 AM, Reinette Chatre wrote:
> Hi Chenyu,
> 
> On 6/22/26 2:07 AM, Chen, Yu C wrote:
>> On 6/19/2026 7:37 AM, Reinette Chatre wrote:
>>> On 6/13/26 12:56 AM, Chen Yu wrote:
> 
> ...
> 
>>>>    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.
>>>
>>
>> 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.
> 

OK, will do.

>>
>>
>>>>    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
>>>> @@ -538,7 +538,7 @@ config X86_CPU_RESCTRL
>>>>      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.
>>>
>>
>> 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.
> 

Got it. In theory, the 32-bit removal could be integrated into [PATCH 6/6],
where readq/writeq are introduced. We can add corresponding comments and 
update
the commit log for [PATCH 6/6] to explain why 32-bit support is being 
dropped.
However, Handling it this way would sneak this trivial change into the 
patch series.
Would the following two alternatives be applicable?
1. Add explanatory text in the current [PATCH 1/6] to state that 
removing 32-bit
    is preparatory work for CMT, alongside background context on the 
motivation
    for this removal. Or
2. Create a standalone preparatory patch solely responsible for dropping 
32-bit support.

>>>> diff --git a/arch/x86/include/asm/resctrl.h b/arch/x86/include/asm/resctrl.h
>>>> index 575f8408a9e7..97c2f6bc7a5f 100644
>>>> --- a/arch/x86/include/asm/resctrl.h
>>>> +++ b/arch/x86/include/asm/resctrl.h
>>>> @@ -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?
> 
> 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.

OK, so I suppose ERDT should only be visible within arch/x86.

>>>> +
>>>> +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
> 
> This could also be named "__erdt_enabled" to highlight that it is internal.
> 

OK.

[ ... ]

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

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

thanks,
Chenyu