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

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

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.