Hi Reinette,
On 5/27/2025 6:42 PM, Reinette Chatre wrote:
Hi Babu,
On 5/27/25 11:40 AM, Moger, Babu wrote:
On 5/27/25 12:54, Reinette Chatre wrote:
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()?
If you have an alternative proposal that would accurately reflect the ABMC
and existing L3 MON features then we can surely consider it.
I don't see any other option at this point. Will change it next revision.
Thanks
Babu
