Re: [PATCH v3 5.15 4/6] iommu/arm-smmu-v3: Document MMU-700 erratum 2812531

[Robin Murphy]

On 2023-08-02 18:02, Easwar Hariharan wrote:
> From: Robin Murphy <robin.murphy@xxxxxxx>
>
> commit 309a15cb16bb075da1c99d46fb457db6a1a2669e upstream
>
> To work around MMU-700 erratum 2812531 we need to ensure that certain
> sequences of commands cannot be issued without an intervening sync. In
> practice this falls out of our current command-batching machinery
> anyway - each batch only contains a single type of invalidation command,
> and ends with a sync. The only exception is when a batch is sufficiently
> large to need issuing across multiple command queue slots, wherein the
> earlier slots will not contain a sync and thus may in theory interleave
> with another batch being issued in parallel to create an affected
> sequence across the slot boundary.
>
> Since MMU-700 supports range invalidate commands and thus we will prefer
> to use them (which also happens to avoid conditions for other errata),
> I'm not entirely sure it's even possible for a single high-level
> invalidate call to generate a batch of more than 63 commands,

Out of interest, have you observed a case where this actually happens?

> but for
> the sake of robustness and documentation, wire up an option to enforce
> that a sync is always inserted for every slot issued.
>
> The other aspect is that the relative order of DVM commands cannot be
> controlled, so DVM cannot be used. Again that is already the status quo,
> but since we have at least defined ARM_SMMU_FEAT_BTM, we can explicitly
> disable it for documentation purposes even if it's not wired up anywhere
> yet.

Note that there seems to be a slight issue with this patch that I 
missed, under discussion here:

https://lore.kernel.org/linux-iommu/27c895b8-1fb0-be88-8bc3-878d754684c8@xxxxxxxxxx/T/#t

Thanks,
Robin.

> Signed-off-by: Robin Murphy <robin.murphy@xxxxxxx>
> Reviewed-by: Nicolin Chen <nicolinc@xxxxxxxxxx>
> Link: https://lore.kernel.org/r/330221cdfd0003cd51b6c04e7ff3566741ad8374.1683731256.git.robin.murphy@xxxxxxx
> Signed-off-by: Will Deacon <will@xxxxxxxxxx>
> Signed-off-by: Easwar Hariharan <eahariha@xxxxxxxxxxxxxxxxxxx>
> ---
>   Documentation/arm64/silicon-errata.rst      |  2 ++
>   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 12 ++++++++++++
>   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h |  1 +
>   3 files changed, 15 insertions(+)
>
> diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst
> index 55e1e074dec1..322df8abbc0e 100644
> --- a/Documentation/arm64/silicon-errata.rst
> +++ b/Documentation/arm64/silicon-errata.rst
> @@ -124,6 +124,8 @@ stable kernels.
>   +----------------+-----------------+-----------------+-----------------------------+
>   | ARM            | MMU-600         | #1076982        | N/A                         |
>   +----------------+-----------------+-----------------+-----------------------------+
> +| ARM            | MMU-700         | #2812531        | N/A                         |
> ++----------------+-----------------+-----------------+-----------------------------+
>   +----------------+-----------------+-----------------+-----------------------------+
>   | Broadcom       | Brahma-B53      | N/A             | ARM64_ERRATUM_845719        |
>   +----------------+-----------------+-----------------+-----------------------------+
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index 782d040a829c..6a551a48d271 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -897,6 +897,12 @@ static void arm_smmu_cmdq_batch_add(struct arm_smmu_device *smmu,
>   				    struct arm_smmu_cmdq_batch *cmds,
>   				    struct arm_smmu_cmdq_ent *cmd)
>   {
> +	if (cmds->num == CMDQ_BATCH_ENTRIES - 1 &&
> +	    (smmu->options & ARM_SMMU_OPT_CMDQ_FORCE_SYNC)) {
> +		arm_smmu_cmdq_issue_cmdlist(smmu, cmds->cmds, cmds->num, true);
> +		cmds->num = 0;
> +	}
> +
>   	if (cmds->num == CMDQ_BATCH_ENTRIES) {
>   		arm_smmu_cmdq_issue_cmdlist(smmu, cmds->cmds, cmds->num, false);
>   		cmds->num = 0;
> @@ -3461,6 +3467,7 @@ static int arm_smmu_device_reset(struct arm_smmu_device *smmu, bool bypass)
>   
>   #define IIDR_IMPLEMENTER_ARM		0x43b
>   #define IIDR_PRODUCTID_ARM_MMU_600	0x483
> +#define IIDR_PRODUCTID_ARM_MMU_700	0x487
>   
>   static void arm_smmu_device_iidr_probe(struct arm_smmu_device *smmu)
>   {
> @@ -3481,6 +3488,11 @@ static void arm_smmu_device_iidr_probe(struct arm_smmu_device *smmu)
>   			if (variant == 0 && revision <= 2)
>   				smmu->features &= ~ARM_SMMU_FEAT_SEV;
>   			break;
> +		case IIDR_PRODUCTID_ARM_MMU_700:
> +			/* Arm erratum 2812531 */
> +			smmu->features &= ~ARM_SMMU_FEAT_BTM;
> +			smmu->options |= ARM_SMMU_OPT_CMDQ_FORCE_SYNC;
> +			break;
>   		}
>   		break;
>   	}
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> index 5964e02c4e57..abaecdf8d5d2 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> @@ -651,6 +651,7 @@ struct arm_smmu_device {
>   #define ARM_SMMU_OPT_SKIP_PREFETCH	(1 << 0)
>   #define ARM_SMMU_OPT_PAGE0_REGS_ONLY	(1 << 1)
>   #define ARM_SMMU_OPT_MSIPOLL		(1 << 2)
> +#define ARM_SMMU_OPT_CMDQ_FORCE_SYNC	(1 << 3)
>   	u32				options;
>   
>   	struct arm_smmu_cmdq		cmdq;