diff mbox series

[v2,12/17] iommufd: Add iommufd_device_replace()

Message ID 12-v2-51b9896e7862+8a8c-iommufd_alloc_jgg@nvidia.com (mailing list archive)
State New, archived
Headers show
Series Add iommufd physical device operations for replace and alloc hwpt | expand

Commit Message

Jason Gunthorpe March 8, 2023, 12:35 a.m. UTC
Replace allows all the devices in a group to move in one step to a new
HWPT. Further, the HWPT move is done without going through a blocking
domain so that the IOMMU driver can implement some level of
non-distruption to ongoing DMA if that has meaning for it (eg for future
special driver domains)

Replace uses a lot of the same logic as normal attach, except the actual
domain change over has different restrictions, and we are careful to
sequence things so that failure is going to leave everything the way it
was, and not get trapped in a blocking domain or something if there is
ENOMEM.

Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
 drivers/iommu/iommufd/device.c | 91 ++++++++++++++++++++++++++++++++++
 drivers/iommu/iommufd/main.c   |  1 +
 2 files changed, 92 insertions(+)

Comments

Baolu Lu March 9, 2023, 1:14 a.m. UTC | #1
On 3/8/23 8:35 AM, Jason Gunthorpe wrote:
> Replace allows all the devices in a group to move in one step to a new
> HWPT. Further, the HWPT move is done without going through a blocking
> domain so that the IOMMU driver can implement some level of
> non-distruption to ongoing DMA if that has meaning for it (eg for future
> special driver domains)
> 
> Replace uses a lot of the same logic as normal attach, except the actual
> domain change over has different restrictions, and we are careful to
> sequence things so that failure is going to leave everything the way it
> was, and not get trapped in a blocking domain or something if there is
> ENOMEM.
> 
> Signed-off-by: Jason Gunthorpe<jgg@nvidia.com>

Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>

Best regards,
baolu
Tian, Kevin March 10, 2023, 11:48 a.m. UTC | #2
> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Wednesday, March 8, 2023 8:36 AM
> 
> @@ -359,6 +360,76 @@ iommufd_device_do_attach(struct iommufd_device
> *idev,
>  	return NULL;
>  }
> 
> +static struct iommufd_hw_pagetable *
> +iommufd_device_do_replace(struct iommufd_device *idev,
> +			  struct iommufd_hw_pagetable *hwpt)
> +{
> +	struct iommufd_group *igroup = idev->igroup;
> +	struct iommufd_hw_pagetable *old_hwpt;
> +	unsigned int num_devices = 0;
> +	struct iommufd_device *cur;
> +	int rc;
> +
> +	mutex_lock(&idev->igroup->lock);
> +
> +	if (hwpt == igroup->hwpt) {
> +		mutex_unlock(&idev->igroup->lock);
> +		return NULL;
> +	}
> +
> +	/* Try to upgrade the domain we have */
> +	list_for_each_entry(cur, &igroup->device_list, group_item) {
> +		num_devices++;
> +		if (cur->enforce_cache_coherency) {
> +			rc = iommufd_hw_pagetable_enforce_cc(hwpt);
> +			if (rc)
> +				goto err_unlock;
> +		}
> +	}
> +
> +	old_hwpt = igroup->hwpt;
> +	if (hwpt->ioas != old_hwpt->ioas) {

Do we allow this function to be used as attach/detach?

if yes old_hwpt could be NULL here.

if no an explicit check on old_hwpt!=NULL is also required.

and looks the policy for physical attach is different from the one that
Nicoline applies for access. iommufd_access_set_ioas() can behave
like normal attach/detach.

should be consistent here.
Jason Gunthorpe March 10, 2023, 3:04 p.m. UTC | #3
On Fri, Mar 10, 2023 at 11:48:58AM +0000, Tian, Kevin wrote:
> > From: Jason Gunthorpe <jgg@nvidia.com>
> > Sent: Wednesday, March 8, 2023 8:36 AM
> > 
> > @@ -359,6 +360,76 @@ iommufd_device_do_attach(struct iommufd_device
> > *idev,
> >  	return NULL;
> >  }
> > 
> > +static struct iommufd_hw_pagetable *
> > +iommufd_device_do_replace(struct iommufd_device *idev,
> > +			  struct iommufd_hw_pagetable *hwpt)
> > +{
> > +	struct iommufd_group *igroup = idev->igroup;
> > +	struct iommufd_hw_pagetable *old_hwpt;
> > +	unsigned int num_devices = 0;
> > +	struct iommufd_device *cur;
> > +	int rc;
> > +
> > +	mutex_lock(&idev->igroup->lock);
> > +
> > +	if (hwpt == igroup->hwpt) {
> > +		mutex_unlock(&idev->igroup->lock);
> > +		return NULL;
> > +	}
> > +
> > +	/* Try to upgrade the domain we have */
> > +	list_for_each_entry(cur, &igroup->device_list, group_item) {
> > +		num_devices++;
> > +		if (cur->enforce_cache_coherency) {
> > +			rc = iommufd_hw_pagetable_enforce_cc(hwpt);
> > +			if (rc)
> > +				goto err_unlock;
> > +		}
> > +	}
> > +
> > +	old_hwpt = igroup->hwpt;
> > +	if (hwpt->ioas != old_hwpt->ioas) {
> 
> Do we allow this function to be used as attach/detach?

No. NULL is never a valid input..

> if yes old_hwpt could be NULL here.

Er that test got lost in the last version it is neeed

> and looks the policy for physical attach is different from the one that
> Nicoline applies for access. iommufd_access_set_ioas() can behave
> like normal attach/detach.

Access should change.

Thanks,
Jason
diff mbox series

Patch

diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
index 0256e65476f045..68d1bee1cdb68c 100644
--- a/drivers/iommu/iommufd/device.c
+++ b/drivers/iommu/iommufd/device.c
@@ -4,6 +4,7 @@ 
 #include <linux/iommufd.h>
 #include <linux/slab.h>
 #include <linux/iommu.h>
+#include "../iommu-priv.h"
 
 #include "io_pagetable.h"
 #include "iommufd_private.h"
@@ -359,6 +360,76 @@  iommufd_device_do_attach(struct iommufd_device *idev,
 	return NULL;
 }
 
+static struct iommufd_hw_pagetable *
+iommufd_device_do_replace(struct iommufd_device *idev,
+			  struct iommufd_hw_pagetable *hwpt)
+{
+	struct iommufd_group *igroup = idev->igroup;
+	struct iommufd_hw_pagetable *old_hwpt;
+	unsigned int num_devices = 0;
+	struct iommufd_device *cur;
+	int rc;
+
+	mutex_lock(&idev->igroup->lock);
+
+	if (hwpt == igroup->hwpt) {
+		mutex_unlock(&idev->igroup->lock);
+		return NULL;
+	}
+
+	/* Try to upgrade the domain we have */
+	list_for_each_entry(cur, &igroup->device_list, group_item) {
+		num_devices++;
+		if (cur->enforce_cache_coherency) {
+			rc = iommufd_hw_pagetable_enforce_cc(hwpt);
+			if (rc)
+				goto err_unlock;
+		}
+	}
+
+	old_hwpt = igroup->hwpt;
+	if (hwpt->ioas != old_hwpt->ioas) {
+		list_for_each_entry(cur, &igroup->device_list, group_item) {
+			rc = iopt_table_enforce_dev_resv_regions(
+				&hwpt->ioas->iopt, cur->dev, NULL);
+			if (rc)
+				goto err_unresv;
+		}
+	}
+
+	rc = iommufd_group_setup_msi(idev->igroup, hwpt);
+	if (rc)
+		goto err_unresv;
+
+	rc = iommu_group_replace_domain(igroup->group, hwpt->domain);
+	if (rc)
+		goto err_unresv;
+
+	if (hwpt->ioas != old_hwpt->ioas) {
+		list_for_each_entry(cur, &igroup->device_list, group_item)
+			iopt_remove_reserved_iova(&old_hwpt->ioas->iopt,
+						  cur->dev);
+	}
+
+	igroup->hwpt = hwpt;
+
+	/* Move the refcounts held by the device_list to the new hwpt */
+	refcount_add(num_devices, &hwpt->obj.users);
+	if (num_devices > 1)
+		WARN_ON(refcount_sub_and_test(num_devices - 1,
+					      &old_hwpt->obj.users));
+	mutex_unlock(&idev->igroup->lock);
+
+	/* Caller must destroy old_hwpt */
+	return old_hwpt;
+err_unresv:
+	list_for_each_entry(cur, &igroup->device_list, group_item)
+		iopt_remove_reserved_iova(&hwpt->ioas->iopt, cur->dev);
+err_unlock:
+	mutex_unlock(&idev->igroup->lock);
+	return ERR_PTR(rc);
+}
+
 typedef struct iommufd_hw_pagetable *(*attach_fn)(
 	struct iommufd_device *idev, struct iommufd_hw_pagetable *hwpt);
 
@@ -509,6 +580,26 @@  int iommufd_device_attach(struct iommufd_device *idev, u32 *pt_id)
 }
 EXPORT_SYMBOL_NS_GPL(iommufd_device_attach, IOMMUFD);
 
+/**
+ * iommufd_device_replace - Change the device's iommu_domain
+ * @idev: device to change
+ * @pt_id: Input a IOMMUFD_OBJ_IOAS, or IOMMUFD_OBJ_HW_PAGETABLE
+ *         Output the IOMMUFD_OBJ_HW_PAGETABLE ID
+ *
+ * This is the same as
+ *   iommufd_device_detach();
+ *   iommufd_device_attach();
+ * If it fails then no change is made to the attachment. The iommu driver may
+ * implement this so there is no disruption in translation. This can only be
+ * called if iommufd_device_attach() has already succeeded.
+ */
+int iommufd_device_replace(struct iommufd_device *idev, u32 *pt_id)
+{
+	return iommufd_device_change_pt(idev, pt_id,
+					&iommufd_device_do_replace);
+}
+EXPORT_SYMBOL_NS_GPL(iommufd_device_replace, IOMMUFD);
+
 /**
  * iommufd_device_detach - Disconnect a device to an iommu_domain
  * @idev: device to detach
diff --git a/drivers/iommu/iommufd/main.c b/drivers/iommu/iommufd/main.c
index e5ed5dfa91a0b5..8597f2fb88da3a 100644
--- a/drivers/iommu/iommufd/main.c
+++ b/drivers/iommu/iommufd/main.c
@@ -461,5 +461,6 @@  module_exit(iommufd_exit);
 MODULE_ALIAS_MISCDEV(VFIO_MINOR);
 MODULE_ALIAS("devname:vfio/vfio");
 #endif
+MODULE_IMPORT_NS(IOMMUFD_INTERNAL);
 MODULE_DESCRIPTION("I/O Address Space Management for passthrough devices");
 MODULE_LICENSE("GPL");