iommu/arm-smmu: Add driver for ARM SMMUv3 devices

drivers/iommu/arm-smmu-v3.c new file +17 -0

+			return -ETIMEDOUT;
+
+		if (wfe) {
+			wfe();
+		} else {
+			cpu_relax();
+			udelay(1);
+		}
+	}
+
+	return 0;
+}
+
+static void queue_write(__le64 *dst, u64 *src, size_t n_dwords)
+{
+	int i;
+

drivers/iommu/arm-smmu-v3.c new file +6 -0

+	queue_inc_prod(q);
+	return 0;
+}
+
+
+/* High-level queue accessors */

drivers/iommu/arm-smmu-v3.c new file +12 -0

+		break;
+	default:
+		return -ENOENT;
+	}
+
+	return 0;
+}
 
+
+static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
+				       struct io_pgtable_cfg *pgtbl_cfg)
+{
+	int ret;

(quoted diff) +98 -0

+	if (!cfg->cdptr) {
+		dev_warn(smmu->dev, "failed to allocate context descriptor\n");
+		goto out_free_asid;
+	}
+
+	cfg->cd.asid	= asid;
+	cfg->cd.ttbr	= pgtbl_cfg->arm_lpae_s1_cfg.ttbr[0];
+	cfg->cd.tcr	= pgtbl_cfg->arm_lpae_s1_cfg.tcr;
+	cfg->cd.mair	= pgtbl_cfg->arm_lpae_s1_cfg.mair[0];
+	return 0;
+
+out_free_asid:
+	arm_smmu_bitmap_free(smmu->asid_map, asid);
+	return ret;
+}
+
 
 
+
+static int arm_smmu_add_device(struct device *dev)
+{
+	int i, ret;
+	u32 sid, *sids;
+	struct pci_dev *pdev;
+	struct iommu_group *group;
+	struct arm_smmu_group *smmu_group;
+
+	/* We only support PCI, for now */
+	if (!dev_is_pci(dev))
+		return -ENODEV;
+
+	pdev = to_pci_dev(dev);
+	group = iommu_group_get_for_dev(dev);
+	if (IS_ERR(group))
+		return PTR_ERR(group);
+
+	smmu_group = iommu_group_get_iommudata(group);
+	if (!smmu_group) {
+		struct arm_smmu_device *smmu = arm_smmu_get_for_pci_dev(pdev);
+		if (!smmu) {
+			ret = -ENOENT;
+			goto out_put_group;
+		}
+
+		smmu_group = kzalloc(sizeof(*smmu_group), GFP_KERNEL);
+		if (!smmu_group) {
+			ret = -ENOMEM;
+			goto out_put_group;
+		}
+
+		smmu_group->ste.valid	= true;
+		smmu_group->smmu	= smmu;
+		iommu_group_set_iommudata(group, smmu_group,
+					  __arm_smmu_release_pci_iommudata);
+	}
+
+	/* Assume SID == RID until firmware tells us otherwise */
+	pci_for_each_dma_alias(pdev, __arm_smmu_get_pci_sid, &sid);
+	for (i = 0; i < smmu_group->num_sids; ++i) {
+		/* If we already know about this SID, then we're done */
+		if (smmu_group->sids[i] == sid)
+			return 0;
+	}
+
+	/* Check the SID is in range of the SMMU and our stream table */
+	if (!arm_smmu_sid_in_range(smmu_group->smmu, sid)) {
+		ret = -ERANGE;
+		goto out_put_group;
+	}
+
+	/* Resize the SID array for the group */
+	smmu_group->num_sids++;
+	sids = krealloc(smmu_group->sids, smmu_group->num_sids * sizeof(*sids),
+			GFP_KERNEL);
+	if (!sids) {
+		smmu_group->num_sids--;
+		ret = -ENOMEM;
+		goto out_put_group;
+	}
+
+	/* Add the new SID */
+	sids[smmu_group->num_sids - 1] = sid;
+	smmu_group->sids = sids;
+	return 0;
+
+out_put_group:
+	iommu_group_put(group);
+	return ret;
+}
 
+static int arm_smmu_alloc_l2_strtab(struct arm_smmu_device *smmu)
+{
+	int ret;
+	unsigned int i;
+	struct arm_smmu_strtab_cfg *cfg = &smmu->strtab_cfg;
+	size_t size = sizeof(*cfg->l1_desc) * cfg->num_l1_descs;
+	void *strtab = smmu->strtab_cfg.strtab;
+
+	cfg->l1_desc = devm_kzalloc(smmu->dev, size, GFP_KERNEL);
+	if (!cfg->l1_desc) {
+		dev_err(smmu->dev, "failed to allocate l1 stream table desc\n");

drivers/iommu/arm-smmu-v3.c new file +18 -0

+		if (!desc->l2ptr) {
+			dev_err(smmu->dev,
+				"failed to allocate l2 stream table %u\n", i);
+			ret = -ENOMEM;
+			goto out_free_l2;
+		}
+
+		arm_smmu_init_bypass_stes(desc->l2ptr, 1 << STRTAB_SPLIT);
+		arm_smmu_write_strtab_l1_desc(strtab, desc);
+		strtab += STRTAB_STE_DWORDS;
+	}
+
+	return 0;
+
+out_free_l2:
+	arm_smmu_free_l2_strtab(smmu);
+	return ret;
+}

drivers/iommu/arm-smmu-v3.c new file +75 -0

+	if (!strtab) {
+		dev_err(smmu->dev, "failed to allocate l1 stream table\n");
+		return -ENOMEM;
+	}
+	smmu->strtab_cfg.strtab = strtab;
+
+	reg  = smmu->strtab_cfg.strtab_dma &
+	       STRTAB_BASE_ADDR_MASK << STRTAB_BASE_ADDR_SHIFT;
+	reg |= STRTAB_BASE_RA;
+	smmu->strtab_cfg.strtab_base = reg;
+
+	if (smmu->features & ARM_SMMU_FEAT_2_LVL_STRTAB) {
+		size = STRTAB_L1_SZ_SHIFT - (ilog2(STRTAB_L1_DESC_DWORDS) + 3);
+		smmu->strtab_cfg.num_l1_descs = 1 << size;
+		size += STRTAB_SPLIT;
+		reg = STRTAB_BASE_CFG_FMT_2LVL;
+
+		ret = arm_smmu_alloc_l2_strtab(smmu);
+		if (ret)
+			goto out_free_l1;
+	} else {
+		size = STRTAB_L1_SZ_SHIFT - (ilog2(STRTAB_STE_DWORDS) + 3);
+		smmu->strtab_cfg.num_l1_descs = 0;
+		reg = STRTAB_BASE_CFG_FMT_LINEAR;
+		arm_smmu_init_bypass_stes(strtab, 1 << size);
+	}
+
+	if (size < smmu->sid_bits)
+		dev_warn(smmu->dev, "%s strtab only covers %u/%u bits of SID\n",
+			 smmu->features & ARM_SMMU_FEAT_2_LVL_STRTAB ?
+			 "2-level" : "linear",
+			 size, smmu->sid_bits);
+
+	reg |= (size & STRTAB_BASE_CFG_LOG2SIZE_MASK)
+		<< STRTAB_BASE_CFG_LOG2SIZE_SHIFT;
+	reg |= (STRTAB_SPLIT & STRTAB_BASE_CFG_SPLIT_MASK)
+		<< STRTAB_BASE_CFG_SPLIT_SHIFT;
+	smmu->strtab_cfg.strtab_base_cfg = reg;
+
+	/* Allocate the first VMID for stage-2 bypass STEs */
+	set_bit(0, smmu->vmid_map);
+	return 0;
+
+out_free_l1:
+	dma_free_coherent(smmu->dev, 1 << STRTAB_L1_SZ_SHIFT, strtab,
+			  smmu->strtab_cfg.strtab_dma);
+	return ret;
+}
+
+static void arm_smmu_free_strtab(struct arm_smmu_device *smmu)
+{
+	struct arm_smmu_strtab_cfg *cfg = &smmu->strtab_cfg;
+
+	arm_smmu_free_l2_strtab(smmu);
+	dma_free_coherent(smmu->dev, 1 << STRTAB_L1_SZ_SHIFT, cfg->strtab,
+			  cfg->strtab_dma);
+}
+
 
+
+static int arm_smmu_device_reset(struct arm_smmu_device *smmu)
+{
+	int ret;
+	u32 reg, enables;
+	struct arm_smmu_cmdq_ent cmd;
+
+	/* Clear CR0 and sync (disables SMMU and queue processing) */
+	reg = readl_relaxed(smmu->base + ARM_SMMU_CR0);
+	if (reg & CR0_SMMUEN)
+		dev_warn(smmu->dev, "SMMU currently enabled! Resetting...\n");
+
+	ret = arm_smmu_device_disable(smmu);
+	if (ret)
+		return ret;
+
+	/* CR1 (table and queue memory attributes) */

drivers/iommu/arm-smmu-v3.c new file +19 -0

+	writel_relaxed(reg, smmu->base + ARM_SMMU_CR2);
+
+	/* Stream table */
+	writeq_relaxed(smmu->strtab_cfg.strtab_base,
+		       smmu->base + ARM_SMMU_STRTAB_BASE);
+	writel_relaxed(smmu->strtab_cfg.strtab_base_cfg,
+		       smmu->base + ARM_SMMU_STRTAB_BASE_CFG);
+
+	/* Command queue */
+	writeq_relaxed(smmu->cmdq.q.q_base, smmu->base + ARM_SMMU_CMDQ_BASE);
+	writel_relaxed(smmu->cmdq.q.prod, smmu->base + ARM_SMMU_CMDQ_PROD);
+	writel_relaxed(smmu->cmdq.q.cons, smmu->base + ARM_SMMU_CMDQ_CONS);
+
+	enables = CR0_CMDQEN;
+	ret = arm_smmu_write_reg_sync(smmu, enables, ARM_SMMU_CR0,
+				      ARM_SMMU_CR0ACK);
+	if (ret) {
+		dev_err(smmu->dev, "failed to enable command queue\n");
+		return ret;

drivers/iommu/arm-smmu-v3.c new file +55 -0

+	arm_smmu_cmdq_issue_cmd(smmu, &cmd);
+	cmd.opcode = CMDQ_OP_TLBI_NSNH_ALL;
+	arm_smmu_cmdq_issue_cmd(smmu, &cmd);
+	cmd.opcode = CMDQ_OP_CMD_SYNC;
+	arm_smmu_cmdq_issue_cmd(smmu, &cmd);
+
+	/* Event queue */
+	writeq_relaxed(smmu->evtq.q.q_base, smmu->base + ARM_SMMU_EVTQ_BASE);
+	writel_relaxed(smmu->evtq.q.prod, smmu->base + ARM_SMMU_EVTQ_PROD);
+	writel_relaxed(smmu->evtq.q.cons, smmu->base + ARM_SMMU_EVTQ_CONS);
+
+	enables |= CR0_EVTQEN;
+	ret = arm_smmu_write_reg_sync(smmu, enables, ARM_SMMU_CR0,
+				      ARM_SMMU_CR0ACK);
+	if (ret) {
+		dev_err(smmu->dev, "failed to enable event queue\n");
+		return ret;
+	}
+
+	/* PRI queue */
+	if (smmu->features & ARM_SMMU_FEAT_PRI) {
+		writeq_relaxed(smmu->priq.q.q_base,
+			       smmu->base + ARM_SMMU_PRIQ_BASE);
+		writel_relaxed(smmu->priq.q.prod,
+			       smmu->base + ARM_SMMU_PRIQ_PROD);
+		writel_relaxed(smmu->priq.q.cons,
+			       smmu->base + ARM_SMMU_PRIQ_CONS);
+
+		enables |= CR0_PRIQEN;
+		ret = arm_smmu_write_reg_sync(smmu, enables, ARM_SMMU_CR0,
+					      ARM_SMMU_CR0ACK);
+		if (ret) {
+			dev_err(smmu->dev, "failed to enable PRI queue\n");
+			return ret;
+		}
+	}
+
+	ret = arm_smmu_setup_irqs(smmu);
+	if (ret) {
+		dev_err(smmu->dev, "failed to setup irqs\n");
+		return ret;
+	}
+
+	/* Enable the SMMU interface */
+	enables |= CR0_SMMUEN;
+	ret = arm_smmu_write_reg_sync(smmu, enables, ARM_SMMU_CR0,
+				      ARM_SMMU_CR0ACK);
+	if (ret) {
+		dev_err(smmu->dev, "failed to enable SMMU interface\n");
+		return ret;
+	}
+
+	return 0;
+}
+

drivers/iommu/arm-smmu-v3.c line 662

Quoted 8 lines

drivers/iommu/arm-smmu-v3.c new file +7 -0

+
+static int queue_poll_cons(struct arm_smmu_queue *q, u32 until, bool wfe)
+{
+     ktime_t timeout = ktime_add_us(ktime_get(), ARM_SMMU_POLL_TIMEOUT_US);
+
+     while (queue_sync_cons(q), __queue_cons_before(q, until)) {
+             if (ktime_compare(ktime_get(), timeout) > 0)

Is it good to limit hardware behavior? May be wait for ever will be better. If SMMU can not consume queue items under normal condition, the SMMU hardware is broken, will lead software system to be crashed later.

Quoted 22 lines

drivers/iommu/arm-smmu-v3.c new file +22 -0

+                     return -ETIMEDOUT;
+
+             if (wfe) {
+                     wfe();
+             } else {
+                     cpu_relax();
+                     udelay(1);
+             }
+     }
+
+     return 0;
+}
+
+static void queue_write(__le64 *dst, u64 *src, size_t n_dwords)
+{
+     int i;
+
+     for (i = 0; i < n_dwords; ++i)
+             *dst++ = cpu_to_le64(*src++);
+}
+
+static int queue_insert_raw(struct arm_smmu_queue *q, u64 *ent)

drivers/iommu/arm-smmu-v3.c line 689

Quoted 5 lines

drivers/iommu/arm-smmu-v3.c new file +5 -0

+{
+     if (queue_full(q))
+             return -ENOSPC;
+
+     queue_write(Q_ENT(q, q->prod), ent, q->ent_dwords);

A dmb or dsb maybe needed. We must insure all data written are completed, then notify hardware to consume.

Quoted 13 lines

drivers/iommu/arm-smmu-v3.c new file +13 -0

+     queue_inc_prod(q);
+     return 0;
+}
+
+
+/* High-level queue accessors */
+static int arm_smmu_cmdq_build_cmd(u64 *cmd, struct arm_smmu_cmdq_ent *ent)
+{
+     memset(cmd, 0, CMDQ_ENT_DWORDS << 3);
+     cmd[0] |= (ent->opcode & CMDQ_0_OP_MASK) << CMDQ_0_OP_SHIFT;
+
+     switch (ent->opcode) {
+     case CMDQ_OP_TLBI_EL2_ALL:

drivers/iommu/arm-smmu-v3.c line 769

Quoted 2 lines

drivers/iommu/arm-smmu-v3.c new file +2 -0

+     case CMDQ_OP_CMD_SYNC:
+             cmd[0] |= CMDQ_SYNC_0_CS_SEV;

We can not always set SIG_SEV, actually it should base upon SMMU_IDR0.SEV(smmu->features & ARM_SMMU_FEAT_SEV)

Quoted 20 lines

drivers/iommu/arm-smmu-v3.c new file +19 -0

+             break;
+     default:
+             return -ENOENT;
+     }
+
+     return 0;
+}
 
+
+static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
+                                    struct io_pgtable_cfg *pgtbl_cfg)
+{
+     int ret;
+     u16 asid;
+     struct arm_smmu_device *smmu = smmu_domain->smmu;
+     struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg;
+
+     asid = arm_smmu_bitmap_alloc(smmu->asid_map, smmu->asid_bits);
+     if (IS_ERR_VALUE(asid))
+             return asid;

drivers/iommu/arm-smmu-v3.c line 1379

Quoted 3 lines

drivers/iommu/arm-smmu-v3.c new file +3 -0

+
+     cfg->cdptr = dma_zalloc_coherent(smmu->dev, CTXDESC_CD_DWORDS << 3,
+                                      &cfg->cdptr_dma, GFP_KERNEL);

Why use dma_zalloc_coherent? iova is coverted from PA by calling phys_to_dma. I afraid PA and iova maybe not equal. In fact, the mapping between iova and PA is rely on SMMU driver itself. Why not use virt_to_phys to get PA, SMMU hardware actually require PA.

Quoted 20 lines

drivers/iommu/arm-smmu-v3.c new file +20 -0

+static int arm_smmu_alloc_l2_strtab(struct arm_smmu_device *smmu)
+{
+     int ret;
+     unsigned int i;
+     struct arm_smmu_strtab_cfg *cfg = &smmu->strtab_cfg;
+     size_t size = sizeof(*cfg->l1_desc) * cfg->num_l1_descs;
+     void *strtab = smmu->strtab_cfg.strtab;
+
+     cfg->l1_desc = devm_kzalloc(smmu->dev, size, GFP_KERNEL);
+     if (!cfg->l1_desc) {
+             dev_err(smmu->dev, "failed to allocate l1 stream table desc\n");
+             return -ENOMEM;
+     }
+
+     size = 1 << (STRTAB_SPLIT + ilog2(STRTAB_STE_DWORDS) + 3);
+     for (i = 0; i < cfg->num_l1_descs; ++i) {
+             struct arm_smmu_strtab_l1_desc *desc = &cfg->l1_desc[i];
+
+             desc->span = STRTAB_SPLIT + 1;
+             desc->l2ptr = dma_zalloc_coherent(smmu->dev, size,

Quoted 1 line

+ &desc->l2ptr_dma, GFP_KERNEL);

No, no, please don't allocate all Lv2 table memory, we should dynamic allocation when needed. Otherwise we can not save memory relative to one level table. And cfg->l1_desc seems not necessary. Before create mapping for a specified StreamID, we read corresponding Lv1 table entry, if L2Ptr is NULL, then we build Lv2 table. Otherwise, means this Lv2 table have already been built, because a Lv2 table is shared by a group of StreamIDs.

Quoted 25 lines

drivers/iommu/arm-smmu-v3.c new file +25 -0

+             if (!desc->l2ptr) {
+                     dev_err(smmu->dev,
+                             "failed to allocate l2 stream table %u\n", i);
+                     ret = -ENOMEM;
+                     goto out_free_l2;
+             }
+
+             arm_smmu_init_bypass_stes(desc->l2ptr, 1 << STRTAB_SPLIT);
+             arm_smmu_write_strtab_l1_desc(strtab, desc);
+             strtab += STRTAB_STE_DWORDS;
+     }
+
+     return 0;
+
+out_free_l2:
+     arm_smmu_free_l2_strtab(smmu);
+     return ret;
+}
+
+static int arm_smmu_init_strtab(struct arm_smmu_device *smmu)
+{
+     void *strtab;
+     u64 reg;
+     u32 size;
+     int ret = 0;

drivers/iommu/arm-smmu-v3.c line 2010

Quoted 3 lines

drivers/iommu/arm-smmu-v3.c new file +3 -0

+
+     strtab = dma_zalloc_coherent(smmu->dev, 1 << STRTAB_L1_SZ_SHIFT,
+                                  &smmu->strtab_cfg.strtab_dma, GFP_KERNEL);

As above, when Lv2 tables are dynamic allocation, we can create Lv1 table base on SMMU_IDR1.SIDSIZE and support non-pic devices. Oh, if SIDSIZE is too large, like 32. Maybe we should use 64K size Lv2 table. But we can only use PAGE_SIZE first, for lazy.

Quoted 80 lines

drivers/iommu/arm-smmu-v3.c new file +79 -0

+     if (!strtab) {
+             dev_err(smmu->dev, "failed to allocate l1 stream table\n");
+             return -ENOMEM;
+     }
+     smmu->strtab_cfg.strtab = strtab;
+
+     reg  = smmu->strtab_cfg.strtab_dma &
+            STRTAB_BASE_ADDR_MASK << STRTAB_BASE_ADDR_SHIFT;
+     reg |= STRTAB_BASE_RA;
+     smmu->strtab_cfg.strtab_base = reg;
+
+     if (smmu->features & ARM_SMMU_FEAT_2_LVL_STRTAB) {
+             size = STRTAB_L1_SZ_SHIFT - (ilog2(STRTAB_L1_DESC_DWORDS) + 3);
+             smmu->strtab_cfg.num_l1_descs = 1 << size;
+             size += STRTAB_SPLIT;
+             reg = STRTAB_BASE_CFG_FMT_2LVL;
+
+             ret = arm_smmu_alloc_l2_strtab(smmu);
+             if (ret)
+                     goto out_free_l1;
+     } else {
+             size = STRTAB_L1_SZ_SHIFT - (ilog2(STRTAB_STE_DWORDS) + 3);
+             smmu->strtab_cfg.num_l1_descs = 0;
+             reg = STRTAB_BASE_CFG_FMT_LINEAR;
+             arm_smmu_init_bypass_stes(strtab, 1 << size);
+     }
+
+     if (size < smmu->sid_bits)
+             dev_warn(smmu->dev, "%s strtab only covers %u/%u bits of SID\n",
+                      smmu->features & ARM_SMMU_FEAT_2_LVL_STRTAB ?
+                      "2-level" : "linear",
+                      size, smmu->sid_bits);
+
+     reg |= (size & STRTAB_BASE_CFG_LOG2SIZE_MASK)
+             << STRTAB_BASE_CFG_LOG2SIZE_SHIFT;
+     reg |= (STRTAB_SPLIT & STRTAB_BASE_CFG_SPLIT_MASK)
+             << STRTAB_BASE_CFG_SPLIT_SHIFT;
+     smmu->strtab_cfg.strtab_base_cfg = reg;
+
+     /* Allocate the first VMID for stage-2 bypass STEs */
+     set_bit(0, smmu->vmid_map);
+     return 0;
+
+out_free_l1:
+     dma_free_coherent(smmu->dev, 1 << STRTAB_L1_SZ_SHIFT, strtab,
+                       smmu->strtab_cfg.strtab_dma);
+     return ret;
+}
+
+static void arm_smmu_free_strtab(struct arm_smmu_device *smmu)
+{
+     struct arm_smmu_strtab_cfg *cfg = &smmu->strtab_cfg;
+
+     arm_smmu_free_l2_strtab(smmu);
+     dma_free_coherent(smmu->dev, 1 << STRTAB_L1_SZ_SHIFT, cfg->strtab,
+                       cfg->strtab_dma);
+}
+
 
+
+static int arm_smmu_device_reset(struct arm_smmu_device *smmu)
+{
+     int ret;
+     u32 reg, enables;
+     struct arm_smmu_cmdq_ent cmd;
+
+     /* Clear CR0 and sync (disables SMMU and queue processing) */
+     reg = readl_relaxed(smmu->base + ARM_SMMU_CR0);
+     if (reg & CR0_SMMUEN)
+             dev_warn(smmu->dev, "SMMU currently enabled! Resetting...\n");
+
+     ret = arm_smmu_device_disable(smmu);
+     if (ret)
+             return ret;
+
+     /* CR1 (table and queue memory attributes) */
+     reg = (CR1_SH_ISH << CR1_TABLE_SH_SHIFT) |
+           (CR1_CACHE_WB << CR1_TABLE_OC_SHIFT) |
+           (CR1_CACHE_WB << CR1_TABLE_IC_SHIFT) |
+           (CR1_SH_ISH << CR1_QUEUE_SH_SHIFT) |

drivers/iommu/arm-smmu-v3.c line 2211

Quoted 6 lines

drivers/iommu/arm-smmu-v3.c new file +6 -0

+           (CR1_CACHE_WB << CR1_QUEUE_OC_SHIFT) |
+           (CR1_CACHE_WB << CR1_QUEUE_IC_SHIFT);
+     writel_relaxed(reg, smmu->base + ARM_SMMU_CR1);
+
+     /* CR2 (random crap) */
+     reg = CR2_PTM | CR2_RECINVMID | CR2_E2H;

Do we need to explicitly set CR2_E2H? Linux only run at EL1.

Quoted 24 lines

drivers/iommu/arm-smmu-v3.c new file +24 -0

+     writel_relaxed(reg, smmu->base + ARM_SMMU_CR2);
+
+     /* Stream table */
+     writeq_relaxed(smmu->strtab_cfg.strtab_base,
+                    smmu->base + ARM_SMMU_STRTAB_BASE);
+     writel_relaxed(smmu->strtab_cfg.strtab_base_cfg,
+                    smmu->base + ARM_SMMU_STRTAB_BASE_CFG);
+
+     /* Command queue */
+     writeq_relaxed(smmu->cmdq.q.q_base, smmu->base + ARM_SMMU_CMDQ_BASE);
+     writel_relaxed(smmu->cmdq.q.prod, smmu->base + ARM_SMMU_CMDQ_PROD);
+     writel_relaxed(smmu->cmdq.q.cons, smmu->base + ARM_SMMU_CMDQ_CONS);
+
+     enables = CR0_CMDQEN;
+     ret = arm_smmu_write_reg_sync(smmu, enables, ARM_SMMU_CR0,
+                                   ARM_SMMU_CR0ACK);
+     if (ret) {
+             dev_err(smmu->dev, "failed to enable command queue\n");
+             return ret;
+     }
+
+     /* Invalidate any cached configuration */
+     cmd.opcode = CMDQ_OP_CFGI_ALL;
+     arm_smmu_cmdq_issue_cmd(smmu, &cmd);

drivers/iommu/arm-smmu-v3.c line 2240

Quoted 5 lines

drivers/iommu/arm-smmu-v3.c new file +5 -0

+     cmd.opcode = CMDQ_OP_CMD_SYNC;
+     arm_smmu_cmdq_issue_cmd(smmu, &cmd);
+
+     /* Invalidate any stale TLB entries */
+     cmd.opcode = CMDQ_OP_TLBI_EL2_ALL;

Do we need to execute CMDQ_OP_TLBI_EL2_ALL? Linux only run at EL1. It at least rely on SMMU_IDR0.Hyp

Hi Leizhen,

Thanks for the review!

Quoted 12 lines · leizhen

drivers/iommu/arm-smmu-v3.c line 662

Quoted 8 lines

drivers/iommu/arm-smmu-v3.c new file +7 -0

+
+static int queue_poll_cons(struct arm_smmu_queue *q, u32 until, bool wfe)
+{
+     ktime_t timeout = ktime_add_us(ktime_get(), ARM_SMMU_POLL_TIMEOUT_US);
+
+     while (queue_sync_cons(q), __queue_cons_before(q, until)) {
+             if (ktime_compare(ktime_get(), timeout) > 0)

Is it good to limit hardware behavior? May be wait for ever will be better. If SMMU can not consume queue items under normal condition, the SMMU hardware is broken, will lead software system to be crashed later.

I disagree. Having a broken SMMU lock-up the entire kernel is considerably worse than having e.g. a DMA master get stuck. If this timeout expires, then we'll print a message and continue without waiting any longer for a CMD_SYNC completion (see arm_smmu_cmdq_issue_cmd).

Quoted 23 lines

Quoted 23 lines

drivers/iommu/arm-smmu-v3.c new file +22 -0

+                     return -ETIMEDOUT;
+
+             if (wfe) {
+                     wfe();
+             } else {
+                     cpu_relax();
+                     udelay(1);
+             }
+     }
+
+     return 0;
+}
+
+static void queue_write(__le64 *dst, u64 *src, size_t n_dwords)
+{
+     int i;
+
+     for (i = 0; i < n_dwords; ++i)
+             *dst++ = cpu_to_le64(*src++);
+}
+
+static int queue_insert_raw(struct arm_smmu_queue *q, u64 *ent)

Quoted 8 lines

drivers/iommu/arm-smmu-v3.c line 689

Quoted 5 lines

drivers/iommu/arm-smmu-v3.c new file +5 -0

+{
+     if (queue_full(q))
+             return -ENOSPC;
+
+     queue_write(Q_ENT(q, q->prod), ent, q->ent_dwords);

A dmb or dsb maybe needed. We must insure all data written are completed, then notify hardware to consume.

The dsb is performed when we update the producer pointer in queue_inc_prod (since we use writel as opposed to writel_relaxed).

Quoted 18 lines

Quoted 17 lines

drivers/iommu/arm-smmu-v3.c new file +15 -0

+     queue_inc_prod(q);
+     return 0;
+}
+
+
+/* High-level queue accessors */
+static int arm_smmu_cmdq_build_cmd(u64 *cmd, struct arm_smmu_cmdq_ent *ent)
+{
+     memset(cmd, 0, CMDQ_ENT_DWORDS << 3);
+     cmd[0] |= (ent->opcode & CMDQ_0_OP_MASK) << CMDQ_0_OP_SHIFT;
+
+     switch (ent->opcode) {
+     case CMDQ_OP_TLBI_EL2_ALL:
 
+     case CMDQ_OP_CMD_SYNC:
+             cmd[0] |= CMDQ_SYNC_0_CS_SEV;

Quoted 2 lines

We can not always set SIG_SEV, actually it should base upon SMMU_IDR0.SEV(smmu->features & ARM_SMMU_FEAT_SEV)

It doesn't matter for the CMD_SYNC command (which treats SIG_SEV as SIG_NONE in this case). What actually matters is that we don't perform wfe() when the SMMU can't issue the event, but that's already taken care of in arm_smmu_cmdq_issue_cmd.

Quoted 21 lines

Quoted 21 lines

drivers/iommu/arm-smmu-v3.c new file +19 -0

+             break;
+     default:
+             return -ENOENT;
+     }
+
+     return 0;
+}
 
+
+static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
+                                    struct io_pgtable_cfg *pgtbl_cfg)
+{
+     int ret;
+     u16 asid;
+     struct arm_smmu_device *smmu = smmu_domain->smmu;
+     struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg;
+
+     asid = arm_smmu_bitmap_alloc(smmu->asid_map, smmu->asid_bits);
+     if (IS_ERR_VALUE(asid))
+             return asid;

Quoted 8 lines

drivers/iommu/arm-smmu-v3.c line 1379

Quoted 3 lines

drivers/iommu/arm-smmu-v3.c new file +3 -0

+
+     cfg->cdptr = dma_zalloc_coherent(smmu->dev, CTXDESC_CD_DWORDS << 3,
+                                      &cfg->cdptr_dma, GFP_KERNEL);

Why use dma_zalloc_coherent? iova is coverted from PA by calling phys_to_dma. I afraid PA and iova maybe not equal. In fact, the mapping between iova and PA is rely on SMMU driver itself. Why not use virt_to_phys to get PA, SMMU hardware actually require PA.

The SMMU structure walker is not allowed to be chained into another SMMU, so the physical and DMA addresses are equal here. That said, the walker does not need to be cache coherent, so I need to use the coherent DMA allocator to get memory of the correct attributes. I also need the table to be naturally aligned.

An alternative would be something like __get_free_pages with homebrew cache maintenance for non-coherent SMMUs, but I don't want to re-invent the DMA mapping code in the driver (we already have a similar mess with the page tables, which I'd like to sort out).

Quoted 21 lines

Quoted 21 lines

drivers/iommu/arm-smmu-v3.c new file +20 -0

+static int arm_smmu_alloc_l2_strtab(struct arm_smmu_device *smmu)
+{
+     int ret;
+     unsigned int i;
+     struct arm_smmu_strtab_cfg *cfg = &smmu->strtab_cfg;
+     size_t size = sizeof(*cfg->l1_desc) * cfg->num_l1_descs;
+     void *strtab = smmu->strtab_cfg.strtab;
+
+     cfg->l1_desc = devm_kzalloc(smmu->dev, size, GFP_KERNEL);
+     if (!cfg->l1_desc) {
+             dev_err(smmu->dev, "failed to allocate l1 stream table desc\n");
+             return -ENOMEM;
+     }
+
+     size = 1 << (STRTAB_SPLIT + ilog2(STRTAB_STE_DWORDS) + 3);
+     for (i = 0; i < cfg->num_l1_descs; ++i) {
+             struct arm_smmu_strtab_l1_desc *desc = &cfg->l1_desc[i];
+
+             desc->span = STRTAB_SPLIT + 1;
+             desc->l2ptr = dma_zalloc_coherent(smmu->dev, size,

Quoted 8 lines

Quoted 1 line

+ &desc->l2ptr_dma, GFP_KERNEL);

No, no, please don't allocate all Lv2 table memory, we should dynamic allocation when needed. Otherwise we can not save memory relative to one level table. And cfg->l1_desc seems not necessary. Before create mapping for a specified StreamID, we read corresponding Lv1 table entry, if L2Ptr is NULL, then we build Lv2 table. Otherwise, means this Lv2 table have already been built, because a Lv2 table is shared by a group of StreamIDs.

The purpose of the two level approach isn't to save memory; it's to remove the need for a single (large) contiguous block. Furthermore, we need all master devices in the system to come up in a state where their transactions bypass the SMMU (i.e. we don't require them to use the SMMU for translation). Achieving this necessitates a fully-populated stream-table before any DMA occurs.

I suppose we could look into populating it based on the ->add_device callback, which currently does have some range checks (arm_smmu_sid_in_range). Is that what you had in mind?

Quoted 26 lines

Quoted 26 lines

drivers/iommu/arm-smmu-v3.c new file +25 -0

+             if (!desc->l2ptr) {
+                     dev_err(smmu->dev,
+                             "failed to allocate l2 stream table %u\n", i);
+                     ret = -ENOMEM;
+                     goto out_free_l2;
+             }
+
+             arm_smmu_init_bypass_stes(desc->l2ptr, 1 << STRTAB_SPLIT);
+             arm_smmu_write_strtab_l1_desc(strtab, desc);
+             strtab += STRTAB_STE_DWORDS;
+     }
+
+     return 0;
+
+out_free_l2:
+     arm_smmu_free_l2_strtab(smmu);
+     return ret;
+}
+
+static int arm_smmu_init_strtab(struct arm_smmu_device *smmu)
+{
+     void *strtab;
+     u64 reg;
+     u32 size;
+     int ret = 0;

Quoted 8 lines

drivers/iommu/arm-smmu-v3.c line 2010

Quoted 3 lines

drivers/iommu/arm-smmu-v3.c new file +3 -0

+
+     strtab = dma_zalloc_coherent(smmu->dev, 1 << STRTAB_L1_SZ_SHIFT,
+                                  &smmu->strtab_cfg.strtab_dma, GFP_KERNEL);

As above, when Lv2 tables are dynamic allocation, we can create Lv1 table base on SMMU_IDR1.SIDSIZE and support non-pic devices. Oh, if SIDSIZE is too large, like 32. Maybe we should use 64K size Lv2 table. But we can only use PAGE_SIZE first, for lazy.

Right now, the l2 tables are 4k and l1 table is 8k. That's (a) nice for allocation on a system with 4k pages and (b) enough to cover a PCI host controller. The problem with supporting 32-bit SIDs is that the l1 will become a sizeable contiguous chunk which we'll have to allocate regardless.

So, if we keep the l2 size at 4k for the moment, how big would you like the l1? For a 32-bit numberspace, it would be 4MB, which I don't think is practical.

Quoted 81 lines

Quoted 81 lines

drivers/iommu/arm-smmu-v3.c new file +79 -0

+     if (!strtab) {
+             dev_err(smmu->dev, "failed to allocate l1 stream table\n");
+             return -ENOMEM;
+     }
+     smmu->strtab_cfg.strtab = strtab;
+
+     reg  = smmu->strtab_cfg.strtab_dma &
+            STRTAB_BASE_ADDR_MASK << STRTAB_BASE_ADDR_SHIFT;
+     reg |= STRTAB_BASE_RA;
+     smmu->strtab_cfg.strtab_base = reg;
+
+     if (smmu->features & ARM_SMMU_FEAT_2_LVL_STRTAB) {
+             size = STRTAB_L1_SZ_SHIFT - (ilog2(STRTAB_L1_DESC_DWORDS) + 3);
+             smmu->strtab_cfg.num_l1_descs = 1 << size;
+             size += STRTAB_SPLIT;
+             reg = STRTAB_BASE_CFG_FMT_2LVL;
+
+             ret = arm_smmu_alloc_l2_strtab(smmu);
+             if (ret)
+                     goto out_free_l1;
+     } else {
+             size = STRTAB_L1_SZ_SHIFT - (ilog2(STRTAB_STE_DWORDS) + 3);
+             smmu->strtab_cfg.num_l1_descs = 0;
+             reg = STRTAB_BASE_CFG_FMT_LINEAR;
+             arm_smmu_init_bypass_stes(strtab, 1 << size);
+     }
+
+     if (size < smmu->sid_bits)
+             dev_warn(smmu->dev, "%s strtab only covers %u/%u bits of SID\n",
+                      smmu->features & ARM_SMMU_FEAT_2_LVL_STRTAB ?
+                      "2-level" : "linear",
+                      size, smmu->sid_bits);
+
+     reg |= (size & STRTAB_BASE_CFG_LOG2SIZE_MASK)
+             << STRTAB_BASE_CFG_LOG2SIZE_SHIFT;
+     reg |= (STRTAB_SPLIT & STRTAB_BASE_CFG_SPLIT_MASK)
+             << STRTAB_BASE_CFG_SPLIT_SHIFT;
+     smmu->strtab_cfg.strtab_base_cfg = reg;
+
+     /* Allocate the first VMID for stage-2 bypass STEs */
+     set_bit(0, smmu->vmid_map);
+     return 0;
+
+out_free_l1:
+     dma_free_coherent(smmu->dev, 1 << STRTAB_L1_SZ_SHIFT, strtab,
+                       smmu->strtab_cfg.strtab_dma);
+     return ret;
+}
+
+static void arm_smmu_free_strtab(struct arm_smmu_device *smmu)
+{
+     struct arm_smmu_strtab_cfg *cfg = &smmu->strtab_cfg;
+
+     arm_smmu_free_l2_strtab(smmu);
+     dma_free_coherent(smmu->dev, 1 << STRTAB_L1_SZ_SHIFT, cfg->strtab,
+                       cfg->strtab_dma);
+}
+
 
+
+static int arm_smmu_device_reset(struct arm_smmu_device *smmu)
+{
+     int ret;
+     u32 reg, enables;
+     struct arm_smmu_cmdq_ent cmd;
+
+     /* Clear CR0 and sync (disables SMMU and queue processing) */
+     reg = readl_relaxed(smmu->base + ARM_SMMU_CR0);
+     if (reg & CR0_SMMUEN)
+             dev_warn(smmu->dev, "SMMU currently enabled! Resetting...\n");
+
+     ret = arm_smmu_device_disable(smmu);
+     if (ret)
+             return ret;
+
+     /* CR1 (table and queue memory attributes) */
+     reg = (CR1_SH_ISH << CR1_TABLE_SH_SHIFT) |
+           (CR1_CACHE_WB << CR1_TABLE_OC_SHIFT) |
+           (CR1_CACHE_WB << CR1_TABLE_IC_SHIFT) |
+           (CR1_SH_ISH << CR1_QUEUE_SH_SHIFT) |

Quoted 8 lines

drivers/iommu/arm-smmu-v3.c line 2211

Quoted 6 lines

drivers/iommu/arm-smmu-v3.c new file +6 -0

+           (CR1_CACHE_WB << CR1_QUEUE_OC_SHIFT) |
+           (CR1_CACHE_WB << CR1_QUEUE_IC_SHIFT);
+     writel_relaxed(reg, smmu->base + ARM_SMMU_CR1);
+
+     /* CR2 (random crap) */
+     reg = CR2_PTM | CR2_RECINVMID | CR2_E2H;

Do we need to explicitly set CR2_E2H? Linux only run at EL1.

Setting E2H won't cause any harm and I'd expect Linux to run there in the future. If we ever decide to share CPU page tables with the SMMU, then we'll need this set to participate in DVM.

Quoted 25 lines

Quoted 25 lines

drivers/iommu/arm-smmu-v3.c new file +24 -0

+     writel_relaxed(reg, smmu->base + ARM_SMMU_CR2);
+
+     /* Stream table */
+     writeq_relaxed(smmu->strtab_cfg.strtab_base,
+                    smmu->base + ARM_SMMU_STRTAB_BASE);
+     writel_relaxed(smmu->strtab_cfg.strtab_base_cfg,
+                    smmu->base + ARM_SMMU_STRTAB_BASE_CFG);
+
+     /* Command queue */
+     writeq_relaxed(smmu->cmdq.q.q_base, smmu->base + ARM_SMMU_CMDQ_BASE);
+     writel_relaxed(smmu->cmdq.q.prod, smmu->base + ARM_SMMU_CMDQ_PROD);
+     writel_relaxed(smmu->cmdq.q.cons, smmu->base + ARM_SMMU_CMDQ_CONS);
+
+     enables = CR0_CMDQEN;
+     ret = arm_smmu_write_reg_sync(smmu, enables, ARM_SMMU_CR0,
+                                   ARM_SMMU_CR0ACK);
+     if (ret) {
+             dev_err(smmu->dev, "failed to enable command queue\n");
+             return ret;
+     }
+
+     /* Invalidate any cached configuration */
+     cmd.opcode = CMDQ_OP_CFGI_ALL;
+     arm_smmu_cmdq_issue_cmd(smmu, &cmd);

Quoted 8 lines

drivers/iommu/arm-smmu-v3.c line 2240

Quoted 5 lines

drivers/iommu/arm-smmu-v3.c new file +5 -0

+     cmd.opcode = CMDQ_OP_CMD_SYNC;
+     arm_smmu_cmdq_issue_cmd(smmu, &cmd);
+
+     /* Invalidate any stale TLB entries */
+     cmd.opcode = CMDQ_OP_TLBI_EL2_ALL;

Do we need to execute CMDQ_OP_TLBI_EL2_ALL? Linux only run at EL1. It at least rely on SMMU_IDR0.Hyp

The SMMU isn't guaranteed to come up clean out of reset, so it's a good idea to perform this invalidation in case of junk in the TLB. Given that Linux owns the stage-2 translation, then this is the right place to do it.

Will

.

Quoted 6 lines · Will Deacon

The purpose of the two level approach isn't to save memory; it's to remove the need for a single (large) contiguous block. Furthermore, we need all master devices in the system to come up in a state where their transactions bypass the SMMU (i.e. we don't require them to use the SMMU for translation). Achieving this necessitates a fully-populated stream-table before any DMA occurs.

OK, but for non-pci devices(maybe pci devices also), initialize all devices(StreamIDs) to bypass mode maybe incorrect. Some devices maybe capable bring attributes by itself, and access different memory with different attributes(device attribute or cacheable attribute); some devices maybe not capable bring attributes by itself, but need access memory with cacheable attribute only, so we should set STE.MTCFG = 1. Provide dts configuration will be better.

Furthermore, I don't agree initialize all devices to bypass by default. Suppose a non-pci device's StreamID can be dynamic configured. We require StreamID-A, but the device actually issue StreamID-B because of software configuration fault. We really hope SMMU can report Bad StreamID fault.

Quoted 3 lines

I suppose we could look into populating it based on the ->add_device callback, which currently does have some range checks (arm_smmu_sid_in_range). Is that what you had in mind?

Yes, maybe attach_dev. But we should allocated the whole Lv1 table base on SMMU_IDR1.SIDSIZE

Quoted 40 lines

Quoted 25 lines

Quoted 25 lines

drivers/iommu/arm-smmu-v3.c new file +25 -0

+             if (!desc->l2ptr) {
+                     dev_err(smmu->dev,
+                             "failed to allocate l2 stream table %u\n", i);
+                     ret = -ENOMEM;
+                     goto out_free_l2;
+             }
+
+             arm_smmu_init_bypass_stes(desc->l2ptr, 1 << STRTAB_SPLIT);
+             arm_smmu_write_strtab_l1_desc(strtab, desc);
+             strtab += STRTAB_STE_DWORDS;
+     }
+
+     return 0;
+
+out_free_l2:
+     arm_smmu_free_l2_strtab(smmu);
+     return ret;
+}
+
+static int arm_smmu_init_strtab(struct arm_smmu_device *smmu)
+{
+     void *strtab;
+     u64 reg;
+     u32 size;
+     int ret = 0;

Quoted 8 lines

drivers/iommu/arm-smmu-v3.c line 2010

Quoted 3 lines

drivers/iommu/arm-smmu-v3.c new file +3 -0

+
+     strtab = dma_zalloc_coherent(smmu->dev, 1 << STRTAB_L1_SZ_SHIFT,
+                                  &smmu->strtab_cfg.strtab_dma, GFP_KERNEL);

As above, when Lv2 tables are dynamic allocation, we can create Lv1 table base on SMMU_IDR1.SIDSIZE and support non-pic devices. Oh, if SIDSIZE is too large, like 32. Maybe we should use 64K size Lv2 table. But we can only use PAGE_SIZE first, for lazy.

Right now, the l2 tables are 4k and l1 table is 8k. That's (a) nice for allocation on a system with 4k pages and (b) enough to cover a PCI host controller. The problem with supporting 32-bit SIDs is that the l1 will become a sizeable contiguous chunk which we'll have to allocate regardless.

Quoted 3 lines

So, if we keep the l2 size at 4k for the moment, how big would you like the l1? For a 32-bit numberspace, it would be 4MB, which I don't think is practical.

Yes, because I don't think SMMU_IDR1.SIDSIZE = 32 really exist, so I said use PAGE_SIZE first.

But now, this patch only support SMMU_IDR1.SIDSIZE <= 16. Suppose SMMU_IDR1.SIDSIZE = 25?Lv2 table size at 4K, then Lv1 table need 2^(25 - 6 + 3) = 2^22 = 4M. If we pre-allocated all Lv2 table, we need 2^25 * 64 = 2^31 = 2G, that's too big. So we must only allocate Lv2 table when we needed.

If SMMU_IDR1.SIDSIZE = 32 really exist(or too big), we need dynamic choose Lv2 table size(4K,16K,64K). Because Lv1 table maybe too big, and can not be allocated by current API, a dts configuration should be added, like lv1-table-base = <0x0 0x0>, and we use ioremap_cache get VA(maybe ioremap, for non-coherent SMMU).

Oh, I can do it after your patches upstreamed, because this problem maybe only I met.

Quoted 9 lines

Quoted 9 lines

Quoted 8 lines

drivers/iommu/arm-smmu-v3.c new file +8 -0

+     /* Invalidate any cached configuration */
+     cmd.opcode = CMDQ_OP_CFGI_ALL;
+     arm_smmu_cmdq_issue_cmd(smmu, &cmd);
+     cmd.opcode = CMDQ_OP_CMD_SYNC;
+     arm_smmu_cmdq_issue_cmd(smmu, &cmd);
+
+     /* Invalidate any stale TLB entries */
+     cmd.opcode = CMDQ_OP_TLBI_EL2_ALL;

Quoted 6 lines

Quoted 2 lines

Do we need to execute CMDQ_OP_TLBI_EL2_ALL? Linux only run at EL1. It at least rely on SMMU_IDR0.Hyp

The SMMU isn't guaranteed to come up clean out of reset, so it's a good idea to perform this invalidation in case of junk in the TLB. Given that Linux owns the stage-2 translation, then this is the right place to do it.

OK. But it should be controled by SMMU_IDR0.Hyp. I means:
if (SMMU_IDR0.Hyp)
execute command CMDQ_OP_TLBI_EL2_ALL

When SMMU_IDR0.Hyp=?0?, this command causes CERROR_ILL

Hi again,

Sorry for the delay in replying, I've been tied up with other stuff.

Quoted 8 lines · leizhen

Quoted 6 lines · Will Deacon

The purpose of the two level approach isn't to save memory; it's to remove the need for a single (large) contiguous block. Furthermore, we need all master devices in the system to come up in a state where their transactions bypass the SMMU (i.e. we don't require them to use the SMMU for translation). Achieving this necessitates a fully-populated stream-table before any DMA occurs.

Quoted 7 lines

OK, but for non-pci devices(maybe pci devices also), initialize all devices(StreamIDs) to bypass mode maybe incorrect. Some devices maybe capable bring attributes by itself, and access different memory with different attributes(device attribute or cacheable attribute); some devices maybe not capable bring attributes by itself, but need access memory with cacheable attribute only, so we should set STE.MTCFG = 1. Provide dts configuration will be better.

If we want to make use of memory overrides, then I think we should add that as a separate patch because it would have implications on the IOMMU API. I don't particularly like putting this policy information in the device-tree binding on a per-driver basis.

Quoted 5 lines

Furthermore, I don't agree initialize all devices to bypass by default. Suppose a non-pci device's StreamID can be dynamic configured. We require StreamID-A, but the device actually issue StreamID-B because of software configuration fault. We really hope SMMU can report Bad StreamID fault.

I think the default behaviour has to be to bypass by default, otherwise we can break DMA for any devices behind an SMMU but not attached to an IOMMU domain. How about I add a cmdline parameter to change the default behaviour?

Quoted 6 lines

Quoted 3 lines

I suppose we could look into populating it based on the ->add_device callback, which currently does have some range checks (arm_smmu_sid_in_range). Is that what you had in mind?

Yes, maybe attach_dev. But we should allocated the whole Lv1 table base on SMMU_IDR1.SIDSIZE

I think it needs to be in add_device so that we can implement the default

bypass case. We could even have one L2 table per PCI bus, but I need to think about how big the L1 table should be.

Quoted 40 lines

Quoted 40 lines

Quoted 25 lines

Quoted 25 lines

drivers/iommu/arm-smmu-v3.c new file +25 -0

+             if (!desc->l2ptr) {
+                     dev_err(smmu->dev,
+                             "failed to allocate l2 stream table %u\n", i);
+                     ret = -ENOMEM;
+                     goto out_free_l2;
+             }
+
+             arm_smmu_init_bypass_stes(desc->l2ptr, 1 << STRTAB_SPLIT);
+             arm_smmu_write_strtab_l1_desc(strtab, desc);
+             strtab += STRTAB_STE_DWORDS;
+     }
+
+     return 0;
+
+out_free_l2:
+     arm_smmu_free_l2_strtab(smmu);
+     return ret;
+}
+
+static int arm_smmu_init_strtab(struct arm_smmu_device *smmu)
+{
+     void *strtab;
+     u64 reg;
+     u32 size;
+     int ret = 0;

Quoted 8 lines

drivers/iommu/arm-smmu-v3.c line 2010

Quoted 3 lines

drivers/iommu/arm-smmu-v3.c new file +3 -0

+
+     strtab = dma_zalloc_coherent(smmu->dev, 1 << STRTAB_L1_SZ_SHIFT,
+                                  &smmu->strtab_cfg.strtab_dma, GFP_KERNEL);

As above, when Lv2 tables are dynamic allocation, we can create Lv1 table base on SMMU_IDR1.SIDSIZE and support non-pic devices. Oh, if SIDSIZE is too large, like 32. Maybe we should use 64K size Lv2 table. But we can only use PAGE_SIZE first, for lazy.

Right now, the l2 tables are 4k and l1 table is 8k. That's (a) nice for allocation on a system with 4k pages and (b) enough to cover a PCI host controller. The problem with supporting 32-bit SIDs is that the l1 will become a sizeable contiguous chunk which we'll have to allocate regardless.

Quoted 6 lines

Quoted 3 lines

So, if we keep the l2 size at 4k for the moment, how big would you like the l1? For a 32-bit numberspace, it would be 4MB, which I don't think is practical.

Yes, because I don't think SMMU_IDR1.SIDSIZE = 32 really exist, so I said use PAGE_SIZE first.

Well, I certainly wouldn't rule anything out.

Quoted 5 lines

But now, this patch only support SMMU_IDR1.SIDSIZE <= 16. Suppose SMMU_IDR1.SIDSIZE = 25?Lv2 table size at 4K, then Lv1 table need 2^(25 - 6 + 3) = 2^22 = 4M. If we pre-allocated all Lv2 table, we need 2^25 * 64 = 2^31 = 2G, that's too big. So we must only allocate Lv2 table when we needed.

I agree that's way too big, but I have limited things to 16-bit for a reason ;)

Quoted 6 lines

If SMMU_IDR1.SIDSIZE = 32 really exist(or too big), we need dynamic choose Lv2 table size(4K,16K,64K). Because Lv1 table maybe too big, and can not be allocated by current API, a dts configuration should be added, like lv1-table-base = <0x0 0x0>, and we use ioremap_cache get VA(maybe ioremap, for non-coherent SMMU).

Quoted 2 lines

Oh, I can do it after your patches upstreamed, because this problem maybe only I met.

I'll have a think about this and see what I can come up with for version 2 of the patch. I'd like to avoid adding additional properties to the DT until they're actually needed, though.

Quoted 17 lines

Quoted 10 lines

Quoted 10 lines

Quoted 9 lines

drivers/iommu/arm-smmu-v3.c new file +8 -0

+     /* Invalidate any cached configuration */
+     cmd.opcode = CMDQ_OP_CFGI_ALL;
+     arm_smmu_cmdq_issue_cmd(smmu, &cmd);
+     cmd.opcode = CMDQ_OP_CMD_SYNC;
+     arm_smmu_cmdq_issue_cmd(smmu, &cmd);
+
+     /* Invalidate any stale TLB entries */
+     cmd.opcode = CMDQ_OP_TLBI_EL2_ALL;

Quoted 6 lines

Quoted 2 lines

Do we need to execute CMDQ_OP_TLBI_EL2_ALL? Linux only run at EL1. It at least rely on SMMU_IDR0.Hyp

The SMMU isn't guaranteed to come up clean out of reset, so it's a good idea to perform this invalidation in case of junk in the TLB. Given that Linux owns the stage-2 translation, then this is the right place to do it.

Quoted 5 lines

OK. But it should be controled by SMMU_IDR0.Hyp. I means:
if (SMMU_IDR0.Hyp)
execute command CMDQ_OP_TLBI_EL2_ALL

When SMMU_IDR0.Hyp=?0?, this command causes CERROR_ILL

Well spotted, thanks!

Will

.

Quoted 7 lines · Will Deacon

Quoted 6 lines · leizhen

If SMMU_IDR1.SIDSIZE = 32 really exist(or too big), we need dynamic choose Lv2 table size(4K,16K,64K). Because Lv1 table maybe too big, and can not be allocated by current API, a dts configuration should be added, like lv1-table-base = <0x0 0x0>, and we use ioremap_cache get VA(maybe ioremap, for non-coherent SMMU).

Quoted 6 lines

Quoted 2 lines

Oh, I can do it after your patches upstreamed, because this problem maybe only I met.

I'll have a think about this and see what I can come up with for version 2 of the patch. I'd like to avoid adding additional properties to the DT until they're actually needed, though.

OK. Will you support non-pci devices in patch version 2?

Quoted 8 lines · leizhen

Quoted 7 lines · Will Deacon

Quoted 6 lines · leizhen

If SMMU_IDR1.SIDSIZE = 32 really exist(or too big), we need dynamic choose Lv2 table size(4K,16K,64K). Because Lv1 table maybe too big, and can not be allocated by current API, a dts configuration should be added, like lv1-table-base = <0x0 0x0>, and we use ioremap_cache get VA(maybe ioremap, for non-coherent SMMU).

Quoted 8 lines

Quoted 6 lines

Quoted 2 lines

Oh, I can do it after your patches upstreamed, because this problem maybe only I met.

I'll have a think about this and see what I can come up with for version 2 of the patch. I'd like to avoid adding additional properties to the DT until they're actually needed, though.

OK. Will you support non-pci devices in patch version 2?

I don't (yet) plan to support non-PCI devices, for two reasons:

(1) The support should be based on top of Laurent's RFC series here:

http://lists.infradead.org/pipermail/linux-arm-kernel/2015-May/343686.html

which needs some review etc.

(2) My simulator only has PCI endpoints

Of course, if you have something that you can test with, then I'm more than happy to review patches adding this support providing that they're based on the series above.

On the table front, my current v2 patch uses 16k level-2 tables allocated lazily (so each one has 256 entries and covers a single PCI bus). I've also capped the SIDSIZE to 21 for the moment, which means we get a 64k table there.

Sound reasonable for the time being?

Will

.

drivers/iommu/arm-smmu-v3.c new file +22 -0

+static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
+{
+	int ret = 0;
+	struct arm_smmu_device *smmu;
+	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+	struct arm_smmu_group *smmu_group = arm_smmu_group_get(dev);
+
+	if (!smmu_group)
+		return -ENOENT;
+
+	/* Already attached to a different domain? */
+	if (smmu_group->domain && smmu_group->domain != smmu_domain)
+		return -EEXIST;
+
+	smmu = smmu_group->smmu;
+	mutex_lock(&smmu_domain->init_mutex);
+
+	if (!smmu_domain->smmu) {
+		smmu_domain->smmu = smmu;
+		ret = arm_smmu_domain_finalise(domain);
+		if (ret) {
+			smmu_domain->smmu = NULL;

Hi Will,

the code looks good overall, I just have some questions below.

Quoted 27 lines

drivers/iommu/arm-smmu-v3.c new file +27 -0

+static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
+{
+	int ret = 0;
+	struct arm_smmu_device *smmu;
+	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+	struct arm_smmu_group *smmu_group = arm_smmu_group_get(dev);
+
+	if (!smmu_group)
+		return -ENOENT;
+
+	/* Already attached to a different domain? */
+	if (smmu_group->domain && smmu_group->domain != smmu_domain)
+		return -EEXIST;
+
+	smmu = smmu_group->smmu;
+	mutex_lock(&smmu_domain->init_mutex);
+
+	if (!smmu_domain->smmu) {
+		smmu_domain->smmu = smmu;
+		ret = arm_smmu_domain_finalise(domain);
+		if (ret) {
+			smmu_domain->smmu = NULL;
+			goto out_unlock;
+		}
+	} else if (smmu_domain->smmu != smmu) {
+		dev_err(dev,
+			"cannot attach to SMMU %s (upstream of %s)\n",

drivers/iommu/arm-smmu-v3.c line 1565

Quoted 5 lines

drivers/iommu/arm-smmu-v3.c new file +5 -0

+			dev_name(smmu_domain->smmu->dev),
+			dev_name(smmu->dev));
+		ret = -ENXIO;
+		goto out_unlock;
+	}

This looks like all devices in a domain need to be behind the same SMMU device, right?

Quoted 4 lines

drivers/iommu/arm-smmu-v3.c new file +4 -0

+	/* Page sizes */
+	if (reg & IDR5_GRAN64K)
+		pgsize_bitmap |= SZ_64K | SZ_512M;
+	if (reg & IDR5_GRAN16K)

drivers/iommu/arm-smmu-v3.c line 2424

Quoted 5 lines

drivers/iommu/arm-smmu-v3.c new file +5 -0

+		pgsize_bitmap |= SZ_16K | SZ_32M;
+	if (reg & IDR5_GRAN4K)
+		pgsize_bitmap |= SZ_4K | SZ_2M | SZ_1G;
+
+	arm_smmu_ops.pgsize_bitmap &= pgsize_bitmap;

So this could effictivly lead to a zero pgsize_bitmap when there are SMMUs in the system with support for different page sizes, no?

Quoted 4 lines · Joerg Roedel

Quoted 4 lines

drivers/iommu/arm-smmu-v3.c new file +4 -0

+	/* Page sizes */
+	if (reg & IDR5_GRAN64K)
+		pgsize_bitmap |= SZ_64K | SZ_512M;
+	if (reg & IDR5_GRAN16K)

Quoted 8 lines

drivers/iommu/arm-smmu-v3.c line 2424

Quoted 5 lines

drivers/iommu/arm-smmu-v3.c new file +5 -0

+		pgsize_bitmap |= SZ_16K | SZ_32M;
+	if (reg & IDR5_GRAN4K)
+		pgsize_bitmap |= SZ_4K | SZ_2M | SZ_1G;
+
+	arm_smmu_ops.pgsize_bitmap &= pgsize_bitmap;

So this could effictivly lead to a zero pgsize_bitmap when there are SMMUs in the system with support for different page sizes, no?

Indeed, if there is no common page size then we end up not being able to support any. I tried to resolve this by moving the bitmap out of the iommu_ops and into the iommu_domain, but you weren't fond of that idea ;)

Hi Will,

Quoted 14 lines · Will Deacon

Quoted 4 lines · Joerg Roedel

Quoted 4 lines

(quoted diff) +4 -0

+	/* Page sizes */
+	if (reg & IDR5_GRAN64K)
+		pgsize_bitmap |= SZ_64K | SZ_512M;
+	if (reg & IDR5_GRAN16K)

Quoted 8 lines

Review

Quoted 5 lines

(quoted diff) +5 -0

+		pgsize_bitmap |= SZ_16K | SZ_32M;
+	if (reg & IDR5_GRAN4K)
+		pgsize_bitmap |= SZ_4K | SZ_2M | SZ_1G;
+
+	arm_smmu_ops.pgsize_bitmap &= pgsize_bitmap;

So this could effictivly lead to a zero pgsize_bitmap when there are SMMUs in the system with support for different page sizes, no?

Quoted 3 lines

Indeed, if there is no common page size then we end up not being able to support any. I tried to resolve this by moving the bitmap out of the iommu_ops and into the iommu_domain, but you weren't fond of that idea ;)

Well, what you could do (and what I think the core should do at some point) is to build the resulting page-size bitmap by taking the biggest minimum page-size from all iommus and OR the page-size bigger than that together. For a system with 3 SMMUs supporting all of the above pgsize_bitmaps the resulting bitmap would look like this:

	SZ_64K | SZ_32M | SZ_2M | SZ_512M | SZ_1G;

With the biggest minimum page-size all of the bigger page-sizes can be emulated. But that is not necessary for this patch-set, just a suggestion for future work.

The trouble with this is, what about the CPU page size? Say you have some multimedia subsystem with its own integrated SMMU and for that they've only implemented the 16K granule scheme because it works best for the video hardware (and the GPU driver is making direct IOMMU API calls to remap carved-out RAM rather than using DMA-mapping). Now, the SMMU on the compute side of the SoC serving the general peripherals will be rendered useless by bumping the system-wide minimum page size up to 16K, because it then can't map that scatterlist of discontiguous 4K pages that the USB controller needs...

I think this really represents another push to get away from (or at least around) the page-at-a-time paradigm - if the IOMMU API itself wasn't too fussed about page sizes and could let drivers handle the full map/unmap requests however they see fit, I think we could bypass a lot of these issues. We've already got the Intel IOMMU driver doing horrible hacks with the pgsize_bitmap to cheat the system, I'm sure we don't want to add any more of that. How about something like the below diff as a first step?

Quoted 11 lines · Robin Murphy

The trouble with this is, what about the CPU page size? Say you have some multimedia subsystem with its own integrated SMMU and for that they've only implemented the 16K granule scheme because it works best for the video hardware (and the GPU driver is making direct IOMMU API calls to remap carved-out RAM rather than using DMA-mapping). Now, the SMMU on the compute side of the SoC serving the general peripherals will be rendered useless by bumping the system-wide minimum page size up to 16K, because it then can't map that scatterlist of discontiguous 4K pages that the USB controller needs...

Quoted 8 lines

I think this really represents another push to get away from (or at least around) the page-at-a-time paradigm - if the IOMMU API itself wasn't too fussed about page sizes and could let drivers handle the full map/unmap requests however they see fit, I think we could bypass a lot of these issues. We've already got the Intel IOMMU driver doing horrible hacks with the pgsize_bitmap to cheat the system, I'm sure we don't want to add any more of that. How about something like the below diff as a first step?

Moving functionality out of the iommu core code into the drivers is the wrong direction imo. It is better to solve it with something like

	struct iommu_domain *iommu_domain_alloc_for_group(struct iommu_group *group);

Which gets us a domain that can only be assigned to that particular group. Since there is a clear one-to-many relationship between a hardware iommu and the groups of devices behind it, we could propagate the pgsize_bitmap from the iommu to the group and then to the domain.

Domains allocated via iommu_domain_alloc() would get the merged pgsize_bitmap like I described in my previous mail.

But to make this happen we need a representation of single hardware iommu instances in the iommu core first.

I like this proposal. The only remaining part is that the pgsize bitmap inherited by the domain can only truly be finalised once the page table is allocated, which in turn can only happen once we've identified the IOMMU instance.

In the case of iommu_domain_alloc_for_group, that's nice and straightforward (i.e. as part of the call) but for the default iommu_domain_alloc() case, we'd have to continue postponing things to ->attach time before we could provide a reliable pgsize_bitmap. This is to handle the case where an SMMU may be able to support only a subset of the pgsize_bitmap on a per-domain basis.

In which situation do you think the merged pgsize_bitmap would get used? The only place I can think of is if we're trying to call iommu_map on a domain with no devices attached. However, if that domain was created using iommu_domain_alloc() then the pgsize_bitmap is the least of our worries -- we don't even know the page table format!

Quoted 4 lines · Will Deacon

I like this proposal. The only remaining part is that the pgsize bitmap inherited by the domain can only truly be finalised once the page table is allocated, which in turn can only happen once we've identified the IOMMU instance.

We know the IOMMU instance for domains allocted with iommu_domain_alloc_for_group because every group is behind a single IOMMU instance. So the page-table can be allocated at domain creation time and mappings can be created before the group itself is attached.

Quoted 6 lines

In the case of iommu_domain_alloc_for_group, that's nice and straightforward (i.e. as part of the call) but for the default iommu_domain_alloc() case, we'd have to continue postponing things to ->attach time before we could provide a reliable pgsize_bitmap. This is to handle the case where an SMMU may be able to support only a subset of the pgsize_bitmap on a per-domain basis.

I don't think we need to postpone anything. Domains returned by iommu_domain_alloc() need to work for all groups. If there are multiple IOMMUs in the system with different capabilities/page-table formats the IOMMU core needs to build multiple page-tables for that domain.

VFIO already does that as a workaround of how things are done currently, but I really think this should be done in the IOMMU core code.

Quoted 5 lines

In which situation do you think the merged pgsize_bitmap would get used? The only place I can think of is if we're trying to call iommu_map on a domain with no devices attached. However, if that domain was created using iommu_domain_alloc() then the pgsize_bitmap is the least of our worries -- we don't even know the page table format!

The first usecase for the merged pgsize_bitmap that comes to mind is to determine the minimum page-size that can be used for a domain. In the SMMUv3 case when there are IOMMUs with different minium page-sizes in one system, this would be the biggest minimum page-size of all IOMMUs.

Quoted 4 lines · Joerg Roedel

I don't think we need to postpone anything. Domains returned by iommu_domain_alloc() need to work for all groups. If there are multiple IOMMUs in the system with different capabilities/page-table formats the IOMMU core needs to build multiple page-tables for that domain.

I really don't think that's feasible. We support an awful lot of combinations that affect the page table format on ARM: there are 5 different formats, each supporting different translation regimes (sets of page size) and each of those needs configuring for input/output address sizes to compute the number of levels. You could easily end up with over 20 page tables and their corresponding sets of control register values.

Given that we only install one page table in the end, I'm struggling to see the issue with postponing its allocation until we've figured out the IOMMU instance thanks to a device attach.