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

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

[Ping Li]

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. On an implementation without
ESR these accesses reach unimplemented register space.

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
-- 
2.34.1

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

[Ben Horgan]

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

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

[Ping Li]

Hi Ben,

You are right, thanks for the pointers to the spec.

Our MSC is an MPAM v1.0 implementation without MPAMF_ESR, and the RTL
responds to accesses of unimplemented registers with a bus error
instead of RAZ/WI - which is what motivated the patch. Reading
MPAMF_ESR from the CPU therefore raises an external abort rather than
reading as zero.

Per RLNXX this implementation is not compliant, so the fix belongs in
hardware: the MSC design team will change accesses to unimplemented
registers to be RAZ/WI, as the specification requires.

With that in place the existing driver code is safe, so I am
withdrawing this patch.

Thanks,
Ping