[PATCH v4 0/3] arm_mpam: resctrl: Fix several CDP bugs

[PATCH v4 0/3] arm_mpam: resctrl: Fix several CDP bugs

[Jinjie Ruan]

Fix several mpam CDP bugs found by testing and AI review.

Changes in v4:
- Only enable CDP on a resource that supports it (Ben)
- Limit num_rmids to the CLOSIDs in use with a simplified fix (Ben)
- v3: https://lore.kernel.org/all/20261106081923.2333070-1-ruanjinjie@huawei.com/

Jinjie Ruan (3):
  arm_mpam: resctrl: Leave CDP off when there are too few PARTIDs
  arm_mpam: resctrl: Only enable CDP on a resource that supports it
  arm_mpam: resctrl: Limit num_rmids to the CLOSIDs in use

 drivers/resctrl/mpam_resctrl.c | 40 ++++++++++++++++++++--------------
 1 file changed, 24 insertions(+), 16 deletions(-)

-- 
2.34.1

[PATCH v4 1/3] arm_mpam: resctrl: Leave CDP off when there are too few PARTIDs

[Jinjie Ruan]

Enabling CDP requires at least 2 PARTIDs. If there are too few PARTIDs,
resctrl_arch_set_cdp_enabled() returns -EINVAL, but only after it has
recorded CDP as enabled, halved the monitor count and turned off the
mbw_max MBA control. resctrl then believes CDP is off, which leaves the
two sides out of step and the halved monitor count and the missing MBA
control are not undone.

Check the PARTID limit before anything is changed so a failed enable has
no effect.

Fixes: 6789fb99282c ("arm_mpam: resctrl: Add CDP emulation")
Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
Cc: <stable@vger.kernel.org> # 7.1+
---
 drivers/resctrl/mpam_resctrl.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/resctrl/mpam_resctrl.c b/drivers/resctrl/mpam_resctrl.c
index 563ca99a581d..573617ba09df 100644
--- a/drivers/resctrl/mpam_resctrl.c
+++ b/drivers/resctrl/mpam_resctrl.c
@@ -219,6 +219,9 @@ int resctrl_arch_set_cdp_enabled(enum resctrl_res_level rid, bool enable)
 	if (enable)
 		pr_warn("CDP is an expert feature and may cause MPAM to malfunction.\n");
 
+	if (enable && mpam_partid_max < 1)
+		return -EINVAL;
+
 	/*
 	 * resctrl_arch_set_cdp_enabled() is only called with enable set to
 	 * false on error and unmount.
@@ -248,9 +251,6 @@ int resctrl_arch_set_cdp_enabled(enum resctrl_res_level rid, bool enable)
 		mpam_resctrl_controls[RDT_RESOURCE_MBA].resctrl_res.alloc_capable = true;
 
 	if (enable) {
-		if (mpam_partid_max < 1)
-			return -EINVAL;
-
 		partid_d = resctrl_get_config_index(RESCTRL_RESERVED_CLOSID, CDP_DATA);
 		partid_i = resctrl_get_config_index(RESCTRL_RESERVED_CLOSID, CDP_CODE);
 	}
-- 
2.34.1

[PATCH v4 2/3] arm_mpam: resctrl: Only enable CDP on a resource that supports it

[Jinjie Ruan]

resctrl_arch_set_cdp_enabled() enables CDP for any cache level that is
requested, even one that has no MPAM cache controls. Mounting resctrl
with -o cdpl2 when only L3 can allocate still turns CDP emulation on for
the whole system, so every CLOSID takes two PARTIDs while resctrl keeps
its CLOSID count at the number of PARTIDs. Control groups in the upper
half then map to PARTIDs beyond mpam_partid_max, which index past the
configuration and monitor arrays and program out of range PARTIDs into
the MSCs. On QEMU with 5 PARTIDs the first monitor read after creating
2 control groups raises MSMONCFG_ID_RANGE for PARTID 5, and MPAM is
disabled.

Do not enable CDP when a resource cannot allocate or is not CDP capable,
as x86 does, so the mount fails instead. Only the enable path is checked,
since unmount disables CDP on both cache levels to restore the MBA control.

A system where only MBA can allocate, or that only has monitors, now
fails a mount with CDP as well: CDP hides the MBA control because
mbw_max is a per-PARTID maximum, leaving nothing for CDP to apply to.

Fixes: 6789fb99282c ("arm_mpam: resctrl: Add CDP emulation")
Link: https://sashiko.dev/#/bug/linux-05c19551-0bba-4a06-aa2c-3c90fab5853d
Suggested-by: Ben Horgan <ben.horgan@arm.com>
Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
Cc: <stable@vger.kernel.org> # 7.1+
---
 drivers/resctrl/mpam_resctrl.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/resctrl/mpam_resctrl.c b/drivers/resctrl/mpam_resctrl.c
index 573617ba09df..afa3c084c475 100644
--- a/drivers/resctrl/mpam_resctrl.c
+++ b/drivers/resctrl/mpam_resctrl.c
@@ -222,6 +222,10 @@ int resctrl_arch_set_cdp_enabled(enum resctrl_res_level rid, bool enable)
 	if (enable && mpam_partid_max < 1)
 		return -EINVAL;
 
+	if (enable && (!mpam_resctrl_controls[rid].resctrl_res.alloc_capable ||
+		       !mpam_resctrl_controls[rid].resctrl_res.cdp_capable))
+		return -EINVAL;
+
 	/*
 	 * resctrl_arch_set_cdp_enabled() is only called with enable set to
 	 * false on error and unmount.
-- 
2.34.1

[PATCH v4 3/3] arm_mpam: resctrl: Limit num_rmids to the CLOSIDs in use

[Jinjie Ruan]

On MPAM an RMID is a PMG within a PARTID, so resctrl can only allocate
the PMGs of the CLOSIDs it uses. info/L3_MON/num_rmids is instead set
to the whole PARTID and PMG space when the monitors are initialised,
and halved when CDP is enabled. It reports more RMIDs than resctrl can
allocate in two cases:

- With CDP each CLOSID takes two PARTIDs, and resctrl halves the
  CLOSID count rounding down, while MPAM halves the whole RMID space.
  With 5 PARTIDs and 2 PMGs resctrl has 2 CLOSIDs and 4 RMIDs, but
  num_rmids reads 5. The count is too high whenever the number of
  PARTIDs is odd and there is more than one PMG.

- On a monitor-only system there are no control groups and resctrl
  only uses the default CLOSID, but num_rmids still covers every
  PARTID. With 5 PARTIDs and 2 PMGs it reads 10 while only 2 RMIDs
  can be allocated.

RMID allocation is limited to the CLOSIDs in use, so only the value
reported to userspace is too high. Creating as many monitor groups as
num_rmids allows fails with -ENOSPC before that number is reached.

Count the CLOSIDs resctrl uses instead: every resource has
mpam_partid_max + 1 CLOSIDs, halved and rounded down while CDP is
enabled, and a system with no resource that can allocate uses only the
default CLOSID. Report (pmg_max + 1) RMIDs for each of those CLOSIDs.
Count at monitor initialisation and from resctrl_arch_set_cdp_enabled(),
which resctrl calls whenever a mount or unmount enables or disables
CDP.

Fixes: 264c285999fc ("arm_mpam: resctrl: Add monitor initialisation and domain boilerplate")
Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
Cc: <stable@vger.kernel.org> # 7.1+
Cc: James Morse <james.morse@arm.com>
Cc: Ben Horgan <ben.horgan@arm.com>
Cc: Reinette Chatre <reinette.chatre@intel.com>
Cc: Fenghua Yu <fenghuay@nvidia.com>
---
v4:
- Simplify the fix with mpam_partid_max, cdp_enabled
  and resctrl_arch_alloc_capable() as Ben suggested.
v3:
- Drop resctrl_arch_set_mon_rmid_count() and count the CLOSIDs in the
  MPAM driver instead, as Ben suggested.
- v2: https://lore.kernel.org/all/20261105123304.2281987-1-ruanjinjie@huawei.com/
v2:
- Also fix num_rmids on monitor-only systems, as Ben suggested.
- Retitle from "arm_mpam: Round the CDP RMID count down to whole CLOSIDs".
- v1: https://lore.kernel.org/all/20261104101622.563280-1-ruanjinjie@huawei.com/
---
 drivers/resctrl/mpam_resctrl.c | 30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/drivers/resctrl/mpam_resctrl.c b/drivers/resctrl/mpam_resctrl.c
index afa3c084c475..fcd41ddffc74 100644
--- a/drivers/resctrl/mpam_resctrl.c
+++ b/drivers/resctrl/mpam_resctrl.c
@@ -200,6 +200,21 @@ static void mpam_resctrl_monitor_sync_abmc_vals(struct rdt_resource *l3)
 	 */
 }
 
+/* The RMID count follows the CLOSIDs resctrl uses. */
+static void mpam_resctrl_set_num_rmid(void)
+{
+	struct rdt_resource *l3 = &mpam_resctrl_controls[RDT_RESOURCE_L3].resctrl_res;
+	u32 num_closid = 1;
+
+	if (resctrl_arch_alloc_capable()) {
+		num_closid = mpam_partid_max + 1;
+		if (cdp_enabled)
+			num_closid /= 2;
+	}
+
+	l3->mon.num_rmid = (mpam_pmg_max + 1) * num_closid;
+}
+
 int resctrl_arch_set_cdp_enabled(enum resctrl_res_level rid, bool enable)
 {
 	u32 partid_i = RESCTRL_RESERVED_CLOSID, partid_d = RESCTRL_RESERVED_CLOSID;
@@ -233,10 +248,7 @@ int resctrl_arch_set_cdp_enabled(enum resctrl_res_level rid, bool enable)
 	cdp_enabled = enable;
 	mpam_resctrl_controls[rid].cdp_enabled = enable;
 
-	if (enable)
-		l3->mon.num_rmid = resctrl_arch_system_num_rmid_idx() / 2;
-	else
-		l3->mon.num_rmid = resctrl_arch_system_num_rmid_idx();
+	mpam_resctrl_set_num_rmid();
 
 	/* The mbw_max feature can't hide cdp as it's a per-partid maximum. */
 	if (cdp_enabled && !mpam_resctrl_controls[RDT_RESOURCE_MBA].cdp_enabled)
@@ -1357,15 +1369,7 @@ static int mpam_resctrl_monitor_init(struct mpam_resctrl_mon *mon,
 	l3->name = "L3";
 	l3->mon_scope = RESCTRL_L3_CACHE;
 
-	/*
-	 * num-rmid is the upper bound for the number of monitoring groups that
-	 * can exist simultaneously, including the default monitoring group for
-	 * each control group. Hence, advertise the whole rmid_idx space even
-	 * though each control group has its own pmg/rmid space. Unfortunately,
-	 * this does mean userspace needs to know the architecture to correctly
-	 * interpret this value.
-	 */
-	l3->mon.num_rmid = resctrl_arch_system_num_rmid_idx();
+	mpam_resctrl_set_num_rmid();
 
 	if (type == QOS_L3_MBM_TOTAL_EVENT_ID) {
 		int err;
-- 
2.34.1

Re: [PATCH v4 1/3] arm_mpam: resctrl: Leave CDP off when there are too few PARTIDs

[Gavin Shan]

On 11/11/26 6:13 PM, Jinjie Ruan wrote:
> Enabling CDP requires at least 2 PARTIDs. If there are too few PARTIDs,
> resctrl_arch_set_cdp_enabled() returns -EINVAL, but only after it has
> recorded CDP as enabled, halved the monitor count and turned off the
> mbw_max MBA control. resctrl then believes CDP is off, which leaves the
> two sides out of step and the halved monitor count and the missing MBA
> control are not undone.
> 
> Check the PARTID limit before anything is changed so a failed enable has
> no effect.
> 
> Fixes: 6789fb99282c ("arm_mpam: resctrl: Add CDP emulation")
> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> Cc: <stable@vger.kernel.org> # 7.1+
> ---
>   drivers/resctrl/mpam_resctrl.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
Reviewed-by: Gavin Shan <gshan@redhat.com>

Re: [PATCH v4 2/3] arm_mpam: resctrl: Only enable CDP on a resource that supports it

[Gavin Shan]

On 11/11/26 6:13 PM, Jinjie Ruan wrote:
> resctrl_arch_set_cdp_enabled() enables CDP for any cache level that is
> requested, even one that has no MPAM cache controls. Mounting resctrl
> with -o cdpl2 when only L3 can allocate still turns CDP emulation on for
> the whole system, so every CLOSID takes two PARTIDs while resctrl keeps
> its CLOSID count at the number of PARTIDs. Control groups in the upper
> half then map to PARTIDs beyond mpam_partid_max, which index past the
> configuration and monitor arrays and program out of range PARTIDs into
> the MSCs. On QEMU with 5 PARTIDs the first monitor read after creating
> 2 control groups raises MSMONCFG_ID_RANGE for PARTID 5, and MPAM is
> disabled.
> 
> Do not enable CDP when a resource cannot allocate or is not CDP capable,
> as x86 does, so the mount fails instead. Only the enable path is checked,
> since unmount disables CDP on both cache levels to restore the MBA control.
> 
> A system where only MBA can allocate, or that only has monitors, now
> fails a mount with CDP as well: CDP hides the MBA control because
> mbw_max is a per-PARTID maximum, leaving nothing for CDP to apply to.
> 
> Fixes: 6789fb99282c ("arm_mpam: resctrl: Add CDP emulation")
> Link: https://sashiko.dev/#/bug/linux-05c19551-0bba-4a06-aa2c-3c90fab5853d
> Suggested-by: Ben Horgan <ben.horgan@arm.com>
> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> Cc: <stable@vger.kernel.org> # 7.1+
> ---
>   drivers/resctrl/mpam_resctrl.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/resctrl/mpam_resctrl.c b/drivers/resctrl/mpam_resctrl.c
> index 573617ba09df..afa3c084c475 100644
> --- a/drivers/resctrl/mpam_resctrl.c
> +++ b/drivers/resctrl/mpam_resctrl.c
> @@ -222,6 +222,10 @@ int resctrl_arch_set_cdp_enabled(enum resctrl_res_level rid, bool enable)
>   	if (enable && mpam_partid_max < 1)
>   		return -EINVAL;
>   
> +	if (enable && (!mpam_resctrl_controls[rid].resctrl_res.alloc_capable ||
> +		       !mpam_resctrl_controls[rid].resctrl_res.cdp_capable))
> +		return -EINVAL;
> +

x86 only checks 'cdp_capable', so this check isn't exactly same as to what's
done on x86, as claimed in the commit log?

int resctrl_arch_set_cdp_enabled(enum resctrl_res_level l, bool enable)
{
         struct rdt_hw_resource *hw_res = &rdt_resources_all[l];

         if (!hw_res->r_resctrl.cdp_capable)
                 return -EINVAL;

         :
}


>   	/*
>   	 * resctrl_arch_set_cdp_enabled() is only called with enable set to
>   	 * false on error and unmount.

Thanks,
Gavin

Re: [PATCH v4 3/3] arm_mpam: resctrl: Limit num_rmids to the CLOSIDs in use

[Gavin Shan]

On 11/11/26 6:13 PM, Jinjie Ruan wrote:
> On MPAM an RMID is a PMG within a PARTID, so resctrl can only allocate
> the PMGs of the CLOSIDs it uses. info/L3_MON/num_rmids is instead set
> to the whole PARTID and PMG space when the monitors are initialised,
> and halved when CDP is enabled. It reports more RMIDs than resctrl can
> allocate in two cases:
> 
> - With CDP each CLOSID takes two PARTIDs, and resctrl halves the
>    CLOSID count rounding down, while MPAM halves the whole RMID space.
>    With 5 PARTIDs and 2 PMGs resctrl has 2 CLOSIDs and 4 RMIDs, but
>    num_rmids reads 5. The count is too high whenever the number of
>    PARTIDs is odd and there is more than one PMG.
> 
> - On a monitor-only system there are no control groups and resctrl
>    only uses the default CLOSID, but num_rmids still covers every
>    PARTID. With 5 PARTIDs and 2 PMGs it reads 10 while only 2 RMIDs
>    can be allocated.
> 
> RMID allocation is limited to the CLOSIDs in use, so only the value
> reported to userspace is too high. Creating as many monitor groups as
> num_rmids allows fails with -ENOSPC before that number is reached.
> 
> Count the CLOSIDs resctrl uses instead: every resource has
> mpam_partid_max + 1 CLOSIDs, halved and rounded down while CDP is
> enabled, and a system with no resource that can allocate uses only the
> default CLOSID. Report (pmg_max + 1) RMIDs for each of those CLOSIDs.
> Count at monitor initialisation and from resctrl_arch_set_cdp_enabled(),
> which resctrl calls whenever a mount or unmount enables or disables
> CDP.
> 
> Fixes: 264c285999fc ("arm_mpam: resctrl: Add monitor initialisation and domain boilerplate")
> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> Cc: <stable@vger.kernel.org> # 7.1+
> Cc: James Morse <james.morse@arm.com>
> Cc: Ben Horgan <ben.horgan@arm.com>
> Cc: Reinette Chatre <reinette.chatre@intel.com>
> Cc: Fenghua Yu <fenghuay@nvidia.com>
> ---
> v4:
> - Simplify the fix with mpam_partid_max, cdp_enabled
>    and resctrl_arch_alloc_capable() as Ben suggested.
> v3:
> - Drop resctrl_arch_set_mon_rmid_count() and count the CLOSIDs in the
>    MPAM driver instead, as Ben suggested.
> - v2: https://lore.kernel.org/all/20261105123304.2281987-1-ruanjinjie@huawei.com/
> v2:
> - Also fix num_rmids on monitor-only systems, as Ben suggested.
> - Retitle from "arm_mpam: Round the CDP RMID count down to whole CLOSIDs".
> - v1: https://lore.kernel.org/all/20261104101622.563280-1-ruanjinjie@huawei.com/
> ---
>   drivers/resctrl/mpam_resctrl.c | 30 +++++++++++++++++-------------
>   1 file changed, 17 insertions(+), 13 deletions(-)

Reviewed-by: Gavin Shan <gshan@redhat.com>