Re: [PATCH v12 03/13] arm_mpam: propagate MSC access errors for MBWU counters

[James Morse]

Hi Andre,

I've picked up this series to send on to Catalin and Will ...

On 01/10/2026 16:34, Andre Przywara wrote:
> Allow the mpam_msc_read_mbwu_l() function to return an error, and
> propagate errors from the lower level up. This also changes the users
> of this function.

> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
> index 3aefeab371788..583560798dd8b 100644
> --- a/drivers/resctrl/mpam_devices.c
> +++ b/drivers/resctrl/mpam_devices.c
> @@ -1327,10 +1338,12 @@ static void __ris_msmon_read(void *arg)
>  	struct mpam_msc *msc = m->ris->vmsc->msc;
>  	u32 mon_sel, ctl_val, flt_val, cur_ctl, cur_flt;
>  
> -	if (!mpam_mon_sel_lock(msc)) {
> +	ACQUIRE(mon_sel_lock, guard)(msc);
> +	if (ACQUIRE_ERR(mon_sel_lock, &guard)) {
>  		m->err = -EIO;
>  		return;
>  	}
> +
>  	mon_sel = FIELD_PREP(MSMON_CFG_MON_SEL_MON_SEL, ctx->mon) |
>  		  FIELD_PREP(MSMON_CFG_MON_SEL_RIS, ris->ris_idx);
>  	mpam_write_monsel_reg(msc, CFG_MON_SEL, mon_sel);


This conflicts with Ben's fixes [0] that turn this into __ris_msmon_read_locked().
I don't think the ACQUIRE buys us anything after that, as none of the callers need
to return early, and __ris_msmon_read_locked() can always just return.

The same thing happens in mpam_save_mbwu_state() where Ben's change means you no
longer need an early return after mpam_msc_read_mbwu_l().

Please double check I've not botched your patch when fixing up the conflict!
https://git.kernel.org/pub/scm/linux/kernel/git/morse/linux.git/tag/?h=mpam/for-next/v7.4_v2

Thanks,

James


[0] https://lore.kernel.org/linux-arm-kernel/20260917145617.2202986-1-ben.horgan@xxxxxxx/


> @@ -1423,7 +1438,6 @@ static void __ris_msmon_read(void *arg)
>  	default:
>  		m->err = -EINVAL;
>  	}
> -	mpam_mon_sel_unlock(msc);
>  
>  	if (nrdy)
>  		m->err = -EBUSY;
> @@ -1784,6 +1798,7 @@ static int mpam_save_mbwu_state(void *arg)
>  {
>  	int i;
>  	u64 val;
> +	int ret;
>  	struct mon_cfg *cfg;
>  	u32 cur_flt, cur_ctl, mon_sel;
>  	struct mpam_msc_ris *ris = arg;
> @@ -1794,7 +1809,8 @@ static int mpam_save_mbwu_state(void *arg)
>  		mbwu_state = &ris->mbwu_state[i];
>  		cfg = &mbwu_state->cfg;
>  
> -		if (WARN_ON_ONCE(!mpam_mon_sel_lock(msc)))
> +		ACQUIRE(mon_sel_lock, guard)(msc);
> +		if (ACQUIRE_ERR(mon_sel_lock, &guard))
>  			return -EIO;
>  
>  		mon_sel = FIELD_PREP(MSMON_CFG_MON_SEL_MON_SEL, i) |
> @@ -1805,7 +1821,9 @@ static int mpam_save_mbwu_state(void *arg)
>  		mpam_write_monsel_reg(msc, CFG_MBWU_CTL, 0);
>  
>  		if (mpam_ris_has_mbwu_long_counter(ris)) {
> -			val = mpam_msc_read_mbwu_l(msc);
> +			ret = mpam_msc_read_mbwu_l(msc, &val);
> +			if (ret)
> +				return ret;
>  			mpam_msc_zero_mbwu_l(msc);
>  		} else {
>  			u32 val32;
> @@ -1821,7 +1839,6 @@ static int mpam_save_mbwu_state(void *arg)
>  		cfg->partid = FIELD_GET(MSMON_CFG_x_FLT_PARTID, cur_flt);
>  		mbwu_state->correction += val;
>  		mbwu_state->enabled = FIELD_GET(MSMON_CFG_x_CTL_EN, cur_ctl);
> -		mpam_mon_sel_unlock(msc);
>  	}
>  
>  	return 0;