[PATCH 23/33] arm_mpam: Allow configuration to be applied and restored during cpu online

[Jonathan Cameron]

On Fri, 7 Nov 2025 12:34:40 +0000
Ben Horgan <ben.horgan at arm.com> wrote:

> From: James Morse <james.morse at arm.com>
> 
> When CPUs come online the MSC's original configuration should be restored.
> 
> Add struct mpam_config to hold the configuration. This has a bitmap of
> features that were modified. Once the maximum partid is known, allocate

I'm not following 'were modified'.  When?  Sometime in the past?
Perhaps "features that have been modified when XXX happens" or

Having read the code I think this is something like "are modified as configuration
is read".

> a configuration array for each component, and reprogram each RIS
> configuration from this.
> 
> CC: Dave Martin <Dave.Martin at arm.com>
> Signed-off-by: James Morse <james.morse at arm.com>
> Cc: Shaopeng Tan (Fujitsu) tan.shaopeng at fujitsu.com
> Cc: Peter Newman peternewman at google.com
> Signed-off-by: Ben Horgan <ben.horgan at arm.com>
> ---
> Changes since v3:
> Drop tags
> Fix component reset, otherwise cpbm wrong and controls not set.
> Add a cfg_lock to guard configuration of an msc

The use of bitmap_set() for things that aren't unsigned long (arrays) is a bad
idea. Much better to use GENMASK() to fill those.

> ---
>  drivers/resctrl/mpam_devices.c  | 268 ++++++++++++++++++++++++++++++--
>  drivers/resctrl/mpam_internal.h |  27 ++++
>  2 files changed, 280 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
> index 3a0ad8d93fff..8b0944bdaf28 100644
> --- a/drivers/resctrl/mpam_devices.c
> +++ b/drivers/resctrl/mpam_devices.c



> @@ -1125,6 +1225,9 @@ static struct mpam_msc *do_mpam_msc_drv_probe(struct platform_device *pdev)
>  	if (err)
>  		return ERR_PTR(err);
>  	err = devm_mutex_init(dev, &msc->error_irq_lock);
> +	if (err)
> +		return ERR_PTR(err);
Trivial: As in earlier patches. I'd put a blank line here for readability.
> +	err = devm_mutex_init(dev, &msc->cfg_lock);
>  	if (err)
>  		return ERR_PTR(err);
>  	mpam_mon_sel_lock_init(msc);
> @@ -1585,6 +1688,70 @@ static void mpam_unregister_irqs(void)
>  	}
>  }
>  
> +static void __destroy_component_cfg(struct mpam_component *comp)
> +{
> +	add_to_garbage(comp->cfg);
> +}
> +
> +static void mpam_reset_component_cfg(struct mpam_component *comp)
> +{
> +	int i;
> +	struct mpam_props *cprops = &comp->class->props;
> +
> +	mpam_assert_partid_sizes_fixed();
> +
> +	if (!comp->cfg)
> +		return;
> +
> +	for (i = 0; i <= mpam_partid_max; i++) {
> +		comp->cfg[i] = (struct mpam_config) {};
> +		bitmap_fill(comp->cfg[i].features, MPAM_FEATURE_LAST);
> +		bitmap_set((unsigned long *)&comp->cfg[i].cpbm, 0, cprops->cpbm_wd);

Why manipulate a u32 with bitmap_set() with a horrible pretend it's an unsigned long cast.
Instead just do:
		comp->cfg[i].cpbm = GENMASK(cprops->cpbm_wd, 0);
Which is indeed what bitmap_set will do internally due to an optimization for small bitmaps
but lets avoid that making one integer pretend to be another of a different length.


> +		bitmap_set((unsigned long *)&comp->cfg[i].mbw_pbm, 0, cprops->mbw_pbm_bits);
> +		bitmap_set((unsigned long *)&comp->cfg[i].mbw_max, 16 - cprops->bwa_wd, cprops->bwa_wd);
> +	}
> +}