Messages in this thread | | | Date | Thu, 14 Dec 2023 21:33:46 +0800 | Subject | Re: [PATCH 8/8] iommu/vt-d: Add set_dev_pasid callback for nested domain | From | Baolu Lu <> |
| |
On 2023/12/14 10:55, Yang, Weijiang wrote: > On 11/27/2023 2:34 PM, Yi Liu wrote: >> From: Lu Baolu <baolu.lu@linux.intel.com> >> >> This allows the upper layers to set a nested type domain to a PASID of a >> device if the PASID feature is supported by the IOMMU hardware. >> >> The set_dev_pasid callback for non-nest domain has already be there, so >> this only needs to add it for nested domains. >> >> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com> >> Signed-off-by: Yi Liu <yi.l.liu@intel.com> >> --- >> drivers/iommu/intel/nested.c | 47 ++++++++++++++++++++++++++++++++++++ >> 1 file changed, 47 insertions(+) >> >> diff --git a/drivers/iommu/intel/nested.c b/drivers/iommu/intel/nested.c >> index 44ad48db7ea0..f6f687750104 100644 >> --- a/drivers/iommu/intel/nested.c >> +++ b/drivers/iommu/intel/nested.c >> @@ -68,6 +68,52 @@ static int intel_nested_attach_dev(struct >> iommu_domain *domain, >> return 0; >> } >> +static int intel_nested_set_dev_pasid(struct iommu_domain *domain, >> + struct device *dev, ioasid_t pasid) >> +{ >> + 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; >> + struct dev_pasid_info *dev_pasid; >> + unsigned long flags; >> + int ret = 0; >> + >> + if (!pasid_supported(iommu)) >> + return -EOPNOTSUPP; >> + >> + if (iommu->agaw < dmar_domain->s2_domain->agaw) >> + return -EINVAL; >> + >> + ret = >> prepare_domain_attach_device(&dmar_domain->s2_domain->domain, dev); >> + if (ret) >> + return ret; >> + >> + dev_pasid = kzalloc(sizeof(*dev_pasid), GFP_KERNEL); >> + if (!dev_pasid) >> + return -ENOMEM; >> + >> + ret = domain_attach_iommu(dmar_domain, iommu); >> + if (ret) >> + goto err_free; >> + >> + ret = intel_pasid_setup_nested(iommu, dev, pasid, dmar_domain); >> + if (ret) >> + goto err_detach_iommu; >> + >> + dev_pasid->dev = dev; >> + dev_pasid->pasid = pasid; >> + spin_lock_irqsave(&dmar_domain->lock, flags); >> + list_add(&dev_pasid->link_domain, &dmar_domain->dev_pasids); >> + spin_unlock_irqrestore(&dmar_domain->lock, flags); > > ---> list_add(&dev_pasid->link_domain, &dmar_domain->dev_pasids); > > dev_pasid is linked at later time, this leads to > domain->has_iotlb_device is not correctly set, which finally results > into a missing of device iotlb flush in iommu_flush_dev_iotlb()when it's > called. > Check this call path: > domain_attach_iommu()->domain_update_iommu_cap()->domain_update_iotlb()->domain->has_iotlb_device = has_iotlb_device; The ugly fixup is to call domain_update_iommu_cap() or domain_update_iotlb() here again before return. > The similar issue is in intel_iommu_set_dev_pasid() and > intel_nested_attach_dev().
Yes, domain->has_iotlb_device must be updated whenever a domain is attached to (or removed from) a RID or PASID. I would be grateful if you could post some patches to fix the set_device_pasid and nested_attach_dev paths.
I assume Yi can fix this series in the next version.
Best regards, baolu
| |