Skip to contentSkip to conversation

iommu aux-domain APIs extensions

PATCH Superseded Lu Baolu 2 participants 8 messages Sep 22, 2020 – Sep 25, 2020

Subsystems (4)
KVM IOMMU VFIO driver Intel IOMMU (VT-d)

v5 of 6 Superseded Latest is v6 →

lore
Share & tools
lore

Lu Baolu

author maintainer
#0 lore raw

v5 cover iommu aux-domain APIs extensions

Hi Jorge and Alex,

A description of this patch series could be found here.

https://lore.kernel.org/linux-iommu/[email protected]/

This version adds some changes according to Alex's review comments.

- Add comments and naming rule for subdevices.
https://lore.kernel.org/linux-iommu/[email protected]/

- Continue detaching even no subdevice parent found.
https://lore.kernel.org/linux-iommu/[email protected]/

- Make subdev_link_device() and subdev_unlink_device() symmetrical.

Please help to review and merge.

Best regards,
baolu

Lu Baolu (5):
iommu: Add optional subdev in aux_at(de)tach ops
iommu: Add iommu_at(de)tach_subdev_group()
iommu: Add iommu_aux_get_domain_for_dev() vfio/type1: Use iommu_aux_at(de)tach_group() APIs iommu/vt-d: Add is_aux_domain support

 drivers/iommu/intel/iommu.c     | 139 +++++++++++++++++-------
 drivers/iommu/iommu.c           | 184 ++++++++++++++++++++++++++++++--
 drivers/vfio/vfio_iommu_type1.c |  43 ++------
 include/linux/intel-iommu.h     |  17 +--
 include/linux/iommu.h           |  46 +++++++-
 5 files changed, 336 insertions(+), 93 deletions(-)
Signature · 1 line
2.17.1
author maintainer
#1 lore raw

PATCH v5 1/5 iommu: Add optional subdev in aux_at(de)tach ops

In the vfio/mdev use case of aux-domain, the subdevices are created from the physical devices with IOMMU_DEV_FEAT_AUX enabled and the aux-domains are attached to the subdevices through the iommu_ops.aux_attach_dev() interface.

Current iommu_ops.aux_at(de)tach_dev() design only takes the aux-domain and the physical device as the parameters, this is insufficient if we want the vendor iommu drivers to learn the knowledge about relationships between the aux-domains and the subdevices. Add a @subdev parameter to iommu_ops.aux_at(de)tach_dev() interfaces so that a subdevice could be opt-in.

Signed-off-by Lu Baolu <[email protected]>
---
 drivers/iommu/intel/iommu.c | 10 ++++++----
 drivers/iommu/iommu.c       | 26 +++++++++++++++++---------
 include/linux/iommu.h       |  6 ++++--
 3 files changed, 27 insertions(+), 15 deletions(-)
drivers/iommu/intel/iommu.c +6 -4
index 1b7d390beb68..86142ce32f21 100644
@@ -5353,8 +5353,9 @@ static int intel_iommu_attach_device(struct iommu_domain *domain,
 	return domain_add_dev_info(to_dmar_domain(domain), dev);
 }
 
-static int intel_iommu_aux_attach_device(struct iommu_domain *domain,
-					 struct device *dev)
+static int
+intel_iommu_aux_attach_device(struct iommu_domain *domain,
+			      struct device *dev, struct device *subdev)
 {
 	int ret;
 

@@ -5374,8 +5375,9 @@ static void intel_iommu_detach_device(struct iommu_domain *domain,
 	dmar_remove_one_dev_info(dev);
 }
 
-static void intel_iommu_aux_detach_device(struct iommu_domain *domain,
-					  struct device *dev)
+static void
+intel_iommu_aux_detach_device(struct iommu_domain *domain, struct device *dev,
+			      struct device *subdev)
 {
 	aux_domain_remove_dev(to_dmar_domain(domain), dev);
 }
drivers/iommu/iommu.c +17 -9
index 6c14c88cd525..4c6e94682327 100644
@@ -2714,7 +2714,7 @@ bool iommu_dev_feature_enabled(struct device *dev, enum iommu_dev_features feat)
 EXPORT_SYMBOL_GPL(iommu_dev_feature_enabled);
 
 /*
- * Aux-domain specific attach/detach.
+ * Aux-domain specific interfaces.
  *
  * Only works if iommu_dev_feature_enabled(dev, IOMMU_DEV_FEAT_AUX) returns
  * true. Also, as long as domains are attached to a device through this

@@ -2722,36 +2722,44 @@ EXPORT_SYMBOL_GPL(iommu_dev_feature_enabled);
  * (iommu_detach_device() can't fail, so we fail when trying to re-attach).
  * This should make us safe against a device being attached to a guest as a
  * whole while there are still pasid users on it (aux and sva).
+ *
+ * Some physical devices can be configured to generate several subdevices.
+ * The modern IOMMUs support the identification and isolation of these
+ * subdevices. Hence they could be passed through to users space. VFIO/mdev
+ * provides a generic framework for subdevice passthrough. Below interfaces
+ * support such use case. Generally, among the parameters of the following
+ * aux-domain specific functions, @physdev represents a physical device,
+ * and @subdev represents a subdevice.
  */
-int iommu_aux_attach_device(struct iommu_domain *domain, struct device *dev)
+int iommu_aux_attach_device(struct iommu_domain *domain, struct device *physdev)
 {
 	int ret = -ENODEV;
 
 	if (domain->ops->aux_attach_dev)
-		ret = domain->ops->aux_attach_dev(domain, dev);
+		ret = domain->ops->aux_attach_dev(domain, physdev, NULL);
 
 	if (!ret)
-		trace_attach_device_to_domain(dev);
+		trace_attach_device_to_domain(physdev);
 
 	return ret;
 }
 EXPORT_SYMBOL_GPL(iommu_aux_attach_device);
 
-void iommu_aux_detach_device(struct iommu_domain *domain, struct device *dev)
+void iommu_aux_detach_device(struct iommu_domain *domain, struct device *physdev)
 {
 	if (domain->ops->aux_detach_dev) {
-		domain->ops->aux_detach_dev(domain, dev);
-		trace_detach_device_from_domain(dev);
+		domain->ops->aux_detach_dev(domain, physdev, NULL);
+		trace_detach_device_from_domain(physdev);
 	}
 }
 EXPORT_SYMBOL_GPL(iommu_aux_detach_device);
 
-int iommu_aux_get_pasid(struct iommu_domain *domain, struct device *dev)
+int iommu_aux_get_pasid(struct iommu_domain *domain, struct device *physdev)
 {
 	int ret = -ENODEV;
 
 	if (domain->ops->aux_get_pasid)
-		ret = domain->ops->aux_get_pasid(domain, dev);
+		ret = domain->ops->aux_get_pasid(domain, physdev);
 
 	return ret;
 }
include/linux/iommu.h +4 -2
index 2ad26d8b4ab9..56297b4bbd0e 100644
@@ -279,8 +279,10 @@ struct iommu_ops {
 	int (*dev_disable_feat)(struct device *dev, enum iommu_dev_features f);
 
 	/* Aux-domain specific attach/detach entries */
-	int (*aux_attach_dev)(struct iommu_domain *domain, struct device *dev);
-	void (*aux_detach_dev)(struct iommu_domain *domain, struct device *dev);
+	int (*aux_attach_dev)(struct iommu_domain *domain, struct device *physdev,
+			      struct device *subdev);
+	void (*aux_detach_dev)(struct iommu_domain *domain, struct device *physdev,
+			       struct device *subdev);
 	int (*aux_get_pasid)(struct iommu_domain *domain, struct device *dev);
 
 	struct iommu_sva *(*sva_bind)(struct device *dev, struct mm_struct *mm,
Signature · 6 lines
2.17.1

_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu
author maintainer
#2 lore raw

PATCH v5 2/5 iommu: Add iommu_at(de)tach_subdev_group()

In reply to Lu Baolu

This adds two new APIs for the use cases like vfio/mdev where subdevices derived from physical devices are created and put in an iommu_group. The new IOMMU API interfaces mimic the vfio_mdev_at(de)tach_domain() directly, testing whether the resulting device supports IOMMU_DEV_FEAT_AUX and using an aux vs non-aux at(de)tach.

By doing this we could

- Set the iommu_group.domain. The iommu_group.domain is private to iommu core (therefore vfio code cannot set it), but we need it set in order for iommu_get_domain_for_dev() to work with a group attached to an aux domain.

- Prefer to use the _attach_group() interfaces while the _attach_device() interfaces are relegated to special cases.

Link https://lore.kernel.org/linux-iommu/[email protected]/ open here
Link https://lore.kernel.org/linux-iommu/[email protected]/ open here
Signed-off-by Lu Baolu <[email protected]>
---
 drivers/iommu/iommu.c | 140 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/iommu.h |  20 ++++++
 2 files changed, 160 insertions(+)
drivers/iommu/iommu.c +140 -0
index 4c6e94682327..77f3a9fc4dfe 100644
@@ -2765,6 +2765,146 @@ int iommu_aux_get_pasid(struct iommu_domain *domain, struct device *physdev)
 }
 EXPORT_SYMBOL_GPL(iommu_aux_get_pasid);
 
+static int __iommu_aux_attach_device(struct iommu_domain *domain,
+				     struct device *physdev,
+				     struct device *subdev)
+{
+	int ret;
+
+	if (unlikely(!domain->ops->aux_attach_dev))
+		return -ENODEV;
+
+	ret = domain->ops->aux_attach_dev(domain, physdev, subdev);
+	if (!ret)
+		trace_attach_device_to_domain(subdev);
+
+	return ret;
+}
+
+static void __iommu_aux_detach_device(struct iommu_domain *domain,
+				      struct device *physdev,
+				      struct device *subdev)
+{
+	if (unlikely(!domain->ops->aux_detach_dev))
+		return;
+
+	domain->ops->aux_detach_dev(domain, physdev, subdev);
+	trace_detach_device_from_domain(subdev);
+}
+
+static int __iommu_attach_subdev_group(struct iommu_domain *domain,
+				       struct iommu_group *group,
+				       iommu_device_lookup_t fn)
+{
+	struct group_device *device;
+	struct device *physdev;
+	int ret = -ENODEV;
+
+	list_for_each_entry(device, &group->devices, list) {
+		physdev = fn(device->dev);
+		if (!physdev) {
+			ret = -ENODEV;
+			break;
+		}
+
+		if (iommu_dev_feature_enabled(physdev, IOMMU_DEV_FEAT_AUX))
+			ret = __iommu_aux_attach_device(domain, physdev,
+							device->dev);
+		else
+			ret = __iommu_attach_device(domain, physdev);
+
+		if (ret)
+			break;
+	}
+
+	return ret;
+}
+
+static void __iommu_detach_subdev_group(struct iommu_domain *domain,
+					struct iommu_group *group,
+					iommu_device_lookup_t fn)
+{
+	struct group_device *device;
+	struct device *physdev;
+
+	list_for_each_entry(device, &group->devices, list) {
+		physdev = fn(device->dev);
+		/*
+		 * We still have an entirely separate detach interface, so
+		 * iterate each group device to avoid an inconsistent state.
+		 */
+		if (!physdev)
+			continue;
+
+		if (iommu_dev_feature_enabled(physdev, IOMMU_DEV_FEAT_AUX))
+			__iommu_aux_detach_device(domain, physdev, device->dev);
+		else
+			__iommu_detach_device(domain, physdev);
+	}
+}
+
+/**
+ * iommu_attach_subdev_group - attach domain to an iommu_group which
+ *			       contains subdevices.
+ *
+ * @domain: domain
+ * @group:  iommu_group which contains subdevices
+ * @fn:     callback for each subdevice in the @iommu_group to retrieve the
+ *          physical device where the subdevice was created from.
+ *
+ * Returns 0 on success, or an error value.
+ */
+int iommu_attach_subdev_group(struct iommu_domain *domain,
+			      struct iommu_group *group,
+			      iommu_device_lookup_t fn)
+{
+	int ret = -ENODEV;
+
+	mutex_lock(&group->mutex);
+	if (group->domain) {
+		ret = -EBUSY;
+		goto unlock_out;
+	}
+
+	ret = __iommu_attach_subdev_group(domain, group, fn);
+	if (ret)
+		__iommu_detach_subdev_group(domain, group, fn);
+	else
+		group->domain = domain;
+
+unlock_out:
+	mutex_unlock(&group->mutex);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(iommu_attach_subdev_group);
+
+/**
+ * iommu_detach_subdev_group - detach domain from an iommu_group which
+ *			       contains subdevices
+ *
+ * @domain: domain
+ * @group:  iommu_group which contains subdevices
+ * @fn:     callback for each subdevice in the @iommu_group to retrieve the
+ *          physical device where the subdevice was created from.
+ *
+ * The domain must have been attached to @group via iommu_attach_subdev_group().
+ */
+void iommu_detach_subdev_group(struct iommu_domain *domain,
+			       struct iommu_group *group,
+			       iommu_device_lookup_t fn)
+{
+	mutex_lock(&group->mutex);
+	if (!group->domain)
+		goto unlock_out;
+
+	__iommu_detach_subdev_group(domain, group, fn);
+	group->domain = NULL;
+
+unlock_out:
+	mutex_unlock(&group->mutex);
+}
+EXPORT_SYMBOL_GPL(iommu_detach_subdev_group);
+
 /**
  * iommu_sva_bind_device() - Bind a process address space to a device
  * @dev: the device
include/linux/iommu.h +20 -0
index 56297b4bbd0e..4df17a8f4540 100644
@@ -48,6 +48,7 @@ struct iommu_fault_event;
 typedef int (*iommu_fault_handler_t)(struct iommu_domain *,
 			struct device *, unsigned long, int, void *);
 typedef int (*iommu_dev_fault_handler_t)(struct iommu_fault *, void *);
+typedef struct device *(*iommu_device_lookup_t)(struct device *);
 
 struct iommu_domain_geometry {
 	dma_addr_t aperture_start; /* First address that can be mapped    */

@@ -631,6 +632,12 @@ bool iommu_dev_feature_enabled(struct device *dev, enum iommu_dev_features f);
 int iommu_aux_attach_device(struct iommu_domain *domain, struct device *dev);
 void iommu_aux_detach_device(struct iommu_domain *domain, struct device *dev);
 int iommu_aux_get_pasid(struct iommu_domain *domain, struct device *dev);
+int iommu_attach_subdev_group(struct iommu_domain *domain,
+			      struct iommu_group *group,
+			      iommu_device_lookup_t fn);
+void iommu_detach_subdev_group(struct iommu_domain *domain,
+			       struct iommu_group *group,
+			       iommu_device_lookup_t fn);
 
 struct iommu_sva *iommu_sva_bind_device(struct device *dev,
 					struct mm_struct *mm,

@@ -1019,6 +1026,19 @@ iommu_aux_get_pasid(struct iommu_domain *domain, struct device *dev)
 	return -ENODEV;
 }
 
+static inline int
+iommu_attach_subdev_group(struct iommu_domain *domain, struct iommu_group *group,
+			  iommu_device_lookup_t fn)
+{
+	return -ENODEV;
+}
+
+static inline void
+iommu_detach_subdev_group(struct iommu_domain *domain, struct iommu_group *group,
+			  iommu_device_lookup_t fn)
+{
+}
+
 static inline struct iommu_sva *
 iommu_sva_bind_device(struct device *dev, struct mm_struct *mm, void *drvdata)
 {
Signature · 6 lines
2.17.1

_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu
author maintainer
#3 lore raw

PATCH v5 3/5 iommu: Add iommu_aux_get_domain_for_dev()

In reply to Lu Baolu

The device driver needs an API to get its aux-domain. A typical usage scenario is:

        unsigned long pasid;
        struct iommu_domain *domain;
        struct device *dev = mdev_dev(mdev);
        struct device *iommu_device = vfio_mdev_get_iommu_device(dev);

        domain = iommu_aux_get_domain_for_dev(dev);
        if (!domain)
                return -ENODEV;

        pasid = iommu_aux_get_pasid(domain, iommu_device);
        if (pasid <= 0)
                return -EINVAL;

         /* Program the device context */
         ....

This adds an API for such use case.

Suggested-by Alex Williamson <[email protected]>
Link https://lore.kernel.org/linux-iommu/[email protected]/ open here
Signed-off-by Lu Baolu <[email protected]>
---
 drivers/iommu/iommu.c | 18 ++++++++++++++++++
 include/linux/iommu.h | 20 ++++++++++++++++++++
 2 files changed, 38 insertions(+)
drivers/iommu/iommu.c +18 -0
index 77f3a9fc4dfe..b6f24bf3bf87 100644
@@ -2905,6 +2905,24 @@ void iommu_detach_subdev_group(struct iommu_domain *domain,
 }
 EXPORT_SYMBOL_GPL(iommu_detach_subdev_group);
 
+struct iommu_domain *iommu_aux_get_domain_for_dev(struct device *subdev)
+{
+	struct iommu_domain *domain = NULL;
+	struct iommu_group *group;
+
+	group = iommu_group_get(subdev);
+	if (!group || !group->domain)
+		return NULL;
+
+	if (domain_is_aux(group->domain, subdev))
+		domain = group->domain;
+
+	iommu_group_put(group);
+
+	return domain;
+}
+EXPORT_SYMBOL_GPL(iommu_aux_get_domain_for_dev);
+
 /**
  * iommu_sva_bind_device() - Bind a process address space to a device
  * @dev: the device
include/linux/iommu.h +20 -0
index 4df17a8f4540..652549de388a 100644
@@ -120,6 +120,7 @@ enum iommu_attr {
 	DOMAIN_ATTR_NESTING,	/* two stages of translation */
 	DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE,
 	DOMAIN_ATTR_MAX,
+	DOMAIN_ATTR_IS_AUX,
 };
 
 /* These are the possible reserved region types */

@@ -622,6 +623,12 @@ static inline void dev_iommu_priv_set(struct device *dev, void *priv)
 	dev->iommu->priv = priv;
 }
 
+static inline bool
+domain_is_aux(struct iommu_domain *domain, struct device *subdev)
+{
+	return iommu_domain_get_attr(domain, DOMAIN_ATTR_IS_AUX, subdev);
+}
+
 int iommu_probe_device(struct device *dev);
 void iommu_release_device(struct device *dev);
 

@@ -638,6 +645,7 @@ int iommu_attach_subdev_group(struct iommu_domain *domain,
 void iommu_detach_subdev_group(struct iommu_domain *domain,
 			       struct iommu_group *group,
 			       iommu_device_lookup_t fn);
+struct iommu_domain *iommu_aux_get_domain_for_dev(struct device *subdev);
 
 struct iommu_sva *iommu_sva_bind_device(struct device *dev,
 					struct mm_struct *mm,

@@ -1039,6 +1047,18 @@ iommu_detach_subdev_group(struct iommu_domain *domain, struct iommu_group *group
 {
 }
 
+static inline bool
+domain_is_aux(struct iommu_domain *domain, struct device *subdev)
+{
+	return false;
+}
+
+static inline struct iommu_domain *
+iommu_aux_get_domain_for_dev(struct device *subdev)
+{
+	return NULL;
+}
+
 static inline struct iommu_sva *
 iommu_sva_bind_device(struct device *dev, struct mm_struct *mm, void *drvdata)
 {
Signature · 6 lines
2.17.1

_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu
author maintainer
#4 lore raw

PATCH v5 4/5 vfio/type1: Use iommu_aux_at(de)tach_group() APIs

In reply to Lu Baolu

Replace iommu_aux_at(de)tach_device() with iommu_at(de)tach_subdev_group().

Signed-off-by Lu Baolu <[email protected]>
---
 drivers/vfio/vfio_iommu_type1.c | 43 +++++----------------------------
 1 file changed, 6 insertions(+), 37 deletions(-)
drivers/vfio/vfio_iommu_type1.c +6 -37
index c255a6683f31..869a71c258d1 100644
@@ -1690,45 +1690,13 @@ static struct device *vfio_mdev_get_iommu_device(struct device *dev)
 	return NULL;
 }
 
-static int vfio_mdev_attach_domain(struct device *dev, void *data)
-{
-	struct iommu_domain *domain = data;
-	struct device *iommu_device;
-
-	iommu_device = vfio_mdev_get_iommu_device(dev);
-	if (iommu_device) {
-		if (iommu_dev_feature_enabled(iommu_device, IOMMU_DEV_FEAT_AUX))
-			return iommu_aux_attach_device(domain, iommu_device);
-		else
-			return iommu_attach_device(domain, iommu_device);
-	}
-
-	return -EINVAL;
-}
-
-static int vfio_mdev_detach_domain(struct device *dev, void *data)
-{
-	struct iommu_domain *domain = data;
-	struct device *iommu_device;
-
-	iommu_device = vfio_mdev_get_iommu_device(dev);
-	if (iommu_device) {
-		if (iommu_dev_feature_enabled(iommu_device, IOMMU_DEV_FEAT_AUX))
-			iommu_aux_detach_device(domain, iommu_device);
-		else
-			iommu_detach_device(domain, iommu_device);
-	}
-
-	return 0;
-}
-
 static int vfio_iommu_attach_group(struct vfio_domain *domain,
 				   struct vfio_group *group)
 {
 	if (group->mdev_group)
-		return iommu_group_for_each_dev(group->iommu_group,
-						domain->domain,
-						vfio_mdev_attach_domain);
+		return iommu_attach_subdev_group(domain->domain,
+						 group->iommu_group,
+						 vfio_mdev_get_iommu_device);
 	else
 		return iommu_attach_group(domain->domain, group->iommu_group);
 }

@@ -1737,8 +1705,9 @@ static void vfio_iommu_detach_group(struct vfio_domain *domain,
 				    struct vfio_group *group)
 {
 	if (group->mdev_group)
-		iommu_group_for_each_dev(group->iommu_group, domain->domain,
-					 vfio_mdev_detach_domain);
+		iommu_detach_subdev_group(domain->domain,
+					  group->iommu_group,
+					  vfio_mdev_get_iommu_device);
 	else
 		iommu_detach_group(domain->domain, group->iommu_group);
 }
Signature · 6 lines
2.17.1

_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu
author maintainer
#5 lore raw

PATCH v5 5/5 iommu/vt-d: Add is_aux_domain support

In reply to Lu Baolu

With subdevice information opt-in through iommu_ops.aux_at(de)tach_dev() interfaces, the vendor iommu driver is able to learn the knowledge about the relationships between the subdevices and the aux-domains. Implement is_aux_domain() support based on the relationship knowledges.

Signed-off-by Lu Baolu <[email protected]>
---
 drivers/iommu/intel/iommu.c | 129 ++++++++++++++++++++++++++----------
 include/linux/intel-iommu.h |  17 +++--
 2 files changed, 105 insertions(+), 41 deletions(-)
drivers/iommu/intel/iommu.c +94 -35
index 86142ce32f21..9ba314e2872f 100644
@@ -334,6 +334,8 @@ static int intel_iommu_attach_device(struct iommu_domain *domain,
 				     struct device *dev);
 static phys_addr_t intel_iommu_iova_to_phys(struct iommu_domain *domain,
 					    dma_addr_t iova);
+static bool intel_iommu_dev_feat_enabled(struct device *dev,
+					 enum iommu_dev_features feat);
 
 #ifdef CONFIG_INTEL_IOMMU_DEFAULT_ON
 int dmar_disabled = 0;

@@ -1881,6 +1883,7 @@ static struct dmar_domain *alloc_domain(int flags)
 		domain->flags |= DOMAIN_FLAG_USE_FIRST_LEVEL;
 	domain->has_iotlb_device = false;
 	INIT_LIST_HEAD(&domain->devices);
+	INIT_LIST_HEAD(&domain->subdevices);
 
 	return domain;
 }

@@ -2629,7 +2632,7 @@ static struct dmar_domain *dmar_insert_one_dev_info(struct intel_iommu *iommu,
 	info->iommu = iommu;
 	info->pasid_table = NULL;
 	info->auxd_enabled = 0;
-	INIT_LIST_HEAD(&info->auxiliary_domains);
+	INIT_LIST_HEAD(&info->subdevices);
 
 	if (dev && dev_is_pci(dev)) {
 		struct pci_dev *pdev = to_pci_dev(info->dev);

@@ -5152,21 +5155,28 @@ static void intel_iommu_domain_free(struct iommu_domain *domain)
 		domain_exit(to_dmar_domain(domain));
 }
 
-/*
- * Check whether a @domain could be attached to the @dev through the
- * aux-domain attach/detach APIs.
- */
-static inline bool
-is_aux_domain(struct device *dev, struct iommu_domain *domain)
+/* Lookup subdev_info in the domain's subdevice siblings. */
+static struct subdev_info *
+subdev_lookup_domain(struct dmar_domain *domain, struct device *dev,
+		     struct device *subdev)
 {
-	struct device_domain_info *info = get_domain_info(dev);
+	struct subdev_info *sinfo = NULL, *tmp;
+
+	assert_spin_locked(&device_domain_lock);
+
+	list_for_each_entry(tmp, &domain->subdevices, link_domain) {
+		if ((!dev || tmp->pdev == dev) && tmp->dev == subdev) {
+			sinfo = tmp;
+			break;
+		}
+	}
 
-	return info && info->auxd_enabled &&
-			domain->type == IOMMU_DOMAIN_UNMANAGED;
+	return sinfo;
 }
 
-static void auxiliary_link_device(struct dmar_domain *domain,
-				  struct device *dev)
+static void
+subdev_link_device(struct dmar_domain *domain, struct device *dev,
+		   struct subdev_info *sinfo)
 {
 	struct device_domain_info *info = get_domain_info(dev);
 

@@ -5174,12 +5184,13 @@ static void auxiliary_link_device(struct dmar_domain *domain,
 	if (WARN_ON(!info))
 		return;
 
-	domain->auxd_refcnt++;
-	list_add(&domain->auxd, &info->auxiliary_domains);
+	list_add(&sinfo->link_phys, &info->subdevices);
+	list_add(&sinfo->link_domain, &domain->subdevices);
 }
 
-static void auxiliary_unlink_device(struct dmar_domain *domain,
-				    struct device *dev)
+static void
+subdev_unlink_device(struct dmar_domain *domain, struct device *dev,
+		     struct subdev_info *sinfo)
 {
 	struct device_domain_info *info = get_domain_info(dev);
 

@@ -5187,24 +5198,26 @@ static void auxiliary_unlink_device(struct dmar_domain *domain,
 	if (WARN_ON(!info))
 		return;
 
-	list_del(&domain->auxd);
-	domain->auxd_refcnt--;
-
-	if (!domain->auxd_refcnt && domain->default_pasid > 0)
-		ioasid_free(domain->default_pasid);
+	list_del(&sinfo->link_phys);
+	list_del(&sinfo->link_domain);
 }
 
-static int aux_domain_add_dev(struct dmar_domain *domain,
-			      struct device *dev)
+static int aux_domain_add_dev(struct dmar_domain *domain, struct device *dev,
+			      struct device *subdev)
 {
 	int ret;
 	unsigned long flags;
 	struct intel_iommu *iommu;
+	struct subdev_info *sinfo;
 
 	iommu = device_to_iommu(dev, NULL, NULL);
 	if (!iommu)
 		return -ENODEV;
 
+	sinfo = kzalloc(sizeof(*sinfo), GFP_KERNEL);
+	if (!sinfo)
+		return -ENOMEM;
+
 	if (domain->default_pasid <= 0) {
 		int pasid;
 

@@ -5214,7 +5227,8 @@ static int aux_domain_add_dev(struct dmar_domain *domain,
 				     NULL);
 		if (pasid == INVALID_IOASID) {
 			pr_err("Can't allocate default pasid\n");
-			return -ENODEV;
+			ret = -ENODEV;
+			goto pasid_failed;
 		}
 		domain->default_pasid = pasid;
 	}

@@ -5240,7 +5254,10 @@ static int aux_domain_add_dev(struct dmar_domain *domain,
 		goto table_failed;
 	spin_unlock(&iommu->lock);
 
-	auxiliary_link_device(domain, dev);
+	sinfo->dev = subdev;
+	sinfo->domain = domain;
+	sinfo->pdev = dev;
+	subdev_link_device(domain, dev, sinfo);
 
 	spin_unlock_irqrestore(&device_domain_lock, flags);
 

@@ -5251,27 +5268,36 @@ static int aux_domain_add_dev(struct dmar_domain *domain,
 attach_failed:
 	spin_unlock(&iommu->lock);
 	spin_unlock_irqrestore(&device_domain_lock, flags);
-	if (!domain->auxd_refcnt && domain->default_pasid > 0)
+	if (list_empty(&domain->subdevices) && domain->default_pasid > 0)
 		ioasid_free(domain->default_pasid);
+pasid_failed:
+	kfree(sinfo);
 
 	return ret;
 }
 
-static void aux_domain_remove_dev(struct dmar_domain *domain,
-				  struct device *dev)
+static void
+aux_domain_remove_dev(struct dmar_domain *domain, struct device *dev,
+		      struct device *subdev)
 {
 	struct device_domain_info *info;
 	struct intel_iommu *iommu;
+	struct subdev_info *sinfo;
 	unsigned long flags;
 
-	if (!is_aux_domain(dev, &domain->domain))
+	if (!intel_iommu_dev_feat_enabled(dev, IOMMU_DEV_FEAT_AUX) ||
+	    domain->domain.type != IOMMU_DOMAIN_UNMANAGED)
 		return;
 
 	spin_lock_irqsave(&device_domain_lock, flags);
 	info = get_domain_info(dev);
 	iommu = info->iommu;
-
-	auxiliary_unlink_device(domain, dev);
+	sinfo = subdev_lookup_domain(domain, dev, subdev);
+	if (!sinfo) {
+		spin_unlock_irqrestore(&device_domain_lock, flags);
+		return;
+	}
+	subdev_unlink_device(domain, dev, sinfo);
 
 	spin_lock(&iommu->lock);
 	intel_pasid_tear_down_entry(iommu, dev, domain->default_pasid, false);

@@ -5279,6 +5305,10 @@ static void aux_domain_remove_dev(struct dmar_domain *domain,
 	spin_unlock(&iommu->lock);
 
 	spin_unlock_irqrestore(&device_domain_lock, flags);
+
+	if (list_empty(&domain->subdevices) && domain->default_pasid > 0)
+		ioasid_free(domain->default_pasid);
+	kfree(sinfo);
 }
 
 static int prepare_domain_attach_device(struct iommu_domain *domain,

@@ -5334,7 +5364,8 @@ static int intel_iommu_attach_device(struct iommu_domain *domain,
 		return -EPERM;
 	}
 
-	if (is_aux_domain(dev, domain))
+	if (intel_iommu_dev_feat_enabled(dev, IOMMU_DEV_FEAT_AUX) &&
+	    domain->type == IOMMU_DOMAIN_UNMANAGED)
 		return -EPERM;
 
 	/* normally dev is not mapped */

@@ -5359,14 +5390,15 @@ intel_iommu_aux_attach_device(struct iommu_domain *domain,
 {
 	int ret;
 
-	if (!is_aux_domain(dev, domain))
+	if (!intel_iommu_dev_feat_enabled(dev, IOMMU_DEV_FEAT_AUX) ||
+	    domain->type != IOMMU_DOMAIN_UNMANAGED)
 		return -EPERM;
 
 	ret = prepare_domain_attach_device(domain, dev);
 	if (ret)
 		return ret;
 
-	return aux_domain_add_dev(to_dmar_domain(domain), dev);
+	return aux_domain_add_dev(to_dmar_domain(domain), dev, subdev);
 }
 
 static void intel_iommu_detach_device(struct iommu_domain *domain,

@@ -5379,7 +5411,7 @@ static void
 intel_iommu_aux_detach_device(struct iommu_domain *domain, struct device *dev,
 			      struct device *subdev)
 {
-	aux_domain_remove_dev(to_dmar_domain(domain), dev);
+	aux_domain_remove_dev(to_dmar_domain(domain), dev, subdev);
 }
 
 /*

@@ -6035,6 +6067,32 @@ static bool intel_iommu_is_attach_deferred(struct iommu_domain *domain,
 	return attach_deferred(dev);
 }
 
+static int
+intel_iommu_domain_get_attr(struct iommu_domain *domain,
+			    enum iommu_attr attr, void *data)
+{
+	struct dmar_domain *dmar_domain = to_dmar_domain(domain);
+	unsigned long flags;
+	int ret;
+
+	if (domain->type != IOMMU_DOMAIN_UNMANAGED)
+		return -EINVAL;
+
+	switch (attr) {
+	case DOMAIN_ATTR_IS_AUX:
+		spin_lock_irqsave(&device_domain_lock, flags);
+		ret = !IS_ERR_OR_NULL(subdev_lookup_domain(dmar_domain,
+							   NULL, data));
+		spin_unlock_irqrestore(&device_domain_lock, flags);
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	return ret;
+}
+
 static int
 intel_iommu_domain_set_attr(struct iommu_domain *domain,
 			    enum iommu_attr attr, void *data)

@@ -6088,6 +6146,7 @@ const struct iommu_ops intel_iommu_ops = {
 	.domain_alloc		= intel_iommu_domain_alloc,
 	.domain_free		= intel_iommu_domain_free,
 	.domain_set_attr	= intel_iommu_domain_set_attr,
+	.domain_get_attr	= intel_iommu_domain_get_attr,
 	.attach_dev		= intel_iommu_attach_device,
 	.detach_dev		= intel_iommu_detach_device,
 	.aux_attach_dev		= intel_iommu_aux_attach_device,
include/linux/intel-iommu.h +11 -6
index 6a3ddaabf3f5..3b112356c6ea 100644
@@ -526,11 +526,9 @@ struct dmar_domain {
 					/* Domain ids per IOMMU. Use u16 since
 					 * domain ids are 16 bit wide according
 					 * to VT-d spec, section 9.3 */
-	unsigned int	auxd_refcnt;	/* Refcount of auxiliary attaching */
-
 	bool has_iotlb_device;
 	struct list_head devices;	/* all devices' list */
-	struct list_head auxd;		/* link to device's auxiliary list */
+	struct list_head subdevices;	/* all subdevices' list */
 	struct iova_domain iovad;	/* iova's that belong to this domain */
 
 	struct dma_pte	*pgd;		/* virtual address */

@@ -603,14 +601,21 @@ struct intel_iommu {
 	struct dmar_drhd_unit *drhd;
 };
 
+/* Per subdevice private data */
+struct subdev_info {
+	struct list_head link_phys;	/* link to phys device siblings */
+	struct list_head link_domain;	/* link to domain siblings */
+	struct device *pdev;		/* physical device derived from */
+	struct device *dev;		/* subdevice node */
+	struct dmar_domain *domain;	/* aux-domain */
+};
+
 /* PCI domain-device relationship */
 struct device_domain_info {
 	struct list_head link;	/* link to domain siblings */
 	struct list_head global; /* link to global list */
 	struct list_head table;	/* link to pasid table */
-	struct list_head auxiliary_domains; /* auxiliary domains
-					     * attached to this device
-					     */
+	struct list_head subdevices; /* subdevices sibling */
 	u32 segment;		/* PCI segment number */
 	u8 bus;			/* PCI bus number */
 	u8 devfn;		/* PCI devfn number */
Signature · 6 lines
2.17.1

_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu
maintainer
#6 lore raw
In reply to Lu Baolu
Quoted 5 lines · Lu Baolu

Hi Jorge and Alex,

A description of this patch series could be found here.

https://lore.kernel.org/linux-iommu/[email protected]/

Hmm, I am wondering if we can avoid all this hassle and special APIs by making the mdev framework more visible outside of the vfio code. There is an underlying bus implementation for mdevs, so is there a reason those can't use the standard iommu-core code to setup IOMMU mappings?

What speaks against doing:

- IOMMU drivers capable of handling mdevs register iommu-ops for the mdev_bus.

- iommu_domain_alloc() takes bus_type as parameter, so there can be special domains be allocated for mdevs.

- Group creation and domain allocation will happen automatically in the iommu-core when a new mdev is registered through device-driver core code.

- There should be no need for special iommu_aux_* APIs, as one can attach a domain directly to &mdev->dev with iommu_attach_device(domain, &mdev->dev).

Doing it this way will probably also keep the mdev-special code in VFIO small.

Regards,

Joerg

author maintainer
#7 lore raw

Hi Joerg,

Quoted 11 lines · Joerg Roedel
Quoted 5 lines · Lu Baolu

Hi Jorge and Alex,

A description of this patch series could be found here.

https://lore.kernel.org/linux-iommu/[email protected]/

Hmm, I am wondering if we can avoid all this hassle and special APIs by making the mdev framework more visible outside of the vfio code. There is an underlying bus implementation for mdevs, so is there a reason those can't use the standard iommu-core code to setup IOMMU mappings?

The original purpose of this series is to enable the device driver to retrieve the aux-domain through iommu core after iommu ops.aux_attach_dev().

The domain was allocated in vfio/mdev, but it's also needed by the device driver in mediated callbacks. The idea of this patch series is to extend the aux-API so that the domain could be saved in group->domain and get by the mediated driver through the existing iommu_get_domain_for_dev().

Back when we were developing the aux-domain, I proposed to keep the domain in vfio/mdev.

https://lore.kernel.org/linux-iommu/[email protected]/

It wasn't discussed at that time due to the lack of real consumer. Intel is now adding aux-domain support in idxd (DMA streaming accelerator) driver which becomes the first real consumer. So this problem is brought back to the table.

Quoted 13 lines

What speaks against doing:

- IOMMU drivers capable of handling mdevs register iommu-ops for the mdev_bus.

- iommu_domain_alloc() takes bus_type as parameter, so there can be special domains be allocated for mdevs.

- Group creation and domain allocation will happen automatically in the iommu-core when a new mdev is registered through device-driver core code.

Quoted 6 lines

- There should be no need for special iommu_aux_* APIs, as one can attach a domain directly to &mdev->dev with iommu_attach_device(domain, &mdev->dev).

Doing it this way will probably also keep the mdev-special code in VFIO small.

Fully understand now. Thanks for guide.

Quoted 5 lines

Regards,

Joerg

Best regards,
baolu