On Tue, Mar 4, 2025 at 8:57 AM Connor Abbott <cwabbott0@xxxxxxxxx> wrote:
>
> In some cases drm/msm has to resume a stalled transaction directly in
> its fault handler. Experimentally this doesn't work on SMMU500 if the
> fault hasn't already been acknowledged by clearing FSR. Rather than
> trying to clear FSR in msm's fault handler and implementing a
> tricky handshake to avoid accidentally clearing FSR twice, we want to
> clear FSR before calling the fault handlers, but this means that the
> contents of registers can change underneath us in the fault handler and
> msm currently uses a private function to read the register contents for
> its own purposes in its fault handler, such as using the
> implementation-defined FSYNR1 to determine which block caused the fault.
> Fix this by making msm use the register values already read by arm-smmu
> itself before clearing FSR rather than messing around with reading
> registers directly.
>
> Signed-off-by: Connor Abbott <cwabbott0@xxxxxxxxx>
Reviewed-by: Rob Clark <robdclark@xxxxxxxxx>
> ---
> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 19 +++++++++----------
> drivers/iommu/arm/arm-smmu/arm-smmu.c | 14 +++++++-------
> drivers/iommu/arm/arm-smmu/arm-smmu.h | 21 +++++++++++----------
> 3 files changed, 27 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> index 6372f3e25c4bc24cb52f9233095170e8aa510a53..186d6ad4fd1c990398df4dec53f4d58ada9e658c 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> @@ -62,16 +62,15 @@ static void qcom_adreno_smmu_get_fault_info(const void *cookie,
> struct adreno_smmu_fault_info *info)
> {
> struct arm_smmu_domain *smmu_domain = (void *)cookie;
> - struct arm_smmu_cfg *cfg = &smmu_domain->cfg;
> - struct arm_smmu_device *smmu = smmu_domain->smmu;
> -
> - info->fsr = arm_smmu_cb_read(smmu, cfg->cbndx, ARM_SMMU_CB_FSR);
> - info->fsynr0 = arm_smmu_cb_read(smmu, cfg->cbndx, ARM_SMMU_CB_FSYNR0);
> - info->fsynr1 = arm_smmu_cb_read(smmu, cfg->cbndx, ARM_SMMU_CB_FSYNR1);
> - info->far = arm_smmu_cb_readq(smmu, cfg->cbndx, ARM_SMMU_CB_FAR);
> - info->cbfrsynra = arm_smmu_gr1_read(smmu, ARM_SMMU_GR1_CBFRSYNRA(cfg->cbndx));
> - info->ttbr0 = arm_smmu_cb_readq(smmu, cfg->cbndx, ARM_SMMU_CB_TTBR0);
> - info->contextidr = arm_smmu_cb_read(smmu, cfg->cbndx, ARM_SMMU_CB_CONTEXTIDR);
> + struct arm_smmu_context_fault_info *cfi = &smmu_domain->cfi;
> +
> + info->fsr = cfi->fsr;
> + info->fsynr0 = cfi->fsynr0;
> + info->fsynr1 = cfi->fsynr1;
> + info->far = cfi->iova;
> + info->cbfrsynra = cfi->cbfrsynra;
> + info->ttbr0 = cfi->ttbr0;
> + info->contextidr = cfi->contextidr;
> }
>
> static void qcom_adreno_smmu_set_stall(const void *cookie, bool enabled)
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> index a9213e0f1579d1e3be0bfba75eea1d5de23117de..498b96e95cb4fdb67c246ef13de1eb8f40d68f7d 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> @@ -453,26 +453,26 @@ void arm_smmu_print_context_fault_info(struct arm_smmu_device *smmu, int idx,
>
> static irqreturn_t arm_smmu_context_fault(int irq, void *dev)
> {
> - struct arm_smmu_context_fault_info cfi;
> struct arm_smmu_domain *smmu_domain = dev;
> + struct arm_smmu_context_fault_info *cfi = &smmu_domain->cfi;
> struct arm_smmu_device *smmu = smmu_domain->smmu;
> static DEFINE_RATELIMIT_STATE(rs, DEFAULT_RATELIMIT_INTERVAL,
> DEFAULT_RATELIMIT_BURST);
> int idx = smmu_domain->cfg.cbndx;
> int ret;
>
> - arm_smmu_read_context_fault_info(smmu, idx, &cfi);
> + arm_smmu_read_context_fault_info(smmu, idx, cfi);
>
> - if (!(cfi.fsr & ARM_SMMU_CB_FSR_FAULT))
> + if (!(cfi->fsr & ARM_SMMU_CB_FSR_FAULT))
> return IRQ_NONE;
>
> - ret = report_iommu_fault(&smmu_domain->domain, NULL, cfi.iova,
> - cfi.fsynr0 & ARM_SMMU_CB_FSYNR0_WNR ? IOMMU_FAULT_WRITE : IOMMU_FAULT_READ);
> + ret = report_iommu_fault(&smmu_domain->domain, NULL, cfi->iova,
> + cfi->fsynr0 & ARM_SMMU_CB_FSYNR0_WNR ? IOMMU_FAULT_WRITE : IOMMU_FAULT_READ);
>
> if (ret == -ENOSYS && __ratelimit(&rs))
> - arm_smmu_print_context_fault_info(smmu, idx, &cfi);
> + arm_smmu_print_context_fault_info(smmu, idx, cfi);
>
> - arm_smmu_cb_write(smmu, idx, ARM_SMMU_CB_FSR, cfi.fsr);
> + arm_smmu_cb_write(smmu, idx, ARM_SMMU_CB_FSR, cfi->fsr);
> return IRQ_HANDLED;
> }
>
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.h b/drivers/iommu/arm/arm-smmu/arm-smmu.h
> index d3bc77dcd4d40f25bc70f3289616fb866649b022..411d807e0a7033833716635efb3968a0bd3ff237 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.h
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.h
> @@ -373,6 +373,16 @@ enum arm_smmu_domain_stage {
> ARM_SMMU_DOMAIN_NESTED,
> };
>
> +struct arm_smmu_context_fault_info {
> + unsigned long iova;
> + u64 ttbr0;
> + u32 fsr;
> + u32 fsynr0;
> + u32 fsynr1;
> + u32 cbfrsynra;
> + u32 contextidr;
> +};
> +
> struct arm_smmu_domain {
> struct arm_smmu_device *smmu;
> struct io_pgtable_ops *pgtbl_ops;
> @@ -380,6 +390,7 @@ struct arm_smmu_domain {
> const struct iommu_flush_ops *flush_ops;
> struct arm_smmu_cfg cfg;
> enum arm_smmu_domain_stage stage;
> + struct arm_smmu_context_fault_info cfi;
> struct mutex init_mutex; /* Protects smmu pointer */
> spinlock_t cb_lock; /* Serialises ATS1* ops and TLB syncs */
> struct iommu_domain domain;
> @@ -541,16 +552,6 @@ struct arm_smmu_device *qcom_smmu_impl_init(struct arm_smmu_device *smmu);
> void arm_smmu_write_context_bank(struct arm_smmu_device *smmu, int idx);
> int arm_mmu500_reset(struct arm_smmu_device *smmu);
>
> -struct arm_smmu_context_fault_info {
> - unsigned long iova;
> - u64 ttbr0;
> - u32 fsr;
> - u32 fsynr0;
> - u32 fsynr1;
> - u32 cbfrsynra;
> - u32 contextidr;
> -};
> -
> void arm_smmu_read_context_fault_info(struct arm_smmu_device *smmu, int idx,
> struct arm_smmu_context_fault_info *cfi);
>
>
> --
> 2.47.1
>
