[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2bbf9401-3a3c-448a-bcb9-9bd6317b1950@amd.com>
Date: Wed, 28 May 2025 14:15:35 -0500
From: "Moger, Babu" <bmoger@....com>
To: Reinette Chatre <reinette.chatre@...el.com>,
Babu Moger <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 10/27] x86/resctrl: Add data structures and
definitions for ABMC assignment
Hi Reinette,
On 5/22/2025 4:10 PM, Reinette Chatre wrote:
> Hi Babu,
>
> On 5/15/25 3:51 PM, 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.
>
> (please update, above describes previous design)
>
Ok. Will drop the last line.
>>
>> The ABMC feature implements an MSR L3_QOS_ABMC_CFG (C000_03FDh).
>> ABMC counter assignment is done by setting the counter id, bandwidth
>> source (RMID) and bandwidth configuration. Users will have the option to
>> change the bandwidth configuration using resctrl interface which will be
>> introduced later in the series.
>
> "will be introduced later in the series" is similar to "in a subsequent patch"
> and should not be used in a changelog. Just describe what this patch does.
ok. Last line is really not required.
>
>>
>> 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>
>> ---
>> v13: Removed the Reviewed-by tag as there is commit log change to remove
>> BMEC reference.
>>
>> v12: No changes.
>>
>> v11: No changes.
>>
>> v10: No changes.
>>
>> v9: Removed the references of L3_QOS_ABMC_DSC.
>> Text changes about configuration in kernel doc.
>>
>> v8: Update the configuration notes in kernel_doc.
>> Few commit message update.
>>
>> v7: Removed the reference of L3_QOS_ABMC_DSC as it is not used anymore.
>> Moved the configuration notes to kernel_doc.
>> Adjusted the tabs for l3_qos_abmc_cfg and checkpatch seems happy.
>>
>> 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 | 1 +
>> arch/x86/kernel/cpu/resctrl/internal.h | 35 ++++++++++++++++++++++++++
>> 2 files changed, 36 insertions(+)
>>
>> diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
>> index 3970e0b16e47..b5b5ebead24f 100644
>> --- a/arch/x86/include/asm/msr-index.h
>> +++ b/arch/x86/include/asm/msr-index.h
>> @@ -1203,6 +1203,7 @@
>> /* - AMD: */
>> #define MSR_IA32_MBA_BW_BASE 0xc0000200
>> #define MSR_IA32_SMBA_BW_BASE 0xc0000280
>> +#define MSR_IA32_L3_QOS_ABMC_CFG 0xc00003fd
>> #define MSR_IA32_L3_QOS_EXT_CFG 0xc00003ff
>> #define MSR_IA32_EVT_CFG_BASE 0xc0000400
>>
>> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
>> index fcc9d23686a1..db6b0c28ee6b 100644
>> --- a/arch/x86/kernel/cpu/resctrl/internal.h
>> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
>> @@ -164,6 +164,41 @@ union cpuid_0x10_x_edx {
>> unsigned int full;
>> };
>>
>> +/*
>> + * ABMC counters are configured by writing to L3_QOS_ABMC_CFG.
>> + * @bw_type : Bandwidth configuration (supported by BMEC)
>> + * tracked by the @cntr_id.
>
> The "supported by BMEC" is unexpected with the new design that separated
> the two features.
My bad. Will remove it.
>> + * @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:
>> + * Counter can be configured across multiple writes to MSR. Configuration
>> + * is applied only when @cfg_en = 1. Counter @cntr_id is reset when the
>> + * configuration is applied.
>> + * @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_ctrl_update(void *arg);
>>
>> int rdt_get_mon_l3_config(struct rdt_resource *r);
>
> Reinette
>
Thanks
Babu
Powered by blists - more mailing lists