| Message ID | 20221115101122.155440-4-angelogioacchino.delregno@collabora.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add support for Qualcomm's legacy IOMMU v2 | expand |
On 15/11/2022 12:11, AngeloGioacchino Del Regno wrote: > Avoid context faults by resetting the context(s) entirely at > detach_dev() time and also do the same before programming the > context for domain initialization. > > Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org> > [Marijn: Rebased over next-20221111] > Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> Probably: Fixes: 0ae349a0f33f ("iommu/qcom: Add qcom_iommu") What causes context faults? Shouldn't the context be disabled once we write 0 to CB_SCTLR? > --- > drivers/iommu/arm/arm-smmu/qcom_iommu.c | 20 ++++++++++++++++++-- > 1 file changed, 18 insertions(+), 2 deletions(-) > > diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c > index 491a8093f3d6..49f4308f1bd2 100644 > --- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c > +++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c > @@ -223,6 +223,20 @@ static irqreturn_t qcom_iommu_fault(int irq, void *dev) > return IRQ_HANDLED; > } > > +static void qcom_iommu_reset_ctx(struct qcom_iommu_ctx *ctx) > +{ > + iommu_writel(ctx, ARM_SMMU_CB_FAR, 0); > + iommu_writel(ctx, ARM_SMMU_CB_FSR, ARM_SMMU_FSR_FAULT); > + iommu_writel(ctx, ARM_SMMU_CB_S1_MAIR1, 0); > + iommu_writel(ctx, ARM_SMMU_CB_PAR, 0); > + iommu_writel(ctx, ARM_SMMU_CB_S1_MAIR0, 0); > + iommu_writel(ctx, ARM_SMMU_CB_SCTLR, 0); > + iommu_writel(ctx, ARM_SMMU_CB_TCR2, 0); > + iommu_writel(ctx, ARM_SMMU_CB_TCR, 0); > + iommu_writeq(ctx, ARM_SMMU_CB_TTBR0, 0); > + iommu_writeq(ctx, ARM_SMMU_CB_TTBR1, 0); > +} > + > static int qcom_iommu_init_domain(struct iommu_domain *domain, > struct qcom_iommu_dev *qcom_iommu, > struct device *dev) > @@ -273,6 +287,8 @@ static int qcom_iommu_init_domain(struct iommu_domain *domain, > ctx->secure_init = true; > } > > + qcom_iommu_reset_ctx(ctx); Is it enough to write 0 to ARM_SMMU_CB_SCTLR here instead of doing the full reset? > + > /* TTBRs */ > iommu_writeq(ctx, ARM_SMMU_CB_TTBR0, > pgtbl_cfg.arm_lpae_s1_cfg.ttbr | > @@ -401,8 +417,8 @@ static void qcom_iommu_detach_dev(struct iommu_domain *domain, struct device *de > for (i = 0; i < fwspec->num_ids; i++) { > struct qcom_iommu_ctx *ctx = to_ctx(qcom_domain, fwspec->ids[i]); > > - /* Disable the context bank: */ > - iommu_writel(ctx, ARM_SMMU_CB_SCTLR, 0); > + /* Disable and reset the context bank */ > + qcom_iommu_reset_ctx(ctx); > > ctx->domain = NULL; > }
diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c index 491a8093f3d6..49f4308f1bd2 100644 --- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c +++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c @@ -223,6 +223,20 @@ static irqreturn_t qcom_iommu_fault(int irq, void *dev) return IRQ_HANDLED; } +static void qcom_iommu_reset_ctx(struct qcom_iommu_ctx *ctx) +{ + iommu_writel(ctx, ARM_SMMU_CB_FAR, 0); + iommu_writel(ctx, ARM_SMMU_CB_FSR, ARM_SMMU_FSR_FAULT); + iommu_writel(ctx, ARM_SMMU_CB_S1_MAIR1, 0); + iommu_writel(ctx, ARM_SMMU_CB_PAR, 0); + iommu_writel(ctx, ARM_SMMU_CB_S1_MAIR0, 0); + iommu_writel(ctx, ARM_SMMU_CB_SCTLR, 0); + iommu_writel(ctx, ARM_SMMU_CB_TCR2, 0); + iommu_writel(ctx, ARM_SMMU_CB_TCR, 0); + iommu_writeq(ctx, ARM_SMMU_CB_TTBR0, 0); + iommu_writeq(ctx, ARM_SMMU_CB_TTBR1, 0); +} + static int qcom_iommu_init_domain(struct iommu_domain *domain, struct qcom_iommu_dev *qcom_iommu, struct device *dev) @@ -273,6 +287,8 @@ static int qcom_iommu_init_domain(struct iommu_domain *domain, ctx->secure_init = true; } + qcom_iommu_reset_ctx(ctx); + /* TTBRs */ iommu_writeq(ctx, ARM_SMMU_CB_TTBR0, pgtbl_cfg.arm_lpae_s1_cfg.ttbr | @@ -401,8 +417,8 @@ static void qcom_iommu_detach_dev(struct iommu_domain *domain, struct device *de for (i = 0; i < fwspec->num_ids; i++) { struct qcom_iommu_ctx *ctx = to_ctx(qcom_domain, fwspec->ids[i]); - /* Disable the context bank: */ - iommu_writel(ctx, ARM_SMMU_CB_SCTLR, 0); + /* Disable and reset the context bank */ + qcom_iommu_reset_ctx(ctx); ctx->domain = NULL; }