Re: [PATCH v3 12/16] arm_mpam: propagate MSC write errors for remaining MSC write users

[Jonathan Cameron]

On Fri, 10 Jul 2026 16:45:16 +0200
Andre Przywara <andre.przywara@xxxxxxx> wrote:

> Allow the remaining MSC device functions to return an error, and
> propagate write errors from the lower level up.
> 
> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx>
> 
> propagate write errors up for mpam_save_mbwu_state()
below the sign off?

> ---
>  drivers/resctrl/mpam_devices.c | 82 +++++++++++++++++++++++-----------
>  1 file changed, 56 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
> index 222515a01b35..ca73029654b6 100644
> --- a/drivers/resctrl/mpam_devices.c
> +++ b/drivers/resctrl/mpam_devices.c
> @@ -1162,15 +1162,20 @@ static int mpam_msc_read_mbwu_l(struct mpam_msc *msc, u64 *res)
>  	return ret;
>  }
>  
> -static void mpam_msc_zero_mbwu_l(struct mpam_msc *msc)
> +static int mpam_msc_zero_mbwu_l(struct mpam_msc *msc)
>  {
> +	int ret;
> +
>  	mpam_mon_sel_lock_held(msc);
>  
>  	WARN_ON_ONCE((MSMON_MBWU_L + sizeof(u64)) > msc->mapped_hwpage_sz);
>  	WARN_ON_ONCE(!cpumask_test_cpu(smp_processor_id(), &msc->accessibility));
>  
> -	__mpam_write_reg(msc, MSMON_MBWU_L, 0);
> -	__mpam_write_reg(msc, MSMON_MBWU_L + 4, 0);
> +	ret = __mpam_write_reg(msc, MSMON_MBWU_L, 0);
> +	if (!ret)
> +		ret = __mpam_write_reg(msc, MSMON_MBWU_L + 4, 0);

Code lines are cheap.  Burn them for simple flow.

	ret = __mpam_write_reg(msc, MSMON_MBWU_L, 0);
	if (ret)
		return ret;

	return __mpam_write_reg(msc, MSMON_MBWU_L + 4, 0);


> +
> +	return ret;
>  }
>  
>  static void gen_msmon_ctl_flt_vals(struct mon_read *m, u32 *ctl_val,
> +static int write_msmon_ctl_flt_vals(struct mon_read *m, u32 ctl_val,
> +				    u32 flt_val)
>  {
>  	struct mpam_msc *msc = m->ris->vmsc->msc;
> +	int ret;
>  
>  	/*
>  	 * Write the ctl_val with the enable bit cleared, reset the counter,
> @@ -1260,26 +1266,37 @@ static void write_msmon_ctl_flt_vals(struct mon_read *m, u32 ctl_val,
>  	 */
>  	switch (m->type) {
>  	case mpam_feat_msmon_csu:
> -		mpam_write_monsel_reg(msc, CFG_CSU_FLT, flt_val);
> -		mpam_write_monsel_reg(msc, CFG_CSU_CTL, ctl_val);
> -		mpam_write_monsel_reg(msc, CSU, 0);
> -		mpam_write_monsel_reg(msc, CFG_CSU_CTL, ctl_val | MSMON_CFG_x_CTL_EN);
> +		ret = mpam_write_monsel_reg(msc, CFG_CSU_FLT, flt_val);
> +		if (!ret)
> +			ret = mpam_write_monsel_reg(msc, CFG_CSU_CTL, ctl_val);
> +		if (!ret)
> +			ret = mpam_write_monsel_reg(msc, CSU, 0);
> +		if (!ret)
> +			ret = mpam_write_monsel_reg(msc, CFG_CSU_CTL, ctl_val | MSMON_CFG_x_CTL_EN);

By now you can probably guess my comments on this.

If you ever find yourself with a case where you can't do guard/ acquire()/early returns
etc, use a helper function that can just return on each err.

It is more code, but I'd much rather see that on error we 'stop' and return
than try and follow the path to see if anything else happens.

		ret = a();
		if (ret)
			return ret;

		ret = b();
		if (ret)
			return ret;

etc.