vfio/mdev: IOMMU aware mediated device

Sharing a physical PCI device in a finer-granularity way is becoming a consensus in the industry. IOMMU vendors are also engaging efforts to support such sharing as well as possible. Among the efforts, the capability of support finer-granularity DMA isolation is a common requirement due to the security consideration. With finer-granularity DMA isolation, all DMA requests out of or to a subset of a physical PCI device can be protected by the IOMMU. As a result, there is a request in software to attach multiple domains to a physical PCI device. One example of such use model is the Intel Scalable IOV [1] [2]. The Intel vt-d 3.0 spec [3] introduces the scalable mode which enables PASID granularity DMA isolation.

This adds the APIs to support multiple domains per device. In order to ease the discussions, we call it 'a domain in auxiliary mode' or simply 'auxiliary domain' when multiple domains are attached to a physical device.

The APIs includes:

* iommu_get_dev_attr(dev, IOMMU_DEV_ATTR_AUXD_CAPABILITY)
- Represents the ability of supporting multiple domains per device.

* iommu_get_dev_attr(dev, IOMMU_DEV_ATTR_AUXD_ENABLED)
- Checks whether the device identified by @dev is working in auxiliary mode.

* iommu_set_dev_attr(dev, IOMMU_DEV_ATTR_AUXD_ENABLE)
- Enables the multiple domains capability for the device referenced by @dev.

* iommu_set_dev_attr(dev, IOMMU_DEV_ATTR_AUXD_DISABLE)
- Disables the multiple domains capability for the device referenced by @dev.

* iommu_attach_device_aux(domain, dev)
- Attaches @domain to @dev in the auxiliary mode. Multiple domains could be attached to a single device in the auxiliary mode with each domain representing an isolated address space for an assignable subset of the device.

* iommu_detach_device_aux(domain, dev)
- Detach @domain which has been attached to @dev in the auxiliary mode.

* iommu_domain_get_attr(domain, DOMAIN_ATTR_AUXD_ID)
- Return ID used for finer-granularity DMA translation. For the Intel Scalable IOV usage model, this will be a PASID. The device which supports Scalalbe IOV needs

to writes this ID to the device register so that DMA

requests could be tagged with a right PASID prefix.

Many people involved in discussions of this design.

Kevin Tian <[email protected]>
Liu Yi L <[email protected]>
Ashok Raj <[email protected]>
Sanjay Kumar <[email protected]>
Jacob Pan <[email protected]> Alex Williamson <[email protected]> Jean-Philippe Brucker <[email protected]>

and some discussions can be found here [4].

[1] https://software.intel.com/en-us/download/intel-scalable-io-virtualization-technical-specification
[2] https://schd.ws/hosted_files/lc32018/00/LC3-SIOV-final.pdf
[3] https://software.intel.com/en-us/download/intel-virtualization-technology-for-directed-io-architecture-specification
[4] https://lkml.org/lkml/2018/7/26/4

Cc Ashok Raj <[email protected]>

Cc Jacob Pan <[email protected]>

Cc Kevin Tian <[email protected]>

Cc Liu Yi L <[email protected]>

Suggested-by Kevin Tian <[email protected]>

Suggested-by Jean-Philippe Brucker <[email protected]>

Signed-off-by Lu Baolu <[email protected]>

---
 drivers/iommu/iommu.c | 52 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/iommu.h | 52 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 104 insertions(+)

drivers/iommu/iommu.c +52 -0

index edbdf5d6962c..0b7c96d1425e 100644
@@ -2030,3 +2030,55 @@ int iommu_fwspec_add_ids(struct device *dev, u32 *ids, int num_ids)
 	return 0;
 }
 EXPORT_SYMBOL_GPL(iommu_fwspec_add_ids);
+
+/*
+ * Generic interfaces to get or set per device IOMMU attributions.
+ */
+int iommu_get_dev_attr(struct device *dev, enum iommu_dev_attr attr, void *data)
+{
+	const struct iommu_ops *ops = dev->bus->iommu_ops;
+
+	if (ops && ops->get_dev_attr)
+		return ops->get_dev_attr(dev, attr, data);
+
+	return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(iommu_get_dev_attr);
+
+int iommu_set_dev_attr(struct device *dev, enum iommu_dev_attr attr, void *data)
+{
+	const struct iommu_ops *ops = dev->bus->iommu_ops;
+
+	if (ops && ops->set_dev_attr)
+		return ops->set_dev_attr(dev, attr, data);
+
+	return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(iommu_set_dev_attr);
+
+/*
+ * APIs to attach/detach a domain to/from a device in the
+ * auxiliary mode.
+ */
+int iommu_attach_device_aux(struct iommu_domain *domain, struct device *dev)
+{
+	int ret = -ENODEV;
+
+	if (domain->ops->attach_dev_aux)
+		ret = domain->ops->attach_dev_aux(domain, dev);
+
+	if (!ret)
+		trace_attach_device_to_domain(dev);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(iommu_attach_device_aux);
+
+void iommu_detach_device_aux(struct iommu_domain *domain, struct device *dev)
+{
+	if (domain->ops->detach_dev_aux) {
+		domain->ops->detach_dev_aux(domain, dev);
+		trace_detach_device_from_domain(dev);
+	}
+}
+EXPORT_SYMBOL_GPL(iommu_detach_device_aux);

include/linux/iommu.h +9 -0

index a1d28f42cb77..9bf1b3f2457a 100644
@@ -126,6 +126,7 @@ enum iommu_attr {
 	DOMAIN_ATTR_NESTING,	/* two stages of translation */
 	DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE,
 	DOMAIN_ATTR_MAX,
+	DOMAIN_ATTR_AUXD_ID,
 };
 
 /* These are the possible reserved region types */

@@ -156,6 +157,14 @@ struct iommu_resv_region {
 	enum iommu_resv_type	type;
 };
 
+/* Per device IOMMU attributions */
+enum iommu_dev_attr {
+	IOMMU_DEV_ATTR_AUXD_CAPABILITY,
+	IOMMU_DEV_ATTR_AUXD_ENABLED,
+	IOMMU_DEV_ATTR_AUXD_ENABLE,
+	IOMMU_DEV_ATTR_AUXD_DISABLE,
+};
+
 #ifdef CONFIG_IOMMU_API
 
 /**

include/linux/iommu.h +35 -0

+ * @set_dev_attr: set per device IOMMU attributions
  * @pgsize_bitmap: bitmap of all possible supported page sizes
  */
 struct iommu_ops {

@@ -226,6 +237,15 @@ struct iommu_ops {
 	int (*of_xlate)(struct device *dev, struct of_phandle_args *args);
 	bool (*is_attach_deferred)(struct iommu_domain *domain, struct device *dev);
 
+	/* Get/set per device IOMMU attributions */
+	int (*get_dev_attr)(struct device *dev,
+			    enum iommu_dev_attr attr, void *data);
+	int (*set_dev_attr)(struct device *dev,
+			    enum iommu_dev_attr attr, void *data);
+	/* Attach/detach aux domain */
+	int (*attach_dev_aux)(struct iommu_domain *domain, struct device *dev);
+	void (*detach_dev_aux)(struct iommu_domain *domain, struct device *dev);
+
 	unsigned long pgsize_bitmap;
 };
 

@@ -398,6 +418,16 @@ void iommu_fwspec_free(struct device *dev);
 int iommu_fwspec_add_ids(struct device *dev, u32 *ids, int num_ids);
 const struct iommu_ops *iommu_ops_from_fwnode(struct fwnode_handle *fwnode);
 
+int iommu_get_dev_attr(struct device *dev,
+		       enum iommu_dev_attr attr, void *data);
+int iommu_set_dev_attr(struct device *dev,
+		       enum iommu_dev_attr attr, void *data);
+
+extern int iommu_attach_device_aux(struct iommu_domain *domain,
+				   struct device *dev);
+extern void iommu_detach_device_aux(struct iommu_domain *domain,
+				    struct device *dev);
+
 #else /* CONFIG_IOMMU_API */
 
 struct iommu_ops {};

@@ -682,6 +712,28 @@ const struct iommu_ops *iommu_ops_from_fwnode(struct fwnode_handle *fwnode)
 	return NULL;
 }
 
+static inline int
+iommu_get_dev_attr(struct device *dev, enum iommu_dev_attr attr, void *data)
+{
+	return -EINVAL;
+}
+
+static inline int
+iommu_set_dev_attr(struct device *dev, enum iommu_dev_attr attr, void *data)
+{
+	return -EINVAL;
+}
+
+static inline int
+iommu_attach_device_aux(struct iommu_domain *domain, struct device *dev)
+{

Hi Lu,

Quoted 47 lines · Lu Baolu

Sharing a physical PCI device in a finer-granularity way is becoming a consensus in the industry. IOMMU vendors are also engaging efforts to support such sharing as well as possible. Among the efforts, the capability of support finer-granularity DMA isolation is a common requirement due to the security consideration. With finer-granularity DMA isolation, all DMA requests out of or to a subset of a physical PCI device can be protected by the IOMMU. As a result, there is a request in software to attach multiple domains to a physical PCI device. One example of such use model is the Intel Scalable IOV [1] [2]. The Intel vt-d 3.0 spec [3] introduces the scalable mode which enables PASID granularity DMA isolation.

This adds the APIs to support multiple domains per device. In order to ease the discussions, we call it 'a domain in auxiliary mode' or simply 'auxiliary domain' when multiple domains are attached to a physical device.

The APIs includes:

* iommu_get_dev_attr(dev, IOMMU_DEV_ATTR_AUXD_CAPABILITY)
- Represents the ability of supporting multiple domains per device.

* iommu_get_dev_attr(dev, IOMMU_DEV_ATTR_AUXD_ENABLED)
- Checks whether the device identified by @dev is working in auxiliary mode.

* iommu_set_dev_attr(dev, IOMMU_DEV_ATTR_AUXD_ENABLE)
- Enables the multiple domains capability for the device referenced by @dev.

* iommu_set_dev_attr(dev, IOMMU_DEV_ATTR_AUXD_DISABLE)
- Disables the multiple domains capability for the device referenced by @dev.

* iommu_attach_device_aux(domain, dev)
- Attaches @domain to @dev in the auxiliary mode. Multiple domains could be attached to a single device in the auxiliary mode with each domain representing an isolated address space for an assignable subset of the device.

* iommu_detach_device_aux(domain, dev)
- Detach @domain which has been attached to @dev in the auxiliary mode.

Quoted 4 lines

* iommu_domain_get_attr(domain, DOMAIN_ATTR_AUXD_ID)
- Return ID used for finer-granularity DMA translation. For the Intel Scalable IOV usage model, this will be a PASID. The device which supports Scalalbe IOV needs

s/Scalalbe/Scalable

Quoted 1 line

to writes this ID to the device register so that DMA

s/writes/write

Quoted 1 line

requests could be tagged with a right PASID prefix.

This is not crystal clear to me as the intel implementation returns the default PASID and not the PASID of the aux domain.

Quoted 116 lines

Many people involved in discussions of this design.

Kevin Tian <[email protected]> Liu Yi L <[email protected]> Ashok Raj <[email protected]> Sanjay Kumar <[email protected]> Jacob Pan <jacob.jun.pan-VuQAYsv1563Yd54FQh9/[email protected]> Alex Williamson <[email protected]> Jean-Philippe Brucker <[email protected]>

and some discussions can be found here [4].

[1] https://software.intel.com/en-us/download/intel-scalable-io-virtualization-technical-specification
[2] https://schd.ws/hosted_files/lc32018/00/LC3-SIOV-final.pdf
[3] https://software.intel.com/en-us/download/intel-virtualization-technology-for-directed-io-architecture-specification
[4] https://lkml.org/lkml/2018/7/26/4

Cc Ashok Raj <[email protected]>

Cc Jacob Pan <jacob.jun.pan-VuQAYsv1563Yd54FQh9/[email protected]>

Cc Kevin Tian <[email protected]>

Cc Liu Yi L <[email protected]>

Suggested-by Kevin Tian <[email protected]>

Suggested-by Jean-Philippe Brucker <[email protected]>

Signed-off-by Lu Baolu <baolu.lu-VuQAYsv1563Yd54FQh9/[email protected]>

---
  drivers/iommu/iommu.c | 52 +++++++++++++++++++++++++++++++++++++++++++
  include/linux/iommu.h | 52 +++++++++++++++++++++++++++++++++++++++++++
  2 files changed, 104 insertions(+)

drivers/iommu/iommu.c +52 -0

index edbdf5d6962c..0b7c96d1425e 100644
@@ -2030,3 +2030,55 @@ int iommu_fwspec_add_ids(struct device *dev, u32 *ids, int num_ids)
  	return 0;
  }
  EXPORT_SYMBOL_GPL(iommu_fwspec_add_ids);
+
+/*
+ * Generic interfaces to get or set per device IOMMU attributions.
+ */
+int iommu_get_dev_attr(struct device *dev, enum iommu_dev_attr attr, void *data)
+{
+	const struct iommu_ops *ops = dev->bus->iommu_ops;
+
+	if (ops && ops->get_dev_attr)
+		return ops->get_dev_attr(dev, attr, data);
+
+	return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(iommu_get_dev_attr);
+
+int iommu_set_dev_attr(struct device *dev, enum iommu_dev_attr attr, void *data)
+{
+	const struct iommu_ops *ops = dev->bus->iommu_ops;
+
+	if (ops && ops->set_dev_attr)
+		return ops->set_dev_attr(dev, attr, data);
+
+	return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(iommu_set_dev_attr);
+
+/*
+ * APIs to attach/detach a domain to/from a device in the
+ * auxiliary mode.
+ */
+int iommu_attach_device_aux(struct iommu_domain *domain, struct device *dev)
+{
+	int ret = -ENODEV;
+
+	if (domain->ops->attach_dev_aux)
+		ret = domain->ops->attach_dev_aux(domain, dev);
+
+	if (!ret)
+		trace_attach_device_to_domain(dev);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(iommu_attach_device_aux);
+
+void iommu_detach_device_aux(struct iommu_domain *domain, struct device *dev)
+{
+	if (domain->ops->detach_dev_aux) {
+		domain->ops->detach_dev_aux(domain, dev);
+		trace_detach_device_from_domain(dev);
+	}
+}
+EXPORT_SYMBOL_GPL(iommu_detach_device_aux);

include/linux/iommu.h +9 -0

index a1d28f42cb77..9bf1b3f2457a 100644
@@ -126,6 +126,7 @@ enum iommu_attr {
  	DOMAIN_ATTR_NESTING,	/* two stages of translation */
  	DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE,
  	DOMAIN_ATTR_MAX,
+	DOMAIN_ATTR_AUXD_ID,
  };
 
  /* These are the possible reserved region types */

@@ -156,6 +157,14 @@ struct iommu_resv_region {
  	enum iommu_resv_type	type;
  };
 
+/* Per device IOMMU attributions */
+enum iommu_dev_attr {
+	IOMMU_DEV_ATTR_AUXD_CAPABILITY,
+	IOMMU_DEV_ATTR_AUXD_ENABLED,
+	IOMMU_DEV_ATTR_AUXD_ENABLE,
+	IOMMU_DEV_ATTR_AUXD_DISABLE,
+};
+
  #ifdef CONFIG_IOMMU_API
 

include/linux/iommu.h struct iommu_resv_region { · line 195

Quoted 6 lines

include/linux/iommu.h +1 -0

  /**

@@ -183,6 +192,8 @@ struct iommu_resv_region {
   * @domain_window_enable: Configure and enable a particular window for a domain
   * @domain_window_disable: Disable a particular window for a domain
   * @of_xlate: add OF master IDs to iommu grouping
+ * @get_dev_attr: get per device IOMMU attributions

s/attributions/attributes here and other locations?

Quoted 65 lines

include/linux/iommu.h +42 -0

+ * @set_dev_attr: set per device IOMMU attributions
   * @pgsize_bitmap: bitmap of all possible supported page sizes
   */
  struct iommu_ops {

@@ -226,6 +237,15 @@ struct iommu_ops {
  	int (*of_xlate)(struct device *dev, struct of_phandle_args *args);
  	bool (*is_attach_deferred)(struct iommu_domain *domain, struct device *dev);
 
+	/* Get/set per device IOMMU attributions */
+	int (*get_dev_attr)(struct device *dev,
+			    enum iommu_dev_attr attr, void *data);
+	int (*set_dev_attr)(struct device *dev,
+			    enum iommu_dev_attr attr, void *data);
+	/* Attach/detach aux domain */
+	int (*attach_dev_aux)(struct iommu_domain *domain, struct device *dev);
+	void (*detach_dev_aux)(struct iommu_domain *domain, struct device *dev);
+
  	unsigned long pgsize_bitmap;
  };
 

@@ -398,6 +418,16 @@ void iommu_fwspec_free(struct device *dev);
  int iommu_fwspec_add_ids(struct device *dev, u32 *ids, int num_ids);
  const struct iommu_ops *iommu_ops_from_fwnode(struct fwnode_handle *fwnode);
 
+int iommu_get_dev_attr(struct device *dev,
+		       enum iommu_dev_attr attr, void *data);
+int iommu_set_dev_attr(struct device *dev,
+		       enum iommu_dev_attr attr, void *data);
+
+extern int iommu_attach_device_aux(struct iommu_domain *domain,
+				   struct device *dev);
+extern void iommu_detach_device_aux(struct iommu_domain *domain,
+				    struct device *dev);
+
  #else /* CONFIG_IOMMU_API */
 
  struct iommu_ops {};

@@ -682,6 +712,28 @@ const struct iommu_ops *iommu_ops_from_fwnode(struct fwnode_handle *fwnode)
  	return NULL;
  }
 
+static inline int
+iommu_get_dev_attr(struct device *dev, enum iommu_dev_attr attr, void *data)
+{
+	return -EINVAL;
+}
+
+static inline int
+iommu_set_dev_attr(struct device *dev, enum iommu_dev_attr attr, void *data)
+{
+	return -EINVAL;
+}
+
+static inline int
+iommu_attach_device_aux(struct iommu_domain *domain, struct device *dev)
+{
+	return -ENODEV;
+}
+
+static inline void
+iommu_detach_device_aux(struct iommu_domain *domain, struct device *dev)
+{
+}
  #endif /* CONFIG_IOMMU_API */
 

include/linux/iommu.h const struct iommu_ops *iommu_ops_from_fwnode(struct fwnode_handle *fwnode) · line 739

Quoted 2 lines

include/linux/iommu.h +0 -0

  #ifdef CONFIG_IOMMU_DEBUGFS

Thanks

Eric

Add the response to IOMMU_DEV_ATTR_AUXD_CAPABILITY capability query through iommu_get_dev_attr().

Cc Ashok Raj <[email protected]>

Cc Jacob Pan <jacob.jun.pan-VuQAYsv1563Yd54FQh9/[email protected]>

Cc Kevin Tian <[email protected]>

Signed-off-by Lu Baolu <baolu.lu-VuQAYsv1563Yd54FQh9/[email protected]>

Signed-off-by Liu Yi L <[email protected]>

---
 drivers/iommu/intel-iommu.c | 38 +++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

drivers/iommu/intel-iommu.c +18 -0

index 5e149d26ea9b..298f7a3fafe8 100644
@@ -5167,6 +5167,24 @@ static phys_addr_t intel_iommu_iova_to_phys(struct iommu_domain *domain,
 	return phys;
 }
 
+static inline bool scalable_mode_support(void)
+{
+	struct dmar_drhd_unit *drhd;
+	struct intel_iommu *iommu;
+	bool ret = true;
+
+	rcu_read_lock();
+	for_each_active_iommu(iommu, drhd) {
+		if (!sm_supported(iommu)) {
+			ret = false;
+			break;
+		}
+	}
+	rcu_read_unlock();
+
+	return ret;
+}
+
 static bool intel_iommu_capable(enum iommu_cap cap)
 {
 	if (cap == IOMMU_CAP_CACHE_COHERENCY)

drivers/iommu/intel-iommu.c +14 -0

+
+	switch (attr) {
+	case IOMMU_DEV_ATTR_AUXD_CAPABILITY:
+		auxd_capable = data;
+		*auxd_capable = scalable_mode_support();
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	return ret;
+}
+
 const struct iommu_ops intel_iommu_ops = {
 	.capable		= intel_iommu_capable,
 	.domain_alloc		= intel_iommu_domain_alloc,

Hi,

Quoted 2 lines · Lu Baolu

Add the response to IOMMU_DEV_ATTR_AUXD_CAPABILITY capability query through iommu_get_dev_attr().

commit title: Advertise auxiliary domain capability?

Quoted 43 lines

Cc Ashok Raj <[email protected]>

Cc Jacob Pan <[email protected]>

Cc Kevin Tian <[email protected]>

Signed-off-by Lu Baolu <[email protected]>

Signed-off-by Liu Yi L <[email protected]>

---
  drivers/iommu/intel-iommu.c | 38 +++++++++++++++++++++++++++++++++++++
  1 file changed, 38 insertions(+)

drivers/iommu/intel-iommu.c +18 -0

index 5e149d26ea9b..298f7a3fafe8 100644
@@ -5167,6 +5167,24 @@ static phys_addr_t intel_iommu_iova_to_phys(struct iommu_domain *domain,
  	return phys;
  }
 
+static inline bool scalable_mode_support(void)
+{
+	struct dmar_drhd_unit *drhd;
+	struct intel_iommu *iommu;
+	bool ret = true;
+
+	rcu_read_lock();
+	for_each_active_iommu(iommu, drhd) {
+		if (!sm_supported(iommu)) {
+			ret = false;
+			break;
+		}
+	}
+	rcu_read_unlock();
+
+	return ret;
+}
+
  static bool intel_iommu_capable(enum iommu_cap cap)
  {
  	if (cap == IOMMU_CAP_CACHE_COHERENCY)

@@ -5331,6 +5349,25 @@ struct intel_iommu *intel_svm_device_to_iommu(struct device *dev)
  }
  #endif /* CONFIG_INTEL_IOMMU_SVM */
 

drivers/iommu/intel-iommu.c struct intel_iommu *intel_svm_device_to_iommu(struct device *dev) · line 5356

Quoted 5 lines

drivers/iommu/intel-iommu.c +5 -0

+static int intel_iommu_get_dev_attr(struct device *dev,
+				    enum iommu_dev_attr attr, void *data)
+{
+	int ret = 0;
+	bool *auxd_capable;

nit: could be local to the case as other cases may use other datatypes.

Quoted 19 lines

drivers/iommu/intel-iommu.c +14 -0

+
+	switch (attr) {
+	case IOMMU_DEV_ATTR_AUXD_CAPABILITY:
+		auxd_capable = data;
+		*auxd_capable = scalable_mode_support();
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	return ret;
+}
+
  const struct iommu_ops intel_iommu_ops = {
  	.capable		= intel_iommu_capable,
  	.domain_alloc		= intel_iommu_domain_alloc,

drivers/iommu/intel-iommu.c const struct iommu_ops intel_iommu_ops = { · line 5388

Quoted 7 lines

drivers/iommu/intel-iommu.c +1 -0

@@ -5345,6 +5382,7 @@ const struct iommu_ops intel_iommu_ops = {
  	.get_resv_regions	= intel_iommu_get_resv_regions,
  	.put_resv_regions	= intel_iommu_put_resv_regions,
  	.device_group		= pci_device_group,
+	.get_dev_attr		= intel_iommu_get_dev_attr,
  	.pgsize_bitmap		= INTEL_IOMMU_PGSIZES,
  };
 

Thanks

Eric

When multiple domains per device has been enabled by the device driver, the device will tag the default PASID for the domain to all DMA traffics out of the subset of this device; and the IOMMU should translate the DMA requests in PASID granularity.

This extends the intel_iommu_attach/detach_device() ops to support managing PASID granular translation structures when the device driver has enabled multiple domains per device.

Cc Ashok Raj <[email protected]>

Cc Jacob Pan <[email protected]>

Cc Kevin Tian <[email protected]>

Signed-off-by Sanjay Kumar <[email protected]>

Signed-off-by Lu Baolu <[email protected]>

Signed-off-by Liu Yi L <[email protected]>

---
 drivers/iommu/intel-iommu.c | 192 +++++++++++++++++++++++++++++++-----
 include/linux/intel-iommu.h |  10 ++
 2 files changed, 180 insertions(+), 22 deletions(-)

drivers/iommu/intel-iommu.c +52 -21

index 2c86ac71c774..a61b25ad0d3b 100644
@@ -2477,6 +2477,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);
 
 	if (dev && dev_is_pci(dev)) {
 		struct pci_dev *pdev = to_pci_dev(info->dev);

@@ -5010,35 +5011,134 @@ static void intel_iommu_domain_free(struct iommu_domain *domain)
 	domain_exit(to_dmar_domain(domain));
 }
 
-static int intel_iommu_attach_device(struct iommu_domain *domain,
-				     struct device *dev)
+/*
+ * Check whether a @domain will be attached to the @dev in the
+ * auxiliary mode.
+ */
+static inline bool
+is_device_attach_aux_domain(struct device *dev, struct iommu_domain *domain)
 {
-	struct dmar_domain *dmar_domain = to_dmar_domain(domain);
-	struct intel_iommu *iommu;
-	int addr_width;
-	u8 bus, devfn;
+	struct device_domain_info *info = dev->archdata.iommu;
 
-	if (device_is_rmrr_locked(dev)) {
-		dev_warn(dev, "Device is ineligible for IOMMU domain attach due to platform RMRR requirement.  Contact your platform vendor.\n");
-		return -EPERM;
-	}
+	return info && info->auxd_enabled &&
+			domain->type == IOMMU_DOMAIN_UNMANAGED;
+}
 
-	/* normally dev is not mapped */
-	if (unlikely(domain_context_mapped(dev))) {
-		struct dmar_domain *old_domain;
+static void auxiliary_link_device(struct dmar_domain *domain,
+				  struct device *dev)
+{
+	struct device_domain_info *info = dev->archdata.iommu;
 
-		old_domain = find_domain(dev);
-		if (old_domain) {
-			rcu_read_lock();
-			dmar_remove_one_dev_info(old_domain, dev);
-			rcu_read_unlock();
+	assert_spin_locked(&device_domain_lock);
+	if (WARN_ON(!info))
+		return;
 
-			if (!domain_type_is_vm_or_si(old_domain) &&
-			     list_empty(&old_domain->devices))
-				domain_exit(old_domain);
+	domain->auxd_refcnt++;
+	list_add(&domain->auxd, &info->auxiliary_domains);
+}
+
+static void auxiliary_unlink_device(struct dmar_domain *domain,
+				    struct device *dev)
+{
+	struct device_domain_info *info = dev->archdata.iommu;
+
+	assert_spin_locked(&device_domain_lock);
+	if (WARN_ON(!info))
+		return;
+
+	list_del(&domain->auxd);
+	domain->auxd_refcnt--;
+
+	if (!domain->auxd_refcnt && domain->default_pasid > 0)
+		intel_pasid_free_id(domain->default_pasid);
+}
+
+static int domain_add_dev_auxd(struct dmar_domain *domain,
+			       struct device *dev)
+{
+	int ret;
+	u8 bus, devfn;
+	unsigned long flags;
+	struct intel_iommu *iommu;
+
+	iommu = device_to_iommu(dev, &bus, &devfn);
+	if (!iommu)
+		return -ENODEV;
+
+	spin_lock_irqsave(&device_domain_lock, flags);
+	if (domain->default_pasid <= 0) {

drivers/iommu/intel-iommu.c +47 -0

+	ret = domain_attach_iommu(domain, iommu);
+	if (ret)
+		goto attach_failed;
+
+	/* Setup the PASID entry for mediated devices: */
+	ret = intel_pasid_setup_second_level(iommu, domain, dev,
+					     domain->default_pasid);
+	if (ret)
+		goto table_failed;
+	spin_unlock(&iommu->lock);
+
+	auxiliary_link_device(domain, dev);
+
+	spin_unlock_irqrestore(&device_domain_lock, flags);
+
+	return 0;
+
+table_failed:
+	domain_detach_iommu(domain, iommu);
+attach_failed:
+	spin_unlock(&iommu->lock);
+	if (!domain->auxd_refcnt && domain->default_pasid > 0)
+		intel_pasid_free_id(domain->default_pasid);
+pasid_failed:
+	spin_unlock_irqrestore(&device_domain_lock, flags);
+
+	return ret;
+}
+
+static void domain_remove_dev_aux(struct dmar_domain *domain,
+				  struct device *dev)
+{
+	struct device_domain_info *info;
+	struct intel_iommu *iommu;
+	unsigned long flags;
+
+	if (!is_device_attach_aux_domain(dev, &domain->domain))
+		return;
+
+	spin_lock_irqsave(&device_domain_lock, flags);
+	info = dev->archdata.iommu;
+	iommu = info->iommu;
+
+	intel_pasid_tear_down_entry(iommu, dev, domain->default_pasid);
+
+	auxiliary_unlink_device(domain, dev);
+

drivers/iommu/intel-iommu.c +5 -0

+	struct dmar_domain *dmar_domain = to_dmar_domain(domain);
+	struct intel_iommu *iommu;
+	int addr_width;
+	u8 bus, devfn;
+
 	iommu = device_to_iommu(dev, &bus, &devfn);
 	if (!iommu)
 		return -ENODEV;

drivers/iommu/intel-iommu.c +36 -0

+
+	if (is_device_attach_aux_domain(dev, domain))
+		return -EPERM;
+
+	/* normally dev is not mapped */
+	if (unlikely(domain_context_mapped(dev))) {
+		struct dmar_domain *old_domain;
+
+		old_domain = find_domain(dev);
+		if (old_domain) {
+			rcu_read_lock();
+			dmar_remove_one_dev_info(old_domain, dev);
+			rcu_read_unlock();
+
+			if (!domain_type_is_vm_or_si(old_domain) &&
+			    list_empty(&old_domain->devices))
+				domain_exit(old_domain);
+		}
+	}
+
+	return __intel_iommu_attach_device(domain, dev);
+}
+
+static int intel_iommu_attach_device_aux(struct iommu_domain *domain,
+					 struct device *dev)
+{
+	return is_device_attach_aux_domain(dev, domain) ?
+			__intel_iommu_attach_device(domain, dev) : -EPERM;
 }
 
 static void intel_iommu_detach_device(struct iommu_domain *domain,

@@ -5080,6 +5220,12 @@ static void intel_iommu_detach_device(struct iommu_domain *domain,
 	dmar_remove_one_dev_info(to_dmar_domain(domain), dev);
 }
 
+static void intel_iommu_detach_device_aux(struct iommu_domain *domain,
+					  struct device *dev)
+{
+	domain_remove_dev_aux(to_dmar_domain(domain), dev);
+}
+
 static int intel_iommu_map(struct iommu_domain *domain,
 			   unsigned long iova, phys_addr_t hpa,
 			   size_t size, int iommu_prot)

@@ -5436,6 +5582,8 @@ const struct iommu_ops intel_iommu_ops = {
 	.domain_free		= intel_iommu_domain_free,
 	.attach_dev		= intel_iommu_attach_device,
 	.detach_dev		= intel_iommu_detach_device,
+	.attach_dev_aux		= intel_iommu_attach_device_aux,
+	.detach_dev_aux		= intel_iommu_detach_device_aux,
 	.map			= intel_iommu_map,
 	.unmap			= intel_iommu_unmap,
 	.iova_to_phys		= intel_iommu_iova_to_phys,

include/linux/intel-iommu.h +7 -0

index 6b198e13e75e..678c7fb05e74 100644
@@ -473,9 +473,11 @@ 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 iova_domain iovad;	/* iova's that belong to this domain */
 
 	struct dma_pte	*pgd;		/* virtual address */

@@ -494,6 +496,11 @@ struct dmar_domain {
 					   2 == 1GiB, 3 == 512GiB, 4 == 1TiB */
 	u64		max_addr;	/* maximum mapped address */
 
+	int		default_pasid;	/*
+					 * The default pasid used for non-SVM
+					 * traffic on mediated devices.
+					 */
+
 	struct iommu_domain domain;	/* generic domain data structure for
 					   iommu core */
 };

Hi Lu,

Quoted 124 lines · Lu Baolu

When multiple domains per device has been enabled by the device driver, the device will tag the default PASID for the domain to all DMA traffics out of the subset of this device; and the IOMMU should translate the DMA requests in PASID granularity.

This extends the intel_iommu_attach/detach_device() ops to support managing PASID granular translation structures when the device driver has enabled multiple domains per device.

Cc Ashok Raj <[email protected]>

Cc Jacob Pan <jacob.jun.pan-VuQAYsv1563Yd54FQh9/[email protected]>

Cc Kevin Tian <[email protected]>

Signed-off-by Sanjay Kumar <[email protected]>

Signed-off-by Lu Baolu <baolu.lu-VuQAYsv1563Yd54FQh9/[email protected]>

Signed-off-by Liu Yi L <[email protected]>

---
  drivers/iommu/intel-iommu.c | 192 +++++++++++++++++++++++++++++++-----
  include/linux/intel-iommu.h |  10 ++
  2 files changed, 180 insertions(+), 22 deletions(-)

drivers/iommu/intel-iommu.c +58 -21

index 2c86ac71c774..a61b25ad0d3b 100644
@@ -2477,6 +2477,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);
 
  	if (dev && dev_is_pci(dev)) {
  		struct pci_dev *pdev = to_pci_dev(info->dev);

@@ -5010,35 +5011,134 @@ static void intel_iommu_domain_free(struct iommu_domain *domain)
  	domain_exit(to_dmar_domain(domain));
  }
 
-static int intel_iommu_attach_device(struct iommu_domain *domain,
-				     struct device *dev)
+/*
+ * Check whether a @domain will be attached to the @dev in the
+ * auxiliary mode.
+ */
+static inline bool
+is_device_attach_aux_domain(struct device *dev, struct iommu_domain *domain)
  {
-	struct dmar_domain *dmar_domain = to_dmar_domain(domain);
-	struct intel_iommu *iommu;
-	int addr_width;
-	u8 bus, devfn;
+	struct device_domain_info *info = dev->archdata.iommu;
 
-	if (device_is_rmrr_locked(dev)) {
-		dev_warn(dev, "Device is ineligible for IOMMU domain attach due to platform RMRR requirement.  Contact your platform vendor.\n");
-		return -EPERM;
-	}
+	return info && info->auxd_enabled &&
+			domain->type == IOMMU_DOMAIN_UNMANAGED;
+}
 
-	/* normally dev is not mapped */
-	if (unlikely(domain_context_mapped(dev))) {
-		struct dmar_domain *old_domain;
+static void auxiliary_link_device(struct dmar_domain *domain,
+				  struct device *dev)
+{
+	struct device_domain_info *info = dev->archdata.iommu;
 
-		old_domain = find_domain(dev);
-		if (old_domain) {
-			rcu_read_lock();
-			dmar_remove_one_dev_info(old_domain, dev);
-			rcu_read_unlock();
+	assert_spin_locked(&device_domain_lock);
+	if (WARN_ON(!info))
+		return;
 
-			if (!domain_type_is_vm_or_si(old_domain) &&
-			     list_empty(&old_domain->devices))
-				domain_exit(old_domain);
+	domain->auxd_refcnt++;
+	list_add(&domain->auxd, &info->auxiliary_domains);
+}
+
+static void auxiliary_unlink_device(struct dmar_domain *domain,
+				    struct device *dev)
+{
+	struct device_domain_info *info = dev->archdata.iommu;
+
+	assert_spin_locked(&device_domain_lock);
+	if (WARN_ON(!info))
+		return;
+
+	list_del(&domain->auxd);
+	domain->auxd_refcnt--;
+
+	if (!domain->auxd_refcnt && domain->default_pasid > 0)
+		intel_pasid_free_id(domain->default_pasid);
+}
+
+static int domain_add_dev_auxd(struct dmar_domain *domain,
+			       struct device *dev)
+{
+	int ret;
+	u8 bus, devfn;
+	unsigned long flags;
+	struct intel_iommu *iommu;
+
+	iommu = device_to_iommu(dev, &bus, &devfn);
+	if (!iommu)
+		return -ENODEV;
+
+	spin_lock_irqsave(&device_domain_lock, flags);
+	if (domain->default_pasid <= 0) {
+		domain->default_pasid = intel_pasid_alloc_id(domain, PASID_MIN,
+				pci_max_pasids(to_pci_dev(dev)), GFP_ATOMIC);
+		if (domain->default_pasid < 0) {
+			pr_err("Can't allocate default pasid\n");
+			ret = -ENODEV;
+			goto pasid_failed;
  		}
  	}
 

drivers/iommu/intel-iommu.c static void intel_iommu_domain_free(struct iommu_domain *domain) · line 5079

Quoted 1 line

drivers/iommu/intel-iommu.c +1 -0

+	spin_lock(&iommu->lock);

You may comment your nested lock policy somewhere.

Quoted 51 lines

drivers/iommu/intel-iommu.c +51 -0

+	ret = domain_attach_iommu(domain, iommu);
+	if (ret)
+		goto attach_failed;
+
+	/* Setup the PASID entry for mediated devices: */
+	ret = intel_pasid_setup_second_level(iommu, domain, dev,
+					     domain->default_pasid);
+	if (ret)
+		goto table_failed;
+	spin_unlock(&iommu->lock);
+
+	auxiliary_link_device(domain, dev);
+
+	spin_unlock_irqrestore(&device_domain_lock, flags);
+
+	return 0;
+
+table_failed:
+	domain_detach_iommu(domain, iommu);
+attach_failed:
+	spin_unlock(&iommu->lock);
+	if (!domain->auxd_refcnt && domain->default_pasid > 0)
+		intel_pasid_free_id(domain->default_pasid);
+pasid_failed:
+	spin_unlock_irqrestore(&device_domain_lock, flags);
+
+	return ret;
+}
+
+static void domain_remove_dev_aux(struct dmar_domain *domain,
+				  struct device *dev)
+{
+	struct device_domain_info *info;
+	struct intel_iommu *iommu;
+	unsigned long flags;
+
+	if (!is_device_attach_aux_domain(dev, &domain->domain))
+		return;
+
+	spin_lock_irqsave(&device_domain_lock, flags);
+	info = dev->archdata.iommu;
+	iommu = info->iommu;
+
+	intel_pasid_tear_down_entry(iommu, dev, domain->default_pasid);
+
+	auxiliary_unlink_device(domain, dev);
+
+	spin_lock(&iommu->lock);
+	domain_detach_iommu(domain, iommu);
+	spin_unlock(&iommu->lock);
+

drivers/iommu/intel-iommu.c static void intel_iommu_domain_free(struct iommu_domain *domain) · line 5136

Quoted 6 lines

drivers/iommu/intel-iommu.c +6 -0

+	spin_unlock_irqrestore(&device_domain_lock, flags);
+}
+
+static int __intel_iommu_attach_device(struct iommu_domain *domain,
+				       struct device *dev)
+{

Maybe introducing __intel_iommu_attach_device in a patch prior to that one would help the review.

Quoted 12 lines

drivers/iommu/intel-iommu.c +5 -0

+	struct dmar_domain *dmar_domain = to_dmar_domain(domain);
+	struct intel_iommu *iommu;
+	int addr_width;
+	u8 bus, devfn;
+
  	iommu = device_to_iommu(dev, &bus, &devfn);
  	if (!iommu)
  		return -ENODEV;

@@ -5071,7 +5171,47 @@ static int intel_iommu_attach_device(struct iommu_domain *domain,
  		dmar_domain->agaw--;
  	}
 

drivers/iommu/intel-iommu.c static int intel_iommu_attach_device(struct iommu_domain *domain, · line 5175

Quoted 3 lines

drivers/iommu/intel-iommu.c +2 -1

-	return domain_add_dev_info(dmar_domain, dev);
+	if (is_device_attach_aux_domain(dev, domain))
+		return domain_add_dev_auxd(dmar_domain, dev);

why not putting this directly into intel_iommu_attach_device_aux()?

drivers/iommu/intel-iommu.c static int intel_iommu_attach_device(struct iommu_domain *domain, · line 5177

Quoted 2 lines

drivers/iommu/intel-iommu.c +2 -0

+	else
+		return domain_add_dev_info(dmar_domain, dev);

and this into intel_iommu_attach_device() as __intel_iommu_attach_device() is the common part now?

drivers/iommu/intel-iommu.c static int intel_iommu_attach_device(struct iommu_domain *domain, · line 5186

Quoted 9 lines

drivers/iommu/intel-iommu.c +9 -0

+}
+
+static int intel_iommu_attach_device(struct iommu_domain *domain,
+				     struct device *dev)
+{
+	if (device_is_rmrr_locked(dev)) {
+		dev_warn(dev, "Device is ineligible for IOMMU domain attach due to platform RMRR requirement.  Contact your platform vendor.\n");
+		return -EPERM;
+	}

shouldn't we test this in the common part (ie. in __intel_iommu_attach_device). Don't RMRR also impact aux domains ?

Quoted 83 lines

drivers/iommu/intel-iommu.c +36 -0

+
+	if (is_device_attach_aux_domain(dev, domain))
+		return -EPERM;
+
+	/* normally dev is not mapped */
+	if (unlikely(domain_context_mapped(dev))) {
+		struct dmar_domain *old_domain;
+
+		old_domain = find_domain(dev);
+		if (old_domain) {
+			rcu_read_lock();
+			dmar_remove_one_dev_info(old_domain, dev);
+			rcu_read_unlock();
+
+			if (!domain_type_is_vm_or_si(old_domain) &&
+			    list_empty(&old_domain->devices))
+				domain_exit(old_domain);
+		}
+	}
+
+	return __intel_iommu_attach_device(domain, dev);
+}
+
+static int intel_iommu_attach_device_aux(struct iommu_domain *domain,
+					 struct device *dev)
+{
+	return is_device_attach_aux_domain(dev, domain) ?
+			__intel_iommu_attach_device(domain, dev) : -EPERM;
  }
 
  static void intel_iommu_detach_device(struct iommu_domain *domain,

@@ -5080,6 +5220,12 @@ static void intel_iommu_detach_device(struct iommu_domain *domain,
  	dmar_remove_one_dev_info(to_dmar_domain(domain), dev);
  }
 
+static void intel_iommu_detach_device_aux(struct iommu_domain *domain,
+					  struct device *dev)
+{
+	domain_remove_dev_aux(to_dmar_domain(domain), dev);
+}
+
  static int intel_iommu_map(struct iommu_domain *domain,
  			   unsigned long iova, phys_addr_t hpa,
  			   size_t size, int iommu_prot)

@@ -5436,6 +5582,8 @@ const struct iommu_ops intel_iommu_ops = {
  	.domain_free		= intel_iommu_domain_free,
  	.attach_dev		= intel_iommu_attach_device,
  	.detach_dev		= intel_iommu_detach_device,
+	.attach_dev_aux		= intel_iommu_attach_device_aux,
+	.detach_dev_aux		= intel_iommu_detach_device_aux,
  	.map			= intel_iommu_map,
  	.unmap			= intel_iommu_unmap,
  	.iova_to_phys		= intel_iommu_iova_to_phys,

include/linux/intel-iommu.h +7 -0

index 6b198e13e75e..678c7fb05e74 100644
@@ -473,9 +473,11 @@ 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 iova_domain iovad;	/* iova's that belong to this domain */
 
  	struct dma_pte	*pgd;		/* virtual address */

@@ -494,6 +496,11 @@ struct dmar_domain {
  					   2 == 1GiB, 3 == 512GiB, 4 == 1TiB */
  	u64		max_addr;	/* maximum mapped address */
 
+	int		default_pasid;	/*
+					 * The default pasid used for non-SVM
+					 * traffic on mediated devices.
+					 */
+
  	struct iommu_domain domain;	/* generic domain data structure for
  					   iommu core */
  };

include/linux/intel-iommu.h struct device_domain_info { · line 558

Quoted 9 lines

include/linux/intel-iommu.h +3 -0

@@ -543,6 +550,9 @@ 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
+					     */
  	u8 bus;			/* PCI bus number */
  	u8 devfn;		/* PCI devfn number */
  	u16 pfsid;		/* SRIOV physical function source ID */

Thanks

Eric

Please use EXPORT_SYMBOL_GPL like most of the vfio code.

A parent device might create different types of mediated devices. For example, a mediated device could be created by the parent device with full isolation and protection provided by the IOMMU. One usage case could be found on Intel platforms where a mediated device is an assignable subset of a PCI, the DMA requests on behalf of it are all tagged with a PASID. Since IOMMU supports PASID-granular translations (scalable mode in vt-d 3.0), this mediated device could be individually protected and isolated by an IOMMU.

This patch adds two new members in struct mdev_device:
* iommu_device
- This, if set, indicates that the mediated device could be fully isolated and protected by IOMMU via attaching an iommu domain to this device. If empty, it indicates using vendor defined isolation.

* iommu_domain
- This is a place holder for an iommu domain. A domain could be store here for later use once it has been attached to the iommu_device of this mdev.

Below helpers are added to set and get above iommu device and iommu domain pointers.

* mdev_set/get_iommu_device(dev, iommu_device)
- Set or get the iommu device which represents this mdev in IOMMU's device scope. Drivers don't need to set the iommu device if it uses vendor defined isolation.

* mdev_set/get_iommu_domain(domain)
- A iommu domain which has been attached to the iommu device in order to protect and isolate the mediated device will be kept in the mdev data structure and could be retrieved later.

Cc Ashok Raj <[email protected]>

Cc Jacob Pan <jacob.jun.pan-VuQAYsv1563Yd54FQh9/[email protected]>

Cc Kevin Tian <[email protected]>

Cc Liu Yi L <[email protected]>

Suggested-by Kevin Tian <[email protected]>

Suggested-by Alex Williamson <[email protected]>

Signed-off-by Lu Baolu <baolu.lu-VuQAYsv1563Yd54FQh9/[email protected]>

---
 drivers/vfio/mdev/mdev_core.c    | 36 ++++++++++++++++++++++++++++++++
 drivers/vfio/mdev/mdev_private.h |  2 ++
 include/linux/mdev.h             | 23 ++++++++++++++++++++
 3 files changed, 61 insertions(+)

drivers/vfio/mdev/mdev_core.c +36 -0

index 0212f0ee8aea..5119809225c5 100644
@@ -390,6 +390,42 @@ int mdev_device_remove(struct device *dev, bool force_remove)
 	return 0;
 }
 
+int mdev_set_iommu_device(struct device *dev, struct device *iommu_device)
+{
+	struct mdev_device *mdev = to_mdev_device(dev);
+
+	mdev->iommu_device = iommu_device;
+
+	return 0;
+}
+EXPORT_SYMBOL(mdev_set_iommu_device);
+
+struct device *mdev_get_iommu_device(struct device *dev)
+{
+	struct mdev_device *mdev = to_mdev_device(dev);
+
+	return mdev->iommu_device;
+}
+EXPORT_SYMBOL(mdev_get_iommu_device);
+
+int mdev_set_iommu_domain(struct device *dev, void *domain)
+{
+	struct mdev_device *mdev = to_mdev_device(dev);
+
+	mdev->iommu_domain = domain;
+
+	return 0;
+}
+EXPORT_SYMBOL(mdev_set_iommu_domain);
+
+void *mdev_get_iommu_domain(struct device *dev)
+{
+	struct mdev_device *mdev = to_mdev_device(dev);
+
+	return mdev->iommu_domain;
+}
+EXPORT_SYMBOL(mdev_get_iommu_domain);
+
 static int __init mdev_init(void)
 {
 	return mdev_bus_register();

drivers/vfio/mdev/mdev_private.h +2 -0

index b5819b7d7ef7..c01518068e84 100644
@@ -34,6 +34,8 @@ struct mdev_device {
 	struct list_head next;
 	struct kobject *type_kobj;
 	bool active;
+	struct device *iommu_device;
+	void *iommu_domain;
 };
 
 #define to_mdev_device(dev)	container_of(dev, struct mdev_device, dev)

include/linux/mdev.h +0 -0

index b6e048e1045f..c46777d3e568 100644

include/linux/mdev.h +9 -0

+ * this mdev in iommu protection scope. By default, the iommu device is NULL,
+ * that indicates using vendor defined isolation.
+ *
+ * @dev: the mediated device that iommu will isolate.
+ * @iommu_device: a pci device which represents the iommu for @dev.
+ *
+ * Return 0 for success, otherwise negative error value.
+ */
+int mdev_set_iommu_device(struct device *dev, struct device *iommu_device);

Quoted 115 lines · On Mon, 5 Nov 2018 15:34:06 +0800 Lu Baolu <[email protected]> wrote:

A parent device might create different types of mediated devices. For example, a mediated device could be created by the parent device with full isolation and protection provided by the IOMMU. One usage case could be found on Intel platforms where a mediated device is an assignable subset of a PCI, the DMA requests on behalf of it are all tagged with a PASID. Since IOMMU supports PASID-granular translations (scalable mode in vt-d 3.0), this mediated device could be individually protected and isolated by an IOMMU.

This patch adds two new members in struct mdev_device:
* iommu_device
- This, if set, indicates that the mediated device could be fully isolated and protected by IOMMU via attaching an iommu domain to this device. If empty, it indicates using vendor defined isolation.

* iommu_domain
- This is a place holder for an iommu domain. A domain could be store here for later use once it has been attached to the iommu_device of this mdev.

Below helpers are added to set and get above iommu device and iommu domain pointers.

* mdev_set/get_iommu_device(dev, iommu_device)
- Set or get the iommu device which represents this mdev in IOMMU's device scope. Drivers don't need to set the iommu device if it uses vendor defined isolation.

* mdev_set/get_iommu_domain(domain)
- A iommu domain which has been attached to the iommu device in order to protect and isolate the mediated device will be kept in the mdev data structure and could be retrieved later.

Cc Ashok Raj <[email protected]>

Cc Jacob Pan <[email protected]>

Cc Kevin Tian <[email protected]>

Cc Liu Yi L <[email protected]>

Suggested-by Kevin Tian <[email protected]>

Suggested-by Alex Williamson <[email protected]>

Signed-off-by Lu Baolu <[email protected]>

---
  drivers/vfio/mdev/mdev_core.c    | 36 ++++++++++++++++++++++++++++++++
  drivers/vfio/mdev/mdev_private.h |  2 ++
  include/linux/mdev.h             | 23 ++++++++++++++++++++
  3 files changed, 61 insertions(+)

drivers/vfio/mdev/mdev_core.c +36 -0

index 0212f0ee8aea..5119809225c5 100644
@@ -390,6 +390,42 @@ int mdev_device_remove(struct device *dev, bool force_remove)
  	return 0;
  }
 
+int mdev_set_iommu_device(struct device *dev, struct device *iommu_device)
+{
+	struct mdev_device *mdev = to_mdev_device(dev);
+
+	mdev->iommu_device = iommu_device;
+
+	return 0;
+}
+EXPORT_SYMBOL(mdev_set_iommu_device);
+
+struct device *mdev_get_iommu_device(struct device *dev)
+{
+	struct mdev_device *mdev = to_mdev_device(dev);
+
+	return mdev->iommu_device;
+}
+EXPORT_SYMBOL(mdev_get_iommu_device);
+
+int mdev_set_iommu_domain(struct device *dev, void *domain)
+{
+	struct mdev_device *mdev = to_mdev_device(dev);
+
+	mdev->iommu_domain = domain;
+
+	return 0;
+}
+EXPORT_SYMBOL(mdev_set_iommu_domain);
+
+void *mdev_get_iommu_domain(struct device *dev)
+{
+	struct mdev_device *mdev = to_mdev_device(dev);
+
+	return mdev->iommu_domain;
+}
+EXPORT_SYMBOL(mdev_get_iommu_domain);
+
  static int __init mdev_init(void)
  {
  	return mdev_bus_register();

drivers/vfio/mdev/mdev_private.h +2 -0

index b5819b7d7ef7..c01518068e84 100644
@@ -34,6 +34,8 @@ struct mdev_device {
  	struct list_head next;
  	struct kobject *type_kobj;
  	bool active;
+	struct device *iommu_device;
+	void *iommu_domain;
  };
 
  #define to_mdev_device(dev)	container_of(dev, struct mdev_device, dev)

include/linux/mdev.h +0 -0

index b6e048e1045f..c46777d3e568 100644

include/linux/mdev.h line 20

Quoted 7 lines

include/linux/mdev.h +4 -0

@@ -14,6 +14,29 @@
  #define MDEV_H
 
  struct mdev_device;
+struct iommu_domain;
+
+/*
+ * Called by the parent device driver to set the PCI device which represents

s/PCI //

There is no requirement or expectation that the device is PCI.

Quoted 15 lines

include/linux/mdev.h +15 -0

+ * this mdev in iommu protection scope. By default, the iommu device is NULL,
+ * that indicates using vendor defined isolation.
+ *
+ * @dev: the mediated device that iommu will isolate.
+ * @iommu_device: a pci device which represents the iommu for @dev.
+ *
+ * Return 0 for success, otherwise negative error value.
+ */
+int mdev_set_iommu_device(struct device *dev, struct device *iommu_device);
+
+struct device *mdev_get_iommu_device(struct device *dev);
+
+/*
+ * Called by vfio iommu modules to save the iommu domain after a domain being
+ * attached to the mediated device.

include/linux/mdev.h line 39

Quoted 4 lines

include/linux/mdev.h +4 -0

+ */
+int mdev_set_iommu_domain(struct device *dev, void *domain);
+
+void *mdev_get_iommu_domain(struct device *dev);

I can't say I really understand the purpose of this, the cover letter indicates this is a placeholder, should we add it separately when we have a requirement for it?

Hi Alex,

Quoted 131 lines · Alex Williamson

Quoted 124 lines · On Mon,  5 Nov 2018 15:34:06 +0800 Lu Baolu <[email protected]> wrote:

A parent device might create different types of mediated devices. For example, a mediated device could be created by the parent device with full isolation and protection provided by the IOMMU. One usage case could be found on Intel platforms where a mediated device is an assignable subset of a PCI, the DMA requests on behalf of it are all tagged with a PASID. Since IOMMU supports PASID-granular translations (scalable mode in vt-d 3.0), this mediated device could be individually protected and isolated by an IOMMU.

This patch adds two new members in struct mdev_device:
* iommu_device
   - This, if set, indicates that the mediated device could be fully isolated and protected by IOMMU via attaching an iommu domain to this device. If empty, it indicates using vendor defined isolation.

* iommu_domain
   - This is a place holder for an iommu domain. A domain could be store here for later use once it has been attached to the iommu_device of this mdev.

Below helpers are added to set and get above iommu device and iommu domain pointers.

* mdev_set/get_iommu_device(dev, iommu_device)
   - Set or get the iommu device which represents this mdev in IOMMU's device scope. Drivers don't need to set the iommu device if it uses vendor defined isolation.

* mdev_set/get_iommu_domain(domain)
   - A iommu domain which has been attached to the iommu device in order to protect and isolate the mediated device will be kept in the mdev data structure and could be retrieved later.

Cc Ashok Raj <[email protected]>

Cc Jacob Pan <[email protected]>

Cc Kevin Tian <[email protected]>

Cc Liu Yi L <[email protected]>

Suggested-by Kevin Tian <[email protected]>

Suggested-by Alex Williamson <[email protected]>

Signed-off-by Lu Baolu <[email protected]>

---
  drivers/vfio/mdev/mdev_core.c    | 36 ++++++++++++++++++++++++++++++++ drivers/vfio/mdev/mdev_private.h |  2 ++ include/linux/mdev.h             | 23 ++++++++++++++++++++

  3 files changed, 61 insertions(+)

drivers/vfio/mdev/mdev_core.c +0 -0

index 0212f0ee8aea..5119809225c5 100644
@@ -390,6 +390,42 @@ int mdev_device_remove(struct device *dev, bool
 force_remove)
       return 0;
   }

  +int mdev_set_iommu_device(struct device *dev, struct device *iommu_device)

drivers/vfio/mdev/mdev_core.c +35 -0

+{
+    struct mdev_device *mdev = to_mdev_device(dev);
+
+    mdev->iommu_device = iommu_device;
+
+    return 0;
+}
+EXPORT_SYMBOL(mdev_set_iommu_device);
+
+struct device *mdev_get_iommu_device(struct device *dev)
+{
+    struct mdev_device *mdev = to_mdev_device(dev);
+
+    return mdev->iommu_device;
+}
+EXPORT_SYMBOL(mdev_get_iommu_device);
+
+int mdev_set_iommu_domain(struct device *dev, void *domain)
+{
+    struct mdev_device *mdev = to_mdev_device(dev);
+
+    mdev->iommu_domain = domain;
+
+    return 0;
+}
+EXPORT_SYMBOL(mdev_set_iommu_domain);
+
+void *mdev_get_iommu_domain(struct device *dev)
+{
+    struct mdev_device *mdev = to_mdev_device(dev);
+
+    return mdev->iommu_domain;
+}
+EXPORT_SYMBOL(mdev_get_iommu_domain);
+

  static int __init mdev_init(void)
  {
      return mdev_bus_register();

drivers/vfio/mdev/mdev_private.h +2 -0

index b5819b7d7ef7..c01518068e84 100644
@@ -34,6 +34,8 @@ struct mdev_device {
       struct list_head next;
       struct kobject *type_kobj;
       bool active;
+    struct device *iommu_device;
+    void *iommu_domain;
   };
     #define to_mdev_device(dev)    container_of(dev, struct
 mdev_device, dev)

include/linux/mdev.h +4 -0

index b6e048e1045f..c46777d3e568 100644
@@ -14,6 +14,29 @@
   #define MDEV_H
     struct mdev_device;
+struct iommu_domain;
+
+/*
+ * Called by the parent device driver to set the PCI device which

Quoted 1 line

represents

s/PCI //

Quoted 2 lines

There is no requirement or expectation that the device is PCI.

Fair enough.

Quoted 18 lines

Quoted 18 lines

+ * this mdev in iommu protection scope. By default, the iommu device is NULL,
+ * that indicates using vendor defined isolation.
+ *
+ * @dev: the mediated device that iommu will isolate.
+ * @iommu_device: a pci device which represents the iommu for @dev.
+ *
+ * Return 0 for success, otherwise negative error value.
+ */
+int mdev_set_iommu_device(struct device *dev, struct device *iommu_device);

include/linux/mdev.h +5 -0

+
+struct device *mdev_get_iommu_device(struct device *dev);
+
+/*
+ * Called by vfio iommu modules to save the iommu domain after a

domain being
+ * attached to the mediated device.

Quoted 8 lines

include/linux/mdev.h line 39

Quoted 4 lines

+ */

include/linux/mdev.h +3 -0

+int mdev_set_iommu_domain(struct device *dev, void *domain);
+
+void *mdev_get_iommu_domain(struct device *dev);

I can't say I really understand the purpose of this, the cover letter indicates this is a placeholder, should we add it separately when we have a requirement for it?

Oh, I am sorry that I used a wrong word. It's not a placeholder for something designed for future, but adding two members that will be used in the following patches. Since they will be used in anther modules (like vfio_iommu), we need function interfaces to get and set them.

mdev->iommu_device:
 -  This, if set, indicates that the mediated device could be fully isolated and protected by IOMMU via attaching an iommu domain to this device. If empty, it indicates using vendor defined isolation.

mdev->iommu_domain:
  - This is used to save the pointer of an iommu domain. Once a domain has been attached to the iommu_device, it should be stored here.

Quoted 177 lines · Lu Baolu

Hi Alex,

Quoted 131 lines · Alex Williamson

Quoted 124 lines · On Mon,  5 Nov 2018 15:34:06 +0800 Lu Baolu <[email protected]> wrote:

A parent device might create different types of mediated devices. For example, a mediated device could be created by the parent device with full isolation and protection provided by the IOMMU. One usage case could be found on Intel platforms where a mediated device is an assignable subset of a PCI, the DMA requests on behalf of it are all tagged with a PASID. Since IOMMU supports PASID-granular translations (scalable mode in vt-d 3.0), this mediated device could be individually protected and isolated by an IOMMU.

This patch adds two new members in struct mdev_device:
* iommu_device
   - This, if set, indicates that the mediated device could be fully isolated and protected by IOMMU via attaching an iommu domain to this device. If empty, it indicates using vendor defined isolation.

* iommu_domain
   - This is a place holder for an iommu domain. A domain could be store here for later use once it has been attached to the iommu_device of this mdev.

Below helpers are added to set and get above iommu device and iommu domain pointers.

* mdev_set/get_iommu_device(dev, iommu_device)
   - Set or get the iommu device which represents this mdev in IOMMU's device scope. Drivers don't need to set the iommu device if it uses vendor defined isolation.

* mdev_set/get_iommu_domain(domain)
   - A iommu domain which has been attached to the iommu device in order to protect and isolate the mediated device will be kept in the mdev data structure and could be retrieved later.

Cc Ashok Raj <[email protected]>

Cc Jacob Pan <[email protected]>

Cc Kevin Tian <[email protected]>

Cc Liu Yi L <[email protected]>

Suggested-by Kevin Tian <[email protected]>

Suggested-by Alex Williamson <[email protected]>

Signed-off-by Lu Baolu <[email protected]>

---
   drivers/vfio/mdev/mdev_core.c    | 36 ++++++++++++++++++++++++++++++++
   drivers/vfio/mdev/mdev_private.h |  2 ++
   include/linux/mdev.h             | 23 ++++++++++++++++++++
   3 files changed, 61 insertions(+)

drivers/vfio/mdev/mdev_core.c +35 -0

index 0212f0ee8aea..5119809225c5 100644
@@ -390,6 +390,42 @@ int mdev_device_remove(struct device *dev, bool
 force_remove)
       return 0;
   }
   +int mdev_set_iommu_device(struct device *dev, struct device
 *iommu_device)
+{
+    struct mdev_device *mdev = to_mdev_device(dev);
+
+    mdev->iommu_device = iommu_device;
+
+    return 0;
+}
+EXPORT_SYMBOL(mdev_set_iommu_device);
+
+struct device *mdev_get_iommu_device(struct device *dev)
+{
+    struct mdev_device *mdev = to_mdev_device(dev);
+
+    return mdev->iommu_device;
+}
+EXPORT_SYMBOL(mdev_get_iommu_device);
+
+int mdev_set_iommu_domain(struct device *dev, void *domain)
+{
+    struct mdev_device *mdev = to_mdev_device(dev);
+
+    mdev->iommu_domain = domain;
+
+    return 0;
+}
+EXPORT_SYMBOL(mdev_set_iommu_domain);
+
+void *mdev_get_iommu_domain(struct device *dev)
+{
+    struct mdev_device *mdev = to_mdev_device(dev);
+
+    return mdev->iommu_domain;
+}
+EXPORT_SYMBOL(mdev_get_iommu_domain);
+
   static int __init mdev_init(void)
   {
       return mdev_bus_register();

drivers/vfio/mdev/mdev_private.h +2 -0

index b5819b7d7ef7..c01518068e84 100644
@@ -34,6 +34,8 @@ struct mdev_device {
       struct list_head next;
       struct kobject *type_kobj;
       bool active;
+    struct device *iommu_device;
+    void *iommu_domain;
   };
     #define to_mdev_device(dev)    container_of(dev, struct
 mdev_device, dev)

include/linux/mdev.h +4 -0

index b6e048e1045f..c46777d3e568 100644
@@ -14,6 +14,29 @@
   #define MDEV_H
     struct mdev_device;
+struct iommu_domain;
+
+/*
+ * Called by the parent device driver to set the PCI device which

Quoted 1 line

represents

s/PCI //

Quoted 2 lines

There is no requirement or expectation that the device is PCI.

Fair enough.

Quoted 18 lines

Quoted 18 lines

+ * this mdev in iommu protection scope. By default, the iommu device is NULL,
+ * that indicates using vendor defined isolation.
+ *
+ * @dev: the mediated device that iommu will isolate.
+ * @iommu_device: a pci device which represents the iommu for @dev.
+ *
+ * Return 0 for success, otherwise negative error value.
+ */
+int mdev_set_iommu_device(struct device *dev, struct device *iommu_device);

include/linux/mdev.h +5 -0

+
+struct device *mdev_get_iommu_device(struct device *dev);
+
+/*
+ * Called by vfio iommu modules to save the iommu domain after a

domain being
+ * attached to the mediated device.

Quoted 8 lines

include/linux/mdev.h line 39

Quoted 4 lines

+ */

include/linux/mdev.h +3 -0

+int mdev_set_iommu_domain(struct device *dev, void *domain);
+
+void *mdev_get_iommu_domain(struct device *dev);

I can't say I really understand the purpose of this, the cover letter indicates this is a placeholder, should we add it separately when we have a requirement for it?

Oh, I am sorry that I used a wrong word. It's not a placeholder for something designed for future, but adding two members that will be used in the following patches. Since they will be used in anther modules (like vfio_iommu), we need function interfaces to get and set them.

mdev->iommu_device:
 -  This, if set, indicates that the mediated device could be fully isolated and protected by IOMMU via attaching an iommu domain to this device. If empty, it indicates using vendor defined isolation.

Quoted 5 lines

mdev->iommu_domain:
  - This is used to save the pointer of an iommu domain. Once a domain has been attached to the iommu_device, it should be stored here.

I don't see mdev->iommu_domain is used anywhere in this series of patch. If this is not being used, then no need to save it. With that symbols mdev_set/get_iommu_domain(domain) are not required.

Please keep symbols mdev_set/get_iommu_device(dev, iommu_device) non-GPL same as other exported symbols from mdev_core module.

Thanks,
Kirti

Quoted 2 lines

Please keep symbols mdev_set/get_iommu_device(dev, iommu_device) non-GPL same as other exported symbols from mdev_core module.

Yes. It will be fixed in the next version.

Quoted 4 lines · Lu Baolu

Quoted 2 lines

Please keep symbols mdev_set/get_iommu_device(dev, iommu_device) non-GPL same as other exported symbols from mdev_core module.

Yes. It will be fixed in the next version.

No. mdev shall not be used to circumvent the exports in the generic vfio code.

Quoted 4 lines · Lu Baolu

Quoted 2 lines

Please keep symbols mdev_set/get_iommu_device(dev, iommu_device) non-GPL same as other exported symbols from mdev_core module.

Yes. It will be fixed in the next version.

No. mdev shall not be used to circumvent the exports in the generic vfio code.

It is about how mdev framework can be used by existing drivers. These symbols doesn't use any other exported symbols.

Quoted 2 lines · Kirti Wankhede

It is about how mdev framework can be used by existing drivers. These symbols doesn't use any other exported symbols.

That is an unfortunate accident of history, but doesn't extent to new ones. It also is another inidicator those drivers probably are derived works of the Linux kernel and might be in legal trouble one way or another.

This adds helpers to attach or detach a domain to a group. This will replace iommu_attach_group() which only works for pci devices.

If a domain is attaching to a group which includes the mediated devices, it should attach to the iommu device (a pci device which represents the mdev in iommu scope) instead. The added helper supports attaching domain to groups for both pci and mdev devices.

Cc Ashok Raj <[email protected]>

Cc Jacob Pan <jacob.jun.pan-VuQAYsv1563Yd54FQh9/[email protected]>

Cc Kevin Tian <[email protected]>

Signed-off-by Lu Baolu <baolu.lu-VuQAYsv1563Yd54FQh9/[email protected]>

Signed-off-by Liu Yi L <[email protected]>

---
 drivers/vfio/vfio_iommu_type1.c | 114 ++++++++++++++++++++++++++++++--
 1 file changed, 107 insertions(+), 7 deletions(-)

drivers/vfio/vfio_iommu_type1.c +39 -0

index d9fd3188615d..178264b330e7 100644
@@ -91,6 +91,7 @@ struct vfio_dma {
 struct vfio_group {
 	struct iommu_group	*iommu_group;
 	struct list_head	next;
+	bool			mdev_group;	/* An mdev group */
 };
 
 /*

@@ -1327,6 +1328,105 @@ static bool vfio_iommu_has_sw_msi(struct iommu_group *group, phys_addr_t *base)
 	return ret;
 }
 
+static struct device *vfio_mdev_get_iommu_device(struct device *dev)
+{
+	struct device *(*fn)(struct device *dev);
+	struct device *iommu_parent;
+
+	fn = symbol_get(mdev_get_iommu_device);
+	if (fn) {
+		iommu_parent = fn(dev);
+		symbol_put(mdev_get_iommu_device);
+
+		return iommu_parent;
+	}
+
+	return NULL;
+}
+
+static int vfio_mdev_set_domain(struct device *dev, struct iommu_domain *domain)
+{
+	int (*fn)(struct device *dev, void *domain);
+	int ret;
+
+	fn = symbol_get(mdev_set_iommu_domain);
+	if (fn) {
+		ret = fn(dev, domain);
+		symbol_put(mdev_set_iommu_domain);
+
+		return ret;
+	}
+
+	return -EINVAL;
+}
+
+static int vfio_mdev_attach_domain(struct device *dev, void *data)
+{
+	struct iommu_domain *domain = data;
+	struct device *iommu_device;
+	int ret;
+

drivers/vfio/vfio_iommu_type1.c +7 -0

+	}
+
+	return -EINVAL;
+}
+
+static int vfio_mdev_detach_domain(struct device *dev, void *data)
+{

drivers/vfio/vfio_iommu_type1.c +36 -6

+		if (aux_mode)
+			iommu_detach_device_aux(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);
+	else
+		return iommu_attach_group(domain->domain, group->iommu_group);
+}
+
+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);
+	else
+		iommu_detach_group(domain->domain, group->iommu_group);
+}
+
 static int vfio_iommu_type1_attach_group(void *iommu_data,
 					 struct iommu_group *iommu_group)
 {

@@ -1402,7 +1502,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
 			goto out_domain;
 	}
 
-	ret = iommu_attach_group(domain->domain, iommu_group);
+	ret = vfio_iommu_attach_group(domain, group);
 	if (ret)
 		goto out_domain;
 

@@ -1434,8 +1534,8 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
 	list_for_each_entry(d, &iommu->domain_list, next) {
 		if (d->domain->ops == domain->domain->ops &&
 		    d->prot == domain->prot) {
-			iommu_detach_group(domain->domain, iommu_group);
-			if (!iommu_attach_group(d->domain, iommu_group)) {
+			vfio_iommu_detach_group(domain, group);
+			if (!vfio_iommu_attach_group(d, group)) {
 				list_add(&group->next, &d->group_list);
 				iommu_domain_free(domain->domain);
 				kfree(domain);

@@ -1443,7 +1543,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
 				return 0;
 			}
 
-			ret = iommu_attach_group(domain->domain, iommu_group);
+			ret = vfio_iommu_attach_group(domain, group);
 			if (ret)
 				goto out_domain;
 		}

@@ -1469,7 +1569,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
 	return 0;
 
 out_detach:
-	iommu_detach_group(domain->domain, iommu_group);
+	vfio_iommu_detach_group(domain, group);
 out_domain:
 	iommu_domain_free(domain->domain);
 out_free:

@@ -1560,7 +1660,7 @@ static void vfio_iommu_type1_detach_group(void *iommu_data,
 		if (!group)
 			continue;
 
-		iommu_detach_group(domain->domain, iommu_group);
+		vfio_iommu_detach_group(domain, group);
 		list_del(&group->next);
 		kfree(group);
 		/*

Hi Lu,

Quoted 3 lines · Lu Baolu

This adds helpers to attach or detach a domain to a group. This will replace iommu_attach_group() which only works for pci devices.

s/pci/non mdev?

Quoted 73 lines

If a domain is attaching to a group which includes the mediated devices, it should attach to the iommu device (a pci device which represents the mdev in iommu scope) instead. The added helper supports attaching domain to groups for both pci and mdev devices.

Cc Ashok Raj <[email protected]>

Cc Jacob Pan <jacob.jun.pan-VuQAYsv1563Yd54FQh9/[email protected]>

Cc Kevin Tian <[email protected]>

Signed-off-by Lu Baolu <baolu.lu-VuQAYsv1563Yd54FQh9/[email protected]>

Signed-off-by Liu Yi L <[email protected]>

---
  drivers/vfio/vfio_iommu_type1.c | 114 ++++++++++++++++++++++++++++++--
  1 file changed, 107 insertions(+), 7 deletions(-)

drivers/vfio/vfio_iommu_type1.c +42 -0

index d9fd3188615d..178264b330e7 100644
@@ -91,6 +91,7 @@ struct vfio_dma {
  struct vfio_group {
  	struct iommu_group	*iommu_group;
  	struct list_head	next;
+	bool			mdev_group;	/* An mdev group */
  };
 
  /*

@@ -1327,6 +1328,105 @@ static bool vfio_iommu_has_sw_msi(struct iommu_group *group, phys_addr_t *base)
  	return ret;
  }
 
+static struct device *vfio_mdev_get_iommu_device(struct device *dev)
+{
+	struct device *(*fn)(struct device *dev);
+	struct device *iommu_parent;
+
+	fn = symbol_get(mdev_get_iommu_device);
+	if (fn) {
+		iommu_parent = fn(dev);
+		symbol_put(mdev_get_iommu_device);
+
+		return iommu_parent;
+	}
+
+	return NULL;
+}
+
+static int vfio_mdev_set_domain(struct device *dev, struct iommu_domain *domain)
+{
+	int (*fn)(struct device *dev, void *domain);
+	int ret;
+
+	fn = symbol_get(mdev_set_iommu_domain);
+	if (fn) {
+		ret = fn(dev, domain);
+		symbol_put(mdev_set_iommu_domain);
+
+		return ret;
+	}
+
+	return -EINVAL;
+}
+
+static int vfio_mdev_attach_domain(struct device *dev, void *data)
+{
+	struct iommu_domain *domain = data;
+	struct device *iommu_device;
+	int ret;
+
+	ret = vfio_mdev_set_domain(dev, domain);
+	if (ret)
+		return ret;

drivers/vfio/vfio_iommu_type1.c static bool vfio_iommu_has_sw_msi(struct iommu_group *group, phys_addr_t *base) · line 1378

Quoted 7 lines

drivers/vfio/vfio_iommu_type1.c +7 -0

+
+	iommu_device = vfio_mdev_get_iommu_device(dev);
+	if (iommu_device) {
+		bool aux_mode = false;
+
+		iommu_get_dev_attr(iommu_device,
+				   IOMMU_DEV_ATTR_AUXD_ENABLED, &aux_mode);

Don' you need to test the returned value before using aux_mode?

drivers/vfio/vfio_iommu_type1.c static bool vfio_iommu_has_sw_msi(struct iommu_group *group, phys_addr_t *base) · line 1382

Quoted 4 lines

drivers/vfio/vfio_iommu_type1.c +4 -0

+		if (aux_mode)
+			return iommu_attach_device_aux(domain, iommu_device);
+		else
+			return iommu_attach_device(domain, iommu_device);

if for some reason the above ops fail, don't you want to call vfio_mdev_set_domain(dev, NULL)

Quoted 11 lines

drivers/vfio/vfio_iommu_type1.c +10 -0

+	}
+
+	return -EINVAL;
+}
+
+static int vfio_mdev_detach_domain(struct device *dev, void *data)
+{
+	struct iommu_domain *domain = data;
+	struct device *iommu_device;
+

drivers/vfio/vfio_iommu_type1.c static bool vfio_iommu_has_sw_msi(struct iommu_group *group, phys_addr_t *base) · line 1399

Quoted 7 lines

drivers/vfio/vfio_iommu_type1.c +7 -0

+	vfio_mdev_set_domain(dev, NULL);
+	iommu_device = vfio_mdev_get_iommu_device(dev);
+	if (iommu_device) {
+		bool aux_mode = false;
+
+		iommu_get_dev_attr(iommu_device,
+				   IOMMU_DEV_ATTR_AUXD_ENABLED, &aux_mode);

same here

Quoted 82 lines

drivers/vfio/vfio_iommu_type1.c +36 -6

+		if (aux_mode)
+			iommu_detach_device_aux(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);
+	else
+		return iommu_attach_group(domain->domain, group->iommu_group);
+}
+
+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);
+	else
+		iommu_detach_group(domain->domain, group->iommu_group);
+}
+
  static int vfio_iommu_type1_attach_group(void *iommu_data,
  					 struct iommu_group *iommu_group)
  {

@@ -1402,7 +1502,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
  			goto out_domain;
  	}
 
-	ret = iommu_attach_group(domain->domain, iommu_group);
+	ret = vfio_iommu_attach_group(domain, group);
  	if (ret)
  		goto out_domain;
 

@@ -1434,8 +1534,8 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
  	list_for_each_entry(d, &iommu->domain_list, next) {
  		if (d->domain->ops == domain->domain->ops &&
  		    d->prot == domain->prot) {
-			iommu_detach_group(domain->domain, iommu_group);
-			if (!iommu_attach_group(d->domain, iommu_group)) {
+			vfio_iommu_detach_group(domain, group);
+			if (!vfio_iommu_attach_group(d, group)) {
  				list_add(&group->next, &d->group_list);
  				iommu_domain_free(domain->domain);
  				kfree(domain);

@@ -1443,7 +1543,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
  				return 0;
  			}
 
-			ret = iommu_attach_group(domain->domain, iommu_group);
+			ret = vfio_iommu_attach_group(domain, group);
  			if (ret)
  				goto out_domain;
  		}

@@ -1469,7 +1569,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
  	return 0;
 
  out_detach:
-	iommu_detach_group(domain->domain, iommu_group);
+	vfio_iommu_detach_group(domain, group);
  out_domain:
  	iommu_domain_free(domain->domain);
  out_free:

@@ -1560,7 +1660,7 @@ static void vfio_iommu_type1_detach_group(void *iommu_data,
  		if (!group)
  			continue;
 
-		iommu_detach_group(domain->domain, iommu_group);
+		vfio_iommu_detach_group(domain, group);
  		list_del(&group->next);
  		kfree(group);
  		/*

drivers/vfio/vfio_iommu_type1.c static void vfio_release_domain(struct vfio_domain *domain, bool external) · line 1731

Quoted 8 lines

drivers/vfio/vfio_iommu_type1.c +1 -1

@@ -1625,7 +1725,7 @@ static void vfio_release_domain(struct vfio_domain *domain, bool external)
  	list_for_each_entry_safe(group, group_tmp,
  				 &domain->group_list, next) {
  		if (!external)
-			iommu_detach_group(domain->domain, group->iommu_group);
+			vfio_iommu_detach_group(domain, group);
  		list_del(&group->next);
  		kfree(group);
  	}

Thanks

Eric