From mboxrd@z Thu Jan 1 00:00:00 1970 From: robin.murphy@arm.com (Robin Murphy) Date: Thu, 30 Jul 2015 20:08:57 +0100 Subject: [PATCH] iommu/arm-smmu-v2: ThunderX(errata-23399) mis-extends 64bit registers In-Reply-To: <20150730184523.GC16663@arm.com> References: <1438278906-29627-1-git-send-email-tchalamarla@caviumnetworks.com> <20150730184523.GC16663@arm.com> Message-ID: <55BA7649.3080608@arm.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 30/07/15 19:45, Will Deacon wrote: > Hello, > > On Thu, Jul 30, 2015 at 06:55:06PM +0100, tchalamarla at caviumnetworks.com wrote: >> From: Tirumalesh Chalamarla >> >> The SMMU architecture defines two different behaviors when 64-bit >> registers are written with 32-bit writes. The first behavior causes >> zero extension into the upper 32-bits. The second behavior splits a >> 64-bit register into "normal" 32-bit register pairs. >> >> On some passes of ThunderX, >> the following registers incorrectly zero extended when they should >> instead behave as normal 32-bit register pairs: >> >> SMMU()_(S)GFAR >> SMMU()_NSGFAR >> SMMU()_CB()_TTBR0 >> SMMU()_CB()_TTBR1 >> SMMU()_CB()_FAR >> >> Signed-off-by: Tirumalesh Chalamarla >> --- >> drivers/iommu/arm-smmu.c | 51 ++++++++++++++++++++++++++++++++++-------------- >> 1 file changed, 36 insertions(+), 15 deletions(-) > > [...] > >> @@ -762,22 +766,39 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain, >> >> /* TTBRs */ >> if (stage1) { >> - reg = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[0]; >> - writel_relaxed(reg, cb_base + ARM_SMMU_CB_TTBR0_LO); >> - reg = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[0] >> 32; >> - reg |= ARM_SMMU_CB_ASID(cfg) << TTBRn_HI_ASID_SHIFT; >> - writel_relaxed(reg, cb_base + ARM_SMMU_CB_TTBR0_HI); >> - >> - reg = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[1]; >> - writel_relaxed(reg, cb_base + ARM_SMMU_CB_TTBR1_LO); >> - reg = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[1] >> 32; >> - reg |= ARM_SMMU_CB_ASID(cfg) << TTBRn_HI_ASID_SHIFT; >> - writel_relaxed(reg, cb_base + ARM_SMMU_CB_TTBR1_HI); >> + if (smmu->options & ARM_SMMU_OPT_64BIT_WRITES_ONLY) { >> + reg64 = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[0]; >> + reg64 |= ((u64) ARM_SMMU_CB_ASID(cfg)) << >> + (TTBRn_HI_ASID_SHIFT + 32); >> + writeq_relaxed(reg, cb_base + ARM_SMMU_CB_TTBR0_LO); >> + >> + reg64 = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[1]; >> + reg64 |= ((u64)ARM_SMMU_CB_ASID(cfg)) << >> + (TTBRn_HI_ASID_SHIFT + 32); >> + writeq_relaxed(reg, cb_base + ARM_SMMU_CB_TTBR1_LO); >> + } else { >> + reg = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[0]; >> + writel_relaxed(reg, cb_base + ARM_SMMU_CB_TTBR0_LO); >> + reg = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[0] >> 32; >> + reg |= ARM_SMMU_CB_ASID(cfg) << TTBRn_HI_ASID_SHIFT; >> + writel_relaxed(reg, cb_base + ARM_SMMU_CB_TTBR0_HI); >> + >> + reg = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[1]; >> + writel_relaxed(reg, cb_base + ARM_SMMU_CB_TTBR1_LO); >> + reg = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[1] >> 32; >> + reg |= ARM_SMMU_CB_ASID(cfg) << TTBRn_HI_ASID_SHIFT; >> + writel_relaxed(reg, cb_base + ARM_SMMU_CB_TTBR1_HI); >> + } > > I'm fine with making this sort of change if you need it, but this is pretty > ugly. Worse, it won't compile for 32-bit ARM. > > How about we add a wrapper around these, say smmu_writeq(...), which can > then either expand to 2x writel_relaxed or 1x writeq_relaxed depending on > CONFIG_64BIT and your erratum workaround? Would we even need a specific erratum workaround then? I don't see an issue with always using writeq on 64-bit, and there's not much we can do for 32-bit if it has no way to write the upper half of the TTBR on this thing. Robin. > > Don't forgot to update the ATOS code too (so you need to write the high word > first). > > Will > _______________________________________________ > iommu mailing list > iommu at lists.linux-foundation.org > https://lists.linuxfoundation.org/mailman/listinfo/iommu >