[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

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

[Ben Horgan]

Hi Ping,

On 11/11/2026 12:56, Ping Li wrote:
> 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.

I see. This is a side effect of the recent addition of support for MPAM-Fb (which uses  the PCC
mailbox). Any error relating to MPAM-Fb is considered as an indication of MPAM being broken and mpam
is disabled using mpam_fb_disable_mpam(). After this point normal driver behaviour cannot be
expected. So, if we do hit one of the MPAM-Fb failures then we've already triggered a tear down of
the driver. Hence, your patch isn't fixing anything that effects the normal operation of the driver.

Having said that, it is not pretty to rely on the MPAM-Fb failures being catastrophic and makes the
driver more fragile and difficult to reason about. Jin Jie has a patch [1] which addresses a problem
related to when control configuration fails due to an MPAM-Fb error. I don't currently intend to
merge either of these patches as I still think there is likely to be more of the same lurking in the
driver and I don't want to do this piecemeal. I may take them later though, after I've audited for
any similar problems; a couple of small changes are worth it for making the driver easier to reason
about.

[1] https://lore.kernel.org/linux-arm-kernel/20261104101541.563113-1-ruanjinjie@huawei.com/

Thanks,

Ben


> 
> Thanks,
> Ping

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

[Ben Horgan]

Hi Ping,

On 11/11/2026 15:00, Ben Horgan wrote:
> Hi Ping,
> 
> On 11/11/2026 12:56, Ping Li wrote:
>> 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.
> 
> I see. This is a side effect of the recent addition of support for MPAM-Fb (which uses  the PCC
> mailbox). Any error relating to MPAM-Fb is considered as an indication of MPAM being broken and mpam
> is disabled using mpam_fb_disable_mpam(). After this point normal driver behaviour cannot be
> expected. So, if we do hit one of the MPAM-Fb failures then we've already triggered a tear down of
> the driver. Hence, your patch isn't fixing anything that effects the normal operation of the driver.
> 
> Having said that, it is not pretty to rely on the MPAM-Fb failures being catastrophic and makes the
> driver more fragile and difficult to reason about. Jin Jie has a patch [1] which addresses a problem
> related to when control configuration fails due to an MPAM-Fb error. I don't currently intend to
> merge either of these patches as I still think there is likely to be more of the same lurking in the
> driver and I don't want to do this piecemeal. I may take them later though, after I've audited for
> any similar problems; a couple of small changes are worth it for making the driver easier to reason
> about.

Just to clarify, it is still necessary that on any MPAM-Fb failure the error is passed up to resctrl
and then the user when they triggered the request. I will shortly post a small series that fixes
some problems in that area.

Thanks,

Ben

> 
> [1] https://lore.kernel.org/linux-arm-kernel/20261104101541.563113-1-ruanjinjie@huawei.com/
> 
> Thanks,
> 
> Ben
> 
> 
>>
>> Thanks,
>> Ping
>