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@xxxxxxx> >> --- >> 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
