Hi Reinette,
On 5/27/25 12:54, Reinette Chatre wrote:
> Hi Babu,
>
> On 5/27/25 10:23 AM, Moger, Babu wrote:
>> On 5/22/25 15:51, Reinette Chatre wrote:
>>> On 5/15/25 3:51 PM, Babu Moger wrote:
>
>>>> diff --git a/arch/x86/kernel/cpu/cpuid-deps.c b/arch/x86/kernel/cpu/cpuid-deps.c
>>>> index a2fbea0be535..2f54831e04e5 100644
>>>> --- a/arch/x86/kernel/cpu/cpuid-deps.c
>>>> +++ b/arch/x86/kernel/cpu/cpuid-deps.c
>>>> @@ -71,6 +71,8 @@ static const struct cpuid_dep cpuid_deps[] = {
>>>> { X86_FEATURE_CQM_MBM_LOCAL, X86_FEATURE_CQM_LLC },
>>>> { X86_FEATURE_BMEC, X86_FEATURE_CQM_MBM_TOTAL },
>>>> { X86_FEATURE_BMEC, X86_FEATURE_CQM_MBM_LOCAL },
>>>> + { X86_FEATURE_ABMC, X86_FEATURE_CQM_MBM_TOTAL },
>>>> + { X86_FEATURE_ABMC, X86_FEATURE_CQM_MBM_LOCAL },
>>>
>>> Is this dependency still accurate now that the implementation switched to the
>>> "extended event ID" variant of ABMC that no longer uses the event IDs associated
>>> with X86_FEATURE_CQM_MBM_TOTAL and X86_FEATURE_CQM_MBM_LOCAL?
>>
>> That's a good question. Unfortunately, we may need to retain this
>> dependency for now, as a significant portion of the code relies on
>> functions like resctrl_is_mbm_event(), resctrl_is_mbm_enabled(),
>> resctrl_arch_is_mbm_total_enabled(), and others.
>>
>
> Avoiding needing to change code is not a valid reason.
>
> I think that without this dependency the code will
> still rely on "functions like resctrl_is_mbm_event(),
> resctrl_is_mbm_enabled(), resctrl_arch_is_mbm_total_enabled(),
> and others." though.
>
> The core shift is to stop thinking about QOS_L3_MBM_TOTAL_EVENT_ID
> to mean the same as X86_FEATURE_CQM_MBM_TOTAL, similarly to stop
> thinking about QOS_L3_MBM_LOCAL_EVENT_ID to mean the same as
> X86_FEATURE_CQM_MBM_LOCAL.
oh. ok.
>
> I expected that for backwards compatibility ABMC will start by
> enabling QOS_L3_MBM_TOTAL_EVENT_ID and QOS_L3_MBM_LOCAL_EVENT_ID
> as part of its initialization, configuring them with the current
> defaults for which memory transactions are expected to be monitored
> by each. With these events enabled the existing flows using, for
> example, resctrl_is_mbm_event(), will continue to work as expected, no?
Yes. It will work as it uses event id.
>
> This would require more familiarity with L3 monitoring enumeration
> on AMD since it will still be required to determine the number of
> RMIDs etc. but if ABMC does not actually depend on these CQM features
> then the current enumeration would need to be re-worked anyway.
Are you suggesting to remove the dependency and rework ABMC enumeration in
get_rdt_mon_resources()?
--
Thanks
Babu Moger
