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

resctrl_late_init() has the __init attribute, but some of the functions called from it do not have the __init attribute.

Add the __init attribute to all the functions in the call sequences to maintain consistency throughout.

Fixes 6a445edce657 ("x86/intel_rdt/cqm: Add RDT monitoring initialization")

Fixes def10853930a ("x86/intel_rdt: Add two new resources for L2 Code and Data Prioritization (CDP)")

Fixes bd334c86b5d7 ("x86/resctrl: Add __init attribute to rdt_get_mon_l3_config()")

Suggested-by Reinette Chatre <[email protected]>

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

---

resctrl_late_init() has the __init attribute, but some of the functions called from it do not have the __init attribute.

Add the __init attribute to all the functions in the call sequences to maintain consistency throughout.

Fixes 6a445edce657 ("x86/intel_rdt/cqm: Add RDT monitoring initialization")

Fixes def10853930a ("x86/intel_rdt: Add two new resources for L2 Code and Data Prioritization (CDP)")

Fixes bd334c86b5d7 ("x86/resctrl: Add __init attribute to rdt_get_mon_l3_config()")

Suggested-by Reinette Chatre <[email protected]>

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

---

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

index 3d1735ed8d1f..f0a331287979 100644
@@ -145,7 +145,7 @@ u32 resctrl_arch_system_num_rmid_idx(void)
  * is always 20 on hsw server parts. The minimum cache bitmask length
  * allowed for HSW server is always 2 bits. Hardcode all of them.
  */
-static inline void cache_alloc_hsw_probe(void)
+static inline __init void cache_alloc_hsw_probe(void)
 {
 	struct rdt_hw_resource *hw_res = &rdt_resources_all[RDT_RESOURCE_L3];
 	struct rdt_resource *r  = &hw_res->r_resctrl;

@@ -277,7 +277,7 @@ static __init bool __rdt_get_mem_config_amd(struct rdt_resource *r)

Hi,

Quoted 5 lines · Babu Moger

resctrl_late_init() has the __init attribute, but some of the functions called from it do not have the __init attribute.

Add the __init attribute to all the functions in the call sequences to maintain consistency throughout.

(BTW, did you just find these cases by inspection, or were you getting build warnings?

Even with CONFIG_DEBUG_SECTION_MISMATCH=y, I struggle to get build warnings about section mismatches on inlined functions. Even building with -fno-inline doesn't flag them all up (though I don't think this suppresses all inlining).

If you have a way of tracking these cases down automatically, I'd be interested to know so that I can apply it elsewhere.)

Cheers
---Dave

Quoted 7 lines

Fixes 6a445edce657 ("x86/intel_rdt/cqm: Add RDT monitoring initialization")

Fixes def10853930a ("x86/intel_rdt: Add two new resources for L2 Code and Data Prioritization (CDP)")

Fixes bd334c86b5d7 ("x86/resctrl: Add __init attribute to rdt_get_mon_l3_config()")

Suggested-by Reinette Chatre <[email protected]>

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

---

[...]

Quoted 14 lines

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

index 3d1735ed8d1f..f0a331287979 100644
@@ -145,7 +145,7 @@ u32 resctrl_arch_system_num_rmid_idx(void)
  * is always 20 on hsw server parts. The minimum cache bitmask length
  * allowed for HSW server is always 2 bits. Hardcode all of them.
  */
-static inline void cache_alloc_hsw_probe(void)
+static inline __init void cache_alloc_hsw_probe(void)
 {
 	struct rdt_hw_resource *hw_res = &rdt_resources_all[RDT_RESOURCE_L3];
 	struct rdt_resource *r  = &hw_res->r_resctrl;

@@ -277,7 +277,7 @@ static __init bool __rdt_get_mem_config_amd(struct rdt_resource *r)

[...]

Hi Dave,

Quoted 12 lines · Dave Martin

Hi,

Quoted 5 lines · Babu Moger

resctrl_late_init() has the __init attribute, but some of the functions called from it do not have the __init attribute.

Add the __init attribute to all the functions in the call sequences to maintain consistency throughout.

(BTW, did you just find these cases by inspection, or were you getting build warnings?

Quoted 7 lines

Even with CONFIG_DEBUG_SECTION_MISMATCH=y, I struggle to get build warnings about section mismatches on inlined functions. Even building with -fno-inline doesn't flag them all up (though I don't think this suppresses all inlining).

If you have a way of tracking these cases down automatically, I'd be interested to know so that I can apply it elsewhere.)

It is mostly by code inspection at this point.

You can refer to this commit [1].

We used to see section mismatch warnings when non-init functions call __init functions.

MODPOST Module.symvers
WARNING: modpost: vmlinux: section mismatch in reference:
rdt_get_mon_l3_config+0x2b5 (section: .text) -> rdt_cpu_has (section:
.init.text)
WARNING: modpost: vmlinux: section mismatch in reference:
rdt_get_mon_l3_config+0x408 (section: .text) -> rdt_cpu_has (section:
.init.text)

1.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v6.14-rc3&id=bd334c86b5d70e5d1c6169991802e62c828d6f38

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.

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

---

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

index c3d7d4c3009a..a7526306f5e4 100644
@@ -1261,3 +1261,39 @@ void __init intel_rdt_mbm_apply_quirk(void)
 	mbm_cf_rmidthreshold = mbm_cf_table[cf_index].rmidthreshold;
 	mbm_cf = mbm_cf_table[cf_index].cf;
 }
+
+static void resctrl_abmc_set_one_amd(void *arg)
+{
+	bool *enable = arg;

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

+ */
+static void _resctrl_abmc_enable(struct rdt_resource *r, bool enable)
+{
+	struct rdt_mon_domain *d;
+
+	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);

Hi Babu,

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

Quoted 1 line

Hardware counters will reset when ABMC state is changed.

I find that the state management in this series is organized better and easier to understand. I do think that it can be simplified more and a hint to this is that it is mentioned here but not done in the code introduced here but instead required from the caller. It seems simpler to me that the architectural state can just be reset at the same time as enable/disable of ABMC?

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

---

...

Quoted 19 lines

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

index c3d7d4c3009a..a7526306f5e4 100644
@@ -1261,3 +1261,39 @@ void __init intel_rdt_mbm_apply_quirk(void)
  	mbm_cf_rmidthreshold = mbm_cf_table[cf_index].rmidthreshold;
  	mbm_cf = mbm_cf_table[cf_index].cf;
  }
+
+static void resctrl_abmc_set_one_amd(void *arg)
+{
+	bool *enable = arg;
+
+	if (*enable)
+		msr_set_bit(MSR_IA32_L3_QOS_EXT_CFG, ABMC_ENABLE_BIT);
+	else
+		msr_clear_bit(MSR_IA32_L3_QOS_EXT_CFG, ABMC_ENABLE_BIT);
+}
+

arch/x86/kernel/cpu/resctrl/monitor.c void __init intel_rdt_mbm_apply_quirk(void) · line 1277

Quoted 3 lines

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

+/*
+ * Update L3_QOS_EXT_CFG MSR on all the CPUs associated with the monitor
+ * domain.

All monitor domains are impacted and above does not clearly state "why". How about
* ABMC enable/disable requires update of L3_QOS_EXT_CFG MSR on all the CPUs
* associated with all monitor domains.

Quoted 16 lines

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

+ */
+static void _resctrl_abmc_enable(struct rdt_resource *r, bool enable)
+{
+	struct rdt_mon_domain *d;
+
+	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);
+}
+
+int resctrl_arch_mbm_cntr_assign_set(struct rdt_resource *r, bool enable)
+{
+	struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
+

arch/x86/kernel/cpu/resctrl/monitor.c void __init intel_rdt_mbm_apply_quirk(void) · line 1295

Quoted 4 lines

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

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

Added benefit of resetting architectural state within this if statement (perhaps simpler to be done within _resctrl_abmc_enable()) is that it will not be done unnecessarily if ABMC is already in requested state.

arch/x86/kernel/cpu/resctrl/monitor.c void __init intel_rdt_mbm_apply_quirk(void) · line 1299

Quoted 5 lines

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

+	}
+
+	return 0;
+}

Reinette

Hi Reinette,

Quoted 9 lines · Reinette Chatre

Hi Babu,

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

Quoted 8 lines

Quoted 1 line

Hardware counters will reset when ABMC state is changed.

I find that the state management in this series is organized better and easier to understand. I do think that it can be simplified more and a hint to this is that it is mentioned here but not done in the code introduced here but instead required from the caller. It seems simpler to me that the architectural state can just be reset at the same time as enable/disable of ABMC?

Right now, it is done from mbm_cntr_reset(). It does both arch and non-arch state reset for all the RMIDs in all the domains. It is called in two places.

1 rdtgroup.c resctrl_mbm_assign_mode_write -> mbm_cntr_reset();

2 rdtgroup.c rdt_kill_sb()-> mbm_cntr_reset();

I will have to introduce another function to reset RMIDs in all the domains. Also, make sure it is called from both these places.

list_for_each_entry(dom, &r->mon_domains, hdr.list) resctrl_arch_reset_rmid_all(r, dom);

I feel current code is much more cleaner.  What do you think?

Quoted 14 lines

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

---

...

Quoted 14 lines

Quoted 14 lines

+ */

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

+static void _resctrl_abmc_enable(struct rdt_resource *r, bool enable)
+{
+    struct rdt_mon_domain *d;
+
+    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);
+}
+
+int resctrl_arch_mbm_cntr_assign_set(struct rdt_resource *r, bool enable)
+{
+    struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
+

Quoted 8 lines

Quoted 4 lines

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

Added benefit of resetting architectural state within this if statement (perhaps simpler to be done within _resctrl_abmc_enable()) is that it will not be done unnecessarily if ABMC is already in requested state.

It will be
      list_for_each_entry(dom, &r->mon_domains, hdr.list) resctrl_arch_reset_rmid_all(r, dom);

Hi Babu,

Quoted 21 lines · Babu

Hi Reinette,

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

I find that the state management in this series is organized better and easier to understand. I do think that it can be simplified more and a hint to this is that it is mentioned here but not done in the code introduced here but instead required from the caller. It seems simpler to me that the architectural state can just be reset at the same time as enable/disable of ABMC?

Quoted 3 lines

Right now, it is done from mbm_cntr_reset(). It does both arch and non-arch state reset for all the RMIDs in all the domains. It is called in two places.

1 rdtgroup.c resctrl_mbm_assign_mode_write -> mbm_cntr_reset();

Please see my response to this usage in the related patch:
https://lore.kernel.org/lkml/[email protected]/
In summary, I find mbm_cntr_reset() ended up being a catch-all for random cleanup and creates confusion with the other mbm_cntr_*() calls.

Quoted 1 line

2 rdtgroup.c rdt_kill_sb()-> mbm_cntr_reset();

Please see my response to this usage in the related patch:
https://lore.kernel.org/lkml/[email protected]/
In summary, it does not solve the problem it originally set out to solve and it can be eliminated.

Quoted 6 lines

I will have to introduce another function to reset RMIDs in all the domains. Also, make sure it is called from both these places.

list_for_each_entry(dom, &r->mon_domains, hdr.list)
            resctrl_arch_reset_rmid_all(r, dom);

I do not see need for new functions, except the one I mention in
https://lore.kernel.org/lkml/[email protected]/
that suggests a new helper for reset of architectural state that does not exist and ends up being open coded in two places in this series.

With only one place (resctrl_mbm_assign_mode_write()) remaining that needs all state reset I think it will be easier to understand if the state reset is open coded within it, replacing mbm_cntr_reset() with:

	list_for_each_entry(dom, &r->mon_domains, hdr.list) {
		mbm_cntr_free_all()
		resctrl_reset_rmid_all() // Just for architectural state
	}

I would not insist on reset of architectural state within the architectural helper. I find that it is best for architecture to maintain its state but I also see there are many precedent for resctrl explicitly managing the state.

Quoted 1 line

I feel current code is much more cleaner.  What do you think?

It is better that previous versions, yes.

Quoted 14 lines

Quoted 14 lines

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

---

...

...

Quoted 23 lines

Quoted 22 lines

Quoted 8 lines

+ */

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

+static void _resctrl_abmc_enable(struct rdt_resource *r, bool enable)
+{
+    struct rdt_mon_domain *d;
+
+    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);

arch/x86/kernel/cpu/resctrl/monitor.c void __init intel_rdt_mbm_apply_quirk(void) · line 1295

Quoted 10 lines

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

+}
+
+int resctrl_arch_mbm_cntr_assign_set(struct rdt_resource *r, bool enable)
+{
+    struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
+
+    if (r->mon.mbm_cntr_assignable &&
+        hw_res->mbm_cntr_assign_enabled != enable) {
+        _resctrl_abmc_enable(r, enable);
+        hw_res->mbm_cntr_assign_enabled = enable;

Added benefit of resetting architectural state within this if statement (perhaps simpler to be done within _resctrl_abmc_enable()) is that it will not be done unnecessarily if ABMC is already in requested state.

Quoted 3 lines

It will be
      list_for_each_entry(dom, &r->mon_domains, hdr.list) resctrl_arch_reset_rmid_all(r, dom);

I am not sure if you are actually planning a new loop here ... as I suggested above this can be added to _resctrl_abmc_enable() where there is already a loop over all monitor domains and all that is needed is to add a call to resctrl_arch_reset_rmid_all(r, dom).

Even so, as I mentioned above, if after fixing automatic counter unassignment you still find that resetting architectural and non-architectural state together then we can go with that to match the other flows (eg. mbm_config_write_domain()).

Quoted 20 lines · Reinette Chatre

Quoted 9 lines · Babu

Quoted 1 line · Reinette Chatre

On 1/22/25 12:20 PM, Babu Moger wrote:

I will have to introduce another function to reset RMIDs in all the domains. Also, make sure it is called from both these places.

list_for_each_entry(dom, &r->mon_domains, hdr.list) resctrl_arch_reset_rmid_all(r, dom);

I do not see need for new functions, except the one I mention in
https://lore.kernel.org/lkml/[email protected]/
that suggests a new helper for reset of architectural state that does not exist and ends up being open coded in two places in this series.

With only one place (resctrl_mbm_assign_mode_write()) remaining that needs all state reset I think it will be easier to understand if the state reset is open coded within it, replacing mbm_cntr_reset() with:

Quoted 4 lines

    list_for_each_entry(dom, &r->mon_domains, hdr.list) { mbm_cntr_free_all()
        resctrl_reset_rmid_all() // Just for architectural state }

You meant "Just for non-architectural state" ?

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.

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 05358e78147b..ca69f2e0909f 100644

Hi Babu,

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

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 void resctrl_file_fflags_init(const char *config, unsigned long fflags); · line 668

Quoted 11 lines

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

index 05358e78147b..ca69f2e0909f 100644
@@ -658,4 +663,6 @@ 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);
+int resctrl_arch_mbm_cntr_assign_set(struct rdt_resource *r, bool enable);
+bool resctrl_arch_mbm_cntr_assign_enabled(struct rdt_resource *r);
 #endif /* _ASM_X86_RESCTRL_INTERNAL_H */

A minor nit - but could these be added to include/linux/resctrl.h instead? This is where they need to end up after the arch/fs split, and its harmless to do it from the beginning.

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

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

---

---
 Documentation/arch/x86/resctrl.rst     | 26 +++++++++++++++++++++
 arch/x86/kernel/cpu/resctrl/rdtgroup.c | 31 ++++++++++++++++++++++++++
 2 files changed, 57 insertions(+)

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

index fb90f08e564e..b5defc5bce0e 100644
@@ -257,6 +257,32 @@ 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

Hi Babu,

Quoted 3 lines · Babu Moger

Introduce the interface file "mbm_assign_mode" to list monitor modes supported.

Quoted 4 lines

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" is backed by the ABMC (Assignable

""mbm_cntr_assign" is backed" -> ""mbm_cntr_assign" mode is backed"?

Quoted 30 lines

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

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

---

---
  Documentation/arch/x86/resctrl.rst     | 26 +++++++++++++++++++++
  arch/x86/kernel/cpu/resctrl/rdtgroup.c | 31 ++++++++++++++++++++++++++
  2 files changed, 57 insertions(+)

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

index fb90f08e564e..b5defc5bce0e 100644
@@ -257,6 +257,32 @@ with the following files:
  	    # cat /sys/fs/resctrl/info/L3_MON/mbm_local_bytes_config
  	    0=0x30;1=0x30;3=0x15;4=0x15
 

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

Quoted 12 lines

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

+"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
+
+	"mbm_cntr_assign":
+
+	In mbm_cntr_assign, monitoring event can only accumulate data while

"In mbm_cntr_assign, monitoring event" -> "In mbm_cntr_assign mode, a monitoring event"?

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

Quoted 13 lines

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

+	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 counters
+	available is described in the "num_mbm_cntrs" file. Changing the mode
+	may cause all counters on a 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 mbm_cntr_assign mode if supported, because reading
+	"mbm_total_bytes" or "mbm_local_bytes" will report 'Unavailable' if

Reinette

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

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

index fb90f08e564e..b5defc5bce0e 100644
@@ -257,6 +257,32 @@ 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
+
+	"mbm_cntr_assign":
+
+	In mbm_cntr_assign, 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 counters

Documentation/arch/x86/resctrl.rst +0 -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/rdtgroup.c +5 -0

index f91fe605766f..3880480a41d2 100644
@@ -854,6 +854,30 @@ 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;
+

Hi Babu,

Quoted 42 lines · Babu Moger

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

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

index fb90f08e564e..b5defc5bce0e 100644
@@ -257,6 +257,32 @@ 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
+
+	"mbm_cntr_assign":
+
+	In mbm_cntr_assign, 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 counters

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

Quoted 10 lines

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

+	available is described in the "num_mbm_cntrs" file. Changing the mode
+	may cause all counters on a 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 mbm_cntr_assign mode if supported, because reading
+	"mbm_total_bytes" or "mbm_local_bytes" will report 'Unavailable' if
+	there is no counter associated with that event.

But if you read a value instead of "Unavailable", that doesn't mean the value is correct. For two reads that succeed, the counter may have been reset in the middle.

I'm suggesting something like:

| it is recommended to use mbm_cntr_assign mode if supported, to avoid counters
| being re-allocated by hardware. This can cause a misleading value to be read,
| or if no counter is associated with that event "Unavailable".

Quoted 17 lines

Documentation/arch/x86/resctrl.rst +0 -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/rdtgroup.c +5 -0

index f91fe605766f..3880480a41d2 100644
@@ -854,6 +854,30 @@ 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;
+

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

Quoted 10 lines

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

+	mutex_lock(&rdtgroup_mutex);
+
+	if (r->mon.mbm_cntr_assignable) {
+		if (resctrl_arch_mbm_cntr_assign_enabled(r)) {
+			seq_puts(s, "[mbm_cntr_assign]\n");
+			seq_puts(s, "default\n");
+		} else {
+			seq_puts(s, "mbm_cntr_assign\n");
+			seq_puts(s, "[default]\n");
+		}

What do you think to an architecture being able to opt-out of this flexibility?

If there aren't enough counters I can expose what the hardware has through this interface
- but if user-space turns it off ... then what?

For MPAM this would need to be some best-effort software allocation strategy that I'd rather not write - its not a problem that can be solved, and any value that is reported is likely to be wrong. For ABMC platforms, existing stable kernels expose a value, so being able to preserve the existing behaviour makes sense. MPAM doesn't have this problem.

Something like this:
----------%<----------

(quoted diff) +9 -4

@@ -861,16 +861,21 @@ 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 = resctrl_arch_mbm_cntr_assign_enabled(r);
 
         mutex_lock(&rdtgroup_mutex);
 
         if (r->mon.mbm_cntr_assignable) {
-               if (resctrl_arch_mbm_cntr_assign_enabled(r)) {
+               if (enabled)
                         seq_puts(s, "[mbm_cntr_assign]\n");
-                       seq_puts(s, "default\n");
-               } else {
-                       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");

----------%<----------

x86 wouldn't define CONFIG_RESCTRL_ASSIGN_FIXED, arm64 would, meaning for MPAM the file would be either:
| [default]
or
| [mbm_cntr_assign]

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

Quoted 9 lines

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

+	} else {
+		seq_puts(s, "[default]\n");
+	}
+
+	mutex_unlock(&rdtgroup_mutex);
+
+	return 0;
+}

Thanks,

James

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

index b5defc5bce0e..31ff764deeeb 100644
@@ -283,6 +283,22 @@ with the following files:
 	"mbm_total_bytes" or "mbm_local_bytes" will report 'Unavailable' if
 	there is no counter associated with that event.
 
+"num_mbm_cntrs":
+	The number of monitoring counters available for assignment when the
+	system supports mbm_cntr_assign mode.
+	::
+
+	  # cat /sys/fs/resctrl/info/L3_MON/num_mbm_cntrs

Hi Babu,

Quoted 11 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 supported. 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 18 lines

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

index b5defc5bce0e..31ff764deeeb 100644
@@ -283,6 +283,22 @@ with the following files:
  	"mbm_total_bytes" or "mbm_local_bytes" will report 'Unavailable' if
  	there is no counter associated with that event.
 
+"num_mbm_cntrs":
+	The number of monitoring counters available for assignment when the
+	system supports mbm_cntr_assign mode.
+	::
+
+	  # cat /sys/fs/resctrl/info/L3_MON/num_mbm_cntrs
+	  32
+
+	The resctrl file system supports tracking up to two memory bandwidth
+	events per monitoring group: mbm_total_bytes and/or mbm_local_bytes.

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

Quoted 6 lines

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

+	Up to two counters can be assigned per monitoring group, one for each
+	memory bandwidth event. More monitoring groups can be tracked by
+	assigning one counter per monitoring group. However, doing so limits
+	memory bandwidth tracking to a single memory bandwidth event per
+	monitoring group.
+

This text needs an update to reflect the switch to per-domain counter assignment.

Does this look ok? Just added domain in the text.

"The number of monitoring counters available in each domain for assignment when the system supports mbm_cntr_assign mode. ::

   # cat /sys/fs/resctrl/info/L3_MON/num_mbm_cntrs
   32

The resctrl file system supports tracking up to two memory bandwidth
events per monitoring group: mbm_total_bytes and/or mbm_local_bytes.
Up to two counters can be assigned per monitoring group, one for each
memory bandwidth event in each domain. More monitoring groups can be tracked by assigning one counter per monitoring group. However, doing so limits memory bandwidth tracking to a single memory bandwidth event per monitoring group."

Hi Reinette,

Quoted 4 lines · Babu

Does this look ok? Just added domain in the text.

"The number of monitoring counters available in each domain for assignment when the system supports mbm_cntr_assign mode. ::

Quoted 8 lines

   # cat /sys/fs/resctrl/info/L3_MON/num_mbm_cntrs
   32

The resctrl file system supports tracking up to two memory bandwidth
events per monitoring group: mbm_total_bytes and/or mbm_local_bytes.
Up to two counters can be assigned per monitoring group, one for each
memory bandwidth event in each domain. More monitoring groups can be tracked by assigning one counter per monitoring group. However, doing so limits memory bandwidth tracking to a single memory bandwidth event per monitoring group."

Revised again:

"The number of monitoring counters available in each domain for assignment when the system supports mbm_cntr_assign mode. For example, on a system with 32 monitoring counters:

::

  # cat /sys/fs/resctrl/info/L3_MON/num_mbm_cntrs
  32

The resctrl file system supports tracking up to two memory bandwidth
events per monitoring group: mbm_total_bytes and/or mbm_local_bytes.
Up to two counters can be assigned per monitoring group, one for each
memory bandwidth event in each domain. More monitoring groups can be tracked by assigning one counter per monitoring group. However, doing so limits memory bandwidth tracking to a single memory bandwidth event per monitoring group."

Thanks
Babu

Hi Babu,

Quoted 18 lines · Babu

Hi Reinette,

Quoted 4 lines · Babu

Does this look ok? Just added domain in the text.

"The number of monitoring counters available in each domain for assignment when the system supports mbm_cntr_assign mode. ::

Quoted 8 lines

   # cat /sys/fs/resctrl/info/L3_MON/num_mbm_cntrs
   32

The resctrl file system supports tracking up to two memory bandwidth
events per monitoring group: mbm_total_bytes and/or mbm_local_bytes.
Up to two counters can be assigned per monitoring group, one for each
memory bandwidth event in each domain. More monitoring groups can be tracked by assigning one counter per monitoring group. However, doing so limits memory bandwidth tracking to a single memory bandwidth event per monitoring group."

Revised again:

Quoted 1 line

"The number of monitoring counters available in each domain for assignment when the system supports mbm_cntr_assign mode. For example, on a system with 32 monitoring counters:

I think we need to be careful with "available" since all these counters may not be available. That is why "available_mbm_cntrs" exist.

How about something like (please feel free to improve):
"The maximum number of monitoring counters (total of available and assigned counters)
in each domain when the system supports mbm_cntr_assign mode."

Could you please make the "For example" a new paragraph (this follows existing style in the docs). It could also be made more specific, for example,

"For example, on a system with 32 monitoring counters in each domain:"

Quoted 5 lines

::

  # cat /sys/fs/resctrl/info/L3_MON/num_mbm_cntrs
  32

The rest of the documentation seems like a repeat of what can be found in the "mbm_assign_mode" section right above it. It does not look as though any information will be lost by dropping the text below?

Quoted 9 lines

The resctrl file system supports tracking up to two memory bandwidth
events per monitoring group: mbm_total_bytes and/or mbm_local_bytes.
Up to two counters can be assigned per monitoring group, one for each
memory bandwidth event in each domain. More monitoring groups can be tracked by assigning one counter per monitoring group. However, doing so limits memory bandwidth tracking to a single memory bandwidth event per monitoring group."

Thanks
Babu

Reinette

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.

Introduce resctrl_arch_mon_event_config_get() and resctrl_arch_mon_event_config_set() to get/set architecture domain specific mbm_total_cfg/mbm_local_cfg values.

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

---

---
 arch/x86/kernel/cpu/resctrl/internal.h | 15 +++++++
 arch/x86/kernel/cpu/resctrl/monitor.c  | 46 +++++++++++++++++++
 arch/x86/kernel/cpu/resctrl/rdtgroup.c | 61 +++++---------------------
 3 files changed, 72 insertions(+), 50 deletions(-)

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

index ab28b9340ee7..cfaea20145d0 100644

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

+ * @d:			Domain for the event
+ * @evtid:		Event type
+ * @mon_config:		Event configuration value
+ */
+struct mon_config_info {
+	struct rdt_mon_domain *d;
+	enum resctrl_event_id evtid;
+	u32 mon_config;
+};
+
 void rdt_last_cmd_clear(void);
 void rdt_last_cmd_puts(const char *s);
 __printf(1, 2)

@@ -674,4 +686,7 @@ int resctrl_arch_mbm_cntr_assign_set(struct rdt_resource *r, bool enable);
 bool resctrl_arch_mbm_cntr_assign_enabled(struct rdt_resource *r);
 void arch_mbm_evt_config_init(struct rdt_hw_mon_domain *hw_dom);
 unsigned int mon_event_config_index_get(u32 evtid);
+void resctrl_arch_mon_event_config_set(void *info);
+u32 resctrl_arch_mon_event_config_get(struct rdt_mon_domain *d,
+				      enum resctrl_event_id eventid);
 #endif /* _ASM_X86_RESCTRL_INTERNAL_H */

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

index 8917c7261680..6fe9e610e9a0 100644
@@ -1324,3 +1324,49 @@ int resctrl_arch_mbm_cntr_assign_set(struct rdt_resource *r, bool enable)
 
 	return 0;
 }
+
+u32 resctrl_arch_mon_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_mon_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) {

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

@@ -1683,33 +1653,23 @@ static int mbm_local_bytes_config_show(struct kernfs_open_file *of,
 	return 0;
 }
 
-static void mon_event_config_write(void *info)
-{
-	struct mon_config_info *mon_info = info;
-	unsigned int index;
-
-	index = mon_event_config_index_get(mon_info->evtid);
-	if (index == INVALID_CONFIG_INDEX) {

This new arch API has sharp corners because of asymmetry of where resctrl runs the arch function. I do not think it is required to change this since we can only speculate about how this may be used in the future but I do think it will be helpful to add comments that highlight:

resctrl_arch_mon_event_config_get() -> May run on CPU that does not belong to domain. resctrl_arch_mon_event_config_set() -> Runs on CPU that belongs to domain.

Quoted 7 lines

This new arch API has sharp corners because of asymmetry of where resctrl runs the arch function. I do not think it is required to change this since we can only speculate about how this may be used in the future but I do think it will be helpful to add comments that highlight:

resctrl_arch_mon_event_config_get() -> May run on CPU that does not belong to domain. resctrl_arch_mon_event_config_set() -> Runs on CPU that belongs to domain.

Here's a vague data point about the future to help with speculation.

I have something coming along the pipeline that also can run on any CPU.

I am contemplating a flag in the rdt_resource structure (in appropriate substructure resctrl_cache/resctrl_membw) to indicate "domain" vs. "any" for operations.

Would something like that be useful here?

Quoted 8 lines

Quoted 7 lines

This new arch API has sharp corners because of asymmetry of where resctrl runs the arch function. I do not think it is required to change this since we can only speculate about how this may be used in the future but I do think it will be helpful to add comments that highlight:

resctrl_arch_mon_event_config_get() -> May run on CPU that does not belong to domain. resctrl_arch_mon_event_config_set() -> Runs on CPU that belongs to domain.

Quoted 8 lines

Here's a vague data point about the future to help with speculation.

I have something coming along the pipeline that also can run on any CPU.

I am contemplating a flag in the rdt_resource structure (in appropriate substructure resctrl_cache/resctrl_membw) to indicate "domain" vs. "any" for operations.

Would something like that be useful here?

hmm ... I cannot envision how this may look. Could you please elaborate?

You mention "a" (singular) flag in rdt_resource while this scenario involves different ops having different scope. This makes me think that this flag may have to be per operation that in turn would need additional infrastructure to manage and track operations.

These "arch" functions are evolving as the work to support MPAM is done and so far I think it has been quite ad-hoc to just refactor arch specific code into "arch" helpers instead of keeping track of which scope they are running in. This currently requires any arch implementing an "arch" helper to be well aware of how resctrl will call it.

Quoted 11 lines

Quoted 11 lines

Quoted 7 lines

This new arch API has sharp corners because of asymmetry of where resctrl runs the arch function. I do not think it is required to change this since we can only speculate about how this may be used in the future but I do think it will be helpful to add comments that highlight:

resctrl_arch_mon_event_config_get() -> May run on CPU that does not belong to domain. resctrl_arch_mon_event_config_set() -> Runs on CPU that belongs to domain.

Here's a vague data point about the future to help with speculation.

I have something coming along the pipeline that also can run on any CPU.

Quoted 12 lines

Quoted 4 lines

I am contemplating a flag in the rdt_resource structure (in appropriate substructure resctrl_cache/resctrl_membw) to indicate "domain" vs. "any" for operations.

Would something like that be useful here?

hmm ... I cannot envision how this may look. Could you please elaborate?

You mention "a" (singular) flag in rdt_resource while this scenario involves different ops having different scope. This makes me think that this flag may have to be per operation that in turn would need additional infrastructure to manage and track operations.

Quoted 5 lines

These "arch" functions are evolving as the work to support MPAM is done and so far I think it has been quite ad-hoc to just refactor arch specific code into "arch" helpers instead of keeping track of which scope they are running in. This currently requires any arch implementing an "arch" helper to be well aware of how resctrl will call it.

I haven't fleshed it out yet. One option would be to have a "choose_cpu_mask()" function that takes resource and domain parameters (and given your comment about this case an operation code). Then use that as the mask in an smp_call*().

Operations that can run anywhere would return a mask with just bit for the current CPU.

Hi Tony,

Quoted 8 lines · Tony

Quoted 7 lines

This new arch API has sharp corners because of asymmetry of where resctrl runs the arch function. I do not think it is required to change this since we can only speculate about how this may be used in the future but I do think it will be helpful to add comments that highlight:

resctrl_arch_mon_event_config_get() -> May run on CPU that does not belong to domain. resctrl_arch_mon_event_config_set() -> Runs on CPU that belongs to domain.

Quoted 8 lines

Here's a vague data point about the future to help with speculation.

I have something coming along the pipeline that also can run on any CPU.

I am contemplating a flag in the rdt_resource structure (in appropriate substructure resctrl_cache/resctrl_membw) to indicate "domain" vs. "any" for operations.

Would something like that be useful here?

hmm ... I cannot envision how this may look. Could you please elaborate?

You mention "a" (singular) flag in rdt_resource while this scenario involves different ops having different scope. This makes me think that this flag may have to be per operation that in turn would need additional infrastructure to manage and track operations.

These "arch" functions are evolving as the work to support MPAM is done and so far I think it has been quite ad-hoc to just refactor arch specific code into "arch" helpers instead of keeping track of which scope they are running in. This currently requires any arch implementing an "arch" helper to be well aware of how resctrl will call it.

Reinette

Quoted 25 lines · Reinette Chatre

Quoted 8 lines · Tony

Quoted 7 lines

This new arch API has sharp corners because of asymmetry of where resctrl runs the arch function. I do not think it is required to change this since we can only speculate about how this may be used in the future but I do think it will be helpful to add comments that highlight:

resctrl_arch_mon_event_config_get() -> May run on CPU that does not belong to domain. resctrl_arch_mon_event_config_set() -> Runs on CPU that belongs to domain.

Quoted 8 lines

Here's a vague data point about the future to help with speculation.

I have something coming along the pipeline that also can run on any CPU.

I am contemplating a flag in the rdt_resource structure (in appropriate substructure resctrl_cache/resctrl_membw) to indicate "domain" vs. "any" for operations.

Would something like that be useful here?

hmm ... I cannot envision how this may look. Could you please elaborate?

You mention "a" (singular) flag in rdt_resource while this scenario involves different ops having different scope. This makes me think that this flag may have to be per operation that in turn would need additional infrastructure to manage and track operations.

Quoted 5 lines

These "arch" functions are evolving as the work to support MPAM is done and so far I think it has been quite ad-hoc to just refactor arch specific code into "arch" helpers instead of keeping track of which scope they are running in. This currently requires any arch implementing an "arch" helper to be well aware of how resctrl will call it.

For MPAM, we must typically do all configuration access from a CPU in a power domain that depends on the power domain of the relevant MPAM MSC, including reads of the configuration.

In the MPAM case, the required topology knowledge is not necessarily identical to the resctrl domain topology, so it doesn't feel right to have the resctrl core code making the decisions.

So, in the interest of keeping the arch interface simple, should cross- calling be delegated to the arch code, at least for now?

Hi Babu,

Quoted 27 lines · Babu Moger

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.

Introduce resctrl_arch_mon_event_config_get() and resctrl_arch_mon_event_config_set() to get/set architecture domain specific mbm_total_cfg/mbm_local_cfg values.

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

---

---
  arch/x86/kernel/cpu/resctrl/internal.h | 15 +++++++
  arch/x86/kernel/cpu/resctrl/monitor.c  | 46 +++++++++++++++++++
  arch/x86/kernel/cpu/resctrl/rdtgroup.c | 61 +++++---------------------
  3 files changed, 72 insertions(+), 50 deletions(-)

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

index ab28b9340ee7..cfaea20145d0 100644
@@ -605,6 +605,18 @@ union cpuid_0x10_x_edx {
  	unsigned int full;
  };
 

arch/x86/kernel/cpu/resctrl/internal.h union cpuid_0x10_x_edx { · line 609

Quoted 2 lines

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

+/**
+ * struct mon_config_info - Monitoring event configuratiin details

Same typo as previous version.

Quoted 66 lines

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

+ * @d:			Domain for the event
+ * @evtid:		Event type
+ * @mon_config:		Event configuration value
+ */
+struct mon_config_info {
+	struct rdt_mon_domain *d;
+	enum resctrl_event_id evtid;
+	u32 mon_config;
+};
+
  void rdt_last_cmd_clear(void);
  void rdt_last_cmd_puts(const char *s);
  __printf(1, 2)

@@ -674,4 +686,7 @@ int resctrl_arch_mbm_cntr_assign_set(struct rdt_resource *r, bool enable);
  bool resctrl_arch_mbm_cntr_assign_enabled(struct rdt_resource *r);
  void arch_mbm_evt_config_init(struct rdt_hw_mon_domain *hw_dom);
  unsigned int mon_event_config_index_get(u32 evtid);
+void resctrl_arch_mon_event_config_set(void *info);
+u32 resctrl_arch_mon_event_config_get(struct rdt_mon_domain *d,
+				      enum resctrl_event_id eventid);
  #endif /* _ASM_X86_RESCTRL_INTERNAL_H */

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

index 8917c7261680..6fe9e610e9a0 100644
@@ -1324,3 +1324,49 @@ int resctrl_arch_mbm_cntr_assign_set(struct rdt_resource *r, bool enable)
 
  	return 0;
  }
+
+u32 resctrl_arch_mon_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_mon_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) {

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

Quoted 10 lines

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

+	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;
+	default:
+		break;
+	}
+}

This new arch API has sharp corners because of asymmetry of where resctrl runs the arch function. I do not think it is required to change this since we can only speculate about how this may be used in the future but I do think it will be helpful to add comments that highlight:

resctrl_arch_mon_event_config_get() -> May run on CPU that does not belong to domain. resctrl_arch_mon_event_config_set() -> Runs on CPU that belongs to domain.

...

Quoted 17 lines

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

@@ -1683,33 +1653,23 @@ static int mbm_local_bytes_config_show(struct kernfs_open_file *of,
  	return 0;
  }
 
-static void mon_event_config_write(void *info)
-{
-	struct mon_config_info *mon_info = info;
-	unsigned int index;
-
-	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;
-	}
-	wrmsr(MSR_IA32_EVT_CFG_BASE + index, mon_info->mon_config, 0);
-}
 

arch/x86/kernel/cpu/resctrl/monitor.c static int mbm_local_bytes_config_show(struct kernfs_open_file *of, · line 1660

Quoted 4 lines

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

  static void mbm_config_write_domain(struct rdt_resource *r,
  				    struct rdt_mon_domain *d, u32 evtid, u32 val)
  {
  	struct mon_config_info mon_info = {0};

As discussed in previous version it is unnecessary to explicitly initialize the structure if it is fully initialized in the code. This avoids need for future cleanups like commit 29eaa7958367 ("x86/resctrl: Slightly clean-up mbm_config_show()")

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

index 8917c7261680..6fe9e610e9a0 100644
@@ -1324,3 +1324,49 @@ int resctrl_arch_mbm_cntr_assign_set(struct rdt_resource *r, bool enable)
 
  	return 0;
  }
+
+u32 resctrl_arch_mon_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_mon_event_config_set(void *info)

Quoted 29 lines · Babu Moger

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

index 8917c7261680..6fe9e610e9a0 100644
@@ -1324,3 +1324,49 @@ int resctrl_arch_mbm_cntr_assign_set(struct rdt_resource *r, bool enable)
         return 0;
   }
+
+u32 resctrl_arch_mon_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_mon_event_config_set(void *info)

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

Quoted 10 lines

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

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

This is the existing code, however it would be better to use wrmsrl() when the higher 32-bit are all 0s:

    wrmsrl(MSR_IA32_EVT_CFG_BASE + index, mon_info->mon_config);

Quoted 6 lines

Quoted 1 line

+    wrmsr(MSR_IA32_EVT_CFG_BASE + index, mon_info->mon_config, 0);

This is the existing code, however it would be better to use wrmsrl() when the higher 32-bit are all 0s:

    wrmsrl(MSR_IA32_EVT_CFG_BASE + index, mon_info->mon_config);

Could you please elaborate what makes this change better?

Quoted 7 lines · Reinette Chatre

Quoted 6 lines

Quoted 1 line

+    wrmsr(MSR_IA32_EVT_CFG_BASE + index, mon_info->mon_config, 0);

This is the existing code, however it would be better to use wrmsrl() when the higher 32-bit are all 0s:

     wrmsrl(MSR_IA32_EVT_CFG_BASE + index, mon_info->mon_config);

Could you please elaborate what makes this change better?

In short, it takes one less argument, and doesn't pass an argument of 0.

The longer story is that hpa and I are refactoring the MSR access APIs to accommodate the immediate form of MSR access instructions.  And we are not happy about that there are too many MSR access APIs and their uses are *random*.  The native wrmsr() and wrmsrl() are essentially the same and the only difference is that wrmsr() passes a 64-bit value to be written into a MSR in *2* u32 arguments.  But we already have struct msr defined in asm/shared/msr.h as:
    struct msr {
            union {
                    struct {
                            u32 l;
                            u32 h;
                    };
                    u64 q;
            };
    };

it's more natural to do the same job with this data structure in most cases.  And we want to remove wrmsr() and only keep wrmsrl(), thus a developer won't have to figure out which one is better to use :-P.

For that to happen, one cleanup is to replace wrmsr(msr, low, 0) with wrmsrl(msr, low) (low is automatically converted to u64 from u32).

However, I'm fine if Babu wants to keep it as-is.

Hi Xin,

Quoted 32 lines · Xin Li

Quoted 7 lines · Reinette Chatre

Quoted 6 lines

Quoted 1 line

+    wrmsr(MSR_IA32_EVT_CFG_BASE + index, mon_info->mon_config, 0);

This is the existing code, however it would be better to use wrmsrl() when the higher 32-bit are all 0s:

     wrmsrl(MSR_IA32_EVT_CFG_BASE + index, mon_info->mon_config);

Could you please elaborate what makes this change better?

In short, it takes one less argument, and doesn't pass an argument of 0.

The longer story is that hpa and I are refactoring the MSR access APIs to accommodate the immediate form of MSR access instructions.  And we are not happy about that there are too many MSR access APIs and their uses are *random*.  The native wrmsr() and wrmsrl() are essentially the same and the only difference is that wrmsr() passes a 64-bit value to be written into a MSR in *2* u32 arguments.  But we already have struct msr defined in asm/shared/msr.h as:
    struct msr {
            union {
                    struct {
                            u32 l;
                            u32 h;
                    };
                    u64 q;
            };
    };

it's more natural to do the same job with this data structure in most cases.  And we want to remove wrmsr() and only keep wrmsrl(), thus a developer won't have to figure out which one is better to use :-P.

Quoted 4 lines

For that to happen, one cleanup is to replace wrmsr(msr, low, 0) with wrmsrl(msr, low) (low is automatically converted to u64 from u32).

However, I'm fine if Babu wants to keep it as-is.

Thanks for the explanation. Changed it to use wrmsrl().

In mbm_cntr_assign mode hardware counters are assigned/unassigned to an MBM event of a monitor group. Hardware counters are assigned/unassigned at monitoring domain level.

Manage a monitoring domain's hardware counters using a per monitoring domain array of struct mbm_cntr_cfg that is indexed by the hardware
counter ID. A hardware counter's configuration contains the MBM event

ID and points to the monitoring group that it is assigned to, with a NULL pointer meaning that the hardware counter is available for assignment.

There is no direct way to determine which hardware counters are assigned

to a particular monitoring group. Check every entry of every hardware
counter configuration array in every monitoring domain to query which

MBM events of a monitoring group is tracked by hardware. Such queries are acceptable because of a very small number of assignable counters.

Suggested-by Peter Newman <[email protected]>

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

---

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

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

index 18110a1afb6d..75a3b56996ca 100644
@@ -4009,6 +4009,7 @@ static void __init rdtgroup_setup_default(void)
 
 static void domain_destroy_mon_state(struct rdt_mon_domain *d)
 {
+	kfree(d->cntr_cfg);
 	bitmap_free(d->rmid_busy_llc);
 	kfree(d->mbm_total);
 	kfree(d->mbm_local);

@@ -4082,6 +4083,16 @@ static int domain_setup_mon_state(struct rdt_resource *r, struct rdt_mon_domain
 			return -ENOMEM;
 		}
 	}
+	if (is_mbm_enabled() && r->mon.mbm_cntr_assignable) {
+		tsize = sizeof(*d->cntr_cfg);
+		d->cntr_cfg = kcalloc(r->mon.num_mbm_cntrs, tsize, GFP_KERNEL);
+		if (!d->cntr_cfg) {
+			bitmap_free(d->rmid_busy_llc);
+			kfree(d->mbm_total);
+			kfree(d->mbm_local);
+			return -ENOMEM;
+		}
+	}
 
 	return 0;
 }

include/linux/resctrl.h +2 -0

index 511cfce8fc21..9a54e307d340 100644
@@ -94,6 +94,18 @@ struct rdt_ctrl_domain {
 	u32				*mbps_val;
 };
 
+/**
+ * struct mbm_cntr_cfg - assignable counter configuration

include/linux/resctrl.h +1 -0

 /**
  * struct rdt_mon_domain - group of CPUs sharing a resctrl monitor resource
  * @hdr:		common header for different domain types

@@ -105,6 +117,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
+ * @cntr_cfg:		assignable counters configuration
  */
 struct rdt_mon_domain {
 	struct rdt_domain_hdr		hdr;

Hi Babu,

Quoted 4 lines · Babu Moger

In mbm_cntr_assign mode hardware counters are assigned/unassigned to an MBM event of a monitor group. Hardware counters are assigned/unassigned at monitoring domain level.

Quoted 3 lines

Manage a monitoring domain's hardware counters using a per monitoring domain array of struct mbm_cntr_cfg that is indexed by the hardware
counter ID. A hardware counter's configuration contains the MBM event

Something strange in this changelog with a few random \t in the text.

Quoted 5 lines

ID and points to the monitoring group that it is assigned to, with a NULL pointer meaning that the hardware counter is available for assignment.

There is no direct way to determine which hardware counters are assigned

... another \t above

Quoted 3 lines

to a particular monitoring group. Check every entry of every hardware
counter configuration array in every monitoring domain to query which

... one more \t above

Quoted 3 lines

MBM events of a monitoring group is tracked by hardware. Such queries are acceptable because of a very small number of assignable counters.

It is not obvious what "very small number" means. Is it possible to give a range to help reader understand the motivation?

Quoted 57 lines

Suggested-by Peter Newman <[email protected]>

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

---

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

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

index 18110a1afb6d..75a3b56996ca 100644
@@ -4009,6 +4009,7 @@ static void __init rdtgroup_setup_default(void)
 
  static void domain_destroy_mon_state(struct rdt_mon_domain *d)
  {
+	kfree(d->cntr_cfg);
  	bitmap_free(d->rmid_busy_llc);
  	kfree(d->mbm_total);
  	kfree(d->mbm_local);

@@ -4082,6 +4083,16 @@ static int domain_setup_mon_state(struct rdt_resource *r, struct rdt_mon_domain
  			return -ENOMEM;
  		}
  	}
+	if (is_mbm_enabled() && r->mon.mbm_cntr_assignable) {
+		tsize = sizeof(*d->cntr_cfg);
+		d->cntr_cfg = kcalloc(r->mon.num_mbm_cntrs, tsize, GFP_KERNEL);
+		if (!d->cntr_cfg) {
+			bitmap_free(d->rmid_busy_llc);
+			kfree(d->mbm_total);
+			kfree(d->mbm_local);
+			return -ENOMEM;
+		}
+	}
 
  	return 0;
  }

include/linux/resctrl.h +9 -0

index 511cfce8fc21..9a54e307d340 100644
@@ -94,6 +94,18 @@ struct rdt_ctrl_domain {
  	u32				*mbps_val;
  };
 
+/**
+ * struct mbm_cntr_cfg - assignable counter configuration
+ * @evtid:		 MBM event to which the counter is assigned. Only valid
+ *			 if @rdtgroup is not NULL.
+ * @rdtgroup:		 resctrl group assigned to the counter. NULL if the
+ *			 counter is free.
+ */
+struct mbm_cntr_cfg {
+	enum resctrl_event_id	evtid;

include/linux/resctrl.h struct rdt_ctrl_domain { · line 108

Quoted 3 lines

include/linux/resctrl.h +3 -0

+	struct rdtgroup		*rdtgrp;
+};
+

$ scripts/kernel-doc -v -none include/linux/resctrl.h
...
include/linux/resctrl.h:107: warning: Function parameter or struct member 'rdtgrp' not described in 'mbm_cntr_cfg'
include/linux/resctrl.h:107: warning: Excess struct member 'rdtgroup' description in 'mbm_cntr_cfg'

...

Quoted 18 lines

/**
* struct rdt_mon_domain - group of CPUs sharing a resctrl monitor resource
* @hdr: common header for different domain types

include/linux/resctrl.h +2 -0

@@ -105,6 +117,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
+ * @cntr_cfg:		assignable counters configuration
   */
  struct rdt_mon_domain {
  	struct rdt_domain_hdr		hdr;

@@ -116,6 +129,7 @@ struct rdt_mon_domain {
  	struct delayed_work		cqm_limbo;
  	int				mbm_work_cpu;
  	int				cqm_work_cpu;
+	struct mbm_cntr_cfg		*cntr_cfg;
  };
 

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

Quoted 1 line

include/linux/resctrl.h +0 -0

  /**

Reinette

Quoted 6 lines · Reinette Chatre

Quoted 2 lines · Babu Moger

to a particular monitoring group. Check every entry of every hardware counter    configuration array in every monitoring domain to query which

... one more \t above

Sure

Quoted 4 lines

Quoted 3 lines

MBM events of a monitoring group is tracked by hardware. Such queries are acceptable because of a very small number of assignable counters.

Quoted 2 lines

It is not obvious what "very small number" means. Is it possible to give a range to help reader understand the motivation?

How about?

MBM events of a monitoring group is tracked by hardware. Such queries are acceptable because of a very small number of assignable counters(32 to 64).

Hi Babu,

Quoted 2 lines · Babu

Quoted 1 line · Reinette Chatre

On 1/22/25 12:20 PM, Babu Moger wrote:

Quoted 6 lines

Quoted 5 lines

Quoted 2 lines

MBM events of a monitoring group is tracked by hardware. Such queries are acceptable because of a very small number of assignable counters.

It is not obvious what "very small number" means. Is it possible to give a range to help reader understand the motivation?

Quoted 4 lines

How about?

MBM events of a monitoring group is tracked by hardware. Such queries are acceptable because of a very small number of assignable counters(32 to 64).

Yes, thank you. This helps to understand the claim.

Reinette

Hi,

Quoted 5 lines · Reinette Chatre

Hi Babu,

Quoted 2 lines · Babu

Quoted 1 line · Reinette Chatre

On 1/22/25 12:20 PM, Babu Moger wrote:

[...]

Quoted 11 lines

Quoted 10 lines

Quoted 5 lines

Quoted 2 lines

MBM events of a monitoring group is tracked by hardware. Such queries are acceptable because of a very small number of assignable counters.

It is not obvious what "very small number" means. Is it possible to give a range to help reader understand the motivation?

How about?

MBM events of a monitoring group is tracked by hardware. Such queries are acceptable because of a very small number of assignable counters(32 to 64).

Quoted 3 lines

Yes, thank you. This helps to understand the claim.

Reinette

Do these queries only happen when userspace reads an mbm_assign_control file?

It might be worth documenting somewhere that writing and (especially) reading an mbm_assign_control file is not intended to be super-fast.

It feels like userspace should not generally rely on reading mbm_assign_control files except for diagnostic purposes, or occasional read-modify-write transformations. Or do expect some other usage model that makes this a hotter path?

Cheers
---Dave

Hi Dave,

Quoted 22 lines · Dave Martin

Hi,

Quoted 5 lines · Reinette Chatre

Hi Babu,

Quoted 2 lines · Babu

Quoted 1 line · Reinette Chatre

On 1/22/25 12:20 PM, Babu Moger wrote:

[...]

Quoted 11 lines

Quoted 10 lines

Quoted 5 lines

Quoted 2 lines

MBM events of a monitoring group is tracked by hardware. Such queries are acceptable because of a very small number of assignable counters.

It is not obvious what "very small number" means. Is it possible to give a range to help reader understand the motivation?

How about?

MBM events of a monitoring group is tracked by hardware. Such queries are acceptable because of a very small number of assignable counters(32 to 64).

Quoted 6 lines

Quoted 3 lines

Yes, thank you. This helps to understand the claim.

Reinette

Do these queries only happen when userspace reads an mbm_assign_control file?

Yes. All these queries are initiated by userspace in the form of individual assignments or creating a group(mkdir).

Quoted 3 lines

It might be worth documenting somewhere that writing and (especially) reading an mbm_assign_control file is not intended to be super-fast.

We can drop the last sentence if it is creating confusion.

Quoted 6 lines

It feels like userspace should not generally rely on reading mbm_assign_control files except for diagnostic purposes, or occasional read-modify-write transformations. Or do expect some other usage model that makes this a hotter path?