Re: [RFC PATCH 35/38] arm_mpam: Add workaround for T241-MPAM-1

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

Hi James,

On 12/5/25 21:58, James Morse wrote:
> From: Shanker Donthineni <sdonthineni@nvidia.com>
> 
> The MPAM bandwidth partitioning controls will not be correctly configured,
> and hardware will retain default configuration register values, meaning
> generally that bandwidth will remain unprovisioned.
> 
> To address the issue, follow the below steps after updating the MBW_MIN
> and/or MBW_MAX registers.
> 
>  - Perform 64b reads from all 12 bridge MPAM shadow registers at offsets
>    (0x360048 + slice*0x10000 + partid*8). These registers are read-only.
>  - Continue iterating until all 12 shadow register values match in a loop.
>    pr_warn_once if the values fail to match within the loop count 1000.
>  - Perform 64b writes with the value 0x0 to the two spare registers at
>    offsets 0x1b0000 and 0x1c0000.
> 
> In the hardware, writes to the MPAMCFG_MBW_MAX MPAMCFG_MBW_MIN registers
> are transformed into broadcast writes to the 12 shadow registers. The
> final two writes to the spare registers cause a final rank of downstream
> micro-architectural MPAM registers to be updated from the shadow copies.
> The intervening loop to read the 12 shadow registers helps avoid a race
> condition where writes to the spare registers occur before all shadow
> registers have been updated.
> 
> Signed-off-by: Shanker Donthineni <sdonthineni@nvidia.com>
> [ morse: Merged the min/max update into a single
>   mpam_quirk_post_config_change() helper. Stashed the t241_id in the msc
>   instead of carrying the physical address around. Test the msc quirk bit
>   instead of a static key. ]
> Signed-off-by: James Morse <james.morse@arm.com>
> ---
>  Documentation/arch/arm64/silicon-errata.rst |  2 +
>  drivers/resctrl/mpam_devices.c              | 87 +++++++++++++++++++++
>  drivers/resctrl/mpam_internal.h             |  9 +++
>  3 files changed, 98 insertions(+)
> 
> diff --git a/Documentation/arch/arm64/silicon-errata.rst b/Documentation/arch/arm64/silicon-errata.rst
> index a7ec57060f64..4e86b85fe3d6 100644
> --- a/Documentation/arch/arm64/silicon-errata.rst
> +++ b/Documentation/arch/arm64/silicon-errata.rst
> @@ -246,6 +246,8 @@ stable kernels.
>  +----------------+-----------------+-----------------+-----------------------------+
>  | NVIDIA         | T241 GICv3/4.x  | T241-FABRIC-4   | N/A                         |
>  +----------------+-----------------+-----------------+-----------------------------+
> +| NVIDIA         | T241 MPAM       | T241-MPAM-1     | N/A                         |
> ++----------------+-----------------+-----------------+-----------------------------+
>  +----------------+-----------------+-----------------+-----------------------------+
>  | Freescale/NXP  | LS2080A/LS1043A | A-008585        | FSL_ERRATUM_A008585         |
>  +----------------+-----------------+-----------------+-----------------------------+
> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
> index f0f6f9b55ad4..f1f03ceade0a 100644
> --- a/drivers/resctrl/mpam_devices.c
> +++ b/drivers/resctrl/mpam_devices.c
> @@ -29,6 +29,16 @@
>  
>  #include "mpam_internal.h"
>  
> +/* Values for the T241 errata workaround */
> +#define T241_CHIPS_MAX			4
> +#define T241_CHIP_NSLICES		12
> +#define T241_SPARE_REG0_OFF		0x1b0000
> +#define T241_SPARE_REG1_OFF		0x1c0000
> +#define T241_CHIP_ID(phys)		FIELD_GET(GENMASK_ULL(44, 43), phys)
> +#define T241_SHADOW_REG_OFF(sidx, pid)	(0x360048 + (sidx) * 0x10000 + (pid) * 8)
> +#define SMCCC_SOC_ID_T241		0x036b0241
> +static void __iomem *t241_scratch_regs[T241_CHIPS_MAX];
> +
>  /*
>   * mpam_list_lock protects the SRCU lists when writing. Once the
>   * mpam_enabled key is enabled these lists are read-only,
> @@ -630,7 +640,44 @@ static struct mpam_msc_ris *mpam_get_or_create_ris(struct mpam_msc *msc,
>  	return ERR_PTR(-ENOENT);
>  }
>  
> +static void mpam_enable_quirk_nvidia_t241(struct mpam_msc *msc,
> +					  const struct mpam_quirk *quirk)
> +{
> +	s32 soc_id = arm_smccc_get_soc_id_version();
> +	struct resource *r;
> +	phys_addr_t phys;
> +
> +	/*
> +	 * A mapping to a device other than the MSC is needed, check
> +	 * SOC_ID is  NVIDIA T241 chip (036b:0241)
> +	 */
> +	if (soc_id < 0 || soc_id != SMCCC_SOC_ID_T241)
> +		return;
> +
> +	r = platform_get_resource(msc->pdev, IORESOURCE_MEM, 0);
> +	if (!r)
> +		return;
> +
> +	/* Find the internal registers base addr from the CHIP ID */
> +	msc->t241_id = T241_CHIP_ID(r->start);
> +	phys = FIELD_PREP(GENMASK_ULL(45, 44), msc->t241_id) | 0x19000000ULL;
> +
> +	t241_scratch_regs[msc->t241_id] = ioremap(phys, SZ_8M);
> +	if (WARN_ON_ONCE(!t241_scratch_regs[msc->t241_id]))
> +		return;
> +
> +	mpam_set_quirk(quirk->workaround, msc);
> +	pr_info_once("Enabled workaround for NVIDIA T241 erratum T241-MPAM-1\n");
> +}
> +
>  static const struct mpam_quirk mpam_quirks[] = {
> +	{
> +	/* NVIDIA t241 erratum T241-MPAM-1 */
> +	.init       = mpam_enable_quirk_nvidia_t241,
> +	.iidr       = MPAM_IIDR_NVIDIA_T421,

MPAM_IIDR_NVIDIA_T421 -> MPAM_IIDR_NVIDIA_T241

> +	.iidr_mask  = MPAM_IIDR_MATCH_ONE,
> +	.workaround = T241_SCRUB_SHADOW_REGS,
> +	},
>  	{ NULL }, /* Sentinel */
>  };
>  
> @@ -1372,6 +1419,44 @@ static void mpam_reset_msc_bitmap(struct mpam_msc *msc, u16 reg, u16 wd)
>  	__mpam_write_reg(msc, reg, bm);
>  }
>  
> +static void mpam_apply_t241_erratum(struct mpam_msc_ris *ris, u16 partid)
> +{
> +	int sidx, i, lcount = 1000;
> +	void __iomem *regs;
> +	u64 val0, val;
> +
> +	regs = t241_scratch_regs[ris->vmsc->msc->t241_id];
> +
> +	for (i = 0; i < lcount; i++) {
> +		/* Read the shadow register at index 0 */
> +		val0 = readq_relaxed(regs + T241_SHADOW_REG_OFF(0, partid));
> +
> +		/* Check if all the shadow registers have the same value */
> +		for (sidx = 1; sidx < T241_CHIP_NSLICES; sidx++) {
> +			val = readq_relaxed(regs +
> +					    T241_SHADOW_REG_OFF(sidx, partid));
> +			if (val != val0)
> +				break;
> +		}
> +		if (sidx == T241_CHIP_NSLICES)
> +			break;
> +	}
> +
> +	if (i == lcount)
> +		pr_warn_once("t241: inconsistent values in shadow regs");
> +
> +	/* Write a value zero to spare registers to take effect of MBW conf */
> +	writeq_relaxed(0, regs + T241_SPARE_REG0_OFF);
> +	writeq_relaxed(0, regs + T241_SPARE_REG1_OFF);
> +}
> +
> +static void mpam_quirk_post_config_change(struct mpam_msc_ris *ris, u16 partid,
> +					  struct mpam_config *cfg)
> +{
> +	if (mpam_has_quirk(T241_SCRUB_SHADOW_REGS, ris->vmsc->msc))
> +		mpam_apply_t241_erratum(ris, partid);
> +}
> +
>  /* Called via IPI. Call while holding an SRCU reference */
>  static void mpam_reprogram_ris_partid(struct mpam_msc_ris *ris, u16 partid,
>  				      struct mpam_config *cfg)
> @@ -1455,6 +1540,8 @@ static void mpam_reprogram_ris_partid(struct mpam_msc_ris *ris, u16 partid,
>  		mpam_write_partsel_reg(msc, PRI, pri_val);
>  	}
>  
> +	mpam_quirk_post_config_change(ris, partid, cfg);
> +
>  	mutex_unlock(&msc->part_sel_lock);
>  }
>  
> diff --git a/drivers/resctrl/mpam_internal.h b/drivers/resctrl/mpam_internal.h
> index de3e5faa12b2..70b78cfd1f5b 100644
> --- a/drivers/resctrl/mpam_internal.h
> +++ b/drivers/resctrl/mpam_internal.h
> @@ -133,6 +133,9 @@ struct mpam_msc {
>  	void __iomem		*mapped_hwpage;
>  	size_t			mapped_hwpage_sz;
>  
> +	/* Values only used on some platforms for quirks */
> +	u32			t241_id;
> +
>  	struct mpam_garbage	garbage;
>  };
>  
> @@ -219,6 +222,7 @@ struct mpam_props {
>  
>  /* Workaround bits for msc->quirks */
>  enum mpam_device_quirks {
> +	T241_SCRUB_SHADOW_REGS,
>  	MPAM_QUIRK_LAST,
>  };
>  
> @@ -239,6 +243,11 @@ struct mpam_quirk {
>  				FIELD_PREP_CONST(MPAMF_IIDR_REVISION,    0xf  ) | \
>  				FIELD_PREP_CONST(MPAMF_IIDR_IMPLEMENTER, 0xfff)
>  
> +#define MPAM_IIDR_NVIDIA_T421   FIELD_PREP_CONST(MPAMF_IIDR_PRODUCTID,   0x241) | \

MPAM_IIDR_NVIDIA_T421 -> MPAM_IIDR_NVIDIA_T241

> +				FIELD_PREP_CONST(MPAMF_IIDR_VARIANT,     0    ) | \
> +				FIELD_PREP_CONST(MPAMF_IIDR_REVISION,    0    ) | \
> +				FIELD_PREP_CONST(MPAMF_IIDR_IMPLEMENTER, 0x36b)
> +
>  
>  /* The values for MSMON_CFG_MBWU_FLT.RWBW */
>  enum mon_filter_options {

Thanks,

Ben