From: Baolu Lu baolu.lu@linux.intel.com Sent: Wednesday, October 9, 2024 9:09 AM
On 2024/9/30 15:19, Tian, Kevin wrote:
From: Baolu Lu baolu.lu@linux.intel.com 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.
Yes. You are right.
intel_pasid_tear_down_entry() takes care of iotlb/devtlb invalidation. So it's fine as long as intel_pasid_tear_down_entry() is called for the IDENTITY domain path, right?
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?
Existing code doesn't skip intel_pasid_tear_down_entry().
if (domain->type == IOMMU_DOMAIN_IDENTITY) { intel_pasid_tear_down_entry(iommu, dev, pasid, false); return; }Or anything I overlooked?
No. Looks I was confused by this patch and misunderstood the existing code.