Re: [PATCH] arm_mpam: Warn when firmware leaves PARTIDs disabled

[Ben Horgan]

Hi Ping,

On 27/10/2026 08:35, Ping Li wrote:
> MPAMF_IDR.HAS_ENDIS indicates the MSC supports the MPAMCFG_EN_FLAGS
> PARTID enable/disable feature. Firmware is required to enable all
> PARTIDs at boot before handing over to software that is unaware of
> these registers. A PARTID left disabled silently ignores all resctrl
> schemata changes, as its traffic is treated as having no resources.
> 
> Debugging an IP that implements HAS_ENDIS but had no visible effect
> from resctrl took a while to root cause. The fix belongs in firmware,

I can see how this would be frustrating. In general, we can't rely on firmware to do the reset
correctly e.g. after a kexec. Hence, I'd rather enable all the PARTID as part of the mpam driver
initialization in the same way that we set all the controls to be non-throttling.

I do have a preliminary patch but there seems to be some contradictions in the specification so I
haven't posted it yet.

There are three points of confusion I'm aware of. Firstly, R-CSLHH says the enabled status is not
affected by narrowing and so just applies to the reqPARTID but the descriptions of MPAMCFG_EN and
MPAMCFG_EN_FLAGS indicate that it effects the intPARTID. Secondly, what happens if MPAMCFG_EN.PARTID
and PARTID_SEL are different? Thirdly, is ENDIS RIS-sensitive or a global setting for the MSC.

For the last one, the conclusion seems to be that it is a global insensitive setting but this is
still not fully reflected in the specification.

I'm not aware of how other ENDIS implementations behave. I have asked for internal clarification on
the points above but perhaps you could raise it with support@xxxxxxx so that the architects are
aware this affects your design.

Thanks,

Ben

> but warn during probe so this misconfiguration is reported loudly
> instead of silently.
> 
> Signed-off-by: Ping Li <leeonion.muyu@xxxxxxxxx>
> ---
>  drivers/resctrl/mpam_devices.c  | 16 ++++++++++++++++
>  drivers/resctrl/mpam_internal.h |  2 ++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
> index 48adc49b833c..f03aa40faa0a 100644
> --- a/drivers/resctrl/mpam_devices.c
> +++ b/drivers/resctrl/mpam_devices.c
> @@ -1159,6 +1159,7 @@ static int mpam_msc_hw_probe(struct mpam_msc *msc)
>  {
>  	int ret;
>  	u64 idr;
> +	u32 en_flags;
>  	u16 partid_max;
>  	u8 ris_idx, pmg_max;
>  	struct mpam_msc_ris *ris;
> @@ -1199,6 +1200,21 @@ static int mpam_msc_hw_probe(struct mpam_msc *msc)
>  			ret = mpam_msc_read_idr(msc, &idr);
>  			if (ret)
>  				return ret;
> +
> +			/*
> +			 * Firmware must enable all PARTIDs before handing the
> +			 * system to software that is unaware of EN_FLAGS, so
> +			 * a disabled PARTID here means resctrl schemata
> +			 * changes silently have no effect.
> +			 */
> +			if (FIELD_GET(MPAMF_IDR_HAS_ENDIS, idr)) {
> +				u32 en_mask = GENMASK(FIELD_GET(MPAMF_IDR_PARTID_MAX, idr) % 32, 0);
> +
> +				ret = __mpam_read_reg(msc, MPAMCFG_EN_FLAGS, &en_flags);
> +				if (!ret && (en_flags & en_mask) != en_mask)
> +					dev_err_once(dev,
> +						     "firmware left PARTIDs disabled, schemata changes will not take effect\n");
> +			}
>  		}
>  
>  		partid_max = FIELD_GET(MPAMF_IDR_PARTID_MAX, idr);
> diff --git a/drivers/resctrl/mpam_internal.h b/drivers/resctrl/mpam_internal.h
> index 1f86cc158100..008dc4e10f38 100644
> --- a/drivers/resctrl/mpam_internal.h
> +++ b/drivers/resctrl/mpam_internal.h
> @@ -574,6 +574,7 @@ int mpam_fb_check_shared_buffer_size(struct mpam_msc *msc);
>  #define MPAMCFG_MBW_MIN		0x0200  /* min mem-bw config */
>  #define MPAMCFG_MBW_MAX		0x0208  /* max mem-bw config */
>  #define MPAMCFG_MBW_WINWD	0x0220  /* mem-bw accounting window config */
> +#define MPAMCFG_EN_FLAGS	0x0320  /* partid enable flags config */
>  #define MPAMCFG_MBW_PBM		0x2000  /* mem-bw portion bitmap config */
>  #define MPAMCFG_PRI		0x0400  /* priority partitioning config */
>  #define MPAMCFG_MBW_PROP	0x0500  /* mem-bw stride config */
> @@ -608,6 +609,7 @@ int mpam_fb_check_shared_buffer_size(struct mpam_msc *msc);
>  #define MPAMF_IDR_HAS_RIS		BIT(32)
>  #define MPAMF_IDR_HAS_EXTD_ESR		BIT(38)
>  #define MPAMF_IDR_HAS_ESR		BIT(39)
> +#define MPAMF_IDR_HAS_ENDIS		BIT(42)
>  #define MPAMF_IDR_RIS_MAX		GENMASK(59, 56)
>  
>  /* MPAMF_MSMON_IDR - MPAM performance monitoring ID register */