Hi Reinette,
On 8/16/24 16:32, Reinette Chatre wrote:
> Hi Babu,
>
> (expanding on what James said)
>
> On 8/6/24 3:00 PM, Babu Moger wrote:
>> The mbm_mode displays list of monitor modes supported.
>>
>> The mbm_cntr_assign is one of the currently supported modes. It is also
>> called ABMC (Assignable Bandwidth Monitoring Counters) feature. ABMC
>> feature provides option to assign a hardware counter to an RMID and
>> monitor the bandwidth as long as it is assigned. ABMC mode is enabled
>> by default when supported.
>>
>> Legacy mode works without the assignment option.
>>
>> Provide an interface to display the monitor mode on the system.
>> $cat /sys/fs/resctrl/info/L3_MON/mbm_mode
>> [mbm_cntr_assign]
>> legacy
>>
>> Switching the mbm_mode will reset all the mbm counters of all resctrl
>> groups.
>
> The changelog also needs to be clear to distinguish the resctrl fs
> "mbm_cntr_assign" mode from how it is backed by ABMC on AMD hardware.
>
> for example (please improve):
>
> Introduce "mbm_cntr_assign" mode that provides the option to assign a
> hardware counter to an RMID and monitor the bandwidth as long as it is
> assigned. On AMD systems "mbm_cntr_assign" is backed by the ABMC
> (Assignable
> Bandwidth Monitoring Counters) hardware feature. "mbm_cntr_assign" mode
> is enabled by default when supported.
>
> "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
>
> Switching the mbm_assign_mode will reset all the MBM counters of all
> resctrl
> groups.
Looks good thanks
>
>
> ....
>
>> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> index 6075b1e5bb77..d8f85b20ab8f 100644
>> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> @@ -845,6 +845,26 @@ static int rdtgroup_rmid_show(struct
>> kernfs_open_file *of,
>> return ret;
>> }
>> +static int rdtgroup_mbm_mode_show(struct kernfs_open_file *of,
>> + struct seq_file *s, void *v)
>> +{
>> + struct rdt_resource *r = of->kn->parent->priv;
>> +
>> + if (r->mon.mbm_cntr_assignable) {
>> + if (resctrl_arch_get_abmc_enabled()) {
>
> Since this state can change during runtime this access needs to be protected.
Sure. Will do.
>
>> + seq_puts(s, "[mbm_cntr_assign]\n");
>> + seq_puts(s, "legacy\n");
>> + } else {
>> + seq_puts(s, "mbm_cntr_assign\n");
>> + seq_puts(s, "[legacy]\n");
>> + }
>> + } else {
>> + seq_puts(s, "[legacy]\n");
>> + }
>> +
>> + return 0;
>> +}
>> +
>> #ifdef CONFIG_PROC_CPU_RESCTRL
>> /*
>
> Reinette
>
>
--
Thanks
Babu Moger
