Re: [PATCH v4 5/7] iommu/vt-d: Make device pasid attachment explicit

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 4/10/23 10:46 AM, Baolu Lu wrote:
@@ -4040,6 +4100,7 @@ static void device_block_translation(struct device *dev)
      spin_lock_irqsave(&info->domain->lock, flags);
      list_del(&info->link);
+    info->dev_attached = 0;
      spin_unlock_irqrestore(&info->domain->lock, flags);
      domain_detach_iommu(info->domain, iommu);
@@ -4186,7 +4247,7 @@ static int intel_iommu_attach_device(struct iommu_domain *domain,
      if (ret)
          return ret;
-    return dmar_domain_attach_device(to_dmar_domain(domain), dev);
+    return dmar_domain_attach_device_pasid(to_dmar_domain(domain), dev, PASID_RID2PASID);
  }

For VT-d driver, attach_dev and attach_dev_pasid have different
meanings. Merging them into one helper may lead to confusion. What do
you think of the following code? The dmar_domain_attach_device_pasid()
helper could be reused for attach_dev_pasid path.

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 7c2f4bd33582..09ae62bc3724 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -2434,6 +2434,40 @@ static int __init si_domain_init(int hw)
      return 0;
  }

+
+static int dmar_domain_attach_device_pasid(struct dmar_domain *domain,
+                       struct intel_iommu *iommu,
+                       struct device *dev, ioasid_t pasid)
+{
+    struct device_pasid_info *dev_pasid;
+    unsigned long flags;
+    int ret;
+
+    dev_pasid = kzalloc(sizeof(*dev_pasid), GFP_KERNEL);
+    if (!dev_pasid)
+        return -ENOMEM;
+
+    if (hw_pass_through && domain_type_is_si(domain))
+        ret = intel_pasid_setup_pass_through(iommu, domain, dev, pasid);
+    else if (domain->use_first_level)
+        ret = domain_setup_first_level(iommu, domain, dev, pasid);
+    else
+        ret = intel_pasid_setup_second_level(iommu, domain, dev, pasid);
+
+    if (ret) {
+        kfree(dev_pasid);
+        return ret;
+    }
+
+    dev_pasid->pasid = pasid;
+    dev_pasid->dev = dev;
+    spin_lock_irqsave(&domain->lock, flags);
+    list_add(&dev_pasid->link_domain, &domain->dev_pasids);
+    spin_unlock_irqrestore(&domain->lock, flags);
+
+    return 0;
+}
+
  static int dmar_domain_attach_device(struct dmar_domain *domain,
                       struct device *dev)
  {
@@ -2458,15 +2492,8 @@ static int dmar_domain_attach_device(struct dmar_domain *domain,
      /* PASID table is mandatory for a PCI device in scalable mode. */
      if (sm_supported(iommu) && !dev_is_real_dma_subdevice(dev)) {
          /* Setup the PASID entry for requests without PASID: */
-        if (hw_pass_through && domain_type_is_si(domain))
-            ret = intel_pasid_setup_pass_through(iommu, domain,
-                    dev, PASID_RID2PASID);
-        else if (domain->use_first_level)
-            ret = domain_setup_first_level(iommu, domain, dev,
-                    PASID_RID2PASID);
-        else
-            ret = intel_pasid_setup_second_level(iommu, domain,
-                    dev, PASID_RID2PASID);
+        ret = dmar_domain_attach_device_pasid(domain, iommu, dev,
+                              PASID_RID2PASID);
          if (ret) {
              dev_err(dev, "Setup RID2PASID failed\n");
              device_block_translation(dev);

Sorry! I forgot one thing. The dev_pasid data allocated in attach_dev
path should be freed in device_block_translation(). Perhaps we need to
add below change?

@@ -4107,6 +4134,7 @@ static void device_block_translation(struct device *dev)
 {
        struct device_domain_info *info = dev_iommu_priv_get(dev);
        struct intel_iommu *iommu = info->iommu;
+       struct device_pasid_info *dev_pasid;
        unsigned long flags;

        iommu_disable_pci_caps(info);
@@ -4118,6 +4146,16 @@ static void device_block_translation(struct device *dev)
                        domain_context_clear(info);
        }

+       spin_lock_irqsave(&info->domain->lock, flags);
+       list_for_each_entry(dev_pasid, &domain->dev_pasids, link_domain) {
+               if (dev_pasid->dev != dev || dev_pasid->pasid != RID2PASID)
+                       continue;
+
+               list_del(&dev_pasid->link_domain);
+               kfree(dev_pasid);
+       }
+       spin_unlock_irqrestore(&info->domain->lock, flags);
+
        if (!info->domain)
                return;

Best regards,
baolu



[Index of Archives]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux PCI]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux