Re: [RFC PATCH 22/36] arm_mpam: Reset MSC controls from cpu hp callbacks

[James Morse]

Hi Ben,

On 28/07/2025 10:49, Ben Horgan wrote:
> On 7/11/25 19:36, James Morse wrote:
> > When a CPU comes online, it may bring a newly accessible MSC with
> > it. Only the default partid has its value reset by hardware, and
> > even then the MSC might not have been reset since its config was
> > previously dirtyied. e.g. Kexec.
> >
> > Any in-use partid must have its configuration restored, or reset.
> > In-use partids may be held in caches and evicted later.
> >
> > MSC are also reset when CPUs are taken offline to cover cases where
> > firmware doesn't reset the MSC over reboot using UEFI, or kexec
> > where there is no firmware involvement.
> >
> > If the configuration for a RIS has not been touched since it was
> > brought online, it does not need resetting again.
> >
> > To reset, write the maximum values for all discovered controls.
> > diff --git a/drivers/platform/arm64/mpam/mpam_devices.c b/drivers/platform/arm64/mpam/
> > mpam_devices.c
> > index 7b042a35405a..d014dbe0ab96 100644
> > --- a/drivers/platform/arm64/mpam/mpam_devices.c
> > +++ b/drivers/platform/arm64/mpam/mpam_devices.c

> > @@ -849,8 +850,116 @@ static int mpam_msc_hw_probe(struct mpam_msc *msc)
> >       return 0;
> >   }
> >   +static void mpam_reset_msc_bitmap(struct mpam_msc *msc, u16 reg, u16 wd)
> > +{
> > +    u32 num_words, msb;
> > +    u32 bm = ~0;
> > +    int i;
> > +
> > +    lockdep_assert_held(&msc->part_sel_lock);
> > +
> > +    if (wd == 0)
> > +        return;
> > +
> > +    /*
> > +     * Write all ~0 to all but the last 32bit-word, which may
> > +     * have fewer bits...
> > +     */
> > +    num_words = DIV_ROUND_UP(wd, 32);
> > +    for (i = 0; i < num_words - 1; i++, reg += sizeof(bm))
> > +        __mpam_write_reg(msc, reg, bm);
> > +
> > +    /*
> > +     * ....and then the last (maybe) partial 32bit word. When wd is a
> > +     * multiple of 32, msb should be 31 to write a full 32bit word.
> > +     */
> > +    msb = (wd - 1) % 32;
> > +    bm = GENMASK(msb, 0);
> > +    if (bm)
> > +        __mpam_write_reg(msc, reg, bm);
> Drop the 'if' as the 0 bit will always be part of the mask.

Yup, this was a harmless leftover of the versions that tried to optionally write the left over bits.


> >   @@ -1419,7 +1541,7 @@ static void mpam_enable_once(void)
> >       mpam_register_cpuhp_callbacks(mpam_cpu_online, mpam_cpu_offline);
> >         printk(KERN_INFO "MPAM enabled with %u partid and %u pmg\n",
> > -           mpam_partid_max + 1, mpam_pmg_max + 1);
> > +        READ_ONCE(mpam_partid_max) + 1, mpam_pmg_max + 1);
>
> Belongs in 'arm_mpam: Probe MSCs to find the supported partid/pmg values'.

That value is now protected by a lock, and can't change at this point, so it no longer needs the READ_ONCE() anyway.


Thanks,

James