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

[Andre Przywara]

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@xxxxxxx>
> > ---
> >   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