On 30/07/15 19:45, Will Deacon wrote:
Hello,

On Thu, Jul 30, 2015 at 06:55:06PM +0100, [email protected] wrote:
From: Tirumalesh Chalamarla <[email protected]>

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 <[email protected]>
---
  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
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu


_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Reply via email to