| 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: <c81c9b9d-9897-4f97-9d41-37a9fbfcfd7e@amd.com>
Date: Thu, 17 Oct 2024 13:52:40 -0500
From: "Moger, Babu" <babu.moger@....com>
To: Reinette Chatre <reinette.chatre@...el.com>, corbet@....net,
fenghua.yu@...el.com, tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
dave.hansen@...ux.intel.com
Cc: x86@...nel.org, hpa@...or.com, paulmck@...nel.org, rdunlap@...radead.org,
tj@...nel.org, peterz@...radead.org, yanjiewtw@...il.com,
kim.phillips@....com, lukas.bulwahn@...il.com, seanjc@...gle.com,
jmattson@...gle.com, leitao@...ian.org, jpoimboe@...nel.org,
kirill.shutemov@...ux.intel.com, jithu.joseph@...el.com,
kai.huang@...el.com, kan.liang@...ux.intel.com,
daniel.sneddon@...ux.intel.com, pbonzini@...hat.com, sandipan.das@....com,
ilpo.jarvinen@...ux.intel.com, peternewman@...gle.com,
maciej.wieczor-retman@...el.com, linux-doc@...r.kernel.org,
linux-kernel@...r.kernel.org, eranian@...gle.com, james.morse@....com
Subject: Re: [PATCH v8 14/25] x86/resctrl: Add data structures and definitions
for ABMC assignment
Hi Reinette,
On 10/15/24 22:21, Reinette Chatre wrote:
> Hi Babu,
>
> On 10/9/24 10:39 AM, Babu Moger wrote:
>> The ABMC feature provides an option to the user to assign a hardware
>> counter to an RMID, event pair and monitor the bandwidth as long as the
>> counter is assigned. The bandwidth events will be tracked by the hardware
>> until the user changes the configuration. Each resctrl group can configure
>> maximum two counters, one for total event and one for local event.
>>
>> The ABMC feature implements an MSR L3_QOS_ABMC_CFG (C000_03FDh).
>> Configuration is done by setting the counter id, bandwidth source (RMID)
>> and bandwidth configuration supported by BMEC (Bandwidth Monitoring Event
>> Configuration).
>>
>> Attempts to read or write the MSR when ABMC is not enabled will result
>> in a #GP(0) exception.
>>
>> Introduce the data structures and definitions for MSR L3_QOS_ABMC_CFG
>> (0xC000_03FDh):
>> =========================================================================
>> Bits Mnemonic Description Access Reset
>> Type Value
>> =========================================================================
>> 63 CfgEn Configuration Enable R/W 0
>>
>> 62 CtrEn Enable/disable counting R/W 0
>>
>> 61:53 – Reserved MBZ 0
>>
>> 52:48 CtrID Counter Identifier R/W 0
>>
>> 47 IsCOS BwSrc field is a CLOSID R/W 0
>> (not an RMID)
>>
>> 46:44 – Reserved MBZ 0
>>
>> 43:32 BwSrc Bandwidth Source R/W 0
>> (RMID or CLOSID)
>>
>> 31:0 BwType Bandwidth configuration R/W 0
>> to track for this counter
>> ==========================================================================
>>
>> The feature details are documented in the 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).
>>
>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537
>> Signed-off-by: Babu Moger <babu.moger@....com>
>> ---
>
> ...
>
>> ---
>> arch/x86/include/asm/msr-index.h | 1 +
>> arch/x86/kernel/cpu/resctrl/internal.h | 33 ++++++++++++++++++++++++++
>> 2 files changed, 34 insertions(+)
>>
>> diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
>> index 43c9dc473aba..2c281c977342 100644
>> --- a/arch/x86/include/asm/msr-index.h
>> +++ b/arch/x86/include/asm/msr-index.h
>> @@ -1196,6 +1196,7 @@
>> #define MSR_IA32_SMBA_BW_BASE 0xc0000280
>> #define MSR_IA32_EVT_CFG_BASE 0xc0000400
>> #define MSR_IA32_L3_QOS_EXT_CFG 0xc00003ff
>> +#define MSR_IA32_L3_QOS_ABMC_CFG 0xc00003fd
>>
>
> As Tony mentioned, also please correct order of this MSR.
Sure.
>
>> /* AMD-V MSRs */
>> #define MSR_VM_CR 0xc0010114
>> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
>> index 86e3e188c119..de397468b945 100644
>> --- a/arch/x86/kernel/cpu/resctrl/internal.h
>> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
>> @@ -602,6 +602,39 @@ union cpuid_0x10_x_edx {
>> unsigned int full;
>> };
>>
>> +/*
>> + * ABMC counters can be configured by writing to L3_QOS_ABMC_CFG.
>
> "ABMC counters are configured by writing to L3_QOS_ABMC_CFG."
Sure.
>
>> + * Reading L3_QOS_ABMC_DSC returns the configuration of the counter id
>> + * specified in L3_QOS_ABMC_CFG.cntr_id.
>
> First and only mention/use of L3_QOS_ABMC_DSC in this series. If this register
> is not used then references to it can be removed.
Sure.
>
>> + * @bw_type : Bandwidth configuration(supported by BMEC)
>
> "configuration(supported" -> "configuration (supported"
Sure.
>
>> + * tracked by the @cntr_id.
>> + * @bw_src : Bandwidth source (RMID or CLOSID).
>> + * @reserved1 : Reserved.
>> + * @is_clos : @bw_src field is a CLOSID (not an RMID).
>> + * @cntr_id : Counter identifier.
>> + * @reserved : Reserved.
>> + * @cntr_en : Counting enable bit.
>> + * @cfg_en : Configuration enable bit.
>> + *
>> + * Configuration and counting:
>> + * cfg_en=0, : No configuration changes applied.
>
> Can this be expanded? (sidenote: It is taking a long time to get clarity on how
> to interact with hardware. These incremental cryptic fragments make it difficult
> to know how to interact with the hardware.)
>
> For example, "No configuration changes applied. Counter can be configured across
> multiple writes to MSR while @cfg_en=0. Configuration applied when @cfg_en=1."
>
>> + * cfg_en=1, cntr_en=0 : Configure cntr_id and but no counting the events.
>
> hmmm ... still the same (""but no counting the events") strange language I
> highlighted in V7 ...
>
> I think it will make things easier to understand if similar language is used
> between the descriptions of the different fields.
>
> "Apply @cntr_id configuration but do not count events."
>
>> + * cfg_en=1, cntr_en=1 : Configure cntr_id and start counting the events.
>
> "Apply @cntr_id configuration and start counting events."
>
> Can it be added here which of these settings (or combination of settings) result
> in counters being reset?
Any change in the configuration will reset the counters.
Little bit lost here. Let me summarize. How about this?
Configuration and counting:
Counter can be configured across multiple writes to MSR. Configuration
is applied only when @cfg_en = 1. The event counters will reset when any
of the configuration is changed.
cfg_en = 1, cntr_en = 0 : Apply @cntr_id configuration but do not count
events.
cfg_en = 1, cntr_en = 1 : Apply @cntr_id configuration and start counting
events.
>
>> + */
>> +union l3_qos_abmc_cfg {
>> + struct {
>> + unsigned long bw_type :32,
>> + bw_src :12,
>> + reserved1: 3,
>> + is_clos : 1,
>> + cntr_id : 5,
>> + reserved : 9,
>> + cntr_en : 1,
>> + cfg_en : 1;
>> + } split;
>> + unsigned long full;
>> +};
>> +
>> void rdt_last_cmd_clear(void);
>> void rdt_last_cmd_puts(const char *s);
>> __printf(1, 2)
>
> Reinette
>
--
Thanks
Babu Moger
Powered by blists - more mailing lists