[PATCH v7 13/24] x86/resctrl: Add data structures and definitions for ABMC assignment

Babu Moger posted 24 patches 1 year, 3 months ago
There is a newer version of this series
[PATCH v7 13/24] x86/resctrl: Add data structures and definitions for ABMC assignment
Posted by Babu Moger 1 year, 3 months ago
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 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
==========================================================================

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>
---
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 | 30 ++++++++++++++++++++++++++
 2 files changed, 31 insertions(+)

diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index d86469bf5d41..dd988a082fa8 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -1183,6 +1183,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
 
 /* 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 6107101f2d8a..27617fe592ed 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -602,6 +602,36 @@ 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.
+ *
+ * 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.
+ */
+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)
-- 
2.34.1

Re: [PATCH v7 13/24] x86/resctrl: Add data structures and definitions for ABMC assignment
Posted by Reinette Chatre 1 year, 3 months ago
Hi Babu,

On 9/4/24 3:21 PM, Babu Moger wrote:
> +/*
> + * 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.
> + *
> + * 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.

Thanks for moving the text ... could it now be made to match the new (outside
AMD arch document) destination? For example, "CfgEn" becomes "@cfg_en",
"CtrID" becomes "@cntr_id" etc. Also please fix language, for example
what does "and but no tracking the events yet" mean? So far this work
has focused on "counting" vs "not counting" events and it is not
clear how this "tracking" fits it ... this seems to be the hardware
view that means "tracking the RMID to which @cntr_id is assigned"?
Please help readers to understand how the implementation is supported
by the hardware.

Reinette
Re: [PATCH v7 13/24] x86/resctrl: Add data structures and definitions for ABMC assignment
Posted by Moger, Babu 1 year, 2 months ago
Hi Reinette,

On 9/19/24 12:08, Reinette Chatre wrote:
> Hi Babu,
> 
> On 9/4/24 3:21 PM, Babu Moger wrote:
>> +/*
>> + * 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.
>> + *
>> + * 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.
> 
> Thanks for moving the text ... could it now be made to match the new (outside
> AMD arch document) destination? For example, "CfgEn" becomes "@cfg_en",

Sure. Will do.

> "CtrID" becomes "@cntr_id" etc. Also please fix language, for example
> what does "and but no tracking the events yet" mean? So far this work
> has focused on "counting" vs "not counting" events and it is not

I will change the text to "not counting".  Hope this will clarify here.

> clear how this "tracking" fits it ... this seems to be the hardware
> view that means "tracking the RMID to which @cntr_id is assigned"?
> Please help readers to understand how the implementation is supported
> by the hardware.

I have checked with hw team on this.
CfgEn: This corresponds counter assignment.
CtrEn: This is to start or stop counting.
       We always set this to 1 to start counting.
-- 
Thanks
Babu Moger
Re: [PATCH v7 13/24] x86/resctrl: Add data structures and definitions for ABMC assignment
Posted by Reinette Chatre 1 year, 2 months ago
Hi Babu,

On 9/23/24 1:21 PM, Moger, Babu wrote:
> Hi Reinette,
> 
> On 9/19/24 12:08, Reinette Chatre wrote:
>> Hi Babu,
>>
>> On 9/4/24 3:21 PM, Babu Moger wrote:
>>> +/*
>>> + * 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.
>>> + *
>>> + * 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.
>>
>> Thanks for moving the text ... could it now be made to match the new (outside
>> AMD arch document) destination? For example, "CfgEn" becomes "@cfg_en",
> 
> Sure. Will do.
> 
>> "CtrID" becomes "@cntr_id" etc. Also please fix language, for example
>> what does "and but no tracking the events yet" mean? So far this work
>> has focused on "counting" vs "not counting" events and it is not
> 
> I will change the text to "not counting".  Hope this will clarify here.
> 
>> clear how this "tracking" fits it ... this seems to be the hardware
>> view that means "tracking the RMID to which @cntr_id is assigned"?
>> Please help readers to understand how the implementation is supported
>> by the hardware.
> 
> I have checked with hw team on this.
> CfgEn: This corresponds counter assignment.

To be specific this corresponds to *hardware* counter assignment? This is
because software sets CfgEn to 1 whether it is assigned from kernel perspective
or not.

Actually ... when I look at the AMD spec it becomes more clear to me. If I
understand the spec correctly the CfgEn bit is used to coordinate changes
between OS and HW. Seems like OS can leisurely write to any fields of
L3_QOS_ABMC_CFG, but only when CfgEn bit is set will the actual hardware
configuration be performed.

> CtrEn: This is to start or stop counting.
>        We always set this to 1 to start counting.

Understood. Now that I read this portion of AMD spec it is more clear to me
and I understand why CfgEn is set in both counter assign and unassign.

Reinette
Re: [PATCH v7 13/24] x86/resctrl: Add data structures and definitions for ABMC assignment
Posted by Moger, Babu 1 year, 2 months ago
Hi Reinette,

On 9/23/24 17:30, Reinette Chatre wrote:
> Hi Babu,
> 
> On 9/23/24 1:21 PM, Moger, Babu wrote:
>> Hi Reinette,
>>
>> On 9/19/24 12:08, Reinette Chatre wrote:
>>> Hi Babu,
>>>
>>> On 9/4/24 3:21 PM, Babu Moger wrote:
>>>> +/*
>>>> + * 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.
>>>> + *
>>>> + * 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.
>>>
>>> Thanks for moving the text ... could it now be made to match the new (outside
>>> AMD arch document) destination? For example, "CfgEn" becomes "@cfg_en",
>>
>> Sure. Will do.
>>
>>> "CtrID" becomes "@cntr_id" etc. Also please fix language, for example
>>> what does "and but no tracking the events yet" mean? So far this work
>>> has focused on "counting" vs "not counting" events and it is not
>>
>> I will change the text to "not counting".  Hope this will clarify here.
>>
>>> clear how this "tracking" fits it ... this seems to be the hardware
>>> view that means "tracking the RMID to which @cntr_id is assigned"?
>>> Please help readers to understand how the implementation is supported
>>> by the hardware.
>>
>> I have checked with hw team on this.
>> CfgEn: This corresponds counter assignment.
> 
> To be specific this corresponds to *hardware* counter assignment? This is
> because software sets CfgEn to 1 whether it is assigned from kernel perspective
> or not.

Yes. We are setting CfgEn = 1 in both assign/unassign.

In case of unassign, we want the counter to stop counting so that software
does not get confused. Otherwise it is really not required.


> 
> Actually ... when I look at the AMD spec it becomes more clear to me. If I
> understand the spec correctly the CfgEn bit is used to coordinate changes
> between OS and HW. Seems like OS can leisurely write to any fields of
> L3_QOS_ABMC_CFG, but only when CfgEn bit is set will the actual hardware
> configuration be performed.
> 
>> CtrEn: This is to start or stop counting.
>>        We always set this to 1 to start counting.
> 
> Understood. Now that I read this portion of AMD spec it is more clear to me
> and I understand why CfgEn is set in both counter assign and unassign.
> 
> Reinette
> 

-- 
Thanks
Babu Moger