Re: [RFC PATCH 27/36] arm_mpam: Allow configuration to be applied and restored during cpu online

[James Morse]

Hi Fenghua,

On 04/08/2025 17:39, Fenghua Yu wrote:
> On 7/11/25 11:36, James Morse wrote:
> > When CPUs come online the original configuration should be restored.
> > Once the maximum partid is known, allocate an configuration array for
> > each component, and reprogram each RIS configuration from this.
> >
> > The MPAM spec describes how multiple controls can interact. To prevent
> > this happening by accident, always reset controls that don't have a
> > valid configuration. This allows the same helper to be used for
> > configuration and reset.

> > diff --git a/drivers/platform/arm64/mpam/mpam_devices.c b/drivers/platform/arm64/mpam/
> > mpam_devices.c
> > index bb3695eb84e9..f3ecfda265d2 100644
> > --- a/drivers/platform/arm64/mpam/mpam_devices.c
> > +++ b/drivers/platform/arm64/mpam/mpam_devices.c>>   @@ -909,51 +913,90 @@ static void mpam_reset_msc_bitmap(struct mpam_msc *msc, u16 reg,
> > +/* Call with MSC lock held */
> > +static int mpam_reprogram_ris(void *_arg)
> > +{
> > +    u16 partid, partid_max;
> > +    struct reprogram_ris *arg = _arg;
> > +    struct mpam_msc_ris *ris = arg->ris;
> > +    struct mpam_config *cfg = arg->cfg;
> > +
> > +    if (ris->in_reset_state)
> > +        return 0;
> > +
> > +    spin_lock(&partid_max_lock);
> > +    partid_max = mpam_partid_max;
> partid_max is not used after the assignment.
> > +    spin_unlock(&partid_max_lock);
>
> Doesn't make sense to lock protect a local variable partid_max which is not used any way.
>
> [SNIP]

Because you cut the user out:
| 	for (partid = 0; partid <= partid_max; partid++)
|		mpam_reprogram_ris_partid(ris, partid, cfg);
|
|	return 0;
| }

mpam_reprogram_ris() needs to snapshot the value because it can be called via mpam_reset_msc() -
which does run before mpam_enable_once(). This can race with mpam_register_requestor(), but the
race can only reduce partid_max, all the MSC are guaranteed to support at least the original value.

Taking the lock is so you don't get a torn value which is larger than the original value.


Thanks,

James