> On Sep 14, 2024, at 09:08, Baolu Lu <baolu.lu@xxxxxxxxxxxxxxx> wrote: > > On 9/13/24 8:21 PM, Yi Liu wrote: >>> On 2024/9/13 09:42, Baolu Lu wrote: >>> On 9/12/24 9:04 PM, Yi Liu wrote: >>>> @@ -4325,24 +4363,18 @@ static int intel_iommu_set_dev_pasid(struct iommu_domain *domain, >>>> ret = intel_pasid_setup_second_level(iommu, dmar_domain, >>>> dev, pasid); >>>> if (ret) >>>> - goto out_unassign_tag; >>>> + goto out_undo_dev_pasid; >>>> - 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); >>>> + if (old) >>>> + domain_remove_dev_pasid(old, dev, pasid); >>>> if (domain->type & __IOMMU_DOMAIN_PAGING) >>>> intel_iommu_debugfs_create_dev_pasid(dev_pasid); >>>> return 0; >>>> -out_unassign_tag: >>>> - cache_tag_unassign_domain(dmar_domain, dev, pasid); >>>> -out_detach_iommu: >>>> - domain_detach_iommu(dmar_domain, iommu); >>>> -out_free: >>>> - kfree(dev_pasid); >>>> + >>>> +out_undo_dev_pasid: >>>> + domain_remove_dev_pasid(domain, dev, pasid); >>>> return ret; >>>> } >>> >>> Do you need to re-install the old domain to the pasid entry in the >>> failure path? >> yes, but no. The old domain is still installed in the pasid entry >> when the failure happened. :) > > I am afraid not. The old domain has already been cleaned up by > intel_pasid_tear_down_entry(). Or not? oops, yes. The tear down was lifted to this function now instead of in the setup helpers. I may move them back to the setup helpers just like v1 does. Good catch! Regards, Yi Liu