> From: Liu, Yi L <yi.l.liu@xxxxxxxxx> > Sent: Friday, June 28, 2024 4:56 PM > > diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c > index b18eebb479de..5d3a12b081a2 100644 > --- a/drivers/iommu/intel/pasid.c > +++ b/drivers/iommu/intel/pasid.c > @@ -314,6 +314,9 @@ int intel_pasid_setup_first_level(struct intel_iommu > *iommu, > return -EINVAL; > } > > + /* Clear the old configuration if it already exists */ > + intel_pasid_tear_down_entry(iommu, dev, pasid, false, true); > + > spin_lock(&iommu->lock); > pte = intel_pasid_get_entry(dev, pasid); > if (!pte) { with this change there will be two invocations on intel_pasid_tear_down_entry() in the call stack of RID attach: intel_iommu_attach_device() device_block_translation() intel_pasid_tear_down_entry() dmar_domain_attach_device() domain_setup_first_level() intel_pasid_tear_down_entry() it's not being a real problem as intel_pasid_tear_down_entry() exits early if the pasid entry is non-present, but it will likely cause confusion when reading the code. What about moving it into intel_iommu_set_dev_pasid() to better show the purpose?