The existing dmar_insert_one_dev_info() implementation looks messy.
This refactors it by moving pasid table allocation to device probe
function, changing the return type to integer, adding the error
rewinding paths.

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

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 7d2fec3041e4..9a9f21fd268a 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -2379,15 +2379,6 @@ static void domain_context_clear_one(struct 
device_domain_info *info, u8 bus, u8
        __iommu_flush_dev_iotlb(info, 0, MAX_AGAW_PFN_WIDTH);
 }
 
-static inline void unlink_domain_info(struct device_domain_info *info)
-{
-       list_del_rcu(&info->link);
-       xa_erase(&device_domain_array,
-                DEVI_IDX(info->segment, info->bus, info->devfn));
-       if (info->dev)
-               dev_iommu_priv_set(info->dev, NULL);
-}
-
 struct dmar_domain *find_domain(struct device *dev)
 {
        struct device_domain_info *info;
@@ -2445,35 +2436,22 @@ static bool dev_is_real_dma_subdevice(struct device 
*dev)
               pci_real_dma_dev(to_pci_dev(dev)) != to_pci_dev(dev);
 }
 
-static struct dmar_domain *dmar_insert_one_dev_info(struct intel_iommu *iommu,
-                                                   int bus, int devfn,
-                                                   struct device *dev,
-                                                   struct dmar_domain *domain)
+static int dmar_insert_one_dev_info(struct intel_iommu *iommu, int bus,
+                                   int devfn, struct device *dev,
+                                   struct dmar_domain *domain)
 {
        struct device_domain_info *info = get_domain_info(dev);
        unsigned long flags;
        int ret;
 
        spin_lock_irqsave(&iommu->lock, flags);
+       /* Link to iommu and get a domain id: */
        ret = domain_attach_iommu(domain, iommu);
-       spin_unlock_irqrestore(&iommu->lock, flags);
        if (ret)
-               return NULL;
-
-       info->domain = domain;
-       list_add_rcu(&info->link, &domain->devices);
-
-       /* PASID table is mandatory for a PCI device in scalable mode. */
-       if (dev && dev_is_pci(dev) && sm_supported(iommu)) {
-               ret = intel_pasid_alloc_table(dev);
-               if (ret) {
-                       dev_err(dev, "PASID table allocation failed\n");
-                       dmar_remove_one_dev_info(dev);
-                       return NULL;
-               }
+               goto attach_iommu_err;
 
-               /* Setup the PASID entry for requests without PASID: */
-               spin_lock_irqsave(&iommu->lock, flags);
+       /* Setup the PASID entry for requests without PASID: */
+       if (dev_is_pci(dev) && sm_supported(iommu)) {
                if (hw_pass_through && domain_type_is_si(domain))
                        ret = intel_pasid_setup_pass_through(iommu, domain,
                                        dev, PASID_RID2PASID);
@@ -2483,21 +2461,31 @@ static struct dmar_domain 
*dmar_insert_one_dev_info(struct intel_iommu *iommu,
                else
                        ret = intel_pasid_setup_second_level(iommu, domain,
                                        dev, PASID_RID2PASID);
-               spin_unlock_irqrestore(&iommu->lock, flags);
-               if (ret) {
-                       dev_err(dev, "Setup RID2PASID failed\n");
-                       dmar_remove_one_dev_info(dev);
-                       return NULL;
-               }
+               if (ret)
+                       goto pasid_setup_err;
        }
+       spin_unlock_irqrestore(&iommu->lock, flags);
 
-       if (dev && domain_context_mapping(domain, dev)) {
-               dev_err(dev, "Domain context map failed\n");
-               dmar_remove_one_dev_info(dev);
-               return NULL;
-       }
+       /* Setup the context entry for device: */
+       ret = domain_context_mapping(domain, dev);
+       if (ret)
+               goto setup_context_err;
 
-       return domain;
+       info->domain = domain;
+       list_add_rcu(&info->link, &domain->devices);
+
+       return 0;
+
+setup_context_err:
+       spin_lock_irqsave(&iommu->lock, flags);
+       if (dev_is_pci(dev) && sm_supported(iommu))
+               intel_pasid_tear_down_entry(iommu, dev, PASID_RID2PASID, false);
+pasid_setup_err:
+       domain_detach_iommu(domain, iommu);
+attach_iommu_err:
+       spin_unlock_irqrestore(&iommu->lock, flags);
+
+       return ret;
 }
 
 static int iommu_domain_identity_map(struct dmar_domain *domain,
@@ -2575,7 +2563,6 @@ static int __init si_domain_init(int hw)
 
 static int domain_add_dev_info(struct dmar_domain *domain, struct device *dev)
 {
-       struct dmar_domain *ndomain;
        struct intel_iommu *iommu;
        u8 bus, devfn;
 
@@ -2583,11 +2570,7 @@ static int domain_add_dev_info(struct dmar_domain 
*domain, struct device *dev)
        if (!iommu)
                return -ENODEV;
 
-       ndomain = dmar_insert_one_dev_info(iommu, bus, devfn, dev, domain);
-       if (ndomain != domain)
-               return -EBUSY;
-
-       return 0;
+       return dmar_insert_one_dev_info(iommu, bus, devfn, dev, domain);
 }
 
 static bool device_has_rmrr(struct device *dev)
@@ -4001,16 +3984,13 @@ static void __dmar_remove_one_dev_info(struct 
device_domain_info *info)
 
                iommu_disable_dev_iotlb(info);
                domain_context_clear(info);
-               intel_pasid_free_table(info->dev);
        }
 
-       unlink_domain_info(info);
+       list_del_rcu(&info->link);
 
        spin_lock_irqsave(&iommu->lock, flags);
        domain_detach_iommu(domain, iommu);
        spin_unlock_irqrestore(&iommu->lock, flags);
-
-       kfree(info);
 }
 
 static void dmar_remove_one_dev_info(struct device *dev)
@@ -4310,8 +4290,10 @@ static struct iommu_device 
*intel_iommu_probe_device(struct device *dev)
        struct pci_dev *pdev = dev_is_pci(dev) ? to_pci_dev(dev) : NULL;
        struct device_domain_info *info;
        struct intel_iommu *iommu;
+       unsigned long index;
        u8 bus, devfn;
        void *curr;
+       int ret;
 
        iommu = device_to_iommu(dev, &bus, &devfn);
        if (!iommu)
@@ -4353,30 +4335,39 @@ static struct iommu_device 
*intel_iommu_probe_device(struct device *dev)
                }
        }
 
-       curr = xa_store(&device_domain_array,
-                       DEVI_IDX(info->segment, info->bus, info->devfn),
-                       info, GFP_KERNEL);
+       index = DEVI_IDX(info->segment, info->bus, info->devfn);
+       curr = xa_store(&device_domain_array, index, info, GFP_KERNEL);
        if (xa_err(curr) || WARN_ON(curr)) {
-               kfree(info);
-               return ERR_PTR(-ENOSPC);
+               ret = -ENOSPC;
+               goto free_out;
        }
 
        dev_iommu_priv_set(dev, info);
+       if (sm_supported(iommu)) {
+               ret = intel_pasid_alloc_table(dev);
+               if (ret)
+                       goto cleanup_out;
+       }
 
        return &iommu->iommu;
+
+cleanup_out:
+       dev_iommu_priv_set(dev, NULL);
+       xa_erase(&device_domain_array, index);
+free_out:
+       kfree(info);
+       return ERR_PTR(ret);
 }
 
 static void intel_iommu_release_device(struct device *dev)
 {
-       struct intel_iommu *iommu;
-
-       iommu = device_to_iommu(dev, NULL, NULL);
-       if (!iommu)
-               return;
-
-       dmar_remove_one_dev_info(dev);
+       struct device_domain_info *info = get_domain_info(dev);
+       unsigned long index = DEVI_IDX(info->segment, info->bus, info->devfn);
 
+       xa_erase(&device_domain_array, index);
+       dev_iommu_priv_set(info->dev, NULL);
        set_dma_ops(dev, NULL);
+       kfree(info);
 }
 
 static void intel_iommu_probe_finalize(struct device *dev)
-- 
2.25.1

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

Reply via email to