Re: [PATCH] iommu/arm-smmu-v2: Workaround for ThunderX errata#27704

[Tirumalesh Chalamarla <tchalamarla-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>]

On 03/02/2016 05:10 AM, Robin Murphy wrote:
> On 24/02/16 21:13, Tirumalesh Chalamarla wrote:
>> Due to Errata#27704 CN88xx SMMUv2,supports  only shared ASID and VMID
>> namespaces; specifically within a given node SMMU0 and SMMU1 share,
>> as does SMMU2 and SMMU3.
>>
>> This patch address these issuee by supplying asid and vmid
>> while calculating ASID and VMID for Thunder SMMUv2.
>>
>> NOTE: resending with commit message fix.
>>
>> changes from V2:
>>     - removed *_base from DT, and replaced with compatible string
>>
>> changes from V1:
>>     - rebased on top of 16 bit VMID patch
>>     - removed redundent options from DT
>>     - insted of transform, DT now supplies starting ASID/VMID
>>
>> Signed-off-by: Tirumalesh Chalamarla <tchalamarla-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>
>> Signed-off-by: Akula Geethasowjanya
>> <Geethasowjanya.Akula-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>
>> ---
>>   .../devicetree/bindings/iommu/arm,smmu.txt         |  1 +
>>   drivers/iommu/arm-smmu.c                           | 48
>> +++++++++++++++++-----
>
> I guess Documentation/arm64/silicon-errata.txt wants updating too?
>
YES, will update it.
>>   2 files changed, 38 insertions(+), 11 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> index 7180745..19fe6f2 100644
>> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> @@ -16,6 +16,7 @@ conditions.
>>                           "arm,mmu-400"
>>                           "arm,mmu-401"
>>                           "arm,mmu-500"
>> +                        "cavium,smmu-v2"
>
> That seems a rather broad name for a compatible - will there definitely
> never be another Cavium implementation of SMMUv2? Even then, if there is
> future respin of CN88xx silicon where the SMMU needs some other
> workaround for some other issue, will that be discoverable from
> SMMU_IDR7 or similar? Even an absurdly over-specific compatible such as
> "cavium,thunderx-cn88xx-pass2-smmu" would be preferable to something
> which later turns out to be insufficient to uniquely identify a specific
> implementation.
>

i don't think there is a SMMUv2 from Cavium again, there will be SMMUv3 
of course.

there will be other flavors like 86xx etc, which has same implementation 
as 88xx so will have same errata. i think this name is more appropriate.

> Otherwise, modulo Will's comments, this looks a lot more reasonable than
> before, thanks.
>
> Robin.
>
>>                     depending on the particular implementation and/or the
>>                     version of the architecture implemented.
>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>> index 247a469..c704f88 100644
>> --- a/drivers/iommu/arm-smmu.c
>> +++ b/drivers/iommu/arm-smmu.c
>> @@ -326,6 +326,12 @@ struct arm_smmu_device {
>>
>>       struct list_head        list;
>>       struct rb_root            masters;
>> +    /*
>> +     *The following fields are specific to Cavium, Thunder
>> +     */
>> +    u32                cavium_smmu_id;
>> +    u32                cavium_id_base;
>> +
>>   };
>>
>>   struct arm_smmu_cfg {
>> @@ -335,8 +341,8 @@ struct arm_smmu_cfg {
>>   };
>>   #define INVALID_IRPTNDX            0xff
>>
>> -#define ARM_SMMU_CB_ASID(cfg)        ((cfg)->cbndx)
>> -#define ARM_SMMU_CB_VMID(cfg)        ((cfg)->cbndx + 1)
>> +#define ARM_SMMU_CB_ASID(smmu, cfg) ((u16)(smmu)->cavium_id_base +
>> (cfg)->cbndx)
>> +#define ARM_SMMU_CB_VMID(smmu, cfg) ((u16)(smmu)->cavium_id_base +
>> (cfg)->cbndx + 1)
>>
>>   enum arm_smmu_domain_stage {
>>       ARM_SMMU_DOMAIN_S1 = 0,
>> @@ -364,6 +370,8 @@ struct arm_smmu_option_prop {
>>       const char *prop;
>>   };
>>
>> +static int cavium_smmu_count;
>> +
>>   static struct arm_smmu_option_prop arm_smmu_options[] = {
>>       { ARM_SMMU_OPT_SECURE_CFG_ACCESS,
>> "calxeda,smmu-secure-config-access" },
>>       { 0, NULL},
>> @@ -575,11 +583,11 @@ static void arm_smmu_tlb_inv_context(void *cookie)
>>
>>       if (stage1) {
>>           base = ARM_SMMU_CB_BASE(smmu) + ARM_SMMU_CB(smmu, cfg->cbndx);
>> -        writel_relaxed(ARM_SMMU_CB_ASID(cfg),
>> +        writel_relaxed(ARM_SMMU_CB_ASID(smmu, cfg),
>>                      base + ARM_SMMU_CB_S1_TLBIASID);
>>       } else {
>>           base = ARM_SMMU_GR0(smmu);
>> -        writel_relaxed(ARM_SMMU_CB_VMID(cfg),
>> +        writel_relaxed(ARM_SMMU_CB_VMID(smmu, cfg),
>>                      base + ARM_SMMU_GR0_TLBIVMID);
>>       }
>>
>> @@ -601,7 +609,7 @@ static void arm_smmu_tlb_inv_range_nosync(unsigned
>> long iova, size_t size,
>>
>>           if (!IS_ENABLED(CONFIG_64BIT) || smmu->version ==
>> ARM_SMMU_V1) {
>>               iova &= ~12UL;
>> -            iova |= ARM_SMMU_CB_ASID(cfg);
>> +            iova |= ARM_SMMU_CB_ASID(smmu, cfg);
>>               do {
>>                   writel_relaxed(iova, reg);
>>                   iova += granule;
>> @@ -609,7 +617,7 @@ static void arm_smmu_tlb_inv_range_nosync(unsigned
>> long iova, size_t size,
>>   #ifdef CONFIG_64BIT
>>           } else {
>>               iova >>= 12;
>> -            iova |= (u64)ARM_SMMU_CB_ASID(cfg) << 48;
>> +            iova |= (u64)ARM_SMMU_CB_ASID(smmu, cfg) << 48;
>>               do {
>>                   writeq_relaxed(iova, reg);
>>                   iova += granule >> 12;
>> @@ -629,7 +637,7 @@ static void arm_smmu_tlb_inv_range_nosync(unsigned
>> long iova, size_t size,
>>   #endif
>>       } else {
>>           reg = ARM_SMMU_GR0(smmu) + ARM_SMMU_GR0_TLBIVMID;
>> -        writel_relaxed(ARM_SMMU_CB_VMID(cfg), reg);
>> +        writel_relaxed(ARM_SMMU_CB_VMID(smmu, cfg), reg);
>>       }
>>   }
>>
>> @@ -738,7 +746,7 @@ static void arm_smmu_init_context_bank(struct
>> arm_smmu_domain *smmu_domain,
>>   #endif
>>           /* if 16bit VMID supported set VMID in CBA2R */
>>           if (smmu->features & ARM_SMMU_FEAT_VMID16)
>> -            reg |= ARM_SMMU_CB_VMID(cfg) << CBA2R_VMID_SHIFT;
>> +            reg |= ARM_SMMU_CB_VMID(smmu, cfg) << CBA2R_VMID_SHIFT;
>>
>>           writel_relaxed(reg, gr1_base + ARM_SMMU_GR1_CBA2R(cfg->cbndx));
>>       }
>> @@ -757,7 +765,7 @@ static void arm_smmu_init_context_bank(struct
>> arm_smmu_domain *smmu_domain,
>>               (CBAR_S1_MEMATTR_WB << CBAR_S1_MEMATTR_SHIFT);
>>       } else if (!(smmu->features & ARM_SMMU_FEAT_VMID16)) {
>>           /*16 bit VMID is not supported set 8 bit VMID here */
>> -        reg |= ARM_SMMU_CB_VMID(cfg) << CBAR_VMID_SHIFT;
>> +        reg |= ARM_SMMU_CB_VMID(smmu, cfg) << CBAR_VMID_SHIFT;
>>       }
>>       writel_relaxed(reg, gr1_base + ARM_SMMU_GR1_CBAR(cfg->cbndx));
>>
>> @@ -765,11 +773,11 @@ static void arm_smmu_init_context_bank(struct
>> arm_smmu_domain *smmu_domain,
>>       if (stage1) {
>>           reg64 = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[0];
>>
>> -        reg64 |= ((u64)ARM_SMMU_CB_ASID(cfg)) << TTBRn_ASID_SHIFT;
>> +        reg64 |= ((u64)ARM_SMMU_CB_ASID(smmu, cfg)) << TTBRn_ASID_SHIFT;
>>           smmu_writeq(reg64, cb_base + ARM_SMMU_CB_TTBR0);
>>
>>           reg64 = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[1];
>> -        reg64 |= ((u64)ARM_SMMU_CB_ASID(cfg)) << TTBRn_ASID_SHIFT;
>> +        reg64 |= ((u64)ARM_SMMU_CB_ASID(smmu, cfg)) << TTBRn_ASID_SHIFT;
>>           smmu_writeq(reg64, cb_base + ARM_SMMU_CB_TTBR1);
>>       } else {
>>           reg64 = pgtbl_cfg->arm_lpae_s2_cfg.vttbr;
>> @@ -1717,6 +1725,7 @@ static const struct of_device_id
>> arm_smmu_of_match[] = {
>>       { .compatible = "arm,mmu-400", .data = (void *)ARM_SMMU_V1 },
>>       { .compatible = "arm,mmu-401", .data = (void *)ARM_SMMU_V1 },
>>       { .compatible = "arm,mmu-500", .data = (void *)ARM_SMMU_V2 },
>> +    { .compatible = "cavium,smmu-v2", .data = (void *)ARM_SMMU_V2 },
>>       { },
>>   };
>>   MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
>> @@ -1827,6 +1836,23 @@ static int arm_smmu_device_dt_probe(struct
>> platform_device *pdev)
>>           }
>>       }
>>
>> +    /*
>> +     * Due to Errata#27704 CN88xx SMMUv2,supports  only shared ASID
>> and VMID
>> +     * namespaces; specifically within a given node SMMU0 and SMMU1
>> share,
>> +     * as does SMMU2 and SMMU3. see if this is a Cavium SMMU, if so
>> +     * set asid and vmid base such that each SMMU gets unique
>> +     * asid/vmid space.
>> +     */
>> +    if (!strcasecmp(of_id->compatible, "cavium,smmu-v2")) {
>> +        /* VMID16 must be present on Cavium SMMUv2*/
>> +        if (!(smmu->features & ARM_SMMU_FEAT_VMID16))
>> +            goto out_free_irqs;
>> +        smmu->cavium_smmu_id = cavium_smmu_count;
>> +        cavium_smmu_count++;
>> +        smmu->cavium_id_base =
>> +            (smmu->cavium_smmu_id * ARM_SMMU_MAX_CBS);
>> +    }
>> +
>>       INIT_LIST_HEAD(&smmu->list);
>>       spin_lock(&arm_smmu_devices_lock);
>>       list_add(&smmu->list, &arm_smmu_devices);
>>
>