All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 0/5] iommu aux-domain APIs extensions
@ 2020-10-30  4:58 ` Lu Baolu
  0 siblings, 0 replies; 30+ messages in thread
From: Lu Baolu @ 2020-10-30  4:58 UTC (permalink / raw)
  To: Joerg Roedel, Alex Williamson
  Cc: Robin Murphy, Jean-Philippe Brucker, Cornelia Huck, Kevin Tian,
	Ashok Raj, Dave Jiang, Liu Yi L, Zeng Xin, iommu, linux-kernel,
	kvm, Lu Baolu

Hi Joerg and Alex,

A description of purpose for this series could be found here.

https://lore.kernel.org/linux-iommu/20200901033422.22249-1-baolu.lu@linux.intel.com/

The previous version was posted here.

https://lore.kernel.org/linux-iommu/20200922061042.31633-1-baolu.lu@linux.intel.com/

This version is evolved according to Joerg's comments posted here.

https://lore.kernel.org/linux-iommu/20200924095532.GK27174@8bytes.org/

This basic idea is that IOMMU registers an iommu_ops for subdevice
bus (for example, the vfio/mdev bus), so that the upper layer device
passthrough framework could use the standard iommu-core code to setup
the IOMMU logistics.

This series was tested by Dave Jiang <dave.jiang@intel.com> with his
idxd driver posted here. Very appreciated!

https://lore.kernel.org/lkml/160021250454.67751.3119489448651243709.stgit@djiang5-desk3.ch.intel.com/

Please help to review and comment.

Best regards,
baolu

Lu Baolu (5):
  vfio/mdev: Register mdev bus earlier during boot
  iommu: Use bus iommu ops for aux related callback
  iommu/vt-d: Make some static functions global
  iommu/vt-d: Add iommu_ops support for subdevice bus
  vfio/type1: Use mdev bus iommu_ops for IOMMU callbacks

 drivers/iommu/intel/Kconfig      |  13 ++++
 drivers/iommu/intel/Makefile     |   1 +
 drivers/iommu/intel/iommu.c      |  79 +++++--------------
 drivers/iommu/intel/siov.c       | 119 ++++++++++++++++++++++++++++
 drivers/iommu/iommu.c            |  16 ++--
 drivers/vfio/mdev/mdev_core.c    |  22 +-----
 drivers/vfio/mdev/mdev_driver.c  |   6 ++
 drivers/vfio/mdev/mdev_private.h |   1 -
 drivers/vfio/vfio_iommu_type1.c  | 128 +++----------------------------
 include/linux/intel-iommu.h      |  53 +++++++++++++
 include/linux/mdev.h             |  14 ----
 11 files changed, 236 insertions(+), 216 deletions(-)
 create mode 100644 drivers/iommu/intel/siov.c

-- 
2.25.1


^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH v6 0/5] iommu aux-domain APIs extensions
@ 2020-10-30  4:58 ` Lu Baolu
  0 siblings, 0 replies; 30+ messages in thread
From: Lu Baolu @ 2020-10-30  4:58 UTC (permalink / raw)
  To: Joerg Roedel, Alex Williamson
  Cc: Jean-Philippe Brucker, Kevin Tian, Dave Jiang, Ashok Raj, kvm,
	Cornelia Huck, linux-kernel, iommu, Robin Murphy

Hi Joerg and Alex,

A description of purpose for this series could be found here.

https://lore.kernel.org/linux-iommu/20200901033422.22249-1-baolu.lu@linux.intel.com/

The previous version was posted here.

https://lore.kernel.org/linux-iommu/20200922061042.31633-1-baolu.lu@linux.intel.com/

This version is evolved according to Joerg's comments posted here.

https://lore.kernel.org/linux-iommu/20200924095532.GK27174@8bytes.org/

This basic idea is that IOMMU registers an iommu_ops for subdevice
bus (for example, the vfio/mdev bus), so that the upper layer device
passthrough framework could use the standard iommu-core code to setup
the IOMMU logistics.

This series was tested by Dave Jiang <dave.jiang@intel.com> with his
idxd driver posted here. Very appreciated!

https://lore.kernel.org/lkml/160021250454.67751.3119489448651243709.stgit@djiang5-desk3.ch.intel.com/

Please help to review and comment.

Best regards,
baolu

Lu Baolu (5):
  vfio/mdev: Register mdev bus earlier during boot
  iommu: Use bus iommu ops for aux related callback
  iommu/vt-d: Make some static functions global
  iommu/vt-d: Add iommu_ops support for subdevice bus
  vfio/type1: Use mdev bus iommu_ops for IOMMU callbacks

 drivers/iommu/intel/Kconfig      |  13 ++++
 drivers/iommu/intel/Makefile     |   1 +
 drivers/iommu/intel/iommu.c      |  79 +++++--------------
 drivers/iommu/intel/siov.c       | 119 ++++++++++++++++++++++++++++
 drivers/iommu/iommu.c            |  16 ++--
 drivers/vfio/mdev/mdev_core.c    |  22 +-----
 drivers/vfio/mdev/mdev_driver.c  |   6 ++
 drivers/vfio/mdev/mdev_private.h |   1 -
 drivers/vfio/vfio_iommu_type1.c  | 128 +++----------------------------
 include/linux/intel-iommu.h      |  53 +++++++++++++
 include/linux/mdev.h             |  14 ----
 11 files changed, 236 insertions(+), 216 deletions(-)
 create mode 100644 drivers/iommu/intel/siov.c

-- 
2.25.1

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH v6 1/5] vfio/mdev: Register mdev bus earlier during boot
  2020-10-30  4:58 ` Lu Baolu
@ 2020-10-30  4:58   ` Lu Baolu
  -1 siblings, 0 replies; 30+ messages in thread
From: Lu Baolu @ 2020-10-30  4:58 UTC (permalink / raw)
  To: Joerg Roedel, Alex Williamson
  Cc: Robin Murphy, Jean-Philippe Brucker, Cornelia Huck, Kevin Tian,
	Ashok Raj, Dave Jiang, Liu Yi L, Zeng Xin, iommu, linux-kernel,
	kvm, Lu Baolu

Move mdev bus registration earlier than IOMMU probe processing so that
the IOMMU drivers could be able to set iommu_ops for the mdev bus. This
only applies when vfio-mdev module is setected to be built-in.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/vfio/mdev/mdev_core.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
index b558d4cfd082..6b9ab71f89e7 100644
--- a/drivers/vfio/mdev/mdev_core.c
+++ b/drivers/vfio/mdev/mdev_core.c
@@ -417,8 +417,12 @@ static void __exit mdev_exit(void)
 	mdev_bus_unregister();
 }
 
+#if IS_BUILTIN(CONFIG_VFIO_MDEV)
+postcore_initcall(mdev_init)
+#else
 module_init(mdev_init)
 module_exit(mdev_exit)
+#endif /* IS_BUILTIN(CONFIG_VFIO_MDEV) */
 
 MODULE_VERSION(DRIVER_VERSION);
 MODULE_LICENSE("GPL v2");
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v6 1/5] vfio/mdev: Register mdev bus earlier during boot
@ 2020-10-30  4:58   ` Lu Baolu
  0 siblings, 0 replies; 30+ messages in thread
From: Lu Baolu @ 2020-10-30  4:58 UTC (permalink / raw)
  To: Joerg Roedel, Alex Williamson
  Cc: Jean-Philippe Brucker, Kevin Tian, Dave Jiang, Ashok Raj, kvm,
	Cornelia Huck, linux-kernel, iommu, Robin Murphy

Move mdev bus registration earlier than IOMMU probe processing so that
the IOMMU drivers could be able to set iommu_ops for the mdev bus. This
only applies when vfio-mdev module is setected to be built-in.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/vfio/mdev/mdev_core.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
index b558d4cfd082..6b9ab71f89e7 100644
--- a/drivers/vfio/mdev/mdev_core.c
+++ b/drivers/vfio/mdev/mdev_core.c
@@ -417,8 +417,12 @@ static void __exit mdev_exit(void)
 	mdev_bus_unregister();
 }
 
+#if IS_BUILTIN(CONFIG_VFIO_MDEV)
+postcore_initcall(mdev_init)
+#else
 module_init(mdev_init)
 module_exit(mdev_exit)
+#endif /* IS_BUILTIN(CONFIG_VFIO_MDEV) */
 
 MODULE_VERSION(DRIVER_VERSION);
 MODULE_LICENSE("GPL v2");
-- 
2.25.1

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v6 2/5] iommu: Use bus iommu ops for aux related callback
  2020-10-30  4:58 ` Lu Baolu
@ 2020-10-30  4:58   ` Lu Baolu
  -1 siblings, 0 replies; 30+ messages in thread
From: Lu Baolu @ 2020-10-30  4:58 UTC (permalink / raw)
  To: Joerg Roedel, Alex Williamson
  Cc: Robin Murphy, Jean-Philippe Brucker, Cornelia Huck, Kevin Tian,
	Ashok Raj, Dave Jiang, Liu Yi L, Zeng Xin, iommu, linux-kernel,
	kvm, Lu Baolu

The aux-domain apis were designed for macro driver where the subdevices
are created and used inside a device driver. Use the device's bus iommu
ops instead of that in iommu domain for various callbacks.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/iommu.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 6bbdd959f9f3..17f2686664db 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -2913,10 +2913,11 @@ EXPORT_SYMBOL_GPL(iommu_dev_feature_enabled);
  */
 int iommu_aux_attach_device(struct iommu_domain *domain, struct device *dev)
 {
+	const struct iommu_ops *ops = dev->bus->iommu_ops;
 	int ret = -ENODEV;
 
-	if (domain->ops->aux_attach_dev)
-		ret = domain->ops->aux_attach_dev(domain, dev);
+	if (ops && ops->aux_attach_dev)
+		ret = ops->aux_attach_dev(domain, dev);
 
 	if (!ret)
 		trace_attach_device_to_domain(dev);
@@ -2927,8 +2928,10 @@ EXPORT_SYMBOL_GPL(iommu_aux_attach_device);
 
 void iommu_aux_detach_device(struct iommu_domain *domain, struct device *dev)
 {
-	if (domain->ops->aux_detach_dev) {
-		domain->ops->aux_detach_dev(domain, dev);
+	const struct iommu_ops *ops = dev->bus->iommu_ops;
+
+	if (ops && ops->aux_detach_dev) {
+		ops->aux_detach_dev(domain, dev);
 		trace_detach_device_from_domain(dev);
 	}
 }
@@ -2936,10 +2939,11 @@ EXPORT_SYMBOL_GPL(iommu_aux_detach_device);
 
 int iommu_aux_get_pasid(struct iommu_domain *domain, struct device *dev)
 {
+	const struct iommu_ops *ops = dev->bus->iommu_ops;
 	int ret = -ENODEV;
 
-	if (domain->ops->aux_get_pasid)
-		ret = domain->ops->aux_get_pasid(domain, dev);
+	if (ops && ops->aux_get_pasid)
+		ret = ops->aux_get_pasid(domain, dev);
 
 	return ret;
 }
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v6 2/5] iommu: Use bus iommu ops for aux related callback
@ 2020-10-30  4:58   ` Lu Baolu
  0 siblings, 0 replies; 30+ messages in thread
From: Lu Baolu @ 2020-10-30  4:58 UTC (permalink / raw)
  To: Joerg Roedel, Alex Williamson
  Cc: Jean-Philippe Brucker, Kevin Tian, Dave Jiang, Ashok Raj, kvm,
	Cornelia Huck, linux-kernel, iommu, Robin Murphy

The aux-domain apis were designed for macro driver where the subdevices
are created and used inside a device driver. Use the device's bus iommu
ops instead of that in iommu domain for various callbacks.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/iommu.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 6bbdd959f9f3..17f2686664db 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -2913,10 +2913,11 @@ EXPORT_SYMBOL_GPL(iommu_dev_feature_enabled);
  */
 int iommu_aux_attach_device(struct iommu_domain *domain, struct device *dev)
 {
+	const struct iommu_ops *ops = dev->bus->iommu_ops;
 	int ret = -ENODEV;
 
-	if (domain->ops->aux_attach_dev)
-		ret = domain->ops->aux_attach_dev(domain, dev);
+	if (ops && ops->aux_attach_dev)
+		ret = ops->aux_attach_dev(domain, dev);
 
 	if (!ret)
 		trace_attach_device_to_domain(dev);
@@ -2927,8 +2928,10 @@ EXPORT_SYMBOL_GPL(iommu_aux_attach_device);
 
 void iommu_aux_detach_device(struct iommu_domain *domain, struct device *dev)
 {
-	if (domain->ops->aux_detach_dev) {
-		domain->ops->aux_detach_dev(domain, dev);
+	const struct iommu_ops *ops = dev->bus->iommu_ops;
+
+	if (ops && ops->aux_detach_dev) {
+		ops->aux_detach_dev(domain, dev);
 		trace_detach_device_from_domain(dev);
 	}
 }
@@ -2936,10 +2939,11 @@ EXPORT_SYMBOL_GPL(iommu_aux_detach_device);
 
 int iommu_aux_get_pasid(struct iommu_domain *domain, struct device *dev)
 {
+	const struct iommu_ops *ops = dev->bus->iommu_ops;
 	int ret = -ENODEV;
 
-	if (domain->ops->aux_get_pasid)
-		ret = domain->ops->aux_get_pasid(domain, dev);
+	if (ops && ops->aux_get_pasid)
+		ret = ops->aux_get_pasid(domain, dev);
 
 	return ret;
 }
-- 
2.25.1

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v6 3/5] iommu/vt-d: Make some static functions global
  2020-10-30  4:58 ` Lu Baolu
@ 2020-10-30  4:58   ` Lu Baolu
  -1 siblings, 0 replies; 30+ messages in thread
From: Lu Baolu @ 2020-10-30  4:58 UTC (permalink / raw)
  To: Joerg Roedel, Alex Williamson
  Cc: Robin Murphy, Jean-Philippe Brucker, Cornelia Huck, Kevin Tian,
	Ashok Raj, Dave Jiang, Liu Yi L, Zeng Xin, iommu, linux-kernel,
	kvm, Lu Baolu

So that they could be used in other files as well. No functional changes.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/intel/iommu.c | 74 +++++++------------------------------
 include/linux/intel-iommu.h | 49 ++++++++++++++++++++++++
 2 files changed, 62 insertions(+), 61 deletions(-)

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 8f5e7b31b3fb..1454fe74f3ba 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -330,10 +330,6 @@ static void domain_exit(struct dmar_domain *domain);
 static void domain_remove_dev_info(struct dmar_domain *domain);
 static void dmar_remove_one_dev_info(struct device *dev);
 static void __dmar_remove_one_dev_info(struct device_domain_info *info);
-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);
 
 #ifdef CONFIG_INTEL_IOMMU_DEFAULT_ON
 int dmar_disabled = 0;
@@ -4423,7 +4419,7 @@ static int md_domain_init(struct dmar_domain *domain, int guest_width)
 	return 0;
 }
 
-static struct iommu_domain *intel_iommu_domain_alloc(unsigned type)
+struct iommu_domain *intel_iommu_domain_alloc(unsigned int type)
 {
 	struct dmar_domain *dmar_domain;
 	struct iommu_domain *domain;
@@ -4462,7 +4458,7 @@ static struct iommu_domain *intel_iommu_domain_alloc(unsigned type)
 	return NULL;
 }
 
-static void intel_iommu_domain_free(struct iommu_domain *domain)
+void intel_iommu_domain_free(struct iommu_domain *domain)
 {
 	if (domain != &si_domain->domain)
 		domain_exit(to_dmar_domain(domain));
@@ -4639,8 +4635,7 @@ static int prepare_domain_attach_device(struct iommu_domain *domain,
 	return 0;
 }
 
-static int intel_iommu_attach_device(struct iommu_domain *domain,
-				     struct device *dev)
+int intel_iommu_attach_device(struct iommu_domain *domain, struct device *dev)
 {
 	int ret;
 
@@ -4669,8 +4664,7 @@ 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)
+int intel_iommu_aux_attach_device(struct iommu_domain *domain, struct device *dev)
 {
 	int ret;
 
@@ -4684,14 +4678,12 @@ static int intel_iommu_aux_attach_device(struct iommu_domain *domain,
 	return aux_domain_add_dev(to_dmar_domain(domain), dev);
 }
 
-static void intel_iommu_detach_device(struct iommu_domain *domain,
-				      struct device *dev)
+void intel_iommu_detach_device(struct iommu_domain *domain, struct device *dev)
 {
 	dmar_remove_one_dev_info(dev);
 }
 
-static void intel_iommu_aux_detach_device(struct iommu_domain *domain,
-					  struct device *dev)
+void intel_iommu_aux_detach_device(struct iommu_domain *domain, struct device *dev)
 {
 	aux_domain_remove_dev(to_dmar_domain(domain), dev);
 }
@@ -4875,9 +4867,8 @@ intel_iommu_sva_invalidate(struct iommu_domain *domain, struct device *dev,
 }
 #endif
 
-static int intel_iommu_map(struct iommu_domain *domain,
-			   unsigned long iova, phys_addr_t hpa,
-			   size_t size, int iommu_prot, gfp_t gfp)
+int intel_iommu_map(struct iommu_domain *domain, unsigned long iova,
+		    phys_addr_t hpa, size_t size, int iommu_prot, gfp_t gfp)
 {
 	struct dmar_domain *dmar_domain = to_dmar_domain(domain);
 	u64 max_addr;
@@ -4913,9 +4904,8 @@ static int intel_iommu_map(struct iommu_domain *domain,
 	return ret;
 }
 
-static size_t intel_iommu_unmap(struct iommu_domain *domain,
-				unsigned long iova, size_t size,
-				struct iommu_iotlb_gather *gather)
+size_t intel_iommu_unmap(struct iommu_domain *domain, unsigned long iova,
+			 size_t size, struct iommu_iotlb_gather *gather)
 {
 	struct dmar_domain *dmar_domain = to_dmar_domain(domain);
 	unsigned long start_pfn, last_pfn;
@@ -4963,8 +4953,7 @@ static void intel_iommu_tlb_sync(struct iommu_domain *domain,
 	dma_free_pagelist(gather->freelist);
 }
 
-static phys_addr_t intel_iommu_iova_to_phys(struct iommu_domain *domain,
-					    dma_addr_t iova)
+phys_addr_t intel_iommu_iova_to_phys(struct iommu_domain *domain, dma_addr_t iova)
 {
 	struct dmar_domain *dmar_domain = to_dmar_domain(domain);
 	struct dma_pte *pte;
@@ -4980,42 +4969,6 @@ 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 inline bool iommu_pasid_support(void)
-{
-	struct dmar_drhd_unit *drhd;
-	struct intel_iommu *iommu;
-	bool ret = true;
-
-	rcu_read_lock();
-	for_each_active_iommu(iommu, drhd) {
-		if (!pasid_supported(iommu)) {
-			ret = false;
-			break;
-		}
-	}
-	rcu_read_unlock();
-
-	return ret;
-}
-
 static inline bool nested_mode_support(void)
 {
 	struct dmar_drhd_unit *drhd;
@@ -5034,7 +4987,7 @@ static inline bool nested_mode_support(void)
 	return ret;
 }
 
-static bool intel_iommu_capable(enum iommu_cap cap)
+bool intel_iommu_capable(enum iommu_cap cap)
 {
 	if (cap == IOMMU_CAP_CACHE_COHERENCY)
 		return domain_update_iommu_snooping(NULL) == 1;
@@ -5084,8 +5037,7 @@ static void intel_iommu_probe_finalize(struct device *dev)
 		set_dma_ops(dev, NULL);
 }
 
-static void intel_iommu_get_resv_regions(struct device *device,
-					 struct list_head *head)
+void intel_iommu_get_resv_regions(struct device *device, struct list_head *head)
 {
 	int prot = DMA_PTE_READ | DMA_PTE_WRITE;
 	struct iommu_resv_region *reg;
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index fbf5b3e7707e..d3928ba87986 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -693,6 +693,42 @@ static inline int first_pte_in_page(struct dma_pte *pte)
 	return !((unsigned long)pte & ~VTD_PAGE_MASK);
 }
 
+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 inline bool iommu_pasid_support(void)
+{
+	struct dmar_drhd_unit *drhd;
+	struct intel_iommu *iommu;
+	bool ret = true;
+
+	rcu_read_lock();
+	for_each_active_iommu(iommu, drhd) {
+		if (!pasid_supported(iommu)) {
+			ret = false;
+			break;
+		}
+	}
+	rcu_read_unlock();
+
+	return ret;
+}
+
 extern struct dmar_drhd_unit * dmar_find_matched_drhd_unit(struct pci_dev *dev);
 extern int dmar_find_matched_atsr_unit(struct pci_dev *dev);
 
@@ -737,6 +773,19 @@ int intel_iommu_enable_pasid(struct intel_iommu *iommu, struct device *dev);
 struct dmar_domain *find_domain(struct device *dev);
 struct device_domain_info *get_domain_info(struct device *dev);
 struct intel_iommu *device_to_iommu(struct device *dev, u8 *bus, u8 *devfn);
+bool intel_iommu_capable(enum iommu_cap cap);
+struct iommu_domain *intel_iommu_domain_alloc(unsigned int type);
+void intel_iommu_domain_free(struct iommu_domain *domain);
+int intel_iommu_attach_device(struct iommu_domain *domain, struct device *dev);
+int intel_iommu_aux_attach_device(struct iommu_domain *domain, struct device *dev);
+void intel_iommu_detach_device(struct iommu_domain *domain, struct device *dev);
+void intel_iommu_aux_detach_device(struct iommu_domain *domain, struct device *dev);
+int intel_iommu_map(struct iommu_domain *domain, unsigned long iova,
+		    phys_addr_t hpa, size_t size, int iommu_prot, gfp_t gfp);
+size_t intel_iommu_unmap(struct iommu_domain *domain, unsigned long iova,
+			 size_t size, struct iommu_iotlb_gather *gather);
+phys_addr_t intel_iommu_iova_to_phys(struct iommu_domain *domain, dma_addr_t iova);
+void intel_iommu_get_resv_regions(struct device *device, struct list_head *head);
 
 #ifdef CONFIG_INTEL_IOMMU_SVM
 extern void intel_svm_check(struct intel_iommu *iommu);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v6 3/5] iommu/vt-d: Make some static functions global
@ 2020-10-30  4:58   ` Lu Baolu
  0 siblings, 0 replies; 30+ messages in thread
From: Lu Baolu @ 2020-10-30  4:58 UTC (permalink / raw)
  To: Joerg Roedel, Alex Williamson
  Cc: Jean-Philippe Brucker, Kevin Tian, Dave Jiang, Ashok Raj, kvm,
	Cornelia Huck, linux-kernel, iommu, Robin Murphy

So that they could be used in other files as well. No functional changes.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/intel/iommu.c | 74 +++++++------------------------------
 include/linux/intel-iommu.h | 49 ++++++++++++++++++++++++
 2 files changed, 62 insertions(+), 61 deletions(-)

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 8f5e7b31b3fb..1454fe74f3ba 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -330,10 +330,6 @@ static void domain_exit(struct dmar_domain *domain);
 static void domain_remove_dev_info(struct dmar_domain *domain);
 static void dmar_remove_one_dev_info(struct device *dev);
 static void __dmar_remove_one_dev_info(struct device_domain_info *info);
-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);
 
 #ifdef CONFIG_INTEL_IOMMU_DEFAULT_ON
 int dmar_disabled = 0;
@@ -4423,7 +4419,7 @@ static int md_domain_init(struct dmar_domain *domain, int guest_width)
 	return 0;
 }
 
-static struct iommu_domain *intel_iommu_domain_alloc(unsigned type)
+struct iommu_domain *intel_iommu_domain_alloc(unsigned int type)
 {
 	struct dmar_domain *dmar_domain;
 	struct iommu_domain *domain;
@@ -4462,7 +4458,7 @@ static struct iommu_domain *intel_iommu_domain_alloc(unsigned type)
 	return NULL;
 }
 
-static void intel_iommu_domain_free(struct iommu_domain *domain)
+void intel_iommu_domain_free(struct iommu_domain *domain)
 {
 	if (domain != &si_domain->domain)
 		domain_exit(to_dmar_domain(domain));
@@ -4639,8 +4635,7 @@ static int prepare_domain_attach_device(struct iommu_domain *domain,
 	return 0;
 }
 
-static int intel_iommu_attach_device(struct iommu_domain *domain,
-				     struct device *dev)
+int intel_iommu_attach_device(struct iommu_domain *domain, struct device *dev)
 {
 	int ret;
 
@@ -4669,8 +4664,7 @@ 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)
+int intel_iommu_aux_attach_device(struct iommu_domain *domain, struct device *dev)
 {
 	int ret;
 
@@ -4684,14 +4678,12 @@ static int intel_iommu_aux_attach_device(struct iommu_domain *domain,
 	return aux_domain_add_dev(to_dmar_domain(domain), dev);
 }
 
-static void intel_iommu_detach_device(struct iommu_domain *domain,
-				      struct device *dev)
+void intel_iommu_detach_device(struct iommu_domain *domain, struct device *dev)
 {
 	dmar_remove_one_dev_info(dev);
 }
 
-static void intel_iommu_aux_detach_device(struct iommu_domain *domain,
-					  struct device *dev)
+void intel_iommu_aux_detach_device(struct iommu_domain *domain, struct device *dev)
 {
 	aux_domain_remove_dev(to_dmar_domain(domain), dev);
 }
@@ -4875,9 +4867,8 @@ intel_iommu_sva_invalidate(struct iommu_domain *domain, struct device *dev,
 }
 #endif
 
-static int intel_iommu_map(struct iommu_domain *domain,
-			   unsigned long iova, phys_addr_t hpa,
-			   size_t size, int iommu_prot, gfp_t gfp)
+int intel_iommu_map(struct iommu_domain *domain, unsigned long iova,
+		    phys_addr_t hpa, size_t size, int iommu_prot, gfp_t gfp)
 {
 	struct dmar_domain *dmar_domain = to_dmar_domain(domain);
 	u64 max_addr;
@@ -4913,9 +4904,8 @@ static int intel_iommu_map(struct iommu_domain *domain,
 	return ret;
 }
 
-static size_t intel_iommu_unmap(struct iommu_domain *domain,
-				unsigned long iova, size_t size,
-				struct iommu_iotlb_gather *gather)
+size_t intel_iommu_unmap(struct iommu_domain *domain, unsigned long iova,
+			 size_t size, struct iommu_iotlb_gather *gather)
 {
 	struct dmar_domain *dmar_domain = to_dmar_domain(domain);
 	unsigned long start_pfn, last_pfn;
@@ -4963,8 +4953,7 @@ static void intel_iommu_tlb_sync(struct iommu_domain *domain,
 	dma_free_pagelist(gather->freelist);
 }
 
-static phys_addr_t intel_iommu_iova_to_phys(struct iommu_domain *domain,
-					    dma_addr_t iova)
+phys_addr_t intel_iommu_iova_to_phys(struct iommu_domain *domain, dma_addr_t iova)
 {
 	struct dmar_domain *dmar_domain = to_dmar_domain(domain);
 	struct dma_pte *pte;
@@ -4980,42 +4969,6 @@ 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 inline bool iommu_pasid_support(void)
-{
-	struct dmar_drhd_unit *drhd;
-	struct intel_iommu *iommu;
-	bool ret = true;
-
-	rcu_read_lock();
-	for_each_active_iommu(iommu, drhd) {
-		if (!pasid_supported(iommu)) {
-			ret = false;
-			break;
-		}
-	}
-	rcu_read_unlock();
-
-	return ret;
-}
-
 static inline bool nested_mode_support(void)
 {
 	struct dmar_drhd_unit *drhd;
@@ -5034,7 +4987,7 @@ static inline bool nested_mode_support(void)
 	return ret;
 }
 
-static bool intel_iommu_capable(enum iommu_cap cap)
+bool intel_iommu_capable(enum iommu_cap cap)
 {
 	if (cap == IOMMU_CAP_CACHE_COHERENCY)
 		return domain_update_iommu_snooping(NULL) == 1;
@@ -5084,8 +5037,7 @@ static void intel_iommu_probe_finalize(struct device *dev)
 		set_dma_ops(dev, NULL);
 }
 
-static void intel_iommu_get_resv_regions(struct device *device,
-					 struct list_head *head)
+void intel_iommu_get_resv_regions(struct device *device, struct list_head *head)
 {
 	int prot = DMA_PTE_READ | DMA_PTE_WRITE;
 	struct iommu_resv_region *reg;
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index fbf5b3e7707e..d3928ba87986 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -693,6 +693,42 @@ static inline int first_pte_in_page(struct dma_pte *pte)
 	return !((unsigned long)pte & ~VTD_PAGE_MASK);
 }
 
+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 inline bool iommu_pasid_support(void)
+{
+	struct dmar_drhd_unit *drhd;
+	struct intel_iommu *iommu;
+	bool ret = true;
+
+	rcu_read_lock();
+	for_each_active_iommu(iommu, drhd) {
+		if (!pasid_supported(iommu)) {
+			ret = false;
+			break;
+		}
+	}
+	rcu_read_unlock();
+
+	return ret;
+}
+
 extern struct dmar_drhd_unit * dmar_find_matched_drhd_unit(struct pci_dev *dev);
 extern int dmar_find_matched_atsr_unit(struct pci_dev *dev);
 
@@ -737,6 +773,19 @@ int intel_iommu_enable_pasid(struct intel_iommu *iommu, struct device *dev);
 struct dmar_domain *find_domain(struct device *dev);
 struct device_domain_info *get_domain_info(struct device *dev);
 struct intel_iommu *device_to_iommu(struct device *dev, u8 *bus, u8 *devfn);
+bool intel_iommu_capable(enum iommu_cap cap);
+struct iommu_domain *intel_iommu_domain_alloc(unsigned int type);
+void intel_iommu_domain_free(struct iommu_domain *domain);
+int intel_iommu_attach_device(struct iommu_domain *domain, struct device *dev);
+int intel_iommu_aux_attach_device(struct iommu_domain *domain, struct device *dev);
+void intel_iommu_detach_device(struct iommu_domain *domain, struct device *dev);
+void intel_iommu_aux_detach_device(struct iommu_domain *domain, struct device *dev);
+int intel_iommu_map(struct iommu_domain *domain, unsigned long iova,
+		    phys_addr_t hpa, size_t size, int iommu_prot, gfp_t gfp);
+size_t intel_iommu_unmap(struct iommu_domain *domain, unsigned long iova,
+			 size_t size, struct iommu_iotlb_gather *gather);
+phys_addr_t intel_iommu_iova_to_phys(struct iommu_domain *domain, dma_addr_t iova);
+void intel_iommu_get_resv_regions(struct device *device, struct list_head *head);
 
 #ifdef CONFIG_INTEL_IOMMU_SVM
 extern void intel_svm_check(struct intel_iommu *iommu);
-- 
2.25.1

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v6 4/5] iommu/vt-d: Add iommu_ops support for subdevice bus
  2020-10-30  4:58 ` Lu Baolu
@ 2020-10-30  4:58   ` Lu Baolu
  -1 siblings, 0 replies; 30+ messages in thread
From: Lu Baolu @ 2020-10-30  4:58 UTC (permalink / raw)
  To: Joerg Roedel, Alex Williamson
  Cc: Robin Murphy, Jean-Philippe Brucker, Cornelia Huck, Kevin Tian,
	Ashok Raj, Dave Jiang, Liu Yi L, Zeng Xin, iommu, linux-kernel,
	kvm, Lu Baolu

The iommu_ops will only take effect when INTEL_IOMMU_SCALABLE_IOV kernel
option is selected. It applies to any device passthrough framework which
implements an underlying bus for the subdevices.

- Subdevice probe:
  When a subdevice is created and added to the bus, iommu_probe_device()
  will be called, where the device will be probed by the iommu core. An
  iommu group will be allocated and the device will be added to it. The
  default domain won't be allocated since there's no use case of using a
  subdevice in the host kernel at this time being. However, it's pretty
  easy to add this support later.

- Domain alloc/free/map/unmap/iova_to_phys operations:
  For such ops, we just reuse those for PCI/PCIe devices.

- Domain attach/detach operations:
  It depends on whether the parent device supports IOMMU_DEV_FEAT_AUX
  feature. If so, the domain will be attached to the parent device as an
  aux-domain; Otherwise, it will be attached to the parent as a primary
  domain.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/intel/Kconfig  |  13 ++++
 drivers/iommu/intel/Makefile |   1 +
 drivers/iommu/intel/iommu.c  |   5 ++
 drivers/iommu/intel/siov.c   | 119 +++++++++++++++++++++++++++++++++++
 include/linux/intel-iommu.h  |   4 ++
 5 files changed, 142 insertions(+)
 create mode 100644 drivers/iommu/intel/siov.c

diff --git a/drivers/iommu/intel/Kconfig b/drivers/iommu/intel/Kconfig
index 28a3d1596c76..94edc332f558 100644
--- a/drivers/iommu/intel/Kconfig
+++ b/drivers/iommu/intel/Kconfig
@@ -86,3 +86,16 @@ config INTEL_IOMMU_SCALABLE_MODE_DEFAULT_ON
 	  is not selected, scalable mode support could also be enabled by
 	  passing intel_iommu=sm_on to the kernel. If not sure, please use
 	  the default value.
+
+config INTEL_IOMMU_SCALABLE_IOV
+	bool "Support for Intel Scalable I/O Virtualization"
+	depends on INTEL_IOMMU
+	select VFIO
+	select VFIO_MDEV
+	select VFIO_MDEV_DEVICE
+	help
+	  Intel Scalable I/O virtualization (SIOV) is a hardware-assisted
+	  PCIe subdevices virtualization. With each subdevice tagged with
+	  an unique ID(PCI/PASID) the VT-d hardware could identify, hence
+	  isolate DMA transactions from different subdevices on a same PCIe
+	  device. Selecting this option will enable the support.
diff --git a/drivers/iommu/intel/Makefile b/drivers/iommu/intel/Makefile
index fb8e1e8c8029..f216385d5d59 100644
--- a/drivers/iommu/intel/Makefile
+++ b/drivers/iommu/intel/Makefile
@@ -4,4 +4,5 @@ obj-$(CONFIG_INTEL_IOMMU) += iommu.o pasid.o
 obj-$(CONFIG_INTEL_IOMMU) += trace.o
 obj-$(CONFIG_INTEL_IOMMU_DEBUGFS) += debugfs.o
 obj-$(CONFIG_INTEL_IOMMU_SVM) += svm.o
+obj-$(CONFIG_INTEL_IOMMU_SCALABLE_IOV) += siov.o
 obj-$(CONFIG_IRQ_REMAP) += irq_remapping.o
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 1454fe74f3ba..dafd8069c2af 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -4298,6 +4298,11 @@ int __init intel_iommu_init(void)
 	up_read(&dmar_global_lock);
 
 	bus_set_iommu(&pci_bus_type, &intel_iommu_ops);
+
+#ifdef CONFIG_INTEL_IOMMU_SCALABLE_IOV
+	intel_siov_init();
+#endif /* CONFIG_INTEL_IOMMU_SCALABLE_IOV */
+
 	if (si_domain && !hw_pass_through)
 		register_memory_notifier(&intel_iommu_memory_nb);
 	cpuhp_setup_state(CPUHP_IOMMU_INTEL_DEAD, "iommu/intel:dead", NULL,
diff --git a/drivers/iommu/intel/siov.c b/drivers/iommu/intel/siov.c
new file mode 100644
index 000000000000..b9470e7ab3d6
--- /dev/null
+++ b/drivers/iommu/intel/siov.c
@@ -0,0 +1,119 @@
+// SPDX-License-Identifier: GPL-2.0
+/**
+ * siov.c - Intel Scalable I/O virtualization support
+ *
+ * Copyright (C) 2020 Intel Corporation
+ *
+ * Author: Lu Baolu <baolu.lu@linux.intel.com>
+ */
+
+#define pr_fmt(fmt)	"DMAR: " fmt
+
+#include <linux/intel-iommu.h>
+#include <linux/iommu.h>
+#include <linux/mdev.h>
+#include <linux/pci.h>
+
+static struct device *subdev_lookup_parent(struct device *dev)
+{
+	if (dev->bus == &mdev_bus_type)
+		return mdev_parent_dev(mdev_from_dev(dev));
+
+	return NULL;
+}
+
+static struct iommu_domain *siov_iommu_domain_alloc(unsigned int type)
+{
+	if (type != IOMMU_DOMAIN_UNMANAGED)
+		return NULL;
+
+	return intel_iommu_domain_alloc(type);
+}
+
+static int siov_iommu_attach_device(struct iommu_domain *domain,
+				    struct device *dev)
+{
+	struct device *parent;
+
+	parent = subdev_lookup_parent(dev);
+	if (!parent || !dev_is_pci(parent))
+		return -ENODEV;
+
+	if (iommu_dev_feature_enabled(parent, IOMMU_DEV_FEAT_AUX))
+		return intel_iommu_aux_attach_device(domain, parent);
+	else
+		return intel_iommu_attach_device(domain, parent);
+}
+
+static void siov_iommu_detach_device(struct iommu_domain *domain,
+				     struct device *dev)
+{
+	struct device *parent;
+
+	parent = subdev_lookup_parent(dev);
+	if (WARN_ON_ONCE(!parent || !dev_is_pci(parent)))
+		return;
+
+	if (iommu_dev_feature_enabled(parent, IOMMU_DEV_FEAT_AUX))
+		intel_iommu_aux_detach_device(domain, parent);
+	else
+		intel_iommu_detach_device(domain, parent);
+}
+
+static struct iommu_device *siov_iommu_probe_device(struct device *dev)
+{
+	struct intel_iommu *iommu;
+	struct device *parent;
+
+	parent = subdev_lookup_parent(dev);
+	if (!parent || !dev_is_pci(parent))
+		return ERR_PTR(-EINVAL);
+
+	iommu = device_to_iommu(parent, NULL, NULL);
+	if (!iommu)
+		return ERR_PTR(-ENODEV);
+
+	return &iommu->iommu;
+}
+
+static void siov_iommu_release_device(struct device *dev)
+{
+}
+
+static void
+siov_iommu_get_resv_regions(struct device *dev, struct list_head *head)
+{
+	struct device *parent;
+
+	parent = subdev_lookup_parent(dev);
+	if (!parent || !dev_is_pci(parent))
+		return;
+
+	intel_iommu_get_resv_regions(parent, head);
+}
+
+static const struct iommu_ops siov_iommu_ops = {
+	.capable		= intel_iommu_capable,
+	.domain_alloc		= siov_iommu_domain_alloc,
+	.domain_free		= intel_iommu_domain_free,
+	.attach_dev		= siov_iommu_attach_device,
+	.detach_dev		= siov_iommu_detach_device,
+	.map			= intel_iommu_map,
+	.unmap			= intel_iommu_unmap,
+	.iova_to_phys		= intel_iommu_iova_to_phys,
+	.probe_device		= siov_iommu_probe_device,
+	.release_device		= siov_iommu_release_device,
+	.get_resv_regions	= siov_iommu_get_resv_regions,
+	.put_resv_regions	= generic_iommu_put_resv_regions,
+	.device_group		= generic_device_group,
+	.pgsize_bitmap		= (~0xFFFUL),
+};
+
+void intel_siov_init(void)
+{
+	if (!scalable_mode_support() || !iommu_pasid_support())
+		return;
+
+	bus_set_iommu(&mdev_bus_type, &siov_iommu_ops);
+	pr_info("Intel(R) Scalable I/O Virtualization supported\n");
+}
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index d3928ba87986..b790efa5509f 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -787,6 +787,10 @@ size_t intel_iommu_unmap(struct iommu_domain *domain, unsigned long iova,
 phys_addr_t intel_iommu_iova_to_phys(struct iommu_domain *domain, dma_addr_t iova);
 void intel_iommu_get_resv_regions(struct device *device, struct list_head *head);
 
+#ifdef CONFIG_INTEL_IOMMU_SCALABLE_IOV
+void intel_siov_init(void);
+#endif /* INTEL_IOMMU_SCALABLE_IOV */
+
 #ifdef CONFIG_INTEL_IOMMU_SVM
 extern void intel_svm_check(struct intel_iommu *iommu);
 extern int intel_svm_enable_prq(struct intel_iommu *iommu);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v6 4/5] iommu/vt-d: Add iommu_ops support for subdevice bus
@ 2020-10-30  4:58   ` Lu Baolu
  0 siblings, 0 replies; 30+ messages in thread
From: Lu Baolu @ 2020-10-30  4:58 UTC (permalink / raw)
  To: Joerg Roedel, Alex Williamson
  Cc: Jean-Philippe Brucker, Kevin Tian, Dave Jiang, Ashok Raj, kvm,
	Cornelia Huck, linux-kernel, iommu, Robin Murphy

The iommu_ops will only take effect when INTEL_IOMMU_SCALABLE_IOV kernel
option is selected. It applies to any device passthrough framework which
implements an underlying bus for the subdevices.

- Subdevice probe:
  When a subdevice is created and added to the bus, iommu_probe_device()
  will be called, where the device will be probed by the iommu core. An
  iommu group will be allocated and the device will be added to it. The
  default domain won't be allocated since there's no use case of using a
  subdevice in the host kernel at this time being. However, it's pretty
  easy to add this support later.

- Domain alloc/free/map/unmap/iova_to_phys operations:
  For such ops, we just reuse those for PCI/PCIe devices.

- Domain attach/detach operations:
  It depends on whether the parent device supports IOMMU_DEV_FEAT_AUX
  feature. If so, the domain will be attached to the parent device as an
  aux-domain; Otherwise, it will be attached to the parent as a primary
  domain.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/intel/Kconfig  |  13 ++++
 drivers/iommu/intel/Makefile |   1 +
 drivers/iommu/intel/iommu.c  |   5 ++
 drivers/iommu/intel/siov.c   | 119 +++++++++++++++++++++++++++++++++++
 include/linux/intel-iommu.h  |   4 ++
 5 files changed, 142 insertions(+)
 create mode 100644 drivers/iommu/intel/siov.c

diff --git a/drivers/iommu/intel/Kconfig b/drivers/iommu/intel/Kconfig
index 28a3d1596c76..94edc332f558 100644
--- a/drivers/iommu/intel/Kconfig
+++ b/drivers/iommu/intel/Kconfig
@@ -86,3 +86,16 @@ config INTEL_IOMMU_SCALABLE_MODE_DEFAULT_ON
 	  is not selected, scalable mode support could also be enabled by
 	  passing intel_iommu=sm_on to the kernel. If not sure, please use
 	  the default value.
+
+config INTEL_IOMMU_SCALABLE_IOV
+	bool "Support for Intel Scalable I/O Virtualization"
+	depends on INTEL_IOMMU
+	select VFIO
+	select VFIO_MDEV
+	select VFIO_MDEV_DEVICE
+	help
+	  Intel Scalable I/O virtualization (SIOV) is a hardware-assisted
+	  PCIe subdevices virtualization. With each subdevice tagged with
+	  an unique ID(PCI/PASID) the VT-d hardware could identify, hence
+	  isolate DMA transactions from different subdevices on a same PCIe
+	  device. Selecting this option will enable the support.
diff --git a/drivers/iommu/intel/Makefile b/drivers/iommu/intel/Makefile
index fb8e1e8c8029..f216385d5d59 100644
--- a/drivers/iommu/intel/Makefile
+++ b/drivers/iommu/intel/Makefile
@@ -4,4 +4,5 @@ obj-$(CONFIG_INTEL_IOMMU) += iommu.o pasid.o
 obj-$(CONFIG_INTEL_IOMMU) += trace.o
 obj-$(CONFIG_INTEL_IOMMU_DEBUGFS) += debugfs.o
 obj-$(CONFIG_INTEL_IOMMU_SVM) += svm.o
+obj-$(CONFIG_INTEL_IOMMU_SCALABLE_IOV) += siov.o
 obj-$(CONFIG_IRQ_REMAP) += irq_remapping.o
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 1454fe74f3ba..dafd8069c2af 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -4298,6 +4298,11 @@ int __init intel_iommu_init(void)
 	up_read(&dmar_global_lock);
 
 	bus_set_iommu(&pci_bus_type, &intel_iommu_ops);
+
+#ifdef CONFIG_INTEL_IOMMU_SCALABLE_IOV
+	intel_siov_init();
+#endif /* CONFIG_INTEL_IOMMU_SCALABLE_IOV */
+
 	if (si_domain && !hw_pass_through)
 		register_memory_notifier(&intel_iommu_memory_nb);
 	cpuhp_setup_state(CPUHP_IOMMU_INTEL_DEAD, "iommu/intel:dead", NULL,
diff --git a/drivers/iommu/intel/siov.c b/drivers/iommu/intel/siov.c
new file mode 100644
index 000000000000..b9470e7ab3d6
--- /dev/null
+++ b/drivers/iommu/intel/siov.c
@@ -0,0 +1,119 @@
+// SPDX-License-Identifier: GPL-2.0
+/**
+ * siov.c - Intel Scalable I/O virtualization support
+ *
+ * Copyright (C) 2020 Intel Corporation
+ *
+ * Author: Lu Baolu <baolu.lu@linux.intel.com>
+ */
+
+#define pr_fmt(fmt)	"DMAR: " fmt
+
+#include <linux/intel-iommu.h>
+#include <linux/iommu.h>
+#include <linux/mdev.h>
+#include <linux/pci.h>
+
+static struct device *subdev_lookup_parent(struct device *dev)
+{
+	if (dev->bus == &mdev_bus_type)
+		return mdev_parent_dev(mdev_from_dev(dev));
+
+	return NULL;
+}
+
+static struct iommu_domain *siov_iommu_domain_alloc(unsigned int type)
+{
+	if (type != IOMMU_DOMAIN_UNMANAGED)
+		return NULL;
+
+	return intel_iommu_domain_alloc(type);
+}
+
+static int siov_iommu_attach_device(struct iommu_domain *domain,
+				    struct device *dev)
+{
+	struct device *parent;
+
+	parent = subdev_lookup_parent(dev);
+	if (!parent || !dev_is_pci(parent))
+		return -ENODEV;
+
+	if (iommu_dev_feature_enabled(parent, IOMMU_DEV_FEAT_AUX))
+		return intel_iommu_aux_attach_device(domain, parent);
+	else
+		return intel_iommu_attach_device(domain, parent);
+}
+
+static void siov_iommu_detach_device(struct iommu_domain *domain,
+				     struct device *dev)
+{
+	struct device *parent;
+
+	parent = subdev_lookup_parent(dev);
+	if (WARN_ON_ONCE(!parent || !dev_is_pci(parent)))
+		return;
+
+	if (iommu_dev_feature_enabled(parent, IOMMU_DEV_FEAT_AUX))
+		intel_iommu_aux_detach_device(domain, parent);
+	else
+		intel_iommu_detach_device(domain, parent);
+}
+
+static struct iommu_device *siov_iommu_probe_device(struct device *dev)
+{
+	struct intel_iommu *iommu;
+	struct device *parent;
+
+	parent = subdev_lookup_parent(dev);
+	if (!parent || !dev_is_pci(parent))
+		return ERR_PTR(-EINVAL);
+
+	iommu = device_to_iommu(parent, NULL, NULL);
+	if (!iommu)
+		return ERR_PTR(-ENODEV);
+
+	return &iommu->iommu;
+}
+
+static void siov_iommu_release_device(struct device *dev)
+{
+}
+
+static void
+siov_iommu_get_resv_regions(struct device *dev, struct list_head *head)
+{
+	struct device *parent;
+
+	parent = subdev_lookup_parent(dev);
+	if (!parent || !dev_is_pci(parent))
+		return;
+
+	intel_iommu_get_resv_regions(parent, head);
+}
+
+static const struct iommu_ops siov_iommu_ops = {
+	.capable		= intel_iommu_capable,
+	.domain_alloc		= siov_iommu_domain_alloc,
+	.domain_free		= intel_iommu_domain_free,
+	.attach_dev		= siov_iommu_attach_device,
+	.detach_dev		= siov_iommu_detach_device,
+	.map			= intel_iommu_map,
+	.unmap			= intel_iommu_unmap,
+	.iova_to_phys		= intel_iommu_iova_to_phys,
+	.probe_device		= siov_iommu_probe_device,
+	.release_device		= siov_iommu_release_device,
+	.get_resv_regions	= siov_iommu_get_resv_regions,
+	.put_resv_regions	= generic_iommu_put_resv_regions,
+	.device_group		= generic_device_group,
+	.pgsize_bitmap		= (~0xFFFUL),
+};
+
+void intel_siov_init(void)
+{
+	if (!scalable_mode_support() || !iommu_pasid_support())
+		return;
+
+	bus_set_iommu(&mdev_bus_type, &siov_iommu_ops);
+	pr_info("Intel(R) Scalable I/O Virtualization supported\n");
+}
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index d3928ba87986..b790efa5509f 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -787,6 +787,10 @@ size_t intel_iommu_unmap(struct iommu_domain *domain, unsigned long iova,
 phys_addr_t intel_iommu_iova_to_phys(struct iommu_domain *domain, dma_addr_t iova);
 void intel_iommu_get_resv_regions(struct device *device, struct list_head *head);
 
+#ifdef CONFIG_INTEL_IOMMU_SCALABLE_IOV
+void intel_siov_init(void);
+#endif /* INTEL_IOMMU_SCALABLE_IOV */
+
 #ifdef CONFIG_INTEL_IOMMU_SVM
 extern void intel_svm_check(struct intel_iommu *iommu);
 extern int intel_svm_enable_prq(struct intel_iommu *iommu);
-- 
2.25.1

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v6 5/5] vfio/type1: Use mdev bus iommu_ops for IOMMU callbacks
  2020-10-30  4:58 ` Lu Baolu
@ 2020-10-30  4:58   ` Lu Baolu
  -1 siblings, 0 replies; 30+ messages in thread
From: Lu Baolu @ 2020-10-30  4:58 UTC (permalink / raw)
  To: Joerg Roedel, Alex Williamson
  Cc: Robin Murphy, Jean-Philippe Brucker, Cornelia Huck, Kevin Tian,
	Ashok Raj, Dave Jiang, Liu Yi L, Zeng Xin, iommu, linux-kernel,
	kvm, Lu Baolu

With the IOMMU driver registering iommu_ops for the mdev_bus, the IOMMU
operations on an mdev could be done in the same way as any normal device
(for example, PCI/PCIe). There's no need to distinguish an mdev from
others for iommu operations. Remove the unnecessary code.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/vfio/mdev/mdev_core.c    |  18 -----
 drivers/vfio/mdev/mdev_driver.c  |   6 ++
 drivers/vfio/mdev/mdev_private.h |   1 -
 drivers/vfio/vfio_iommu_type1.c  | 128 +++----------------------------
 include/linux/mdev.h             |  14 ----
 5 files changed, 18 insertions(+), 149 deletions(-)

diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
index 6b9ab71f89e7..f4fd5f237c49 100644
--- a/drivers/vfio/mdev/mdev_core.c
+++ b/drivers/vfio/mdev/mdev_core.c
@@ -386,24 +386,6 @@ int mdev_device_remove(struct device *dev)
 	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);
-
 static int __init mdev_init(void)
 {
 	return mdev_bus_register();
diff --git a/drivers/vfio/mdev/mdev_driver.c b/drivers/vfio/mdev/mdev_driver.c
index 0d3223aee20b..487402f16355 100644
--- a/drivers/vfio/mdev/mdev_driver.c
+++ b/drivers/vfio/mdev/mdev_driver.c
@@ -18,6 +18,9 @@ static int mdev_attach_iommu(struct mdev_device *mdev)
 	int ret;
 	struct iommu_group *group;
 
+	if (iommu_present(&mdev_bus_type))
+		return 0;
+
 	group = iommu_group_alloc();
 	if (IS_ERR(group))
 		return PTR_ERR(group);
@@ -33,6 +36,9 @@ static int mdev_attach_iommu(struct mdev_device *mdev)
 
 static void mdev_detach_iommu(struct mdev_device *mdev)
 {
+	if (iommu_present(&mdev_bus_type))
+		return;
+
 	iommu_group_remove_device(&mdev->dev);
 	dev_info(&mdev->dev, "MDEV: detaching iommu\n");
 }
diff --git a/drivers/vfio/mdev/mdev_private.h b/drivers/vfio/mdev/mdev_private.h
index 7d922950caaf..efe0aefdb52f 100644
--- a/drivers/vfio/mdev/mdev_private.h
+++ b/drivers/vfio/mdev/mdev_private.h
@@ -31,7 +31,6 @@ struct mdev_device {
 	void *driver_data;
 	struct list_head next;
 	struct kobject *type_kobj;
-	struct device *iommu_device;
 	bool active;
 };
 
diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index bb2684cc245e..e231b7070ca5 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -100,7 +100,6 @@ struct vfio_dma {
 struct vfio_group {
 	struct iommu_group	*iommu_group;
 	struct list_head	next;
-	bool			mdev_group;	/* An mdev group */
 	bool			pinned_page_dirty_scope;
 };
 
@@ -1675,102 +1674,6 @@ static bool vfio_iommu_has_sw_msi(struct list_head *group_resv_regions,
 	return ret;
 }
 
-static struct device *vfio_mdev_get_iommu_device(struct device *dev)
-{
-	struct device *(*fn)(struct device *dev);
-	struct device *iommu_device;
-
-	fn = symbol_get(mdev_get_iommu_device);
-	if (fn) {
-		iommu_device = fn(dev);
-		symbol_put(mdev_get_iommu_device);
-
-		return iommu_device;
-	}
-
-	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);
-	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 bool vfio_bus_is_mdev(struct bus_type *bus)
-{
-	struct bus_type *mdev_bus;
-	bool ret = false;
-
-	mdev_bus = symbol_get(mdev_bus_type);
-	if (mdev_bus) {
-		ret = (bus == mdev_bus);
-		symbol_put(mdev_bus_type);
-	}
-
-	return ret;
-}
-
-static int vfio_mdev_iommu_device(struct device *dev, void *data)
-{
-	struct device **old = data, *new;
-
-	new = vfio_mdev_get_iommu_device(dev);
-	if (!new || (*old && *old != new))
-		return -EINVAL;
-
-	*old = new;
-
-	return 0;
-}
-
 /*
  * This is a helper function to insert an address range to iova list.
  * The list is initially created with a single entry corresponding to
@@ -1999,7 +1902,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
 	struct vfio_iommu *iommu = iommu_data;
 	struct vfio_group *group;
 	struct vfio_domain *domain, *d;
-	struct bus_type *bus = NULL;
+	struct bus_type *bus = NULL, *mdev_bus;
 	int ret;
 	bool resv_msi, msi_remap;
 	phys_addr_t resv_msi_base = 0;
@@ -2037,15 +1940,10 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
 	if (ret)
 		goto out_free;
 
-	if (vfio_bus_is_mdev(bus)) {
-		struct device *iommu_device = NULL;
-
-		group->mdev_group = true;
-
-		/* Determine the isolation type */
-		ret = iommu_group_for_each_dev(iommu_group, &iommu_device,
-					       vfio_mdev_iommu_device);
-		if (ret || !iommu_device) {
+	mdev_bus = symbol_get(mdev_bus_type);
+	if (mdev_bus) {
+		if (bus == mdev_bus && !iommu_present(bus)) {
+			symbol_put(mdev_bus_type);
 			if (!iommu->external_domain) {
 				INIT_LIST_HEAD(&domain->group_list);
 				iommu->external_domain = domain;
@@ -2070,8 +1968,6 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
 
 			return 0;
 		}
-
-		bus = iommu_device->bus;
 	}
 
 	domain->domain = iommu_domain_alloc(bus);
@@ -2089,7 +1985,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
 			goto out_domain;
 	}
 
-	ret = vfio_iommu_attach_group(domain, group);
+	ret = iommu_attach_group(domain->domain, iommu_group);
 	if (ret)
 		goto out_domain;
 
@@ -2157,15 +2053,15 @@ 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) {
-			vfio_iommu_detach_group(domain, group);
-			if (!vfio_iommu_attach_group(d, group)) {
+			iommu_detach_group(domain->domain, iommu_group);
+			if (!iommu_attach_group(d->domain, iommu_group)) {
 				list_add(&group->next, &d->group_list);
 				iommu_domain_free(domain->domain);
 				kfree(domain);
 				goto done;
 			}
 
-			ret = vfio_iommu_attach_group(domain, group);
+			ret = iommu_attach_group(domain->domain, iommu_group);
 			if (ret)
 				goto out_domain;
 		}
@@ -2202,7 +2098,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
 	return 0;
 
 out_detach:
-	vfio_iommu_detach_group(domain, group);
+	iommu_detach_group(domain->domain, iommu_group);
 out_domain:
 	iommu_domain_free(domain->domain);
 	vfio_iommu_iova_free(&iova_copy);
@@ -2385,7 +2281,7 @@ static void vfio_iommu_type1_detach_group(void *iommu_data,
 		if (!group)
 			continue;
 
-		vfio_iommu_detach_group(domain, group);
+		iommu_detach_group(domain->domain, iommu_group);
 		update_dirty_scope = !group->pinned_page_dirty_scope;
 		list_del(&group->next);
 		kfree(group);
@@ -2466,7 +2362,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)
-			vfio_iommu_detach_group(domain, group);
+			iommu_detach_group(domain->domain, group->iommu_group);
 		list_del(&group->next);
 		kfree(group);
 	}
diff --git a/include/linux/mdev.h b/include/linux/mdev.h
index 0ce30ca78db0..f7aee86bd2b0 100644
--- a/include/linux/mdev.h
+++ b/include/linux/mdev.h
@@ -12,20 +12,6 @@
 
 struct mdev_device;
 
-/*
- * Called by the parent device driver to set the device which represents
- * 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);
-
 /**
  * struct mdev_parent_ops - Structure to be registered for each parent device to
  * register the device to mdev module.
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v6 5/5] vfio/type1: Use mdev bus iommu_ops for IOMMU callbacks
@ 2020-10-30  4:58   ` Lu Baolu
  0 siblings, 0 replies; 30+ messages in thread
From: Lu Baolu @ 2020-10-30  4:58 UTC (permalink / raw)
  To: Joerg Roedel, Alex Williamson
  Cc: Jean-Philippe Brucker, Kevin Tian, Dave Jiang, Ashok Raj, kvm,
	Cornelia Huck, linux-kernel, iommu, Robin Murphy

With the IOMMU driver registering iommu_ops for the mdev_bus, the IOMMU
operations on an mdev could be done in the same way as any normal device
(for example, PCI/PCIe). There's no need to distinguish an mdev from
others for iommu operations. Remove the unnecessary code.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/vfio/mdev/mdev_core.c    |  18 -----
 drivers/vfio/mdev/mdev_driver.c  |   6 ++
 drivers/vfio/mdev/mdev_private.h |   1 -
 drivers/vfio/vfio_iommu_type1.c  | 128 +++----------------------------
 include/linux/mdev.h             |  14 ----
 5 files changed, 18 insertions(+), 149 deletions(-)

diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
index 6b9ab71f89e7..f4fd5f237c49 100644
--- a/drivers/vfio/mdev/mdev_core.c
+++ b/drivers/vfio/mdev/mdev_core.c
@@ -386,24 +386,6 @@ int mdev_device_remove(struct device *dev)
 	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);
-
 static int __init mdev_init(void)
 {
 	return mdev_bus_register();
diff --git a/drivers/vfio/mdev/mdev_driver.c b/drivers/vfio/mdev/mdev_driver.c
index 0d3223aee20b..487402f16355 100644
--- a/drivers/vfio/mdev/mdev_driver.c
+++ b/drivers/vfio/mdev/mdev_driver.c
@@ -18,6 +18,9 @@ static int mdev_attach_iommu(struct mdev_device *mdev)
 	int ret;
 	struct iommu_group *group;
 
+	if (iommu_present(&mdev_bus_type))
+		return 0;
+
 	group = iommu_group_alloc();
 	if (IS_ERR(group))
 		return PTR_ERR(group);
@@ -33,6 +36,9 @@ static int mdev_attach_iommu(struct mdev_device *mdev)
 
 static void mdev_detach_iommu(struct mdev_device *mdev)
 {
+	if (iommu_present(&mdev_bus_type))
+		return;
+
 	iommu_group_remove_device(&mdev->dev);
 	dev_info(&mdev->dev, "MDEV: detaching iommu\n");
 }
diff --git a/drivers/vfio/mdev/mdev_private.h b/drivers/vfio/mdev/mdev_private.h
index 7d922950caaf..efe0aefdb52f 100644
--- a/drivers/vfio/mdev/mdev_private.h
+++ b/drivers/vfio/mdev/mdev_private.h
@@ -31,7 +31,6 @@ struct mdev_device {
 	void *driver_data;
 	struct list_head next;
 	struct kobject *type_kobj;
-	struct device *iommu_device;
 	bool active;
 };
 
diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index bb2684cc245e..e231b7070ca5 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -100,7 +100,6 @@ struct vfio_dma {
 struct vfio_group {
 	struct iommu_group	*iommu_group;
 	struct list_head	next;
-	bool			mdev_group;	/* An mdev group */
 	bool			pinned_page_dirty_scope;
 };
 
@@ -1675,102 +1674,6 @@ static bool vfio_iommu_has_sw_msi(struct list_head *group_resv_regions,
 	return ret;
 }
 
-static struct device *vfio_mdev_get_iommu_device(struct device *dev)
-{
-	struct device *(*fn)(struct device *dev);
-	struct device *iommu_device;
-
-	fn = symbol_get(mdev_get_iommu_device);
-	if (fn) {
-		iommu_device = fn(dev);
-		symbol_put(mdev_get_iommu_device);
-
-		return iommu_device;
-	}
-
-	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);
-	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 bool vfio_bus_is_mdev(struct bus_type *bus)
-{
-	struct bus_type *mdev_bus;
-	bool ret = false;
-
-	mdev_bus = symbol_get(mdev_bus_type);
-	if (mdev_bus) {
-		ret = (bus == mdev_bus);
-		symbol_put(mdev_bus_type);
-	}
-
-	return ret;
-}
-
-static int vfio_mdev_iommu_device(struct device *dev, void *data)
-{
-	struct device **old = data, *new;
-
-	new = vfio_mdev_get_iommu_device(dev);
-	if (!new || (*old && *old != new))
-		return -EINVAL;
-
-	*old = new;
-
-	return 0;
-}
-
 /*
  * This is a helper function to insert an address range to iova list.
  * The list is initially created with a single entry corresponding to
@@ -1999,7 +1902,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
 	struct vfio_iommu *iommu = iommu_data;
 	struct vfio_group *group;
 	struct vfio_domain *domain, *d;
-	struct bus_type *bus = NULL;
+	struct bus_type *bus = NULL, *mdev_bus;
 	int ret;
 	bool resv_msi, msi_remap;
 	phys_addr_t resv_msi_base = 0;
@@ -2037,15 +1940,10 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
 	if (ret)
 		goto out_free;
 
-	if (vfio_bus_is_mdev(bus)) {
-		struct device *iommu_device = NULL;
-
-		group->mdev_group = true;
-
-		/* Determine the isolation type */
-		ret = iommu_group_for_each_dev(iommu_group, &iommu_device,
-					       vfio_mdev_iommu_device);
-		if (ret || !iommu_device) {
+	mdev_bus = symbol_get(mdev_bus_type);
+	if (mdev_bus) {
+		if (bus == mdev_bus && !iommu_present(bus)) {
+			symbol_put(mdev_bus_type);
 			if (!iommu->external_domain) {
 				INIT_LIST_HEAD(&domain->group_list);
 				iommu->external_domain = domain;
@@ -2070,8 +1968,6 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
 
 			return 0;
 		}
-
-		bus = iommu_device->bus;
 	}
 
 	domain->domain = iommu_domain_alloc(bus);
@@ -2089,7 +1985,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
 			goto out_domain;
 	}
 
-	ret = vfio_iommu_attach_group(domain, group);
+	ret = iommu_attach_group(domain->domain, iommu_group);
 	if (ret)
 		goto out_domain;
 
@@ -2157,15 +2053,15 @@ 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) {
-			vfio_iommu_detach_group(domain, group);
-			if (!vfio_iommu_attach_group(d, group)) {
+			iommu_detach_group(domain->domain, iommu_group);
+			if (!iommu_attach_group(d->domain, iommu_group)) {
 				list_add(&group->next, &d->group_list);
 				iommu_domain_free(domain->domain);
 				kfree(domain);
 				goto done;
 			}
 
-			ret = vfio_iommu_attach_group(domain, group);
+			ret = iommu_attach_group(domain->domain, iommu_group);
 			if (ret)
 				goto out_domain;
 		}
@@ -2202,7 +2098,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
 	return 0;
 
 out_detach:
-	vfio_iommu_detach_group(domain, group);
+	iommu_detach_group(domain->domain, iommu_group);
 out_domain:
 	iommu_domain_free(domain->domain);
 	vfio_iommu_iova_free(&iova_copy);
@@ -2385,7 +2281,7 @@ static void vfio_iommu_type1_detach_group(void *iommu_data,
 		if (!group)
 			continue;
 
-		vfio_iommu_detach_group(domain, group);
+		iommu_detach_group(domain->domain, iommu_group);
 		update_dirty_scope = !group->pinned_page_dirty_scope;
 		list_del(&group->next);
 		kfree(group);
@@ -2466,7 +2362,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)
-			vfio_iommu_detach_group(domain, group);
+			iommu_detach_group(domain->domain, group->iommu_group);
 		list_del(&group->next);
 		kfree(group);
 	}
diff --git a/include/linux/mdev.h b/include/linux/mdev.h
index 0ce30ca78db0..f7aee86bd2b0 100644
--- a/include/linux/mdev.h
+++ b/include/linux/mdev.h
@@ -12,20 +12,6 @@
 
 struct mdev_device;
 
-/*
- * Called by the parent device driver to set the device which represents
- * 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);
-
 /**
  * struct mdev_parent_ops - Structure to be registered for each parent device to
  * register the device to mdev module.
-- 
2.25.1

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* RE: [PATCH v6 2/5] iommu: Use bus iommu ops for aux related callback
  2020-10-30  4:58   ` Lu Baolu
@ 2020-10-30  5:55     ` Tian, Kevin
  -1 siblings, 0 replies; 30+ messages in thread
From: Tian, Kevin @ 2020-10-30  5:55 UTC (permalink / raw)
  To: Lu Baolu, Joerg Roedel, Alex Williamson
  Cc: Robin Murphy, Jean-Philippe Brucker, Cornelia Huck, Raj, Ashok,
	Jiang, Dave, Liu, Yi L, Zeng, Xin, iommu, linux-kernel, kvm

> From: Lu Baolu <baolu.lu@linux.intel.com>
> Sent: Friday, October 30, 2020 12:58 PM
> 
> The aux-domain apis were designed for macro driver where the subdevices
> are created and used inside a device driver. Use the device's bus iommu
> ops instead of that in iommu domain for various callbacks.

IIRC there are only two users on these apis. One is VFIO, and the other
is on the ARM side (not checked in yet). Jean, can you help confirm 
whether ARM-side usage still relies on aux apis even with this change?
If no, possibly they can be removed completely?

Thanks
Kevin

> 
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> ---
>  drivers/iommu/iommu.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 6bbdd959f9f3..17f2686664db 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -2913,10 +2913,11 @@
> EXPORT_SYMBOL_GPL(iommu_dev_feature_enabled);
>   */
>  int iommu_aux_attach_device(struct iommu_domain *domain, struct device
> *dev)
>  {
> +	const struct iommu_ops *ops = dev->bus->iommu_ops;
>  	int ret = -ENODEV;
> 
> -	if (domain->ops->aux_attach_dev)
> -		ret = domain->ops->aux_attach_dev(domain, dev);
> +	if (ops && ops->aux_attach_dev)
> +		ret = ops->aux_attach_dev(domain, dev);
> 
>  	if (!ret)
>  		trace_attach_device_to_domain(dev);
> @@ -2927,8 +2928,10 @@
> EXPORT_SYMBOL_GPL(iommu_aux_attach_device);
> 
>  void iommu_aux_detach_device(struct iommu_domain *domain, struct
> device *dev)
>  {
> -	if (domain->ops->aux_detach_dev) {
> -		domain->ops->aux_detach_dev(domain, dev);
> +	const struct iommu_ops *ops = dev->bus->iommu_ops;
> +
> +	if (ops && ops->aux_detach_dev) {
> +		ops->aux_detach_dev(domain, dev);
>  		trace_detach_device_from_domain(dev);
>  	}
>  }
> @@ -2936,10 +2939,11 @@
> EXPORT_SYMBOL_GPL(iommu_aux_detach_device);
> 
>  int iommu_aux_get_pasid(struct iommu_domain *domain, struct device
> *dev)
>  {
> +	const struct iommu_ops *ops = dev->bus->iommu_ops;
>  	int ret = -ENODEV;
> 
> -	if (domain->ops->aux_get_pasid)
> -		ret = domain->ops->aux_get_pasid(domain, dev);
> +	if (ops && ops->aux_get_pasid)
> +		ret = ops->aux_get_pasid(domain, dev);
> 
>  	return ret;
>  }
> --
> 2.25.1


^ permalink raw reply	[flat|nested] 30+ messages in thread

* RE: [PATCH v6 2/5] iommu: Use bus iommu ops for aux related callback
@ 2020-10-30  5:55     ` Tian, Kevin
  0 siblings, 0 replies; 30+ messages in thread
From: Tian, Kevin @ 2020-10-30  5:55 UTC (permalink / raw)
  To: Lu Baolu, Joerg Roedel, Alex Williamson
  Cc: Jean-Philippe Brucker, Jiang, Dave, Raj, Ashok, kvm,
	Cornelia Huck, linux-kernel, iommu, Robin Murphy

> From: Lu Baolu <baolu.lu@linux.intel.com>
> Sent: Friday, October 30, 2020 12:58 PM
> 
> The aux-domain apis were designed for macro driver where the subdevices
> are created and used inside a device driver. Use the device's bus iommu
> ops instead of that in iommu domain for various callbacks.

IIRC there are only two users on these apis. One is VFIO, and the other
is on the ARM side (not checked in yet). Jean, can you help confirm 
whether ARM-side usage still relies on aux apis even with this change?
If no, possibly they can be removed completely?

Thanks
Kevin

> 
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> ---
>  drivers/iommu/iommu.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 6bbdd959f9f3..17f2686664db 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -2913,10 +2913,11 @@
> EXPORT_SYMBOL_GPL(iommu_dev_feature_enabled);
>   */
>  int iommu_aux_attach_device(struct iommu_domain *domain, struct device
> *dev)
>  {
> +	const struct iommu_ops *ops = dev->bus->iommu_ops;
>  	int ret = -ENODEV;
> 
> -	if (domain->ops->aux_attach_dev)
> -		ret = domain->ops->aux_attach_dev(domain, dev);
> +	if (ops && ops->aux_attach_dev)
> +		ret = ops->aux_attach_dev(domain, dev);
> 
>  	if (!ret)
>  		trace_attach_device_to_domain(dev);
> @@ -2927,8 +2928,10 @@
> EXPORT_SYMBOL_GPL(iommu_aux_attach_device);
> 
>  void iommu_aux_detach_device(struct iommu_domain *domain, struct
> device *dev)
>  {
> -	if (domain->ops->aux_detach_dev) {
> -		domain->ops->aux_detach_dev(domain, dev);
> +	const struct iommu_ops *ops = dev->bus->iommu_ops;
> +
> +	if (ops && ops->aux_detach_dev) {
> +		ops->aux_detach_dev(domain, dev);
>  		trace_detach_device_from_domain(dev);
>  	}
>  }
> @@ -2936,10 +2939,11 @@
> EXPORT_SYMBOL_GPL(iommu_aux_detach_device);
> 
>  int iommu_aux_get_pasid(struct iommu_domain *domain, struct device
> *dev)
>  {
> +	const struct iommu_ops *ops = dev->bus->iommu_ops;
>  	int ret = -ENODEV;
> 
> -	if (domain->ops->aux_get_pasid)
> -		ret = domain->ops->aux_get_pasid(domain, dev);
> +	if (ops && ops->aux_get_pasid)
> +		ret = ops->aux_get_pasid(domain, dev);
> 
>  	return ret;
>  }
> --
> 2.25.1

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 30+ messages in thread

* RE: [PATCH v6 4/5] iommu/vt-d: Add iommu_ops support for subdevice bus
  2020-10-30  4:58   ` Lu Baolu
@ 2020-10-30  6:13     ` Tian, Kevin
  -1 siblings, 0 replies; 30+ messages in thread
From: Tian, Kevin @ 2020-10-30  6:13 UTC (permalink / raw)
  To: Lu Baolu, Joerg Roedel, Alex Williamson
  Cc: Robin Murphy, Jean-Philippe Brucker, Cornelia Huck, Raj, Ashok,
	Jiang, Dave, Liu, Yi L, Zeng, Xin, iommu, linux-kernel, kvm,
	Kirti Wankhede

> From: Lu Baolu <baolu.lu@linux.intel.com>
> Sent: Friday, October 30, 2020 12:58 PM
> 
> The iommu_ops will only take effect when INTEL_IOMMU_SCALABLE_IOV
> kernel
> option is selected. It applies to any device passthrough framework which
> implements an underlying bus for the subdevices.
> 
> - Subdevice probe:
>   When a subdevice is created and added to the bus, iommu_probe_device()
>   will be called, where the device will be probed by the iommu core. An
>   iommu group will be allocated and the device will be added to it. The
>   default domain won't be allocated since there's no use case of using a
>   subdevice in the host kernel at this time being. However, it's pretty
>   easy to add this support later.
> 
> - Domain alloc/free/map/unmap/iova_to_phys operations:
>   For such ops, we just reuse those for PCI/PCIe devices.

One question. Just be curious whether every IOMMU vendor supports
only one iommu_ops for all bus types. For Intel obviously the answer is
yes. But if a vendor supports bus-type specific iommu_ops for physical
devices, this may impose a restriction to VFIO or other passthrough 
frameworks, because VFIO today maintains only one mdev bus while the 
parent devices could come from different bus types. In the end the ops
for subdevice should be same as the one used for the parent. Then it may 
require VFIO to organize subdevices based on parent bus types. 

> 
> - Domain attach/detach operations:
>   It depends on whether the parent device supports IOMMU_DEV_FEAT_AUX
>   feature. If so, the domain will be attached to the parent device as an
>   aux-domain; Otherwise, it will be attached to the parent as a primary
>   domain.
> 
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> ---
>  drivers/iommu/intel/Kconfig  |  13 ++++
>  drivers/iommu/intel/Makefile |   1 +
>  drivers/iommu/intel/iommu.c  |   5 ++
>  drivers/iommu/intel/siov.c   | 119 +++++++++++++++++++++++++++++++++++
>  include/linux/intel-iommu.h  |   4 ++
>  5 files changed, 142 insertions(+)
>  create mode 100644 drivers/iommu/intel/siov.c
> 
> diff --git a/drivers/iommu/intel/Kconfig b/drivers/iommu/intel/Kconfig
> index 28a3d1596c76..94edc332f558 100644
> --- a/drivers/iommu/intel/Kconfig
> +++ b/drivers/iommu/intel/Kconfig
> @@ -86,3 +86,16 @@ config INTEL_IOMMU_SCALABLE_MODE_DEFAULT_ON
>  	  is not selected, scalable mode support could also be enabled by
>  	  passing intel_iommu=sm_on to the kernel. If not sure, please use
>  	  the default value.
> +
> +config INTEL_IOMMU_SCALABLE_IOV

INTEL_IOMMU_SUBDEVICE? if just talking from IOMMU p.o.v...

> +	bool "Support for Intel Scalable I/O Virtualization"
> +	depends on INTEL_IOMMU
> +	select VFIO
> +	select VFIO_MDEV
> +	select VFIO_MDEV_DEVICE
> +	help
> +	  Intel Scalable I/O virtualization (SIOV) is a hardware-assisted
> +	  PCIe subdevices virtualization. With each subdevice tagged with
> +	  an unique ID(PCI/PASID) the VT-d hardware could identify, hence
> +	  isolate DMA transactions from different subdevices on a same PCIe
> +	  device. Selecting this option will enable the support.
> diff --git a/drivers/iommu/intel/Makefile b/drivers/iommu/intel/Makefile
> index fb8e1e8c8029..f216385d5d59 100644
> --- a/drivers/iommu/intel/Makefile
> +++ b/drivers/iommu/intel/Makefile
> @@ -4,4 +4,5 @@ obj-$(CONFIG_INTEL_IOMMU) += iommu.o pasid.o
>  obj-$(CONFIG_INTEL_IOMMU) += trace.o
>  obj-$(CONFIG_INTEL_IOMMU_DEBUGFS) += debugfs.o
>  obj-$(CONFIG_INTEL_IOMMU_SVM) += svm.o
> +obj-$(CONFIG_INTEL_IOMMU_SCALABLE_IOV) += siov.o
>  obj-$(CONFIG_IRQ_REMAP) += irq_remapping.o
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index 1454fe74f3ba..dafd8069c2af 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -4298,6 +4298,11 @@ int __init intel_iommu_init(void)
>  	up_read(&dmar_global_lock);
> 
>  	bus_set_iommu(&pci_bus_type, &intel_iommu_ops);
> +
> +#ifdef CONFIG_INTEL_IOMMU_SCALABLE_IOV
> +	intel_siov_init();
> +#endif /* CONFIG_INTEL_IOMMU_SCALABLE_IOV */
> +
>  	if (si_domain && !hw_pass_through)
>  		register_memory_notifier(&intel_iommu_memory_nb);
>  	cpuhp_setup_state(CPUHP_IOMMU_INTEL_DEAD,
> "iommu/intel:dead", NULL,
> diff --git a/drivers/iommu/intel/siov.c b/drivers/iommu/intel/siov.c
> new file mode 100644
> index 000000000000..b9470e7ab3d6
> --- /dev/null
> +++ b/drivers/iommu/intel/siov.c
> @@ -0,0 +1,119 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/**
> + * siov.c - Intel Scalable I/O virtualization support
> + *
> + * Copyright (C) 2020 Intel Corporation
> + *
> + * Author: Lu Baolu <baolu.lu@linux.intel.com>
> + */
> +
> +#define pr_fmt(fmt)	"DMAR: " fmt
> +
> +#include <linux/intel-iommu.h>
> +#include <linux/iommu.h>
> +#include <linux/mdev.h>
> +#include <linux/pci.h>
> +
> +static struct device *subdev_lookup_parent(struct device *dev)
> +{
> +	if (dev->bus == &mdev_bus_type)
> +		return mdev_parent_dev(mdev_from_dev(dev));

What about finding the parent through device core? Then the logic
should work for all subdevice frameworks.

> +
> +	return NULL;
> +}
> +
> +static struct iommu_domain *siov_iommu_domain_alloc(unsigned int type)
> +{
> +	if (type != IOMMU_DOMAIN_UNMANAGED)
> +		return NULL;
> +
> +	return intel_iommu_domain_alloc(type);
> +}
> +
> +static int siov_iommu_attach_device(struct iommu_domain *domain,
> +				    struct device *dev)
> +{
> +	struct device *parent;
> +
> +	parent = subdev_lookup_parent(dev);
> +	if (!parent || !dev_is_pci(parent))
> +		return -ENODEV;
> +
> +	if (iommu_dev_feature_enabled(parent, IOMMU_DEV_FEAT_AUX))
> +		return intel_iommu_aux_attach_device(domain, parent);
> +	else
> +		return intel_iommu_attach_device(domain, parent);

if subdevice then FEAT_AUX should be assumed true? otherwise why
do we use subdevice interface to attach domain to physical device?

Thanks
Kevin

> +}
> +
> +static void siov_iommu_detach_device(struct iommu_domain *domain,
> +				     struct device *dev)
> +{
> +	struct device *parent;
> +
> +	parent = subdev_lookup_parent(dev);
> +	if (WARN_ON_ONCE(!parent || !dev_is_pci(parent)))
> +		return;
> +
> +	if (iommu_dev_feature_enabled(parent, IOMMU_DEV_FEAT_AUX))
> +		intel_iommu_aux_detach_device(domain, parent);
> +	else
> +		intel_iommu_detach_device(domain, parent);
> +}
> +
> +static struct iommu_device *siov_iommu_probe_device(struct device *dev)
> +{
> +	struct intel_iommu *iommu;
> +	struct device *parent;
> +
> +	parent = subdev_lookup_parent(dev);
> +	if (!parent || !dev_is_pci(parent))
> +		return ERR_PTR(-EINVAL);
> +
> +	iommu = device_to_iommu(parent, NULL, NULL);
> +	if (!iommu)
> +		return ERR_PTR(-ENODEV);
> +
> +	return &iommu->iommu;
> +}
> +
> +static void siov_iommu_release_device(struct device *dev)
> +{
> +}
> +
> +static void
> +siov_iommu_get_resv_regions(struct device *dev, struct list_head *head)
> +{
> +	struct device *parent;
> +
> +	parent = subdev_lookup_parent(dev);
> +	if (!parent || !dev_is_pci(parent))
> +		return;
> +
> +	intel_iommu_get_resv_regions(parent, head);
> +}
> +
> +static const struct iommu_ops siov_iommu_ops = {
> +	.capable		= intel_iommu_capable,
> +	.domain_alloc		= siov_iommu_domain_alloc,
> +	.domain_free		= intel_iommu_domain_free,
> +	.attach_dev		= siov_iommu_attach_device,
> +	.detach_dev		= siov_iommu_detach_device,
> +	.map			= intel_iommu_map,
> +	.unmap			= intel_iommu_unmap,
> +	.iova_to_phys		= intel_iommu_iova_to_phys,
> +	.probe_device		= siov_iommu_probe_device,
> +	.release_device		= siov_iommu_release_device,
> +	.get_resv_regions	= siov_iommu_get_resv_regions,
> +	.put_resv_regions	= generic_iommu_put_resv_regions,
> +	.device_group		= generic_device_group,
> +	.pgsize_bitmap		= (~0xFFFUL),
> +};
> +
> +void intel_siov_init(void)
> +{
> +	if (!scalable_mode_support() || !iommu_pasid_support())
> +		return;
> +
> +	bus_set_iommu(&mdev_bus_type, &siov_iommu_ops);
> +	pr_info("Intel(R) Scalable I/O Virtualization supported\n");
> +}
> diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
> index d3928ba87986..b790efa5509f 100644
> --- a/include/linux/intel-iommu.h
> +++ b/include/linux/intel-iommu.h
> @@ -787,6 +787,10 @@ size_t intel_iommu_unmap(struct iommu_domain
> *domain, unsigned long iova,
>  phys_addr_t intel_iommu_iova_to_phys(struct iommu_domain *domain,
> dma_addr_t iova);
>  void intel_iommu_get_resv_regions(struct device *device, struct list_head
> *head);
> 
> +#ifdef CONFIG_INTEL_IOMMU_SCALABLE_IOV
> +void intel_siov_init(void);
> +#endif /* INTEL_IOMMU_SCALABLE_IOV */
> +
>  #ifdef CONFIG_INTEL_IOMMU_SVM
>  extern void intel_svm_check(struct intel_iommu *iommu);
>  extern int intel_svm_enable_prq(struct intel_iommu *iommu);
> --
> 2.25.1


^ permalink raw reply	[flat|nested] 30+ messages in thread

* RE: [PATCH v6 4/5] iommu/vt-d: Add iommu_ops support for subdevice bus
@ 2020-10-30  6:13     ` Tian, Kevin
  0 siblings, 0 replies; 30+ messages in thread
From: Tian, Kevin @ 2020-10-30  6:13 UTC (permalink / raw)
  To: Lu Baolu, Joerg Roedel, Alex Williamson
  Cc: Jean-Philippe Brucker, Jiang, Dave, Raj, Ashok, kvm,
	Cornelia Huck, linux-kernel, Kirti Wankhede, iommu, Robin Murphy

> From: Lu Baolu <baolu.lu@linux.intel.com>
> Sent: Friday, October 30, 2020 12:58 PM
> 
> The iommu_ops will only take effect when INTEL_IOMMU_SCALABLE_IOV
> kernel
> option is selected. It applies to any device passthrough framework which
> implements an underlying bus for the subdevices.
> 
> - Subdevice probe:
>   When a subdevice is created and added to the bus, iommu_probe_device()
>   will be called, where the device will be probed by the iommu core. An
>   iommu group will be allocated and the device will be added to it. The
>   default domain won't be allocated since there's no use case of using a
>   subdevice in the host kernel at this time being. However, it's pretty
>   easy to add this support later.
> 
> - Domain alloc/free/map/unmap/iova_to_phys operations:
>   For such ops, we just reuse those for PCI/PCIe devices.

One question. Just be curious whether every IOMMU vendor supports
only one iommu_ops for all bus types. For Intel obviously the answer is
yes. But if a vendor supports bus-type specific iommu_ops for physical
devices, this may impose a restriction to VFIO or other passthrough 
frameworks, because VFIO today maintains only one mdev bus while the 
parent devices could come from different bus types. In the end the ops
for subdevice should be same as the one used for the parent. Then it may 
require VFIO to organize subdevices based on parent bus types. 

> 
> - Domain attach/detach operations:
>   It depends on whether the parent device supports IOMMU_DEV_FEAT_AUX
>   feature. If so, the domain will be attached to the parent device as an
>   aux-domain; Otherwise, it will be attached to the parent as a primary
>   domain.
> 
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> ---
>  drivers/iommu/intel/Kconfig  |  13 ++++
>  drivers/iommu/intel/Makefile |   1 +
>  drivers/iommu/intel/iommu.c  |   5 ++
>  drivers/iommu/intel/siov.c   | 119 +++++++++++++++++++++++++++++++++++
>  include/linux/intel-iommu.h  |   4 ++
>  5 files changed, 142 insertions(+)
>  create mode 100644 drivers/iommu/intel/siov.c
> 
> diff --git a/drivers/iommu/intel/Kconfig b/drivers/iommu/intel/Kconfig
> index 28a3d1596c76..94edc332f558 100644
> --- a/drivers/iommu/intel/Kconfig
> +++ b/drivers/iommu/intel/Kconfig
> @@ -86,3 +86,16 @@ config INTEL_IOMMU_SCALABLE_MODE_DEFAULT_ON
>  	  is not selected, scalable mode support could also be enabled by
>  	  passing intel_iommu=sm_on to the kernel. If not sure, please use
>  	  the default value.
> +
> +config INTEL_IOMMU_SCALABLE_IOV

INTEL_IOMMU_SUBDEVICE? if just talking from IOMMU p.o.v...

> +	bool "Support for Intel Scalable I/O Virtualization"
> +	depends on INTEL_IOMMU
> +	select VFIO
> +	select VFIO_MDEV
> +	select VFIO_MDEV_DEVICE
> +	help
> +	  Intel Scalable I/O virtualization (SIOV) is a hardware-assisted
> +	  PCIe subdevices virtualization. With each subdevice tagged with
> +	  an unique ID(PCI/PASID) the VT-d hardware could identify, hence
> +	  isolate DMA transactions from different subdevices on a same PCIe
> +	  device. Selecting this option will enable the support.
> diff --git a/drivers/iommu/intel/Makefile b/drivers/iommu/intel/Makefile
> index fb8e1e8c8029..f216385d5d59 100644
> --- a/drivers/iommu/intel/Makefile
> +++ b/drivers/iommu/intel/Makefile
> @@ -4,4 +4,5 @@ obj-$(CONFIG_INTEL_IOMMU) += iommu.o pasid.o
>  obj-$(CONFIG_INTEL_IOMMU) += trace.o
>  obj-$(CONFIG_INTEL_IOMMU_DEBUGFS) += debugfs.o
>  obj-$(CONFIG_INTEL_IOMMU_SVM) += svm.o
> +obj-$(CONFIG_INTEL_IOMMU_SCALABLE_IOV) += siov.o
>  obj-$(CONFIG_IRQ_REMAP) += irq_remapping.o
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index 1454fe74f3ba..dafd8069c2af 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -4298,6 +4298,11 @@ int __init intel_iommu_init(void)
>  	up_read(&dmar_global_lock);
> 
>  	bus_set_iommu(&pci_bus_type, &intel_iommu_ops);
> +
> +#ifdef CONFIG_INTEL_IOMMU_SCALABLE_IOV
> +	intel_siov_init();
> +#endif /* CONFIG_INTEL_IOMMU_SCALABLE_IOV */
> +
>  	if (si_domain && !hw_pass_through)
>  		register_memory_notifier(&intel_iommu_memory_nb);
>  	cpuhp_setup_state(CPUHP_IOMMU_INTEL_DEAD,
> "iommu/intel:dead", NULL,
> diff --git a/drivers/iommu/intel/siov.c b/drivers/iommu/intel/siov.c
> new file mode 100644
> index 000000000000..b9470e7ab3d6
> --- /dev/null
> +++ b/drivers/iommu/intel/siov.c
> @@ -0,0 +1,119 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/**
> + * siov.c - Intel Scalable I/O virtualization support
> + *
> + * Copyright (C) 2020 Intel Corporation
> + *
> + * Author: Lu Baolu <baolu.lu@linux.intel.com>
> + */
> +
> +#define pr_fmt(fmt)	"DMAR: " fmt
> +
> +#include <linux/intel-iommu.h>
> +#include <linux/iommu.h>
> +#include <linux/mdev.h>
> +#include <linux/pci.h>
> +
> +static struct device *subdev_lookup_parent(struct device *dev)
> +{
> +	if (dev->bus == &mdev_bus_type)
> +		return mdev_parent_dev(mdev_from_dev(dev));

What about finding the parent through device core? Then the logic
should work for all subdevice frameworks.

> +
> +	return NULL;
> +}
> +
> +static struct iommu_domain *siov_iommu_domain_alloc(unsigned int type)
> +{
> +	if (type != IOMMU_DOMAIN_UNMANAGED)
> +		return NULL;
> +
> +	return intel_iommu_domain_alloc(type);
> +}
> +
> +static int siov_iommu_attach_device(struct iommu_domain *domain,
> +				    struct device *dev)
> +{
> +	struct device *parent;
> +
> +	parent = subdev_lookup_parent(dev);
> +	if (!parent || !dev_is_pci(parent))
> +		return -ENODEV;
> +
> +	if (iommu_dev_feature_enabled(parent, IOMMU_DEV_FEAT_AUX))
> +		return intel_iommu_aux_attach_device(domain, parent);
> +	else
> +		return intel_iommu_attach_device(domain, parent);

if subdevice then FEAT_AUX should be assumed true? otherwise why
do we use subdevice interface to attach domain to physical device?

Thanks
Kevin

> +}
> +
> +static void siov_iommu_detach_device(struct iommu_domain *domain,
> +				     struct device *dev)
> +{
> +	struct device *parent;
> +
> +	parent = subdev_lookup_parent(dev);
> +	if (WARN_ON_ONCE(!parent || !dev_is_pci(parent)))
> +		return;
> +
> +	if (iommu_dev_feature_enabled(parent, IOMMU_DEV_FEAT_AUX))
> +		intel_iommu_aux_detach_device(domain, parent);
> +	else
> +		intel_iommu_detach_device(domain, parent);
> +}
> +
> +static struct iommu_device *siov_iommu_probe_device(struct device *dev)
> +{
> +	struct intel_iommu *iommu;
> +	struct device *parent;
> +
> +	parent = subdev_lookup_parent(dev);
> +	if (!parent || !dev_is_pci(parent))
> +		return ERR_PTR(-EINVAL);
> +
> +	iommu = device_to_iommu(parent, NULL, NULL);
> +	if (!iommu)
> +		return ERR_PTR(-ENODEV);
> +
> +	return &iommu->iommu;
> +}
> +
> +static void siov_iommu_release_device(struct device *dev)
> +{
> +}
> +
> +static void
> +siov_iommu_get_resv_regions(struct device *dev, struct list_head *head)
> +{
> +	struct device *parent;
> +
> +	parent = subdev_lookup_parent(dev);
> +	if (!parent || !dev_is_pci(parent))
> +		return;
> +
> +	intel_iommu_get_resv_regions(parent, head);
> +}
> +
> +static const struct iommu_ops siov_iommu_ops = {
> +	.capable		= intel_iommu_capable,
> +	.domain_alloc		= siov_iommu_domain_alloc,
> +	.domain_free		= intel_iommu_domain_free,
> +	.attach_dev		= siov_iommu_attach_device,
> +	.detach_dev		= siov_iommu_detach_device,
> +	.map			= intel_iommu_map,
> +	.unmap			= intel_iommu_unmap,
> +	.iova_to_phys		= intel_iommu_iova_to_phys,
> +	.probe_device		= siov_iommu_probe_device,
> +	.release_device		= siov_iommu_release_device,
> +	.get_resv_regions	= siov_iommu_get_resv_regions,
> +	.put_resv_regions	= generic_iommu_put_resv_regions,
> +	.device_group		= generic_device_group,
> +	.pgsize_bitmap		= (~0xFFFUL),
> +};
> +
> +void intel_siov_init(void)
> +{
> +	if (!scalable_mode_support() || !iommu_pasid_support())
> +		return;
> +
> +	bus_set_iommu(&mdev_bus_type, &siov_iommu_ops);
> +	pr_info("Intel(R) Scalable I/O Virtualization supported\n");
> +}
> diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
> index d3928ba87986..b790efa5509f 100644
> --- a/include/linux/intel-iommu.h
> +++ b/include/linux/intel-iommu.h
> @@ -787,6 +787,10 @@ size_t intel_iommu_unmap(struct iommu_domain
> *domain, unsigned long iova,
>  phys_addr_t intel_iommu_iova_to_phys(struct iommu_domain *domain,
> dma_addr_t iova);
>  void intel_iommu_get_resv_regions(struct device *device, struct list_head
> *head);
> 
> +#ifdef CONFIG_INTEL_IOMMU_SCALABLE_IOV
> +void intel_siov_init(void);
> +#endif /* INTEL_IOMMU_SCALABLE_IOV */
> +
>  #ifdef CONFIG_INTEL_IOMMU_SVM
>  extern void intel_svm_check(struct intel_iommu *iommu);
>  extern int intel_svm_enable_prq(struct intel_iommu *iommu);
> --
> 2.25.1

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 30+ messages in thread

* RE: [PATCH v6 5/5] vfio/type1: Use mdev bus iommu_ops for IOMMU callbacks
  2020-10-30  4:58   ` Lu Baolu
@ 2020-10-30  6:16     ` Tian, Kevin
  -1 siblings, 0 replies; 30+ messages in thread
From: Tian, Kevin @ 2020-10-30  6:16 UTC (permalink / raw)
  To: Lu Baolu, Joerg Roedel, Alex Williamson
  Cc: Robin Murphy, Jean-Philippe Brucker, Cornelia Huck, Raj, Ashok,
	Jiang, Dave, Liu, Yi L, Zeng, Xin, iommu, linux-kernel, kvm

> From: Lu Baolu <baolu.lu@linux.intel.com>
> Sent: Friday, October 30, 2020 12:58 PM
> 
> With the IOMMU driver registering iommu_ops for the mdev_bus, the
> IOMMU
> operations on an mdev could be done in the same way as any normal device
> (for example, PCI/PCIe). There's no need to distinguish an mdev from
> others for iommu operations. Remove the unnecessary code.

This is really a nice cleanup as the output of this change! :)

Thanks
Kevin

> 
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> ---
>  drivers/vfio/mdev/mdev_core.c    |  18 -----
>  drivers/vfio/mdev/mdev_driver.c  |   6 ++
>  drivers/vfio/mdev/mdev_private.h |   1 -
>  drivers/vfio/vfio_iommu_type1.c  | 128 +++----------------------------
>  include/linux/mdev.h             |  14 ----
>  5 files changed, 18 insertions(+), 149 deletions(-)
> 
> diff --git a/drivers/vfio/mdev/mdev_core.c
> b/drivers/vfio/mdev/mdev_core.c
> index 6b9ab71f89e7..f4fd5f237c49 100644
> --- a/drivers/vfio/mdev/mdev_core.c
> +++ b/drivers/vfio/mdev/mdev_core.c
> @@ -386,24 +386,6 @@ int mdev_device_remove(struct device *dev)
>  	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);
> -
>  static int __init mdev_init(void)
>  {
>  	return mdev_bus_register();
> diff --git a/drivers/vfio/mdev/mdev_driver.c
> b/drivers/vfio/mdev/mdev_driver.c
> index 0d3223aee20b..487402f16355 100644
> --- a/drivers/vfio/mdev/mdev_driver.c
> +++ b/drivers/vfio/mdev/mdev_driver.c
> @@ -18,6 +18,9 @@ static int mdev_attach_iommu(struct mdev_device
> *mdev)
>  	int ret;
>  	struct iommu_group *group;
> 
> +	if (iommu_present(&mdev_bus_type))
> +		return 0;
> +
>  	group = iommu_group_alloc();
>  	if (IS_ERR(group))
>  		return PTR_ERR(group);
> @@ -33,6 +36,9 @@ static int mdev_attach_iommu(struct mdev_device
> *mdev)
> 
>  static void mdev_detach_iommu(struct mdev_device *mdev)
>  {
> +	if (iommu_present(&mdev_bus_type))
> +		return;
> +
>  	iommu_group_remove_device(&mdev->dev);
>  	dev_info(&mdev->dev, "MDEV: detaching iommu\n");
>  }
> diff --git a/drivers/vfio/mdev/mdev_private.h
> b/drivers/vfio/mdev/mdev_private.h
> index 7d922950caaf..efe0aefdb52f 100644
> --- a/drivers/vfio/mdev/mdev_private.h
> +++ b/drivers/vfio/mdev/mdev_private.h
> @@ -31,7 +31,6 @@ struct mdev_device {
>  	void *driver_data;
>  	struct list_head next;
>  	struct kobject *type_kobj;
> -	struct device *iommu_device;
>  	bool active;
>  };
> 
> diff --git a/drivers/vfio/vfio_iommu_type1.c
> b/drivers/vfio/vfio_iommu_type1.c
> index bb2684cc245e..e231b7070ca5 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -100,7 +100,6 @@ struct vfio_dma {
>  struct vfio_group {
>  	struct iommu_group	*iommu_group;
>  	struct list_head	next;
> -	bool			mdev_group;	/* An mdev group */
>  	bool			pinned_page_dirty_scope;
>  };
> 
> @@ -1675,102 +1674,6 @@ static bool vfio_iommu_has_sw_msi(struct
> list_head *group_resv_regions,
>  	return ret;
>  }
> 
> -static struct device *vfio_mdev_get_iommu_device(struct device *dev)
> -{
> -	struct device *(*fn)(struct device *dev);
> -	struct device *iommu_device;
> -
> -	fn = symbol_get(mdev_get_iommu_device);
> -	if (fn) {
> -		iommu_device = fn(dev);
> -		symbol_put(mdev_get_iommu_device);
> -
> -		return iommu_device;
> -	}
> -
> -	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);
> -	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 bool vfio_bus_is_mdev(struct bus_type *bus)
> -{
> -	struct bus_type *mdev_bus;
> -	bool ret = false;
> -
> -	mdev_bus = symbol_get(mdev_bus_type);
> -	if (mdev_bus) {
> -		ret = (bus == mdev_bus);
> -		symbol_put(mdev_bus_type);
> -	}
> -
> -	return ret;
> -}
> -
> -static int vfio_mdev_iommu_device(struct device *dev, void *data)
> -{
> -	struct device **old = data, *new;
> -
> -	new = vfio_mdev_get_iommu_device(dev);
> -	if (!new || (*old && *old != new))
> -		return -EINVAL;
> -
> -	*old = new;
> -
> -	return 0;
> -}
> -
>  /*
>   * This is a helper function to insert an address range to iova list.
>   * The list is initially created with a single entry corresponding to
> @@ -1999,7 +1902,7 @@ static int vfio_iommu_type1_attach_group(void
> *iommu_data,
>  	struct vfio_iommu *iommu = iommu_data;
>  	struct vfio_group *group;
>  	struct vfio_domain *domain, *d;
> -	struct bus_type *bus = NULL;
> +	struct bus_type *bus = NULL, *mdev_bus;
>  	int ret;
>  	bool resv_msi, msi_remap;
>  	phys_addr_t resv_msi_base = 0;
> @@ -2037,15 +1940,10 @@ static int vfio_iommu_type1_attach_group(void
> *iommu_data,
>  	if (ret)
>  		goto out_free;
> 
> -	if (vfio_bus_is_mdev(bus)) {
> -		struct device *iommu_device = NULL;
> -
> -		group->mdev_group = true;
> -
> -		/* Determine the isolation type */
> -		ret = iommu_group_for_each_dev(iommu_group,
> &iommu_device,
> -					       vfio_mdev_iommu_device);
> -		if (ret || !iommu_device) {
> +	mdev_bus = symbol_get(mdev_bus_type);
> +	if (mdev_bus) {
> +		if (bus == mdev_bus && !iommu_present(bus)) {
> +			symbol_put(mdev_bus_type);
>  			if (!iommu->external_domain) {
>  				INIT_LIST_HEAD(&domain->group_list);
>  				iommu->external_domain = domain;
> @@ -2070,8 +1968,6 @@ static int vfio_iommu_type1_attach_group(void
> *iommu_data,
> 
>  			return 0;
>  		}
> -
> -		bus = iommu_device->bus;
>  	}
> 
>  	domain->domain = iommu_domain_alloc(bus);
> @@ -2089,7 +1985,7 @@ static int vfio_iommu_type1_attach_group(void
> *iommu_data,
>  			goto out_domain;
>  	}
> 
> -	ret = vfio_iommu_attach_group(domain, group);
> +	ret = iommu_attach_group(domain->domain, iommu_group);
>  	if (ret)
>  		goto out_domain;
> 
> @@ -2157,15 +2053,15 @@ 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) {
> -			vfio_iommu_detach_group(domain, group);
> -			if (!vfio_iommu_attach_group(d, group)) {
> +			iommu_detach_group(domain->domain,
> iommu_group);
> +			if (!iommu_attach_group(d->domain, iommu_group))
> {
>  				list_add(&group->next, &d->group_list);
>  				iommu_domain_free(domain->domain);
>  				kfree(domain);
>  				goto done;
>  			}
> 
> -			ret = vfio_iommu_attach_group(domain, group);
> +			ret = iommu_attach_group(domain->domain,
> iommu_group);
>  			if (ret)
>  				goto out_domain;
>  		}
> @@ -2202,7 +2098,7 @@ static int vfio_iommu_type1_attach_group(void
> *iommu_data,
>  	return 0;
> 
>  out_detach:
> -	vfio_iommu_detach_group(domain, group);
> +	iommu_detach_group(domain->domain, iommu_group);
>  out_domain:
>  	iommu_domain_free(domain->domain);
>  	vfio_iommu_iova_free(&iova_copy);
> @@ -2385,7 +2281,7 @@ static void vfio_iommu_type1_detach_group(void
> *iommu_data,
>  		if (!group)
>  			continue;
> 
> -		vfio_iommu_detach_group(domain, group);
> +		iommu_detach_group(domain->domain, iommu_group);
>  		update_dirty_scope = !group->pinned_page_dirty_scope;
>  		list_del(&group->next);
>  		kfree(group);
> @@ -2466,7 +2362,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)
> -			vfio_iommu_detach_group(domain, group);
> +			iommu_detach_group(domain->domain, group-
> >iommu_group);
>  		list_del(&group->next);
>  		kfree(group);
>  	}
> diff --git a/include/linux/mdev.h b/include/linux/mdev.h
> index 0ce30ca78db0..f7aee86bd2b0 100644
> --- a/include/linux/mdev.h
> +++ b/include/linux/mdev.h
> @@ -12,20 +12,6 @@
> 
>  struct mdev_device;
> 
> -/*
> - * Called by the parent device driver to set the device which represents
> - * 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);
> -
>  /**
>   * struct mdev_parent_ops - Structure to be registered for each parent
> device to
>   * register the device to mdev module.
> --
> 2.25.1


^ permalink raw reply	[flat|nested] 30+ messages in thread

* RE: [PATCH v6 5/5] vfio/type1: Use mdev bus iommu_ops for IOMMU callbacks
@ 2020-10-30  6:16     ` Tian, Kevin
  0 siblings, 0 replies; 30+ messages in thread
From: Tian, Kevin @ 2020-10-30  6:16 UTC (permalink / raw)
  To: Lu Baolu, Joerg Roedel, Alex Williamson
  Cc: Jean-Philippe Brucker, Jiang, Dave, Raj, Ashok, kvm,
	Cornelia Huck, linux-kernel, iommu, Robin Murphy

> From: Lu Baolu <baolu.lu@linux.intel.com>
> Sent: Friday, October 30, 2020 12:58 PM
> 
> With the IOMMU driver registering iommu_ops for the mdev_bus, the
> IOMMU
> operations on an mdev could be done in the same way as any normal device
> (for example, PCI/PCIe). There's no need to distinguish an mdev from
> others for iommu operations. Remove the unnecessary code.

This is really a nice cleanup as the output of this change! :)

Thanks
Kevin

> 
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> ---
>  drivers/vfio/mdev/mdev_core.c    |  18 -----
>  drivers/vfio/mdev/mdev_driver.c  |   6 ++
>  drivers/vfio/mdev/mdev_private.h |   1 -
>  drivers/vfio/vfio_iommu_type1.c  | 128 +++----------------------------
>  include/linux/mdev.h             |  14 ----
>  5 files changed, 18 insertions(+), 149 deletions(-)
> 
> diff --git a/drivers/vfio/mdev/mdev_core.c
> b/drivers/vfio/mdev/mdev_core.c
> index 6b9ab71f89e7..f4fd5f237c49 100644
> --- a/drivers/vfio/mdev/mdev_core.c
> +++ b/drivers/vfio/mdev/mdev_core.c
> @@ -386,24 +386,6 @@ int mdev_device_remove(struct device *dev)
>  	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);
> -
>  static int __init mdev_init(void)
>  {
>  	return mdev_bus_register();
> diff --git a/drivers/vfio/mdev/mdev_driver.c
> b/drivers/vfio/mdev/mdev_driver.c
> index 0d3223aee20b..487402f16355 100644
> --- a/drivers/vfio/mdev/mdev_driver.c
> +++ b/drivers/vfio/mdev/mdev_driver.c
> @@ -18,6 +18,9 @@ static int mdev_attach_iommu(struct mdev_device
> *mdev)
>  	int ret;
>  	struct iommu_group *group;
> 
> +	if (iommu_present(&mdev_bus_type))
> +		return 0;
> +
>  	group = iommu_group_alloc();
>  	if (IS_ERR(group))
>  		return PTR_ERR(group);
> @@ -33,6 +36,9 @@ static int mdev_attach_iommu(struct mdev_device
> *mdev)
> 
>  static void mdev_detach_iommu(struct mdev_device *mdev)
>  {
> +	if (iommu_present(&mdev_bus_type))
> +		return;
> +
>  	iommu_group_remove_device(&mdev->dev);
>  	dev_info(&mdev->dev, "MDEV: detaching iommu\n");
>  }
> diff --git a/drivers/vfio/mdev/mdev_private.h
> b/drivers/vfio/mdev/mdev_private.h
> index 7d922950caaf..efe0aefdb52f 100644
> --- a/drivers/vfio/mdev/mdev_private.h
> +++ b/drivers/vfio/mdev/mdev_private.h
> @@ -31,7 +31,6 @@ struct mdev_device {
>  	void *driver_data;
>  	struct list_head next;
>  	struct kobject *type_kobj;
> -	struct device *iommu_device;
>  	bool active;
>  };
> 
> diff --git a/drivers/vfio/vfio_iommu_type1.c
> b/drivers/vfio/vfio_iommu_type1.c
> index bb2684cc245e..e231b7070ca5 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -100,7 +100,6 @@ struct vfio_dma {
>  struct vfio_group {
>  	struct iommu_group	*iommu_group;
>  	struct list_head	next;
> -	bool			mdev_group;	/* An mdev group */
>  	bool			pinned_page_dirty_scope;
>  };
> 
> @@ -1675,102 +1674,6 @@ static bool vfio_iommu_has_sw_msi(struct
> list_head *group_resv_regions,
>  	return ret;
>  }
> 
> -static struct device *vfio_mdev_get_iommu_device(struct device *dev)
> -{
> -	struct device *(*fn)(struct device *dev);
> -	struct device *iommu_device;
> -
> -	fn = symbol_get(mdev_get_iommu_device);
> -	if (fn) {
> -		iommu_device = fn(dev);
> -		symbol_put(mdev_get_iommu_device);
> -
> -		return iommu_device;
> -	}
> -
> -	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);
> -	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 bool vfio_bus_is_mdev(struct bus_type *bus)
> -{
> -	struct bus_type *mdev_bus;
> -	bool ret = false;
> -
> -	mdev_bus = symbol_get(mdev_bus_type);
> -	if (mdev_bus) {
> -		ret = (bus == mdev_bus);
> -		symbol_put(mdev_bus_type);
> -	}
> -
> -	return ret;
> -}
> -
> -static int vfio_mdev_iommu_device(struct device *dev, void *data)
> -{
> -	struct device **old = data, *new;
> -
> -	new = vfio_mdev_get_iommu_device(dev);
> -	if (!new || (*old && *old != new))
> -		return -EINVAL;
> -
> -	*old = new;
> -
> -	return 0;
> -}
> -
>  /*
>   * This is a helper function to insert an address range to iova list.
>   * The list is initially created with a single entry corresponding to
> @@ -1999,7 +1902,7 @@ static int vfio_iommu_type1_attach_group(void
> *iommu_data,
>  	struct vfio_iommu *iommu = iommu_data;
>  	struct vfio_group *group;
>  	struct vfio_domain *domain, *d;
> -	struct bus_type *bus = NULL;
> +	struct bus_type *bus = NULL, *mdev_bus;
>  	int ret;
>  	bool resv_msi, msi_remap;
>  	phys_addr_t resv_msi_base = 0;
> @@ -2037,15 +1940,10 @@ static int vfio_iommu_type1_attach_group(void
> *iommu_data,
>  	if (ret)
>  		goto out_free;
> 
> -	if (vfio_bus_is_mdev(bus)) {
> -		struct device *iommu_device = NULL;
> -
> -		group->mdev_group = true;
> -
> -		/* Determine the isolation type */
> -		ret = iommu_group_for_each_dev(iommu_group,
> &iommu_device,
> -					       vfio_mdev_iommu_device);
> -		if (ret || !iommu_device) {
> +	mdev_bus = symbol_get(mdev_bus_type);
> +	if (mdev_bus) {
> +		if (bus == mdev_bus && !iommu_present(bus)) {
> +			symbol_put(mdev_bus_type);
>  			if (!iommu->external_domain) {
>  				INIT_LIST_HEAD(&domain->group_list);
>  				iommu->external_domain = domain;
> @@ -2070,8 +1968,6 @@ static int vfio_iommu_type1_attach_group(void
> *iommu_data,
> 
>  			return 0;
>  		}
> -
> -		bus = iommu_device->bus;
>  	}
> 
>  	domain->domain = iommu_domain_alloc(bus);
> @@ -2089,7 +1985,7 @@ static int vfio_iommu_type1_attach_group(void
> *iommu_data,
>  			goto out_domain;
>  	}
> 
> -	ret = vfio_iommu_attach_group(domain, group);
> +	ret = iommu_attach_group(domain->domain, iommu_group);
>  	if (ret)
>  		goto out_domain;
> 
> @@ -2157,15 +2053,15 @@ 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) {
> -			vfio_iommu_detach_group(domain, group);
> -			if (!vfio_iommu_attach_group(d, group)) {
> +			iommu_detach_group(domain->domain,
> iommu_group);
> +			if (!iommu_attach_group(d->domain, iommu_group))
> {
>  				list_add(&group->next, &d->group_list);
>  				iommu_domain_free(domain->domain);
>  				kfree(domain);
>  				goto done;
>  			}
> 
> -			ret = vfio_iommu_attach_group(domain, group);
> +			ret = iommu_attach_group(domain->domain,
> iommu_group);
>  			if (ret)
>  				goto out_domain;
>  		}
> @@ -2202,7 +2098,7 @@ static int vfio_iommu_type1_attach_group(void
> *iommu_data,
>  	return 0;
> 
>  out_detach:
> -	vfio_iommu_detach_group(domain, group);
> +	iommu_detach_group(domain->domain, iommu_group);
>  out_domain:
>  	iommu_domain_free(domain->domain);
>  	vfio_iommu_iova_free(&iova_copy);
> @@ -2385,7 +2281,7 @@ static void vfio_iommu_type1_detach_group(void
> *iommu_data,
>  		if (!group)
>  			continue;
> 
> -		vfio_iommu_detach_group(domain, group);
> +		iommu_detach_group(domain->domain, iommu_group);
>  		update_dirty_scope = !group->pinned_page_dirty_scope;
>  		list_del(&group->next);
>  		kfree(group);
> @@ -2466,7 +2362,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)
> -			vfio_iommu_detach_group(domain, group);
> +			iommu_detach_group(domain->domain, group-
> >iommu_group);
>  		list_del(&group->next);
>  		kfree(group);
>  	}
> diff --git a/include/linux/mdev.h b/include/linux/mdev.h
> index 0ce30ca78db0..f7aee86bd2b0 100644
> --- a/include/linux/mdev.h
> +++ b/include/linux/mdev.h
> @@ -12,20 +12,6 @@
> 
>  struct mdev_device;
> 
> -/*
> - * Called by the parent device driver to set the device which represents
> - * 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);
> -
>  /**
>   * struct mdev_parent_ops - Structure to be registered for each parent
> device to
>   * register the device to mdev module.
> --
> 2.25.1

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v6 2/5] iommu: Use bus iommu ops for aux related callback
  2020-10-30  5:55     ` Tian, Kevin
@ 2020-10-30 10:47       ` Jean-Philippe Brucker
  -1 siblings, 0 replies; 30+ messages in thread
From: Jean-Philippe Brucker @ 2020-10-30 10:47 UTC (permalink / raw)
  To: Tian, Kevin
  Cc: Lu Baolu, Joerg Roedel, Alex Williamson, Robin Murphy,
	Cornelia Huck, Raj, Ashok, Jiang, Dave, Liu, Yi L, Zeng, Xin,
	iommu, linux-kernel, kvm, Jordan Crouse

On Fri, Oct 30, 2020 at 05:55:53AM +0000, Tian, Kevin wrote:
> > From: Lu Baolu <baolu.lu@linux.intel.com>
> > Sent: Friday, October 30, 2020 12:58 PM
> > 
> > The aux-domain apis were designed for macro driver where the subdevices
> > are created and used inside a device driver. Use the device's bus iommu
> > ops instead of that in iommu domain for various callbacks.
> 
> IIRC there are only two users on these apis. One is VFIO, and the other
> is on the ARM side (not checked in yet). Jean, can you help confirm 
> whether ARM-side usage still relies on aux apis even with this change?

No, I have something out of tree but no plan to upstream it anymore, and
the SMMUv2 implementation is out as well:

https://lore.kernel.org/linux-iommu/20200713173556.GC3815@jcrouse1-lnx.qualcomm.com/

> If no, possibly they can be removed completely?

No objection from me. They can be added back later (I still belive adding
PASID to the DMA API would be nice to have once more HW implements it).

Thanks,
Jean

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v6 2/5] iommu: Use bus iommu ops for aux related callback
@ 2020-10-30 10:47       ` Jean-Philippe Brucker
  0 siblings, 0 replies; 30+ messages in thread
From: Jean-Philippe Brucker @ 2020-10-30 10:47 UTC (permalink / raw)
  To: Tian, Kevin
  Cc: Jiang, Dave, Raj, Ashok, kvm, Cornelia Huck, iommu, linux-kernel,
	Alex Williamson, Robin Murphy

On Fri, Oct 30, 2020 at 05:55:53AM +0000, Tian, Kevin wrote:
> > From: Lu Baolu <baolu.lu@linux.intel.com>
> > Sent: Friday, October 30, 2020 12:58 PM
> > 
> > The aux-domain apis were designed for macro driver where the subdevices
> > are created and used inside a device driver. Use the device's bus iommu
> > ops instead of that in iommu domain for various callbacks.
> 
> IIRC there are only two users on these apis. One is VFIO, and the other
> is on the ARM side (not checked in yet). Jean, can you help confirm 
> whether ARM-side usage still relies on aux apis even with this change?

No, I have something out of tree but no plan to upstream it anymore, and
the SMMUv2 implementation is out as well:

https://lore.kernel.org/linux-iommu/20200713173556.GC3815@jcrouse1-lnx.qualcomm.com/

> If no, possibly they can be removed completely?

No objection from me. They can be added back later (I still belive adding
PASID to the DMA API would be nice to have once more HW implements it).

Thanks,
Jean
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v6 1/5] vfio/mdev: Register mdev bus earlier during boot
  2020-10-30  4:58   ` Lu Baolu
@ 2020-10-30 17:02     ` Alex Williamson
  -1 siblings, 0 replies; 30+ messages in thread
From: Alex Williamson @ 2020-10-30 17:02 UTC (permalink / raw)
  To: Lu Baolu
  Cc: Joerg Roedel, Robin Murphy, Jean-Philippe Brucker, Cornelia Huck,
	Kevin Tian, Ashok Raj, Dave Jiang, Liu Yi L, Zeng Xin, iommu,
	linux-kernel, kvm

On Fri, 30 Oct 2020 12:58:05 +0800
Lu Baolu <baolu.lu@linux.intel.com> wrote:

> Move mdev bus registration earlier than IOMMU probe processing so that
> the IOMMU drivers could be able to set iommu_ops for the mdev bus. This
> only applies when vfio-mdev module is setected to be built-in.
> 
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> ---

Most kernels will build this as a module, so having different behavior
and apparently different IOMMU support for built-in vs module is
broken.  Thanks,

Alex

>  drivers/vfio/mdev/mdev_core.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
> index b558d4cfd082..6b9ab71f89e7 100644
> --- a/drivers/vfio/mdev/mdev_core.c
> +++ b/drivers/vfio/mdev/mdev_core.c
> @@ -417,8 +417,12 @@ static void __exit mdev_exit(void)
>  	mdev_bus_unregister();
>  }
>  
> +#if IS_BUILTIN(CONFIG_VFIO_MDEV)
> +postcore_initcall(mdev_init)
> +#else
>  module_init(mdev_init)
>  module_exit(mdev_exit)
> +#endif /* IS_BUILTIN(CONFIG_VFIO_MDEV) */
>  
>  MODULE_VERSION(DRIVER_VERSION);
>  MODULE_LICENSE("GPL v2");


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v6 1/5] vfio/mdev: Register mdev bus earlier during boot
@ 2020-10-30 17:02     ` Alex Williamson
  0 siblings, 0 replies; 30+ messages in thread
From: Alex Williamson @ 2020-10-30 17:02 UTC (permalink / raw)
  To: Lu Baolu
  Cc: Jean-Philippe Brucker, Kevin Tian, Dave Jiang, Ashok Raj, kvm,
	Cornelia Huck, linux-kernel, iommu, Robin Murphy

On Fri, 30 Oct 2020 12:58:05 +0800
Lu Baolu <baolu.lu@linux.intel.com> wrote:

> Move mdev bus registration earlier than IOMMU probe processing so that
> the IOMMU drivers could be able to set iommu_ops for the mdev bus. This
> only applies when vfio-mdev module is setected to be built-in.
> 
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> ---

Most kernels will build this as a module, so having different behavior
and apparently different IOMMU support for built-in vs module is
broken.  Thanks,

Alex

>  drivers/vfio/mdev/mdev_core.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
> index b558d4cfd082..6b9ab71f89e7 100644
> --- a/drivers/vfio/mdev/mdev_core.c
> +++ b/drivers/vfio/mdev/mdev_core.c
> @@ -417,8 +417,12 @@ static void __exit mdev_exit(void)
>  	mdev_bus_unregister();
>  }
>  
> +#if IS_BUILTIN(CONFIG_VFIO_MDEV)
> +postcore_initcall(mdev_init)
> +#else
>  module_init(mdev_init)
>  module_exit(mdev_exit)
> +#endif /* IS_BUILTIN(CONFIG_VFIO_MDEV) */
>  
>  MODULE_VERSION(DRIVER_VERSION);
>  MODULE_LICENSE("GPL v2");

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v6 4/5] iommu/vt-d: Add iommu_ops support for subdevice bus
  2020-10-30  4:58   ` Lu Baolu
@ 2020-10-30 20:56     ` Alex Williamson
  -1 siblings, 0 replies; 30+ messages in thread
From: Alex Williamson @ 2020-10-30 20:56 UTC (permalink / raw)
  To: Lu Baolu
  Cc: Joerg Roedel, Robin Murphy, Jean-Philippe Brucker, Cornelia Huck,
	Kevin Tian, Ashok Raj, Dave Jiang, Liu Yi L, Zeng Xin, iommu,
	linux-kernel, kvm

On Fri, 30 Oct 2020 12:58:08 +0800
Lu Baolu <baolu.lu@linux.intel.com> wrote:

> +static const struct iommu_ops siov_iommu_ops = {
> +	.capable		= intel_iommu_capable,
> +	.domain_alloc		= siov_iommu_domain_alloc,
> +	.domain_free		= intel_iommu_domain_free,
> +	.attach_dev		= siov_iommu_attach_device,
> +	.detach_dev		= siov_iommu_detach_device,
> +	.map			= intel_iommu_map,
> +	.unmap			= intel_iommu_unmap,
> +	.iova_to_phys		= intel_iommu_iova_to_phys,
> +	.probe_device		= siov_iommu_probe_device,
> +	.release_device		= siov_iommu_release_device,
> +	.get_resv_regions	= siov_iommu_get_resv_regions,
> +	.put_resv_regions	= generic_iommu_put_resv_regions,
> +	.device_group		= generic_device_group,
> +	.pgsize_bitmap		= (~0xFFFUL),
> +};
> +
> +void intel_siov_init(void)
> +{
> +	if (!scalable_mode_support() || !iommu_pasid_support())
> +		return;
> +
> +	bus_set_iommu(&mdev_bus_type, &siov_iommu_ops);
> +	pr_info("Intel(R) Scalable I/O Virtualization supported\n");
> +}

How can you presume to take over iommu_ops for an entire virtual bus?
This also forces mdev and all the dependencies of mdev to be built into
the kernel.  I don't find that acceptable.  Thanks,

Alex


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v6 4/5] iommu/vt-d: Add iommu_ops support for subdevice bus
@ 2020-10-30 20:56     ` Alex Williamson
  0 siblings, 0 replies; 30+ messages in thread
From: Alex Williamson @ 2020-10-30 20:56 UTC (permalink / raw)
  To: Lu Baolu
  Cc: Jean-Philippe Brucker, Kevin Tian, Dave Jiang, Ashok Raj, kvm,
	Cornelia Huck, linux-kernel, iommu, Robin Murphy

On Fri, 30 Oct 2020 12:58:08 +0800
Lu Baolu <baolu.lu@linux.intel.com> wrote:

> +static const struct iommu_ops siov_iommu_ops = {
> +	.capable		= intel_iommu_capable,
> +	.domain_alloc		= siov_iommu_domain_alloc,
> +	.domain_free		= intel_iommu_domain_free,
> +	.attach_dev		= siov_iommu_attach_device,
> +	.detach_dev		= siov_iommu_detach_device,
> +	.map			= intel_iommu_map,
> +	.unmap			= intel_iommu_unmap,
> +	.iova_to_phys		= intel_iommu_iova_to_phys,
> +	.probe_device		= siov_iommu_probe_device,
> +	.release_device		= siov_iommu_release_device,
> +	.get_resv_regions	= siov_iommu_get_resv_regions,
> +	.put_resv_regions	= generic_iommu_put_resv_regions,
> +	.device_group		= generic_device_group,
> +	.pgsize_bitmap		= (~0xFFFUL),
> +};
> +
> +void intel_siov_init(void)
> +{
> +	if (!scalable_mode_support() || !iommu_pasid_support())
> +		return;
> +
> +	bus_set_iommu(&mdev_bus_type, &siov_iommu_ops);
> +	pr_info("Intel(R) Scalable I/O Virtualization supported\n");
> +}

How can you presume to take over iommu_ops for an entire virtual bus?
This also forces mdev and all the dependencies of mdev to be built into
the kernel.  I don't find that acceptable.  Thanks,

Alex

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v6 5/5] vfio/type1: Use mdev bus iommu_ops for IOMMU callbacks
  2020-10-30  6:16     ` Tian, Kevin
@ 2020-10-30 21:06       ` Alex Williamson
  -1 siblings, 0 replies; 30+ messages in thread
From: Alex Williamson @ 2020-10-30 21:06 UTC (permalink / raw)
  To: Tian, Kevin
  Cc: Lu Baolu, Joerg Roedel, Robin Murphy, Jean-Philippe Brucker,
	Cornelia Huck, Raj, Ashok, Jiang, Dave, Liu, Yi L, Zeng, Xin,
	iommu, linux-kernel, kvm

On Fri, 30 Oct 2020 06:16:28 +0000
"Tian, Kevin" <kevin.tian@intel.com> wrote:

> > From: Lu Baolu <baolu.lu@linux.intel.com>
> > Sent: Friday, October 30, 2020 12:58 PM
> > 
> > With the IOMMU driver registering iommu_ops for the mdev_bus, the
> > IOMMU
> > operations on an mdev could be done in the same way as any normal device
> > (for example, PCI/PCIe). There's no need to distinguish an mdev from
> > others for iommu operations. Remove the unnecessary code.  
> 
> This is really a nice cleanup as the output of this change! :)

It's easy to remove a bunch of code when the result is breaking
everyone else.  Please share with me how SR-IOV backed mdevs continue
to work on AMD platforms, or how they might work on ARM platforms, when
siov_iommu_ops (VT-d only) becomes the one and only provider of
iommu_ops on the mdev bus.  Hard NAK on this series.  Thanks,

Alex 


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v6 5/5] vfio/type1: Use mdev bus iommu_ops for IOMMU callbacks
@ 2020-10-30 21:06       ` Alex Williamson
  0 siblings, 0 replies; 30+ messages in thread
From: Alex Williamson @ 2020-10-30 21:06 UTC (permalink / raw)
  To: Tian, Kevin
  Cc: Jean-Philippe Brucker, Jiang, Dave, Raj, Ashok, kvm,
	Cornelia Huck, linux-kernel, iommu, Robin Murphy

On Fri, 30 Oct 2020 06:16:28 +0000
"Tian, Kevin" <kevin.tian@intel.com> wrote:

> > From: Lu Baolu <baolu.lu@linux.intel.com>
> > Sent: Friday, October 30, 2020 12:58 PM
> > 
> > With the IOMMU driver registering iommu_ops for the mdev_bus, the
> > IOMMU
> > operations on an mdev could be done in the same way as any normal device
> > (for example, PCI/PCIe). There's no need to distinguish an mdev from
> > others for iommu operations. Remove the unnecessary code.  
> 
> This is really a nice cleanup as the output of this change! :)

It's easy to remove a bunch of code when the result is breaking
everyone else.  Please share with me how SR-IOV backed mdevs continue
to work on AMD platforms, or how they might work on ARM platforms, when
siov_iommu_ops (VT-d only) becomes the one and only provider of
iommu_ops on the mdev bus.  Hard NAK on this series.  Thanks,

Alex 

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v6 5/5] vfio/type1: Use mdev bus iommu_ops for IOMMU callbacks
  2020-10-30 21:06       ` Alex Williamson
@ 2020-11-03  5:22         ` Lu Baolu
  -1 siblings, 0 replies; 30+ messages in thread
From: Lu Baolu @ 2020-11-03  5:22 UTC (permalink / raw)
  To: Alex Williamson, Tian, Kevin
  Cc: baolu.lu, Joerg Roedel, Robin Murphy, Jean-Philippe Brucker,
	Cornelia Huck, Raj, Ashok, Jiang, Dave, Liu, Yi L, Zeng, Xin,
	iommu, linux-kernel, kvm

Hi Alex,

On 10/31/20 5:06 AM, Alex Williamson wrote:
> On Fri, 30 Oct 2020 06:16:28 +0000
> "Tian, Kevin" <kevin.tian@intel.com> wrote:
> 
>>> From: Lu Baolu <baolu.lu@linux.intel.com>
>>> Sent: Friday, October 30, 2020 12:58 PM
>>>
>>> With the IOMMU driver registering iommu_ops for the mdev_bus, the
>>> IOMMU
>>> operations on an mdev could be done in the same way as any normal device
>>> (for example, PCI/PCIe). There's no need to distinguish an mdev from
>>> others for iommu operations. Remove the unnecessary code.
>>
>> This is really a nice cleanup as the output of this change! :)
> 
> It's easy to remove a bunch of code when the result is breaking
> everyone else.  Please share with me how SR-IOV backed mdevs continue
> to work on AMD platforms, or how they might work on ARM platforms, when
> siov_iommu_ops (VT-d only) becomes the one and only provider of
> iommu_ops on the mdev bus.  Hard NAK on this series.  Thanks,

I focused too much on a feature and forgot about university. I should
apologize for this. Sorry about it!

Back to the original intention of this series. The aux domain was
allocated in vfio/mdev, but it's also needed by the vDCM component of a
device driver for mediated callbacks. Currently vfio/mdev or iommu core
has no support for this.

We had a proposal when we first did aux-domain support. But was not
discussed since there was no consumer at that time.

https://lore.kernel.org/linux-iommu/20181105073408.21815-7-baolu.lu@linux.intel.com/

Does it look good to you? I can send patches of such solution for
discussion if you think it's a right way.

Extending the iommu core for subdevice passthrough support sounds an
interesting topic, but it will take much time before we reach a
consensus. It sounds a good topic for the next year's LPC/MC :-).

Best regards,
baolu

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v6 5/5] vfio/type1: Use mdev bus iommu_ops for IOMMU callbacks
@ 2020-11-03  5:22         ` Lu Baolu
  0 siblings, 0 replies; 30+ messages in thread
From: Lu Baolu @ 2020-11-03  5:22 UTC (permalink / raw)
  To: Alex Williamson, Tian, Kevin
  Cc: Jean-Philippe Brucker, Jiang, Dave, Raj, Ashok, kvm,
	Cornelia Huck, linux-kernel, iommu, Robin Murphy

Hi Alex,

On 10/31/20 5:06 AM, Alex Williamson wrote:
> On Fri, 30 Oct 2020 06:16:28 +0000
> "Tian, Kevin" <kevin.tian@intel.com> wrote:
> 
>>> From: Lu Baolu <baolu.lu@linux.intel.com>
>>> Sent: Friday, October 30, 2020 12:58 PM
>>>
>>> With the IOMMU driver registering iommu_ops for the mdev_bus, the
>>> IOMMU
>>> operations on an mdev could be done in the same way as any normal device
>>> (for example, PCI/PCIe). There's no need to distinguish an mdev from
>>> others for iommu operations. Remove the unnecessary code.
>>
>> This is really a nice cleanup as the output of this change! :)
> 
> It's easy to remove a bunch of code when the result is breaking
> everyone else.  Please share with me how SR-IOV backed mdevs continue
> to work on AMD platforms, or how they might work on ARM platforms, when
> siov_iommu_ops (VT-d only) becomes the one and only provider of
> iommu_ops on the mdev bus.  Hard NAK on this series.  Thanks,

I focused too much on a feature and forgot about university. I should
apologize for this. Sorry about it!

Back to the original intention of this series. The aux domain was
allocated in vfio/mdev, but it's also needed by the vDCM component of a
device driver for mediated callbacks. Currently vfio/mdev or iommu core
has no support for this.

We had a proposal when we first did aux-domain support. But was not
discussed since there was no consumer at that time.

https://lore.kernel.org/linux-iommu/20181105073408.21815-7-baolu.lu@linux.intel.com/

Does it look good to you? I can send patches of such solution for
discussion if you think it's a right way.

Extending the iommu core for subdevice passthrough support sounds an
interesting topic, but it will take much time before we reach a
consensus. It sounds a good topic for the next year's LPC/MC :-).

Best regards,
baolu
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v6 5/5] vfio/type1: Use mdev bus iommu_ops for IOMMU callbacks
  2020-11-03  5:22         ` Lu Baolu
@ 2020-11-12  2:31           ` Lu Baolu
  -1 siblings, 0 replies; 30+ messages in thread
From: Lu Baolu @ 2020-11-12  2:31 UTC (permalink / raw)
  To: Alex Williamson, Tian, Kevin
  Cc: baolu.lu, Joerg Roedel, Robin Murphy, Jean-Philippe Brucker,
	Cornelia Huck, Raj, Ashok, Jiang, Dave, Liu, Yi L, Zeng, Xin,
	iommu, linux-kernel, kvm

Hi Alex,

On 11/3/20 1:22 PM, Lu Baolu wrote:
> Hi Alex,
> 
> On 10/31/20 5:06 AM, Alex Williamson wrote:
>> On Fri, 30 Oct 2020 06:16:28 +0000
>> "Tian, Kevin" <kevin.tian@intel.com> wrote:
>>
>>>> From: Lu Baolu <baolu.lu@linux.intel.com>
>>>> Sent: Friday, October 30, 2020 12:58 PM
>>>>
>>>> With the IOMMU driver registering iommu_ops for the mdev_bus, the
>>>> IOMMU
>>>> operations on an mdev could be done in the same way as any normal 
>>>> device
>>>> (for example, PCI/PCIe). There's no need to distinguish an mdev from
>>>> others for iommu operations. Remove the unnecessary code.
>>>
>>> This is really a nice cleanup as the output of this change! :)
>>
>> It's easy to remove a bunch of code when the result is breaking
>> everyone else.  Please share with me how SR-IOV backed mdevs continue
>> to work on AMD platforms, or how they might work on ARM platforms, when
>> siov_iommu_ops (VT-d only) becomes the one and only provider of
>> iommu_ops on the mdev bus.  Hard NAK on this series.  Thanks,
> 
> I focused too much on a feature and forgot about university. I should
> apologize for this. Sorry about it!
> 
> Back to the original intention of this series. The aux domain was
> allocated in vfio/mdev, but it's also needed by the vDCM component of a
> device driver for mediated callbacks. Currently vfio/mdev or iommu core
> has no support for this.
> 
> We had a proposal when we first did aux-domain support. But was not
> discussed since there was no consumer at that time.
> 
> https://lore.kernel.org/linux-iommu/20181105073408.21815-7-baolu.lu@linux.intel.com/ 

Exposing iommu_domain outside of the vfio/iommu abstract seems not a
secure idea. I have posted a new proposal. Can you please help to
review?

https://lore.kernel.org/linux-iommu/20201112022407.2063896-1-baolu.lu@linux.intel.com/

Best regards.
baolu

> 
> 
> Does it look good to you? I can send patches of such solution for
> discussion if you think it's a right way.
> 
> Extending the iommu core for subdevice passthrough support sounds an
> interesting topic, but it will take much time before we reach a
> consensus. It sounds a good topic for the next year's LPC/MC :-).
> 
> Best regards,
> baolu

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v6 5/5] vfio/type1: Use mdev bus iommu_ops for IOMMU callbacks
@ 2020-11-12  2:31           ` Lu Baolu
  0 siblings, 0 replies; 30+ messages in thread
From: Lu Baolu @ 2020-11-12  2:31 UTC (permalink / raw)
  To: Alex Williamson, Tian, Kevin
  Cc: Jean-Philippe Brucker, Jiang, Dave, Raj, Ashok, kvm,
	Cornelia Huck, linux-kernel, iommu, Robin Murphy

Hi Alex,

On 11/3/20 1:22 PM, Lu Baolu wrote:
> Hi Alex,
> 
> On 10/31/20 5:06 AM, Alex Williamson wrote:
>> On Fri, 30 Oct 2020 06:16:28 +0000
>> "Tian, Kevin" <kevin.tian@intel.com> wrote:
>>
>>>> From: Lu Baolu <baolu.lu@linux.intel.com>
>>>> Sent: Friday, October 30, 2020 12:58 PM
>>>>
>>>> With the IOMMU driver registering iommu_ops for the mdev_bus, the
>>>> IOMMU
>>>> operations on an mdev could be done in the same way as any normal 
>>>> device
>>>> (for example, PCI/PCIe). There's no need to distinguish an mdev from
>>>> others for iommu operations. Remove the unnecessary code.
>>>
>>> This is really a nice cleanup as the output of this change! :)
>>
>> It's easy to remove a bunch of code when the result is breaking
>> everyone else.  Please share with me how SR-IOV backed mdevs continue
>> to work on AMD platforms, or how they might work on ARM platforms, when
>> siov_iommu_ops (VT-d only) becomes the one and only provider of
>> iommu_ops on the mdev bus.  Hard NAK on this series.  Thanks,
> 
> I focused too much on a feature and forgot about university. I should
> apologize for this. Sorry about it!
> 
> Back to the original intention of this series. The aux domain was
> allocated in vfio/mdev, but it's also needed by the vDCM component of a
> device driver for mediated callbacks. Currently vfio/mdev or iommu core
> has no support for this.
> 
> We had a proposal when we first did aux-domain support. But was not
> discussed since there was no consumer at that time.
> 
> https://lore.kernel.org/linux-iommu/20181105073408.21815-7-baolu.lu@linux.intel.com/ 

Exposing iommu_domain outside of the vfio/iommu abstract seems not a
secure idea. I have posted a new proposal. Can you please help to
review?

https://lore.kernel.org/linux-iommu/20201112022407.2063896-1-baolu.lu@linux.intel.com/

Best regards.
baolu

> 
> 
> Does it look good to you? I can send patches of such solution for
> discussion if you think it's a right way.
> 
> Extending the iommu core for subdevice passthrough support sounds an
> interesting topic, but it will take much time before we reach a
> consensus. It sounds a good topic for the next year's LPC/MC :-).
> 
> Best regards,
> baolu
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2020-11-12  5:34 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-30  4:58 [PATCH v6 0/5] iommu aux-domain APIs extensions Lu Baolu
2020-10-30  4:58 ` Lu Baolu
2020-10-30  4:58 ` [PATCH v6 1/5] vfio/mdev: Register mdev bus earlier during boot Lu Baolu
2020-10-30  4:58   ` Lu Baolu
2020-10-30 17:02   ` Alex Williamson
2020-10-30 17:02     ` Alex Williamson
2020-10-30  4:58 ` [PATCH v6 2/5] iommu: Use bus iommu ops for aux related callback Lu Baolu
2020-10-30  4:58   ` Lu Baolu
2020-10-30  5:55   ` Tian, Kevin
2020-10-30  5:55     ` Tian, Kevin
2020-10-30 10:47     ` Jean-Philippe Brucker
2020-10-30 10:47       ` Jean-Philippe Brucker
2020-10-30  4:58 ` [PATCH v6 3/5] iommu/vt-d: Make some static functions global Lu Baolu
2020-10-30  4:58   ` Lu Baolu
2020-10-30  4:58 ` [PATCH v6 4/5] iommu/vt-d: Add iommu_ops support for subdevice bus Lu Baolu
2020-10-30  4:58   ` Lu Baolu
2020-10-30  6:13   ` Tian, Kevin
2020-10-30  6:13     ` Tian, Kevin
2020-10-30 20:56   ` Alex Williamson
2020-10-30 20:56     ` Alex Williamson
2020-10-30  4:58 ` [PATCH v6 5/5] vfio/type1: Use mdev bus iommu_ops for IOMMU callbacks Lu Baolu
2020-10-30  4:58   ` Lu Baolu
2020-10-30  6:16   ` Tian, Kevin
2020-10-30  6:16     ` Tian, Kevin
2020-10-30 21:06     ` Alex Williamson
2020-10-30 21:06       ` Alex Williamson
2020-11-03  5:22       ` Lu Baolu
2020-11-03  5:22         ` Lu Baolu
2020-11-12  2:31         ` Lu Baolu
2020-11-12  2:31           ` Lu Baolu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.