> From: Liu, Yi L <yi.l.liu@xxxxxxxxx> > Sent: Friday, June 28, 2024 4:56 PM > > @@ -806,5 +806,7 @@ void intel_iommu_debugfs_create_dev_pasid(struct > dev_pasid_info *dev_pasid) > /* Remove the device pasid debugfs directory. */ > void intel_iommu_debugfs_remove_dev_pasid(struct dev_pasid_info > *dev_pasid) > { > + if (!dev_pasid->debugfs_dentry) > + return; > debugfs_remove_recursive(dev_pasid->debugfs_dentry); debugfs_remove_recursive() already includes a check on null entry. > -static void intel_iommu_remove_dev_pasid(struct device *dev, ioasid_t > pasid, > - struct iommu_domain *domain) > +static void domain_undo_dev_pasid(struct iommu_domain *domain, > + struct device *dev, ioasid_t pasid) 'domain_remove_dev_pasid' > +static void intel_iommu_remove_dev_pasid(struct device *dev, ioasid_t > pasid, > + struct iommu_domain *domain) > +{ > + struct device_domain_info *info = dev_iommu_priv_get(dev); > + struct intel_iommu *iommu = info->iommu; > + > intel_pasid_tear_down_entry(iommu, dev, pasid, false, true); > + if (domain->type == IOMMU_DOMAIN_IDENTITY) > + return; > + domain_undo_dev_pasid(domain, dev, pasid); this reverts the order by tearing down the pasid entry in the beginning. but I cannot think of a problem here. > > - 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); > + /* Undo the association between the old domain and pasid of a > device */ > + if (old) > + domain_undo_dev_pasid(old, dev, pasid); the comment is useless