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

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

Hi Jinjie,

On 12/11/2026 01:53, Jinjie Ruan wrote:
> 
> 
> 在 2026/11/11 23:00, 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
> 
> Yes, I also noticed that.
> 
>> 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.
> 
> I agree to consider these issues together, and there may be other bugs,
> including resctrl, such as schema write failures, but they appear to be
> okay..
> 
> https://lore.kernel.org/all/20261104102304.703535-1-ruanjinjie@huawei.com/

Thanks for bringing this to my attention. I'll add a comment there.

Thanks,

Ben

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