Re: [PATCH v4 09/10] iommu: Make iommu_queue_iopf() more generic

[Baolu Lu]

On 8/25/23 4:17 PM, Tian, Kevin wrote:
> > +
> > +	list_for_each_entry(iopf, &iopf_param->faults, list) {
> > +		if (WARN_ON(iopf->fault.prm.pasid == pasid))
> > +			break;
> > +	}
> > +	mutex_unlock(&iopf_param->lock);
> > +}
> > +
> >   void iommu_detach_device(struct iommu_domain *domain, struct device
> > *dev)
> >   {
> >   	struct iommu_group *group;
> > @@ -1959,6 +1980,7 @@ void iommu_detach_device(struct iommu_domain
> > *domain, struct device *dev)
> >   	if (!group)
> >   		return;
> >
> > +	assert_no_pending_iopf(dev, IOMMU_NO_PASID);
> >   	mutex_lock(&group->mutex);
> >   	if (WARN_ON(domain != group->domain) ||
> >   	    WARN_ON(list_count_nodes(&group->devices) != 1))
> > @@ -3269,6 +3291,7 @@ void iommu_detach_device_pasid(struct
> > iommu_domain *domain, struct device *dev,
> >   {
> >   	struct iommu_group *group = iommu_group_get(dev);
> >
> > +	assert_no_pending_iopf(dev, pasid);
> this doesn't look correct. A sane driver will stop triggering new
> page request before calling detach but there are still pending ones
> not drained until iopf_queue_flush_dev() called by
> ops->remove_dev_pasid().
>
> then this check will cause false warning.

You are right. It is not only incorrect but also pointless. The iommu
driver should flush the iopf queues in the path of detaching domains. I
will remove it if no objection.

Best regards,
baolu