RE: [PATCH 06/11] iommu: Move the iommu driver sysfs setup into iommu_init/deinit_driver()

2023-04-26 Thread Tian, Kevin
> From: Jason Gunthorpe 
> Sent: Thursday, April 20, 2023 12:12 AM
> 
> It makes logical sense that once the driver is attached to the device the
> sysfs links appear, even if we haven't fully created the group_device or
> attached the device to a domain.
> 
> Fix the missing error handling on sysfs creation since
> iommu_init_driver() can trivially handle this.
> 
> Signed-off-by: Jason Gunthorpe 

Reviewed-by: Kevin Tian 


[PATCH 06/11] iommu: Move the iommu driver sysfs setup into iommu_init/deinit_driver()

2023-04-19 Thread Jason Gunthorpe
It makes logical sense that once the driver is attached to the device the
sysfs links appear, even if we haven't fully created the group_device or
attached the device to a domain.

Fix the missing error handling on sysfs creation since
iommu_init_driver() can trivially handle this.

Signed-off-by: Jason Gunthorpe 
---
 drivers/iommu/iommu-sysfs.c |  6 --
 drivers/iommu/iommu.c   | 13 +
 2 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/iommu/iommu-sysfs.c b/drivers/iommu/iommu-sysfs.c
index 99869217fbec7d..c8aba0e2a30d70 100644
--- a/drivers/iommu/iommu-sysfs.c
+++ b/drivers/iommu/iommu-sysfs.c
@@ -107,9 +107,6 @@ int iommu_device_link(struct iommu_device *iommu, struct 
device *link)
 {
int ret;
 
-   if (!iommu || IS_ERR(iommu))
-   return -ENODEV;
-
ret = sysfs_add_link_to_group(>dev->kobj, "devices",
  >kobj, dev_name(link));
if (ret)
@@ -126,9 +123,6 @@ EXPORT_SYMBOL_GPL(iommu_device_link);
 
 void iommu_device_unlink(struct iommu_device *iommu, struct device *link)
 {
-   if (!iommu || IS_ERR(iommu))
-   return;
-
sysfs_remove_link(>kobj, "iommu");
sysfs_remove_link_from_group(>dev->kobj, "devices", 
dev_name(link));
 }
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index e428de5b386833..dbaf3ed9012c45 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -348,12 +348,16 @@ static int iommu_init_driver(struct device *dev, const 
struct iommu_ops *ops)
goto err_module_put;
}
 
+   ret = iommu_device_link(iommu_dev, dev);
+   if (ret)
+   goto err_release;
+
group = ops->device_group(dev);
if (WARN_ON_ONCE(group == NULL))
group = ERR_PTR(-EINVAL);
if (IS_ERR(group)) {
ret = PTR_ERR(group);
-   goto err_release;
+   goto err_unlink;
}
dev->iommu_group = group;
 
@@ -363,6 +367,8 @@ static int iommu_init_driver(struct device *dev, const 
struct iommu_ops *ops)
dev->iommu->attach_deferred = ops->is_attach_deferred(dev);
return 0;
 
+err_unlink:
+   iommu_device_unlink(iommu_dev, dev);
 err_release:
if (ops->release_device)
ops->release_device(dev);
@@ -380,6 +386,8 @@ static void iommu_deinit_driver(struct device *dev)
 
lockdep_assert_held(>mutex);
 
+   iommu_device_unlink(dev->iommu->iommu_dev, dev);
+
/*
 * release_device() must stop using any attached domain on the device.
 * If there are still other devices in the group they are not effected
@@ -454,7 +462,6 @@ static int __iommu_probe_device(struct device *dev, struct 
list_head *group_list
iommu_group_put(group);
 
mutex_unlock(_probe_device_lock);
-   iommu_device_link(dev->iommu->iommu_dev, dev);
 
return 0;
 
@@ -577,8 +584,6 @@ static void iommu_release_device(struct device *dev)
if (!dev->iommu || !group)
return;
 
-   iommu_device_unlink(dev->iommu->iommu_dev, dev);
-
__iommu_group_remove_device(dev);
 }
 
-- 
2.40.0