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: <be1f6d4e-6310-4345-a1b5-090fde96b7fe@amd.com>
Date: Thu, 29 May 2025 10:21:16 -0500
From: "Moger, Babu" <babu.moger@....com>
To: Reinette Chatre <reinette.chatre@...el.com>, corbet@....net,
 tony.luck@...el.com, tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
 dave.hansen@...ux.intel.com
Cc: james.morse@....com, dave.martin@....com, fenghuay@...dia.com,
 x86@...nel.org, hpa@...or.com, paulmck@...nel.org,
 akpm@...ux-foundation.org, thuth@...hat.com, rostedt@...dmis.org,
 ardb@...nel.org, gregkh@...uxfoundation.org, daniel.sneddon@...ux.intel.com,
 jpoimboe@...nel.org, alexandre.chartre@...cle.com,
 pawan.kumar.gupta@...ux.intel.com, thomas.lendacky@....com,
 perry.yuan@....com, seanjc@...gle.com, kai.huang@...el.com,
 xiaoyao.li@...el.com, kan.liang@...ux.intel.com, xin3.li@...el.com,
 ebiggers@...gle.com, xin@...or.com, sohil.mehta@...el.com,
 andrew.cooper3@...rix.com, mario.limonciello@....com,
 linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
 peternewman@...gle.com, maciej.wieczor-retman@...el.com, eranian@...gle.com,
 Xiaojian.Du@....com, gautham.shenoy@....com
Subject: Re: [PATCH v13 12/27] x86/resctrl: Introduce event configuration
 modes

Hi Reinette,

On 5/22/25 17:05, Reinette Chatre wrote:
> Hi Babu,
> 
> On 5/15/25 3:51 PM, Babu Moger wrote:
>> MBM events can be configured using either BMEC (Bandwidth Monitoring Event
>> Configuration) or the mbm_cntr_assign mode.
>>
>> Introduce a data structure to represent the various event configuration
>> modes and their corresponding values.
>>
>> Suggested-by: Reinette Chatre <reinette.chatre@...el.com>
> 
> I cannot recall suggesting this.
> 
> (/me digs)
> 
> Are you perhaps referring to https://lore.kernel.org/lkml/d2966a26-4483-4808-a538-bb20973dd2a1@intel.com/

Yes.
> 
> This is not referring to new modes but the existing mbm_cntr_assign modes.
> resctrl knows which "mbm_cntr_assign" mode is active and it can use that
> to determine whether BMEC can be exposed to user space or not. There is
> already enough information in resctrl to know whether BMEC files should be
> exposed or not.
> 
> I think this work self makes clear that these modes are useless since
> patch #25 that determines whether to hide BMEC files doesn't even
> use it.

Sure. I will remove change related to mbm_mode.

> 
> 
>> Signed-off-by: Babu Moger <babu.moger@....com>
>> ---
>> v13: New patch to handle different event configuration types with
>>      mbm_cntr_assign mode.
>> ---
>>  fs/resctrl/internal.h         |  6 ++++--
>>  fs/resctrl/monitor.c          |  4 ++--
>>  fs/resctrl/rdtgroup.c         |  2 +-
>>  include/linux/resctrl_types.h | 11 +++++++++++
>>  4 files changed, 18 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/resctrl/internal.h b/fs/resctrl/internal.h
>> index 9a8cf6f11151..0fae374559ba 100644
>> --- a/fs/resctrl/internal.h
>> +++ b/fs/resctrl/internal.h
>> @@ -55,13 +55,15 @@ static inline struct rdt_fs_context *rdt_fc2context(struct fs_context *fc)
>>   * struct mon_evt - Entry in the event list of a resource
>>   * @evtid:		event id
>>   * @name:		name of the event
>> - * @configurable:	true if the event is configurable
>> + * @mbm_mode:		monitoring mode (BMEC or mbm_cntr_assign)
>> + * @evt_cfg:		event configuration value decoding reads, writes.
>>   * @list:		entry in &rdt_resource->evt_list
>>   */
>>  struct mon_evt {
>>  	enum resctrl_event_id	evtid;
>>  	char			*name;
>> -	bool			configurable;
>> +	enum resctrl_mbm_mode	mbm_mode;
>> +	u32			evt_cfg;
> 
> This very important yet totally unrelated member sneaked in without
> any mention.

Yes. We only need evt_cfg. I will just add it as separate patch.

> 
>>  	struct list_head	list;
>>  };
>>  
>> diff --git a/fs/resctrl/monitor.c b/fs/resctrl/monitor.c
>> index 2548aee0151c..8e403587a02f 100644
>> --- a/fs/resctrl/monitor.c
>> +++ b/fs/resctrl/monitor.c
>> @@ -903,12 +903,12 @@ int resctrl_mon_resource_init(void)
>>  	l3_mon_evt_init(r);
>>  
>>  	if (resctrl_arch_is_evt_configurable(QOS_L3_MBM_TOTAL_EVENT_ID)) {
>> -		mbm_total_event.configurable = true;
>> +		mbm_total_event.mbm_mode = MBM_MODE_BMEC;
>>  		resctrl_file_fflags_init("mbm_total_bytes_config",
>>  					 RFTYPE_MON_INFO | RFTYPE_RES_CACHE);
>>  	}
>>  	if (resctrl_arch_is_evt_configurable(QOS_L3_MBM_LOCAL_EVENT_ID)) {
>> -		mbm_local_event.configurable = true;
>> +		mbm_local_event.mbm_mode = MBM_MODE_BMEC;
>>  		resctrl_file_fflags_init("mbm_local_bytes_config",
>>  					 RFTYPE_MON_INFO | RFTYPE_RES_CACHE);
>>  	}
>> diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
>> index 752750e3e443..f192b2736a77 100644
>> --- a/fs/resctrl/rdtgroup.c
>> +++ b/fs/resctrl/rdtgroup.c
>> @@ -1152,7 +1152,7 @@ static int rdt_mon_features_show(struct kernfs_open_file *of,
>>  
>>  	list_for_each_entry(mevt, &r->mon.evt_list, list) {
>>  		seq_printf(seq, "%s\n", mevt->name);
>> -		if (mevt->configurable)
>> +		if (mevt->mbm_mode == MBM_MODE_BMEC)
> 
> This can instead be a call to a utility that returns whether BMEC should be
> visible based on resctrl_mon::mbm_cntr_assignable and rdt_hw_resource::mbm_cntr_assign_enabled
> (via resctrl_arch_mbm_cntr_assign_enabled() of course).

Sure. Will do.

> 
>>  			seq_printf(seq, "%s_config\n", mevt->name);
>>  	}
>>  
>> diff --git a/include/linux/resctrl_types.h b/include/linux/resctrl_types.h
>> index a25fb9c4070d..26cd1fec72db 100644
>> --- a/include/linux/resctrl_types.h
>> +++ b/include/linux/resctrl_types.h
>> @@ -47,4 +47,15 @@ enum resctrl_event_id {
>>  	QOS_NUM_EVENTS,
>>  };
>>  
>> +/*
>> + * Event configuration mode.
>> + * Events can be configured either in BMEC (Bandwidth Monitoring Event
>> + * Configuration) mode or mbm_cntr_assign mode.
>> + */
>> +enum resctrl_mbm_mode {
>> +	MBM_MODE_NONE,
>> +	MBM_MODE_BMEC,
>> +	MBM_MODE_ASSIGN,
>> +};
>> +
>>  #endif /* __LINUX_RESCTRL_TYPES_H */
> 
> Reinette
> 

-- 
Thanks
Babu Moger

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ