[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <919ab857-30cf-00a5-1a40-fb180678792e@arm.com>
Date: Mon, 7 Aug 2023 21:14:24 +0100
From: Robin Murphy <robin.murphy@....com>
To: Easwar Hariharan <eahariha@...ux.microsoft.com>,
stable@...r.kernel.org
Cc: easwar.hariharan@...rosoft.com, Nicolin Chen <nicolinc@...dia.com>,
Will Deacon <will@...nel.org>,
Catalin Marinas <catalin.marinas@....com>,
Jonathan Corbet <corbet@....net>,
Joerg Roedel <joro@...tes.org>,
Sasha Levin <sashal@...nel.org>,
Yicong Yang <yangyicong@...ilicon.com>,
Tomas Krcka <krckatom@...zon.de>,
"moderated list:ARM64 PORT (AARCH64 ARCHITECTURE)"
<linux-arm-kernel@...ts.infradead.org>,
"open list:DOCUMENTATION" <linux-doc@...r.kernel.org>,
open list <linux-kernel@...r.kernel.org>,
"open list:IOMMU DRIVERS" <iommu@...ts.linux-foundation.org>,
"open list:IOMMU DRIVERS" <iommu@...ts.linux.dev>
Subject: Re: [PATCH v3 5.15 4/6] iommu/arm-smmu-v3: Document MMU-700 erratum
2812531
On 2023-08-02 18:02, Easwar Hariharan wrote:
> From: Robin Murphy <robin.murphy@....com>
>
> 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@huawei.com/T/#t
Thanks,
Robin.
> Signed-off-by: Robin Murphy <robin.murphy@....com>
> Reviewed-by: Nicolin Chen <nicolinc@...dia.com>
> Link: https://lore.kernel.org/r/330221cdfd0003cd51b6c04e7ff3566741ad8374.1683731256.git.robin.murphy@arm.com
> Signed-off-by: Will Deacon <will@...nel.org>
> Signed-off-by: Easwar Hariharan <eahariha@...ux.microsoft.com>
> ---
> 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;
Powered by blists - more mailing lists