Re: [PATCH] arm_mpam: Apply T241-MPAM-6 to 63-bit counters

[Ben Horgan]

Hi Shanker,

On 7/28/26 14:42, Shanker Donthineni wrote:
> Hi Ben,
> 
> On 7/28/2026 5:48 AM, Ben Horgan wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> Hi Shanker,
>>
>> On 7/27/26 20:13, Shanker Donthineni wrote:
>>> T241-MPAM-6 causes all MBWU counter formats to count 64-byte
>>> requests instead of bytes. Commit dc48eb1ff27c excluded the 63-bit
>> As described in that commit message, T241-MPAM-6 does not affect LWD counters. Is there another
>> canonical description of T241-MPAM-6. Otherwise, this just looks like you've discovered a new h/w
>> bug :)
> The original commit message for dc48eb1ff27c explicitly states that T241-MPAM-6 affects both
> MSMON_MBWU and MSMON_MBWU_L:
> 
>  "The registers MSMON_MBWU_L and MSMON_MBWU return the number of requests rather than the number
>   of bytes transferred."
> 
> LWD (when HAS_LONG=1) selects the width/format of MSMON_MBWU_L; it does not identify a separate
> register outside the scope described above.

Oh yes, you are right. The L_VALUE/LWD_VALUE naming is just internal to the driver.

> 
>>> MSMON_MBWU_LWD format while scaling the shorter counters. Systems
>>> selecting the preferred 63-bit counter consequently report bandwidth
>>> values that are 64 times too small.
>> So, the MSC with IIDR MPAM_IIDR_NVIDIA_T241 has different sizes of long bandwidth counters between
>> RIS. This seems a bit unusual. Please could you share which RIS have 44 bit long counters and which
>> have 63 bits. (MPAMF_MBWUMON_IDR.LWD gives this)
> T241 does not implement RIS (MPAMF_IDR.HAS_RIS=0). Its L3 MSC implements a 63-bit long bandwidth
> counter, as indicated by HAS_LONG=1 and LWD=1; therefore, there are no per-RIS differences in
> counter width.

Ok, so there are no 44 bit counters?

Reviewed-by: Ben Horgan <ben.horgan@xxxxxxx>

Thanks,

Ben

> 
> The mpam_feat_msmon_mbwu_63counter exclusion in dc48eb1ff27c means the workaround is not applied
> to the long-counter format implemented by the T241 L3 MSC. This patch removes that exclusion and
> makes the implementation consistent with the original erratum description and commit message.
> 
>> Thanks,
>>
>> Ben
>>
>>> Apply the scale to both the sampled value and overflow correction for
>>> the 63-bit format. Unsigned arithmetic retains modulo-u64 behavior
>>> when the scaled counter range exceeds u64.
>>>
>>> Fixes: dc48eb1ff27c ("arm_mpam: Add workaround for T241-MPAM-6")
>>> Link: https://lore.kernel.org/lkml/20240816131432.993859-1-sdonthineni@xxxxxxxxxx/
>>> Signed-off-by: Shanker Donthineni <sdonthineni@xxxxxxxxxx>
>>> ---
>>>   drivers/resctrl/mpam_devices.c | 6 ++----
>>>   1 file changed, 2 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
>>> index 2f09f4b78bd3b..b68f5599e8dbe 100644
>>> --- a/drivers/resctrl/mpam_devices.c
>>> +++ b/drivers/resctrl/mpam_devices.c
>>> @@ -1196,8 +1196,7 @@ static u64 mpam_msmon_overflow_val(enum mpam_device_features type,
>>>   {
>>>        u64 overflow_val = __mpam_msmon_overflow_val(type);
>>>
>>> -     if (mpam_has_quirk(T241_MBW_COUNTER_SCALE_64, msc) &&
>>> -         type != mpam_feat_msmon_mbwu_63counter)
>>> +     if (mpam_has_quirk(T241_MBW_COUNTER_SCALE_64, msc))
>>>                overflow_val *= 64;
>>>
>>>        return overflow_val;
>>> @@ -1293,8 +1292,7 @@ static void __ris_msmon_read(void *arg)
>>>                        now = FIELD_GET(MSMON___VALUE, now);
>>>                }
>>>
>>> -             if (mpam_has_quirk(T241_MBW_COUNTER_SCALE_64, msc) &&
>>> -                 m->type != mpam_feat_msmon_mbwu_63counter)
>>> +             if (mpam_has_quirk(T241_MBW_COUNTER_SCALE_64, msc))
>>>                        now *= 64;
>>>
>>>                if (nrdy)
>