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

[Ben Horgan <ben.horgan@arm.com>]

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)