linux-kernel - Re: [PATCH v6 07/22] x86/resctrl: Introduce the interface to display monitor mode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c82e67e6-b2ff-40a8-9992-f3f68c397f53@amd.com>
Date: Mon, 19 Aug 2024 14:27:57 -0500
From: "Moger, Babu" <babu.moger@....com>
To: Reinette Chatre <reinette.chatre@...el.com>, corbet@....net,
 fenghua.yu@...el.com, tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
 dave.hansen@...ux.intel.com
Cc: x86@...nel.org, hpa@...or.com, paulmck@...nel.org, rdunlap@...radead.org,
 tj@...nel.org, peterz@...radead.org, yanjiewtw@...il.com,
 kim.phillips@....com, lukas.bulwahn@...il.com, seanjc@...gle.com,
 jmattson@...gle.com, leitao@...ian.org, jpoimboe@...nel.org,
 rick.p.edgecombe@...el.com, kirill.shutemov@...ux.intel.com,
 jithu.joseph@...el.com, kai.huang@...el.com, kan.liang@...ux.intel.com,
 daniel.sneddon@...ux.intel.com, pbonzini@...hat.com, sandipan.das@....com,
 ilpo.jarvinen@...ux.intel.com, peternewman@...gle.com,
 maciej.wieczor-retman@...el.com, linux-doc@...r.kernel.org,
 linux-kernel@...r.kernel.org, eranian@...gle.com, james.morse@....com
Subject: Re: [PATCH v6 07/22] x86/resctrl: Introduce the interface to display
 monitor mode

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