[PATCH 2/4] resctrl: Fix PLZA RMID_EN to be mode-based and relax RDTMON_GROUP constraint for assign_mon

[Qinyun Tan <qinyuntan@linux.alibaba.com>]

Two issues identified during review of the upstream v3 PLZA patches,
confirmed by the patch author (Babu Moger):

1) resctrl_arch_configure_kmode() hardcodes rmid_en=1 for both
   inherit_mon and assign_mon modes.  Per the AMD hardware manual
   (Publication #69193), RMID_EN is a source selector: when set,
   CPL=0 uses the PLZA RMID; when clear, CPL=0 falls back to
   PQR_ASSOC[RMID] (the user process's RMID).  With rmid_en=1 in
   inherit_mon mode, kernel-mode traffic is always counted under the
   PLZA group's RMID, defeating the "inherit" semantics.

   Add a bool assign_rmid parameter to resctrl_arch_configure_kmode().
   Set rmid_en=1 only for assign_mon (GLOBAL_ASSIGN_CTRL_ASSIGN_MON_PER_CPU),
   and rmid_en=0 for inherit_mon (GLOBAL_ASSIGN_CTRL_INHERIT_MON_PER_CPU)
   so that kernel-mode RMID correctly inherits from PQR_ASSOC.

   The assign_rmid value is derived from the active kmode in all three
   call sites: rdtgroup_config_kmode(), rdtgroup_config_kmode_clear(),
   and kmode_cpus_write().

2) global_assign_ctrl_assign_mon_per_cpu (assign_mon) forces binding
   to an RDTMON_GROUP, requiring users to create an extra monitor
   sub-group.  Since the kernel-mode group is dedicated (no user tasks),
   the ctrl group's own RMID is clean and usable.  The extra monitor
   group wastes a scarce RMID for no benefit.

   Remove the RDTMON_GROUP constraint for assign_mon so it accepts
   both RDTCTRL_GROUP and RDTMON_GROUP.  The RDTCTRL_GROUP constraint
   for inherit_mon is kept (it needs only a CLOSID, not a separate
   RMID).

Upstream confirmation:
  https://lore.kernel.org/all/1d7c79bf-1e40-4db7-8f66-45f234b6d87e@amd.com/

Signed-off-by: Qinyun Tan <qinyuntan@linux.alibaba.com>
Reviewed-by: Xunlei Pang <xlpang@linux.alibaba.com>
---
 arch/x86/kernel/cpu/resctrl/ctrlmondata.c |  9 ++--
 drivers/resctrl/mpam_resctrl.c            |  2 +-
 fs/resctrl/rdtgroup.c                     | 55 ++++++++++++++---------
 include/linux/resctrl.h                   |  8 ++--
 4 files changed, 45 insertions(+), 29 deletions(-)

diff --git a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
index 5de02a45b58e2..98bc079024219 100644
--- a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
+++ b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
@@ -234,18 +234,21 @@ static void resctrl_kmode_set_one_amd(void *arg)
  * resctrl_arch_configure_kmode() - x86/AMD: program PLZA MSR on a CPU subset
  * @cpu_mask:	CPUs to receive the update (see on_each_cpu_mask() for online subset).
  * @closid:	CLOSID field written into the MSR with CLOSID_EN set.
- * @rmid:	RMID field written into the MSR with RMID_EN set.
+ * @rmid:	RMID field written into the MSR (only used when @assign_rmid is true).
+ * @assign_rmid: When true, set RMID_EN so CPL=0 uses the PLZA RMID; when false,
+ *		clear RMID_EN so CPL=0 inherits RMID from PQR_ASSOC.
  * @enable:	Value for the PLZA_EN split field.
  *
  * Context: Do not call with IRQs off or from IRQ context except as allowed for
  * on_each_cpu_mask(); see kernel/smp.c.
  */
-void resctrl_arch_configure_kmode(const struct cpumask *cpu_mask, u32 closid, u32 rmid, bool enable)
+void resctrl_arch_configure_kmode(const struct cpumask *cpu_mask, u32 closid,
+				  u32 rmid, bool assign_rmid, bool enable)
 {
 	union msr_pqr_plza_assoc plza = { 0 };
 
 	plza.split.rmid = rmid;
-	plza.split.rmid_en = 1;
+	plza.split.rmid_en = assign_rmid ? 1 : 0;
 	plza.split.closid = closid;
 	plza.split.closid_en = 1;
 	plza.split.plza_en = enable;
diff --git a/drivers/resctrl/mpam_resctrl.c b/drivers/resctrl/mpam_resctrl.c
index fc2b3f508c039..dcf0339d92e5b 100644
--- a/drivers/resctrl/mpam_resctrl.c
+++ b/drivers/resctrl/mpam_resctrl.c
@@ -166,7 +166,7 @@ void resctrl_arch_get_kmode_support(struct resctrl_kmode_cfg *kcfg)
 }
 
 void resctrl_arch_configure_kmode(const struct cpumask *cpu_mask, u32 closid,
-				  u32 rmid, bool enable)
+				  u32 rmid, bool assign_rmid, bool enable)
 {
 }
 
diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
index 0b2606f10e2e9..1d9bd34eb9cdd 100644
--- a/fs/resctrl/rdtgroup.c
+++ b/fs/resctrl/rdtgroup.c
@@ -457,6 +457,7 @@ static int rdtgroup_kmode_cpus_show(struct kernfs_open_file *of, struct seq_file
 static int kmode_cpus_write(struct rdtgroup *rdtgrp, cpumask_var_t newmask,
 			    cpumask_var_t tmpmask)
 {
+	bool assign_rmid = (resctrl_kcfg.kmode_cur == BIT(GLOBAL_ASSIGN_CTRL_ASSIGN_MON_PER_CPU));
 	u32 closid, rmid;
 
 	if (rdtgrp->type == RDTMON_GROUP) {
@@ -475,18 +476,22 @@ static int kmode_cpus_write(struct rdtgroup *rdtgrp, cpumask_var_t newmask,
 		 */
 		cpumask_andnot(tmpmask, cpu_online_mask, newmask);
 		if (!cpumask_empty(tmpmask))
-			resctrl_arch_configure_kmode(tmpmask, closid, rmid, false);
-		resctrl_arch_configure_kmode(newmask, closid, rmid, true);
+			resctrl_arch_configure_kmode(tmpmask, closid, rmid,
+						     assign_rmid, false);
+		resctrl_arch_configure_kmode(newmask, closid, rmid,
+					     assign_rmid, true);
 	} else {
 		/* CPUs dropped from this group: old & ~newmask. */
 		cpumask_andnot(tmpmask, &rdtgrp->kmode_cpu_mask, newmask);
 		if (!cpumask_empty(tmpmask))
-			resctrl_arch_configure_kmode(tmpmask, closid, rmid, false);
+			resctrl_arch_configure_kmode(tmpmask, closid, rmid,
+						     assign_rmid, false);
 
 		/* CPUs newly added: newmask & ~old. */
 		cpumask_andnot(tmpmask, newmask, &rdtgrp->kmode_cpu_mask);
 		if (!cpumask_empty(tmpmask))
-			resctrl_arch_configure_kmode(tmpmask, closid, rmid, true);
+			resctrl_arch_configure_kmode(tmpmask, closid, rmid,
+						     assign_rmid, true);
 	}
 
 	cpumask_copy(&rdtgrp->kmode_cpu_mask, newmask);
@@ -1305,29 +1310,32 @@ static void resctrl_kmode_files_set_visible(struct rdtgroup *rdtgrp, bool visibl
 /**
  * rdtgroup_config_kmode() - Push @rdtgrp's kernel CLOSID/RMID to hardware
  * @rdtgrp:	Resctrl group whose CLOSID/RMID should be programmed.
+ * @kmode:	Kernel-mode policy being activated, as a
+ *		BIT(&enum resctrl_kernel_modes) value.  Used to determine
+ *		the RMID_EN setting: BIT(GLOBAL_ASSIGN_CTRL_ASSIGN_MON_PER_CPU)
+ *		sets RMID_EN so CPL=0 uses the PLZA RMID; other modes clear it
+ *		so CPL=0 inherits the user process's RMID from PQR_ASSOC.
  *
  * Derives CLOSID/RMID from @rdtgrp->type:
  *   - RDTMON_GROUP: parent control group's CLOSID with the monitor group's RMID.
- *   - RDTCTRL_GROUP: the control group's own CLOSID and default RMID.
+ *   - RDTCTRL_GROUP: the control group's own CLOSID and RMID.
  *
  * Calls resctrl_arch_configure_kmode() with the kernel-mode binding enabled
  * on the online subset of @rdtgrp->kmode_cpu_mask (or all online CPUs when
  * that mask is empty), and disabled on the complementary online CPUs so
  * stale enable bits from a previously bound group are cleared in the same
- * reprogram step.  The caller (resctrl_kernel_mode_write()) is responsible
- * for validating that the (kmode, group type) pair is permitted before
- * invoking this helper.
+ * reprogram step.
  *
  * Context: Caller must hold rdtgroup_mutex.
  *
  * Return: 0 on success, -EINVAL for a pseudo-locked group, -ENOMEM if
  * cpumask allocation fails.
  */
-static int rdtgroup_config_kmode(struct rdtgroup *rdtgrp)
+static int rdtgroup_config_kmode(struct rdtgroup *rdtgrp, u32 kmode)
 {
 	cpumask_var_t enable_mask, disable_mask;
 	u32 closid, rmid;
-	bool need_disable;
+	bool need_disable, assign_rmid;
 
 	if (rdtgrp->mode == RDT_MODE_PSEUDO_LOCKED) {
 		rdt_last_cmd_puts("Resource group is pseudo-locked\n");
@@ -1363,11 +1371,15 @@ static int rdtgroup_config_kmode(struct rdtgroup *rdtgrp)
 		cpumask_andnot(disable_mask, cpu_online_mask, &rdtgrp->kmode_cpu_mask);
 	}
 
+	assign_rmid = (kmode == BIT(GLOBAL_ASSIGN_CTRL_ASSIGN_MON_PER_CPU));
+
 	if (!cpumask_empty(enable_mask))
-		resctrl_arch_configure_kmode(enable_mask, closid, rmid, true);
+		resctrl_arch_configure_kmode(enable_mask, closid, rmid,
+					     assign_rmid, true);
 
 	if (need_disable && !cpumask_empty(disable_mask))
-		resctrl_arch_configure_kmode(disable_mask, closid, rmid, false);
+		resctrl_arch_configure_kmode(disable_mask, closid, rmid,
+					     assign_rmid, false);
 
 	rdtgrp->kmode = true;
 	resctrl_kmode_files_set_visible(rdtgrp, true);
@@ -1409,6 +1421,7 @@ static int rdtgroup_config_kmode(struct rdtgroup *rdtgrp)
 static int rdtgroup_config_kmode_clear(struct rdtgroup *rdtgrp, int kmode)
 {
 	cpumask_var_t disable_mask;
+	bool assign_rmid;
 	u32 closid, rmid;
 
 	if (!rdtgrp)
@@ -1428,12 +1441,15 @@ static int rdtgroup_config_kmode_clear(struct rdtgroup *rdtgrp, int kmode)
 		rmid = rdtgrp->mon.rmid;
 	}
 
+	assign_rmid = (kmode == BIT(GLOBAL_ASSIGN_CTRL_ASSIGN_MON_PER_CPU));
+
 	if (cpumask_empty(&rdtgrp->kmode_cpu_mask))
 		cpumask_copy(disable_mask, cpu_online_mask);
 	else
 		cpumask_copy(disable_mask, &rdtgrp->kmode_cpu_mask);
 
-	resctrl_arch_configure_kmode(disable_mask, closid, rmid, false);
+	resctrl_arch_configure_kmode(disable_mask, closid, rmid,
+				     assign_rmid, false);
 	free_cpumask_var(disable_mask);
 
 out_clear:
@@ -1665,19 +1681,14 @@ static ssize_t resctrl_kernel_mode_write(struct kernfs_open_file *of,
 
 	/*
 	 * global_assign_ctrl_assign_mon_per_cpu binds one CLOSID and RMID for
-	 * all kernel work (Documentation/filesystems/resctrl.rst uses
-	 * "<ctrl>/<mon>/", i.e. an RDTMON_GROUP).
+	 * all kernel work.  Both RDTCTRL_GROUP and RDTMON_GROUP are accepted:
+	 * a control group is sufficient (its own CLOSID and RMID are used) and
+	 * avoids wasting an RMID on an unnecessary monitor sub-group.
 	 *
 	 * global_assign_ctrl_inherit_mon_per_cpu assigns one CLOSID globally
 	 * while leaving RMID inheritance to user contexts; that uses the
 	 * control group's CLOSID slot only, i.e. an RDTCTRL_GROUP.
 	 */
-	if (kmode == BIT(GLOBAL_ASSIGN_CTRL_ASSIGN_MON_PER_CPU) &&
-	    rdtgrp->type != RDTMON_GROUP) {
-		rdt_last_cmd_puts("global_assign_ctrl_assign_mon_per_cpu requires a monitor group\n");
-		ret = -EINVAL;
-		goto out_unlock;
-	}
 	if (kmode == BIT(GLOBAL_ASSIGN_CTRL_INHERIT_MON_PER_CPU) &&
 	    rdtgrp->type != RDTCTRL_GROUP) {
 		rdt_last_cmd_puts("global_assign_ctrl_inherit_mon_per_cpu requires a control group\n");
@@ -1693,7 +1704,7 @@ static ssize_t resctrl_kernel_mode_write(struct kernfs_open_file *of,
 	}
 
 	if (kmode != BIT(INHERIT_CTRL_AND_MON)) {
-		ret = rdtgroup_config_kmode(rdtgrp);
+		ret = rdtgroup_config_kmode(rdtgrp, kmode);
 		if (ret) {
 			rdt_last_cmd_puts("Kernel mode change failed\n");
 			goto out_unlock;
diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
index e2135f0d42506..256449a0c92ff 100644
--- a/include/linux/resctrl.h
+++ b/include/linux/resctrl.h
@@ -724,11 +724,13 @@ void resctrl_arch_get_kmode_support(struct resctrl_kmode_cfg *kcfg);
  * resctrl_arch_configure_kmode() - Program MSR_IA32_PQR_PLZA_ASSOC on CPUs in @cpu_mask
  * @cpu_mask:	Target CPUs; on_each_cpu_mask() applies the callback on the online subset.
  * @closid:	CLOSID written to the MSR with CLOSID_EN set.
- * @rmid:	RMID written to the MSR with RMID_EN set.
+ * @rmid:	RMID written to the MSR (only used when @assign_rmid is true).
+ * @assign_rmid: When true, set RMID_EN so CPL=0 uses the PLZA RMID; when false,
+ *		clear RMID_EN so CPL=0 inherits RMID from PQR_ASSOC.
  * @enable:	PLZA_EN field value for this update.
  */
-void resctrl_arch_configure_kmode(const struct cpumask *cpu_mask, u32 closid, u32 rmid,
-				  bool enable);
+void resctrl_arch_configure_kmode(const struct cpumask *cpu_mask, u32 closid,
+				  u32 rmid, bool assign_rmid, bool enable);
 
 extern unsigned int resctrl_rmid_realloc_threshold;
 extern unsigned int resctrl_rmid_realloc_limit;
-- 
2.43.7