* [PATCH] arm_mpam: Only program MATCH_PMG when the monitor supports it
@ 2026-11-02 7:13 Ping Li
2026-11-02 9:46 ` Ben Horgan
0 siblings, 1 reply; 3+ messages in thread
From: Ping Li @ 2026-11-02 7:13 UTC (permalink / raw)
To: james.morse, ben.horgan
Cc: linux-kernel, linux-arm-kernel, reinette.chatre, fenghuay, 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
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] arm_mpam: Only program MATCH_PMG when the monitor supports it
2026-11-02 7:13 [PATCH] arm_mpam: Only program MATCH_PMG when the monitor supports it Ping Li
@ 2026-11-02 9:46 ` Ben Horgan
2026-11-02 12:05 ` Ping Li
0 siblings, 1 reply; 3+ messages in thread
From: Ben Horgan @ 2026-11-02 9:46 UTC (permalink / raw)
To: Ping Li, james.morse
Cc: linux-kernel, linux-arm-kernel, reinette.chatre, fenghuay
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)
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] arm_mpam: Only program MATCH_PMG when the monitor supports it
2026-11-02 9:46 ` Ben Horgan
@ 2026-11-02 12:05 ` Ping Li
0 siblings, 0 replies; 3+ messages in thread
From: Ping Li @ 2026-11-02 12:05 UTC (permalink / raw)
To: ben.horgan, james.morse
Cc: linux-kernel, linux-arm-kernel, reinette.chatre, fenghuay
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-11-02 12:05 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-02 7:13 [PATCH] arm_mpam: Only program MATCH_PMG when the monitor supports it Ping Li
2026-11-02 9:46 ` Ben Horgan
2026-11-02 12:05 ` Ping Li
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®