Re: [PATCH v2 04/15] arm_mpam: propagate MSC read errors for mpam_msc_read_mbwu_l()

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

Hi Andre,

On 7/9/26 08:36, Andre Przywara wrote:
> Hi Ben,
> 
> On 7/1/26 22:06, Ben Horgan wrote:
>> Hi Andre,
>>
>> On 7/2/26 17:22, Andre Przywara wrote:
>>> Allow the mpam_msc_read_mbwu_l() function to return an error, and
>>> propagate read errors from the lower level up.
>>>
>>> Signed-off-by: Andre Przywara <andre.przywara@arm.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 011d1e3544d7..1d06902fb970 100644
>>> --- a/drivers/resctrl/mpam_devices.c
>>> +++ b/drivers/resctrl/mpam_devices.c
>>> @@ -1106,6 +1106,7 @@ static bool
>>> mpam_ris_has_mbwu_long_counter(struct mpam_msc_ris *ris)
>>>     static u64 mpam_msc_read_mbwu_l(struct mpam_msc *msc)
>>
>> Now that your changing the other helpers to return an error and pass a
>> pointer to hold the register value it would seem more consistent for
>> this call to follow the same pattern.
> 
> Ah, I wasn't entirely sure whether this MSMON___L_NRDY is just some
> error flag (abuse), since NRDY is some kind of architecture feature.

Yeah, as only 32 bit reads are supported it's abused to report the case
where we can't get a stable reading of the upper bits. Architecturally,
NRDY is (optionally) used to say we haven't yet got a stable value after
a configuration.

Thanks,

Ben

> If that's indeed here the case, I will be changing the prototype and
> adjust the function.
> 
> Cheers,
> Andre
> 
>> Thanks,
>>
>> Ben
>>
>>>   {
>>> +    int ret;
>>>       int retry = 3;
>>>       u32 mbwu_l_low;
>>>       u32 mbwu_l_high1, mbwu_l_high2;
>>> @@ -1115,11 +1116,17 @@ static u64 mpam_msc_read_mbwu_l(struct
>>> mpam_msc *msc)
>>>       WARN_ON_ONCE((MSMON_MBWU_L + sizeof(u64)) > msc-
>>> >mapped_hwpage_sz);
>>>       WARN_ON_ONCE(!cpumask_test_cpu(smp_processor_id(), &msc-
>>> >accessibility));
>>>   -    __mpam_read_reg(msc, MSMON_MBWU_L + 4, &mbwu_l_high2);
>>> +    ret = __mpam_read_reg(msc, MSMON_MBWU_L + 4, &mbwu_l_high2);
>>> +    if (ret)
>>> +        return MSMON___L_NRDY;
>>> +
>>>       do {
>>>           mbwu_l_high1 = mbwu_l_high2;
>>> -        __mpam_read_reg(msc, MSMON_MBWU_L, &mbwu_l_low);
>>> -        __mpam_read_reg(msc, MSMON_MBWU_L + 4, &mbwu_l_high2);
>>> +        ret = __mpam_read_reg(msc, MSMON_MBWU_L, &mbwu_l_low);
>>> +        if (!ret)
>>> +            ret = __mpam_read_reg(msc, MSMON_MBWU_L + 4,
>>> &mbwu_l_high2);
>>> +        if (ret)
>>> +            return MSMON___L_NRDY;
>>>             retry--;
>>>       } while (mbwu_l_high1 != mbwu_l_high2 && retry > 0);
>>
>