Hi Reinette, On 4/11/25 15:56, Reinette Chatre wrote: > Hi Babu, > > On 4/3/25 5:18 PM, Babu Moger wrote: >> Introduce the interface file "mbm_assign_mode" to list monitor modes > > Using "resctrl file" instead of "interface file" should help to make it > clear what this patch does. ok. Sure. > >> supported. >> >> The "mbm_cntr_assign" mode provides the option to assign a counter to >> an RMID, event pair and monitor the bandwidth as long as it is assigned. >> >> On AMD systems "mbm_cntr_assign" mode is backed by the ABMC (Assignable >> Bandwidth Monitoring Counters) hardware feature and is enabled by default. >> >> The "default" mode is the existing monitoring mode that works without the >> explicit counter assignment, instead relying on dynamic counter assignment >> by hardware that may result in hardware not dedicating a counter resulting >> in monitoring data reads returning "Unavailable". >> >> Provide an interface to display the monitor mode on the system. >> $ cat /sys/fs/resctrl/info/L3_MON/mbm_assign_mode >> [mbm_cntr_assign] >> default >> >> Added an IS_ENABLED(CONFIG_RESCTRL_ASSIGN_FIXED) check to handle Arm64 > > (needs imperative) Sure. > >> platforms. On x86, CONFIG_RESCTRL_ASSIGN_FIXED is not defined, whereas on >> Arm64, it is. As a result, for MPAM, the file would be either: > > CONFIG_RESCTRL_ASSIGN_FIXED does not yet exist anywhere so this motivation needs > to provide stronger support for why it is used before it exists. There is a precedent > here with RESCTRL_RMID_DEPENDS_ON_CLOSID already used while it does not yet > appear in a Kconfig file. I would propose that this is motivated by noting > how it is already understood how Arm supports assignable counters this was recommended > by James to prepare for that work. Since this is user interface this > work is done early to ensure user interface is compatible with that upcoming > support. Also set folks at ease that IS_ENABLED() works as expected with a > non-existing config. How about this? Add IS_ENABLED(CONFIG_RESCTRL_ASSIGN_FIXED) check to support Arm64. On x86, CONFIG_RESCTRL_ASSIGN_FIXED is not defined. On Arm64, it will be defined when the "mbm_cntr_assign" mode is supported. Add an IS_ENABLED(CONFIG_RESCTRL_ASSIGN_FIXED) check early to ensure the user interface remains compatible with upcoming Arm64 support. IS_ENABLED() safely evaluates to 0 when the configuration is not defined. As a result, for MPAM, the file would be either: [default] or [mbm_cntr_assign] > > >> [default] >> or >> [mbm_cntr_assign] >> >> Signed-off-by: Babu Moger <babu.moger@xxxxxxx> >> --- >> v12: Minor text update in change log and user documentation. >> Added the check CONFIG_RESCTRL_ASSIGN_FIXED to take care of arm platforms. >> This will be defined only in arm and not in x86. >> >> v11: Renamed rdtgroup_mbm_assign_mode_show() to resctrl_mbm_assign_mode_show(). >> Removed few texts in resctrl.rst about AMD specific information. >> Updated few texts. >> >> v10: Added few more text to user documentation clarify on the default mode. >> >> v9: Updated user documentation based on comments. >> >> v8: Commit message update. >> >> v7: Updated the descriptions/commit log in resctrl.rst to generic text. >> Thanks to James and Reinette. >> Rename mbm_mode to mbm_assign_mode. >> Introduced mutex lock in rdtgroup_mbm_mode_show(). >> >> v6: Added documentation for mbm_cntr_assign and legacy mode. >> Moved mbm_mode fflags initialization to static initialization. >> >> v5: Changed interface name to mbm_mode. >> It will be always available even if ABMC feature is not supported. >> Added description in resctrl.rst about ABMC mode. >> Fixed display abmc and legacy consistantly. >> >> v4: Fixed the checks for legacy and abmc mode. Default it ABMC. >> >> v3: New patch to display ABMC capability. >> >> ???END >> --- >> Documentation/arch/x86/resctrl.rst | 27 +++++++++++++++++++ >> arch/x86/kernel/cpu/resctrl/rdtgroup.c | 37 ++++++++++++++++++++++++++ >> 2 files changed, 64 insertions(+) >> >> diff --git a/Documentation/arch/x86/resctrl.rst b/Documentation/arch/x86/resctrl.rst >> index fb90f08e564e..bb96b44019fe 100644 >> --- a/Documentation/arch/x86/resctrl.rst >> +++ b/Documentation/arch/x86/resctrl.rst >> @@ -257,6 +257,33 @@ with the following files: >> # cat /sys/fs/resctrl/info/L3_MON/mbm_local_bytes_config >> 0=0x30;1=0x30;3=0x15;4=0x15 >> >> +"mbm_assign_mode": >> + Reports the list of monitoring modes supported. The enclosed brackets >> + indicate which mode is enabled. >> + :: >> + >> + # cat /sys/fs/resctrl/info/L3_MON/mbm_assign_mode >> + [mbm_cntr_assign] >> + default >> + >> + "mbm_cntr_assign": >> + >> + In mbm_cntr_assign mode, a monitoring event can only accumulate data >> + while it is backed by a hardware counter. The user-space is able to >> + specify which of the events in CTRL_MON or MON groups should have a >> + counter assigned using the "mbm_assign_control" file. The number of > > "mbm_assign_control" no longer exist. The user-space is able to specify which of the events in CTRL_MON or MON groups should have a counter assigned using the "mbm_L3_assignments" interface file in each resctrl group. > >> + counters available is described in the "num_mbm_cntrs" file. Changing >> + the mode may cause all counters on the resource to reset. >> + >> + "default": >> + >> + In default mode, resctrl assumes there is a hardware counter for each >> + event within every CTRL_MON and MON group. On AMD platforms, it is >> + recommended to use the mbm_cntr_assign mode, if supported, to prevent >> + the hardware from resetting counters between reads. This can result in > > "from resetting counters" -> "from re-allocating counters"? How about? "from resetting MBM events between reads" > >> + misleading values or display "Unavailable" if no counter is assigned >> + to the event. >> + >> "max_threshold_occupancy": >> Read/write file provides the largest value (in >> bytes) at which a previously used LLC_occupancy >> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c >> index 17de38e26f94..626be6becca7 100644 >> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c >> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c >> @@ -882,6 +882,36 @@ static int rdtgroup_rmid_show(struct kernfs_open_file *of, >> return ret; >> } >> >> +static int resctrl_mbm_assign_mode_show(struct kernfs_open_file *of, >> + struct seq_file *s, void *v) >> +{ >> + struct rdt_resource *r = of->kn->parent->priv; >> + bool enabled; >> + >> + mutex_lock(&rdtgroup_mutex); >> + enabled = resctrl_arch_mbm_cntr_assign_enabled(r); >> + >> + if (r->mon.mbm_cntr_assignable) { >> + if (enabled) >> + seq_puts(s, "[mbm_cntr_assign]\n"); >> + else >> + seq_puts(s, "[default]\n"); >> + >> + if (!IS_ENABLED(CONFIG_RESCTRL_ASSIGN_FIXED)) { >> + if (enabled) >> + seq_puts(s, "default\n"); >> + else >> + seq_puts(s, "mbm_cntr_assign\n"); >> + } >> + } else { >> + seq_puts(s, "[default]\n"); >> + } >> + >> + mutex_unlock(&rdtgroup_mutex); >> + >> + return 0; >> +} >> + >> #ifdef CONFIG_PROC_CPU_RESCTRL >> >> /* >> @@ -1908,6 +1938,13 @@ static struct rftype res_common_files[] = { >> .seq_show = mbm_local_bytes_config_show, >> .write = mbm_local_bytes_config_write, >> }, >> + { >> + .name = "mbm_assign_mode", >> + .mode = 0444, >> + .kf_ops = &rdtgroup_kf_single_ops, >> + .seq_show = resctrl_mbm_assign_mode_show, >> + .fflags = RFTYPE_MON_INFO, > > Needs a RFTYPE_RES_CACHE? I am not very sure about this. This flag is added to the files in info/L3. "mbm_assign_mode" goes in info/L3_MON/ The files in L3_MON does not have these flags set (for example mon_features, num_rmids). > >> + }, >> { >> .name = "cpus", >> .mode = 0644, > > Reinette > -- Thanks Babu Moger