Re: [PATCH v12 08/26] x86/resctrl: Introduce the interface to display monitor mode

Moger, Babu
2025-04-14T19:52:32Z

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