x86/resctrl : Support AMD Assignable Bandwidth Monitoring Counters (ABMC)

If the BMEC (Bandwidth Monitoring Event Configuration) feature is supported, the bandwidth events can be configured to track specific events. The event configuration is domain specific. Event configurations are not stored in resctrl but instead always read from or written to hardware directly when prompted by user space.

Read the event configuration from the hardware during domain initialization and store the configuration value in the rdt_hw_mon_domain structure for later use when the user requests to display it.

Signed-off-by Babu Moger <[email protected]>

---
v12: Fixed the conflicts due to recent merge.
This patch is for BMEC and there is no dependancy on ABMC feature.

Moved it earlier.

v11: Resolved minor conflicts due to code displacement. Actual code didnt
change.

v10: Conflicts due to code displacement. Actual code didnt change.

v9: Added Reviewed-by tag. No other changes.

v8: Renamed resctrl_mbm_evt_config_init() to arch_mbm_evt_config_init()
Minor commit message update.

v7: Fixed initializing INVALID_CONFIG_VALUE to mbm_local_cfg in case of error.

v6: Renamed resctrl_arch_mbm_evt_config -> resctrl_mbm_evt_config_init
Initialized value to INVALID_CONFIG_VALUE if it is not configurable. Minor commit message update.

v5: Exported mon_event_config_index_get.
Renamed arch_domain_mbm_evt_config to resctrl_arch_mbm_evt_config.

v4: Read the configuration information from the hardware to initialize.
Added few commit messages.
Fixed the tab spaces.

v3: Minor changes related to rebase in mbm_config_write_domain.

v2: No changes.

---
 arch/x86/kernel/cpu/resctrl/core.c     |  2 ++
 arch/x86/kernel/cpu/resctrl/internal.h |  9 +++++++++
 arch/x86/kernel/cpu/resctrl/monitor.c  | 26 ++++++++++++++++++++++++++
 arch/x86/kernel/cpu/resctrl/rdtgroup.c |  2 +-
 4 files changed, 38 insertions(+), 1 deletion(-)

arch/x86/kernel/cpu/resctrl/core.c +2 -0

index cf29681d01e0..a28de257168f 100644
@@ -558,6 +558,8 @@ static void domain_add_cpu_mon(int cpu, struct rdt_resource *r)
 		return;
 	}
 
+	arch_mbm_evt_config_init(hw_dom);
+
 	list_add_tail_rcu(&d->hdr.list, add_pos);
 
 	err = resctrl_online_mon_domain(r, d);

arch/x86/kernel/cpu/resctrl/internal.h +5 -0

index c44c5b496355..9846153aa48f 100644
@@ -32,6 +32,9 @@
  */
 #define MBM_CNTR_WIDTH_OFFSET_MAX (62 - MBM_CNTR_WIDTH_BASE)
 
+#define INVALID_CONFIG_VALUE		U32_MAX
+#define INVALID_CONFIG_INDEX		UINT_MAX
+
 /**
  * cpumask_any_housekeeping() - Choose any CPU in @mask, preferring those that
  *			        aren't marked nohz_full

@@ -335,6 +338,8 @@ struct rdt_hw_ctrl_domain {
  * @d_resctrl:	Properties exposed to the resctrl file system
  * @arch_mbm_total:	arch private state for MBM total bandwidth
  * @arch_mbm_local:	arch private state for MBM local bandwidth
+ * @mbm_total_cfg:	MBM total bandwidth configuration
+ * @mbm_local_cfg:	MBM local bandwidth configuration
  *
  * Members of this structure are accessed via helpers that provide abstraction.
  */

arch/x86/kernel/cpu/resctrl/internal.h +2 -0

 static inline struct rdt_hw_ctrl_domain *resctrl_to_arch_ctrl_dom(struct rdt_ctrl_domain *r)

@@ -504,6 +511,8 @@ void resctrl_file_fflags_init(const char *config, unsigned long fflags);
 void rdt_staged_configs_clear(void);
 bool closid_allocated(unsigned int closid);
 int resctrl_find_cleanest_closid(void);
+void arch_mbm_evt_config_init(struct rdt_hw_mon_domain *hw_dom);
+unsigned int mon_event_config_index_get(u32 evtid);
 
 #ifdef CONFIG_RESCTRL_FS_PSEUDO_LOCK
 int rdtgroup_locksetup_enter(struct rdtgroup *rdtgrp);

arch/x86/kernel/cpu/resctrl/monitor.c +0 -0

index a93ed7d2a160..abd337fbd01d 100644
@@ -1284,6 +1284,32 @@ int __init rdt_get_mon_l3_config(struct rdt_resource *r)
 	return 0;
 }
 

Hi Babu,

Quoted 5 lines · Babu Moger

If the BMEC (Bandwidth Monitoring Event Configuration) feature is supported, the bandwidth events can be configured to track specific events. The event configuration is domain specific. Event configurations are not stored in resctrl but instead always read from or written to hardware directly when prompted by user space.

Why is this a problem?

Quoted 5 lines

Read the event configuration from the hardware during domain initialization and store the configuration value in the rdt_hw_mon_domain structure for later use when the user requests to display it.

Why is this required?

This series is about adding support for ABMC while this appears to be an optimization for BMEC. Even more, as I see it, this optimization makes resctrl support of ABMC and BMEC confusing (more below).

Quoted 1 line

Quoted 4 lines

Signed-off-by Babu Moger <[email protected]>

---
v12: Fixed the conflicts due to recent merge.
This patch is for BMEC and there is no dependancy on ABMC feature.

Why still do it?

Quoted 73 lines

Moved it earlier.

v11: Resolved minor conflicts due to code displacement. Actual code didnt
change.

v10: Conflicts due to code displacement. Actual code didnt change.

v9: Added Reviewed-by tag. No other changes.

v8: Renamed resctrl_mbm_evt_config_init() to arch_mbm_evt_config_init()
Minor commit message update.

v7: Fixed initializing INVALID_CONFIG_VALUE to mbm_local_cfg in case of error.

v6: Renamed resctrl_arch_mbm_evt_config -> resctrl_mbm_evt_config_init
Initialized value to INVALID_CONFIG_VALUE if it is not configurable. Minor commit message update.

v5: Exported mon_event_config_index_get.
Renamed arch_domain_mbm_evt_config to resctrl_arch_mbm_evt_config.

v4: Read the configuration information from the hardware to initialize.
Added few commit messages.
Fixed the tab spaces.

v3: Minor changes related to rebase in mbm_config_write_domain.

v2: No changes.

---
 arch/x86/kernel/cpu/resctrl/core.c     |  2 ++
 arch/x86/kernel/cpu/resctrl/internal.h |  9 +++++++++
 arch/x86/kernel/cpu/resctrl/monitor.c  | 26 ++++++++++++++++++++++++++
 arch/x86/kernel/cpu/resctrl/rdtgroup.c |  2 +-
 4 files changed, 38 insertions(+), 1 deletion(-)

arch/x86/kernel/cpu/resctrl/core.c +2 -0

index cf29681d01e0..a28de257168f 100644
@@ -558,6 +558,8 @@ static void domain_add_cpu_mon(int cpu, struct rdt_resource *r)
 		return;
 	}
 
+	arch_mbm_evt_config_init(hw_dom);
+
 	list_add_tail_rcu(&d->hdr.list, add_pos);
 
 	err = resctrl_online_mon_domain(r, d);

arch/x86/kernel/cpu/resctrl/internal.h +5 -0

index c44c5b496355..9846153aa48f 100644
@@ -32,6 +32,9 @@
  */
 #define MBM_CNTR_WIDTH_OFFSET_MAX (62 - MBM_CNTR_WIDTH_BASE)
 
+#define INVALID_CONFIG_VALUE		U32_MAX
+#define INVALID_CONFIG_INDEX		UINT_MAX
+
 /**
  * cpumask_any_housekeeping() - Choose any CPU in @mask, preferring those that
  *			        aren't marked nohz_full

@@ -335,6 +338,8 @@ struct rdt_hw_ctrl_domain {
  * @d_resctrl:	Properties exposed to the resctrl file system
  * @arch_mbm_total:	arch private state for MBM total bandwidth
  * @arch_mbm_local:	arch private state for MBM local bandwidth
+ * @mbm_total_cfg:	MBM total bandwidth configuration
+ * @mbm_local_cfg:	MBM local bandwidth configuration
  *
  * Members of this structure are accessed via helpers that provide abstraction.
  */

arch/x86/kernel/cpu/resctrl/internal.h struct rdt_hw_mon_domain { · line 352

Quoted 6 lines

arch/x86/kernel/cpu/resctrl/internal.h +2 -0

@@ -342,6 +347,8 @@ struct rdt_hw_mon_domain {
 	struct rdt_mon_domain		d_resctrl;
 	struct arch_mbm_state		*arch_mbm_total;
 	struct arch_mbm_state		*arch_mbm_local;
+	u32				mbm_total_cfg;
+	u32				mbm_local_cfg;
 };

This introduces an architecture managed per-domain event configuration while the rest of the series introduces a resctrl fs managed global event configuration. I see this as the start of a source for confusion about how events are configured since there is no further connection between this per-domain event configuration maintained by the architecture and the global event configuration maintained by resctrl fs.

Quoted 19 lines

arch/x86/kernel/cpu/resctrl/internal.h +2 -0

 static inline struct rdt_hw_ctrl_domain *resctrl_to_arch_ctrl_dom(struct rdt_ctrl_domain *r)

@@ -504,6 +511,8 @@ void resctrl_file_fflags_init(const char *config, unsigned long fflags);
 void rdt_staged_configs_clear(void);
 bool closid_allocated(unsigned int closid);
 int resctrl_find_cleanest_closid(void);
+void arch_mbm_evt_config_init(struct rdt_hw_mon_domain *hw_dom);
+unsigned int mon_event_config_index_get(u32 evtid);
 
 #ifdef CONFIG_RESCTRL_FS_PSEUDO_LOCK
 int rdtgroup_locksetup_enter(struct rdtgroup *rdtgrp);

arch/x86/kernel/cpu/resctrl/monitor.c +0 -0

index a93ed7d2a160..abd337fbd01d 100644
@@ -1284,6 +1284,32 @@ int __init rdt_get_mon_l3_config(struct rdt_resource *r)
 	return 0;
 }
 

arch/x86/kernel/cpu/resctrl/monitor.c int __init rdt_get_mon_l3_config(struct rdt_resource *r) · line 1296

Quoted 10 lines

arch/x86/kernel/cpu/resctrl/monitor.c +10 -0

+void arch_mbm_evt_config_init(struct rdt_hw_mon_domain *hw_dom)
+{
+	unsigned int index;
+	u64 msrval;
+
+	/*
+	 * Read the configuration registers QOS_EVT_CFG_n, where <n> is
+	 * the BMEC event number (EvtID).
+	 */
+	if (mbm_total_event.configurable) {

Please keep an eye on where things are going in the arch/fs split. mbm_total_event is private to resctrl fs and arch code cannot reach into it. There is the arch helper resctrl_arch_is_evt_configurable() but I also think that this helper needs to be reconsidered in the light of ABMC.

Overall I think this ABMC support needs to consider what already exists for BMEC support and ensure that both are supported coherently. For example, when a monitor domain has a "MBM local bandwidth configuration" then it should be obvious what that means.

The event configuration is domain specific and initialized during domain initialization. The values are stored in struct rdt_hw_mon_domain.

It is not required to read the configuration register every time user asks for it. Use the value stored in struct rdt_hw_mon_domain instead.

Hi Babu,

Quoted 3 lines · Babu Moger

The event configuration is domain specific and initialized during domain initialization. The values are stored in struct rdt_hw_mon_domain.

Quoted 2 lines

It is not required to read the configuration register every time user asks for it. Use the value stored in struct rdt_hw_mon_domain instead.

Storing and maintaining the event configuration creates confusion with the new event configurations introduced in the rest of this series. I think that it will be simpler to keep BMEC support as-is.

Users can create as many monitor groups as RMIDs supported by the hardware. However, bandwidth monitoring feature on AMD system only guarantees that RMIDs currently assigned to a processor will be tracked by hardware. The counters of any other RMIDs which are no longer being tracked will be reset to zero. The MBM event counters return "Unavailable" for the RMIDs that are not tracked by hardware. So, there can be only limited number of groups that can give guaranteed monitoring numbers. With ever changing configurations there is no way to definitely know which of these groups are being tracked for certain point of time. Users do not have the option to monitor a group or set of groups for certain period of time without worrying about RMID being reset in between.

The ABMC feature provides an option to the user to assign a hardware counter to an RMID, event pair and monitor the bandwidth as long as it is assigned. The assigned RMID will be tracked by the hardware until the user unassigns it manually. There is no need to worry about counters being reset during this period. Additionally, the user can specify a bitmask identifying the specific bandwidth types from the given source to track with the counter.

Without ABMC enabled, monitoring will work in current mode without assignment option.

Linux resctrl subsystem provides the interface to count maximum of two memory bandwidth events per group, from a combination of available total and local events. Keeping the current interface, users can enable a maximum of 2 ABMC counters per group. User will also have the option to enable only one counter to the group. If the system runs out of assignable ABMC counters, kernel will display an error. Users need to disable an already enabled counter to make space for new assignments.

The feature can be detected via CPUID_Fn80000020_EBX_x00 bit 5. Bits Description
5 ABMC (Assignable Bandwidth Monitoring Counters)

The feature details are documented in APM listed below [1].
[1] AMD64 Architecture Programmer's Manual Volume 2: System Programming Publication # 24593 Revision 3.41 section 19.3.3.3 Assignable Bandwidth Monitoring (ABMC).

Link https://bugzilla.kernel.org/show_bug.cgi?id=206537

Signed-off-by Babu Moger <[email protected]>

---

Note: Checkpatch checks/warnings are ignored to maintain coding style.

v12: Removed the dependancy on X86_FEATURE_BMEC.

Hi Babu,

Quoted 23 lines · Babu Moger

Users can create as many monitor groups as RMIDs supported by the hardware. However, bandwidth monitoring feature on AMD system only guarantees that RMIDs currently assigned to a processor will be tracked by hardware. The counters of any other RMIDs which are no longer being tracked will be reset to zero. The MBM event counters return "Unavailable" for the RMIDs that are not tracked by hardware. So, there can be only limited number of groups that can give guaranteed monitoring numbers. With ever changing configurations there is no way to definitely know which of these groups are being tracked for certain point of time. Users do not have the option to monitor a group or set of groups for certain period of time without worrying about RMID being reset in between.

The ABMC feature provides an option to the user to assign a hardware counter to an RMID, event pair and monitor the bandwidth as long as it is assigned. The assigned RMID will be tracked by the hardware until the user unassigns it manually. There is no need to worry about counters being reset during this period. Additionally, the user can specify a bitmask identifying the specific bandwidth types from the given source to track with the counter.

Without ABMC enabled, monitoring will work in current mode without assignment option.

Quoted 7 lines

Linux resctrl subsystem provides the interface to count maximum of two memory bandwidth events per group, from a combination of available total and local events. Keeping the current interface, users can enable a maximum of 2 ABMC counters per group. User will also have the option to enable only one counter to the group. If the system runs out of assignable ABMC counters, kernel will display an error. Users need to disable an already enabled counter to make space for new assignments.

The above paragraph sounds like it is still talking about the original global assignment of counters.

Quoted 11 lines

The feature can be detected via CPUID_Fn80000020_EBX_x00 bit 5. Bits Description
5 ABMC (Assignable Bandwidth Monitoring Counters)

The feature details are documented in APM listed below [1].
[1] AMD64 Architecture Programmer's Manual Volume 2: System Programming Publication # 24593 Revision 3.41 section 19.3.3.3 Assignable Bandwidth Monitoring (ABMC).

Quoted 7 lines

Link https://bugzilla.kernel.org/show_bug.cgi?id=206537

Signed-off-by Babu Moger <[email protected]>

---

Note: Checkpatch checks/warnings are ignored to maintain coding style.

v12: Removed the dependancy on X86_FEATURE_BMEC.

Considering this removal it is not clear to me how the BMEC and ABMC features are managed on a platform. Since this dependency existed I assume platforms that support both ABMC and BMEC exist and after previous discussion [1] I expected to see that BMEC support will be disabled when ABMC is detected but I do not see this done in this series. From what I can tell, looking at patch "x86/resctrl: Detect Assignable Bandwidth Monitoring feature details" BMEC and ABMC are both detected and enabled while I do not see any interactions handled. For example, a user modifying the BMEC appears to have no impact on existing ABMC assigned counters. Could you please clarify how event configuration works on platforms that support both ABMC and BMEC?

Reinette

[1] https://lore.kernel.org/all/[email protected]/

Here is my proposal to handle this case. This can be separate patch.

arch/x86/kernel/cpu/resctrl/rdtgroup.c +1 -1

index d10cf1e5b914..772f2f77faee 100644
@@ -1370,7 +1370,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->configurable &&
 !resctrl_arch_mbm_cntr_assign_enabled(r))
                        seq_printf(seq, "%s_config\n", mevt->name);

        }

arch/x86/kernel/cpu/resctrl/rdtgroup.c +0 -0

@@ -1846,6 +1846,11 @@ static int mbm_config_show(struct seq_file *s,

struct rdt_resource *r, u32 evtid

        cpus_read_lock();
        mutex_lock(&rdtgroup_mutex);

arch/x86/kernel/cpu/resctrl/rdtgroup.c +2 -0

+       if (resctrl_arch_mbm_cntr_assign_enabled(r)) {
+               rdt_last_cmd_puts("Event configuration(BMEC) not supported

with mbm_cntr_assign mode\n");

arch/x86/kernel/cpu/resctrl/rdtgroup.c +3 -0

+               return -EINVAL;
+       }
+
        list_for_each_entry(dom, &r->mon_domains, hdr.list) {
                if (sep)
                        seq_puts(s, ";");

@@ -1865,21 +1870,24 @@ static int mbm_config_show(struct seq_file *s,

struct rdt_resource *r, u32 evtid
static int mbm_total_bytes_config_show(struct kernfs_open_file *of,
struct seq_file *seq, void *v)
{

arch/x86/kernel/cpu/resctrl/rdtgroup.c +12 -4

+       int ret;
        struct rdt_resource *r = of->kn->parent->priv;
 
-       mbm_config_show(seq, r, QOS_L3_MBM_TOTAL_EVENT_ID);
+       ret = mbm_config_show(seq, r, QOS_L3_MBM_TOTAL_EVENT_ID);
 
-       return 0;
+       return ret;
 }
 
 static int mbm_local_bytes_config_show(struct kernfs_open_file *of,
                                       struct seq_file *seq, void *v)
 {
+       int ret;
+
        struct rdt_resource *r = of->kn->parent->priv;
 
-       mbm_config_show(seq, r, QOS_L3_MBM_LOCAL_EVENT_ID);
+       ret = mbm_config_show(seq, r, QOS_L3_MBM_LOCAL_EVENT_ID);
 
-       return 0;
+       return ret;
 }
 
 static void mbm_config_write_domain(struct rdt_resource *r,

@@ -1932,6 +1940,11 @@ static int mon_config_write(struct rdt_resource *r,
 char *tok, u32 evtid)
        /* Walking r->domains, ensure it can't race with cpuhp */
        lockdep_assert_cpus_held();
 
+       if (resctrl_arch_mbm_cntr_assign_enabled(r)) {
+               rdt_last_cmd_puts("Event configuration(BMEC) not supported
 with mbm_cntr_assign mode\n");
+               return -EINVAL;
+       }
+
 next:

        if (!tok || tok[0] == '\0')
                return 0;

Hi Babu,

Quoted 68 lines · Babu

Here is my proposal to handle this case. This can be separate patch.

arch/x86/kernel/cpu/resctrl/rdtgroup.c +1 -1

index d10cf1e5b914..772f2f77faee 100644
@@ -1370,7 +1370,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->configurable &&
 !resctrl_arch_mbm_cntr_assign_enabled(r))
                        seq_printf(seq, "%s_config\n", mevt->name);

        }

arch/x86/kernel/cpu/resctrl/rdtgroup.c +0 -0

@@ -1846,6 +1846,11 @@ static int mbm_config_show(struct seq_file *s,

struct rdt_resource *r, u32 evtid

        cpus_read_lock();
        mutex_lock(&rdtgroup_mutex);

arch/x86/kernel/cpu/resctrl/rdtgroup.c +2 -0

+       if (resctrl_arch_mbm_cntr_assign_enabled(r)) {
+               rdt_last_cmd_puts("Event configuration(BMEC) not supported

with mbm_cntr_assign mode\n");

arch/x86/kernel/cpu/resctrl/rdtgroup.c +3 -0

+               return -EINVAL;
+       }
+
        list_for_each_entry(dom, &r->mon_domains, hdr.list) {
                if (sep)
                        seq_puts(s, ";");

@@ -1865,21 +1870,24 @@ static int mbm_config_show(struct seq_file *s,

struct rdt_resource *r, u32 evtid
static int mbm_total_bytes_config_show(struct kernfs_open_file *of,
struct seq_file *seq, void *v)
{

arch/x86/kernel/cpu/resctrl/rdtgroup.c +7 -4

+       int ret;
        struct rdt_resource *r = of->kn->parent->priv;
 
-       mbm_config_show(seq, r, QOS_L3_MBM_TOTAL_EVENT_ID);
+       ret = mbm_config_show(seq, r, QOS_L3_MBM_TOTAL_EVENT_ID);
 
-       return 0;
+       return ret;
 }
 
 static int mbm_local_bytes_config_show(struct kernfs_open_file *of,
                                       struct seq_file *seq, void *v)
 {
+       int ret;
+
        struct rdt_resource *r = of->kn->parent->priv;
 
-       mbm_config_show(seq, r, QOS_L3_MBM_LOCAL_EVENT_ID);
+       ret = mbm_config_show(seq, r, QOS_L3_MBM_LOCAL_EVENT_ID);
 
-       return 0;
+       return ret;
 }
 
 static void mbm_config_write_domain(struct rdt_resource *r,

@@ -1932,6 +1940,11 @@ static int mon_config_write(struct rdt_resource *r,
 char *tok, u32 evtid)
        /* Walking r->domains, ensure it can't race with cpuhp */
        lockdep_assert_cpus_held();
 

Quoted 10 lines

arch/x86/kernel/cpu/resctrl/rdtgroup.c +5 -0

+       if (resctrl_arch_mbm_cntr_assign_enabled(r)) {
+               rdt_last_cmd_puts("Event configuration(BMEC) not supported
 with mbm_cntr_assign mode\n");
+               return -EINVAL;
+       }
+
 next:

        if (!tok || tok[0] == '\0')
                return 0;

Instead of chasing every call that may involve BMEC I think it will be simpler to disable BMEC support during initialization when ABMC is detected. Specifically, on systems that support both BMEC and ABMC rdt_cpu_has(X86_FEATURE_BMEC) returns false.

I would also like to consider enhancing mevt->configurable to handle all different ways in which events can be configured. For example, making mevt->configurable an enum that captures how event can be configured instead of keeping mevt->configurable a boolean for BMEC support and handling ABMC completely separately. I hope this may become clearer when using struct mon_evt for ABMC also.

Hi Reinette,

Quoted 72 lines · Reinette Chatre

Hi Babu,

Quoted 68 lines · Babu

Here is my proposal to handle this case. This can be separate patch.

arch/x86/kernel/cpu/resctrl/rdtgroup.c +1 -1

index d10cf1e5b914..772f2f77faee 100644
@@ -1370,7 +1370,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->configurable &&
 !resctrl_arch_mbm_cntr_assign_enabled(r))
                        seq_printf(seq, "%s_config\n", mevt->name);

        }

arch/x86/kernel/cpu/resctrl/rdtgroup.c +0 -0

@@ -1846,6 +1846,11 @@ static int mbm_config_show(struct seq_file *s,

struct rdt_resource *r, u32 evtid

        cpus_read_lock();
        mutex_lock(&rdtgroup_mutex);

arch/x86/kernel/cpu/resctrl/rdtgroup.c +2 -0

+       if (resctrl_arch_mbm_cntr_assign_enabled(r)) {
+               rdt_last_cmd_puts("Event configuration(BMEC) not supported

with mbm_cntr_assign mode\n");

arch/x86/kernel/cpu/resctrl/rdtgroup.c +3 -0

+               return -EINVAL;
+       }
+
        list_for_each_entry(dom, &r->mon_domains, hdr.list) {
                if (sep)
                        seq_puts(s, ";");

@@ -1865,21 +1870,24 @@ static int mbm_config_show(struct seq_file *s,

struct rdt_resource *r, u32 evtid
static int mbm_total_bytes_config_show(struct kernfs_open_file *of,
struct seq_file *seq, void *v)
{

arch/x86/kernel/cpu/resctrl/rdtgroup.c +7 -4

+       int ret;
        struct rdt_resource *r = of->kn->parent->priv;
 
-       mbm_config_show(seq, r, QOS_L3_MBM_TOTAL_EVENT_ID);
+       ret = mbm_config_show(seq, r, QOS_L3_MBM_TOTAL_EVENT_ID);
 
-       return 0;
+       return ret;
 }
 
 static int mbm_local_bytes_config_show(struct kernfs_open_file *of,
                                       struct seq_file *seq, void *v)
 {
+       int ret;
+
        struct rdt_resource *r = of->kn->parent->priv;
 
-       mbm_config_show(seq, r, QOS_L3_MBM_LOCAL_EVENT_ID);
+       ret = mbm_config_show(seq, r, QOS_L3_MBM_LOCAL_EVENT_ID);
 
-       return 0;
+       return ret;
 }
 
 static void mbm_config_write_domain(struct rdt_resource *r,

@@ -1932,6 +1940,11 @@ static int mon_config_write(struct rdt_resource *r,
 char *tok, u32 evtid)
        /* Walking r->domains, ensure it can't race with cpuhp */
        lockdep_assert_cpus_held();
 

Quoted 15 lines

Quoted 10 lines

arch/x86/kernel/cpu/resctrl/rdtgroup.c +5 -0

+       if (resctrl_arch_mbm_cntr_assign_enabled(r)) {
+               rdt_last_cmd_puts("Event configuration(BMEC) not supported
 with mbm_cntr_assign mode\n");
+               return -EINVAL;
+       }
+
 next:

        if (!tok || tok[0] == '\0')
                return 0;

Instead of chasing every call that may involve BMEC I think it will be simpler to disable BMEC support during initialization when ABMC is detected. Specifically, on systems that support both BMEC and ABMC rdt_cpu_has(X86_FEATURE_BMEC) returns false.

There is one problem with this approach. Users have the option to switch between the assignment modes. System will boot with ABMC by default if supported. But, users can switch to 'default' mode after the boot. By disabling the BMEC completely, it will not be possible to do that.

Quoted 7 lines

I would also like to consider enhancing mevt->configurable to handle all different ways in which events can be configured. For example, making mevt->configurable an enum that captures how event can be configured instead of keeping mevt->configurable a boolean for BMEC support and handling ABMC completely separately. I hope this may become clearer when using struct mon_evt for ABMC also.

Sure. I can try that.

Hi Babu,

Quoted 91 lines · Babu

Hi Reinette,

Quoted 72 lines · Reinette Chatre

Hi Babu,

Quoted 68 lines · Babu

Here is my proposal to handle this case. This can be separate patch.

arch/x86/kernel/cpu/resctrl/rdtgroup.c +1 -1

index d10cf1e5b914..772f2f77faee 100644
@@ -1370,7 +1370,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->configurable &&
 !resctrl_arch_mbm_cntr_assign_enabled(r))
                        seq_printf(seq, "%s_config\n", mevt->name);

        }

arch/x86/kernel/cpu/resctrl/rdtgroup.c +0 -0

@@ -1846,6 +1846,11 @@ static int mbm_config_show(struct seq_file *s,

struct rdt_resource *r, u32 evtid

        cpus_read_lock();
        mutex_lock(&rdtgroup_mutex);

arch/x86/kernel/cpu/resctrl/rdtgroup.c +2 -0

+       if (resctrl_arch_mbm_cntr_assign_enabled(r)) {
+               rdt_last_cmd_puts("Event configuration(BMEC) not supported

with mbm_cntr_assign mode\n");

arch/x86/kernel/cpu/resctrl/rdtgroup.c +3 -0

+               return -EINVAL;
+       }
+
        list_for_each_entry(dom, &r->mon_domains, hdr.list) {
                if (sep)
                        seq_puts(s, ";");

@@ -1865,21 +1870,24 @@ static int mbm_config_show(struct seq_file *s,

struct rdt_resource *r, u32 evtid
static int mbm_total_bytes_config_show(struct kernfs_open_file *of,
struct seq_file *seq, void *v)
{

arch/x86/kernel/cpu/resctrl/rdtgroup.c +7 -4

+       int ret;
        struct rdt_resource *r = of->kn->parent->priv;
 
-       mbm_config_show(seq, r, QOS_L3_MBM_TOTAL_EVENT_ID);
+       ret = mbm_config_show(seq, r, QOS_L3_MBM_TOTAL_EVENT_ID);
 
-       return 0;
+       return ret;
 }
 
 static int mbm_local_bytes_config_show(struct kernfs_open_file *of,
                                       struct seq_file *seq, void *v)
 {
+       int ret;
+
        struct rdt_resource *r = of->kn->parent->priv;
 
-       mbm_config_show(seq, r, QOS_L3_MBM_LOCAL_EVENT_ID);
+       ret = mbm_config_show(seq, r, QOS_L3_MBM_LOCAL_EVENT_ID);
 
-       return 0;
+       return ret;
 }
 
 static void mbm_config_write_domain(struct rdt_resource *r,

@@ -1932,6 +1940,11 @@ static int mon_config_write(struct rdt_resource *r,
 char *tok, u32 evtid)
        /* Walking r->domains, ensure it can't race with cpuhp */
        lockdep_assert_cpus_held();
 

Quoted 15 lines

Quoted 10 lines

arch/x86/kernel/cpu/resctrl/rdtgroup.c +5 -0

+       if (resctrl_arch_mbm_cntr_assign_enabled(r)) {
+               rdt_last_cmd_puts("Event configuration(BMEC) not supported
 with mbm_cntr_assign mode\n");
+               return -EINVAL;
+       }
+
 next:

        if (!tok || tok[0] == '\0')
                return 0;

Instead of chasing every call that may involve BMEC I think it will be simpler to disable BMEC support during initialization when ABMC is detected. Specifically, on systems that support both BMEC and ABMC rdt_cpu_has(X86_FEATURE_BMEC) returns false.

Quoted 4 lines

There is one problem with this approach. Users have the option to switch between the assignment modes. System will boot with ABMC by default if supported. But, users can switch to 'default' mode after the boot. By disabling the BMEC completely, it will not be possible to do that.

Good point. Thank you. Another option is to hide (see kernfs_show()) mbm_total_bytes_config and mbm_local_bytes_config when ABMC is enabled. To me this seems like a clear interface to user space, when user interface changes the mode the interface changes to reflect new mode.

Introduce the interface file "mbm_assign_mode" to list monitor modes

supported.

The "mbm_cntr_assign" mode provides the option to assign a counter to an RMID, event pair and monitor the bandwidth as long as it is assigned.

On AMD systems "mbm_cntr_assign" mode is backed by the ABMC (Assignable Bandwidth Monitoring Counters) hardware feature and is enabled by default.

The "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

Added an IS_ENABLED(CONFIG_RESCTRL_ASSIGN_FIXED) check to handle Arm64

platforms. On x86, CONFIG_RESCTRL_ASSIGN_FIXED is not defined, whereas on Arm64, it is. As a result, for MPAM, the file would be either:

[default]
or
[mbm_cntr_assign]

Signed-off-by Babu Moger <[email protected]>

---
v12: Minor text update in change log and user documentation.
Added the check CONFIG_RESCTRL_ASSIGN_FIXED to take care of arm platforms. This will be defined only in arm and not in x86.

v11: Renamed rdtgroup_mbm_assign_mode_show() to resctrl_mbm_assign_mode_show().
Removed few texts in resctrl.rst about AMD specific information. Updated few texts.

v10: Added few more text to user documentation clarify on the default mode.

v9: Updated user documentation based on comments.

v8: Commit message update.

v7: Updated the descriptions/commit log in resctrl.rst to generic text.
Thanks to James and Reinette.
Rename mbm_mode to mbm_assign_mode.
Introduced mutex lock in rdtgroup_mbm_mode_show().

v6: Added documentation for mbm_cntr_assign and legacy mode.
Moved mbm_mode fflags initialization to static initialization.

v5: Changed interface name to mbm_mode.
It will be always available even if ABMC feature is not supported. Added description in resctrl.rst about ABMC mode.
Fixed display abmc and legacy consistantly.

v4: Fixed the checks for legacy and abmc mode. Default it ABMC.

v3: New patch to display ABMC capability.

???END

---
 Documentation/arch/x86/resctrl.rst     | 27 +++++++++++++++++++
 arch/x86/kernel/cpu/resctrl/rdtgroup.c | 37 ++++++++++++++++++++++++++
 2 files changed, 64 insertions(+)

Documentation/arch/x86/resctrl.rst +5 -0

index fb90f08e564e..bb96b44019fe 100644
@@ -257,6 +257,33 @@ with the following files:
 	    # cat /sys/fs/resctrl/info/L3_MON/mbm_local_bytes_config
 	    0=0x30;1=0x30;3=0x15;4=0x15
 
+"mbm_assign_mode":
+	Reports the list of monitoring modes supported. The enclosed brackets
+	indicate which mode is enabled.
+	::
+

Documentation/arch/x86/resctrl.rst +3 -0

+	misleading values or display "Unavailable" if no counter is assigned
+	to the event.
+
 "max_threshold_occupancy":
 		Read/write file provides the largest value (in
 		bytes) at which a previously used LLC_occupancy

arch/x86/kernel/cpu/resctrl/rdtgroup.c +30 -0

index 17de38e26f94..626be6becca7 100644
@@ -882,6 +882,36 @@ static int rdtgroup_rmid_show(struct kernfs_open_file *of,
 	return ret;
 }
 
+static int resctrl_mbm_assign_mode_show(struct kernfs_open_file *of,
+					struct seq_file *s, void *v)
+{
+	struct rdt_resource *r = of->kn->parent->priv;
+	bool enabled;
+
+	mutex_lock(&rdtgroup_mutex);
+	enabled = resctrl_arch_mbm_cntr_assign_enabled(r);
+
+	if (r->mon.mbm_cntr_assignable) {
+		if (enabled)
+			seq_puts(s, "[mbm_cntr_assign]\n");
+		else
+			seq_puts(s, "[default]\n");
+
+		if (!IS_ENABLED(CONFIG_RESCTRL_ASSIGN_FIXED)) {
+			if (enabled)
+				seq_puts(s, "default\n");
+			else
+				seq_puts(s, "mbm_cntr_assign\n");
+		}
+	} else {
+		seq_puts(s, "[default]\n");
+	}
+
+	mutex_unlock(&rdtgroup_mutex);
+
+	return 0;
+}
+
 #ifdef CONFIG_PROC_CPU_RESCTRL
 
 /*

Hi Babu,

Quoted 1 line · Babu Moger

Introduce the interface file "mbm_assign_mode" to list monitor modes

Using "resctrl file" instead of "interface file" should help to make it clear what this patch does.

Quoted 14 lines

supported.

The "mbm_cntr_assign" mode provides the option to assign a counter to an RMID, event pair and monitor the bandwidth as long as it is assigned.

On AMD systems "mbm_cntr_assign" mode is backed by the ABMC (Assignable Bandwidth Monitoring Counters) hardware feature and is enabled by default.

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

Quoted 6 lines

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

Added an IS_ENABLED(CONFIG_RESCTRL_ASSIGN_FIXED) check to handle Arm64

(needs imperative)

Quoted 3 lines

platforms. On x86, CONFIG_RESCTRL_ASSIGN_FIXED is not defined, whereas on Arm64, it is. As a result, for MPAM, the file would be either:

CONFIG_RESCTRL_ASSIGN_FIXED does not yet exist anywhere so this motivation needs to provide stronger support for why it is used before it exists. There is a precedent here with RESCTRL_RMID_DEPENDS_ON_CLOSID already used while it does not yet appear in a Kconfig file. I would propose that this is motivated by noting how it is already understood how Arm supports assignable counters this was recommended by James to prepare for that work. Since this is user interface this work is done early to ensure user interface is compatible with that upcoming support. Also set folks at ease that IS_ENABLED() works as expected with a non-existing config.

Quoted 62 lines

[default]
or
[mbm_cntr_assign]

Signed-off-by Babu Moger <[email protected]>

---
v12: Minor text update in change log and user documentation.
Added the check CONFIG_RESCTRL_ASSIGN_FIXED to take care of arm platforms. This will be defined only in arm and not in x86.

v11: Renamed rdtgroup_mbm_assign_mode_show() to resctrl_mbm_assign_mode_show().
Removed few texts in resctrl.rst about AMD specific information. Updated few texts.

v10: Added few more text to user documentation clarify on the default mode.

v9: Updated user documentation based on comments.

v8: Commit message update.

v7: Updated the descriptions/commit log in resctrl.rst to generic text.
Thanks to James and Reinette.
Rename mbm_mode to mbm_assign_mode.
Introduced mutex lock in rdtgroup_mbm_mode_show().

v6: Added documentation for mbm_cntr_assign and legacy mode.
Moved mbm_mode fflags initialization to static initialization.

v5: Changed interface name to mbm_mode.
It will be always available even if ABMC feature is not supported. Added description in resctrl.rst about ABMC mode.
Fixed display abmc and legacy consistantly.

v4: Fixed the checks for legacy and abmc mode. Default it ABMC.

v3: New patch to display ABMC capability.

???END

---
 Documentation/arch/x86/resctrl.rst     | 27 +++++++++++++++++++
 arch/x86/kernel/cpu/resctrl/rdtgroup.c | 37 ++++++++++++++++++++++++++
 2 files changed, 64 insertions(+)

Documentation/arch/x86/resctrl.rst +9 -0

index fb90f08e564e..bb96b44019fe 100644
@@ -257,6 +257,33 @@ with the following files:
 	    # cat /sys/fs/resctrl/info/L3_MON/mbm_local_bytes_config
 	    0=0x30;1=0x30;3=0x15;4=0x15
 
+"mbm_assign_mode":
+	Reports the list of monitoring modes supported. The enclosed brackets
+	indicate which mode is enabled.
+	::
+
+	  # cat /sys/fs/resctrl/info/L3_MON/mbm_assign_mode
+	  [mbm_cntr_assign]
+	  default
+

Documentation/arch/x86/resctrl.rst with the following files: · line 274

Quoted 6 lines

Documentation/arch/x86/resctrl.rst +6 -0

+	"mbm_cntr_assign":
+
+	In mbm_cntr_assign mode, a monitoring event can only accumulate data
+	while it is backed by a hardware counter. The user-space is able to
+	specify which of the events in CTRL_MON or MON groups should have a
+	counter assigned using the "mbm_assign_control" file. The number of

"mbm_assign_control" no longer exist.

Documentation/arch/x86/resctrl.rst with the following files: · line 283

Quoted 10 lines

Documentation/arch/x86/resctrl.rst +9 -0

+	counters available is described in the "num_mbm_cntrs" file. Changing
+	the mode may cause all counters on the resource to reset.
+
+	"default":
+
+	In default mode, resctrl assumes there is a hardware counter for each
+	event within every CTRL_MON and MON group. On AMD platforms, it is
+	recommended to use the mbm_cntr_assign mode, if supported, to prevent
+	the hardware from resetting counters between reads. This can result in

"from resetting counters" -> "from re-allocating counters"?

Quoted 47 lines

Documentation/arch/x86/resctrl.rst +3 -0

+	misleading values or display "Unavailable" if no counter is assigned
+	to the event.
+
 "max_threshold_occupancy":
 		Read/write file provides the largest value (in
 		bytes) at which a previously used LLC_occupancy

arch/x86/kernel/cpu/resctrl/rdtgroup.c +30 -0

index 17de38e26f94..626be6becca7 100644
@@ -882,6 +882,36 @@ static int rdtgroup_rmid_show(struct kernfs_open_file *of,
 	return ret;
 }
 
+static int resctrl_mbm_assign_mode_show(struct kernfs_open_file *of,
+					struct seq_file *s, void *v)
+{
+	struct rdt_resource *r = of->kn->parent->priv;
+	bool enabled;
+
+	mutex_lock(&rdtgroup_mutex);
+	enabled = resctrl_arch_mbm_cntr_assign_enabled(r);
+
+	if (r->mon.mbm_cntr_assignable) {
+		if (enabled)
+			seq_puts(s, "[mbm_cntr_assign]\n");
+		else
+			seq_puts(s, "[default]\n");
+
+		if (!IS_ENABLED(CONFIG_RESCTRL_ASSIGN_FIXED)) {
+			if (enabled)
+				seq_puts(s, "default\n");
+			else
+				seq_puts(s, "mbm_cntr_assign\n");
+		}
+	} else {
+		seq_puts(s, "[default]\n");
+	}
+
+	mutex_unlock(&rdtgroup_mutex);
+
+	return 0;
+}
+
 #ifdef CONFIG_PROC_CPU_RESCTRL
 

arch/x86/kernel/cpu/resctrl/rdtgroup.c static struct rftype res_common_files[] = { · line 1946

Quoted 11 lines

arch/x86/kernel/cpu/resctrl/rdtgroup.c +6 -0

 /*

@@ -1908,6 +1938,13 @@ static struct rftype res_common_files[] = {
 		.seq_show	= mbm_local_bytes_config_show,
 		.write		= mbm_local_bytes_config_write,
 	},
+	{
+		.name		= "mbm_assign_mode",
+		.mode		= 0444,
+		.kf_ops		= &rdtgroup_kf_single_ops,
+		.seq_show	= resctrl_mbm_assign_mode_show,
+		.fflags		= RFTYPE_MON_INFO,

Needs a RFTYPE_RES_CACHE?

arch/x86/kernel/cpu/resctrl/rdtgroup.c static struct rftype res_common_files[] = { · line 1950

Quoted 5 lines

arch/x86/kernel/cpu/resctrl/rdtgroup.c +1 -0

+	},
 	{
 		.name		= "cpus",
 		.mode		= 0644,

Reinette

Hi Reinette,

Quoted 7 lines · Reinette Chatre

Hi Babu,

Quoted 2 lines · Babu Moger

platforms. On x86, CONFIG_RESCTRL_ASSIGN_FIXED is not defined, whereas on Arm64, it is. As a result, for MPAM, the file would be either:

Quoted 9 lines

CONFIG_RESCTRL_ASSIGN_FIXED does not yet exist anywhere so this motivation needs to provide stronger support for why it is used before it exists. There is a precedent here with RESCTRL_RMID_DEPENDS_ON_CLOSID already used while it does not yet appear in a Kconfig file. I would propose that this is motivated by noting how it is already understood how Arm supports assignable counters this was recommended by James to prepare for that work. Since this is user interface this work is done early to ensure user interface is compatible with that upcoming support. Also set folks at ease that IS_ENABLED() works as expected with a non-existing config.

How about this?

Add IS_ENABLED(CONFIG_RESCTRL_ASSIGN_FIXED) check to support Arm64.

On x86, CONFIG_RESCTRL_ASSIGN_FIXED is not defined. On Arm64, it will be defined when the "mbm_cntr_assign" mode is supported.

Add an IS_ENABLED(CONFIG_RESCTRL_ASSIGN_FIXED) check early to ensure the user interface remains compatible with upcoming Arm64 support. IS_ENABLED() safely evaluates to 0 when the configuration is not defined.

As a result, for MPAM, the file would be either:
[default]
or
[mbm_cntr_assign]

Quoted 63 lines

Quoted 63 lines

[default]
or
[mbm_cntr_assign]

Signed-off-by Babu Moger <[email protected]>

---
v12: Minor text update in change log and user documentation.
Added the check CONFIG_RESCTRL_ASSIGN_FIXED to take care of arm platforms. This will be defined only in arm and not in x86.

v11: Renamed rdtgroup_mbm_assign_mode_show() to resctrl_mbm_assign_mode_show().
Removed few texts in resctrl.rst about AMD specific information. Updated few texts.

v10: Added few more text to user documentation clarify on the default mode.

v9: Updated user documentation based on comments.

v8: Commit message update.

v7: Updated the descriptions/commit log in resctrl.rst to generic text.
Thanks to James and Reinette.
Rename mbm_mode to mbm_assign_mode.
Introduced mutex lock in rdtgroup_mbm_mode_show().

v6: Added documentation for mbm_cntr_assign and legacy mode.
Moved mbm_mode fflags initialization to static initialization.

v5: Changed interface name to mbm_mode.
It will be always available even if ABMC feature is not supported. Added description in resctrl.rst about ABMC mode.
Fixed display abmc and legacy consistantly.

v4: Fixed the checks for legacy and abmc mode. Default it ABMC.

v3: New patch to display ABMC capability.

???END

---
 Documentation/arch/x86/resctrl.rst     | 27 +++++++++++++++++++
 arch/x86/kernel/cpu/resctrl/rdtgroup.c | 37 ++++++++++++++++++++++++++
 2 files changed, 64 insertions(+)

Documentation/arch/x86/resctrl.rst +9 -0

index fb90f08e564e..bb96b44019fe 100644
@@ -257,6 +257,33 @@ with the following files:
 	    # cat /sys/fs/resctrl/info/L3_MON/mbm_local_bytes_config
 	    0=0x30;1=0x30;3=0x15;4=0x15
 
+"mbm_assign_mode":
+	Reports the list of monitoring modes supported. The enclosed brackets
+	indicate which mode is enabled.
+	::
+
+	  # cat /sys/fs/resctrl/info/L3_MON/mbm_assign_mode
+	  [mbm_cntr_assign]
+	  default
+

Quoted 8 lines

Documentation/arch/x86/resctrl.rst with the following files: · line 274

Quoted 6 lines

Documentation/arch/x86/resctrl.rst +6 -0

+	"mbm_cntr_assign":
+
+	In mbm_cntr_assign mode, a monitoring event can only accumulate data
+	while it is backed by a hardware counter. The user-space is able to
+	specify which of the events in CTRL_MON or MON groups should have a
+	counter assigned using the "mbm_assign_control" file. The number of

"mbm_assign_control" no longer exist.

The user-space is able to specify which of the events in CTRL_MON or MON groups should have a counter assigned using the "mbm_L3_assignments" interface file in each resctrl group.

Quoted 13 lines

Documentation/arch/x86/resctrl.rst with the following files: · line 283

Quoted 11 lines

Documentation/arch/x86/resctrl.rst +9 -0

+	counters available is described in the "num_mbm_cntrs" file. Changing
+	the mode may cause all counters on the resource to reset.
+
+	"default":
+
+	In default mode, resctrl assumes there is a hardware counter for each
+	event within every CTRL_MON and MON group. On AMD platforms, it is
+	recommended to use the mbm_cntr_assign mode, if supported, to prevent
+	the hardware from resetting counters between reads. This can result in

"from resetting counters" -> "from re-allocating counters"?

How about?

"from resetting MBM events between reads"

Quoted 5 lines

Quoted 5 lines

Documentation/arch/x86/resctrl.rst +0 -0

 /*

@@ -1908,6 +1938,13 @@ static struct rftype res_common_files[] = {
 		.seq_show	= mbm_local_bytes_config_show,
 		.write		= mbm_local_bytes_config_write,
 	},

Quoted 8 lines

Documentation/arch/x86/resctrl.rst static struct rftype res_common_files[] = { · line 1946

Quoted 6 lines

Documentation/arch/x86/resctrl.rst +6 -0

+	{
+		.name		= "mbm_assign_mode",
+		.mode		= 0444,
+		.kf_ops		= &rdtgroup_kf_single_ops,
+		.seq_show	= resctrl_mbm_assign_mode_show,
+		.fflags		= RFTYPE_MON_INFO,

Needs a RFTYPE_RES_CACHE?

I am not very sure about this. This flag is added to the files in info/L3.

"mbm_assign_mode" goes in info/L3_MON/

The files in L3_MON does not have these flags set (for example mon_features, num_rmids).

Hi Babu,

Quoted 31 lines · Babu

Hi Reinette,

Quoted 7 lines · Reinette Chatre

Hi Babu,

Quoted 2 lines · Babu Moger

platforms. On x86, CONFIG_RESCTRL_ASSIGN_FIXED is not defined, whereas on Arm64, it is. As a result, for MPAM, the file would be either:

Quoted 9 lines

CONFIG_RESCTRL_ASSIGN_FIXED does not yet exist anywhere so this motivation needs to provide stronger support for why it is used before it exists. There is a precedent here with RESCTRL_RMID_DEPENDS_ON_CLOSID already used while it does not yet appear in a Kconfig file. I would propose that this is motivated by noting how it is already understood how Arm supports assignable counters this was recommended by James to prepare for that work. Since this is user interface this work is done early to ensure user interface is compatible with that upcoming support. Also set folks at ease that IS_ENABLED() works as expected with a non-existing config.

How about this?

Add IS_ENABLED(CONFIG_RESCTRL_ASSIGN_FIXED) check to support Arm64.

On x86, CONFIG_RESCTRL_ASSIGN_FIXED is not defined. On Arm64, it will be defined when the "mbm_cntr_assign" mode is supported.

Add an IS_ENABLED(CONFIG_RESCTRL_ASSIGN_FIXED) check early to ensure the user interface remains compatible with upcoming Arm64 support. IS_ENABLED() safely evaluates to 0 when the configuration is not defined.

Quoted 5 lines

As a result, for MPAM, the file would be either:
[default]
or
[mbm_cntr_assign]

Sounds good to me.

Quoted 72 lines

Quoted 63 lines

Quoted 63 lines

[default]
or
[mbm_cntr_assign]

Signed-off-by Babu Moger <[email protected]>

---
v12: Minor text update in change log and user documentation.
Added the check CONFIG_RESCTRL_ASSIGN_FIXED to take care of arm platforms. This will be defined only in arm and not in x86.

v11: Renamed rdtgroup_mbm_assign_mode_show() to resctrl_mbm_assign_mode_show().
Removed few texts in resctrl.rst about AMD specific information. Updated few texts.

v10: Added few more text to user documentation clarify on the default mode.

v9: Updated user documentation based on comments.

v8: Commit message update.

v7: Updated the descriptions/commit log in resctrl.rst to generic text.
Thanks to James and Reinette.
Rename mbm_mode to mbm_assign_mode.
Introduced mutex lock in rdtgroup_mbm_mode_show().

v6: Added documentation for mbm_cntr_assign and legacy mode.
Moved mbm_mode fflags initialization to static initialization.

v5: Changed interface name to mbm_mode.
It will be always available even if ABMC feature is not supported. Added description in resctrl.rst about ABMC mode.
Fixed display abmc and legacy consistantly.

v4: Fixed the checks for legacy and abmc mode. Default it ABMC.

v3: New patch to display ABMC capability.

???END

---
  Documentation/arch/x86/resctrl.rst     | 27 +++++++++++++++++++
  arch/x86/kernel/cpu/resctrl/rdtgroup.c | 37 ++++++++++++++++++++++++++
  2 files changed, 64 insertions(+)

Documentation/arch/x86/resctrl.rst +9 -0

index fb90f08e564e..bb96b44019fe 100644
@@ -257,6 +257,33 @@ with the following files:
  	    # cat /sys/fs/resctrl/info/L3_MON/mbm_local_bytes_config
  	    0=0x30;1=0x30;3=0x15;4=0x15
 
+"mbm_assign_mode":
+	Reports the list of monitoring modes supported. The enclosed brackets
+	indicate which mode is enabled.
+	::
+
+	  # cat /sys/fs/resctrl/info/L3_MON/mbm_assign_mode
+	  [mbm_cntr_assign]
+	  default
+

Quoted 8 lines

Documentation/arch/x86/resctrl.rst with the following files: · line 274

Quoted 6 lines

Documentation/arch/x86/resctrl.rst +6 -0

+	"mbm_cntr_assign":
+
+	In mbm_cntr_assign mode, a monitoring event can only accumulate data
+	while it is backed by a hardware counter. The user-space is able to
+	specify which of the events in CTRL_MON or MON groups should have a
+	counter assigned using the "mbm_assign_control" file. The number of

"mbm_assign_control" no longer exist.

Quoted 3 lines

The user-space is able to specify which of the events in CTRL_MON or MON groups should have a counter assigned using the "mbm_L3_assignments" interface file in each resctrl group.

I think it can be assumed the reader represents the user space. If doing so this can be simplified like:

Use "mbm_L3_assignments" found in each CTRL_MON and MON group to specify which of the events should have a counter assigned.

Quoted 14 lines

Quoted 13 lines

Documentation/arch/x86/resctrl.rst with the following files: · line 283

Quoted 11 lines

Documentation/arch/x86/resctrl.rst +9 -0

+	counters available is described in the "num_mbm_cntrs" file. Changing
+	the mode may cause all counters on the resource to reset.
+
+	"default":
+
+	In default mode, resctrl assumes there is a hardware counter for each
+	event within every CTRL_MON and MON group. On AMD platforms, it is
+	recommended to use the mbm_cntr_assign mode, if supported, to prevent
+	the hardware from resetting counters between reads. This can result in

"from resetting counters" -> "from re-allocating counters"?

Quoted 3 lines

How about?

"from resetting MBM events between reads"

With more detail, how about:

", to prevent reset of MBM events between reads resulting from hardware re-allocating counters"?

Quoted 15 lines

Quoted 14 lines

Documentation/arch/x86/resctrl.rst static struct rftype res_common_files[] = { · line 1946

Quoted 12 lines

Documentation/arch/x86/resctrl.rst +6 -0

  /*

@@ -1908,6 +1938,13 @@ static struct rftype res_common_files[] = {
  		.seq_show	= mbm_local_bytes_config_show,
  		.write		= mbm_local_bytes_config_write,
  	},
+	{
+		.name		= "mbm_assign_mode",
+		.mode		= 0444,
+		.kf_ops		= &rdtgroup_kf_single_ops,
+		.seq_show	= resctrl_mbm_assign_mode_show,
+		.fflags		= RFTYPE_MON_INFO,

Needs a RFTYPE_RES_CACHE?

Quoted 7 lines

I am not very sure about this. This flag is added to the files in info/L3.

"mbm_assign_mode" goes in info/L3_MON/

The files in L3_MON does not have these flags set (for example mon_features, num_rmids).

My assumption is that mon_features and num_rmids are generic monitoring files that should be supported by all resources that support monitoring. When resctrl starts to handle resource specific information then it should be clear what type or resource it applies to. I understand that this may not seem obvious since resctrl only supports monitoring on L3 resource.

Another view, consider existing code in resctrl_mon_resource_init() where the MBM configuration files are made specific to RFTYPE_RES_CACHE. I see mbm_assign_mode to be very similar to these.

The mbm_cntr_assign mode provides an option to the user to assign a counter to an RMID, event pair and monitor the bandwidth as long as the counter is assigned. Number of assignments depend on number of monitoring counters available.

Provide the interface to display the number of monitoring counters

supported in each domain. The resctrl file 'num_mbm_cntrs' is visible to user space when the system supports mbm_cntr_assign mode.

Signed-off-by Babu Moger <[email protected]>

---

---
 Documentation/arch/x86/resctrl.rst     | 11 ++++++++++
 arch/x86/kernel/cpu/resctrl/monitor.c  |  3 +++
 arch/x86/kernel/cpu/resctrl/rdtgroup.c | 30 ++++++++++++++++++++++++++
 3 files changed, 44 insertions(+)

Documentation/arch/x86/resctrl.rst +11 -0

index bb96b44019fe..35d908befdfb 100644
@@ -284,6 +284,17 @@ with the following files:
 	misleading values or display "Unavailable" if no counter is assigned
 	to the event.
 
+"num_mbm_cntrs":
+	The maximum number of monitoring counters (total of available and assigned
+	counters) in each domain when the system supports mbm_cntr_assign mode.
+
+	For example, on a system with maximum of 32 memory bandwidth monitoring
+	counters in each of its L3 domains:
+	::
+
+	  # cat /sys/fs/resctrl/info/L3_MON/num_mbm_cntrs
+	  0=32;1=32
+
 "max_threshold_occupancy":
 		Read/write file provides the largest value (in
 		bytes) at which a previously used LLC_occupancy

arch/x86/kernel/cpu/resctrl/monitor.c +0 -0

index 6ed7e51d3fdb..028b49878ad0 100644

arch/x86/kernel/cpu/resctrl/monitor.c +1 -0

+
 	return 0;
 }
 

arch/x86/kernel/cpu/resctrl/rdtgroup.c +3 -0

index 626be6becca7..0c9d7a702b93 100644
@@ -912,6 +912,30 @@ static int resctrl_mbm_assign_mode_show(struct kernfs_open_file *of,
 	return 0;
 }
 
+static int resctrl_num_mbm_cntrs_show(struct kernfs_open_file *of,
+				      struct seq_file *s, void *v)
+{

arch/x86/kernel/cpu/resctrl/rdtgroup.c +6 -0

+
+	mutex_unlock(&rdtgroup_mutex);
+	cpus_read_unlock();
+	return 0;
+}
+
 #ifdef CONFIG_PROC_CPU_RESCTRL
 
 /*

@@ -1945,6 +1969,12 @@ static struct rftype res_common_files[] = {
 		.seq_show	= resctrl_mbm_assign_mode_show,
 		.fflags		= RFTYPE_MON_INFO,

Hi Babu,

Quoted 6 lines · Babu Moger

The mbm_cntr_assign mode provides an option to the user to assign a counter to an RMID, event pair and monitor the bandwidth as long as the counter is assigned. Number of assignments depend on number of monitoring counters available.

Provide the interface to display the number of monitoring counters

An interface can also be a function. To help make this work obvious it can be specific:

Create 'num_mbm_cntrs' resctrl file that displays the number of monitoring counters supported in each domain. 'num_mbm_cntrs' is only visible to user space when the system supports mbm_cntr_assign mode.

Quoted 6 lines

supported in each domain. The resctrl file 'num_mbm_cntrs' is visible to user space when the system supports mbm_cntr_assign mode.

Signed-off-by Babu Moger <[email protected]>

---

...

Quoted 36 lines

---
 Documentation/arch/x86/resctrl.rst     | 11 ++++++++++
 arch/x86/kernel/cpu/resctrl/monitor.c  |  3 +++
 arch/x86/kernel/cpu/resctrl/rdtgroup.c | 30 ++++++++++++++++++++++++++
 3 files changed, 44 insertions(+)

Documentation/arch/x86/resctrl.rst +11 -0

index bb96b44019fe..35d908befdfb 100644
@@ -284,6 +284,17 @@ with the following files:
 	misleading values or display "Unavailable" if no counter is assigned
 	to the event.
 
+"num_mbm_cntrs":
+	The maximum number of monitoring counters (total of available and assigned
+	counters) in each domain when the system supports mbm_cntr_assign mode.
+
+	For example, on a system with maximum of 32 memory bandwidth monitoring
+	counters in each of its L3 domains:
+	::
+
+	  # cat /sys/fs/resctrl/info/L3_MON/num_mbm_cntrs
+	  0=32;1=32
+
 "max_threshold_occupancy":
 		Read/write file provides the largest value (in
 		bytes) at which a previously used LLC_occupancy

arch/x86/kernel/cpu/resctrl/monitor.c +0 -0

index 6ed7e51d3fdb..028b49878ad0 100644
@@ -1234,6 +1234,9 @@ int __init resctrl_mon_resource_init(void)
 	else if (resctrl_arch_is_mbm_total_enabled())
 		mba_mbps_default_event = QOS_L3_MBM_TOTAL_EVENT_ID;
 

arch/x86/kernel/cpu/resctrl/monitor.c int __init resctrl_mon_resource_init(void) · line 1238

Quoted 2 lines

arch/x86/kernel/cpu/resctrl/monitor.c +2 -0

+	if (r->mon.mbm_cntr_assignable)
+		resctrl_file_fflags_init("num_mbm_cntrs", RFTYPE_MON_INFO);

Missing RFTYPE_RES_CACHE?

Quoted 13 lines

arch/x86/kernel/cpu/resctrl/monitor.c +1 -0

+
 	return 0;
 }
 

arch/x86/kernel/cpu/resctrl/rdtgroup.c +0 -0

index 626be6becca7..0c9d7a702b93 100644
@@ -912,6 +912,30 @@ static int resctrl_mbm_assign_mode_show(struct kernfs_open_file *of,
 	return 0;
 }
 

arch/x86/kernel/cpu/resctrl/rdtgroup.c static int resctrl_mbm_assign_mode_show(struct kernfs_open_file *of, · line 927

Quoted 13 lines

arch/x86/kernel/cpu/resctrl/rdtgroup.c +13 -0

+static int resctrl_num_mbm_cntrs_show(struct kernfs_open_file *of,
+				      struct seq_file *s, void *v)
+{
+	struct rdt_resource *r = of->kn->parent->priv;
+	struct rdt_mon_domain *dom;
+	bool sep = false;
+
+	cpus_read_lock();
+	mutex_lock(&rdtgroup_mutex);
+
+	list_for_each_entry(dom, &r->mon_domains, hdr.list) {
+		if (sep)
+			seq_puts(s, ";");

seq_putc() can be used.

arch/x86/kernel/cpu/resctrl/rdtgroup.c static int resctrl_mbm_assign_mode_show(struct kernfs_open_file *of, · line 932

Quoted 5 lines

arch/x86/kernel/cpu/resctrl/rdtgroup.c +5 -0

+
+		seq_printf(s, "%d=%d", dom->hdr.id, r->mon.num_mbm_cntrs);
+		sep = true;
+	}
+	seq_puts(s, "\n");

seq_putc() can be used.

Quoted 13 lines

arch/x86/kernel/cpu/resctrl/rdtgroup.c +6 -0

+
+	mutex_unlock(&rdtgroup_mutex);
+	cpus_read_unlock();
+	return 0;
+}
+
 #ifdef CONFIG_PROC_CPU_RESCTRL
 
 /*

@@ -1945,6 +1969,12 @@ static struct rftype res_common_files[] = {
 		.seq_show	= resctrl_mbm_assign_mode_show,
 		.fflags		= RFTYPE_MON_INFO,

arch/x86/kernel/cpu/resctrl/rdtgroup.c static struct rftype res_common_files[] = { · line 1980

Quoted 10 lines

arch/x86/kernel/cpu/resctrl/rdtgroup.c +6 -0

 	},
+	{
+		.name		= "num_mbm_cntrs",
+		.mode		= 0444,
+		.kf_ops		= &rdtgroup_kf_single_ops,
+		.seq_show	= resctrl_num_mbm_cntrs_show,
+	},
 	{
 		.name		= "cpus",
 		.mode		= 0644,

Reinette

The ABMC feature provides an option to the user to assign a hardware counter to an RMID, event pair and monitor the bandwidth as long as the counter is assigned. The bandwidth events will be tracked by the hardware until the user changes the configuration. Each resctrl group can configure maximum two counters, one for total event and one for local event.

The ABMC feature implements an MSR L3_QOS_ABMC_CFG (C000_03FDh). Configuration is done by setting the counter id, bandwidth source (RMID) and bandwidth configuration supported by BMEC (Bandwidth Monitoring Event Configuration).

Hi Babu,

Quoted 6 lines · Babu Moger

The ABMC feature provides an option to the user to assign a hardware counter to an RMID, event pair and monitor the bandwidth as long as the counter is assigned. The bandwidth events will be tracked by the hardware until the user changes the configuration. Each resctrl group can configure maximum two counters, one for total event and one for local event.

Quoted 4 lines

The ABMC feature implements an MSR L3_QOS_ABMC_CFG (C000_03FDh). Configuration is done by setting the counter id, bandwidth source (RMID) and bandwidth configuration supported by BMEC (Bandwidth Monitoring Event Configuration).

Apart from the BMEC optimization in patch #1 and patch #2 this is the first and only mention of BMEC dependency I see in this series while I do not see implementation support for this. What am I missing?

Hi Reinette,

Quoted 9 lines · Reinette Chatre

Hi Babu,

Quoted 6 lines · Babu Moger

The ABMC feature provides an option to the user to assign a hardware counter to an RMID, event pair and monitor the bandwidth as long as the counter is assigned. The bandwidth events will be tracked by the hardware until the user changes the configuration. Each resctrl group can configure maximum two counters, one for total event and one for local event.

Quoted 9 lines

Quoted 4 lines

The ABMC feature implements an MSR L3_QOS_ABMC_CFG (C000_03FDh). Configuration is done by setting the counter id, bandwidth source (RMID) and bandwidth configuration supported by BMEC (Bandwidth Monitoring Event Configuration).

Apart from the BMEC optimization in patch #1 and patch #2 this is the first and only mention of BMEC dependency I see in this series while I do not see implementation support for this. What am I missing?

My mistake. I should have corrected it. How about this?

"The ABMC feature implements an MSR L3_QOS_ABMC_CFG (C000_03FDh). ABMC counter assignment is done by setting the counter id, bandwidth source (RMID) and bandwidth configuration. Users will have the option to change the bandwidth configuration using resctrl interface which will be introduced later in the series."

Hi Babu,

Quoted 22 lines · Babu

Hi Reinette,

Quoted 9 lines · Reinette Chatre

Hi Babu,

Quoted 6 lines · Babu Moger

The ABMC feature provides an option to the user to assign a hardware counter to an RMID, event pair and monitor the bandwidth as long as the counter is assigned. The bandwidth events will be tracked by the hardware until the user changes the configuration. Each resctrl group can configure maximum two counters, one for total event and one for local event.

Quoted 9 lines

Quoted 4 lines

The ABMC feature implements an MSR L3_QOS_ABMC_CFG (C000_03FDh). Configuration is done by setting the counter id, bandwidth source (RMID) and bandwidth configuration supported by BMEC (Bandwidth Monitoring Event Configuration).

Apart from the BMEC optimization in patch #1 and patch #2 this is the first and only mention of BMEC dependency I see in this series while I do not see implementation support for this. What am I missing?

Quoted 8 lines

My mistake. I should have corrected it. How about this?

"The ABMC feature implements an MSR L3_QOS_ABMC_CFG (C000_03FDh). ABMC counter assignment is done by setting the counter id, bandwidth source (RMID) and bandwidth configuration. Users will have the option to change the bandwidth configuration using resctrl interface which will be introduced later in the series."

Please just stick to what this patch does. The part starting with "Users will ..." can cause confusion. To support what bandwidth configuration means the description can point to existing definitions in include/linux/resctrl_types.h without needing to mention BMEC.

The ABMC feature provides an option to the user to assign a hardware counter to an RMID, event pair and monitor the bandwidth as long as it is assigned. The assigned RMID will be tracked by the hardware until the user unassigns it manually.

Implement an architecture-specific handler to assign and unassign the counter. Configure counters by writing to the L3_QOS_ABMC_CFG MSR, specifying the counter ID, bandwidth source (RMID), and event configuration.

The feature details are documented in the APM listed below [1].
[1] AMD64 Architecture Programmer's Manual Volume 2: System Programming

    Publication # 24593 Revision 3.41 section 19.3.3.3 Assignable Bandwidth
    Monitoring (ABMC).

Link https://bugzilla.kernel.org/show_bug.cgi?id=206537

Signed-off-by Babu Moger <[email protected]>

---

---
 arch/x86/kernel/cpu/resctrl/monitor.c | 39 +++++++++++++++++++++++++++
 include/linux/resctrl.h               | 15 +++++++++++
 2 files changed, 54 insertions(+)

arch/x86/kernel/cpu/resctrl/monitor.c +20 -0

index 8a88ac29d57d..77f8662dc50b 100644
@@ -1430,3 +1430,42 @@ int resctrl_arch_mbm_cntr_assign_set(struct rdt_resource *r, bool enable)
 
 	return 0;
 }
+
+static void resctrl_abmc_config_one_amd(void *info)
+{
+	union l3_qos_abmc_cfg *abmc_cfg = info;
+
+	wrmsrl(MSR_IA32_L3_QOS_ABMC_CFG, abmc_cfg->full);
+}
+
+/*
+ * Send an IPI to the domain to assign the counter to RMID, event pair.
+ */
+int resctrl_arch_config_cntr(struct rdt_resource *r, struct rdt_mon_domain *d,
+			     enum resctrl_event_id evtid, u32 rmid, u32 closid,
+			     u32 cntr_id, u32 evt_cfg, bool assign)
+{
+	struct rdt_hw_mon_domain *hw_dom = resctrl_to_arch_mon_dom(d);
+	union l3_qos_abmc_cfg abmc_cfg = { 0 };
+	struct arch_mbm_state *am;
+
+	abmc_cfg.split.cfg_en = 1;

Hi Babu,

Quoted 18 lines · Babu Moger

The ABMC feature provides an option to the user to assign a hardware counter to an RMID, event pair and monitor the bandwidth as long as it is assigned. The assigned RMID will be tracked by the hardware until the user unassigns it manually.

Implement an architecture-specific handler to assign and unassign the counter. Configure counters by writing to the L3_QOS_ABMC_CFG MSR, specifying the counter ID, bandwidth source (RMID), and event configuration.

The feature details are documented in the APM listed below [1].
[1] AMD64 Architecture Programmer's Manual Volume 2: System Programming

    Publication # 24593 Revision 3.41 section 19.3.3.3 Assignable Bandwidth
    Monitoring (ABMC).

Link https://bugzilla.kernel.org/show_bug.cgi?id=206537

Signed-off-by Babu Moger <[email protected]>

---

...

Quoted 38 lines

---
 arch/x86/kernel/cpu/resctrl/monitor.c | 39 +++++++++++++++++++++++++++
 include/linux/resctrl.h               | 15 +++++++++++
 2 files changed, 54 insertions(+)

arch/x86/kernel/cpu/resctrl/monitor.c +25 -0

index 8a88ac29d57d..77f8662dc50b 100644
@@ -1430,3 +1430,42 @@ int resctrl_arch_mbm_cntr_assign_set(struct rdt_resource *r, bool enable)
 
 	return 0;
 }
+
+static void resctrl_abmc_config_one_amd(void *info)
+{
+	union l3_qos_abmc_cfg *abmc_cfg = info;
+
+	wrmsrl(MSR_IA32_L3_QOS_ABMC_CFG, abmc_cfg->full);
+}
+
+/*
+ * Send an IPI to the domain to assign the counter to RMID, event pair.
+ */
+int resctrl_arch_config_cntr(struct rdt_resource *r, struct rdt_mon_domain *d,
+			     enum resctrl_event_id evtid, u32 rmid, u32 closid,
+			     u32 cntr_id, u32 evt_cfg, bool assign)
+{
+	struct rdt_hw_mon_domain *hw_dom = resctrl_to_arch_mon_dom(d);
+	union l3_qos_abmc_cfg abmc_cfg = { 0 };
+	struct arch_mbm_state *am;
+
+	abmc_cfg.split.cfg_en = 1;
+	abmc_cfg.split.cntr_en = assign ? 1 : 0;
+	abmc_cfg.split.cntr_id = cntr_id;
+	abmc_cfg.split.bw_src = rmid;
+	abmc_cfg.split.bw_type = evt_cfg;
+

arch/x86/kernel/cpu/resctrl/monitor.c int resctrl_arch_mbm_cntr_assign_set(struct rdt_resource *r, bool enable) · line 1462

Quoted 5 lines

arch/x86/kernel/cpu/resctrl/monitor.c +5 -0

+	smp_call_function_any(&d->hdr.cpu_mask, resctrl_abmc_config_one_amd, &abmc_cfg, 1);
+
+	/*
+	 * Reset the architectural state so that reading of hardware
+	 * counter is not considered as an overflow in next update.

Please add something like: "The hardware counter is reset (because cfg_en == 1) so there is no need to record initial non-zero counts."

include/linux/resctrl.h

Quoted 14 lines

arch/x86/kernel/cpu/resctrl/monitor.c +9 -0

+	 */
+	if (assign) {
+		am = get_arch_mbm_state(hw_dom, rmid, evtid);
+		if (am)
+			memset(am, 0, sizeof(*am));
+	}
+
+	return 0;
+}

include/linux/resctrl.h +0 -0

index 294b15de664e..60270606f1b8 100644

Please keep the declaration internal to the arch code. It can be moved when other architecture needs it.

Quoted 5 lines

include/linux/resctrl.h +0 -0

@@ -394,6 +394,21 @@ void resctrl_arch_mon_event_config_set(void *config_info);
 u32 resctrl_arch_mon_event_config_get(struct rdt_mon_domain *d,
 				      enum resctrl_event_id eventid);