Message ID | 20230724111335.107427-7-yi.l.liu@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | Add Intel VT-d nested translation | expand |
> From: Liu, Yi L <yi.l.liu@intel.com> > Sent: Monday, July 24, 2023 7:13 PM > + > +static int intel_nested_attach_dev(struct iommu_domain *domain, > + struct device *dev) > +{ > + struct device_domain_info *info = dev_iommu_priv_get(dev); > + struct dmar_domain *dmar_domain = to_dmar_domain(domain); > + struct intel_iommu *iommu = info->iommu; > + unsigned long flags; > + int ret = 0; > + > + if (info->domain) > + device_block_translation(dev); > + > + if (iommu->agaw < dmar_domain->s2_domain->agaw) { > + dev_err_ratelimited(dev, "Adjusted guest address width not > compatible\n"); > + return -ENODEV; > + } this is the check duplicated with patch04. > + > + ret = domain_attach_iommu(dmar_domain, iommu); > + if (ret) { > + dev_err_ratelimited(dev, "Failed to attach domain to > iommu\n"); > + return ret; > + } > + [...] > + domain_update_iommu_cap(dmar_domain); iommu_cap is already updated in domain_attach_iommu(). > > static const struct iommu_domain_ops intel_nested_domain_ops = { > + .attach_dev = intel_nested_attach_dev, > .free = intel_nested_domain_free, > + .enforce_cache_coherency = intel_iommu_enforce_cache_coherency, this is not required. enforce_cache_coherency() will be called on parent hwpt when it's being created. patch04 should check parent's force_snooping to set pgsnp in the pasid entry. As Jason explained it should be done only for kernel owned page table.
On 2023/8/2 15:22, Tian, Kevin wrote: >> From: Liu, Yi L <yi.l.liu@intel.com> >> Sent: Monday, July 24, 2023 7:13 PM >> + >> +static int intel_nested_attach_dev(struct iommu_domain *domain, >> + struct device *dev) >> +{ >> + struct device_domain_info *info = dev_iommu_priv_get(dev); >> + struct dmar_domain *dmar_domain = to_dmar_domain(domain); >> + struct intel_iommu *iommu = info->iommu; >> + unsigned long flags; >> + int ret = 0; >> + >> + if (info->domain) >> + device_block_translation(dev); >> + >> + if (iommu->agaw < dmar_domain->s2_domain->agaw) { >> + dev_err_ratelimited(dev, "Adjusted guest address width not >> compatible\n"); >> + return -ENODEV; >> + } > > this is the check duplicated with patch04. Ack. No need for a check here. > >> + >> + ret = domain_attach_iommu(dmar_domain, iommu); >> + if (ret) { >> + dev_err_ratelimited(dev, "Failed to attach domain to >> iommu\n"); >> + return ret; >> + } >> + > > [...] > >> + domain_update_iommu_cap(dmar_domain); > > iommu_cap is already updated in domain_attach_iommu(). Ack. We will eventually remove this helper when every domain is allocated for a specific device. >> >> static const struct iommu_domain_ops intel_nested_domain_ops = { >> + .attach_dev = intel_nested_attach_dev, >> .free = intel_nested_domain_free, >> + .enforce_cache_coherency = intel_iommu_enforce_cache_coherency, > > this is not required. enforce_cache_coherency() will be called on parent > hwpt when it's being created. patch04 should check parent's force_snooping > to set pgsnp in the pasid entry. > > As Jason explained it should be done only for kernel owned page table. Ack and thanks! Best regards, baolu
diff --git a/drivers/iommu/intel/nested.c b/drivers/iommu/intel/nested.c index 80a64ba87d46..98164894f22f 100644 --- a/drivers/iommu/intel/nested.c +++ b/drivers/iommu/intel/nested.c @@ -11,8 +11,58 @@ #define pr_fmt(fmt) "DMAR: " fmt #include <linux/iommu.h> +#include <linux/pci.h> +#include <linux/pci-ats.h> #include "iommu.h" +#include "pasid.h" + +static int intel_nested_attach_dev(struct iommu_domain *domain, + struct device *dev) +{ + struct device_domain_info *info = dev_iommu_priv_get(dev); + struct dmar_domain *dmar_domain = to_dmar_domain(domain); + struct intel_iommu *iommu = info->iommu; + unsigned long flags; + int ret = 0; + + if (info->domain) + device_block_translation(dev); + + if (iommu->agaw < dmar_domain->s2_domain->agaw) { + dev_err_ratelimited(dev, "Adjusted guest address width not compatible\n"); + return -ENODEV; + } + + /* Is s2_domain compatible with this IOMMU? */ + ret = prepare_domain_attach_device(&dmar_domain->s2_domain->domain, dev); + if (ret) { + dev_err_ratelimited(dev, "s2 domain is not compatible\n"); + return ret; + } + + ret = domain_attach_iommu(dmar_domain, iommu); + if (ret) { + dev_err_ratelimited(dev, "Failed to attach domain to iommu\n"); + return ret; + } + + ret = intel_pasid_setup_nested(iommu, dev, + PASID_RID2PASID, dmar_domain); + if (ret) { + domain_detach_iommu(dmar_domain, iommu); + dev_err_ratelimited(dev, "Failed to setup pasid entry\n"); + return ret; + } + + info->domain = dmar_domain; + spin_lock_irqsave(&dmar_domain->lock, flags); + list_add(&info->link, &dmar_domain->devices); + spin_unlock_irqrestore(&dmar_domain->lock, flags); + domain_update_iommu_cap(dmar_domain); + + return 0; +} static void intel_nested_domain_free(struct iommu_domain *domain) { @@ -20,7 +70,9 @@ static void intel_nested_domain_free(struct iommu_domain *domain) } static const struct iommu_domain_ops intel_nested_domain_ops = { + .attach_dev = intel_nested_attach_dev, .free = intel_nested_domain_free, + .enforce_cache_coherency = intel_iommu_enforce_cache_coherency, }; struct iommu_domain *intel_nested_domain_alloc(struct iommu_domain *s2_domain,