Re: [PATCH v4 6/6] x86/resctrl: Add support for L3 occupancy monitoring via RMID MMIO read

[Reinette Chatre <reinette.chatre@intel.com>]

Hi Chenyu,

On 6/22/26 10:00 PM, Chen, Yu C wrote:
> Hi Reinette,
> 
> On 6/23/2026 5:30 AM, Reinette Chatre wrote:
>> Hi Chenyu,
>>>>> diff --git a/arch/x86/include/asm/resctrl.h b/arch/x86/include/asm/resctrl.h
>>>>> index 97c2f6bc7a5f..9b3b03279dd8 100644
>>>>> --- a/arch/x86/include/asm/resctrl.h
>>>>> +++ b/arch/x86/include/asm/resctrl.h
>>>>> @@ -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
>>>>> --- a/arch/x86/kernel/cpu/resctrl/core.c
>>>>> +++ b/arch/x86/kernel/cpu/resctrl/core.c
>>>>> @@ -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);
>>>>>            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.
>>
>> 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.
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.

Reinette