On 2022/5/2 21:17, Jason Gunthorpe wrote:
On Sun, May 01, 2022 at 07:24:32PM +0800, Lu Baolu wrote:
+static bool domain_support_force_snooping(struct dmar_domain *domain)
+{
+ struct device_domain_info *info;
+ unsigned long flags;
+ bool support = true;
+
+ spin_lock_irqsave(&device_domain_lock, flags);
+ if (list_empty(&domain->devices))
+ goto out;
Why? list_for_each_entry will just do nothing..
Yes. I will remove above two lines.
+ list_for_each_entry(info, &domain->devices, link) {
+ if (!ecap_sc_support(info->iommu->ecap)) {
+ support = false;
+ break;
+ }
+ }
+out:
+ spin_unlock_irqrestore(&device_domain_lock, flags);
+ return support;
+}
+
+static void domain_set_force_snooping(struct dmar_domain *domain)
+{
+ struct device_domain_info *info;
+ unsigned long flags;
+
+ /*
+ * Second level page table supports per-PTE snoop control. The
+ * iommu_map() interface will handle this by setting SNP bit.
+ */
+ if (!domain_use_first_level(domain))
+ return;
+
+ spin_lock_irqsave(&device_domain_lock, flags);
+ if (list_empty(&domain->devices))
+ goto out_unlock;
+
+ list_for_each_entry(info, &domain->devices, link)
+ intel_pasid_setup_page_snoop_control(info->iommu, info->dev,
+ PASID_RID2PASID);
+
+out_unlock:
+ spin_unlock_irqrestore(&device_domain_lock, flags);
+}
+
static bool intel_iommu_enforce_cache_coherency(struct iommu_domain *domain)
{
struct dmar_domain *dmar_domain = to_dmar_domain(domain);
- if (!domain_update_iommu_snooping(NULL))
+ if (!domain_support_force_snooping(dmar_domain))
return false;
Maybe exit early if force_snooping = true?
Yes, should check "force_snooping = true" and return directly if
force_snooping has already been set. As you pointed below, the new
domain_attach should take care of this flag as well. Thanks!
+ domain_set_force_snooping(dmar_domain);
dmar_domain->force_snooping = true;
+
return true;
}
diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
index f8d215d85695..815c744e6a34 100644
+++ b/drivers/iommu/intel/pasid.c
@@ -762,3 +762,21 @@ int intel_pasid_setup_pass_through(struct intel_iommu
*iommu,
return 0;
}
+
+/*
+ * Set the page snoop control for a pasid entry which has been set up.
+ */
So the 'first level' is only used with pasid?
Yes. A fake pasid (RID2PASID in spec) is used for legacy transactions
(those w/o pasid).
+void intel_pasid_setup_page_snoop_control(struct intel_iommu *iommu,
+ struct device *dev, u32 pasid)
+{
+ struct pasid_entry *pte;
+ u16 did;
+
+ pte = intel_pasid_get_entry(dev, pasid);
+ if (WARN_ON(!pte || !pasid_pte_is_present(pte)))
+ return;
+
+ pasid_set_pgsnp(pte);
Doesn't this need to be done in other places too, like when a new attach
is made? Patch 5 removed it, but should that be made if
domain->force_snooping?
Yes. I missed this. Will take care of this in the next version.
Jason
Best regards,
baolu
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
