[PATCH] arm_mpam: Account for a MBWU overflow before reading the counter value

[PATCH] arm_mpam: Account for a MBWU overflow before reading the counter value

[Ping Li]

While checking the driver's overflow handling and NRDY behaviour with
the design engineering team, the following problem was found in
__ris_msmon_read_locked():

__ris_msmon_read_locked() reads the MSMON_CFG_MBWU_CTL overflow status
bit, clears it, then reads the counter value. The counter value read is
where the overflow is accounted for, by adding the counter size to the
per-monitor correction value.

If the monitor reports NRDY when the counter value is read,
__ris_msmon_read_locked() returns -EBUSY, and mpam_msmon_read() retries
after waiting. The overflow status bit was already cleared, so the
retry reads it as 0, and the overflow is never accounted for. Each
occurrence silently loses a counter-size increment from the accumulated
bandwidth count.

The overflow status bit was read before it was cleared, so the overflow
has already happened. Account for it before attempting to read the
counter value, as the cleared status bit cannot be re-read.

Signed-off-by: Ping Li <leeonion.muyu@gmail.com>
---
 drivers/resctrl/mpam_devices.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
index 27abf60f4d67..5384c9a4a1ad 100644
--- a/drivers/resctrl/mpam_devices.c
+++ b/drivers/resctrl/mpam_devices.c
@@ -1528,6 +1528,16 @@ static void __ris_msmon_read_locked(struct mon_read *m)
 			return;
 	}
 
+	/*
+	 * The overflow status bit was read above, before being cleared by
+	 * software. The overflow has already happened, so account for it
+	 * even if reading the counter value below fails, as the cleared
+	 * status bit cannot be re-read.
+	 */
+	if (overflow)
+		ris->mbwu_state[ctx->mon].correction +=
+			mpam_msmon_overflow_val(m->type, msc);
+
 	switch (m->type) {
 	case mpam_feat_msmon_csu:
 		m->err = mpam_read_monsel_reg(msc, CSU, &now32);
@@ -1569,9 +1579,6 @@ static void __ris_msmon_read_locked(struct mon_read *m)
 
 		mbwu_state = &ris->mbwu_state[ctx->mon];
 
-		if (overflow)
-			mbwu_state->correction += mpam_msmon_overflow_val(m->type, msc);
-
 		/*
 		 * Include bandwidth consumed before the last hardware reset and
 		 * a counter size increment for each overflow.
-- 
2.34.1

Re: [PATCH] arm_mpam: Account for a MBWU overflow before reading the counter value

[Ben Horgan]

Hi Ping,

On 11/11/2026 07:17, Ping Li wrote:
> While checking the driver's overflow handling and NRDY behaviour with
> the design engineering team, the following problem was found in
> __ris_msmon_read_locked():
> 
> __ris_msmon_read_locked() reads the MSMON_CFG_MBWU_CTL overflow status
> bit, clears it, then reads the counter value. The counter value read is
> where the overflow is accounted for, by adding the counter size to the
> per-monitor correction value.
> 
> If the monitor reports NRDY when the counter value is read,
> __ris_msmon_read_locked() returns -EBUSY, and mpam_msmon_read() retries
> after waiting. The overflow status bit was already cleared, so the
> retry reads it as 0, and the overflow is never accounted for. Each
> occurrence silently loses a counter-size increment from the accumulated
> bandwidth count.

I was under the impression that this couldn't happen. Is the a theoretical fix or has this been
observed in practice?

From IHI0099B.d 6.2.2 I_PCDHG regarding MBWU and the NRDY:
"
If 1, then the monitor instance is not ready and the VALUE field does not have an accurate count or
measurement due to recent changes to the monitor settings.
"

Based on this, for MBWU you should only see NRDY set in the MAX_NRDY_USEC after a change of monitor
settings. At such a reset the VALUE is reset to 0. Are you seeing or potentially seeing an overflow
in this window of time?

Looking at 6.2.4 I_RZWDC we have "When an overflow occurs... The overflowed monitor VALUE and NRDY
fields can be reset to zero" but the use of 'can' rather than 'are' renders that statement mostly
useless.

Thanks,

Ben

> 
> The overflow status bit was read before it was cleared, so the overflow
> has already happened. Account for it before attempting to read the
> counter value, as the cleared status bit cannot be re-read.
> 
> Signed-off-by: Ping Li <leeonion.muyu@gmail.com>
> ---
>  drivers/resctrl/mpam_devices.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
> index 27abf60f4d67..5384c9a4a1ad 100644
> --- a/drivers/resctrl/mpam_devices.c
> +++ b/drivers/resctrl/mpam_devices.c
> @@ -1528,6 +1528,16 @@ static void __ris_msmon_read_locked(struct mon_read *m)
>  			return;
>  	}
>  
> +	/*
> +	 * The overflow status bit was read above, before being cleared by
> +	 * software. The overflow has already happened, so account for it
> +	 * even if reading the counter value below fails, as the cleared
> +	 * status bit cannot be re-read.
> +	 */
> +	if (overflow)
> +		ris->mbwu_state[ctx->mon].correction +=
> +			mpam_msmon_overflow_val(m->type, msc);
> +
>  	switch (m->type) {
>  	case mpam_feat_msmon_csu:
>  		m->err = mpam_read_monsel_reg(msc, CSU, &now32);
> @@ -1569,9 +1579,6 @@ static void __ris_msmon_read_locked(struct mon_read *m)
>  
>  		mbwu_state = &ris->mbwu_state[ctx->mon];
>  
> -		if (overflow)
> -			mbwu_state->correction += mpam_msmon_overflow_val(m->type, msc);
> -
>  		/*
>  		 * Include bandwidth consumed before the last hardware reset and
>  		 * a counter size increment for each overflow.

Re: [PATCH] arm_mpam: Account for a MBWU overflow before reading the counter value

[Ping Li]

From: leeonion.muyu@gmail.com

Hi Ben,

Thanks for taking a look.

> Is the a theoretical fix or has this been observed in practice?

Not observed in practice. My hardware team was reviewing the software
overflow and NRDY checking logic with me, and we found this while
walking through the code together.

> Based on this, for MBWU you should only see NRDY set in the
> MAX_NRDY_USEC after a change of monitor settings. At such a reset
> the VALUE is reset to 0. Are you seeing or potentially seeing an
> overflow in this window of time?

I agree. Our starting assumption was IHI0099B.d 6.2.1 RGKWNB - that on
a settings update the monitor VALUE field keeps counting using the new
criteria without automatically resetting to 0. But as you point out,
looking at the reconfiguration path in write_msmon_ctl_flt_vals(),
the driver deliberately writes VALUE to 0 as part of the IDGYLF
sequence, so the accumulated count is discarded there and even a
31-bit monitor cannot overflow within the NRDY window after a
reconfiguration. The overflow and NRDY cannot collide.

My apologies - there was a second trigger in my original analysis
that the commit message failed to mention: any error reading the
counter value after the status bit has been cleared loses the
accounting the same way, and mpam_msmon_read() does not retry
non-EBUSY errors. For a MPAM_F capable MSC, mpam_msc_read_mbwu_l()
and mpam_read_monsel_reg() go through the PCC mailbox and can return
-ETIMEDOUT or -EIO on transport errors. So I believe the patch still
provides protection against this failure path - it moves the
accounting to the point where the status bit has been read but not
yet cleared, and it is a no-op in the normal path.

Thanks,
Ping