Re: [PATCH v10 08/14] arm_mpam: propagate MSC access errors for interrupt control

[Ben Horgan <ben.horgan@arm.com>]

Hi Andre,

On 11/09/2026 12:28, Andre Przywara wrote:
> Allow the functions dealing with interrupt registration and enablement
> to check for and return errors, and propagate MSC read and write errors
> from the lower level up.
> This does not cover the IRQ handler yet, as this needs some more
> attention.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> Reviewed-by: Srivathsa L Rao <srivathsa.rao@oss.qualcomm.com>

Commenting here as this is the last error propagation patch. I think we need a clean boundary for
the error propagation. As you said a while back I think we can propagate error for all non-static
functions from mpam_devices.c and the callers can discard the errors if appropriate. For instance,
mpam_reset_class_locked() should propagate any error.

Thanks,

Ben

> ---
>  drivers/resctrl/mpam_devices.c | 31 +++++++++++++++----------------
>  1 file changed, 15 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
> index 558efab75f911..eb26c25b9f970 100644
> --- a/drivers/resctrl/mpam_devices.c
> +++ b/drivers/resctrl/mpam_devices.c
> @@ -1105,7 +1105,9 @@ static int mpam_msc_hw_probe(struct mpam_msc *msc)
>  	}
>  
>  	/* Clear any stale errors */
> -	mpam_msc_clear_esr(msc);
> +	ret = mpam_msc_clear_esr(msc);
> +	if (ret)
> +		return ret;
>  
>  	spin_lock(&partid_max_lock);
>  	mpam_partid_max = min(mpam_partid_max, msc->partid_max);
> @@ -2668,9 +2670,7 @@ static int mpam_enable_msc_ecr(void *_msc)
>  {
>  	struct mpam_msc *msc = _msc;
>  
> -	__mpam_write_reg(msc, MPAMF_ECR, MPAMF_ECR_INTEN);
> -
> -	return 0;
> +	return __mpam_write_reg(msc, MPAMF_ECR, MPAMF_ECR_INTEN);
>  }
>  
>  /* This can run in mpam_disable(), and the interrupt handler on the same CPU */
> @@ -2678,9 +2678,7 @@ static int mpam_disable_msc_ecr(void *_msc)
>  {
>  	struct mpam_msc *msc = _msc;
>  
> -	__mpam_write_reg(msc, MPAMF_ECR, 0);
> -
> -	return 0;
> +	return __mpam_write_reg(msc, MPAMF_ECR, 0);
>  }
>  
>  static irqreturn_t __mpam_irq_handler(int irq, struct mpam_msc *msc)
> @@ -2777,11 +2775,13 @@ static int mpam_register_irqs(void)
>  				return err;
>  		}
>  
> -		mutex_lock(&msc->error_irq_lock);
> -		msc->error_irq_req = true;
> -		mpam_touch_msc(msc, mpam_enable_msc_ecr, msc);
> -		msc->error_irq_hw_enabled = true;
> -		mutex_unlock(&msc->error_irq_lock);
> +		scoped_guard(mutex, &msc->error_irq_lock) {
> +			msc->error_irq_req = true;
> +			err = mpam_touch_msc(msc, mpam_enable_msc_ecr, msc);
> +			if (err)
> +				return err;
> +			msc->error_irq_hw_enabled = true;
> +		}
>  	}
>  
>  	return 0;
> @@ -2800,10 +2800,10 @@ static void mpam_unregister_irqs(void)
>  		if (irq <= 0)
>  			continue;
>  
> -		mutex_lock(&msc->error_irq_lock);
> +		guard(mutex)(&msc->error_irq_lock);
>  		if (msc->error_irq_hw_enabled) {
> -			mpam_touch_msc(msc, mpam_disable_msc_ecr, msc);
> -			msc->error_irq_hw_enabled = false;
> +			if (!mpam_touch_msc(msc, mpam_disable_msc_ecr, msc))
> +				msc->error_irq_hw_enabled = false;
>  		}
>  
>  		if (msc->error_irq_req) {
> @@ -2815,7 +2815,6 @@ static void mpam_unregister_irqs(void)
>  			}
>  			msc->error_irq_req = false;
>  		}
> -		mutex_unlock(&msc->error_irq_lock);
>  	}
>  }
>