> From: Jason Gunthorpe <jgg@xxxxxxxxxx>
> Sent: Tuesday, March 7, 2023 4:45 AM
>
> 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
>
but setup_msi() is still done for every device which is inconsistent
with what patch5 does.
