RE: [PATCH 09/14] iommufd: Add iommufd_device_replace()

["Tian, Kevin" <kevin.tian@intel.com>]

> From: Jason Gunthorpe <jgg@nvidia.com>
> 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@nvidia.com>
> > > 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.