is some thing like this looks good

+#ifdef CONFIG_64BIT
+#define smmu_writeq(reg64, addr)       writeq_relaxed((reg64), (addr))
+#else
+#define smmu_writeq(reg64, addr)                       \
+               writel_relaxed(((reg64) >> 32), ((addr) + 4));  \
+               writel_relaxed((reg64), (addr))
+
+
 /* Configuration registers */
 #define ARM_SMMU_GR0_sCR0              0x0
 #define sCR0_CLIENTPD                  (1 << 0)
@@ -226,7 +234,7 @@
 #define TTBCR2_SEP_SHIFT               15
 #define TTBCR2_SEP_UPSTREAM            (0x7 << TTBCR2_SEP_SHIFT)

-#define TTBRn_HI_ASID_SHIFT            16
+#define TTBRn_ASID_SHIFT               48

 #define FSR_MULTI                      (1 << 31)
 #define FSR_SS                         (1 << 30)
@@ -719,6 +727,7 @@ static void arm_smmu_init_context_bank(struct 
arm_smmu_domain *smmu_domain,
                                       struct io_pgtable_cfg *pgtbl_cfg)
 {
        u32 reg;
+       u64 reg64;
        bool stage1;
        struct arm_smmu_cfg *cfg = &smmu_domain->cfg;
        struct arm_smmu_device *smmu = smmu_domain->smmu;
@@ -762,22 +771,16 @@ 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);
+               reg64 = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[0];
+               reg64 |= ((u64)ARM_SMMU_CB_ASID(cfg)) << TTBRn_ASID_SHIFT;
+               smmu_writeq(reg64, cb_base + ARM_SMMU_CB_TTBR0_LO);
+
+               reg64 = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[1];
+               reg64 |= ARM_SMMU_CB_ASID(cfg) << TTBRn_ASID_SHIFT;
+               smmu_writeq(reg, cb_base + ARM_SMMU_CB_TTBR1_LO);
        } else {
-               reg = pgtbl_cfg->arm_lpae_s2_cfg.vttbr;
-               writel_relaxed(reg, cb_base + ARM_SMMU_CB_TTBR0_LO);
-               reg = pgtbl_cfg->arm_lpae_s2_cfg.vttbr >> 32;
-               writel_relaxed(reg, cb_base + ARM_SMMU_CB_TTBR0_HI);
+               reg64 = pgtbl_cfg->arm_lpae_s2_cfg.vttbr;
+               smmu_writeq(reg, cb_base + ARM_SMMU_CB_TTBR0_LO);
        }

        /* TTBCR */
@@ -1236,10 +1239,8 @@ static phys_addr_t arm_smmu_iova_to_phys_hard(struct 
iommu_domain *domain,
                u32 reg = iova & ~0xfff;
                writel_relaxed(reg, cb_base + ARM_SMMU_CB_ATS1PR_LO);
        } else {
-               u32 reg = iova & ~0xfff;
-               writel_relaxed(reg, cb_base + ARM_SMMU_CB_ATS1PR_LO);
-               reg = ((u64)iova & ~0xfff) >> 32;
-               writel_relaxed(reg, cb_base + ARM_SMMU_CB_ATS1PR_HI);
+               u64 reg = iova & ~0xfff;
+               smmu_writeq(reg, cb_base + ARM_SMMU_CB_ATS1PR_LO);
        }

        if (readl_poll_timeout_atomic(cb_base + ARM_SMMU_CB_ATSR, tmp,
~

On Jul 30, 2015, at 12:07 PM, Chalamarla, Tirumalesh 
<tchalama...@caviumnetworks.com<mailto:tchalama...@caviumnetworks.com>> wrote:


On Jul 30, 2015, at 11:45 AM, Will Deacon 
<will.dea...@arm.com<mailto:will.dea...@arm.com>> wrote:

Hello,

On Thu, Jul 30, 2015 at 06:55:06PM +0100, 
tchalama...@caviumnetworks.com<mailto:tchalama...@caviumnetworks.com> wrote:
From: Tirumalesh Chalamarla 
<tchalama...@caviumnetworks.com<mailto:tchalama...@caviumnetworks.com>>

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 
<tchalama...@caviumnetworks.com<mailto:tchalama...@caviumnetworks.com>>
---
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?

 i think we don’t even need to restrict it for work around.
we can just use CONFIG_64BIT and smmu_writeq.

if thats fine i will repost the patch.

Thanks,
Tirumalesh.
Don't forgot to update the ATOS code too (so you need to write the high word
first).

Will


_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Reply via email to