Re: [PATCH v13 10/27] x86/resctrl: Add data structures and definitions for ABMC assignment

["Moger, Babu" <bmoger@amd.com>]

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@amd.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