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.. > + 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? > + 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? > +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? Jason _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu