On 2/9/16, 3:52 AM, "Robin Murphy" <robin.mur...@arm.com> wrote:
>On 05/02/16 18:47, tchalama...@caviumnetworks.com wrote:
>> From: Tirumalesh Chalamarla <tchalama...@caviumnetworks.com>
>>
>> An issue exists whereby the Linux kernel requires that ASIDs are a
>> unique namespace per SMMU.
>
>I too tend to consider the SMMUv2 architecture an "issue", but we should
>probably still call it by its name ;)
>
>> That ASID-local choice conflicts with the
>> CN88xx SMMU, where only shared ASID namespaces are supported;
>> specifically within a given node SMMU0 and SMMU1 share, as does SMMU2
>> and SMMU3. CN88xx SMMU also requires global VMIDs.
>
>"Global" meaning unique across all 4 SMMUs? Or just unique across each
>pair as per ASIDs?...
>
>> This patch tries to address these issuee by supplying asid and vmid
>> transformations from devicetree.
>>
>> Signed-off-by: Akula Geethasowjanya <geethasowjanya.ak...@caviumnetworks.com>
>> Signed-off-by: Tirumalesh Chalamarla <tchalama...@caviumnetworks.com>
>> ---
>> .../devicetree/bindings/iommu/arm,smmu.txt | 16 ++++++
>> drivers/iommu/arm-smmu.c | 59
>> ++++++++++++++++++----
>> 2 files changed, 65 insertions(+), 10 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> index 7180745..eef06d0 100644
>> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> @@ -55,6 +55,22 @@ conditions.
>> aliases of secure registers have to be used during
>> SMMU configuration.
>>
>> +- thunderx,smmu-asid-transform : Enable proper handling for buggy
>> + implementations that require special transformations
>> + for smmu asid. if this property exists asid-transform
>> + property must be present.
>> +
>> +- thunderx,smmu-vmid-transform : Enable proper handling for buggy
>> + implementations that require special transformations
>> + for smmu vmid. if this property exists vmid-transform
>> + property must be present.
>> +
>> +- asid-transform : Transform mask that needs to be applied to asid.
>> + This property has to be specified as '/bits/ 8' value.
>> +
>> +- vmid-transform : Transform mask that needs to be applied to vmid.
>> + This property has to be specified as '/bits/ 8' value.
>> +
>
>...because I've seen a ThunderX boot log indicating 4x128 context banks,
>and I don't see how applying an 8-bit mask to an 8-bit value is going to
>produce anything _globally_ unique in that case.
>
>Regardless, I'm not sure this is an accurate description of the hardware
>as DT should be - this is more implying that each SMMU _requires_ a
>particular set of ASIDs/VMIDs to be used, which is not the same thing
>that the commit message and other comments say.
I am rewriting this patch to include additional patches to support 16bit VMID
and ASID. Will rebase this patch on top of it.
Though for 128 bit 7 bits are sufficient I thought it is still good idea to
have this on top of 16bit VMID and ASID.
>
>Robin.
>
>> Example:
>>
>> smmu {
>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>> index 59ee4b8..8047834 100644
>> --- a/drivers/iommu/arm-smmu.c
>> +++ b/drivers/iommu/arm-smmu.c
>> @@ -300,8 +300,12 @@ struct arm_smmu_device {
>> u32 features;
>>
>> #define ARM_SMMU_OPT_SECURE_CFG_ACCESS (1 << 0)
>> +#define ARM_SMMU_OPT_TRANSFORM_ASID (1 << 1)
>> +#define ARM_SMMU_OPT_TRANSFORM_VMID (1 << 2)
>> u32 options;
>> enum arm_smmu_arch_version version;
>> + u8 asid_transform;
>> + u8 vmid_transform;
>>
>> u32 num_context_banks;
>> u32 num_s2_context_banks;
>> @@ -330,8 +334,25 @@ 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)
>> +/*
>> + * ThunderX has a unique requirement because of ERRATA#27704
>> + * An issue exists whereby the Linux kernel requires that ASIDs are a
>> + * unique namespace per SMMU. That ASID-local choice conflicts with the
>> + * CN88xx SMMU, where only shared ASID namespaces are supported;
>> + * specifically within a given node SMMU0 and SMMU1 share, as does SMMU2
>> + * and SMMU3.
>> + * CN88xx SMMU also requires global VMIDs.
>> + */
>> +
>> +#define ARM_SMMU_CB_ASID(smmu, cfg) \
>> + (((smmu)->options & ARM_SMMU_OPT_TRANSFORM_ASID) \
>> + ? ((cfg)->cbndx | (smmu)->asid_transform) \
>> + : ((cfg)->cbndx))
>> +
>> +#define ARM_SMMU_CB_VMID(smmu, cfg) \
>> + (((smmu)->options & ARM_SMMU_OPT_TRANSFORM_VMID) \
>> + ? (((cfg)->cbndx + 1) | (smmu)->vmid_transform) \
>> + : ((cfg)->cbndx + 1))
>>
>> enum arm_smmu_domain_stage {
>> ARM_SMMU_DOMAIN_S1 = 0,
>> @@ -361,6 +382,8 @@ struct arm_smmu_option_prop {
>>
>> static struct arm_smmu_option_prop arm_smmu_options[] = {
>> { ARM_SMMU_OPT_SECURE_CFG_ACCESS, "calxeda,smmu-secure-config-access" },
>> + { ARM_SMMU_OPT_TRANSFORM_ASID, "thunderx,smmu-asid-transform" },
>> + { ARM_SMMU_OPT_TRANSFORM_VMID, "thunderx,smmu-vmid-transform" },
>> { 0, NULL},
>> };
>>
>> @@ -570,11 +593,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);
>> }
>>
>> @@ -596,7 +619,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;
>> @@ -604,7 +627,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;
>> @@ -624,7 +647,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);
>> }
>> }
>>
>> @@ -752,7 +775,7 @@ static void arm_smmu_init_context_bank(struct
>> arm_smmu_domain *smmu_domain,
>> reg |= (CBAR_S1_BPSHCFG_NSH << CBAR_S1_BPSHCFG_SHIFT) |
>> (CBAR_S1_MEMATTR_WB << CBAR_S1_MEMATTR_SHIFT);
>> } else {
>> - 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));
>>
>> @@ -760,11 +783,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;
>> @@ -1793,6 +1816,22 @@ static int arm_smmu_device_dt_probe(struct
>> platform_device *pdev)
>>
>> parse_driver_options(smmu);
>>
>> + if (smmu->options & ARM_SMMU_OPT_TRANSFORM_ASID) {
>> + if (of_property_read_u8(dev->of_node, "#asid-transform",
>> + &smmu->asid_transform)) {
>> + dev_err(dev, "missing #asid-transform property\n");
>> + return -ENODEV;
>> + }
>> + }
>> +
>> + if (smmu->options & ARM_SMMU_OPT_TRANSFORM_VMID) {
>> + if (of_property_read_u8(dev->of_node, "#vmid-transform",
>> + &smmu->vmid_transform)) {
>> + dev_err(dev, "missing #vmid-transform property\n");
>> + return -ENODEV;
>> + }
>> + }
>> +
>> if (smmu->version > ARM_SMMU_V1 &&
>> smmu->num_context_banks != smmu->num_context_irqs) {
>> dev_err(dev,
>>
>
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
