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: <5e2fb414-3593-4d1d-b47c-a7dafb454e0c@intel.com>
Date: Fri, 16 Aug 2024 14:32:58 -0700
From: Reinette Chatre <reinette.chatre@...el.com>
To: Babu Moger <babu.moger@....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 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.


....

> 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.

> +			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


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ