> From: Baolu Lu <baolu.lu@xxxxxxxxxxxxxxx>
> Sent: Friday, September 13, 2024 10:17 AM
>
> On 9/13/24 9:35 AM, Baolu Lu wrote:
> > On 9/12/24 9:04 PM, Yi Liu wrote:
> >> +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;
>
this is the right thing to do, but also suggesting a bug in existing
code. intel_pasid_tear_down_entry() is not just for PRQ drain.
It's also about iotlb/devtlb invalidation. From device p.o.v it
has no idea about the translation mode in the IOMMU side and
always caches the valid mappings in devtlb when ATS is enabled.
Existing code skips all those housekeeping for identify domain
by early return before intel_pasid_tear_down_entry(). We need
a separate fix for it before this series?
