Re: [PATCH v6 02/40] arm_mpam: Reset when feature configuration bit unset

[Jinjie Ruan]

在 2026/3/13 22:45, Ben Horgan 写道:
> To indicate that the configuration, of the controls used by resctrl, in a
> RIS need resetting to driver defaults the reset flags in mpam_config are
> set. However, these flags are only ever set temporarily at RIS scope in
> mpam_reset_ris() and hence mpam_cpu_online() will never reset these
> controls to default. As the hardware reset is unknown this leads to unknown
> configuration when the control values haven't been configured away from the
> defaults.
> 
> Use the policy that an unset feature configuration bit means reset. In this
> way the mpam_config in the component can encode that it should be in reset
> state and mpam_reprogram_msc() will reset controls as needed.
> 
> Fixes: 09b89d2a72f3 ("arm_mpam: Allow configuration to be applied and restored during cpu online")
> Signed-off-by: Ben Horgan <ben.horgan@xxxxxxx>
> ---
> This goes back to the initial feature configuration policy that James
> used in the MPAM base driver rfc but I unfortunately
> suggested him to change it.
> ---
>  drivers/resctrl/mpam_devices.c | 40 ++++++++++------------------------
>  1 file changed, 12 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
> index 0fd6590a9b5c..ff861291bd4e 100644
> --- a/drivers/resctrl/mpam_devices.c
> +++ b/drivers/resctrl/mpam_devices.c
> @@ -1364,17 +1364,15 @@ static void mpam_reprogram_ris_partid(struct mpam_msc_ris *ris, u16 partid,
>  		__mpam_intpart_sel(ris->ris_idx, partid, msc);
>  	}
>  
> -	if (mpam_has_feature(mpam_feat_cpor_part, rprops) &&
> -	    mpam_has_feature(mpam_feat_cpor_part, cfg)) {
> -		if (cfg->reset_cpbm)
> -			mpam_reset_msc_bitmap(msc, MPAMCFG_CPBM, rprops->cpbm_wd);
> -		else
> +	if (mpam_has_feature(mpam_feat_cpor_part, rprops)) {
> +		if (mpam_has_feature(mpam_feat_cpor_part, cfg))
>  			mpam_write_partsel_reg(msc, CPBM, cfg->cpbm);
> +		else
> +			mpam_reset_msc_bitmap(msc, MPAMCFG_CPBM, rprops->cpbm_wd);
>  	}
>  
> -	if (mpam_has_feature(mpam_feat_mbw_part, rprops) &&
> -	    mpam_has_feature(mpam_feat_mbw_part, cfg)) {
> -		if (cfg->reset_mbw_pbm)
> +	if (mpam_has_feature(mpam_feat_mbw_part, rprops)) {
> +		if (mpam_has_feature(mpam_feat_mbw_part, cfg))
>  			mpam_reset_msc_bitmap(msc, MPAMCFG_MBW_PBM, rprops->mbw_pbm_bits);

This condition is written backwards, but it looks like it has already
been fixed later.

Otherwise, LGTM
Reviewed-by: Jinjie Ruan <ruanjinjie@xxxxxxxxxx>

>  		else
>  			mpam_write_partsel_reg(msc, MBW_PBM, cfg->mbw_pbm);
> @@ -1384,16 +1382,14 @@ static void mpam_reprogram_ris_partid(struct mpam_msc_ris *ris, u16 partid,
>  	    mpam_has_feature(mpam_feat_mbw_min, cfg))
>  		mpam_write_partsel_reg(msc, MBW_MIN, 0);
>  
> -	if (mpam_has_feature(mpam_feat_mbw_max, rprops) &&
> -	    mpam_has_feature(mpam_feat_mbw_max, cfg)) {
> -		if (cfg->reset_mbw_max)
> -			mpam_write_partsel_reg(msc, MBW_MAX, MPAMCFG_MBW_MAX_MAX);
> -		else
> +	if (mpam_has_feature(mpam_feat_mbw_max, rprops)) {
> +		if (mpam_has_feature(mpam_feat_mbw_max, cfg))
>  			mpam_write_partsel_reg(msc, MBW_MAX, cfg->mbw_max);
> +		else
> +			mpam_write_partsel_reg(msc, MBW_MAX, MPAMCFG_MBW_MAX_MAX);
>  	}
>  
> -	if (mpam_has_feature(mpam_feat_mbw_prop, rprops) &&
> -	    mpam_has_feature(mpam_feat_mbw_prop, cfg))
> +	if (mpam_has_feature(mpam_feat_mbw_prop, rprops))
>  		mpam_write_partsel_reg(msc, MBW_PROP, 0);
>  
>  	if (mpam_has_feature(mpam_feat_cmax_cmax, rprops))
> @@ -1491,16 +1487,6 @@ static int mpam_save_mbwu_state(void *arg)
>  	return 0;
>  }
>  
> -static void mpam_init_reset_cfg(struct mpam_config *reset_cfg)
> -{
> -	*reset_cfg = (struct mpam_config) {
> -		.reset_cpbm = true,
> -		.reset_mbw_pbm = true,
> -		.reset_mbw_max = true,
> -	};
> -	bitmap_fill(reset_cfg->features, MPAM_FEATURE_LAST);
> -}
> -
>  /*
>   * Called via smp_call_on_cpu() to prevent migration, while still being
>   * pre-emptible. Caller must hold mpam_srcu.
> @@ -1508,14 +1494,12 @@ static void mpam_init_reset_cfg(struct mpam_config *reset_cfg)
>  static int mpam_reset_ris(void *arg)
>  {
>  	u16 partid, partid_max;
> -	struct mpam_config reset_cfg;
> +	struct mpam_config reset_cfg = {};
>  	struct mpam_msc_ris *ris = arg;
>  
>  	if (ris->in_reset_state)
>  		return 0;
>  
> -	mpam_init_reset_cfg(&reset_cfg);
> -
>  	spin_lock(&partid_max_lock);
>  	partid_max = mpam_partid_max;
>  	spin_unlock(&partid_max_lock);