Re: [PATCH v2 3/6] iommu/vt-d: Make intel_iommu_set_dev_pasid() to handle domain replacement

[Baolu Lu <baolu.lu@xxxxxxxxxxxxxxx>]

On 9/13/24 9:35 AM, Baolu Lu wrote:
> On 9/12/24 9:04 PM, Yi Liu wrote:
> > set_dev_pasid op is going to support domain replacement and keep the old
> > hardware config if it fails. Make the Intel iommu driver be prepared for
> > it.
> >
> > Signed-off-by: Yi Liu <yi.l.liu@xxxxxxxxx>
> > ---
> >   drivers/iommu/intel/iommu.c | 98 ++++++++++++++++++++++++-------------
> >   1 file changed, 65 insertions(+), 33 deletions(-)
> >
> > diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> > index 80b587de226d..6f5a8e549f3f 100644
> > --- a/drivers/iommu/intel/iommu.c
> > +++ b/drivers/iommu/intel/iommu.c
> > @@ -4248,8 +4248,8 @@ static int intel_iommu_iotlb_sync_map(struct 
> > iommu_domain *domain,
> >       return 0;
> >   }
> > -static void intel_iommu_remove_dev_pasid(struct device *dev, ioasid_t 
> > pasid,
> > -                     struct iommu_domain *domain)
> > +static void domain_remove_dev_pasid(struct iommu_domain *domain,
> > +                    struct device *dev, ioasid_t pasid)
> >   {
> >       struct device_domain_info *info = dev_iommu_priv_get(dev);
> >       struct dev_pasid_info *curr, *dev_pasid = NULL;
> > @@ -4257,11 +4257,6 @@ static void intel_iommu_remove_dev_pasid(struct 
> > device *dev, ioasid_t pasid,
> >       struct dmar_domain *dmar_domain;
> >       unsigned long flags;
> > -    if (domain->type == IOMMU_DOMAIN_IDENTITY) {
> > -        intel_pasid_tear_down_entry(iommu, dev, pasid, 0);
> > -        return;
> > -    }
> > -
> >       dmar_domain = to_dmar_domain(domain);
> >       spin_lock_irqsave(&dmar_domain->lock, flags);
> >       list_for_each_entry(curr, &dmar_domain->dev_pasids, link_domain) {
> > @@ -4278,13 +4273,24 @@ static void 
> > intel_iommu_remove_dev_pasid(struct device *dev, ioasid_t pasid,
> >       domain_detach_iommu(dmar_domain, iommu);
> >       intel_iommu_debugfs_remove_dev_pasid(dev_pasid);
> >       kfree(dev_pasid);
> > +}
> > +
> > +static void intel_iommu_remove_dev_pasid(struct device *dev, ioasid_t 
> > pasid,
> > +                     struct iommu_domain *domain)
> > +{
> > +    struct device_domain_info *info = dev_iommu_priv_get(dev);
> > +    struct intel_iommu *iommu = info->iommu;
> > +
> >       intel_pasid_tear_down_entry(iommu, dev, pasid,
> >                       INTEL_PASID_TEARDOWN_DRAIN_PRQ);
> > +    if (domain->type == IOMMU_DOMAIN_IDENTITY)
> > +        return;
>
> The static identity domain is not capable of handling page requests.
> Therefore there is no need to drain PRQ for an identity domain removal.
>
> So it probably should be something like this:
>
>      if (domain->type == IOMMU_DOMAIN_IDENTITY) {
>          intel_pasid_tear_down_entry(iommu, dev, pasid, 0);
>          return;
>      }
>
>      intel_pasid_tear_down_entry(iommu, dev, pasid,
>                                      INTEL_PASID_TEARDOWN_DRAIN_PRQ);

Just revisited this. It seems that we just need to drain PRQ if the
attached domain is iopf-capable. Therefore, how about making it like
this?

	unsigned int flags = 0;

	if (domain->iopf_handler)
		flags |= INTEL_PASID_TEARDOWN_DRAIN_PRQ;

	intel_pasid_tear_down_entry(iommu, dev, pasid, flags);

	/* Identity domain has no meta data for pasid. */
	if (domain->type == IOMMU_DOMAIN_IDENTITY)
		return;

Thanks,
baolu