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

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

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

Quoted 3 lines

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

Can you please update the CPUID database with that new bit:

    https://gitlab.com/x86-cpuid.org/x86-cpuid-db

The cache allocation and memory bandwidth allocation feature properties are consolidated into cache and membw structures respectively.

In preparation for more monitoring properties that will clobber the existing resource struct more, re-organize the monitoring specific properties to also be in a separate structure.

Suggested-by Reinette Chatre <[email protected]>

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

---

include/linux/resctrl.h +8 -1

+ * @num_rmid:		Number of RMIDs available
+ * @evt_list:		List of monitoring events
+ */
+struct resctrl_mon {
+	int			num_rmid;
+	struct list_head	evt_list;
+};
+
  /**
   * struct rdt_resource - attributes of a resctrl resource
   * @rid:		The index of the resource
   * @alloc_capable:	Is allocation available on this machine
   * @mon_capable:	Is monitor feature available on this machine
- * @num_rmid:		Number of RMIDs available
   * @ctrl_scope:		Scope of this resource for control functions
   * @mon_scope:		Scope of this resource for monitor functions
   * @cache:		Cache allocation related data

include/linux/resctrl.h +1 -1

@@ -207,11 +215,11 @@ struct rdt_resource {
  	int			rid;
  	bool			alloc_capable;
  	bool			mon_capable;
-	int			num_rmid;
  	enum resctrl_scope	ctrl_scope;
  	enum resctrl_scope	mon_scope;
  	struct resctrl_cache	cache;
  	struct resctrl_membw	membw;
+	struct resctrl_mon	mon;
  	struct list_head	ctrl_domains;
  	struct list_head	mon_domains;
  	char			*name;

Hi Babu,

Quoted 2 lines · Babu Moger

The cache allocation and memory bandwidth allocation feature properties are consolidated into cache and membw structures respectively.

"are consolidated into cache and membw structures respectively" -> "are consolidated into struct resctrl_cache and struct resctrl_membw respectively"

Quoted 9 lines

In preparation for more monitoring properties that will clobber the existing resource struct more, re-organize the monitoring specific properties to also be in a separate structure.

Suggested-by Reinette Chatre <[email protected]>

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

---

...

Quoted 5 lines

include/linux/resctrl.h +0 -0

@@ -182,12 +182,21 @@ enum resctrl_scope {
       RESCTRL_L3_NODE,
   };

  +/**
+ * struct resctrl_mon - Monitoring related data

To capture that this is not global monitoring data but instead resource specific:
"Monitoring related data" -> "Monitoring related data of a resctrl resource"

Quoted 21 lines

+ * @num_rmid:        Number of RMIDs available
+ * @evt_list:        List of monitoring events
+ */

include/linux/resctrl.h +5 -0

+struct resctrl_mon {
+    int            num_rmid;
+    struct list_head    evt_list;
+};
+

  /**
   * struct rdt_resource - attributes of a resctrl resource
   * @rid:        The index of the resource
   * @alloc_capable:    Is allocation available on this machine
   * @mon_capable:    Is monitor feature available on this machine
- * @num_rmid:        Number of RMIDs available
   * @ctrl_scope:        Scope of this resource for control functions
   * @mon_scope:        Scope of this resource for monitor functions
   * @cache:        Cache allocation related data

include/linux/resctrl.h +0 -0

@@ -199,7 +208,6 @@ enum resctrl_scope {
    * @default_ctrl:    Specifies default cache cbm or memory B/W percent.
    * @format_str:        Per resource format string to show domain value

Quoted 6 lines

(quoted diff) +0 -0

    * @parse_ctrlval:    Per resource function pointer to parse control

values
- * @evt_list:        List of monitoring events
   * @fflags:        flags to choose base and info files
   * @cdp_capable:    Is the CDP feature available on this resource */

Please add a kernel-doc entry for the new member.

Quoted 15 lines

include/linux/resctrl.h +1 -1

@@ -207,11 +215,11 @@ struct rdt_resource {
       int            rid;
       bool            alloc_capable;
       bool            mon_capable;
-    int            num_rmid;
       enum resctrl_scope    ctrl_scope;
       enum resctrl_scope    mon_scope;
       struct resctrl_cache    cache;
       struct resctrl_membw    membw;
+    struct resctrl_mon    mon;
       struct list_head    ctrl_domains;
       struct list_head    mon_domains;
       char            *name;

include/linux/resctrl.h struct rdt_resource { · line 234

Quoted 7 lines

include/linux/resctrl.h +0 -1

@@ -221,7 +229,6 @@ struct rdt_resource {
       int            (*parse_ctrlval)(struct rdt_parse_data *data,
                            struct resctrl_schema *s,
                            struct rdt_ctrl_domain *d);
-    struct list_head    evt_list;
       unsigned long        fflags;
       bool            cdp_capable;
   };

Reinette

ABMC feature details are reported via CPUID Fn8000_0020_EBX_x5. Bits Description
15:0 MAX_ABMC Maximum Supported Assignable Bandwidth
Monitoring Counter ID + 1

ABMC feature details are reported via CPUID Fn8000_0020_EBX_x5. Bits Description
15:0 MAX_ABMC Maximum Supported Assignable Bandwidth
Monitoring Counter ID + 1

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

Detect the feature and number of assignable counters supported.

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

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

---
v6: Commit message update.
Renamed abmc_capable to mbm_cntr_assignable.

v5: Name change num_cntrs to num_mbm_cntrs.
Moved abmc_capable to resctrl_mon.

v4: Removed resctrl_arch_has_abmc(). Added all the code inline. We dont
need to separate this as arch code.

v3: Removed changes related to mon_features.
Moved rdt_cpu_has to core.c and added new function resctrl_arch_has_abmc. Also moved the fields mbm_assign_capable and mbm_assign_cntrs to rdt_resource. (James)

v2: Changed the field name to mbm_assign_capable from abmc_capable.

---
  arch/x86/kernel/cpu/resctrl/monitor.c | 12 ++++++++++++
  include/linux/resctrl.h               |  4 ++++
  2 files changed, 16 insertions(+)

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

index 795fe91a8feb..88312b5f0069 100644

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

+				r->mon.num_mbm_cntrs = 64;
+		}
  	}
 
  	l3_mon_evt_init(r);

include/linux/resctrl.h +1 -0

index 1097559f4987..72c498deeb5e 100644
@@ -185,10 +185,14 @@ enum resctrl_scope {
  /**
   * struct resctrl_mon - Monitoring related data
   * @num_rmid:		Number of RMIDs available
+ * @num_mbm_cntrs:	Number of monitoring counters

Hi Babu,

Quoted 43 lines · Babu Moger

ABMC feature details are reported via CPUID Fn8000_0020_EBX_x5. Bits Description
15:0 MAX_ABMC Maximum Supported Assignable Bandwidth Monitoring Counter ID + 1

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

Detect the feature and number of assignable counters supported.

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

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

---
v6: Commit message update.
     Renamed abmc_capable to mbm_cntr_assignable.

v5: Name change num_cntrs to num_mbm_cntrs.
     Moved abmc_capable to resctrl_mon.

v4: Removed resctrl_arch_has_abmc(). Added all the code inline. We dont need to separate this as arch code.

v3: Removed changes related to mon_features.
     Moved rdt_cpu_has to core.c and added new function resctrl_arch_has_abmc.
     Also moved the fields mbm_assign_capable and mbm_assign_cntrs to rdt_resource. (James)

v2: Changed the field name to mbm_assign_capable from abmc_capable.
---
  arch/x86/kernel/cpu/resctrl/monitor.c | 12 ++++++++++++ include/linux/resctrl.h               |  4 ++++

  2 files changed, 16 insertions(+)

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

index 795fe91a8feb..88312b5f0069 100644
@@ -1229,6 +1229,18 @@ int __init rdt_get_mon_l3_config(struct
 rdt_resource *r)

arch/x86/kernel/cpu/resctrl/monitor.c int __init rdt_get_mon_l3_config(struct · line 1239

Quoted 10 lines

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

               mbm_local_event.configurable = true;
               mbm_config_rftype_init("mbm_local_bytes_config");
           }
+
+        if (rdt_cpu_has(X86_FEATURE_ABMC)) {
+            r->mon.mbm_cntr_assignable = true;
+            /*
+             * Query CPUID_Fn80000020_EBX_x05 for number of
+             * ABMC counters.
+             */

At this point this comment seems unnecessary. Not an issue, it can stay of you
prefer.

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

Quoted 3 lines

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

+            cpuid_count(0x80000020, 5, &eax, &ebx, &ecx, &edx);
+            r->mon.num_mbm_cntrs = (ebx & 0xFFFF) + 1;
+            if (WARN_ON(r->mon.num_mbm_cntrs > 64))

Please document where this "64" limit comes from. This is potentially a problem
since the resctrl fs managed bitmap is hardcoded to be of size 64 but the arch code
sets how many counters are supported. Will comment more later on bitmap portions, but
to handle this I expect resctrl fs should at least sanity check the number of counters
before attempting to initialize its bitmap ... or better, as James suggests, make the
bitmap creation dynamic.

Quoted 17 lines

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

+                r->mon.num_mbm_cntrs = 64;
+        }
       }
         l3_mon_evt_init(r);

include/linux/resctrl.h +2 -0

index 1097559f4987..72c498deeb5e 100644
@@ -185,10 +185,14 @@ enum resctrl_scope {
   /**
    * struct resctrl_mon - Monitoring related data
    * @num_rmid:        Number of RMIDs available
+ * @num_mbm_cntrs:    Number of monitoring counters
+ * @mbm_cntr_assignable:Is system capable of supporting monitor

assignment?
   * @evt_list:        List of monitoring events

Quoted 8 lines

   */
  struct resctrl_mon {
      int            num_rmid;
+    int            num_mbm_cntrs;
+    bool            mbm_cntr_assignable;
      struct list_head    evt_list;
  };

Reinette

Add the functionality to enable/disable AMD ABMC feature.

AMD ABMC feature is enabled by setting enabled bit(0) in MSR L3_QOS_EXT_CFG. When the state of ABMC is changed, the MSR needs to be updated on all the logical processors in the QOS Domain.

Hardware counters will reset when ABMC state is changed. Reset the architectural state so that reading of hardware counter is not considered as an overflow in next update.

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

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

index 2bd207624eec..154983a67646 100644

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

index 7e76f8d839fc..6075b1e5bb77 100644
@@ -2402,6 +2402,63 @@ int resctrl_arch_set_cdp_enabled(enum resctrl_res_level l, bool enable)
 
+static void _resctrl_abmc_enable(struct rdt_resource *r, bool enable)
+{
+	struct rdt_mon_domain *d;
 
 

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

+	if (r->mon.mbm_cntr_assignable && !hw_res->mbm_cntr_assign_enabled) {
+		_resctrl_abmc_enable(r, true);
+		hw_res->mbm_cntr_assign_enabled = true;
+	}

Hi Babu,

Some boring comments about where the code goes...

Quoted 24 lines

Add the functionality to enable/disable AMD ABMC feature.

AMD ABMC feature is enabled by setting enabled bit(0) in MSR L3_QOS_EXT_CFG. When the state of ABMC is changed, the MSR needs to be updated on all the logical processors in the QOS Domain.

Hardware counters will reset when ABMC state is changed. Reset the architectural state so that reading of hardware counter is not considered as an overflow in next update.

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

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

index 2bd207624eec..154983a67646 100644
@@ -536,6 +541,14 @@ int resctrl_arch_set_cdp_enabled(enum resctrl_res_level l, bool enable);
 
 void arch_mon_domain_online(struct rdt_resource *r, struct rdt_mon_domain *d);
 

arch/x86/kernel/cpu/resctrl/internal.h int resctrl_arch_set_cdp_enabled(enum resctrl_res_level l, bool enable); · line 547

Quoted 4 lines

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

+static inline bool resctrl_arch_get_abmc_enabled(void)
+{
+	return rdt_resources_all[RDT_RESOURCE_L3].mbm_cntr_assign_enabled;
+}

Once the filesystem code moves to /fs/resctrl, this can't be inlined from the architectures internal.h. Accessing rdt_resources_all[] from asm/resctrl.h isn't something that is done today... could you move this to be a non-inline function in core.c?

(this saves me moving it later!)

arch/x86/kernel/cpu/resctrl/internal.h int resctrl_arch_set_cdp_enabled(enum resctrl_res_level l, bool enable); · line 550

Quoted 2 lines

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

+int resctrl_arch_mbm_cntr_assign_enable(void);
+void resctrl_arch_mbm_cntr_assign_disable(void);

Please add these in linux/resctrl.h - it saves me moving them later!

Quoted 10 lines

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

index 7e76f8d839fc..6075b1e5bb77 100644
@@ -2402,6 +2402,63 @@ int resctrl_arch_set_cdp_enabled(enum resctrl_res_level l, bool enable)
 
+static void _resctrl_abmc_enable(struct rdt_resource *r, bool enable)
+{
+	struct rdt_mon_domain *d;

arch/x86/kernel/cpu/resctrl/rdtgroup.c int resctrl_arch_set_cdp_enabled(enum resctrl_res_level l, bool enable) · line 2431

Quoted 10 lines

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

+	/*
+	 * Hardware counters will reset after switching the monitor mode.
+	 * Reset the architectural state so that reading of hardware
+	 * counter is not considered as an overflow in the next update.
+	 */
+	list_for_each_entry(d, &r->mon_domains, hdr.list) {
+		on_each_cpu_mask(&d->hdr.cpu_mask,
+				 resctrl_abmc_set_one_amd, &enable, 1);
+		resctrl_arch_reset_rmid_all(r, d);
+	}

Is there any mileage in getting resctrl_arch_mbm_cntr_assign_enable()'s caller to do this? Every architecture that supports this will have to do this, and neither x86 nor arm64 are able to do it atomically, or quicker than calling resctrl_arch_reset_rmid_all() for each domain.

Quoted 2 lines

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

+}

arch/x86/kernel/cpu/resctrl/rdtgroup.c int resctrl_arch_set_cdp_enabled(enum resctrl_res_level l, bool enable) · line 2434

Quoted 1 line

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

+int resctrl_arch_mbm_cntr_assign_enable(void)

Could we pass the struct rdt_resource in - instead of hard coding it to be the L3 - you already check hw_res->mbm_cntr_assign_enabled so no additional check is needed...

Background: I'd like to reduce the amount of "I magically know its the L3" to reduce the work for whoever has to add monitor support for something other than the L3. (I've currently no plans - but someone is going to build it!)

Quoted 5 lines

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

+{
+	struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_L3].r_resctrl;
+	struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);

arch/x86/kernel/cpu/resctrl/rdtgroup.c int resctrl_arch_set_cdp_enabled(enum resctrl_res_level l, bool enable) · line 2421

Quoted 1 line

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

+	lockdep_assert_held(&rdtgroup_mutex);

After the split between the architecture and filesystem code - this lock is private to the filesystem. If you need to prevent concurrent enable/disable calls the architecture should take its own mutex.

| static DEFINE_MUTEX(abmc_lock);
?

Quoted 6 lines

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

+	if (r->mon.mbm_cntr_assignable && !hw_res->mbm_cntr_assign_enabled) {
+		_resctrl_abmc_enable(r, true);
+		hw_res->mbm_cntr_assign_enabled = true;
+	}

arch/x86/kernel/cpu/resctrl/rdtgroup.c int resctrl_arch_set_cdp_enabled(enum resctrl_res_level l, bool enable) · line 2454

Quoted 10 lines

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

+
+	return 0;
+}
+
+void resctrl_arch_mbm_cntr_assign_disable(void)
+{
+	struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_L3].r_resctrl;
+	struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
+
+	lockdep_assert_held(&rdtgroup_mutex);

(same plea for passing the resource in, and not referring to the filesystem's locks)

arch/x86/kernel/cpu/resctrl/rdtgroup.c int resctrl_arch_set_cdp_enabled(enum resctrl_res_level l, bool enable) · line 2460

Quoted 7 lines

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

+	if (hw_res->mbm_cntr_assign_enabled) {
+		_resctrl_abmc_enable(r, false);
+		hw_res->mbm_cntr_assign_enabled = false;
+	}
+}

The work you do in these functions is pretty symmetric. Is it worth combining them into:

| resctrl_arch_mbm_cntr_assign_set(struct rdt_resource *r, bool enable) {
|	struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
|
|	if (hw_res->mbm_cntr_assign_enabled != enable) {
|		_resctrl_abmc_enable(r, enable
|		hw_res->mbm_cntr_assign_enabled = enable;
|	}
| }

I think you need a resctrl_arch_mbm_cntr_assign_test() too - I'll comment on that patch...

Thanks,

James

Add the functionality to enable/disable AMD ABMC feature.

AMD ABMC feature is enabled by setting enabled bit(0) in MSR L3_QOS_EXT_CFG. When the state of ABMC is changed, the MSR needs to be updated on all the logical processors in the QOS Domain.

Hardware counters will reset when ABMC state is changed. Reset the

architectural state so that reading of hardware counter is not considered

as an overflow in next update.

The ABMC 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]>

---
v6: Renamed abmc_enabled to mbm_cntr_assign_enabled.
Used msr_set_bit and msr_clear_bit for msr updates.
Renamed resctrl_arch_abmc_enable() to resctrl_arch_mbm_cntr_assign_enable(). Renamed resctrl_arch_abmc_disable() to resctrl_arch_mbm_cntr_assign_disable(). Made _resctrl_abmc_enable to return void.

v5: Renamed resctrl_abmc_enable to resctrl_arch_abmc_enable.
Renamed resctrl_abmc_disable to resctrl_arch_abmc_disable. Introduced resctrl_arch_get_abmc_enabled to get abmc state from non-arch code.
Renamed resctrl_abmc_set_all to _resctrl_abmc_enable(). Modified commit log to make it clear about AMD ABMC feature.

v3: No changes.

v2: Few text changes in commit message.

---
  arch/x86/include/asm/msr-index.h       |  1 +
  arch/x86/kernel/cpu/resctrl/internal.h | 13 ++++++
  arch/x86/kernel/cpu/resctrl/rdtgroup.c | 57 ++++++++++++++++++++++++++
  3 files changed, 71 insertions(+)

arch/x86/include/asm/msr-index.h +1 -0

index 82c6a4d350e0..d86469bf5d41 100644
@@ -1182,6 +1182,7 @@
  #define MSR_IA32_MBA_BW_BASE		0xc0000200
  #define MSR_IA32_SMBA_BW_BASE		0xc0000280
  #define MSR_IA32_EVT_CFG_BASE		0xc0000400
+#define MSR_IA32_L3_QOS_EXT_CFG		0xc00003ff
 
  /* MSR_IA32_VMX_MISC bits */
  #define MSR_IA32_VMX_MISC_INTEL_PT                 (1ULL << 14)

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

index 2bd207624eec..154983a67646 100644
@@ -56,6 +56,9 @@
  /* Max event bits supported */
  #define MAX_EVT_CONFIG_BITS		GENMASK(6, 0)
 
+/* Setting bit 0 in L3_QOS_EXT_CFG enables the ABMC feature. */
+#define ABMC_ENABLE_BIT			0
+
  /**
   * cpumask_any_housekeeping() - Choose any CPU in @mask, preferring those that
   *			        aren't marked nohz_full

@@ -477,6 +480,7 @@ struct rdt_parse_data {
   * @mbm_cfg_mask:	Bandwidth sources that can be tracked when Bandwidth
   *			Monitoring Event Configuration (BMEC) is supported.
   * @cdp_enabled:	CDP state of this resource
+ * @mbm_cntr_assign_enabled:	ABMC feature is enabled
   *
   * Members of this structure are either private to the architecture
   * e.g. mbm_width, or accessed via helpers that provide abstraction. e.g.

@@ -491,6 +495,7 @@ struct rdt_hw_resource {
  	unsigned int		mbm_width;
  	unsigned int		mbm_cfg_mask;
  	bool			cdp_enabled;
+	bool			mbm_cntr_assign_enabled;
  };
 
  static inline struct rdt_hw_resource *resctrl_to_arch_res(struct rdt_resource *r)

Hi Babu,

Quoted 7 lines · Babu Moger

Add the functionality to enable/disable AMD ABMC feature.

AMD ABMC feature is enabled by setting enabled bit(0) in MSR L3_QOS_EXT_CFG.  When the state of ABMC is changed, the MSR needs to be updated on all the logical processors in the QOS Domain.

Hardware counters will reset when ABMC state is changed. Reset the

Could you please clarify how this works when ABMC state is changed on one CPU in a domain vs all (as done in this patch) CPUs of a domain? In this patch it is clear that all hardware counters are reset and consequently the architectural state maintained by resctrl is reset also. Later, when the code is added to handle CPU online I see that ABMC state is changed on a new online CPU but I do not see matching reset of architectural state. (more in that patch later)

Quoted 2 lines

architectural state so that reading of hardware counter is not considered

"architectural state" -> "architectural state maintained by resctrl"

Quoted 2 lines

as an overflow in next update.

"so that reading of hardware counter" -> "so that reading of the/a(?) hardware counter"

Quoted 78 lines

The ABMC 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]>

---
v6: Renamed abmc_enabled to mbm_cntr_assign_enabled. Used msr_set_bit and msr_clear_bit for msr updates. Renamed resctrl_arch_abmc_enable() to resctrl_arch_mbm_cntr_assign_enable().
     Renamed resctrl_arch_abmc_disable() to resctrl_arch_mbm_cntr_assign_disable().
     Made _resctrl_abmc_enable to return void.

v5: Renamed resctrl_abmc_enable to resctrl_arch_abmc_enable. Renamed resctrl_abmc_disable to resctrl_arch_abmc_disable. Introduced resctrl_arch_get_abmc_enabled to get abmc state from non-arch code.
     Renamed resctrl_abmc_set_all to _resctrl_abmc_enable(). Modified commit log to make it clear about AMD ABMC feature.

v3: No changes.

v2: Few text changes in commit message.
---
  arch/x86/include/asm/msr-index.h       |  1 + arch/x86/kernel/cpu/resctrl/internal.h | 13 ++++++ arch/x86/kernel/cpu/resctrl/rdtgroup.c | 57 ++++++++++++++++++++++++++

  3 files changed, 71 insertions(+)

arch/x86/include/asm/msr-index.h +1 -0

index 82c6a4d350e0..d86469bf5d41 100644
@@ -1182,6 +1182,7 @@
   #define MSR_IA32_MBA_BW_BASE        0xc0000200
   #define MSR_IA32_SMBA_BW_BASE        0xc0000280
   #define MSR_IA32_EVT_CFG_BASE        0xc0000400
+#define MSR_IA32_L3_QOS_EXT_CFG        0xc00003ff
     /* MSR_IA32_VMX_MISC bits */
   #define MSR_IA32_VMX_MISC_INTEL_PT                 (1ULL << 14)

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

index 2bd207624eec..154983a67646 100644
@@ -56,6 +56,9 @@
   /* Max event bits supported */
   #define MAX_EVT_CONFIG_BITS        GENMASK(6, 0)

  +/* Setting bit 0 in L3_QOS_EXT_CFG enables the ABMC feature. */

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

+#define ABMC_ENABLE_BIT            0
+

  /**
   * cpumask_any_housekeeping() - Choose any CPU in @mask, preferring those that
   *                    aren't marked nohz_full

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

@@ -477,6 +480,7 @@ struct rdt_parse_data {
    * @mbm_cfg_mask:    Bandwidth sources that can be tracked when Bandwidth
    *            Monitoring Event Configuration (BMEC) is supported.
    * @cdp_enabled:    CDP state of this resource
+ * @mbm_cntr_assign_enabled:    ABMC feature is enabled
    *
    * Members of this structure are either private to the architecture
    * e.g. mbm_width, or accessed via helpers that provide abstraction. e.g.

@@ -491,6 +495,7 @@ struct rdt_hw_resource {
       unsigned int        mbm_width;
       unsigned int        mbm_cfg_mask;
       bool            cdp_enabled;
+    bool            mbm_cntr_assign_enabled;
   };

    static inline struct rdt_hw_resource *resctrl_to_arch_res(struct rdt_resource *r)

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

@@ -536,6 +541,14 @@ int resctrl_arch_set_cdp_enabled(enum
 resctrl_res_level l, bool enable);

Quoted 3 lines

    void arch_mon_domain_online(struct rdt_resource *r, struct rdt_mon_domain *d);
  +static inline bool resctrl_arch_get_abmc_enabled(void)

This function will be called by resctrl fs code. Please contain the "abmc" naming to the
x86 architecture code and let resctrl fs just refer to it as "mbm_assign"/"mbm_cntr_assign".

Quoted 11 lines

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

+{
+    return rdt_resources_all[RDT_RESOURCE_L3].mbm_cntr_assign_enabled;
+}
+
+int resctrl_arch_mbm_cntr_assign_enable(void);
+void resctrl_arch_mbm_cntr_assign_disable(void);
+

  /*
   * To return the common struct rdt_resource, which is contained in struct
   * rdt_hw_resource, walk the resctrl member of struct rdt_hw_resource.

Reinette

Hi Reinette,

Quoted 11 lines · Reinette Chatre

Hi Babu,

Quoted 7 lines · Babu Moger

Add the functionality to enable/disable AMD ABMC feature.

AMD ABMC feature is enabled by setting enabled bit(0) in MSR L3_QOS_EXT_CFG.  When the state of ABMC is changed, the MSR needs to be updated on all the logical processors in the QOS Domain.

Hardware counters will reset when ABMC state is changed. Reset the

Quoted 7 lines

Could you please clarify how this works when ABMC state is changed on one CPU in a domain vs all (as done in this patch) CPUs of a domain? In this patch it is clear that all hardware counters are reset and consequently the architectural state maintained by resctrl is reset also. Later, when the code is added to handle CPU online I see that ABMC state is changed on a new online CPU but I do not see matching reset of architectural state. (more in that patch later)

Yes. I missed testing this scenario.
When new cpu comes online, it should inherit the abmc state which is set already. it should not force it either way. In that case, it is not required to reset the architectural state.

I need to make few changes to make it work properly.

We need to set abmc state to "enabled" during the init when abmc is detected. resctrl_late_init -> .. -> rdt_get_mon_l3_config

This only happens once during the init.

Then during the hotplug, just update the abmc state which is already set duing the init. This should work fine.

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.

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

index 30586728a4cd..d4ec605b200a 100644
@@ -257,6 +257,40 @@ with the following files:
 	    # cat /sys/fs/resctrl/info/L3_MON/mbm_local_bytes_config
 	    0=0x30;1=0x30;3=0x15;4=0x15
 
+"mbm_mode":
+	Reports the list of assignable monitoring features supported. The
+	enclosed brackets indicate which feature is enabled.
+	::
+
+	  cat /sys/fs/resctrl/info/L3_MON/mbm_mode
+	  [mbm_cntr_assign]
+	  legacy
+
+	"mbm_cntr_assign":
+		AMD's ABMC feature is one of the mbm_cntr_assign mode supported.
+		The 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

Hi Babu,

Quoted 44 lines · Babu Moger

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.

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

index 30586728a4cd..d4ec605b200a 100644
@@ -257,6 +257,40 @@ with the following files:
 	    # cat /sys/fs/resctrl/info/L3_MON/mbm_local_bytes_config
 	    0=0x30;1=0x30;3=0x15;4=0x15
 
+"mbm_mode":
+	Reports the list of assignable monitoring features supported. The
+	enclosed brackets indicate which feature is enabled.
+	::
+
+	  cat /sys/fs/resctrl/info/L3_MON/mbm_mode
+	  [mbm_cntr_assign]
+	  legacy
+
+	"mbm_cntr_assign":
+		AMD's ABMC feature is one of the mbm_cntr_assign mode supported.
+		The 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

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

Quoted 10 lines

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

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

While debugging my rebase of MPAM on top of this series, I've come back to this wording a few times to try and work out what I should expect to see ...

Is it possible to disentangle the AMD hardware feature description from the description of the filesystem behaviour this enables? You are really describing what the hardware does if you don't enable this mode...

An incomplete suggestion of the shape would be something like:

| In mbm_cntr_assign mode user-space is able to specify which control
| or monitor groups in resctrl should have a hardware counter assigned
| using the 'mbm_control' file. The number of hardware counters available
| is described in the 'num_mbm_cntrs' file.
| Changing this mode will cause all counters on a resource to reset.
|
| The feature is needed on platforms which support more control and monitor
| groups than hardware counters, meaning 'unassigned' control or monitor groups will
| report 'Unavailable' or not count all the traffic in an unpredictable way.
|
| Platforms with AMDs ABMC feature enable this mode by default so that counters
| remain assigned even when the corresponding RMID is not in use by any processor.

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

Quoted 2 lines

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

+	"Legacy":

Calling "enough hardware counters" 'legacy' is a bit curious.... 'default'?

(but I haven't worked out the benefit of disabling this mode, so maybe it doesn't need a name.)

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

Quoted 3 lines

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

+		Legacy mode works without the assignment option. The monitoring works
+		as long as there are enough RMID counters available to support number
+		of monitoring groups.

How can user-space tell this is the case? Could we be specific as to what 'works' means?

Something like:

| By default resctrl assumes each control and monitor group has a hardware counter.
| Hardware without this property will still allow more control or monitor groups
| than 'num_mbm_cntrs' to be created. Reading the mbm files may report
| 'Unavailable' if there is no hardware resource assigned.

N.B. I don't suggest referring to the num_rmid file in these as MPAM doesn't have an equivalent property.

Thanks,

James

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.

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

index 6075b1e5bb77..d8f85b20ab8f 100644

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

+			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");
+		}

Hi Babu,

(expanding on what James said)

Quoted 10 lines · Babu Moger

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.

Quoted 7 lines

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.

....

Quoted 10 lines

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

index 6075b1e5bb77..d8f85b20ab8f 100644
@@ -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,

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

Quoted 6 lines

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

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

Quoted 6 lines

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

+            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");

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

Quoted 10 lines

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

+        }
+    } else {
+        seq_puts(s, "[legacy]\n");
+    }
+
+    return 0;
+}
+
   #ifdef CONFIG_PROC_CPU_RESCTRL
     /*

Reinette

The ABMC feature provides an option to the user to assign a hardware

counter to an RMID 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.

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

---
v6: No changes.

v5: Changed the display name from num_cntrs to num_mbm_cntrs.
Updated the commit message.
Moved the patch after mbm_mode is introduced.

v4: Changed the counter name to num_cntrs. And few text changes.

v3: Changed the field name to mbm_assign_cntrs.

v2: Changed the field name to mbm_assignable_counters from abmc_counte

---
  Documentation/arch/x86/resctrl.rst     |  3 +++
  arch/x86/kernel/cpu/resctrl/monitor.c  |  2 ++
  arch/x86/kernel/cpu/resctrl/rdtgroup.c | 16 ++++++++++++++++
  3 files changed, 21 insertions(+)

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

index d4ec605b200a..fe9f10766c4f 100644
@@ -291,6 +291,9 @@ with the following files:
  		as long as there are enough RMID counters available to support number
  		of monitoring groups.
 
+"num_mbm_cntrs":
+	The number of monitoring counters available for assignment.
+
  "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 5e8706ab6361..83329cefebf7 100644

Hi Babu,

Quoted 1 line · Babu Moger

The ABMC feature provides an option to the user to assign a hardware

Here and in all patches, when referring to resctrl fs please use the more generic "mbm_assign_cntr" mode to distinguish it from the hardware/architecture
specific code that involves ABMC. Something like

"The ABMC feature provides" -> ""mbm_cntr_assign" mode provides"

I also think that being explicit with this separation will help us to see gaps in interface between resctrl fs and arch.

Quoted 2 lines

counter to an RMID and monitor the bandwidth as long as the counter is

Please clarify the scope of this feature. Above mentions that a counter is assigned to an RMID but later it is mentioned that the counter is assigned to an event. Perhaps consistently mention that a counter is assigned to a RMID,event pair?

Quoted 47 lines

assigned. Number of assignments depend on number of monitoring counters available.

Provide the interface to display the number of monitoring counters supported.

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

---
v6: No changes.

v5: Changed the display name from num_cntrs to num_mbm_cntrs. Updated the commit message.
     Moved the patch after mbm_mode is introduced.

v4: Changed the counter name to num_cntrs. And few text changes.

v3: Changed the field name to mbm_assign_cntrs.

v2: Changed the field name to mbm_assignable_counters from abmc_counte
---
  Documentation/arch/x86/resctrl.rst     |  3 +++ arch/x86/kernel/cpu/resctrl/monitor.c  |  2 ++ arch/x86/kernel/cpu/resctrl/rdtgroup.c | 16 ++++++++++++++++

  3 files changed, 21 insertions(+)

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

index d4ec605b200a..fe9f10766c4f 100644
@@ -291,6 +291,9 @@ with the following files:

          as long as there are enough RMID counters available to support number
          of monitoring groups.
  +"num_mbm_cntrs":
+    The number of monitoring counters available for assignment. +
  "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 5e8706ab6361..83329cefebf7 100644

arch/x86/kernel/cpu/resctrl/monitor.c int __init rdt_get_mon_l3_config(struct · line 1247

Quoted 6 lines

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

@@ -1242,6 +1242,8 @@ int __init rdt_get_mon_l3_config(struct
 rdt_resource *r)
               r->mon.num_mbm_cntrs = (ebx & 0xFFFF) + 1;
               if (WARN_ON(r->mon.num_mbm_cntrs > 64))
                   r->mon.num_mbm_cntrs = 64;
+
+            resctrl_file_fflags_init("num_mbm_cntrs", RFTYPE_MON_INFO);

The arch code should not access the resctrl file flags. This should be moved to make
the MPAM support easier. With the arch code setting r->mon.mbm_cntr_assignable the
fs code can use that to set the flags. Something similar to below patch is needed:
https://lore.kernel.org/lkml/[email protected]/

Quoted 30 lines · Reinette Chatre

Quoted 21 lines · Babu Moger

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

index d4ec605b200a..fe9f10766c4f 100644
@@ -291,6 +291,9 @@ with the following files:
           as long as there are enough RMID counters available to support

number
          of monitoring groups.
  +"num_mbm_cntrs":
+    The number of monitoring counters available for assignment.

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

+
   "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 5e8706ab6361..83329cefebf7 100644

arch/x86/kernel/cpu/resctrl/monitor.c int __init rdt_get_mon_l3_config(struct · line 1247

Quoted 6 lines

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

@@ -1242,6 +1242,8 @@ int __init rdt_get_mon_l3_config(struct
 rdt_resource *r)
               r->mon.num_mbm_cntrs = (ebx & 0xFFFF) + 1;
               if (WARN_ON(r->mon.num_mbm_cntrs > 64))
                   r->mon.num_mbm_cntrs = 64;
+
+            resctrl_file_fflags_init("num_mbm_cntrs", RFTYPE_MON_INFO);

Quoted 7 lines

The arch code should not access the resctrl file flags. This should be moved to make
the MPAM support easier. With the arch code setting r->mon.mbm_cntr_assignable the
fs code can use that to set the flags. Something similar to below patch is needed:
https://lore.kernel.org/lkml/[email protected]/

It is just moving the calls resctrl_file_fflags_init() to resctrl_init(). The rdt_resource fields are already setup here. Something like
https://lore.kernel.org/lkml/[email protected]/

I feel it is better done when MBAM fs/arch separation.

Hardware provides a set of counters when mbm_cntr_assignable feature is supported. These counters are used for assigning the events in resctrl group when the feature is enabled.

Introduce mbm_cntrs_free_map bitmap to track available and free counters and set of routines to allocate and free the counters.

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

index ab4fab3b7cf1..c818965e36c9 100644

Hi Babu,

Quoted 13 lines · Babu Moger

Hardware provides a set of counters when mbm_cntr_assignable feature is supported. These counters are used for assigning the events in resctrl group when the feature is enabled.

Introduce mbm_cntrs_free_map bitmap to track available and free counters and set of routines to allocate and free the counters.

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

index ab4fab3b7cf1..c818965e36c9 100644

arch/x86/kernel/cpu/resctrl/rdtgroup.c bool closid_allocated(unsigned int closid) · line 194

Quoted 10 lines

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

@@ -185,6 +185,37 @@ bool closid_allocated(unsigned int closid)
 	return !test_bit(closid, &closid_free_map);
 }
 
+/*
+ * Counter bitmap for tracking the available counters.
+ * ABMC feature provides set of hardware counters for enabling events.
+ * Each event takes one hardware counter. Kernel needs to keep track
+ * of number of available counters.
+ */
+static DECLARE_BITMAP(mbm_cntrs_free_map, 64);

Please make this resctrl limit of '64' a define in linux/resctrl.h so the arch code knows what the limit is!

MPAM platforms may have more than this - and really bad things happen if mbm_cntrs_init() passes bitmap_fill() a value greater than 64.

Even better - could we dynamically allocate this bitmap using the size advertised by the architecture code?

Hardware provides a set of counters when mbm_cntr_assignable feature is supported. These counters are used for assigning the events in resctrl group when the feature is enabled.

Introduce mbm_cntrs_free_map bitmap to track available and free counters

and set of routines to allocate and free the counters.

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

---

---
  arch/x86/kernel/cpu/resctrl/internal.h |  2 ++
  arch/x86/kernel/cpu/resctrl/rdtgroup.c | 33 ++++++++++++++++++++++++++
  2 files changed, 35 insertions(+)

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

index 154983a67646..6263362496a3 100644
@@ -662,6 +662,8 @@ void __check_limbo(struct rdt_mon_domain *d, bool force_free);
  void rdt_domain_reconfigure_cdp(struct rdt_resource *r);
  void __init resctrl_file_fflags_init(const char *config,
  				     unsigned long fflags);
+int mbm_cntr_alloc(struct rdt_resource *r);
+void mbm_cntr_free(u32 cntr_id);
  void rdt_staged_configs_clear(void);
  bool closid_allocated(unsigned int closid);
  int resctrl_find_cleanest_closid(void);

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

index ab4fab3b7cf1..c818965e36c9 100644

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

+}
+
+int mbm_cntr_alloc(struct rdt_resource *r)
+{
+	int cntr_id;
+
+	cntr_id = find_first_bit(mbm_cntrs_free_map, r->mon.num_mbm_cntrs);
+	if (cntr_id >= r->mon.num_mbm_cntrs)
+		return -ENOSPC;
+
+	__clear_bit(cntr_id, mbm_cntrs_free_map);
+
+	return cntr_id;
+}
+
+void mbm_cntr_free(u32 cntr_id)
+{
+	__set_bit(cntr_id, mbm_cntrs_free_map);
+}
+
  /**
   * rdtgroup_mode_by_closid - Return mode of resource group with closid
   * @closid: closid if the resource group

Hi Babu,

Quoted 3 lines · Babu Moger

Hardware provides a set of counters when mbm_cntr_assignable feature is supported. These counters are used for assigning the events in resctrl group when the feature is enabled.

"in resctrl group" -> "in a resctrl group"?

Quoted 2 lines

Introduce mbm_cntrs_free_map bitmap to track available and free counters

What is the difference between an available and a free counter?

Quoted 31 lines

and set of routines to allocate and free the counters.

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

---

---
  arch/x86/kernel/cpu/resctrl/internal.h |  2 ++ arch/x86/kernel/cpu/resctrl/rdtgroup.c | 33 ++++++++++++++++++++++++++

  2 files changed, 35 insertions(+)

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

index 154983a67646..6263362496a3 100644
@@ -662,6 +662,8 @@ void __check_limbo(struct rdt_mon_domain *d, bool
 force_free);
   void rdt_domain_reconfigure_cdp(struct rdt_resource *r);
   void __init resctrl_file_fflags_init(const char *config,
                        unsigned long fflags);
+int mbm_cntr_alloc(struct rdt_resource *r);
+void mbm_cntr_free(u32 cntr_id);
   void rdt_staged_configs_clear(void);
   bool closid_allocated(unsigned int closid);

  int resctrl_find_cleanest_closid(void);

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

index ab4fab3b7cf1..c818965e36c9 100644

Quoted 6 lines

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

@@ -185,6 +185,37 @@ bool closid_allocated(unsigned int closid)
       return !test_bit(closid, &closid_free_map);
   }

  +/*
+ * Counter bitmap for tracking the available counters.
+ * ABMC feature provides set of hardware counters for enabling events.

"ABMC feature" -> "mbm_cntr_assign mode"

Quoted 2 lines

+ * Each event takes one hardware counter. Kernel needs to keep track

"Each event takes one hardware counter" -> "Each RMID and event pair takes one hardware counter" ?

Quoted 3 lines

+ * of number of available counters.

"of number of available counters" -> "of the number of available counters"?

arch/x86/kernel/cpu/resctrl/rdtgroup.c bool closid_allocated(unsigned int closid) · line 198

Quoted 7 lines

+ */

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

+static DECLARE_BITMAP(mbm_cntrs_free_map, 64);
+
+static void mbm_cntrs_init(struct rdt_resource *r)
+{
+    bitmap_fill(mbm_cntrs_free_map, r->mon.num_mbm_cntrs);

Apart from what James mentioned about the different sizes, please also add checking that the resource actually supports monitoring and assignable counters before proceeding with the bitmap ops.

Quoted 30 lines

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

+}
+
+int mbm_cntr_alloc(struct rdt_resource *r)
+{
+    int cntr_id;
+
+    cntr_id = find_first_bit(mbm_cntrs_free_map, r->mon.num_mbm_cntrs);
+    if (cntr_id >= r->mon.num_mbm_cntrs)
+        return -ENOSPC;
+
+    __clear_bit(cntr_id, mbm_cntrs_free_map);
+
+    return cntr_id;
+}
+
+void mbm_cntr_free(u32 cntr_id)
+{
+    __set_bit(cntr_id, mbm_cntrs_free_map);
+}
+

  /**
   * rdtgroup_mode_by_closid - Return mode of resource group with closid
   * @closid: closid if the resource group

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

@@ -2748,6 +2779,8 @@ static int rdt_get_tree(struct fs_context *fc)
         closid_init();
   +    mbm_cntrs_init(&rdt_resources_all[RDT_RESOURCE_L3].r_resctrl);
+
       if (resctrl_arch_mon_capable())
           flags |= RFTYPE_MON;

Quoted 1 line

This is also an example of what James mentioned elsewhere where there is an assumption that this feature applies to the L3 resource. This has a consequence
that some code is global (like mbm_cntrs_free_map), assuming the L3 resource, while
other code takes the resource as parameter (eg. mbm_cntr_alloc()). This results
in inconsistent interface where, for example, allocating a counter needs

resource
as parameter but freeing a counter does not. James already proposed different treatment of the bitmap and L3 resource parameters, I expect with such guidance
the interfaces will become more intuitive.

Quoted 8 lines · Reinette Chatre

On 8/6/24 3:00 PM, Babu Moger wrote:

resource
as parameter but freeing a counter does not. James already proposed different treatment of the bitmap and L3 resource parameters, I expect with such guidance
the interfaces will become more intuitive.

How about making "mbm_cntrs_free_map" as part of struct resctrl_mon? It will be pointer and allocated dynamically based on number of counters. All the related information (num_mbm_cntrs and mbm_cntr_assignable) is already part of this data structure.

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

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

Introduce resctrl_arch_event_config_get() and resctrl_arch_event_config_set() to get/set architecture domain specific mbm_total_cfg/mbm_local_cfg values. Also, remove unused config value definitions.

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

---
v6: Fixed inconstancy with types. Made all the types to u32 for config
value.
Removed few rdt_last_cmd_puts as it is not necessary. Removed unused config value definitions.
Few more updates to commit message.

v5: Introduced resctrl_arch_event_config_get and
resctrl_arch_event_config_get() based on our discussion.
https://lore.kernel.org/lkml/[email protected]/

v4: New patch.

---
  arch/x86/kernel/cpu/resctrl/internal.h |  21 -----
  arch/x86/kernel/cpu/resctrl/rdtgroup.c | 104 ++++++++++++++-----------
  include/linux/resctrl.h                |   4 +
  3 files changed, 64 insertions(+), 65 deletions(-)

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

index 4d8cc36a8d79..1021227d8c7e 100644
@@ -32,27 +32,6 @@
   */
  #define MBM_CNTR_WIDTH_OFFSET_MAX (62 - MBM_CNTR_WIDTH_BASE)
 
-/* Reads to Local DRAM Memory */
-#define READS_TO_LOCAL_MEM		BIT(0)
-
-/* Reads to Remote DRAM Memory */
-#define READS_TO_REMOTE_MEM		BIT(1)
-
-/* Non-Temporal Writes to Local Memory */
-#define NON_TEMP_WRITE_TO_LOCAL_MEM	BIT(2)
-
-/* Non-Temporal Writes to Remote Memory */
-#define NON_TEMP_WRITE_TO_REMOTE_MEM	BIT(3)
-
-/* Reads to Local Memory the system identifies as "Slow Memory" */
-#define READS_TO_LOCAL_S_MEM		BIT(4)
-
-/* Reads to Remote Memory the system identifies as "Slow Memory" */
-#define READS_TO_REMOTE_S_MEM		BIT(5)
-
-/* Dirty Victims to All Types of Memory */
-#define DIRTY_VICTIMS_TO_ALL_MEM	BIT(6)
-
  /* Max event bits supported */
  #define MAX_EVT_CONFIG_BITS		GENMASK(6, 0)
 

arch/x86/kernel/cpu/resctrl/rdtgroup.c +48 -23

index 02afd3442876..0047b4eb0ff5 100644
@@ -1605,10 +1605,57 @@ static int rdtgroup_size_show(struct kernfs_open_file *of,
  }
 
  struct mon_config_info {
+	struct rdt_mon_domain *d;
  	u32 evtid;
  	u32 mon_config;
  };
 
+u32 resctrl_arch_event_config_get(struct rdt_mon_domain *d,
+				  enum resctrl_event_id eventid)
+{
+	struct rdt_hw_mon_domain *hw_dom = resctrl_to_arch_mon_dom(d);
+
+	switch (eventid) {
+	case QOS_L3_OCCUP_EVENT_ID:
+		break;
+	case QOS_L3_MBM_TOTAL_EVENT_ID:
+		return hw_dom->mbm_total_cfg;
+	case QOS_L3_MBM_LOCAL_EVENT_ID:
+		return hw_dom->mbm_local_cfg;
+	}
+
+	/* Never expect to get here */
+	WARN_ON_ONCE(1);
+
+	return INVALID_CONFIG_VALUE;
+}
+
+void resctrl_arch_event_config_set(void *info)
+{
+	struct mon_config_info *mon_info = info;
+	struct rdt_hw_mon_domain *hw_dom;
+	unsigned int index;
+
+	index = mon_event_config_index_get(mon_info->evtid);
+	if (index == INVALID_CONFIG_INDEX)
+		return;
+
+	wrmsr(MSR_IA32_EVT_CFG_BASE + index, mon_info->mon_config, 0);
+
+	hw_dom = resctrl_to_arch_mon_dom(mon_info->d);
+
+	switch (mon_info->evtid) {
+	case QOS_L3_OCCUP_EVENT_ID:
+		break;
+	case QOS_L3_MBM_TOTAL_EVENT_ID:
+		hw_dom->mbm_total_cfg = mon_info->mon_config;
+		break;
+	case QOS_L3_MBM_LOCAL_EVENT_ID:
+		hw_dom->mbm_local_cfg =  mon_info->mon_config;
+		break;
+	}
+}
+
  /**
   * mon_event_config_index_get - get the hardware index for the
   *                              configurable event

@@ -1631,33 +1678,11 @@ unsigned int mon_event_config_index_get(u32 evtid)
  	}
  }
 
-static void mon_event_config_read(void *info)
-{
-	struct mon_config_info *mon_info = info;
-	unsigned int index;
-	u64 msrval;
-
-	index = mon_event_config_index_get(mon_info->evtid);
-	if (index == INVALID_CONFIG_INDEX) {
-		pr_warn_once("Invalid event id %d\n", mon_info->evtid);
-		return;
-	}
-	rdmsrl(MSR_IA32_EVT_CFG_BASE + index, msrval);
-
-	/* Report only the valid event configuration bits */
-	mon_info->mon_config = msrval & MAX_EVT_CONFIG_BITS;
-}
-
-static void mondata_config_read(struct rdt_mon_domain *d, struct mon_config_info *mon_info)
-{
-	smp_call_function_any(&d->hdr.cpu_mask, mon_event_config_read, mon_info, 1);
-}
-
  static int mbm_config_show(struct seq_file *s, struct rdt_resource *r, u32 evtid)
  {
-	struct mon_config_info mon_info = {0};
  	struct rdt_mon_domain *dom;
  	bool sep = false;
+	u32 val;
 
  	cpus_read_lock();
  	mutex_lock(&rdtgroup_mutex);

Hi Babu,

Quoted 2 lines · Babu Moger

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

"The values is stored in rdt_hw_mon_domain." -> "The values are stored in struct rdt_hw_mon_domain."

Quoted 4 lines

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

"rdt_hw_mon_domain" -> "struct rdt_hw_mon_domain"

Quoted 6 lines

Introduce resctrl_arch_event_config_get() and resctrl_arch_event_config_set() to get/set architecture domain specific mbm_total_cfg/mbm_local_cfg values. Also, remove unused config value definitions.

hmmm ... while the config values are not used they are now established ABI and any other architecture that wants to support configurable events will need to follow these definitions. It is thus required to keep them documented in the kernel in support of future changes. I understand that they are documented in user docs, but could we keep them in the kernel code also? Since they are unused they could perhaps be moved to comments as a compromise?

Quoted 152 lines

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

---
v6: Fixed inconstancy with types. Made all the types to u32 for config value.
     Removed few rdt_last_cmd_puts as it is not necessary. Removed unused config value definitions.
     Few more updates to commit message.

v5: Introduced resctrl_arch_event_config_get and resctrl_arch_event_config_get() based on our discussion.
https://lore.kernel.org/lkml/[email protected]/

v4: New patch.
---
  arch/x86/kernel/cpu/resctrl/internal.h |  21 ----- arch/x86/kernel/cpu/resctrl/rdtgroup.c | 104 ++++++++++++++----------- include/linux/resctrl.h                |   4 +

  3 files changed, 64 insertions(+), 65 deletions(-)

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

index 4d8cc36a8d79..1021227d8c7e 100644
@@ -32,27 +32,6 @@
    */
   #define MBM_CNTR_WIDTH_OFFSET_MAX (62 - MBM_CNTR_WIDTH_BASE)

  -/* Reads to Local DRAM Memory */

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

-#define READS_TO_LOCAL_MEM        BIT(0)
-
-/* Reads to Remote DRAM Memory */
-#define READS_TO_REMOTE_MEM        BIT(1)
-
-/* Non-Temporal Writes to Local Memory */
-#define NON_TEMP_WRITE_TO_LOCAL_MEM    BIT(2)
-
-/* Non-Temporal Writes to Remote Memory */
-#define NON_TEMP_WRITE_TO_REMOTE_MEM    BIT(3)
-
-/* Reads to Local Memory the system identifies as "Slow Memory" */
-#define READS_TO_LOCAL_S_MEM        BIT(4)
-
-/* Reads to Remote Memory the system identifies as "Slow Memory" */
-#define READS_TO_REMOTE_S_MEM        BIT(5)
-
-/* Dirty Victims to All Types of Memory */
-#define DIRTY_VICTIMS_TO_ALL_MEM    BIT(6)
-

  /* Max event bits supported */
  #define MAX_EVT_CONFIG_BITS        GENMASK(6, 0) diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c index 02afd3442876..0047b4eb0ff5 100644

arch/x86/kernel/cpu/resctrl/rdtgroup.c +47 -22

@@ -1605,10 +1605,57 @@ static int rdtgroup_size_show(struct
 kernfs_open_file *of,
   }
     struct mon_config_info {
+    struct rdt_mon_domain *d;
       u32 evtid;
       u32 mon_config;
   };
   +u32 resctrl_arch_event_config_get(struct rdt_mon_domain *d,
+                  enum resctrl_event_id eventid)
+{
+    struct rdt_hw_mon_domain *hw_dom = resctrl_to_arch_mon_dom(d);
+
+    switch (eventid) {
+    case QOS_L3_OCCUP_EVENT_ID:
+        break;
+    case QOS_L3_MBM_TOTAL_EVENT_ID:
+        return hw_dom->mbm_total_cfg;
+    case QOS_L3_MBM_LOCAL_EVENT_ID:
+        return hw_dom->mbm_local_cfg;
+    }
+
+    /* Never expect to get here */
+    WARN_ON_ONCE(1);
+
+    return INVALID_CONFIG_VALUE;
+}
+
+void resctrl_arch_event_config_set(void *info)
+{
+    struct mon_config_info *mon_info = info;
+    struct rdt_hw_mon_domain *hw_dom;
+    unsigned int index;
+
+    index = mon_event_config_index_get(mon_info->evtid);
+    if (index == INVALID_CONFIG_INDEX)
+        return;
+
+    wrmsr(MSR_IA32_EVT_CFG_BASE + index, mon_info->mon_config, 0);
+
+    hw_dom = resctrl_to_arch_mon_dom(mon_info->d);
+
+    switch (mon_info->evtid) {
+    case QOS_L3_OCCUP_EVENT_ID:
+        break;
+    case QOS_L3_MBM_TOTAL_EVENT_ID:
+        hw_dom->mbm_total_cfg = mon_info->mon_config;
+        break;
+    case QOS_L3_MBM_LOCAL_EVENT_ID:
+        hw_dom->mbm_local_cfg =  mon_info->mon_config;
+        break;
+    }
+}
+
   /**
    * mon_event_config_index_get - get the hardware index for the
    *                              configurable event

@@ -1631,33 +1678,11 @@ unsigned int mon_event_config_index_get(u32 evtid)
       }
   }
   -static void mon_event_config_read(void *info)
-{
-    struct mon_config_info *mon_info = info;
-    unsigned int index;
-    u64 msrval;
-
-    index = mon_event_config_index_get(mon_info->evtid);
-    if (index == INVALID_CONFIG_INDEX) {
-        pr_warn_once("Invalid event id %d\n", mon_info->evtid);
-        return;
-    }
-    rdmsrl(MSR_IA32_EVT_CFG_BASE + index, msrval);
-
-    /* Report only the valid event configuration bits */
-    mon_info->mon_config = msrval & MAX_EVT_CONFIG_BITS;
-}
-
-static void mondata_config_read(struct rdt_mon_domain *d, struct
 mon_config_info *mon_info)
-{
-    smp_call_function_any(&d->hdr.cpu_mask, mon_event_config_read,
 mon_info, 1);
-}
-
   static int mbm_config_show(struct seq_file *s, struct rdt_resource *r,
 u32 evtid)
   {
-    struct mon_config_info mon_info = {0};
       struct rdt_mon_domain *dom;
       bool sep = false;
+    u32 val;

        cpus_read_lock();
      mutex_lock(&rdtgroup_mutex);

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

@@ -1666,11 +1691,11 @@ static int mbm_config_show(struct seq_file *s,

Quoted 8 lines

struct rdt_resource *r, u32 evtid
          if (sep)
              seq_puts(s, ";");
  -        memset(&mon_info, 0, sizeof(struct mon_config_info)); -        mon_info.evtid = evtid;
-        mondata_config_read(dom, &mon_info);
+        val = resctrl_arch_event_config_get(dom, evtid); +        if (val == INVALID_CONFIG_VALUE)

Can this check and the "break" that follows be dropped? val being INVALID_CONFIG_VALUE would be a kernel bug and resctrl_arch_event_config_get()
would already have printed the WARN. In this unlikely scenario I find it unexpected that mbm_config_show() will return success in this case and the below seq_printf() would handle the printing of INVALID_CONFIG_VALUE without issue anyway.

Quoted 7 lines

+            break;
  -        seq_printf(s, "%d=0x%02x", dom->hdr.id, mon_info.mon_config); +        seq_printf(s, "%d=0x%02x", dom->hdr.id, val);
          sep = true;
      }
      seq_puts(s, "\n");

Reinette

Quoted 7 lines · Reinette Chatre

Quoted 4 lines · Babu Moger

Introduce resctrl_arch_event_config_get() and resctrl_arch_event_config_set() to get/set architecture domain specific mbm_total_cfg/mbm_local_cfg values. Also, remove unused config value definitions.

Quoted 7 lines

hmmm ... while the config values are not used they are now established ABI and any other architecture that wants to support configurable events will need to follow these definitions. It is thus required to keep them documented in the kernel in support of future changes. I understand that they are documented in user docs, but could we keep them in the kernel code also? Since they are unused they could perhaps be moved to comments as a compromise?

How about just keeping them as is? I will just not remove it.

The MBM counters are allocated at resctrl group level. It is tracked by

mbm_cntrs_free_map. Then it is assigned to the domain based on the user input. It needs to be tracked at domain level also.

Add the mbm_cntr_map bitmap in rdt_mon_domain structure to keep track of

assignment at domain level. The global counter at mbm_cntrs_free_map can be released when assignment at all the domain are cleared.

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

---
v6: New patch to add domain level assignment.

---
  arch/x86/kernel/cpu/resctrl/rdtgroup.c | 10 ++++++++++
  include/linux/resctrl.h                |  2 ++
  2 files changed, 12 insertions(+)

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

index 0047b4eb0ff5..1a90c671a027 100644
@@ -4127,6 +4127,7 @@ static void __init rdtgroup_setup_default(void)
 
  static void domain_destroy_mon_state(struct rdt_mon_domain *d)
  {
+	bitmap_free(d->mbm_cntr_map);
  	bitmap_free(d->rmid_busy_llc);
  	kfree(d->mbm_total);
  	kfree(d->mbm_local);

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

+		d->mbm_cntr_map = bitmap_zalloc(r->mon.num_mbm_cntrs, GFP_KERNEL);
+		if (!d->mbm_cntr_map) {
+			bitmap_free(d->rmid_busy_llc);
+			kfree(d->mbm_total);
+			kfree(d->mbm_local);
+			return -ENOMEM;
+		}
+	}
 
  	return 0;
  }

include/linux/resctrl.h +1 -0

index ef08f75191f2..034fa994e84f 100644
@@ -105,6 +105,7 @@ struct rdt_ctrl_domain {
   * @cqm_limbo:		worker to periodically read CQM h/w counters
   * @mbm_work_cpu:	worker CPU for MBM h/w counters
   * @cqm_work_cpu:	worker CPU for CQM h/w counters
+ * @mbm_cntr_map:	bitmap to track domain counter assignment
   */
  struct rdt_mon_domain {
  	struct rdt_domain_hdr		hdr;

Hi Babu,

Quoted 1 line · Babu Moger

The MBM counters are allocated at resctrl group level. It is tracked by

Are they not allocated globally? (but maybe that is about to change?

Quoted 3 lines

mbm_cntrs_free_map. Then it is assigned to the domain based on the user input. It needs to be tracked at domain level also.

Please elaborate why it needs to be tracked at domain level.

Quoted 3 lines

Add the mbm_cntr_map bitmap in rdt_mon_domain structure to keep track of

"rdt_mon_domain structure" -> "struct rdt_mon_domain"

Quoted 3 lines

assignment at domain level. The global counter at mbm_cntrs_free_map can be released when assignment at all the domain are cleared.

"all the domain" -> "all the domains"?

Quoted 23 lines

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

---
v6: New patch to add domain level assignment.
---
  arch/x86/kernel/cpu/resctrl/rdtgroup.c | 10 ++++++++++ include/linux/resctrl.h                |  2 ++

  2 files changed, 12 insertions(+)

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

index 0047b4eb0ff5..1a90c671a027 100644
@@ -4127,6 +4127,7 @@ static void __init rdtgroup_setup_default(void)
     static void domain_destroy_mon_state(struct rdt_mon_domain *d)
   {
+    bitmap_free(d->mbm_cntr_map);
       bitmap_free(d->rmid_busy_llc);
       kfree(d->mbm_total);
       kfree(d->mbm_local);

@@ -4200,6 +4201,15 @@ static int domain_setup_mon_state(struct

rdt_resource *r, struct rdt_mon_domain

Quoted 4 lines

              return -ENOMEM;
          }
      }
+    if (is_mbm_enabled()) {

This should also depend on whether the resource supports counter assignment, and that it
is enabled to ensure that r->mon.num_mbm_cntrs is valid.

Quoted 24 lines

+        d->mbm_cntr_map = bitmap_zalloc(r->mon.num_mbm_cntrs, GFP_KERNEL); +        if (!d->mbm_cntr_map) {
+            bitmap_free(d->rmid_busy_llc);
+            kfree(d->mbm_total);
+            kfree(d->mbm_local);
+            return -ENOMEM;
+        }
+    }
        return 0;
  }

include/linux/resctrl.h +1 -0

index ef08f75191f2..034fa994e84f 100644
@@ -105,6 +105,7 @@ struct rdt_ctrl_domain {
    * @cqm_limbo:        worker to periodically read CQM h/w counters
    * @mbm_work_cpu:    worker CPU for MBM h/w counters
    * @cqm_work_cpu:    worker CPU for CQM h/w counters
+ * @mbm_cntr_map:    bitmap to track domain counter assignment
    */
   struct rdt_mon_domain {
       struct rdt_domain_hdr        hdr;

include/linux/resctrl.h struct rdt_mon_domain { · line 122

Quoted 6 lines

include/linux/resctrl.h +1 -0

@@ -116,6 +117,7 @@ struct rdt_mon_domain {
       struct delayed_work        cqm_limbo;
       int                mbm_work_cpu;
       int                cqm_work_cpu;
+    unsigned long            *mbm_cntr_map;
   };
     /**

Reinette

The ABMC feature provides an option to the user to assign a hardware counter to an RMID 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 a pair of MSRs, L3_QOS_ABMC_CFG (C000_03FDh) and L3_QOS_ABMC_DSC (C000_3FEh). The counters are configured by writing to MSR L3_QOS_ABMC_CFG. Configuration is done by setting the counter id, bandwidth source (RMID) and bandwidth configuration supported by BMEC (Bandwidth Monitoring Event Configuration).

L3_QOS_ABMC_DSC is a read-only MSR. Reading L3_QOS_ABMC_DSC returns the configuration of the counter id specified in L3_QOS_ABMC_CFG.cntr_id with rmid(bw_src) and event configuration(bw_type).

Attempts to read or write these MSRs when ABMC is not enabled will result in a #GP(0) exception.

Introduce data structures and definitions for ABMC MSRs.

MSR L3_QOS_ABMC_CFG (0xC000_03FDh) and L3_QOS_ABMC_DSC (0xC000_03FEh) details.

=========================================================================
Bits Mnemonic Description Access Reset
Type Value
=========================================================================
63 CfgEn Configuration Enable R/W 0

62 CtrEn Enable/disable Tracking R/W 0

61:53 – Reserved MBZ 0

52:48 CtrID Counter Identifier R/W 0

47 IsCOS BwSrc field is a CLOSID R/W 0
(not an RMID)

46:44 – Reserved MBZ 0

43:32 BwSrc Bandwidth Source R/W 0
(RMID or CLOSID)

31:0 BwType Bandwidth configuration R/W 0
to track for this counter
==========================================================================

Configuration and tracking:
CfgEn=1,CtrEn=0 : Configure CtrID and but no tracking the events yet. CfgEn=1,CtrEn=1 : Configure CtrID and start tracking events.

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

---
v6: Removed all the fs related changes.
Added note on CfgEn,CtrEn.
Removed the definitions which are not used.
Removed cntr_id initialization.

v5: Moved assignment flags here (path 10/19 of v4).
Added MON_CNTR_UNSET definition to initialize cntr_id's. More details in commit log.
Renamed few fields in l3_qos_abmc_cfg for readability.

v4: Added more descriptions.
Changed the name abmc_ctr_id to ctr_id.
Added L3_QOS_ABMC_DSC. Used for reading the configuration.

v3: No changes.

v2: No changes.

---
  arch/x86/include/asm/msr-index.h       |  2 ++
  arch/x86/kernel/cpu/resctrl/internal.h | 26 ++++++++++++++++++++++++++
  2 files changed, 28 insertions(+)

arch/x86/include/asm/msr-index.h +2 -0

index d86469bf5d41..5b3931a59d5a 100644
@@ -1183,6 +1183,8 @@
  #define MSR_IA32_SMBA_BW_BASE		0xc0000280
  #define MSR_IA32_EVT_CFG_BASE		0xc0000400
  #define MSR_IA32_L3_QOS_EXT_CFG		0xc00003ff
+#define MSR_IA32_L3_QOS_ABMC_CFG	0xc00003fd
+#define MSR_IA32_L3_QOS_ABMC_DSC	0xc00003fe
 
  /* MSR_IA32_VMX_MISC bits */
  #define MSR_IA32_VMX_MISC_INTEL_PT                 (1ULL << 14)

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

index 1021227d8c7e..af3efa35a62e 100644
@@ -589,6 +589,32 @@ union cpuid_0x10_x_edx {
  	unsigned int full;
  };
 
+/*
+ * ABMC counters can be configured by writing to L3_QOS_ABMC_CFG.
+ * @bw_type		: Bandwidth configuration(supported by BMEC)
+ *			  tracked by the @cntr_id.
+ * @bw_src		: Bandwidth source (RMID or CLOSID).
+ * @reserved1		: Reserved.
+ * @is_clos		: @bw_src field is a CLOSID (not an RMID).
+ * @cntr_id		: Counter identifier.
+ * @reserved		: Reserved.
+ * @cntr_en		: Tracking enable bit.
+ * @cfg_en		: Configuration enable bit.
+ */
+union l3_qos_abmc_cfg {
+	struct {
+		unsigned long	bw_type	:32,
+				bw_src	:12,