iommu/arm-smmu-v3: Introduce an RCU-protected invalidation array

From: Jason Gunthorpe <[email protected]>

Create a new data structure to hold an array of invalidations that need to be performed for the domain based on what masters are attached, to replace the single smmu pointer and linked list of masters in the current design.

Each array entry holds one of the invalidation actions - S1_ASID, S2_VMID, ATS or their variant with information to feed invalidation commands to HW. It is structured so that multiple SMMUs can participate in the same array, removing one key limitation of the current system.

To maximize performance, a sorted array is used as the data structure. It allows grouping SYNCs together to parallelize invalidations. For instance, it will group all the ATS entries after the ASID/VMID entry, so they will all be pushed to the PCI devices in parallel with one SYNC.

To minimize the locking cost on the invalidation fast path (reader of the invalidation array), the array is managed with RCU.

Provide a set of APIs to add/delete entries to/from an array, which cover cannot-fail attach cases, e.g. attaching to arm_smmu_blocked_domain. Also add kunit coverage for those APIs.

Signed-off-by Jason Gunthorpe <[email protected]>

Reviewed-by Jason Gunthorpe <[email protected]>

Co-developed-by Nicolin Chen <[email protected]>

Signed-off-by Nicolin Chen <[email protected]>

---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   |  91 +++++++
 .../iommu/arm/arm-smmu-v3/arm-smmu-v3-test.c  |  93 +++++++
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   | 248 ++++++++++++++++++
 3 files changed, 432 insertions(+)

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +56 -0

index 96a23ca633cb6..757158b9ea655 100644
@@ -649,6 +649,85 @@ struct arm_smmu_cmdq_batch {
 	int				num;
 };
 
+/*
+ * The order here also determines the sequence in which commands are sent to the
+ * command queue. E.g. TLBI must be done before ATC_INV.
+ */
+enum arm_smmu_inv_type {
+	INV_TYPE_S1_ASID,
+	INV_TYPE_S2_VMID,
+	INV_TYPE_S2_VMID_S1_CLEAR,
+	INV_TYPE_ATS,
+	INV_TYPE_ATS_FULL,
+};
+
+struct arm_smmu_inv {
+	struct arm_smmu_device *smmu;
+	u8 type;
+	u8 size_opcode;
+	u8 nsize_opcode;
+	u32 id; /* ASID or VMID or SID */
+	union {
+		size_t pgsize; /* ARM_SMMU_FEAT_RANGE_INV */
+		u32 ssid; /* INV_TYPE_ATS */
+	};
+
+	refcount_t users; /* users=0 to mark as a trash to be purged */
+};
+
+static inline bool arm_smmu_inv_is_ats(struct arm_smmu_inv *inv)
+{
+	return inv->type == INV_TYPE_ATS || inv->type == INV_TYPE_ATS_FULL;
+}
+
+/**
+ * struct arm_smmu_invs - Per-domain invalidation array
+ * @num_invs: number of invalidations in the flexible array
+ * @rwlock: optional rwlock to fench ATS operations
+ * @has_ats: flag if the array contains an INV_TYPE_ATS or INV_TYPE_ATS_FULL
+ * @rcu: rcu head for kfree_rcu()
+ * @inv: flexible invalidation array
+ *
+ * The arm_smmu_invs is an RCU data structure. During a ->attach_dev callback,
+ * arm_smmu_invs_merge(), arm_smmu_invs_unref() and arm_smmu_invs_purge() will
+ * be used to allocate a new copy of an old array for addition and deletion in
+ * the old domain's and new domain's invs arrays.
+ *
+ * The arm_smmu_invs_unref() mutates a given array, by internally reducing the
+ * users counts of some given entries. This exists to support a no-fail routine
+ * like attaching to an IOMMU_DOMAIN_BLOCKED. And it could pair with a followup
+ * arm_smmu_invs_purge() call to generate a new clean array.
+ *
+ * Concurrent invalidation thread will push every invalidation described in the
+ * array into the command queue for each invalidation event. It is designed like
+ * this to optimize the invalidation fast path by avoiding locks.
+ *
+ * A domain can be shared across SMMU instances. When an instance gets removed,
+ * it would delete all the entries that belong to that SMMU instance. Then, a
+ * synchronize_rcu() would have to be called to sync the array, to prevent any

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +5 -0

+	rwlock_init(&new_invs->rwlock);
+	new_invs->num_invs = num_invs;
+	return new_invs;
+}
+

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +56 -0

+		int cmp;
+
+		/* Skip any trash entry */
+		if (i < invs->num_invs && !refcount_read(&invs->inv[i].users)) {
+			num_trashes++;
+			i++;
+			continue;
+		}
+
+		cmp = arm_smmu_invs_cmp(invs, i, to_merge, j);
+		if (cmp < 0) {
+			/* not found in to_merge, leave alone */
+			i++;
+		} else if (cmp == 0) {
+			/* same item */
+			i++;
+			j++;
+		} else {
+			/* unique to to_merge */
+			num_adds++;
+			j++;
+		}
+	}
+
+	new_invs = arm_smmu_invs_alloc(invs->num_invs - num_trashes + num_adds);
+	if (IS_ERR(new_invs))
+		return new_invs;
+
+	new = new_invs->inv;
+	for (i = j = 0; i < invs->num_invs || j < to_merge->num_invs;) {
+		int cmp;
+
+		if (i < invs->num_invs && !refcount_read(&invs->inv[i].users)) {
+			i++;
+			continue;
+		}
+
+		cmp = arm_smmu_invs_cmp(invs, i, to_merge, j);
+		if (cmp < 0) {
+			*new = invs->inv[i];
+			i++;
+		} else if (cmp == 0) {
+			*new = invs->inv[i];
+			refcount_inc(&new->users);
+			i++;
+			j++;
+		} else {
+			*new = to_merge->inv[j];
+			refcount_set(&new->users, 1);
+			j++;
+		}
+
+		/*
+		 * Check that the new array is sorted. This also validates that
+		 * to_merge is sorted.
+		 */

Quoted 1 line · Nicolin Chen

+EXPORT_SYMBOL_IF_KUNIT(arm_smmu_invs_merge);

There's nothing really SMMU-specific about this data structure manipulation. Do you think we can abstract the invalidation array concept into a library which other IOMMU drivers could use too?

Quoted 4 lines · Will Deacon

Quoted 1 line · Nicolin Chen

+EXPORT_SYMBOL_IF_KUNIT(arm_smmu_invs_merge);

Quoted 3 lines

There's nothing really SMMU-specific about this data structure manipulation. Do you think we can abstract the invalidation array concept into a library which other IOMMU drivers could use too?

Yea, I am trying to shift to that at this moment, hopefully to combine the iotlb tag (asid/vmid) allocation as well. We do see it could be quite useful in AMD driver already.

Quoted 5 lines · Nicolin Chen

Quoted 4 lines · Will Deacon

Quoted 1 line · Nicolin Chen

+EXPORT_SYMBOL_IF_KUNIT(arm_smmu_invs_merge);

Quoted 7 lines

Quoted 3 lines

There's nothing really SMMU-specific about this data structure manipulation. Do you think we can abstract the invalidation array concept into a library which other IOMMU drivers could use too?

Yea, I am trying to shift to that at this moment, hopefully to combine the iotlb tag (asid/vmid) allocation as well. We do see it could be quite useful in AMD driver already.

I do want to see this, but also without doing work on the other drivers to integrate something it will be hard to design.

My preference is to get the basic system agreed here in ARM and then move to a generalization. There is quite a bit of work needed in the other drivers before they could use it - which makes me worry it might never happen anyhow. riscv is probably the closest, followed by intel.

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c static void arm_smmu_page_response(struct device *dev, struct iopf_fault *unused · line 1071

Quoted 11 lines · Nicolin Chen

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +11 -0

+VISIBLE_IF_KUNIT
+struct arm_smmu_invs *arm_smmu_invs_merge(struct arm_smmu_invs *invs,
+					  struct arm_smmu_invs *to_merge)
+{
+	struct arm_smmu_invs *new_invs;
+	struct arm_smmu_inv *new;
+	size_t num_trashes = 0;
+	size_t num_adds = 0;
+	size_t i, j;
+
+	for (i = j = 0; i < invs->num_invs || j < to_merge->num_invs;) {

Maybe worth having a simple iterator macro for this?

Quoted 14 lines · Will Deacon

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c static void arm_smmu_page_response(struct device *dev, struct iopf_fault *unused · line 1071

Quoted 11 lines · Nicolin Chen

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +11 -0

+VISIBLE_IF_KUNIT
+struct arm_smmu_invs *arm_smmu_invs_merge(struct arm_smmu_invs *invs,
+					  struct arm_smmu_invs *to_merge)
+{
+	struct arm_smmu_invs *new_invs;
+	struct arm_smmu_inv *new;
+	size_t num_trashes = 0;
+	size_t num_adds = 0;
+	size_t i, j;
+
+	for (i = j = 0; i < invs->num_invs || j < to_merge->num_invs;) {

Maybe worth having a simple iterator macro for this?

I added two macros:

Quoted 18 lines · Nicolin Chen

Quoted 14 lines · Will Deacon

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c static void arm_smmu_page_response(struct device *dev, struct iopf_fault *unused · line 1071

Quoted 11 lines · Nicolin Chen

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +11 -0

+VISIBLE_IF_KUNIT
+struct arm_smmu_invs *arm_smmu_invs_merge(struct arm_smmu_invs *invs,
+					  struct arm_smmu_invs *to_merge)
+{
+	struct arm_smmu_invs *new_invs;
+	struct arm_smmu_inv *new;
+	size_t num_trashes = 0;
+	size_t num_adds = 0;
+	size_t i, j;
+
+	for (i = j = 0; i < invs->num_invs || j < to_merge->num_invs;) {

Maybe worth having a simple iterator macro for this?

I added two macros:

Review

Quoted 5 lines

(quoted diff) +5 -0

+#define arm_smmu_invs_for_each_inv(invs, idx, inv)              \
+	for (idx = 0, inv = &invs->inv[0]; idx < invs->num_invs; \
+	     inv = &invs->inv[++idx])
+#define arm_smmu_invs_for_each_idx_dual(invs1, idx1, invs2, idx2) \
+	for (idx1 = idx2 = 0; idx1 < invs1->num_invs || idx2 < invs2->num_invs;)

I think pull more stuff in. Something like this:

static inline struct arm_smmu_inv * arm_smmu_invs_iter_next(struct arm_smmu_invs *invs, size_t next,
size_t *idx)

{
	while (true) {
		if (next >= invs->num_invs) {
			*idx = next;
			return NULL;
		}
		if (!refcount_read(&invs->inv[next].users)) {
			next++;
			continue;
		}
		*idx = next;
		return &invs->inv[next];
	}
}

static int arm_smmu_inv_cmp(const struct arm_smmu_inv *l,
const struct arm_smmu_inv *r)

{
	if (l->smmu != r->smmu)
		return cmp_int((uintptr_t)l->smmu, (uintptr_t)r->smmu);
	if (l->type != r->type)
		return cmp_int(l->type, r->type);
	return cmp_int(l->id, r->id);
}

static inline int arm_smmu_invs_iter_next_cmp(struct arm_smmu_invs *invs_lhs,
size_t next_lhs, size_t *idx_lhs, struct arm_smmu_invs *invs_rhs, size_t next_rhs, size_t *idx_rhs)

{
	struct arm_smmu_inv *cur_lhs =
		arm_smmu_invs_iter_next(invs_lhs, 0, idx_lhs);

	/*
	 * Compare of two sorted arrays items. If one side is past the end of
	 * the array, return the other side to let it run out the iteration.
	 */
	if (!cur_lhs)
		return -1;
	if (next_rhs >= invs_rhs->num_invs)
		return 1;
	return arm_smmu_inv_cmp(cur_lhs, &invs_rhs->inv[next_rhs]);
}

/*
* Iterates over all non-trash entries in invs. idx is a stack variable
* to store the index, cur is a stack variable of 'struct arm_smmu_inv *' */

#define arm_smmu_invs_for_each_inv(invs, idx, cur)              \
	for (cur = arm_smmu_invs_iter_next(invs, 0, &(idx)); cur; \
	     cur = arm_smmu_invs_iter_next(invs, idx + 1, &(idx)))

/*
* Iterate over two sorted arrays computing a merge sort */

#define arm_smmu_invs_for_each_merge(invs_lhs, idx_lhs, invs_rhs, idx_rhs, \
				     cmp)                                  \
	for (cmp = arm_smmu_invs_iter_next_cmp(invs_lhs, 0, &(idx_lhs),    \
					       invs_rhs, 0, &(idx_rhs));   \
	     idx_lhs < invs_lhs->num_invs || idx_rhs < invs_rhs->num_invs; \
	     cmp = arm_smmu_invs_iter_next_cmp(                            \
		     invs_lhs, idx_lhs + (cmp <= 0 ? 1 : 0), &(idx_lhs),   \
		     invs_rhs, idx_rhs + (cmp >= 0 ? 1 : 0), &(idx_rhs)))

And then change the loops computing num_trash to work directly on actual things ignoring trash:

	arm_smmu_invs_for_each_merge(invs, i, to_merge, j, cmp)
		new_size++;
	new_invs = arm_smmu_invs_alloc(new_size);

Name should probably be for_each_.... though

When a master is attached from an old domain to a new domain, it needs to build an invalidation array to delete and add the array entries from/onto the invalidation arrays of those two domains, passed via the to_merge and to_unref arguments into arm_smmu_invs_merge/unref() respectively.

Since the master->num_streams might differ across masters, a memory would have to be allocated when building an to_merge/to_unref array which might fail with -ENOMEM.

On the other hand, an attachment to arm_smmu_blocked_domain must not fail so it's the best to avoid any memory allocation in that path.

Pre-allocate a fixed size invalidation array for every master. This array will be used as a scratch to fill dynamically when building a to_merge or to_unref invs array. Sort fwspec->ids in an ascending order to fit to the arm_smmu_invs_merge() function.

Co-developed-by Jason Gunthorpe <[email protected]>

Signed-off-by Jason Gunthorpe <[email protected]>

Reviewed-by Jason Gunthorpe <[email protected]>

Signed-off-by Nicolin Chen <[email protected]>

---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h |  8 ++++++
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 27 +++++++++++++++++++++
 2 files changed, 35 insertions(+)

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +8 -0

index 757158b9ea655..7b81a82c0dfe4 100644
@@ -922,6 +922,14 @@ struct arm_smmu_master {
 	struct arm_smmu_device		*smmu;
 	struct device			*dev;
 	struct arm_smmu_stream		*streams;
+	/*
+	 * Scratch memory for a to_merge or to_unref array to build a per-domain
+	 * invalidation array. It'll be pre-allocated with enough enries for all
+	 * possible build scenarios. It can be used by only one caller at a time
+	 * until the arm_smmu_invs_merge/unref() finishes. Must be locked by the
+	 * iommu_group mutex.
+	 */
+	struct arm_smmu_invs		*build_invs;
 	struct arm_smmu_vmaster		*vmaster; /* use smmu->streams_mutex */
 	/* Locked by the iommu core using the group mutex */
 	struct arm_smmu_ctx_desc_cfg	cd_table;

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +10 -0

index 8266d0839a927..26b8492a13f20 100644
@@ -3693,12 +3693,22 @@ static int arm_smmu_init_sid_strtab(struct arm_smmu_device *smmu, u32 sid)
 	return 0;
 }
 
+static int arm_smmu_ids_cmp(const void *_l, const void *_r)
+{
+	const typeof_member(struct iommu_fwspec, ids[0]) *l = _l;
+	const typeof_member(struct iommu_fwspec, ids[0]) *r = _r;
+
+	return cmp_int(*l, *r);
+}
+
 static int arm_smmu_insert_master(struct arm_smmu_device *smmu,
 				  struct arm_smmu_master *master)
 {
 	int i;
 	int ret = 0;
 	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(master->dev);
+	bool ats_supported = dev_is_pci(master->dev) &&
+			     pci_ats_supported(to_pci_dev(master->dev));
 
 	master->streams = kcalloc(fwspec->num_ids, sizeof(*master->streams),
 				  GFP_KERNEL);

@@ -3706,6 +3716,21 @@ static int arm_smmu_insert_master(struct arm_smmu_device *smmu,
 		return -ENOMEM;
 	master->num_streams = fwspec->num_ids;

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c static int arm_smmu_insert_master(struct arm_smmu_device *smmu, · line 3727

Quoted 5 lines · Nicolin Chen

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

+		/* Put the ids into order for sorted to_merge/to_unref arrays */
+		sort_nonatomic(fwspec->ids, fwspec->num_ids,
+			       sizeof(fwspec->ids[0]), arm_smmu_ids_cmp, NULL);
+		/* ATS case adds num_ids of entries, on top of the base case */
+		master->build_invs = arm_smmu_invs_alloc(2 + fwspec->num_ids);

Although I can't point at a specific issue here, I'm nervous about mutating the 'fwspec->ids' array from within the driver, The array isn't allocated or populated directly by the driver and so I don't think we really have any business sorting it. Could we hack iommu_fwspec_add_ids() to keep the array ordered instead?

Quoted 12 lines · Will Deacon

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c static int arm_smmu_insert_master(struct arm_smmu_device *smmu, · line 3727

Quoted 5 lines · Nicolin Chen

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

+		/* Put the ids into order for sorted to_merge/to_unref arrays */
+		sort_nonatomic(fwspec->ids, fwspec->num_ids,
+			       sizeof(fwspec->ids[0]), arm_smmu_ids_cmp, NULL);
+		/* ATS case adds num_ids of entries, on top of the base case */
+		master->build_invs = arm_smmu_invs_alloc(2 + fwspec->num_ids);

Although I can't point at a specific issue here, I'm nervous about mutating the 'fwspec->ids' array from within the driver, The array isn't allocated or populated directly by the driver and so I don't think we really have any business sorting it. Could we hack iommu_fwspec_add_ids() to keep the array ordered instead?

Yea, I think it makes sense to do it in the core, once we have the data structure provided by the core as well.

Quoted 14 lines · Nicolin Chen

Quoted 12 lines · Will Deacon

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c static int arm_smmu_insert_master(struct arm_smmu_device *smmu, · line 3727

Quoted 5 lines · Nicolin Chen

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

+		/* Put the ids into order for sorted to_merge/to_unref arrays */
+		sort_nonatomic(fwspec->ids, fwspec->num_ids,
+			       sizeof(fwspec->ids[0]), arm_smmu_ids_cmp, NULL);
+		/* ATS case adds num_ids of entries, on top of the base case */
+		master->build_invs = arm_smmu_invs_alloc(2 + fwspec->num_ids);

Although I can't point at a specific issue here, I'm nervous about mutating the 'fwspec->ids' array from within the driver, The array isn't allocated or populated directly by the driver and so I don't think we really have any business sorting it. Could we hack iommu_fwspec_add_ids() to keep the array ordered instead?

Quoted 2 lines

Yea, I think it makes sense to do it in the core, once we have the data structure provided by the core as well.

I would be more worried about sorting it everywhere for every driver. I feel confident SMMUv3 doesn't use it, but something really old and embedded focused like tegra or omap, IDK.

So I wouldn't propose to change iommu_fwspec_add_ids().

If you want to be conservative then the thing to do is sort the master->streams that arm_smmu_insert_master() copies the fwspec into. It just has to be sorted prior to feeding it into the rbtree.

Then consistently use master->streams as the sorted list.

Quoted 15 lines · Jason Gunthorpe

Quoted 14 lines · Nicolin Chen

Quoted 12 lines · Will Deacon

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c static int arm_smmu_insert_master(struct arm_smmu_device *smmu, · line 3727

Quoted 5 lines · Nicolin Chen

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

+		/* Put the ids into order for sorted to_merge/to_unref arrays */
+		sort_nonatomic(fwspec->ids, fwspec->num_ids,
+			       sizeof(fwspec->ids[0]), arm_smmu_ids_cmp, NULL);
+		/* ATS case adds num_ids of entries, on top of the base case */
+		master->build_invs = arm_smmu_invs_alloc(2 + fwspec->num_ids);

Although I can't point at a specific issue here, I'm nervous about mutating the 'fwspec->ids' array from within the driver, The array isn't allocated or populated directly by the driver and so I don't think we really have any business sorting it. Could we hack iommu_fwspec_add_ids() to keep the array ordered instead?

Quoted 8 lines

Quoted 2 lines

Yea, I think it makes sense to do it in the core, once we have the data structure provided by the core as well.

I would be more worried about sorting it everywhere for every driver. I feel confident SMMUv3 doesn't use it, but something really old and embedded focused like tegra or omap, IDK.

So I wouldn't propose to change iommu_fwspec_add_ids().

Perhaps a different helper iommu_fwspec_add_ids_sorted()? Drivers can choose to use the sorted version, when they want to implement the invalidation array. And SMMU can be the first only caller.

Quoted 5 lines

If you want to be conservative then the thing to do is sort the master->streams that arm_smmu_insert_master() copies the fwspec into. It just has to be sorted prior to feeding it into the rbtree.

Then consistently use master->streams as the sorted list.

How about kmemdup() an local id array to bridge betwen fwspec->ids and rbtree?

Quoted 5 lines

If you want to be conservative then the thing to do is sort the master->streams that arm_smmu_insert_master() copies the fwspec into. It just has to be sorted prior to feeding it into the rbtree.

Then consistently use master->streams as the sorted list.

How about kmemdup() an local id array to bridge betwen fwspec->ids and rbtree?

Update the invs array with the invalidations required by each domain type during attachment operations.

Only an SVA domain or a paging domain will have an invs array:
a. SVA domain will add an INV_TYPE_S1_ASID per SMMU and an INV_TYPE_ATS
per SID

b. Non-nesting-parent paging domain with no ATS-enabled master will add
a single INV_TYPE_S1_ASID or INV_TYPE_S2_VMID per SMMU

c. Non-nesting-parent paging domain with ATS-enabled master(s) will do
(b) and add an INV_TYPE_ATS per SID

d. Nesting-parent paging domain will add an INV_TYPE_S2_VMID followed by
an INV_TYPE_S2_VMID_S1_CLEAR per vSMMU. For an ATS-enabled master, it will add an INV_TYPE_ATS_FULL per SID

Note that case #d prepares for a future implementation of VMID allocation which requires a followup series for S2 domain sharing. So when a nesting parent domain is attached through a vSMMU instance using a nested domain. VMID will be allocated per vSMMU instance v.s. currectly per S2 domain.

The per-domain invalidation is not needed until the domain is attached to a master (when it starts to possibly use TLB). This will make it possible to attach the domain to multiple SMMUs and avoid unnecessary invalidation overhead during teardown if no STEs/CDs refer to the domain. It also means that when the last device is detached, the old domain must flush its ASID or VMID, since any new iommu_unmap() call would not trigger invalidations given an empty domain->invs array.

Introduce some arm_smmu_invs helper functions for building scratch arrays, preparing and installing old/new domain's invalidation arrays.

Co-developed-by Jason Gunthorpe <[email protected]>

Signed-off-by Jason Gunthorpe <[email protected]>

Reviewed-by Jason Gunthorpe <[email protected]>

Signed-off-by Nicolin Chen <[email protected]>

---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h |  17 ++
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 242 +++++++++++++++++++-
 2 files changed, 258 insertions(+), 1 deletion(-)

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +0 -0

index 7b81a82c0dfe4..5899429a514ab 100644

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +11 -0

+ *            for old domain, this is the master->build_invs to pass in as the
+ *            to_unref argument to an arm_smmu_invs_unref() call
+ */
+struct arm_smmu_inv_state {
+	struct arm_smmu_invs __rcu **invs_ptr;
+	struct arm_smmu_invs *old_invs;
+	struct arm_smmu_invs *new_invs;
+};
+
 struct arm_smmu_attach_state {
 	/* Inputs */
 	struct iommu_domain *old_domain;

@@ -1103,6 +1118,8 @@ struct arm_smmu_attach_state {
 	ioasid_t ssid;
 	/* Resulting state */
 	struct arm_smmu_vmaster *vmaster;
+	struct arm_smmu_inv_state old_domain_invst;
+	struct arm_smmu_inv_state new_domain_invst;
 	bool ats_enabled;
 };
 

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +32 -0

index 26b8492a13f20..22875a951488d 100644
@@ -3058,6 +3058,97 @@ static void arm_smmu_disable_iopf(struct arm_smmu_master *master,
 		iopf_queue_remove_device(master->smmu->evtq.iopf, master->dev);
 }
 
+/*
+ * Use the preallocated scratch array at master->build_invs, to build a to_merge
+ * or to_unref array, to pass into a following arm_smmu_invs_merge/unref() call.
+ *
+ * Do not free the returned invs array. It is reused, and will be overwritten by
+ * the next arm_smmu_master_build_invs() call.
+ */
+static struct arm_smmu_invs *
+arm_smmu_master_build_invs(struct arm_smmu_master *master, bool ats_enabled,
+			   ioasid_t ssid, struct arm_smmu_domain *smmu_domain)
+{
+	const bool e2h = master->smmu->features & ARM_SMMU_FEAT_E2H;
+	struct arm_smmu_invs *build_invs = master->build_invs;
+	const bool nesting = smmu_domain->nest_parent;
+	struct arm_smmu_inv *cur;
+
+	iommu_group_mutex_assert(master->dev);
+
+	cur = build_invs->inv;
+
+	switch (smmu_domain->stage) {
+	case ARM_SMMU_DOMAIN_SVA:
+	case ARM_SMMU_DOMAIN_S1:
+		*cur = (struct arm_smmu_inv){
+			.smmu = master->smmu,
+			.type = INV_TYPE_S1_ASID,
+			.id = smmu_domain->cd.asid,
+			.size_opcode = e2h ? CMDQ_OP_TLBI_EL2_VA :
+					     CMDQ_OP_TLBI_NH_VA,
+			.nsize_opcode = e2h ? CMDQ_OP_TLBI_EL2_ASID :
+					      CMDQ_OP_TLBI_NH_ASID
+		};

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +118 -0

+	default:
+		WARN_ON(true);
+		return NULL;
+	}
+
+	/* Range-based invalidation requires the leaf pgsize for calculation */
+	if (master->smmu->features & ARM_SMMU_FEAT_RANGE_INV)
+		cur->pgsize = __ffs(smmu_domain->domain.pgsize_bitmap);
+	cur++;
+
+	/* All the nested S1 ASIDs have to be flushed when S2 parent changes */
+	if (nesting) {
+		*cur = (struct arm_smmu_inv){
+			.smmu = master->smmu,
+			.type = INV_TYPE_S2_VMID_S1_CLEAR,
+			.id = smmu_domain->s2_cfg.vmid,
+			.size_opcode = CMDQ_OP_TLBI_NH_ALL,
+			.nsize_opcode = CMDQ_OP_TLBI_NH_ALL,
+		};
+		cur++;
+	}
+
+	if (ats_enabled) {
+		size_t i;
+
+		for (i = 0; i < master->num_streams; i++) {
+			/*
+			 * If an S2 used as a nesting parent is changed we have
+			 * no option but to completely flush the ATC.
+			 */
+			*cur = (struct arm_smmu_inv){
+				.smmu = master->smmu,
+				.type = nesting ? INV_TYPE_ATS_FULL :
+						  INV_TYPE_ATS,
+				.id = master->streams[i].id,
+				.ssid = ssid,
+				.size_opcode = CMDQ_OP_ATC_INV,
+				.nsize_opcode = CMDQ_OP_ATC_INV,
+			};
+			cur++;
+		}
+	}
+
+	/* Note this build_invs must have been sorted */
+
+	build_invs->num_invs = cur - build_invs->inv;
+	return build_invs;
+}
+
 static void arm_smmu_remove_master_domain(struct arm_smmu_master *master,
 					  struct iommu_domain *domain,
 					  ioasid_t ssid)

@@ -3087,6 +3178,146 @@ static void arm_smmu_remove_master_domain(struct arm_smmu_master *master,
 	kfree(master_domain);
 }
 
+/*
+ * During attachment, the updates of the two domain->invs arrays are sequenced:
+ *  1. new domain updates its invs array, merging master->build_invs
+ *  2. new domain starts to include the master during its invalidation
+ *  3. master updates its STE switching from the old domain to the new domain
+ *  4. old domain still includes the master during its invalidation
+ *  5. old domain updates its invs array, unreferencing master->build_invs
+ *
+ * For 1 and 5, prepare the two updated arrays in advance, handling any changes
+ * that can possibly failure. So the actual update of either 1 or 5 won't fail.
+ * arm_smmu_asid_lock ensures that the old invs in the domains are intact while
+ * we are sequencing to update them.
+ */
+static int arm_smmu_attach_prepare_invs(struct arm_smmu_attach_state *state,
+					struct arm_smmu_domain *new_smmu_domain)
+{
+	struct arm_smmu_domain *old_smmu_domain =
+		to_smmu_domain_devices(state->old_domain);
+	struct arm_smmu_master *master = state->master;
+	ioasid_t ssid = state->ssid;
+
+	/* A re-attach case doesn't need to update invs array */
+	if (new_smmu_domain == old_smmu_domain)
+		return 0;
+
+	/*
+	 * At this point a NULL domain indicates the domain doesn't use the
+	 * IOTLB, see to_smmu_domain_devices().
+	 */
+	if (new_smmu_domain) {
+		struct arm_smmu_inv_state *invst = &state->new_domain_invst;
+		struct arm_smmu_invs *build_invs;
+
+		invst->invs_ptr = &new_smmu_domain->invs;
+		invst->old_invs = rcu_dereference_protected(
+			new_smmu_domain->invs,
+			lockdep_is_held(&arm_smmu_asid_lock));
+		build_invs = arm_smmu_master_build_invs(
+			master, state->ats_enabled, ssid, new_smmu_domain);
+		if (!build_invs)
+			return -EINVAL;
+
+		invst->new_invs =
+			arm_smmu_invs_merge(invst->old_invs, build_invs);
+		if (IS_ERR(invst->new_invs))
+			return PTR_ERR(invst->new_invs);
+	}
+
+	if (old_smmu_domain) {
+		struct arm_smmu_inv_state *invst = &state->old_domain_invst;
+
+		invst->invs_ptr = &old_smmu_domain->invs;
+		invst->old_invs = rcu_dereference_protected(
+			old_smmu_domain->invs,
+			lockdep_is_held(&arm_smmu_asid_lock));
+		/* For old_smmu_domain, new_invs points to master->build_invs */
+		invst->new_invs = arm_smmu_master_build_invs(
+			master, master->ats_enabled, ssid, old_smmu_domain);
+	}
+
+	return 0;
+}
+
+/* Must be installed before arm_smmu_install_ste_for_dev() */
+static void
+arm_smmu_install_new_domain_invs(struct arm_smmu_attach_state *state)
+{
+	struct arm_smmu_inv_state *invst = &state->new_domain_invst;
+

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +48 -0

+	kfree_rcu(invst->old_invs, rcu);
+}
+
+/*
+ * When an array entry's users count reaches zero, it means the ASID/VMID is no
+ * longer being invalidated by map/unmap and must be cleaned. The rule is that
+ * all ASIDs/VMIDs not in an invalidation array are left cleared in the IOTLB.
+ */
+static void arm_smmu_invs_flush_iotlb_tags(struct arm_smmu_inv *inv)
+{
+	struct arm_smmu_cmdq_ent cmd = {};
+
+	switch (inv->type) {
+	case INV_TYPE_S1_ASID:
+		cmd.tlbi.asid = inv->id;
+		break;
+	case INV_TYPE_S2_VMID:
+		/* S2_VMID using nsize_opcode covers S2_VMID_S1_CLEAR */
+		cmd.tlbi.vmid = inv->id;
+		break;
+	default:
+		return;
+	}
+
+	cmd.opcode = inv->nsize_opcode;
+	arm_smmu_cmdq_issue_cmd_with_sync(inv->smmu, &cmd);
+}
+
+/* Should be installed after arm_smmu_install_ste_for_dev() */
+static void
+arm_smmu_install_old_domain_invs(struct arm_smmu_attach_state *state)
+{
+	struct arm_smmu_inv_state *invst = &state->old_domain_invst;
+	struct arm_smmu_invs *old_invs = invst->old_invs;
+	struct arm_smmu_invs *new_invs;
+	size_t num_trashes;
+
+	lockdep_assert_held(&arm_smmu_asid_lock);
+
+	if (!invst->invs_ptr)
+		return;
+
+	num_trashes = arm_smmu_invs_unref(old_invs, invst->new_invs,
+					  arm_smmu_invs_flush_iotlb_tags);
+	if (!num_trashes)
+		return;
+
+	new_invs = arm_smmu_invs_purge(old_invs, num_trashes);

Quoted 17 lines

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

+	switch (smmu_domain->stage) {
+	case ARM_SMMU_DOMAIN_SVA:
+	case ARM_SMMU_DOMAIN_S1:
+		*cur = (struct arm_smmu_inv){
+			.smmu = master->smmu,
+			.type = INV_TYPE_S1_ASID,
+			.id = smmu_domain->cd.asid,
+			.size_opcode = e2h ? CMDQ_OP_TLBI_EL2_VA :
+					     CMDQ_OP_TLBI_NH_VA,
+			.nsize_opcode = e2h ? CMDQ_OP_TLBI_EL2_ASID :
+					      CMDQ_OP_TLBI_NH_ASID
+		};
+		break;
+	case ARM_SMMU_DOMAIN_S2:
+		*cur = (struct arm_smmu_inv){
+			.smmu = master->smmu,
+			.type = INV_TYPE_S2_VMID,

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c static void arm_smmu_disable_iopf(struct arm_smmu_master *master, · line 3102

Quoted 5 lines

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

+			.id = smmu_domain->s2_cfg.vmid,
+			.size_opcode = CMDQ_OP_TLBI_S2_IPA,
+			.nsize_opcode = CMDQ_OP_TLBI_S12_VMALL,
+		};
+		break;

Having a helper to add an invalidation command would make this a little more compact and you could also check against the size of the array.

Quoted 11 lines

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +11 -0

+/* Must be installed before arm_smmu_install_ste_for_dev() */
+static void
+arm_smmu_install_new_domain_invs(struct arm_smmu_attach_state *state)
+{
+	struct arm_smmu_inv_state *invst = &state->new_domain_invst;
+
+	if (!invst->invs_ptr)
+		return;
+
+	rcu_assign_pointer(*invst->invs_ptr, invst->new_invs);
+	/*

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c static void arm_smmu_remove_master_domain(struct arm_smmu_master *master, · line 3259

Quoted 5 lines

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

+	 * We are committed to updating the STE. Ensure the invalidation array
+	 * is visable to concurrent map/unmap threads, and acquire any racying
+	 * IOPTE updates.
+	 */
+	smp_mb();

Sorry, but the comment hasn't really helped me here. We're ordering the publishing of the invalidation array above before ... what?

Quoted 9 lines · Will Deacon

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c static void arm_smmu_remove_master_domain(struct arm_smmu_master *master, · line 3259

Quoted 6 lines

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

+	/*
+	 * We are committed to updating the STE. Ensure the invalidation array
+	 * is visable to concurrent map/unmap threads, and acquire any racying
+	 * IOPTE updates.
+	 */
+	smp_mb();

Sorry, but the comment hasn't really helped me here. We're ordering the publishing of the invalidation array above before ... what?

"concurrent map/unmap threads" means other threads calling arm_smmu_iotlb_sync().. I think the matching comment describing this and introducing the pair'd smp_mb() is in a later patch.

Nicolin maybe this barrier line should be moved to be added in the patch that add's the pair'd barrier and description?

Quoted 17 lines · Will Deacon

Quoted 17 lines

(quoted diff) +17 -0

+	switch (smmu_domain->stage) {
+	case ARM_SMMU_DOMAIN_SVA:
+	case ARM_SMMU_DOMAIN_S1:
+		*cur = (struct arm_smmu_inv){
+			.smmu = master->smmu,
+			.type = INV_TYPE_S1_ASID,
+			.id = smmu_domain->cd.asid,
+			.size_opcode = e2h ? CMDQ_OP_TLBI_EL2_VA :
+					     CMDQ_OP_TLBI_NH_VA,
+			.nsize_opcode = e2h ? CMDQ_OP_TLBI_EL2_ASID :
+					      CMDQ_OP_TLBI_NH_ASID
+		};
+		break;
+	case ARM_SMMU_DOMAIN_S2:
+		*cur = (struct arm_smmu_inv){
+			.smmu = master->smmu,
+			.type = INV_TYPE_S2_VMID,

Quoted 8 lines

Review

Quoted 5 lines

(quoted diff) +5 -0

+			.id = smmu_domain->s2_cfg.vmid,
+			.size_opcode = CMDQ_OP_TLBI_S2_IPA,
+			.nsize_opcode = CMDQ_OP_TLBI_S12_VMALL,
+		};
+		break;

Having a helper to add an invalidation command would make this a little more compact and you could also check against the size of the array.

Yeah but it makes all the parameters positional instead of nicely named..