x86/resctrl: Add kernel-mode (e.g., PLZA) support to the resctrl subsystem

When memory bandwidth associated with a CLOSID is aggressively throttled, and a task with that CLOSID moves into kernel mode, the kernel operations are also aggressively throttled. This can stall forward progress and eventually degrade overall system performance.

AMD hardware supports a feature Privilege Level Zero Association (PLZA), which allows the CPU's CLOSID association to be changed during the

transition from user mode to kernel mode. This allows the kernel to run using a different CLOSID than user space, which can improve system

performance.

PLZA also applies to cache allocation and monitoring features and provides flexibility to manage allocation and monitoring associations independently.

The feature is detected via CPUID_Fn80000020_EBX_x00 [Bit 9]: Privilege Level Zero Association (PLZA).

The PLZA feature details are documented in [1] available from [2].

[1] AMD64 Zen6 Platform Quality of Service (PQOS) Extensions:

    Publication # 69193 Revision: 1.00, Issue Date: March 2026

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

Acked-by Borislav Petkov (AMD) <[email protected]>

Link https://bugzilla.kernel.org/show_bug.cgi?id=206537 # [2]

---

Hi Babu,

Quoted 5 lines · Babu Moger

When memory bandwidth associated with a CLOSID is aggressively throttled, and a task with that CLOSID moves into kernel mode, the kernel operations are also aggressively throttled. This can stall forward progress and eventually degrade overall system performance.

Quoted 2 lines

AMD hardware supports a feature Privilege Level Zero Association (PLZA), which allows the CPU's CLOSID association to be changed during the

CLOSID -> CLOSID and/or RMID?

Quoted 2 lines

transition from user mode to kernel mode. This allows the kernel to run using a different CLOSID than user space, which can improve system

CLOSID -> CLOSID and/or RMID?

Quoted 1 line

performance.

"... and/or separately monitor the work done in kernel mode."?

Quoted 4 lines

PLZA also applies to cache allocation and monitoring features and provides flexibility to manage allocation and monitoring associations independently.

This reads like an afterthought. Are users expected to use cache allocation while PLZA is enabled? Since only one resource group can be assigned I expect there to be quite a few sharp corners when both are in use. Kernel work done on behalf of high as well as low priority work will use the same cache allocations and thus neither could actually get full benefit from having dedicated cache portions?

Quoted 1 line · Reinette Chatre

On 8/26/26 12:32 PM, Babu Moger wrote:

Quoted 6 lines

Quoted 5 lines

PLZA also applies to cache allocation and monitoring features and provides flexibility to manage allocation and monitoring associations independently.

Quoted 6 lines

This reads like an afterthought. Are users expected to use cache allocation while PLZA is enabled? Since only one resource group can be assigned I expect there to be quite a few sharp corners when both are in use. Kernel work done on behalf of high as well as low priority work will use the same cache allocations and thus neither could actually get full benefit from having dedicated cache portions?

The best-known use case today is to leave both cache allocation and memory bandwidth allocation at their default (maximum) settings and enable PLZA on all CPUs.

Users can, however, configure both cache allocation and memory bandwidth limits. In those cases, there can be corner-case behaviors and trade-offs.

If you think the current wording is misleading, I can drop that text and rephrase it along these lines to include both cache and memory.

"When cache and/or memory bandwidth associated with a CLOSID is aggressively throttled, and a task with that CLOSID moves into kernel mode, the kernel operations are also aggressively throttled. This can stall forward progress and eventually degrade overall system performance.

AMD hardware supports the Privilege Level Zero Association (PLZA) feature, which allows the CPU's CLOSID and/or RMID association to be changed during transitions from user mode to kernel mode. This enables the kernel to run with a different CLOSID and/or RMID than user space, improving system performance and/or allowing kernel work to be monitored separately.">

Hi Babu,

Quoted 2 lines · Babu Moger

Quoted 1 line · Reinette Chatre

On 8/26/26 12:32 PM, Babu Moger wrote:

...

Quoted 13 lines

Quoted 6 lines

Quoted 5 lines

PLZA also applies to cache allocation and monitoring features and provides flexibility to manage allocation and monitoring associations independently.

Quoted 6 lines

This reads like an afterthought. Are users expected to use cache allocation while PLZA is enabled? Since only one resource group can be assigned I expect there to be quite a few sharp corners when both are in use. Kernel work done on behalf of high as well as low priority work will use the same cache allocations and thus neither could actually get full benefit from having dedicated cache portions?

Quoted 3 lines

The best-known use case today is to leave both cache allocation and memory bandwidth allocation at their default (maximum) settings and enable PLZA on all CPUs.

Interesting. This sounds like all kernel work can access the entire cache and thus evict data from any user space tasks that may have used cache allocation to avoid interference.

Quoted 4 lines

Users can, however, configure both cache allocation and memory bandwidth limits. In those cases, there can be corner-case behaviors and trade-offs.

If you think the current wording is misleading, I can drop that text and rephrase it along these lines to include both cache and memory.

No, the wording is not misleading. The description just focuses on memory bandwidth allocation leaving impression that the feature only applies to that resource when it applies to all resources.

Quoted 7 lines

"When cache and/or memory bandwidth associated with a CLOSID is aggressively throttled, and a task with that CLOSID moves into kernel mode, the kernel operations are also aggressively throttled. This can stall forward progress and eventually degrade overall system performance.

Quoted 6 lines

AMD hardware supports the Privilege Level Zero Association (PLZA) feature, which allows the CPU's CLOSID and/or RMID association to be changed during transitions from user mode to kernel mode. This enables the kernel to run with a different CLOSID and/or RMID than user space, improving system performance and/or allowing kernel work to be monitored separately.">

I think what you have for memory bandwidth sounds good. I do not think the cache allocation and memory bandwidth usage descriptions should be mixed since the implications are so different. Could you please just add a snippet that highlights since the feature assigns CLOSID/RMID to kernel work it influences allocation and monitoring of all resources, not just memory bandwidth?

Add a kernel command-line parameter to enable or disable the exposure of the PLZA (Privilege Level Zero Association) feature to resctrl.

This allows administrators to control PLZA visibility at boot time via the rdt= option (e.g. rdt=plza or rdt=!plza).

Resctrl exposes PLZA by default without requiring any explicit rdt= option if supported. The rdt=!plza parameter can be used to disable PLZA exposure to resctrl, while rdt=plza explicitly enables it.

Hi Babu,

Quoted 9 lines · Babu Moger

Add a kernel command-line parameter to enable or disable the exposure of the PLZA (Privilege Level Zero Association) feature to resctrl.

This allows administrators to control PLZA visibility at boot time via the rdt= option (e.g. rdt=plza or rdt=!plza).

Resctrl exposes PLZA by default without requiring any explicit rdt= option if supported. The rdt=!plza parameter can be used to disable PLZA exposure to resctrl, while rdt=plza explicitly enables it.

There is a lot of repetition. How about something like:

Add "plza" to the rdt= command-line parameter so PLZA exposure to resctrl can be enabled or disabled at boot. PLZA is exposed by default when supported; rdt=!plza suppresses it.

Patch looks good.

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

+ * MSR_IA32_PQR_ASSOC irrespective of privilege level.
+ *
+ * @rmid		: RMID programmed for PLZA.
+ * @reserved1		: Reserved.
+ * @rmid_en		: Enable RMID for PLZA.
+ * @closid		: CLOSID programmed for PLZA.
+ * @reserved2		: Reserved.
+ * @closid_en		: Enable CLOSID for PLZA.
+ * @reserved3		: Reserved.
+ * @plza_en		: Enable PLZA. When enabled, PLZA applies to the
+ *			  given CPU.
+ */
+union msr_pqr_plza_assoc {
+	struct {
+		u64	rmid		:12,
+			reserved1	:19,
+			rmid_en		: 1,
+			closid		: 4,
+			reserved2	:11,

Hi Babu,

Quoted 8 lines · Babu Moger

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

index e3cfa0c10e92..2f4c60487a5d 100644
@@ -222,6 +222,53 @@ union l3_qos_abmc_cfg {
  	unsigned long full;
  };
 

arch/x86/kernel/cpu/resctrl/internal.h union l3_qos_abmc_cfg { · line 225

Quoted 1 line

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

+/*

I understand that this is not specifically kernel-doc (since no /**) but it clearly follows the pattern and also needs to match the style used in this file. Considering this, please use custom that describes members first before the long description? With members described first the long description can then use @ when referring to the members.

Quoted 4 lines

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

+ * Privilege Level Zero Association (PLZA) is configured via
+ * MSR_IA32_PQR_PLZA_ASSOC. This MSR must be programmed on every CPU in
+ * a QoS domain. Each domain supports a single PLZA CLOSID and/or RMID

arch/x86/kernel/cpu/resctrl/internal.h union l3_qos_abmc_cfg { · line 238

Quoted 10 lines

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

+ * association, and all fields except PLZA_EN must be programmed
+ * identically across the domain to ensure consistent behavior.
+ *
+ * RMID and CLOSID selection are controlled independently. When PLZA is
+ * enabled (plza_en = 1), CPL0 uses CLOSID and RMID values from
+ * MSR_IA32_PQR_PLZA_ASSOC depending on the corresponding enable bits.
+ * When PLZA is disabled (plza_en = 0), the CLOSID and RMID are always
+ * taken from MSR_IA32_PQR_ASSOC regardless of privilege level.
+ *
+ *   - rmid_en = 1: use the RMID programmed in this MSR.

"this MSR" -> "MSR_IA32_PQR_PLZA_ASSOC" or keeping text similar to the rmid_en = 0 scenario may make it easier to follow what the bit accomplishes:

	- @rmid_en = 1: use the RMID from MSR_IA32_PQR_PLZA_ASSOC if
			@plza_en = 1

arch/x86/kernel/cpu/resctrl/internal.h union l3_qos_abmc_cfg { · line 240

Quoted 3 lines

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

+ *   - rmid_en = 0: use the RMID from MSR_IA32_PQR_ASSOC.
+ *   - closid_en = 1: use the CLOSID programmed in this MSR.

same comment as for rmid_en bit:

	- @closid_en = 1: use the CLOSID from MSR_IA32_PQR_PLZA_ASSOC
			  if @plza_en = 1

arch/x86/kernel/cpu/resctrl/internal.h union l3_qos_abmc_cfg { · line 245

Quoted 5 lines

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

+ *   - closid_en = 0: use the CLOSID from MSR_IA32_PQR_ASSOC.
+ *
+ * This allows PLZA to override only RMID, only CLOSID, or both. If both
+ * rmid_en and closid_en are clear, the effective behavior is the same as
+ * when PLZA is disabled, with RMID and CLOSID derived from

"derived from" -> "used from"?

Kernel mode traffic can use different allocation and monitoring

associations than the originating user task. On x86, Privilege Level Zero Association (PLZA) enables the kernel to switch to a different CLOSID/RMID when entering kernel mode.

Architectures need a common way to name kernel modes before resctrl can report what is active or what the platform supports.

Introduce enum resctrl_kernel_mode:
- RESCTRL_INHERIT_USER: Kernel mode inherits allocation and monitoring from the user task.
- RESCTRL_ASSIGN_GLOBAL_ENABLE_PER_CPU: Kernel mode may use allocation and/or monitoring associations that differ from the user task. On x86, CLOSID and RMID can be configured independently for kernel mode.

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

---

---
 include/linux/resctrl.h | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

include/linux/resctrl.h +0 -0

index dd09c2ce9a0f..4245d1e65ccc 100644
@@ -704,6 +704,31 @@ int resctrl_arch_io_alloc_enable(struct rdt_resource *r, bool enable);
  */
 bool resctrl_arch_get_io_alloc_enabled(struct rdt_resource *r);
 

Hi Babu,

Quoted 1 line · Babu Moger

Kernel mode traffic can use different allocation and monitoring

"traffic" -> "work" to not imply this is just about memory traffic but also includes cache allocation?

Quoted 17 lines

associations than the originating user task. On x86, Privilege Level Zero Association (PLZA) enables the kernel to switch to a different CLOSID/RMID when entering kernel mode.

Architectures need a common way to name kernel modes before resctrl can report what is active or what the platform supports.

Introduce enum resctrl_kernel_mode:
- RESCTRL_INHERIT_USER: Kernel mode inherits allocation and monitoring from the user task.
- RESCTRL_ASSIGN_GLOBAL_ENABLE_PER_CPU: Kernel mode may use allocation and/or monitoring associations that differ from the user task. On x86, CLOSID and RMID can be configured independently for kernel mode.

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

---

...

Quoted 12 lines

---
  include/linux/resctrl.h | 25 +++++++++++++++++++++++++
  1 file changed, 25 insertions(+)

include/linux/resctrl.h +0 -0

index dd09c2ce9a0f..4245d1e65ccc 100644
@@ -704,6 +704,31 @@ int resctrl_arch_io_alloc_enable(struct rdt_resource *r, bool enable);
   */
  bool resctrl_arch_get_io_alloc_enabled(struct rdt_resource *r);
 

include/linux/resctrl.h int resctrl_arch_io_alloc_enable(struct rdt_resource *r, bool enable); · line 716

Quoted 10 lines

include/linux/resctrl.h +10 -0

+/**
+ * enum resctrl_kernel_mode - Kernel mode control and monitoring association.
+ *
+ * @RESCTRL_INHERIT_USER:
+ *     Kernel mode inherits both allocation and monitoring associations
+ *     from the user space task. On x86, kernel uses the same CLOSID and
+ *     RMID as the user-space task.
+ *
+ * @RESCTRL_ASSIGN_GLOBAL_ENABLE_PER_CPU:
+ *     Kernel mode may use allocation and monitoring associations that

Why is this "may use"? Could this be made more accurate? Consider, for example:
All kernel mode work uses the same allocation and/or monitoring association via an assigned CTRL_MON or MON group. The mode can be enabled/disabled per-CPU. When the mode is disabled on a CPU then the kernel work on that CPU inherits the allocation and monitoring association from the user space task, effectively the kernel mode becomes RESCTRL_INHERIT_USER on that CPU.

include/linux/resctrl.h int resctrl_arch_io_alloc_enable(struct rdt_resource *r, bool enable); · line 718

Quoted 3 lines

include/linux/resctrl.h +2 -0

+ *     differ from the user task. On x86, CLOSID and RMID are configured
+ *     independently; either or both may differ from user mode.

Please first provide all details that support what resctrl fs exposes to user space before, if at all, adding any x86 specific details.

include/linux/resctrl.h int resctrl_arch_io_alloc_enable(struct rdt_resource *r, bool enable); · line 722

Quoted 4 lines

include/linux/resctrl.h +4 -0

+ *
+ *     All online CPUs are included by default. A subset may be selected
+ *     through the resctrl group interface, and a CTRL_MON or MON group
+ *     may be associated with this mode.

Please note that the word "associated" appears in multiple contexts here.

include/linux/resctrl.h int resctrl_arch_io_alloc_enable(struct rdt_resource *r, bool enable); · line 726

Quoted 5 lines

include/linux/resctrl.h +4 -0

+ */
+enum resctrl_kernel_mode {
+	RESCTRL_INHERIT_USER,
+	RESCTRL_ASSIGN_GLOBAL_ENABLE_PER_CPU,

Something that stands out in this series is the multiple occurrences of "assigned" in the new interface. What do you think of just dropping "assign" from the kernel mode name?

Kernel modes defined by enum resctrl_kernel_mode must be applied on the CPUs when user space activates, deactivates, or updates a configuration.

Generic resctrl has no architecture hook to apply these modes across a CPU mask when the active mode changes.

Add resctrl_arch_configure_kmode() to program kernel mode allocation and monitoring associations on @cpu_mask. Accept separate assign_ctrl and assign_mon parameters so CLOSID and RMID can be assigned independently.

Implement the x86 hook to program per-CPU PLZA settings. On x86, PLZA programs these associations per CPU, with CLOSID and RMID configured independently.

Provide an MPAM stub so the filesystem layer can call the hook on systems without PLZA.

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

---

---
 arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 38 +++++++++++++++++++++++
 drivers/resctrl/mpam_resctrl.c            |  6 ++++

 include/linux/resctrl.h                   | 33 ++++++++++++++++++++
 3 files changed, 77 insertions(+)

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

index e74f1ed54b86..40fd5e31c94e 100644
@@ -131,3 +131,41 @@ int resctrl_arch_io_alloc_enable(struct rdt_resource *r, bool enable)
 
 	return 0;
 }
+

arch/x86/kernel/cpu/resctrl/ctrlmondata.c +17 -0

+ * @assign_mon: Whether PLZA should provide the kernel mode RMID.
+ * @enable:	Whether PLZA should provide the kernel mode association.
+ */
+void resctrl_arch_configure_kmode(const struct cpumask *cpu_mask, u32 closid,
+				  bool assign_ctrl, u32 rmid,
+				  bool assign_mon, bool enable)
+{
+	union msr_pqr_plza_assoc plza = { 0 };
+
+	plza.split.rmid = rmid;
+	plza.split.rmid_en = assign_mon;
+	plza.split.closid = closid;
+	plza.split.closid_en = assign_ctrl;
+	plza.split.plza_en = enable;
+
+	on_each_cpu_mask(cpu_mask, resctrl_kmode_set_one_amd, &plza, 1);
+}

drivers/resctrl/mpam_resctrl.c +6 -0

index 9d223057953a..286284ac8423 100644
@@ -139,6 +139,12 @@ bool resctrl_arch_get_io_alloc_enabled(struct rdt_resource *r)
 	return false;
 }
 
+void resctrl_arch_configure_kmode(const struct cpumask *cpu_mask, u32 closid,
+				  bool assign_ctrl, u32 rmid, bool assign_mon,
+				  bool enable)
+{
+}
+
 void resctrl_arch_pre_mount(void)
 {
 }

include/linux/resctrl.h +0 -0

index 4245d1e65ccc..8b30eef835aa 100644

include/linux/resctrl.h +4 -0

+ * @cpu_mask:	CPUs to assign the kernel mode on.
+ * @closid:	CLOSID that matches the RMID to program kernel mode. Depending
+ *		on the architecture, the counter may match traffic of both
+ *		@closid and @rmid, or @rmid only.

Hi Babu,

Quoted 3 lines · Babu Moger

Kernel modes defined by enum resctrl_kernel_mode must be applied on the CPUs when user space activates, deactivates, or updates a configuration.

Not necessarily. This is just what PLZA/RESCTRL_ASSIGN_GLOBAL_ENABLE_PER_CPU requires, no? The "applied on the CPUs" seems specific to the RESCTRL_ASSIGN_GLOBAL_ENABLE_PER_CPU mode - hence the name includes "enable per CPU". Above text implies that all possible kernel modes require this, we know that upcoming ones don't so this could just be specific to the only kernel mode that needs it?

Quoted 4 lines

Generic resctrl has no architecture hook to apply these modes across a CPU mask when the active mode changes.

I cannot believe this. v3 of this series wrote the changelogs of this new feature enabling as bugfixes. I asked you several times in v3 to not do this:

https://lore.kernel.org/lkml/[email protected]/
https://lore.kernel.org/lkml/[email protected]/
https://lore.kernel.org/lkml/[email protected]/

v4 did not follow that style ... but now this style of presenting enabling code as bugfix is back in v5! In v2 I already expressed frustration that every new series seemingly starts from scratch

https://lore.kernel.org/lkml/[email protected]/

Now a new version seems to forget feedback from just two versions ago :(

Quoted 5 lines

Add resctrl_arch_configure_kmode() to program kernel mode allocation and monitoring associations on @cpu_mask. Accept separate assign_ctrl and assign_mon parameters so CLOSID and RMID can be assigned independently.

Below is just a sampling from the last two versions of me asking you to not just verbatim describe the code:

https://lore.kernel.org/lkml/[email protected]/ V3
https://lore.kernel.org/lkml/[email protected]/ V3
https://lore.kernel.org/lkml/[email protected]/ V3
https://lore.kernel.org/lkml/[email protected]/ V4
https://lore.kernel.org/lkml/[email protected]/ V4
https://lore.kernel.org/lkml/[email protected]/ V4

Again, please do not just verbatim describe what clearly can be seen from the patch. Use the changelog to describe why the code behaves a certain way.

Maybe you need this request to come from Boris instead before you start following the guidance? Here are some examples:

https://lore.kernel.org/all/20240702124524.GEZoP2ZKcTcKl1ca1R@fat_crate.local/
https://lore.kernel.org/lkml/20250911165433.GBaML-yTUZHkywuJIe@fat_crate.local/

Quoted 1 line

From here on the changelogs all seem to have this strange pattern of:

"Architecture needs X"

"Architecture is missing X"

"Verbatim description of X implementation"

Apart from the issues mentioned above this interchangeable repetition turns the changelogs into a blur. The x86 format for changelogs is described in Documentation/process/maintainer-tip.rst. Just follow that. This should not be new to you.

Do not expect further comments on any of the changelogs that follow. I consider them all unusable.

I clearly demonstrate above that you ignore my feedback. There really seems no reason for me to provide any. I'll make a final attempt to provide feedback to *just* the patches (as much as I can without being able to use the changelogs) to try and help this work make progress.

Quoted 10 lines

Implement the x86 hook to program per-CPU PLZA settings. On x86, PLZA programs these associations per CPU, with CLOSID and RMID configured independently.

Provide an MPAM stub so the filesystem layer can call the hook on systems without PLZA.

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

---

...

Quoted 3 lines

---
  arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 38 +++++++++++++++++++++++
  drivers/resctrl/mpam_resctrl.c            |  6 ++++

Needs "arm" in subject prefix.

Quoted 10 lines

  include/linux/resctrl.h                   | 33 ++++++++++++++++++++
  3 files changed, 77 insertions(+)

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

index e74f1ed54b86..40fd5e31c94e 100644
@@ -131,3 +131,41 @@ int resctrl_arch_io_alloc_enable(struct rdt_resource *r, bool enable)
 

arch/x86/kernel/cpu/resctrl/ctrlmondata.c int resctrl_arch_io_alloc_enable(struct rdt_resource *r, bool enable) · line 144

Quoted 13 lines

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

  	return 0;
  }
+
+static void resctrl_kmode_set_one_amd(void *arg)
+{
+	union msr_pqr_plza_assoc *plza = arg;
+
+	wrmsrq(MSR_IA32_PQR_PLZA_ASSOC, plza->full);
+}
+
+/*
+ * Program Privilege Level Zero Association (PLZA) on @cpu_mask.
+ *

Please follow the custom with function parameters described first, followed by description.

arch/x86/kernel/cpu/resctrl/ctrlmondata.c int resctrl_arch_io_alloc_enable(struct rdt_resource *r, bool enable) · line 149

Quoted 6 lines

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

+ * When @enable is true, kernel mode allocation on @cpu_mask uses @closid from
+ * MSR_IA32_PQR_PLZA_ASSOC if @assign_ctrl is true, otherwise the CLOSID from
+ * MSR_IA32_PQR_ASSOC. Kernel mode monitoring uses @rmid from
+ * MSR_IA32_PQR_PLZA_ASSOC if @assign_mon is true, otherwise the RMID of the
+ * current task.

This just seems to duplicate the description of union msr_pqr_plza_assoc?

arch/x86/kernel/cpu/resctrl/ctrlmondata.c int resctrl_arch_io_alloc_enable(struct rdt_resource *r, bool enable) · line 152

Quoted 4 lines

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

+ *
+ * @cpu_mask:	CPUs whose PLZA MSR should be updated.
+ * @closid:	CLOSID to use for kernel mode allocation when @assign_ctrl is true.

Contrary to what the comment states the closid parameter is always programmed, whether assign_ctrl is true or false. A valid closid is thus expected to always be provided?

arch/x86/kernel/cpu/resctrl/ctrlmondata.c int resctrl_arch_io_alloc_enable(struct rdt_resource *r, bool enable) · line 154

Quoted 3 lines

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

+ * @assign_ctrl: Whether PLZA should provide the kernel mode CLOSID.
+ * @rmid:	RMID to use for kernel mode monitoring when @assign_mon is true.

Same comment.

Quoted 42 lines

arch/x86/kernel/cpu/resctrl/ctrlmondata.c +17 -0

+ * @assign_mon: Whether PLZA should provide the kernel mode RMID.
+ * @enable:	Whether PLZA should provide the kernel mode association.
+ */
+void resctrl_arch_configure_kmode(const struct cpumask *cpu_mask, u32 closid,
+				  bool assign_ctrl, u32 rmid,
+				  bool assign_mon, bool enable)
+{
+	union msr_pqr_plza_assoc plza = { 0 };
+
+	plza.split.rmid = rmid;
+	plza.split.rmid_en = assign_mon;
+	plza.split.closid = closid;
+	plza.split.closid_en = assign_ctrl;
+	plza.split.plza_en = enable;
+
+	on_each_cpu_mask(cpu_mask, resctrl_kmode_set_one_amd, &plza, 1);
+}

drivers/resctrl/mpam_resctrl.c +6 -0

index 9d223057953a..286284ac8423 100644
@@ -139,6 +139,12 @@ bool resctrl_arch_get_io_alloc_enabled(struct rdt_resource *r)
  	return false;
  }
 
+void resctrl_arch_configure_kmode(const struct cpumask *cpu_mask, u32 closid,
+				  bool assign_ctrl, u32 rmid, bool assign_mon,
+				  bool enable)
+{
+}
+
  void resctrl_arch_pre_mount(void)
  {
  }

include/linux/resctrl.h +0 -0

index 4245d1e65ccc..8b30eef835aa 100644
@@ -729,6 +729,39 @@ enum resctrl_kernel_mode {
 
  #define RESCTRL_NUM_KERNEL_MODES (RESCTRL_KMODE_LAST + 1)
 

include/linux/resctrl.h enum resctrl_kernel_mode { · line 733

Quoted 2 lines

include/linux/resctrl.h +2 -0

+/**
+ * resctrl_arch_configure_kmode() - Program kernel mode association

resctrl_arch_configure_kmode() implies a generic kernel mode callback but the parameters are specific to the global, per-CPU mode. I expect that either the kernel mode self be a parameter or the callback be unique to the kernel mode.
To simplify the parameter management this could be the latter and renamed
to something like "resctrl_arch_configure_kmode_global()"/"resctrl_arch_configure_global_kmode()" ?

Quoted 5 lines

include/linux/resctrl.h +4 -0

+ * @cpu_mask:	CPUs to assign the kernel mode on.
+ * @closid:	CLOSID that matches the RMID to program kernel mode. Depending
+ *		on the architecture, the counter may match traffic of both
+ *		@closid and @rmid, or @rmid only.

include/linux/resctrl.h enum resctrl_kernel_mode { · line 747

Quoted 10 lines

include/linux/resctrl.h +10 -0

+ * @assign_ctrl: true to assign @closid for kernel mode; false to inherit
+ *		association from the user-space task.
+ * @rmid:	RMID to program the kernel mode. Some architectures may use
+ *		CLOSID/RMID separately, others will consider them together.
+ * @assign_mon:	true to assign @rmid for kernel mode; false to inherit
+ *		monitoring association from the user-space task.
+ * @enable:	true to enable kernel mode association on CPUs in @cpu_mask;
+ *		false to disable kernel mode.
+ *
+ * The function can be called in the following scenarios:

"can be" -> "is"?

include/linux/resctrl.h enum resctrl_kernel_mode { · line 748

Quoted 1 line

include/linux/resctrl.h +1 -0

+ * - If a per-cpu kernel mode is active when user space switches to a new

per-cpu -> per-CPU

include/linux/resctrl.h enum resctrl_kernel_mode { · line 749

Quoted 2 lines

include/linux/resctrl.h +1 -0

+ *   per-cpu kernel mode then resctrl_arch_configure_kmode() will first be

"a new per-cpu kernel mode" - what does this refer to? There is only one per-CPU kernel mode, no? It may help to refer to the kernel modes explicitly by their enum value to be clear which modes this callback applies to.

include/linux/resctrl.h enum resctrl_kernel_mode { · line 755

Quoted 7 lines

include/linux/resctrl.h +6 -0

+ *   called to de-activate the active kernel mode on all CPUs that the
+ *   kernel mode is active on.
+ * - When user space switches to a new per-cpu kernel mode then
+ *   resctrl_arch_configure_kmode() is called with cpu_online_mask.
+ * - When user space adds a CPU to an active per-cpu kernel mode.
+ * - When user space removes a CPU from an active per-cpu kernel mode.

Above scenarios all have the "per-cpu kernel mode" in description that confirms that this callback is dedicated to this single kernel mode and not actually a generic "enable kernel mode" callback.

Below does not seem to fall under "scenario" like the above but actually represents a contract between fs and arch that can be separated and highlighted.

include/linux/resctrl.h enum resctrl_kernel_mode { · line 757

Quoted 3 lines

include/linux/resctrl.h +2 -0

+ * - resctrl fs will always provide the same closid, assign_ctrl, rmid,
+ *   and assign_mon parameters when activating a kernel mode, all

"a kernel mode" -> this callback is not generic so it should be specific to which modes it applies to.

---
 fs/resctrl/internal.h | 51 +++++++++++++++++++++++++++++++++++++++++++
 fs/resctrl/rdtgroup.c | 33 ++++++++++++++++++++++++++++
 2 files changed, 84 insertions(+)

fs/resctrl/internal.h +7 -0

index e62a277dee85..4087bf44a06d 100644
@@ -314,6 +314,57 @@ struct mbm_state {
 	u32	prev_bw;
 };
 
+/**
+ * enum kmode_state - Control or monitoring state for a kernel mode
+ * @KMODE_INHERIT: Inherit from the user space task.
+ * @KMODE_ASSIGN: Use a global assignment for kernel mode.
+ */
+enum kmode_state {
+	KMODE_INHERIT,

fs/resctrl/internal.h +6 -0

+ */
+struct resctrl_kmode_caps {
+	DECLARE_BITMAP(kmode_sup, RESCTRL_NUM_KERNEL_MODES);
+	bool				ctrl_en;
+	bool				mon_en;
+};

fs/resctrl/internal.h +6 -0

+ */
+struct resctrl_kmode_cfg {
+	struct resctrl_kmode_caps	caps;
+	struct resctrl_kmode_active	active;
+};
+
 extern struct mutex rdtgroup_mutex;
 
 static inline const char *rdt_kn_name(const struct kernfs_node *kn)

fs/resctrl/rdtgroup.c +0 -0

index 5dcbb0a964e8..3c53f3f74e5a 100644

fs/resctrl/rdtgroup.c +1 -0

+
 /*
  * Memory bandwidth monitoring event to use for the default CTRL_MON group
  * and each new CTRL_MON group created by the user.  Only relevant when

fs/resctrl/rdtgroup.c +17 -0

+ */
+static void resctrl_kmode_init(void)
+{
+	resctrl_kcfg.caps.ctrl_en = resctrl_arch_alloc_capable();
+	resctrl_kcfg.active.ctrl_mode = KMODE_INHERIT;
+	resctrl_kcfg.caps.mon_en = resctrl_arch_mon_capable();
+	resctrl_kcfg.active.mon_mode = KMODE_INHERIT;
+	resctrl_kcfg.active.k_rdtgrp = NULL;
+
+	if (resctrl_kcfg.caps.ctrl_en || resctrl_kcfg.caps.mon_en) {
+		resctrl_kcfg.active.kmode_cur = RESCTRL_INHERIT_USER;
+		__set_bit(RESCTRL_INHERIT_USER, resctrl_kcfg.caps.kmode_sup);
+	} else {
+		bitmap_zero(resctrl_kcfg.caps.kmode_sup, RESCTRL_NUM_KERNEL_MODES);
+	}
+}
+
 void resctrl_file_fflags_init(const char *config, unsigned long fflags)
 {
 	struct rftype *rft;

Hi Babu,

Quoted 29 lines · Babu Moger

---
  fs/resctrl/internal.h | 51 +++++++++++++++++++++++++++++++++++++++++++
  fs/resctrl/rdtgroup.c | 33 ++++++++++++++++++++++++++++
  2 files changed, 84 insertions(+)

fs/resctrl/internal.h +16 -0

index e62a277dee85..4087bf44a06d 100644
@@ -314,6 +314,57 @@ struct mbm_state {
  	u32	prev_bw;
  };
 
+/**
+ * enum kmode_state - Control or monitoring state for a kernel mode
+ * @KMODE_INHERIT: Inherit from the user space task.
+ * @KMODE_ASSIGN: Use a global assignment for kernel mode.
+ */
+enum kmode_state {
+	KMODE_INHERIT,
+	KMODE_ASSIGN
+};
+
+/**
+ * struct resctrl_kmode_caps - Static kernel mode capabilities
+ * @kmode_sup:	Bitmap of supported kernel modes. Empty when neither
+ *		@ctrl_en nor @mon_en is set and kernel mode policy is
+ *		unavailable on this system.
+ * @ctrl_en:	Whether kernel mode may use global assignment for control.

fs/resctrl/internal.h struct mbm_state { · line 333

Quoted 1 line

fs/resctrl/internal.h +1 -0

+ * @mon_en:	Whether kernel mode may use global assignment for monitoring.

Why is ctrl_en and mon_en needed? It seems to just store the output of whether system supports allocation and monitoring. I only see these used when user interacts reads or writes the kernel mode so not a "hot path" that needs to be optimized. Can these just be dropped and just use resctrl_arch_alloc_capable() and resctrl_arch_mon_capable() directly? Please note they are in process of being changed/renamed:

https://lore.kernel.org/lkml/[email protected]/

Quoted 12 lines

fs/resctrl/internal.h +12 -0

+ */
+struct resctrl_kmode_caps {
+	DECLARE_BITMAP(kmode_sup, RESCTRL_NUM_KERNEL_MODES);
+	bool				ctrl_en;
+	bool				mon_en;
+};
+
+/**
+ * struct resctrl_kmode_active - Runtime kernel mode state
+ * @kmode_cur:	Currently selected kernel mode.
+ * @ctrl_mode:	Control assignment state when kernel mode is active.
+ * @mon_mode:	Monitoring assignment state when kernel mode is active.

fs/resctrl/internal.h struct mbm_state { · line 349

Quoted 4 lines

fs/resctrl/internal.h +4 -0

+ * @k_rdtgrp:	Resource group backing global assignment mode.
+ *
+ * When @kmode_cur is %RESCTRL_INHERIT_USER, assignment state is ignored and
+ * @k_rdtgrp is %NULL.

This implies that this is only the state for RESCTRL_ASSIGN_GLOBAL_ENABLE_PER_CPU. If it is made specifically so there is no need to pretend it is generic and add all these caveats.

fs/resctrl/internal.h struct mbm_state { · line 354

Quoted 6 lines

fs/resctrl/internal.h +5 -0

+ */
+struct resctrl_kmode_active {
+	enum resctrl_kernel_mode	kmode_cur;
+	enum kmode_state		ctrl_mode;
+	enum kmode_state		mon_mode;

Is "mode" accurate? This is all about kernel "mode" and now control and monitoring have other modes?

fs/resctrl/internal.h struct mbm_state { · line 355

Quoted 2 lines

fs/resctrl/internal.h +1 -0

+	struct rdtgroup			*k_rdtgrp;

Could naming be consistent? Consider, for example, kmode_rdtgrp? Although if this struct can be specific to the global per-CPU kernel mode then it can just be "rdtgrp".

fs/resctrl/internal.h struct mbm_state { · line 361

Quoted 7 lines

fs/resctrl/internal.h +6 -0

+};
+
+/**
+ * struct resctrl_kmode_cfg - Global kernel mode state
+ * @caps:	Supported modes and assignment capabilities.
+ * @active:	Active mode, assignment state, and assigned group.

Please do not list the struct members as part of its description elsewhere since that will be difficult to keep accurate. Just describe what the struct represents.

Quoted 18 lines

fs/resctrl/internal.h +6 -0

+ */
+struct resctrl_kmode_cfg {
+	struct resctrl_kmode_caps	caps;
+	struct resctrl_kmode_active	active;
+};
+
  extern struct mutex rdtgroup_mutex;
 
  static inline const char *rdt_kn_name(const struct kernfs_node *kn)

fs/resctrl/rdtgroup.c +0 -0

index 5dcbb0a964e8..3c53f3f74e5a 100644
@@ -76,6 +76,13 @@ static void rdtgroup_destroy_root(void);
 
  struct dentry *debugfs_resctrl;
 

fs/resctrl/rdtgroup.c static void rdtgroup_destroy_root(void); · line 82

Quoted 4 lines

fs/resctrl/rdtgroup.c +4 -0

+/*
+ * Global kernel mode policy state: supported modes, active mode, assignment
+ * capabilities, assignment state, and the resource group selected for a global
+ * assignment.

Same here - please do not just provide a list of the struct's members. A high level description instead.

This code is really strange. This whole series is difficult to read. I have not seen these styles used before and surprised that it comes from you.

fs/resctrl/rdtgroup.c static void rdtgroup_destroy_root(void); · line 84

Quoted 3 lines

fs/resctrl/rdtgroup.c +2 -0

+ */
+static struct resctrl_kmode_cfg resctrl_kcfg;

I think the code will be easier to read if "resctrl_kcfg" -> "resctrl_kmode".

Quoted 9 lines

fs/resctrl/rdtgroup.c +1 -0

+
  /*
   * Memory bandwidth monitoring event to use for the default CTRL_MON group
   * and each new CTRL_MON group created by the user.  Only relevant when

@@ -2297,6 +2304,30 @@ static void io_alloc_init(void)
  	}
  }
 

fs/resctrl/rdtgroup.c static void io_alloc_init(void) · line 2313

Quoted 7 lines

fs/resctrl/rdtgroup.c +7 -0

+/*
+ * Initialize kernel mode policy defaults from architecture capabilities.
+ *
+ * When ctrl_en or mon_en is set, RESCTRL_INHERIT_USER is supported and
+ * selected as the initial active mode. When neither is set, kmode_sup is
+ * left empty, kernel mode policy is unavailable, and kmode_cur remains at
+ * its zero-initialized default (RESCTRL_INHERIT_USER) but is unused.

Above just verbatim describes the code. Please provide higher level why the code does what it does.

fs/resctrl/rdtgroup.c +7 -0

+ */
+void resctrl_set_kmode_support(enum resctrl_kernel_mode kmode)
+{
+	if (kmode < RESCTRL_NUM_KERNEL_MODES)
+		__set_bit(kmode, resctrl_kcfg.caps.kmode_sup);
+}
+
 #ifdef CONFIG_PROC_CPU_RESCTRL
 /*
  * A task can only be part of one resctrl control group and of one monitor

include/linux/resctrl.h +0 -0

index 8b30eef835aa..051aece77eb9 100644

include/linux/resctrl.h +5 -0

+ * can be exposed through the resctrl filesystem.
+ * RESCTRL_INHERIT_USER is registered by resctrl_kmode_init() and does
+ * not need to be registered again.
+ *
+ * Architectures use this interface during resctrl initialization to

Hi Babu,

Quoted 8 lines · Babu Moger

fs/resctrl/rdtgroup.c +0 -0

index 3c53f3f74e5a..3bb0e3a203ac 100644
@@ -892,6 +892,19 @@ static int rdtgroup_rmid_show(struct kernfs_open_file *of,
  	return ret;
  }
 

fs/resctrl/rdtgroup.c static int rdtgroup_rmid_show(struct kernfs_open_file *of, · line 897

Quoted 3 lines

fs/resctrl/rdtgroup.c +3 -0

+/**
+ * resctrl_set_kmode_support() - Register a supported kernel mode
+ * @kmode:	Kernel mode policy supported by the architecture.

How is "Kernel mode" different from "Kernel mode policy"?

fs/resctrl/rdtgroup.c static int rdtgroup_rmid_show(struct kernfs_open_file *of, · line 900

Quoted 4 lines

fs/resctrl/rdtgroup.c +3 -0

+ *
+ * Set the corresponding bit in resctrl_kcfg.caps.kmode_sup so @kmode
+ * is visible to the resctrl file system.

Verbatim description of what can be seen from the code.

Quoted 19 lines

fs/resctrl/rdtgroup.c +7 -0

+ */
+void resctrl_set_kmode_support(enum resctrl_kernel_mode kmode)
+{
+	if (kmode < RESCTRL_NUM_KERNEL_MODES)
+		__set_bit(kmode, resctrl_kcfg.caps.kmode_sup);
+}
+
  #ifdef CONFIG_PROC_CPU_RESCTRL
  /*
   * A task can only be part of one resctrl control group and of one monitor

include/linux/resctrl.h +0 -0

index 8b30eef835aa..051aece77eb9 100644
@@ -729,6 +729,23 @@ enum resctrl_kernel_mode {
 
  #define RESCTRL_NUM_KERNEL_MODES (RESCTRL_KMODE_LAST + 1)
 

include/linux/resctrl.h enum resctrl_kernel_mode { · line 736

Quoted 5 lines

include/linux/resctrl.h +5 -0

+/**
+ * resctrl_set_kmode_support() - Register a supported kernel mode
+ * @kmode: Kernel mode supported by the architecture.
+ *
+ * Set the corresponding bit in resctrl_kcfg.caps.kmode_sup so @kmode

No need to describe resctrl fs internal details here.

---
 Documentation/filesystems/resctrl.rst |  29 +++++++
 fs/resctrl/rdtgroup.c                 | 115 ++++++++++++++++++++++++++
 2 files changed, 144 insertions(+)

Documentation/filesystems/resctrl.rst +1 -0

index f3e941404967..c6e8cf828e18 100644
@@ -528,6 +528,35 @@ conveyed in the error returns from file operations. E.g.
 	# cat info/last_cmd_status
 	mask f7 has non-consecutive 1-bits
 
+"kernel_mode":

Documentation/filesystems/resctrl.rst +7 -0

+
+	Example::
+
+	  # cat info/kernel_mode
+	  [inherit_user]
+	  assign_global_enable_per_cpu:ctrl=assign;mon=assign;group=//
+
 Resource alloc and monitor groups
 =================================
 

fs/resctrl/rdtgroup.c +0 -0

index 3bb0e3a203ac..992af586a194 100644

fs/resctrl/rdtgroup.c +33 -0

+};
+
+static_assert(ARRAY_SIZE(resctrl_mode_str) == RESCTRL_NUM_KERNEL_MODES);
+
+static const char *resctrl_kmode_state_str(enum kmode_state state)
+{
+	return state == KMODE_ASSIGN ? "assign" : "inherit";
+}
+
+static void resctrl_kmode_group_path(struct rdtgroup *rdtgrp,
+				     const char **ctrl, const char **mon)
+{
+	*ctrl = "";
+	*mon = "";
+
+	if (!rdtgrp)
+		return;
+
+	if (rdtgrp->type == RDTMON_GROUP) {
+		*ctrl = rdt_kn_name(rdtgrp->mon.parent->kn);
+		*mon = rdt_kn_name(rdtgrp->kn);
+	} else {
+		*ctrl = rdt_kn_name(rdtgrp->kn);
+	}
+}
+
+/**
+ * resctrl_kernel_mode_show() - Display supported and active kernel modes
+ * @of: kernfs open file
+ * @seq: output seq_file
+ * @v: unused
+ *
+ * Lists one line per mode set in resctrl_kcfg.caps.kmode_sup. Brackets the

fs/resctrl/rdtgroup.c +23 -0

+ *
+ * Return: 0 on success, or -ENOENT on error.
+ */
+static int resctrl_kernel_mode_show(struct kernfs_open_file *of,
+				    struct seq_file *seq, void *v)
+{
+	const char *ctrl_state, *mon_state;
+	enum resctrl_kernel_mode mode;
+	struct rdtgroup *rdtgrp;
+	const char *ctrl, *mon;
+	bool active;
+	int ret = 0;
+
+	if (!info_kn_lock(of->kn))
+		return -ENOENT;
+
+	for (mode = 0; mode < RESCTRL_NUM_KERNEL_MODES; mode++) {
+		if (!test_bit(mode, resctrl_kcfg.caps.kmode_sup))
+			continue;
+
+		active = (resctrl_kcfg.active.kmode_cur == mode);
+
+		if (mode == RESCTRL_INHERIT_USER) {

fs/resctrl/rdtgroup.c +14 -0

+				   resctrl_mode_str[mode], ctrl_state, mon_state,
+				   ctrl, mon);
+		} else {
+			seq_printf(seq, "%s:ctrl=%s;mon=%s;group=%s/%s/\n",
+				   resctrl_mode_str[mode], ctrl_state, mon_state,
+				   ctrl, mon);
+		}
+	}
+
+out_unlock:
+	info_kn_unlock(of->kn);
+	return ret;
+}
+
 void *rdt_kn_parent_priv(struct kernfs_node *kn)
 {
 	/*

@@ -1995,6 +2103,13 @@ static struct rftype res_common_files[] = {
 		.seq_show	= rdt_last_cmd_status_show,
 		.fflags		= RFTYPE_TOP_INFO,
 	},

Hi Babu,

Quoted 14 lines · Babu Moger

---
  Documentation/filesystems/resctrl.rst |  29 +++++++
  fs/resctrl/rdtgroup.c                 | 115 ++++++++++++++++++++++++++
  2 files changed, 144 insertions(+)

Documentation/filesystems/resctrl.rst +1 -0

index f3e941404967..c6e8cf828e18 100644
@@ -528,6 +528,35 @@ conveyed in the error returns from file operations. E.g.
  	# cat info/last_cmd_status
  	mask f7 has non-consecutive 1-bits
 
+"kernel_mode":

Documentation/filesystems/resctrl.rst conveyed in the error returns from file operations. E.g. · line 541

Quoted 10 lines

Documentation/filesystems/resctrl.rst +10 -0

+	In the top level of the "info" directory, "kernel_mode" reports
+	supported and active kernel modes available on the system.
+
+	Reading the file lists one mode per line. The active mode is wrapped in
+	square brackets. inherit_user is shown without options.
+	assign_global_enable_per_cpu is shown as::
+
+	  assign_global_enable_per_cpu:ctrl=<assign|inherit>;mon=<assign|inherit>;group=<ctrl>/<mon>/
+
+	- inherit_user: inherit allocation and monitoring from the user task.

To make this easier to read, instead of mixing the modes, could you please have text like:

Possible kernel modes are:

<description of inherit_user>

<description of assign_global_enable_per_cpu>

In addition to being easier to read something like above will also be easier to expand.

Documentation/filesystems/resctrl.rst conveyed in the error returns from file operations. E.g. · line 544

Quoted 4 lines

Documentation/filesystems/resctrl.rst +3 -0

+	- assign_global_enable_per_cpu: kernel mode may use separate
+	  allocation and/or monitoring associations. ctrl= and mon= show
+	  whether each is assigned or inherited, and group= identifies the

Earlier is "ctrl=<assign|inherit>;mon=<assign|inherit>" and above is "ctrl= and mon= show whether each is assigned or inherited". This is rewrites the same thing in different ways without helping to explain what the parameters and their values mean.

Documentation/filesystems/resctrl.rst conveyed in the error returns from file operations. E.g. · line 545

Quoted 3 lines

Documentation/filesystems/resctrl.rst +1 -0

+	  associated resource group using <CTRL_MON>/<MON>/ path syntax.

This text is difficult to parse. It switches between "assigned" and "associated" *in the same sentence* when referring to the same thing. Please use consistent terms and just be specific about what this actually by avoiding this vague language.

Documentation/filesystems/resctrl.rst conveyed in the error returns from file operations. E.g. · line 549

Quoted 5 lines

Documentation/filesystems/resctrl.rst +4 -0

+
+	Only supported modes are listed. On an inactive
+	assign_global_enable_per_cpu line, ctrl= and mon= show platform
+	capability defaults rather than the last active assignment, and

I find the text to be very vague. Above could be moved to section dedicated to the assign_global_enable_per_cpu mode and be made specific. For example,

When inactive, the parameter values displayed are the default values that will be used when no new value is provided during enabling/activation(*)

(*) pick a term and stick with it

Documentation/filesystems/resctrl.rst conveyed in the error returns from file operations. E.g. · line 552

Quoted 4 lines

Documentation/filesystems/resctrl.rst +3 -0

+	group=// is shown. When the mode is active, ctrl= and mon= report
+	the current assignment state and group= identifies the associated
+	resource group.

Last sentence just repeats the paragraph above, no?

Quoted 20 lines

Documentation/filesystems/resctrl.rst +7 -0

+
+	Example::
+
+	  # cat info/kernel_mode
+	  [inherit_user]
+	  assign_global_enable_per_cpu:ctrl=assign;mon=assign;group=//
+
  Resource alloc and monitor groups
  =================================
 

fs/resctrl/rdtgroup.c +0 -0

index 3bb0e3a203ac..992af586a194 100644
@@ -1010,6 +1010,114 @@ static int rdt_last_cmd_status_show(struct kernfs_open_file *of,
  	return 0;
  }
 

fs/resctrl/rdtgroup.c static int rdt_last_cmd_status_show(struct kernfs_open_file *of, · line 1014

Quoted 2 lines

fs/resctrl/rdtgroup.c +2 -0

+/* Sysfs lines for info/kernel_mode; indexed by enum resctrl_kernel_mode */
+static const char * const resctrl_mode_str[] = {

This is incredibly close to resctrl's existing rdtgroup_mode_str while being very high level. Could it be renamed to be more specific to this feature? For example, resctrl_kmode_str[]?

fs/resctrl/rdtgroup.c static int rdt_last_cmd_status_show(struct kernfs_open_file *of, · line 1016

Quoted 3 lines

fs/resctrl/rdtgroup.c +2 -0

+	[RESCTRL_INHERIT_USER]	=			"inherit_user",
+	[RESCTRL_ASSIGN_GLOBAL_ENABLE_PER_CPU] =	"assign_global_enable_per_cpu"

As mentioned earlier I am starting to think that the "assign" in the name is not necessary and creates confusion with the "assign" values of its parameters.

Quoted 38 lines

fs/resctrl/rdtgroup.c +37 -0

+};
+
+static_assert(ARRAY_SIZE(resctrl_mode_str) == RESCTRL_NUM_KERNEL_MODES);
+
+static const char *resctrl_kmode_state_str(enum kmode_state state)
+{
+	return state == KMODE_ASSIGN ? "assign" : "inherit";
+}
+
+static void resctrl_kmode_group_path(struct rdtgroup *rdtgrp,
+				     const char **ctrl, const char **mon)
+{
+	*ctrl = "";
+	*mon = "";
+
+	if (!rdtgrp)
+		return;
+
+	if (rdtgrp->type == RDTMON_GROUP) {
+		*ctrl = rdt_kn_name(rdtgrp->mon.parent->kn);
+		*mon = rdt_kn_name(rdtgrp->kn);
+	} else {
+		*ctrl = rdt_kn_name(rdtgrp->kn);
+	}
+}
+
+/**
+ * resctrl_kernel_mode_show() - Display supported and active kernel modes
+ * @of: kernfs open file
+ * @seq: output seq_file
+ * @v: unused
+ *
+ * Lists one line per mode set in resctrl_kcfg.caps.kmode_sup. Brackets the
+ * active mode. inherit_user is shown without any options.
+ * assign_global_enable_per_cpu is shown as:
+ *
+ *   assign_global_enable_per_cpu:ctrl=<assign|inherit>;mon=<assign|inherit>;\

fs/resctrl/rdtgroup.c static int rdt_last_cmd_status_show(struct kernfs_open_file *of, · line 1059

Quoted 6 lines

fs/resctrl/rdtgroup.c +6 -0

+ *					group=<ctrl>/<mon>/
+ *
+ * When assign_global_enable_per_cpu is inactive, ctrl=assign and mon=assign
+ * reflect resctrl_kcfg.caps.ctrl_en and resctrl_kcfg.caps.mon_en, and
+ * group=//. When active, assign state comes from resctrl_kcfg.active.ctrl_mode
+ * and resctrl_kcfg.active.mon_mode.

The code is much easier to read than above summary of it.

Quoted 28 lines

fs/resctrl/rdtgroup.c +27 -0

+ *
+ * Return: 0 on success, or -ENOENT on error.
+ */
+static int resctrl_kernel_mode_show(struct kernfs_open_file *of,
+				    struct seq_file *seq, void *v)
+{
+	const char *ctrl_state, *mon_state;
+	enum resctrl_kernel_mode mode;
+	struct rdtgroup *rdtgrp;
+	const char *ctrl, *mon;
+	bool active;
+	int ret = 0;
+
+	if (!info_kn_lock(of->kn))
+		return -ENOENT;
+
+	for (mode = 0; mode < RESCTRL_NUM_KERNEL_MODES; mode++) {
+		if (!test_bit(mode, resctrl_kcfg.caps.kmode_sup))
+			continue;
+
+		active = (resctrl_kcfg.active.kmode_cur == mode);
+
+		if (mode == RESCTRL_INHERIT_USER) {
+			seq_printf(seq, active ? "[%s]\n" : "%s\n",
+				   resctrl_mode_str[mode]);
+			continue;
+		}

fs/resctrl/rdtgroup.c static int rdt_last_cmd_status_show(struct kernfs_open_file *of, · line 1092

Quoted 6 lines

fs/resctrl/rdtgroup.c +6 -0

+
+		if (active) {
+			ctrl_state = resctrl_kmode_state_str(resctrl_kcfg.active.ctrl_mode);
+			mon_state = resctrl_kmode_state_str(resctrl_kcfg.active.mon_mode);
+			rdtgrp = resctrl_kcfg.active.k_rdtgrp;
+			if (WARN_ON(!rdtgrp)) {

Just pr_warn() is sufficient and avoids the discussion about panic_on_warn kernels.

fs/resctrl/rdtgroup.c static int rdt_last_cmd_status_show(struct kernfs_open_file *of, · line 1099

Quoted 8 lines

fs/resctrl/rdtgroup.c +7 -0

+				rdt_last_cmd_puts("Invalid kernel mode group\n");
+				ret = -ENOENT;
+				goto out_unlock;
+			}
+			resctrl_kmode_group_path(rdtgrp, &ctrl, &mon);
+		} else {
+			ctrl_state = resctrl_kcfg.caps.ctrl_en ? "assign" : "inherit";

This does not look right. As I understand ctrl_en represents whether the system supports allocation or not. So above means that if system supports allocation then the allocation state is "assign" while a system that does *not* support allocation inherits *allocation* association from user space?

fs/resctrl/rdtgroup.c static int rdt_last_cmd_status_show(struct kernfs_open_file *of, · line 1105

Quoted 7 lines

fs/resctrl/rdtgroup.c +6 -0

+			mon_state = resctrl_kcfg.caps.mon_en ? "assign" : "inherit";
+			ctrl = "";
+			mon = "";
+		}
+
+		if (active) {

Why are there two separate "if (active)" blocks?

fs/resctrl/rdtgroup.c static int rdt_last_cmd_status_show(struct kernfs_open_file *of, · line 1106

Quoted 2 lines

fs/resctrl/rdtgroup.c +1 -0

+			seq_printf(seq, "[%s:ctrl=%s;mon=%s;group=%s/%s/]\n",

This does not look right. The reason why this version moved to a single global mode with parameters is to be able to support systems that do not support allocation or monitoring. Having these separate parameters thus enables resctrl to only show the "ctrl=" parameter on a system that only supports allocation and only show the "mon=" parameter on a system that only supports monitoring. Above just keeps showing both whether system supports allocation/monitoring or not.

Hi Babu,

Quoted 4 lines · Babu Moger

fs/resctrl/rdtgroup.c +0 -0

@@ -2377,6 +2380,33 @@ static int rdtgroup_add_files(struct kernfs_node *kn, unsigned long fflags)
  	return ret;
  }
 

fs/resctrl/rdtgroup.c static int rdtgroup_add_files(struct kernfs_node *kn, unsigned long fflags) · line 2389

Quoted 7 lines

fs/resctrl/rdtgroup.c +7 -0

+/*
+ * resctrl_hidden_files_set_visible() - Show or hide files marked hidden
+ * @kn: resource group kernfs_node
+ * @show: whether to show or hide
+ *
+ * Iterate res_common_files entries marked hidden and show or hide the
+ * corresponding file under @kn.

As a generic helper this is not appropriate since it just blindly makes *all* hidden files belonging to a resource group visible or invisible. This pretends to be a generic function but then assumes that only kernel mode related files are ever created with the hidden flag set because as the kernel mode changes all hidden files belonging to the named parent changes visibility.

Hi Reinette,

Quoted 20 lines · Reinette Chatre

Hi Babu,

Quoted 10 lines · Babu Moger

(quoted diff) +0 -0

@@ -2377,6 +2380,33 @@ static int rdtgroup_add_files(struct kernfs_node *kn, unsigned long fflags)
       return ret;
   }

  +/*
+ * resctrl_hidden_files_set_visible() - Show or hide files marked hidden
+ * @kn: resource group kernfs_node
+ * @show: whether to show or hide
+ *
+ * Iterate res_common_files entries marked hidden and show or hide the
+ * corresponding file under @kn.

As a generic helper this is not appropriate since it just blindly makes *all* hidden files belonging to a resource group visible or invisible. This pretends to be a generic function but then assumes that only kernel mode related files are ever created with the hidden flag set because as the kernel mode changes all hidden files belonging to the named parent changes visibility.

Are you suggesting that I update the code comments to be more specific about the group, or are you suggesting that the code itself should be changed?

Hi Babu,

Quoted 24 lines · Babu Moger

Hi Reinette,

Quoted 20 lines · Reinette Chatre

Hi Babu,

fs/resctrl/rdtgroup.c static int rdtgroup_add_files(struct kernfs_node *kn, unsigned long fflags) · line 2388

Quoted 10 lines · Babu Moger

fs/resctrl/rdtgroup.c +6 -0

@@ -2377,6 +2380,33 @@ static int rdtgroup_add_files(struct kernfs_node *kn, unsigned long fflags)
       return ret;
   }
   +/*
+ * resctrl_hidden_files_set_visible() - Show or hide files marked hidden
+ * @kn: resource group kernfs_node
+ * @show: whether to show or hide
+ *
+ * Iterate res_common_files entries marked hidden and show or hide the
+ * corresponding file under @kn.

As a generic helper this is not appropriate since it just blindly makes *all* hidden files belonging to a resource group visible or invisible. This pretends to be a generic function but then assumes that only kernel mode related files are ever created with the hidden flag set because as the kernel mode changes all hidden files belonging to the named parent changes visibility.

Quoted 3 lines

Are you suggesting that I update the code comments to be more specific about the group, or are you suggesting that the code itself should be changed?

Both. The "hidden" flag is *only* about whether a file should be visible at time of creation, whether it is by default "hidden". That is accomplished without this function.

Whether a file is visible or not after initial creation is feature specific. For example, as is done with resctrl_bmec_files_show(). There is no use case for "make all hidden files visible".

resctrl_hidden_files_set_visible() is a hack because it pretends to introduce a generic feature but the implementation only supports two specific files. Consider the scenario when somebody tries to add another resource group file that is by default hidden. Just setting the "hidden" flag will be a reasonable thing to do, but that unrelated file will have its visibility connected to the kernel mode feature!

Please replace resctrl_hidden_files_set_visible() with a kernel mode specific function that only controls visibility of this feature's specific files. That could even be in a separate patch.

 fs/resctrl/internal.h |  5 +++++
 fs/resctrl/rdtgroup.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+)

fs/resctrl/internal.h +0 -0

index b56f95625072..01f9f596f9b1 100644

fs/resctrl/internal.h +3 -0

+ * @kmode_cpu_mask:		CPUs in this group's kernel-mode association
  * @plr:			pseudo-locked region
  */
 struct rdtgroup {

@@ -229,6 +232,8 @@ struct rdtgroup {
 	struct mongroup			mon;
 	enum rdtgrp_mode		mode;
 	enum resctrl_event_id		mba_mbps_event;
+	bool				kmode;
+	struct cpumask			kmode_cpu_mask;
 	struct pseudo_lock_region	*plr;
 };
 

fs/resctrl/rdtgroup.c +6 -0

index d2fff8adf915..06e74b027044 100644
@@ -389,6 +389,32 @@ static int rdtgroup_cpus_show(struct kernfs_open_file *of,
 	return ret;
 }
 
+/*
+ * Display CPU masks for the kernel-mode associated resource group.
+ * Supports both "kmode_cpus" (bitmap format) and "kmode_cpus_list"
+ * (range list format); the output format is selected accordingly.
+ *
+ * Returns -ENOENT on error.

fs/resctrl/rdtgroup.c +20 -0

+		seq_printf(s, is_cpu_list(of) ? "%*pbl\n" : "%*pb\n",
+			   cpumask_pr_args(&rdtgrp->kmode_cpu_mask));
+	} else {
+		ret = -ENOENT;
+	}
+	rdtgroup_kn_unlock(of->kn);
+
+	return ret;
+}
+
 /*
  * Update the PGR_ASSOC MSR on all cpus in @cpu_mask,
  *

@@ -2288,6 +2314,23 @@ static struct rftype res_common_files[] = {
 		.flags		= RFTYPE_FLAGS_CPUS_LIST,
 		.fflags		= RFTYPE_BASE,
 	},
+	{
+		.name		= "kmode_cpus",
+		.mode		= 0444,
+		.hidden		= true,
+		.kf_ops		= &rdtgroup_kf_single_ops,
+		.seq_show	= rdtgroup_kmode_cpus_show,
+		.fflags		= RFTYPE_BASE,
+	},
+	{
+		.name		= "kmode_cpus_list",

Hi Babu,

Quoted 4 lines · Babu Moger

  fs/resctrl/internal.h |  5 +++++
  fs/resctrl/rdtgroup.c | 43 +++++++++++++++++++++++++++++++++++++++++++
  2 files changed, 48 insertions(+)

fs/resctrl/internal.h struct mongroup { · line 220

Quoted 10 lines

fs/resctrl/internal.h +2 -0

index b56f95625072..01f9f596f9b1 100644
@@ -216,6 +216,9 @@ struct mongroup {
   * @mon:			mongroup related data
   * @mode:			mode of resource group
   * @mba_mbps_event:		input monitoring event id when mba_sc is enabled
+ * @kmode:			true if this group has an active kernel-mode
+ *				association

This really just seems specific to the global per-CPU kernel mode. Why is it necessary? Is resctrl_kcfg.active.k_rdtgrp not sufficient by itself? (more below)

Quoted 28 lines

fs/resctrl/internal.h +3 -0

+ * @kmode_cpu_mask:		CPUs in this group's kernel-mode association
   * @plr:			pseudo-locked region
   */
  struct rdtgroup {

@@ -229,6 +232,8 @@ struct rdtgroup {
  	struct mongroup			mon;
  	enum rdtgrp_mode		mode;
  	enum resctrl_event_id		mba_mbps_event;
+	bool				kmode;
+	struct cpumask			kmode_cpu_mask;
  	struct pseudo_lock_region	*plr;
  };
 

fs/resctrl/rdtgroup.c +6 -0

index d2fff8adf915..06e74b027044 100644
@@ -389,6 +389,32 @@ static int rdtgroup_cpus_show(struct kernfs_open_file *of,
  	return ret;
  }
 
+/*
+ * Display CPU masks for the kernel-mode associated resource group.
+ * Supports both "kmode_cpus" (bitmap format) and "kmode_cpus_list"
+ * (range list format); the output format is selected accordingly.
+ *
+ * Returns -ENOENT on error.

fs/resctrl/rdtgroup.c static int rdtgroup_cpus_show(struct kernfs_open_file *of, · line 407

Quoted 10 lines

fs/resctrl/rdtgroup.c +10 -0

+ */
+static int rdtgroup_kmode_cpus_show(struct kernfs_open_file *of,
+				    struct seq_file *s, void *v)
+{
+	struct rdtgroup *rdtgrp;
+	int ret = 0;
+
+	rdtgrp = rdtgroup_kn_lock_live(of->kn);
+
+	if (rdtgrp && rdtgrp->kmode) {

First, it would be helpful to explain why the second test is needed since it is not obvious that a resource group may get re-assigned while a user has this file open.

Second, could a check of rdtgrp->kmode be replaced with a comparison between rdtgrp and resctrl_kcfg.active.k_rdtgrp ? If so, why is rdtgrp::kmode needed? I think one part of this is how this series blurs between a generic "kernel mode" and the specific only-supported global per-CPU kernel mode that makes it difficult to know what state can be used.

---
 fs/resctrl/rdtgroup.c | 59 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

fs/resctrl/rdtgroup.c +14 -0

index 06e74b027044..97e4176669e5 100644
@@ -4951,11 +4951,67 @@ int resctrl_online_mon_domain(struct rdt_resource *r, struct rdt_domain_hdr *hdr
 	return err;
 }
 
+/*
+ * resctrl_kmode_online_cpu() - Program kernel mode association for @cpu
+ * @cpu: CPU that has just been brought online
+ *
+ * If assign_global_enable_per_cpu is active and the rdtgroup has an active
+ * kernel-mode association, add @cpu to kmode_cpu_mask and program the
+ * corresponding kernel mode association.
+ */
+static void resctrl_kmode_online_cpu(unsigned int cpu)
+{
+	struct rdtgroup *rdtgrp = resctrl_kcfg.active.k_rdtgrp;
+	bool assign_ctrl, assign_mon;
+
+	if (resctrl_kcfg.active.kmode_cur != RESCTRL_ASSIGN_GLOBAL_ENABLE_PER_CPU ||

fs/resctrl/rdtgroup.c +26 -0

+}
+
+/*
+ * resctrl_kmode_offline_cpu() - Clear kernel mode association for @cpu
+ * @cpu: CPU being taken offline.
+ *
+ * If assign_global_enable_per_cpu is active, disable the kernel mode
+ * association for @cpu and remove it from kmode_cpu_mask.
+ */
+static void resctrl_kmode_offline_cpu(unsigned int cpu)
+{
+	struct rdtgroup *rdtgrp = resctrl_kcfg.active.k_rdtgrp;
+	bool assign_ctrl, assign_mon;
+
+	if (resctrl_kcfg.active.kmode_cur != RESCTRL_ASSIGN_GLOBAL_ENABLE_PER_CPU ||
+	    !rdtgrp || !rdtgrp->kmode)
+		return;
+
+	if (!cpumask_test_cpu(cpu, &rdtgrp->kmode_cpu_mask))
+		return;
+
+	assign_ctrl = (resctrl_kcfg.active.ctrl_mode == KMODE_ASSIGN);
+	assign_mon = (resctrl_kcfg.active.mon_mode == KMODE_ASSIGN);
+
+	cpumask_clear_cpu(cpu, &rdtgrp->kmode_cpu_mask);
+

fs/resctrl/rdtgroup.c +1 -0

+	resctrl_kmode_online_cpu(cpu);
 	mutex_unlock(&rdtgroup_mutex);
 }
 

Hi Babu,

Quoted 26 lines · Babu Moger

---
  fs/resctrl/rdtgroup.c | 59 +++++++++++++++++++++++++++++++++++++++++++
  1 file changed, 59 insertions(+)

fs/resctrl/rdtgroup.c +14 -0

index 06e74b027044..97e4176669e5 100644
@@ -4951,11 +4951,67 @@ int resctrl_online_mon_domain(struct rdt_resource *r, struct rdt_domain_hdr *hdr
  	return err;
  }
 
+/*
+ * resctrl_kmode_online_cpu() - Program kernel mode association for @cpu
+ * @cpu: CPU that has just been brought online
+ *
+ * If assign_global_enable_per_cpu is active and the rdtgroup has an active
+ * kernel-mode association, add @cpu to kmode_cpu_mask and program the
+ * corresponding kernel mode association.
+ */
+static void resctrl_kmode_online_cpu(unsigned int cpu)
+{
+	struct rdtgroup *rdtgrp = resctrl_kcfg.active.k_rdtgrp;
+	bool assign_ctrl, assign_mon;
+
+	if (resctrl_kcfg.active.kmode_cur != RESCTRL_ASSIGN_GLOBAL_ENABLE_PER_CPU ||

fs/resctrl/rdtgroup.c int resctrl_online_mon_domain(struct rdt_resource *r, struct rdt_domain_hdr *hdr · line 4977

Quoted 10 lines

fs/resctrl/rdtgroup.c +10 -0

+	    !rdtgrp || !rdtgrp->kmode)
+		return;
+
+	assign_ctrl = (resctrl_kcfg.active.ctrl_mode == KMODE_ASSIGN);
+	assign_mon = (resctrl_kcfg.active.mon_mode == KMODE_ASSIGN);
+
+	cpumask_set_cpu(cpu, &rdtgrp->kmode_cpu_mask);
+
+	resctrl_arch_configure_kmode(cpumask_of(cpu), rdtgrp->closid, assign_ctrl,
+				     rdtgrp->mon.rmid, assign_mon, true);

If I counted right, at end of this series, resctrl_arch_configure_kmode() is called six times and _every_ call is preceded with the same two line pattern above:

	assign_ctrl = (resctrl_kcfg.active.ctrl_mode == KMODE_ASSIGN);
	assign_mon = (resctrl_kcfg.active.mon_mode == KMODE_ASSIGN);

Could this be simplified with a single helper that does something like:

static void resctrl_configure_global_kmode(const struct cpumask *mask,
struct rdtgroup *rdtgrp, bool enable)

{
   	resctrl_arch_configure_global_kmode(mask, rdtgrp->closid,
   					    resctrl_kcfg.active.ctrl_mode == KMODE_ASSIGN,
					    rdtgrp->mon.rmid,
					    resctrl_kcfg.active.mon_mode == KMODE_ASSIGN,
					    enable);
}

Quoted 31 lines

fs/resctrl/rdtgroup.c +30 -0

+}
+
+/*
+ * resctrl_kmode_offline_cpu() - Clear kernel mode association for @cpu
+ * @cpu: CPU being taken offline.
+ *
+ * If assign_global_enable_per_cpu is active, disable the kernel mode
+ * association for @cpu and remove it from kmode_cpu_mask.
+ */
+static void resctrl_kmode_offline_cpu(unsigned int cpu)
+{
+	struct rdtgroup *rdtgrp = resctrl_kcfg.active.k_rdtgrp;
+	bool assign_ctrl, assign_mon;
+
+	if (resctrl_kcfg.active.kmode_cur != RESCTRL_ASSIGN_GLOBAL_ENABLE_PER_CPU ||
+	    !rdtgrp || !rdtgrp->kmode)
+		return;
+
+	if (!cpumask_test_cpu(cpu, &rdtgrp->kmode_cpu_mask))
+		return;
+
+	assign_ctrl = (resctrl_kcfg.active.ctrl_mode == KMODE_ASSIGN);
+	assign_mon = (resctrl_kcfg.active.mon_mode == KMODE_ASSIGN);
+
+	cpumask_clear_cpu(cpu, &rdtgrp->kmode_cpu_mask);
+
+	resctrl_arch_configure_kmode(cpumask_of(cpu), rdtgrp->closid, assign_ctrl,
+				     rdtgrp->mon.rmid, assign_mon, false);
+}
+

fs/resctrl/rdtgroup.c int resctrl_online_mon_domain(struct rdt_resource *r, struct rdt_domain_hdr *hdr · line 5013

Quoted 6 lines

fs/resctrl/rdtgroup.c +1 -0

  void resctrl_online_cpu(unsigned int cpu)
  {
  	mutex_lock(&rdtgroup_mutex);
  	/* The CPU is set in default rdtgroup after online. */
  	cpumask_set_cpu(cpu, &rdtgroup_default.cpu_mask);
+	/* Program any active kernel mode association on this CPU. */

This comment is not helpful.