On Thu, Mar 02, 2023 at 08:20:05AM +0000, Tian, Kevin wrote: > > From: Jason Gunthorpe <jgg@xxxxxxxxxx> > > Sent: Saturday, February 25, 2023 8:28 AM > > > > +static struct iommufd_hw_pagetable * > > +iommufd_device_do_replace_locked(struct iommufd_device *idev, > > + struct iommufd_hw_pagetable *hwpt) > > +{ > > + struct iommufd_hw_pagetable *old_hwpt; > > + int rc; > > + > > + lockdep_assert_held(&idev->igroup->lock); > > + > > + /* Try to upgrade the domain we have */ > > + if (idev->enforce_cache_coherency) { > > + rc = iommufd_hw_pagetable_enforce_cc(hwpt); > > + if (rc) > > + return ERR_PTR(-EINVAL); > > + } > > + > > + rc = iommufd_device_setup_msi(idev, hwpt); > > + if (rc) > > + return ERR_PTR(rc); > > + > > + old_hwpt = idev->igroup->hwpt; > > + if (hwpt->ioas != old_hwpt->ioas) { > > + rc = iopt_table_enforce_group_resv_regions( > > + &hwpt->ioas->iopt, idev->igroup->group, NULL); > > + if (rc) > > + return ERR_PTR(rc); > > + } > > This is inconsistent with the earlier cleanup in the attach path > where setup_msi/enforce_group_resv_region are done only > once per group (if that is the right thing to do). Logically replace is 'detach every device in the group' - which makes devices 0 - then 'reattach them all' to the new ioas. So at this point it is still being done only once per group. The 2nd idevs to call this function will see hwpt->ioas == old_hwpt->ioas Jason