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: <8e22044c-f36e-41d0-bf1b-311ac8758da3@intel.com>
Date: Tue, 27 May 2025 16:42:45 -0700
From: Reinette Chatre <reinette.chatre@...el.com>
To: <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 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.

Reinette