Re: [PATCH] arm_mpam: Do not access MPAMF_ESR on MSCs that do not have it

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

Hi Ping,

On 19/10/2026 13:46, Ping Li wrote:
> mpam_msc_clear_esr() and mpam_msc_read_esr() read MPAMF_ESR
> unconditionally, but the MPAM architecture only provides this
> register when MPAMF_IDR.HAS_ESR is set. 

This isn't always true. With MSC v1.0 then there may be MPAMF_ESR but there is no MPAMF_IDR.HAS_ESR bit.

On an implementation without
> ESR these accesses reach unimplemented register space.

Rather, they are either fully implemented or RAZ/WI.

In version B.d of the MPAM spec section 7.3,
RLNXX:
If FEAT_MPAMv1p0 is implemented and it is not possible to detect error conditions, then MPAMF_ESR
and MPAMF_ECR must be RAZ/WI.

PNPQQ:
If all of the following apply to an error condition, then an MPAM MSC implementation is required to
detect and report that error:
• At least one feature that can cause the error condition is supported.
• The MSC is designed such that the error condition can occur.
• Detection of the error condition is required. See Required error condition detection.


These two rules together indicate that for MSC v1.0 that MPAMF_ESR should either be RAZ/WI or
implemented.

SJFMB:
If all of the following apply, MPAMF_IDR.HAS_ESR is permitted to be 0:
 • FEAT_MPAMv0p1, or from FEAT_MPAMv1p1, is implemented.
 • No error conditions meet the criteria in PNPQQ.

Together with the comment in 9.5.5 for HAS_ESR (quoted below), this indicates that for MSC v1.1 that
MPAMF_ESR should be RAZ/WI or implemented.

"
If an MSC cannot encounter any of the error conditions listed in MPAM MSC Errors of Arm® Memory
System Resource Partitioning and Monitoring (MPAM) System Component Specification (ARM IHI 0099),
both the MPAMF_ESR and MPAMF_ECR must be RAZ/WI.
"

As such, reading and writing MPAMF_ESR should either behave as per the register definition or be
benign but ineffectual. Have you got a platform that behaves differently?

Thanks,

Ben

> 
> Read HAS_ESR from the IDR during probe, make clear_esr() a no-op on
> such MSCs, and have read_esr() return -ENODEV as the register
> cannot be read.
> 
> Signed-off-by: Ping Li <leeonion.muyu@gmail.com>
> ---
>  drivers/resctrl/mpam_devices.c  | 7 +++++++
>  drivers/resctrl/mpam_internal.h | 1 +
>  2 files changed, 8 insertions(+)
> 
> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
> index 48adc49b833c..a473b2872183 100644
> --- a/drivers/resctrl/mpam_devices.c
> +++ b/drivers/resctrl/mpam_devices.c
> @@ -388,6 +388,9 @@ static int mpam_msc_clear_esr(struct mpam_msc *msc)
>  	u32 esr_low;
>  	int ret;
>  
> +	if (!msc->has_esr)
> +		return 0;
> +
>  	ret = __mpam_read_reg(msc, MPAMF_ESR, &esr_low);
>  	if (ret)
>  		return ret;
> @@ -415,6 +418,9 @@ static int mpam_msc_read_esr(struct mpam_msc *msc, u64 *res)
>  	u32 esr_high, esr_low;
>  	int ret;
>  
> +	if (!msc->has_esr)
> +		return -ENODEV;
> +
>  	ret = __mpam_read_reg(msc, MPAMF_ESR, &esr_low);
>  	if (ret)
>  		return ret;
> @@ -1205,6 +1211,7 @@ static int mpam_msc_hw_probe(struct mpam_msc *msc)
>  		pmg_max = FIELD_GET(MPAMF_IDR_PMG_MAX, idr);
>  		msc->partid_max = min(msc->partid_max, partid_max);
>  		msc->pmg_max = min(msc->pmg_max, pmg_max);
> +		msc->has_esr = FIELD_GET(MPAMF_IDR_HAS_ESR, idr);
>  		msc->has_extd_esr = FIELD_GET(MPAMF_IDR_HAS_EXTD_ESR, idr);
>  
>  		scoped_guard(mutex, &mpam_list_lock) {
> diff --git a/drivers/resctrl/mpam_internal.h b/drivers/resctrl/mpam_internal.h
> index 1f86cc158100..b648c006e102 100644
> --- a/drivers/resctrl/mpam_internal.h
> +++ b/drivers/resctrl/mpam_internal.h
> @@ -80,6 +80,7 @@ struct mpam_msc {
>  	int			fb_id;
>  	u32			nrdy_usec;
>  	cpumask_t		accessibility;
> +	bool			has_esr;
>  	bool			has_extd_esr;
>  
>  	int				reenable_error_ppi;
> 
> base-commit: 8ea457f746d915d55f78b43d33b1cd618f3d93ad