[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <49ee6ebe-96a3-4e3b-bb60-090be2e9e17b@amd.com>
Date: Tue, 20 Aug 2024 15:56:02 -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,
rick.p.edgecombe@...el.com, 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 v6 13/22] x86/resctrl: Add data structures and definitions
for ABMC assignment
Hi Reinette,
On 8/16/24 16:38, Reinette Chatre wrote:
> Hi Babu,
>
> This patch now only introduces one data structure so the subject could
> be made more specific.
How about?
x86/resctrl: Add data structures for L3_QOS_ABMC_CFG MSR
>
> On 8/6/24 3:00 PM, Babu Moger wrote:
>> The ABMC feature provides an option to the user to assign a hardware
>> counter to an RMID 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.
>>
>>
>
> (extra empty line)
Sure.
>
>> The ABMC feature implements a pair of MSRs, L3_QOS_ABMC_CFG (C000_03FDh)
>> and L3_QOS_ABMC_DSC (C000_3FEh). The counters are configured by writing
>> to MSR L3_QOS_ABMC_CFG. Configuration is done by setting the counter id,
>> bandwidth source (RMID) and bandwidth configuration supported by BMEC
>> (Bandwidth Monitoring Event Configuration).
>>
>> L3_QOS_ABMC_DSC is a read-only MSR. Reading L3_QOS_ABMC_DSC returns the
>> configuration of the counter id specified in L3_QOS_ABMC_CFG.cntr_id
>> with rmid(bw_src) and event configuration(bw_type).
>>
>> Attempts to read or write these MSRs when ABMC is not enabled will result
>> in a #GP(0) exception.
>>
>> Introduce data structures and definitions for ABMC MSRs.
>>
>> MSR L3_QOS_ABMC_CFG (0xC000_03FDh) and L3_QOS_ABMC_DSC (0xC000_03FEh)
>> details.
>
> The changelog and patch introduce L3_QOS_ABMC_DSC but I cannot see that it is
> used in this series.
Yes. I was using it in v5 to read the configuration back. It is not
required anymore. I will remove it.
>
>> =========================================================================
>> Bits Mnemonic Description Access Reset
>> Type Value
>> =========================================================================
>> 63 CfgEn Configuration Enable R/W 0
>>
>> 62 CtrEn Enable/disable Tracking 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
>> ==========================================================================
>>
>> Configuration and tracking:
>> CfgEn=1,CtrEn=0 : Configure CtrID and but no tracking the events yet.
>> CfgEn=1,CtrEn=1 : Configure CtrID and start tracking events.
>
> Could you please add the above snippet noting field combinations to the
> kernel-doc of the union?
Sure.
>
>>
>> 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>
>> ---
>> v6: Removed all the fs related changes.
>> Added note on CfgEn,CtrEn.
>> Removed the definitions which are not used.
>> Removed cntr_id initialization.
>>
>> v5: Moved assignment flags here (path 10/19 of v4).
>> Added MON_CNTR_UNSET definition to initialize cntr_id's.
>> More details in commit log.
>> Renamed few fields in l3_qos_abmc_cfg for readability.
>>
>> v4: Added more descriptions.
>> Changed the name abmc_ctr_id to ctr_id.
>> Added L3_QOS_ABMC_DSC. Used for reading the configuration.
>>
>> v3: No changes.
>>
>> v2: No changes.
>> ---
>> arch/x86/include/asm/msr-index.h | 2 ++
>> arch/x86/kernel/cpu/resctrl/internal.h | 26 ++++++++++++++++++++++++++
>> 2 files changed, 28 insertions(+)
>>
>> diff --git a/arch/x86/include/asm/msr-index.h
>> b/arch/x86/include/asm/msr-index.h
>> index d86469bf5d41..5b3931a59d5a 100644
>> --- a/arch/x86/include/asm/msr-index.h
>> +++ b/arch/x86/include/asm/msr-index.h
>> @@ -1183,6 +1183,8 @@
>> #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
>> +#define MSR_IA32_L3_QOS_ABMC_DSC 0xc00003fe
>> /* MSR_IA32_VMX_MISC bits */
>> #define MSR_IA32_VMX_MISC_INTEL_PT (1ULL << 14)
>> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h
>> b/arch/x86/kernel/cpu/resctrl/internal.h
>> index 1021227d8c7e..af3efa35a62e 100644
>> --- a/arch/x86/kernel/cpu/resctrl/internal.h
>> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
>> @@ -589,6 +589,32 @@ union cpuid_0x10_x_edx {
>> unsigned int full;
>> };
>> +/*
>> + * ABMC counters can be configured by writing to L3_QOS_ABMC_CFG.
>> + * @bw_type : Bandwidth configuration(supported by BMEC)
>> + * 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 : Tracking enable bit.
>> + * @cfg_en : Configuration enable bit.
>> + */
>> +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;
>> +};
>> +
>
> This data structure still uses tabs that seem to have goal of aligning
> members
> but the tabs are used inconsistently and members are not lining up either.
Sorry. I always have issues with these tabs. Will address it next revision.
>
>> 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