iommu aux-domain APIs extensions

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

---

 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");

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.

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

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

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?

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.

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

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

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),

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.

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

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

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! :)

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

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