[PATCH] arm_mpam: Only program MATCH_PMG when the monitor supports it

[PATCH] arm_mpam: Only program MATCH_PMG when the monitor supports it

[Ping Li]

An MSC of ours implements the MBWU monitor without PMG support:
MPAMF_MBWUMON_IDR.NO_MATCH_PMG is 1, so MSMON_CFG_x_CTL.MATCH_PMG is
RES0. On this hardware resctrl monitor reads always returned 0.

Tracking this down: __read_mon() unconditionally sets match_pmg,
because the resctrl (closid, rmid) model maps rmid to PMG. The
expected configuration generated by gen_msmon_ctl_flt_vals() then
contains MATCH_PMG, but the hardware ignores the bit, so the
configuration read back never matches, config_mismatch is always
true and the monitor is reprogrammed on every read - the counter
never accumulates.

The architecture allows an MSMON implementation without PMG
support, so read the NO_MATCH_PMG bits during probe and only
program MATCH_PMG when the monitor supports PMG filtering,
falling back to filtering by PARTID only. The CSU monitor has
the same issue and is covered the same way.

Signed-off-by: Ping Li <leeonion.muyu@gmail.com>
---
 drivers/resctrl/mpam_devices.c  | 22 +++++++++++++++++++++-
 drivers/resctrl/mpam_internal.h |  4 ++++
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
index c067b6d57d71..2086ff121b59 100644
--- a/drivers/resctrl/mpam_devices.c
+++ b/drivers/resctrl/mpam_devices.c
@@ -1089,6 +1089,9 @@ static int mpam_ris_hw_probe(struct mpam_msc_ris *ris)
 				if (FIELD_GET(MPAMF_CSUMON_IDR_HAS_XCL, csumonidr))
 					mpam_set_feature(mpam_feat_msmon_csu_xcl, props);
 
+				if (!FIELD_GET(MPAMF_CSUMON_IDR_NO_MATCH_PMG, csumonidr))
+					mpam_set_feature(mpam_feat_msmon_csu_match_pmg, props);
+
 				/* Is NRDY hardware managed? */
 				err = mpam_ris_hw_probe_csu_nrdy(ris);
 				if (err < 0)
@@ -1118,6 +1121,9 @@ static int mpam_ris_hw_probe(struct mpam_msc_ris *ris)
 				if (FIELD_GET(MPAMF_MBWUMON_IDR_HAS_RWBW, mbwumon_idr))
 					mpam_set_feature(mpam_feat_msmon_mbwu_rwbw, props);
 
+				if (!FIELD_GET(MPAMF_MBWUMON_IDR_NO_MATCH_PMG, mbwumon_idr))
+					mpam_set_feature(mpam_feat_msmon_mbwu_match_pmg, props);
+
 				has_long = FIELD_GET(MPAMF_MBWUMON_IDR_HAS_LONG, mbwumon_idr);
 				if (has_long) {
 					if (FIELD_GET(MPAMF_MBWUMON_IDR_LWD, mbwumon_idr))
@@ -1309,6 +1315,20 @@ static int mpam_msc_zero_mbwu_l(struct mpam_msc *msc)
 	return __mpam_write_reg(msc, MSMON_MBWU_L + 4, 0);
 }
 
+static bool msmon_supports_match_pmg(struct mon_read *m)
+{
+	switch (m->type) {
+	case mpam_feat_msmon_csu:
+		return mpam_has_feature(mpam_feat_msmon_csu_match_pmg, &m->ris->props);
+	case mpam_feat_msmon_mbwu_31counter:
+	case mpam_feat_msmon_mbwu_44counter:
+	case mpam_feat_msmon_mbwu_63counter:
+		return mpam_has_feature(mpam_feat_msmon_mbwu_match_pmg, &m->ris->props);
+	default:
+		return true;
+	}
+}
+
 static void gen_msmon_ctl_flt_vals(struct mon_read *m, u32 *ctl_val,
 				   u32 *flt_val)
 {
@@ -1322,7 +1342,7 @@ static void gen_msmon_ctl_flt_vals(struct mon_read *m, u32 *ctl_val,
 
 	*flt_val = FIELD_PREP(MSMON_CFG_x_FLT_PARTID, ctx->partid);
 
-	if (m->ctx->match_pmg) {
+	if (m->ctx->match_pmg && msmon_supports_match_pmg(m)) {
 		*ctl_val |= MSMON_CFG_x_CTL_MATCH_PMG;
 		*flt_val |= FIELD_PREP(MSMON_CFG_x_FLT_PMG, ctx->pmg);
 	}
diff --git a/drivers/resctrl/mpam_internal.h b/drivers/resctrl/mpam_internal.h
index 1f86cc158100..88cd5ac4cde3 100644
--- a/drivers/resctrl/mpam_internal.h
+++ b/drivers/resctrl/mpam_internal.h
@@ -227,12 +227,14 @@ enum mpam_device_features {
 	mpam_feat_msmon_csu,
 	mpam_feat_msmon_csu_capture,
 	mpam_feat_msmon_csu_xcl,
+	mpam_feat_msmon_csu_match_pmg,
 	mpam_feat_msmon_mbwu,
 	mpam_feat_msmon_mbwu_31counter,
 	mpam_feat_msmon_mbwu_44counter,
 	mpam_feat_msmon_mbwu_63counter,
 	mpam_feat_msmon_mbwu_capture,
 	mpam_feat_msmon_mbwu_rwbw,
+	mpam_feat_msmon_mbwu_match_pmg,
 	mpam_feat_partid_nrw,
 	MPAM_FEATURE_LAST
 };
@@ -645,6 +647,7 @@ int mpam_fb_check_shared_buffer_size(struct mpam_msc *msc);
 
 /* MPAMF_CSUMON_IDR - MPAM cache storage usage monitor ID register */
 #define MPAMF_CSUMON_IDR_NUM_MON	GENMASK(15, 0)
+#define MPAMF_CSUMON_IDR_NO_MATCH_PMG	BIT(22)
 #define MPAMF_CSUMON_IDR_HAS_OFLOW_CAPT	BIT(24)
 #define MPAMF_CSUMON_IDR_HAS_CEVNT_OFLW	BIT(25)
 #define MPAMF_CSUMON_IDR_HAS_OFSR	BIT(26)
@@ -655,6 +658,7 @@ int mpam_fb_check_shared_buffer_size(struct mpam_msc *msc);
 
 /* MPAMF_MBWUMON_IDR - MPAM memory bandwidth usage monitor ID register */
 #define MPAMF_MBWUMON_IDR_NUM_MON	GENMASK(15, 0)
+#define MPAMF_MBWUMON_IDR_NO_MATCH_PMG	BIT(22)
 #define MPAMF_MBWUMON_IDR_HAS_RWBW	BIT(28)
 #define MPAMF_MBWUMON_IDR_LWD		BIT(29)
 #define MPAMF_MBWUMON_IDR_HAS_LONG	BIT(30)
-- 
2.34.1

Re: [PATCH] arm_mpam: Only program MATCH_PMG when the monitor supports it

[Ben Horgan]

Hi Ping,

On 02/11/2026 07:13, Ping Li wrote:
> An MSC of ours implements the MBWU monitor without PMG support:
> MPAMF_MBWUMON_IDR.NO_MATCH_PMG is 1, so MSMON_CFG_x_CTL.MATCH_PMG is
> RES0. On this hardware resctrl monitor reads always returned 0.
> 
> Tracking this down: __read_mon() unconditionally sets match_pmg,
> because the resctrl (closid, rmid) model maps rmid to PMG. The
> expected configuration generated by gen_msmon_ctl_flt_vals() then
> contains MATCH_PMG, but the hardware ignores the bit, so the
> configuration read back never matches, config_mismatch is always
> true and the monitor is reprogrammed on every read - the counter
> never accumulates.
> 
> The architecture allows an MSMON implementation without PMG
> support, so read the NO_MATCH_PMG bits during probe and only
> program MATCH_PMG when the monitor supports PMG filtering,
> falling back to filtering by PARTID only. The CSU monitor has
> the same issue and is covered the same way.

I'm afraid this doesn't work. resctrl will show the same counts for every monitor group under a
control group rather than the counts related to the tasks/cpus associated with the individual
monitor groups.

Have you actually built NO_MATCH_PMG? I ask because I understood that NO_MATCH_PMG and
NO_MATCH_PARTID were erroneously added to the MPAMv1 specification and were going to be removed.
What you should build or not is question for support@arm.com rather than the list though.

Thanks,

Ben

> 
> Signed-off-by: Ping Li <leeonion.muyu@gmail.com>
> ---
>  drivers/resctrl/mpam_devices.c  | 22 +++++++++++++++++++++-
>  drivers/resctrl/mpam_internal.h |  4 ++++
>  2 files changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
> index c067b6d57d71..2086ff121b59 100644
> --- a/drivers/resctrl/mpam_devices.c
> +++ b/drivers/resctrl/mpam_devices.c
> @@ -1089,6 +1089,9 @@ static int mpam_ris_hw_probe(struct mpam_msc_ris *ris)
>  				if (FIELD_GET(MPAMF_CSUMON_IDR_HAS_XCL, csumonidr))
>  					mpam_set_feature(mpam_feat_msmon_csu_xcl, props);
>  
> +				if (!FIELD_GET(MPAMF_CSUMON_IDR_NO_MATCH_PMG, csumonidr))
> +					mpam_set_feature(mpam_feat_msmon_csu_match_pmg, props);
> +
>  				/* Is NRDY hardware managed? */
>  				err = mpam_ris_hw_probe_csu_nrdy(ris);
>  				if (err < 0)
> @@ -1118,6 +1121,9 @@ static int mpam_ris_hw_probe(struct mpam_msc_ris *ris)
>  				if (FIELD_GET(MPAMF_MBWUMON_IDR_HAS_RWBW, mbwumon_idr))
>  					mpam_set_feature(mpam_feat_msmon_mbwu_rwbw, props);
>  
> +				if (!FIELD_GET(MPAMF_MBWUMON_IDR_NO_MATCH_PMG, mbwumon_idr))
> +					mpam_set_feature(mpam_feat_msmon_mbwu_match_pmg, props);
> +
>  				has_long = FIELD_GET(MPAMF_MBWUMON_IDR_HAS_LONG, mbwumon_idr);
>  				if (has_long) {
>  					if (FIELD_GET(MPAMF_MBWUMON_IDR_LWD, mbwumon_idr))
> @@ -1309,6 +1315,20 @@ static int mpam_msc_zero_mbwu_l(struct mpam_msc *msc)
>  	return __mpam_write_reg(msc, MSMON_MBWU_L + 4, 0);
>  }
>  
> +static bool msmon_supports_match_pmg(struct mon_read *m)
> +{
> +	switch (m->type) {
> +	case mpam_feat_msmon_csu:
> +		return mpam_has_feature(mpam_feat_msmon_csu_match_pmg, &m->ris->props);
> +	case mpam_feat_msmon_mbwu_31counter:
> +	case mpam_feat_msmon_mbwu_44counter:
> +	case mpam_feat_msmon_mbwu_63counter:
> +		return mpam_has_feature(mpam_feat_msmon_mbwu_match_pmg, &m->ris->props);
> +	default:
> +		return true;
> +	}
> +}
> +
>  static void gen_msmon_ctl_flt_vals(struct mon_read *m, u32 *ctl_val,
>  				   u32 *flt_val)
>  {
> @@ -1322,7 +1342,7 @@ static void gen_msmon_ctl_flt_vals(struct mon_read *m, u32 *ctl_val,
>  
>  	*flt_val = FIELD_PREP(MSMON_CFG_x_FLT_PARTID, ctx->partid);
>  
> -	if (m->ctx->match_pmg) {
> +	if (m->ctx->match_pmg && msmon_supports_match_pmg(m)) {
>  		*ctl_val |= MSMON_CFG_x_CTL_MATCH_PMG;
>  		*flt_val |= FIELD_PREP(MSMON_CFG_x_FLT_PMG, ctx->pmg);
>  	}
> diff --git a/drivers/resctrl/mpam_internal.h b/drivers/resctrl/mpam_internal.h
> index 1f86cc158100..88cd5ac4cde3 100644
> --- a/drivers/resctrl/mpam_internal.h
> +++ b/drivers/resctrl/mpam_internal.h
> @@ -227,12 +227,14 @@ enum mpam_device_features {
>  	mpam_feat_msmon_csu,
>  	mpam_feat_msmon_csu_capture,
>  	mpam_feat_msmon_csu_xcl,
> +	mpam_feat_msmon_csu_match_pmg,
>  	mpam_feat_msmon_mbwu,
>  	mpam_feat_msmon_mbwu_31counter,
>  	mpam_feat_msmon_mbwu_44counter,
>  	mpam_feat_msmon_mbwu_63counter,
>  	mpam_feat_msmon_mbwu_capture,
>  	mpam_feat_msmon_mbwu_rwbw,
> +	mpam_feat_msmon_mbwu_match_pmg,
>  	mpam_feat_partid_nrw,
>  	MPAM_FEATURE_LAST
>  };
> @@ -645,6 +647,7 @@ int mpam_fb_check_shared_buffer_size(struct mpam_msc *msc);
>  
>  /* MPAMF_CSUMON_IDR - MPAM cache storage usage monitor ID register */
>  #define MPAMF_CSUMON_IDR_NUM_MON	GENMASK(15, 0)
> +#define MPAMF_CSUMON_IDR_NO_MATCH_PMG	BIT(22)
>  #define MPAMF_CSUMON_IDR_HAS_OFLOW_CAPT	BIT(24)
>  #define MPAMF_CSUMON_IDR_HAS_CEVNT_OFLW	BIT(25)
>  #define MPAMF_CSUMON_IDR_HAS_OFSR	BIT(26)
> @@ -655,6 +658,7 @@ int mpam_fb_check_shared_buffer_size(struct mpam_msc *msc);
>  
>  /* MPAMF_MBWUMON_IDR - MPAM memory bandwidth usage monitor ID register */
>  #define MPAMF_MBWUMON_IDR_NUM_MON	GENMASK(15, 0)
> +#define MPAMF_MBWUMON_IDR_NO_MATCH_PMG	BIT(22)
>  #define MPAMF_MBWUMON_IDR_HAS_RWBW	BIT(28)
>  #define MPAMF_MBWUMON_IDR_LWD		BIT(29)
>  #define MPAMF_MBWUMON_IDR_HAS_LONG	BIT(30)

Re: [PATCH] arm_mpam: Only program MATCH_PMG when the monitor supports it

[Ping Li]

Hi Ben,

Apologies for raising this directly on the list - hardware questions
like this should go to support@arm.com first.

To answer your question: our memory controller is still at the FPGA
stage. It has 32 PARTIDs but no PMG. After talking with our hardware
team and reading the specification, I had wrongly assumed this design
was legitimate. Our intended usage was one control group in resctrl
with no further monitor groups underneath, so the PARTID-only
monitoring would have been sufficient for us.

I will get our hardware team to review the RTL with Arm support, and
will drop this patch.

Sorry for the noise, and thanks for the pointers.

Ping

Re: [PATCH] arm_mpam: Only program MATCH_PMG when the monitor supports it

[Ping Li]

Hi Ben,

Sorry to bother you again. I was looking at how the driver derives
mpam_pmg_max, and I think there is a similar blind spot:

mpam_msc_hw_probe() folds every MSC's MPAMF_IDR.PMG_MAX into the
global mpam_pmg_max, regardless of whether the MSC has any monitors.

Concretely, on our platform the DSU has no monitors and reports
PMG_MAX=1. Our memory controller MSC has monitors and currently
reports PMG_MAX=0, but if we later add PMG support and report
PMG_MAX=31, the global value would still be clamped to 1 by the DSU,
and resctrl would only expose 2 RMIDs per CLOSID instead of 32.

Should the convergence only consider MSCs that have monitors?
The PE-side value from MPAMIDR_EL1.PMG_MAX would still apply, as
that is the maximum PMG the cores can tag traffic with.

Thanks,
Ping

Re: [PATCH] arm_mpam: Only program MATCH_PMG when the monitor supports it

[Ben Horgan]

Hi Ping,

On 03/11/2026 02:37, Ping Li wrote:
> Hi Ben,
> 
> Sorry to bother you again. I was looking at how the driver derives
> mpam_pmg_max, and I think there is a similar blind spot:
> 
> mpam_msc_hw_probe() folds every MSC's MPAMF_IDR.PMG_MAX into the
> global mpam_pmg_max, regardless of whether the MSC has any monitors.
> 
> Concretely, on our platform the DSU has no monitors and reports
> PMG_MAX=1. Our memory controller MSC has monitors and currently
> reports PMG_MAX=0, but if we later add PMG support and report
> PMG_MAX=31, the global value would still be clamped to 1 by the DSU,
> and resctrl would only expose 2 RMIDs per CLOSID instead of 32.
> 
> Should the convergence only consider MSCs that have monitors?

No, the PMG_MAX can also indicate the largest propagated and so still
needs to be taken into account. Rules I_GSHTX and I_BTFZC are relevant here.

Thanks,

Ben

> The PE-side value from MPAMIDR_EL1.PMG_MAX would still apply, as
> that is the maximum PMG the cores can tag traffic with.
> 
> Thanks,
> Ping

Re: [PATCH] arm_mpam: Only program MATCH_PMG when the monitor supports it

[Ping Li]

Hi Ben,

Sharing the outcome of our discussions with the hardware team and Arm
support, to close this thread:

Although the MPAM specification allows up to 8 bits of PMG, our
hardware team found that the DSU120AE and the Cortex-A520 only
propagate 1 bit of PMG. Assuming 1 bit was as good as none, and
without consulting the software team, they considered the 2 monitor
groups it would allow dispensable. So the MSC they designed
implements MSMON but no PMG support, and the bus carries PARTID only,
without the PMG bit.

Sorry for the noise again on this one, and thanks for the pointers.

Ping

Re: [PATCH] arm_mpam: Only program MATCH_PMG when the monitor supports it

[Ben Horgan]

Hi Ping,

On 03/11/2026 09:55, Ping Li wrote:
> Hi Ben,
> 
> Sharing the outcome of our discussions with the hardware team and Arm
> support, to close this thread:
> 
> Although the MPAM specification allows up to 8 bits of PMG, our
> hardware team found that the DSU120AE and the Cortex-A520 only
> propagate 1 bit of PMG. Assuming 1 bit was as good as none, and
> without consulting the software team, they considered the 2 monitor
> groups it would allow dispensable. So the MSC they designed
> implements MSMON but no PMG support, and the bus carries PARTID only,
> without the PMG bit.

Thanks for sharing this information. 1 bit of PMG is common because
until recently CHI didn't allow more.

Ben

> 
> Sorry for the noise again on this one, and thanks for the pointers.
> 
> Ping