| 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
| ||
|
Message-ID: <aca368e0-0936-4852-9cf6-688931e0c65d@amd.com>
Date: Wed, 25 Jun 2025 12:58:31 -0500
From: "Moger, Babu" <babu.moger@....com>
To: Reinette Chatre <reinette.chatre@...el.com>, corbet@....net,
tony.luck@...el.com, Dave.Martin@....com, james.morse@....com,
tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
dave.hansen@...ux.intel.com
Cc: x86@...nel.org, hpa@...or.com, akpm@...ux-foundation.org,
rostedt@...dmis.org, paulmck@...nel.org, thuth@...hat.com, ardb@...nel.org,
gregkh@...uxfoundation.org, seanjc@...gle.com, thomas.lendacky@....com,
pawan.kumar.gupta@...ux.intel.com, manali.shukla@....com,
perry.yuan@....com, kai.huang@...el.com, peterz@...radead.org,
xiaoyao.li@...el.com, kan.liang@...ux.intel.com, mario.limonciello@....com,
xin3.li@...el.com, gautham.shenoy@....com, xin@...or.com,
chang.seok.bae@...el.com, fenghuay@...dia.com, peternewman@...gle.com,
maciej.wieczor-retman@...el.com, eranian@...gle.com,
linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v14 09/32] x86/resctrl: Detect Assignable Bandwidth
Monitoring feature details
Hi Reinette,
On 6/24/25 16:33, Reinette Chatre wrote:
> Hi Babu,
>
> On 6/13/25 2:04 PM, Babu Moger wrote:
>> ABMC feature details are reported via CPUID Fn8000_0020_EBX_x5.
>> Bits Description
>> 15:0 MAX_ABMC Maximum Supported Assignable Bandwidth
>> Monitoring Counter ID + 1
>>
>> The feature details are documented in APM listed below [1].
>> [1] AMD64 Architecture Programmer's Manual Volume 2: System Programming
>> Publication # 24593 Revision 3.41 section 19.3.3.3 Assignable Bandwidth
>> Monitoring (ABMC).
>>
>> Detect the feature and number of assignable counters supported. Also,
>> enable QOS_L3_MBM_TOTAL_EVENT_ID and QOS_L3_MBM_LOCAL_EVENT_ID upon
>> detecting the ABMC feature. The current expectation is to support
>> these two events by default when ABMC is enabled.
>
> "The current expectation ..." this need not be vague since this is what
> this series does. Perhaps previous sentence can be:
> "For backward compatibility, upon detecting the assignable counter feature,
> enable the mbm_total_bytes and mbm_local_bytes events that users are
> familiar with as part of original L3 MBM support." Although, when it comes to
> this patch this may not be appropriate in that this is something that
> resctrl fs should do, not the architecture.
Sure. Added the above line. Removed the "The current expectation" line.
>
>
>>
>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537
>> Signed-off-by: Babu Moger <babu.moger@....com>
>> ---
>
> ...
>
>> ---
>> arch/x86/kernel/cpu/resctrl/core.c | 4 ++--
>> arch/x86/kernel/cpu/resctrl/monitor.c | 11 ++++++++---
>> include/linux/resctrl.h | 4 ++++
>> 3 files changed, 14 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
>> index 22a414802cbb..01b210febc7d 100644
>> --- a/arch/x86/kernel/cpu/resctrl/core.c
>> +++ b/arch/x86/kernel/cpu/resctrl/core.c
>> @@ -873,11 +873,11 @@ static __init bool get_rdt_mon_resources(void)
>> resctrl_enable_mon_event(QOS_L3_OCCUP_EVENT_ID);
>> ret = true;
>> }
>> - if (rdt_cpu_has(X86_FEATURE_CQM_MBM_TOTAL)) {
>> + if (rdt_cpu_has(X86_FEATURE_CQM_MBM_TOTAL) || rdt_cpu_has(X86_FEATURE_ABMC)) {
>> resctrl_enable_mon_event(QOS_L3_MBM_TOTAL_EVENT_ID);
>> ret = true;
>> }
>> - if (rdt_cpu_has(X86_FEATURE_CQM_MBM_LOCAL)) {
>> + if (rdt_cpu_has(X86_FEATURE_CQM_MBM_LOCAL) || rdt_cpu_has(X86_FEATURE_ABMC)) {
>> resctrl_enable_mon_event(QOS_L3_MBM_LOCAL_EVENT_ID);
>> ret = true;
>
> This backward compatibility needs to be managed by resctrl fs, no? What do you think of
> instead doing:
Looks good to me.
diff --git a/fs/resctrl/monitor.c b/fs/resctrl/monitor.c
index dcc6c00eb362..7e816341da6a 100644
--- a/fs/resctrl/monitor.c
+++ b/fs/resctrl/monitor.c
@@ -924,6 +924,11 @@ int resctrl_mon_resource_init(void)
else if (resctrl_is_mon_event_enabled(QOS_L3_MBM_TOTAL_EVENT_ID))
mba_mbps_default_event = QOS_L3_MBM_TOTAL_EVENT_ID;
+ if (r->mon.mbm_cntr_assignable) {
+ resctrl_enable_mon_event(QOS_L3_MBM_TOTAL_EVENT_ID);
+ resctrl_enable_mon_event(QOS_L3_MBM_LOCAL_EVENT_ID);
+ }
+
return 0;
}
>
> int resctrl_mon_resource_init(void) {
>
> ...
> if (r->mon.mbm_cntr_assignable) {
> resctrl_enable_mon_event(QOS_L3_MBM_TOTAL_EVENT_ID);
> resctrl_enable_mon_event(QOS_L3_MBM_LOCAL_EVENT_ID);
> }
> ...
> }
>
> There is another dependency that does not seem to be handled ... ABMC requires
> properties enumerated in resctrl_cpu_detect(), but that enumeration is only
> done if legacy monitoring features are supported, not ABMC. Does AMD support
> enumeration CPUID.(EAX=0xF, ECX=1) if ABMC is supported but not the legacy MBM
> total and local?
Yes. The CPUID.(EAX=0xF, ECX=1) is kind of building block. I would think
it will always be supported.
Added this check now.
diff --git a/arch/x86/kernel/cpu/resctrl/core.c
b/arch/x86/kernel/cpu/resctrl/core.c
index 22a414802cbb..e9a8c4778d22 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
@@ -988,7 +988,8 @@ void resctrl_cpu_detect(struct cpuinfo_x86 *c)
if (cpu_has(c, X86_FEATURE_CQM_OCCUP_LLC) ||
cpu_has(c, X86_FEATURE_CQM_MBM_TOTAL) ||
- cpu_has(c, X86_FEATURE_CQM_MBM_LOCAL)) {
+ cpu_has(c, X86_FEATURE_CQM_MBM_LOCAL) ||
+ cpu_has(c, X86_FEATURE_ABMC)) {
u32 eax, ebx, ecx, edx;
/* QoS sub-leaf, EAX=0Fh, ECX=1 */
--
Thanks
Babu Moger
Powered by blists - more mailing lists