Re: [PATCH v7 07/11] arm_mpam: propagate MSC access errors for mpam_reprogram_ris_partid()

[Andre Przywara <andre.przywara@arm.com>]

Hi,

On 8/4/26 07:57, Srivathsa L Rao wrote:
> Hi Andre,
> 
> mpam_reprogram_ris_partid() writes hardware registers sequentially.
> If a write fails midway (possible on PCC MSCs via SCMI), the earlier
> writes are already committed and the hardware is left in a partially
> configured state.
> Currently this is safe only because mpam_fb_disable_mpam() tears
> everything down on any firmware error. If that behavior is ever
> relaxed (e.g. for transient BUSY handling), a partial state could
> persist and produce weird behavior?
> 
> I wanted to point this as a design note.

Yes, this might be a potential problem, but as you said at the moment we 
tear down MPAM anyway.
I don't know if we can ever really handle MPAM-Fb errors in a meaningful 
way, but indeed a retry-if-busy seems a worthwhile addition - at a later 
time ;-)

Thanks for all the reviews and comments!

Cheers,
Andre

> On 7/31/2026 10:33 PM, Andre Przywara wrote:
>> Allow the mpam_reprogram_ris_partid() function check for and return
>> errors, and propagate MSC read and write errors from the lower level up.
>> This also covers the callers of this function: mpam_reset_ris() and
>> apply_config().
>>
>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>> ---
>>   drivers/resctrl/mpam_devices.c | 136 ++++++++++++++++++++++-----------
>>   1 file changed, 93 insertions(+), 43 deletions(-)
>>
>> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/ 
>> mpam_devices.c
>> index 38450c55e45e..32088ad1d67e 100644
>> --- a/drivers/resctrl/mpam_devices.c
>> +++ b/drivers/resctrl/mpam_devices.c
>> @@ -1601,7 +1601,7 @@ void mpam_msmon_reset_mbwu(struct mpam_component 
>> *comp, struct mon_cfg *ctx)
>>       }
>>   }
>>   ...
> 
>> +static int mpam_reprogram_ris_partid(struct mpam_msc_ris *ris, u16 
>> partid,
>> +                     struct mpam_config *cfg)
>>   {
>>       u32 pri_val = 0;
>>       ..
>> -    mutex_unlock(&msc->part_sel_lock);
>> +    return 0;
>>   }
> -- 
> 
> Best Regards,
> Srivathsa