Skip to contentSkip to conversation

iommu aux-domain APIs extensions

PATCH In review Lu Baolu 4 participants 15 messages Oct 30, 2020 – Nov 12, 2020

Subsystems (5)
lore
Share & tools
lore

Lu Baolu

author maintainer
#0 lore raw

v6 cover iommu aux-domain APIs extensions

Hi Joerg and Alex,

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

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

The previous version was posted here.

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

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

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

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 <[email protected]> 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
Signature · 1 line
2.25.1
author maintainer
#1 lore raw

PATCH v6 1/5 vfio/mdev: Register mdev bus earlier during boot

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.

---
 drivers/vfio/mdev/mdev_core.c | 4 ++++
 1 file changed, 4 insertions(+)
drivers/vfio/mdev/mdev_core.c +4 -0
index b558d4cfd082..6b9ab71f89e7 100644
@@ -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");
Signature · 1 line
2.25.1
#2 lore raw

Re: [PATCH v6 1/5] vfio/mdev: Register mdev bus earlier during boot

Quoted 6 lines · On Fri, 30 Oct 2020 12:58:05 +0800 Lu Baolu <[email protected]> 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.

---

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

Quoted 20 lines
 drivers/vfio/mdev/mdev_core.c | 4 ++++
 1 file changed, 4 insertions(+)
drivers/vfio/mdev/mdev_core.c +4 -0
index b558d4cfd082..6b9ab71f89e7 100644
@@ -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");
author maintainer
#3 lore raw

PATCH v6 2/5 iommu: Use bus iommu ops for aux related callback

In reply to 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.

---
 drivers/iommu/iommu.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)
drivers/iommu/iommu.c +10 -6
index 6bbdd959f9f3..17f2686664db 100644
@@ -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;
 }
Signature · 6 lines
2.25.1

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

RE: [PATCH v6 2/5] iommu: Use bus iommu ops for aux related callback

Quoted 6 lines

From: Lu Baolu <[email protected]>
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

Quoted 59 lines
---
 drivers/iommu/iommu.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)
drivers/iommu/iommu.c +3 -2
index 6bbdd959f9f3..17f2686664db 100644
@@ -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);
 
drivers/iommu/iommu.c +4 -2
 	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);
drivers/iommu/iommu.c +3 -2
 	}
 }
@@ -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

#5 lore raw

Re: [PATCH v6 2/5] iommu: Use bus iommu ops for aux related callback

Quoted 10 lines · Kevin
Quoted 6 lines

From: Lu Baolu <[email protected]>
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/[email protected]/

Quoted 1 line

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

author maintainer
#6 lore raw

PATCH v6 3/5 iommu/vt-d: Make some static functions global

In reply to Lu Baolu

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

---
 drivers/iommu/intel/iommu.c | 74 +++++++------------------------------
 include/linux/intel-iommu.h | 49 ++++++++++++++++++++++++
 2 files changed, 62 insertions(+), 61 deletions(-)
drivers/iommu/intel/iommu.c +13 -61
index 8f5e7b31b3fb..1454fe74f3ba 100644
@@ -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;
include/linux/intel-iommu.h +49 -0

61 lines not on this page · raw message

Signature · 6 lines
2.25.1

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

PATCH v6 4/5 iommu/vt-d: Add iommu_ops support for subdevice bus

In reply to 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.

---
 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
drivers/iommu/intel/Kconfig +13 -0
index 28a3d1596c76..94edc332f558 100644
@@ -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.
drivers/iommu/intel/Makefile +1 -0
index fb8e1e8c8029..f216385d5d59 100644
@@ -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
drivers/iommu/intel/iommu.c +5 -0
index 1454fe74f3ba..dafd8069c2af 100644
@@ -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,
drivers/iommu/intel/siov.c new file +119 -0

119 lines not on this page · raw message

include/linux/intel-iommu.h +4 -0
index d3928ba87986..b790efa5509f 100644
@@ -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);
Signature · 6 lines
2.25.1

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

RE: [PATCH v6 4/5] iommu/vt-d: Add iommu_ops support for subdevice bus

Quoted 16 lines

From: Lu Baolu <[email protected]>
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.

Quoted 2 lines

- 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.

Quoted 22 lines

- 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.

---
 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
drivers/iommu/intel/Kconfig +0 -0
index 28a3d1596c76..94edc332f558 100644

drivers/iommu/intel/Kconfig config INTEL_IOMMU_SCALABLE_MODE_DEFAULT_ON · line 90

Quoted 5 lines
drivers/iommu/intel/Kconfig +2 -0
@@ -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...

Quoted 54 lines
drivers/iommu/intel/Kconfig +11 -0
+	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.
drivers/iommu/intel/Makefile +1 -0
index fb8e1e8c8029..f216385d5d59 100644
@@ -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
drivers/iommu/intel/iommu.c +5 -0
index 1454fe74f3ba..dafd8069c2af 100644
@@ -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,

drivers/iommu/intel/siov.c new file +10 -0
new file mode 100644
index 000000000000..b9470e7ab3d6
@@ -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 <[email protected]>
+ */
+
+#define pr_fmt(fmt)	"DMAR: " fmt

drivers/iommu/intel/siov.c line 20

Quoted 10 lines
drivers/iommu/intel/siov.c new file +10 -0
+
+#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.

Quoted 15 lines
drivers/iommu/intel/siov.c new file +15 -0
+
+	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)
+{

drivers/iommu/intel/siov.c line 45

Quoted 10 lines
drivers/iommu/intel/siov.c new file +10 -0
+	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

Quoted 94 lines
drivers/iommu/intel/siov.c new file +74 -0
+}
+
+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");
+}
include/linux/intel-iommu.h +4 -0
index d3928ba87986..b790efa5509f 100644
@@ -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

#9 lore raw

Re: [PATCH v6 4/5] iommu/vt-d: Add iommu_ops support for subdevice bus

In reply to Lu Baolu
Quoted 15 lines · On Fri, 30 Oct 2020 12:58:08 +0800 Lu Baolu <[email protected]> wrote:
drivers/iommu/intel/siov.c new file +15 -0
+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),

drivers/iommu/intel/siov.c line 119

Quoted 10 lines
drivers/iommu/intel/siov.c new file +10 -0
+};
+
+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
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu

author maintainer
#10 lore raw

PATCH v6 5/5 vfio/type1: Use mdev bus iommu_ops for IOMMU callbacks

In reply to 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.

---
 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(-)
drivers/vfio/mdev/mdev_core.c +0 -18
index 6b9ab71f89e7..f4fd5f237c49 100644
@@ -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();
drivers/vfio/mdev/mdev_driver.c +6 -0
index 0d3223aee20b..487402f16355 100644
@@ -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");
 }
drivers/vfio/mdev/mdev_private.h +0 -1
index 7d922950caaf..efe0aefdb52f 100644
@@ -31,7 +31,6 @@ struct mdev_device {
 	void *driver_data;
 	struct list_head next;
 	struct kobject *type_kobj;
-	struct device *iommu_device;
 	bool active;
 };
 
drivers/vfio/vfio_iommu_type1.c +12 -116

194 lines not on this page · raw message

include/linux/mdev.h +0 -14
index 0ce30ca78db0..f7aee86bd2b0 100644
@@ -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.
Signature · 6 lines
2.25.1

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

RE: [PATCH v6 5/5] vfio/type1: Use mdev bus iommu_ops for IOMMU callbacks

Quoted 8 lines

From: Lu Baolu <[email protected]>
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

Quoted 340 lines
---
 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(-)
drivers/vfio/mdev/mdev_core.c +0 -18
index 6b9ab71f89e7..f4fd5f237c49 100644
@@ -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)
 {
drivers/vfio/mdev/mdev_core.c +0 -0
 	return mdev_bus_register();
drivers/vfio/mdev/mdev_driver.c +3 -0
index 0d3223aee20b..487402f16355 100644
@@ -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))
drivers/vfio/mdev/mdev_driver.c +3 -0
 		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");

}

drivers/vfio/mdev/mdev_private.h +0 -1
index 7d922950caaf..efe0aefdb52f 100644
@@ -31,7 +31,6 @@ struct mdev_device {
 	void *driver_data;
 	struct list_head next;
 	struct kobject *type_kobj;
-	struct device *iommu_device;
 	bool active;
 };
 
drivers/vfio/vfio_iommu_type1.c +0 -28
index bb2684cc245e..e231b7070ca5 100644
@@ -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);

drivers/vfio/vfio_iommu_type1.c +0 -13
-	}
-
-	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))

drivers/vfio/vfio_iommu_type1.c +0 -17
-			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-
Quoted 1 line

iommu_group);

drivers/vfio/vfio_iommu_type1.c +0 -7
-}
-
-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,

drivers/vfio/vfio_iommu_type1.c +0 -3
-					 vfio_mdev_detach_domain);
-	else
-		iommu_detach_group(domain->domain, group-
Quoted 1 line

iommu_group);

drivers/vfio/vfio_iommu_type1.c +1 -30
-}
-
-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;
drivers/vfio/vfio_iommu_type1.c +4 -9
 	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) {
drivers/vfio/vfio_iommu_type1.c +0 -2
 				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;
 	}
 
drivers/vfio/vfio_iommu_type1.c +4 -4
 	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);

drivers/vfio/vfio_iommu_type1.c +1 -1
 			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);
drivers/vfio/vfio_iommu_type1.c +1 -1
 	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);
drivers/vfio/vfio_iommu_type1.c +1 -1
 		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);
drivers/vfio/vfio_iommu_type1.c +0 -0
 		kfree(group);
 	}
include/linux/mdev.h +0 -14
index 0ce30ca78db0..f7aee86bd2b0 100644
@@ -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

#12 lore raw

Re: [PATCH v6 5/5] vfio/type1: Use mdev bus iommu_ops for IOMMU callbacks

Quoted 10 lines · Kevin
Quoted 8 lines

From: Lu Baolu <[email protected]>
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
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu

author maintainer
#13 lore raw

Re: [PATCH v6 5/5] vfio/type1: Use mdev bus iommu_ops for IOMMU callbacks

Hi Alex,

Quoted 12 lines · Alex Williamson
Quoted 9 lines · Kevin
Quoted 8 lines

From: Lu Baolu <[email protected]>
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.

Quoted 7 lines
Quoted 1 line

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/[email protected]/

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

author maintainer
#14 lore raw

Re: [PATCH v6 5/5] vfio/type1: Use mdev bus iommu_ops for IOMMU callbacks

Hi Alex,

Quoted 31 lines · Lu Baolu

Hi Alex,

Quoted 12 lines · Alex Williamson
Quoted 9 lines · Kevin
Quoted 8 lines

From: Lu Baolu <[email protected]>
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.

Quoted 7 lines
Quoted 1 line

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.

Quoted 4 lines

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/[email protected]/

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/[email protected]/

Best regards.
baolu

Quoted 11 lines

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
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu