Re: [PATCH v13 12/27] x86/resctrl: Introduce event configuration modes

[Reinette Chatre <reinette.chatre@intel.com>]

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@intel.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/

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.


> Signed-off-by: Babu Moger <babu.moger@amd.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.

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

>  			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