Re: [PATCH 8/8] iommu/vt-d: Add set_dev_pasid callback for nested domain

[Yang, Weijiang]

On 11/27/2023 2:34 PM, Yi Liu wrote:

From: Lu Baolu <baolu...@linux.intel.com>

This allows the upper layers to set a nested type domain to a PASID of a
device if the PASID feature is supported by the IOMMU hardware.

The set_dev_pasid callback for non-nest domain has already be there, so
this only needs to add it for nested domains.

Signed-off-by: Lu Baolu <baolu...@linux.intel.com>
Signed-off-by: Yi Liu <yi.l....@intel.com>
---
  drivers/iommu/intel/nested.c | 47 ++++++++++++++++++++++++++++++++++++
  1 file changed, 47 insertions(+)

diff --git a/drivers/iommu/intel/nested.c b/drivers/iommu/intel/nested.c
index 44ad48db7ea0..f6f687750104 100644
--- a/drivers/iommu/intel/nested.c
+++ b/drivers/iommu/intel/nested.c
@@ -68,6 +68,52 @@ static int intel_nested_attach_dev(struct iommu_domain 
*domain,
        return 0;
  }

+static int intel_nested_set_dev_pasid(struct iommu_domain *domain,

+                                     struct device *dev, ioasid_t pasid)
+{
+       struct device_domain_info *info = dev_iommu_priv_get(dev);
+       struct dmar_domain *dmar_domain = to_dmar_domain(domain);
+       struct intel_iommu *iommu = info->iommu;
+       struct dev_pasid_info *dev_pasid;
+       unsigned long flags;
+       int ret = 0;
+
+       if (!pasid_supported(iommu))
+               return -EOPNOTSUPP;
+
+       if (iommu->agaw < dmar_domain->s2_domain->agaw)
+               return -EINVAL;
+
+       ret = prepare_domain_attach_device(&dmar_domain->s2_domain->domain, 
dev);
+       if (ret)
+               return ret;
+
+       dev_pasid = kzalloc(sizeof(*dev_pasid), GFP_KERNEL);
+       if (!dev_pasid)
+               return -ENOMEM;
+
+       ret = domain_attach_iommu(dmar_domain, iommu);
+       if (ret)
+               goto err_free;
+
+       ret = intel_pasid_setup_nested(iommu, dev, pasid, dmar_domain);
+       if (ret)
+               goto err_detach_iommu;
+
+       dev_pasid->dev = dev;
+       dev_pasid->pasid = pasid;
+       spin_lock_irqsave(&dmar_domain->lock, flags);
+       list_add(&dev_pasid->link_domain, &dmar_domain->dev_pasids);
+       spin_unlock_irqrestore(&dmar_domain->lock, flags);

---> list_add(&dev_pasid->link_domain, &dmar_domain->dev_pasids);

dev_pasid is linked at later time, this leads to domain->has_iotlb_device is 
not correctly set, which finally results into a missing of device iotlb flush in 
iommu_flush_dev_iotlb()when it's called.
Check this call path:
domain_attach_iommu()->domain_update_iommu_cap()->domain_update_iotlb()->domain->has_iotlb_device
 = has_iotlb_device; The ugly fixup is to call domain_update_iommu_cap() or 
domain_update_iotlb() here again before return.
The similar issue is in intel_iommu_set_dev_pasid() and 
intel_nested_attach_dev().

+
+       return 0;
+err_detach_iommu:
+       domain_detach_iommu(dmar_domain, iommu);
+err_free:
+       kfree(dev_pasid);
+       return ret;
+}
+
  static void intel_nested_domain_free(struct iommu_domain *domain)
  {
        kfree(to_dmar_domain(domain));
@@ -128,6 +174,7 @@ static int intel_nested_cache_invalidate_user(struct 
iommu_domain *domain,

static const struct iommu_domain_ops intel_nested_domain_ops = {

        .attach_dev             = intel_nested_attach_dev,
+       .set_dev_pasid          = intel_nested_set_dev_pasid,
        .free                   = intel_nested_domain_free,
        .cache_invalidate_user  = intel_nested_cache_invalidate_user,
  };