linux-kernel - Re: [PATCH v13 01/27] x86/cpufeatures: Add support for Assignable Bandwidth Monitoring Counters (ABMC)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <bbb6b6fb-7b9d-45ab-87eb-82747c370cb1@amd.com>
Date: Wed, 28 May 2025 11:18:51 -0500
From: "Moger, Babu" <bmoger@....com>
To: Reinette Chatre <reinette.chatre@...el.com>, babu.moger@....com,
 corbet@....net, tony.luck@...el.com, tglx@...utronix.de, mingo@...hat.com,
 bp@...en8.de, dave.hansen@...ux.intel.com
Cc: james.morse@....com, dave.martin@....com, fenghuay@...dia.com,
 x86@...nel.org, hpa@...or.com, paulmck@...nel.org,
 akpm@...ux-foundation.org, thuth@...hat.com, rostedt@...dmis.org,
 ardb@...nel.org, gregkh@...uxfoundation.org, daniel.sneddon@...ux.intel.com,
 jpoimboe@...nel.org, alexandre.chartre@...cle.com,
 pawan.kumar.gupta@...ux.intel.com, thomas.lendacky@....com,
 perry.yuan@....com, seanjc@...gle.com, kai.huang@...el.com,
 xiaoyao.li@...el.com, kan.liang@...ux.intel.com, xin3.li@...el.com,
 ebiggers@...gle.com, xin@...or.com, sohil.mehta@...el.com,
 andrew.cooper3@...rix.com, mario.limonciello@....com,
 linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
 peternewman@...gle.com, maciej.wieczor-retman@...el.com, eranian@...gle.com,
 Xiaojian.Du@....com, gautham.shenoy@....com
Subject: Re: [PATCH v13 01/27] x86/cpufeatures: Add support for Assignable
 Bandwidth Monitoring Counters (ABMC)

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