Re: [PATCH 1/3] iommu/vt-d: Move intel_iommu info from struct intel_svm to struct intel_svm_dev

Re: [PATCH 1/3] iommu/vt-d: Move intel_iommu info from struct intel_svm to struct intel_svm_dev

[Lu Baolu]

Hi,

On 2020/12/20 8:03, Liu Yi L wrote:
> Current struct intel_svm has a field to record the struct intel_iommu
> pointer for a PASID bind. And struct intel_svm will be shared by all
> the devices bind to the same process. The devices may be behind different
> DMAR units. As the iommu driver code uses the intel_iommu pointer stored
> in intel_svm struct to do cache invalidations, it may only flush the cache
> on a single DMAR unit, for others, the cache invalidation is missed.
> 
> As intel_svm struct already has a device list, this patch just moves the
> intel_iommu pointer to be a field of intel_svm_dev struct.
> 
> Fixes: 2f26e0a9c986 ("iommu/vt-d: Add basic SVM PASID support")
> Cc: Lu Baolu <baolu.lu@linux.intel.com>
> Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
> Cc: Raj Ashok <ashok.raj@intel.com>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Reported-by: Guo Kaijie <Kaijie.Guo@intel.com>
> Reported-by: Xin Zeng <xin.zeng@intel.com>

Kaijie or Xin, can you please confirm whether this fix work for you?

Best regards,
baolu

> Signed-off-by: Guo Kaijie <Kaijie.Guo@intel.com>
> Signed-off-by: Xin Zeng <xin.zeng@intel.com>
> Signed-off-by: Liu Yi L <yi.l.liu@intel.com>
> ---
>   drivers/iommu/intel/svm.c   | 9 +++++----
>   include/linux/intel-iommu.h | 2 +-
>   2 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
> index 3242ebd0bca3..4a10c9ff368c 100644
> --- a/drivers/iommu/intel/svm.c
> +++ b/drivers/iommu/intel/svm.c
> @@ -142,7 +142,7 @@ static void intel_flush_svm_range_dev (struct intel_svm *svm, struct intel_svm_d
>   	}
>   	desc.qw2 = 0;
>   	desc.qw3 = 0;
> -	qi_submit_sync(svm->iommu, &desc, 1, 0);
> +	qi_submit_sync(sdev->iommu, &desc, 1, 0);
>   
>   	if (sdev->dev_iotlb) {
>   		desc.qw0 = QI_DEV_EIOTLB_PASID(svm->pasid) |
> @@ -166,7 +166,7 @@ static void intel_flush_svm_range_dev (struct intel_svm *svm, struct intel_svm_d
>   		}
>   		desc.qw2 = 0;
>   		desc.qw3 = 0;
> -		qi_submit_sync(svm->iommu, &desc, 1, 0);
> +		qi_submit_sync(sdev->iommu, &desc, 1, 0);
>   	}
>   }
>   
> @@ -211,7 +211,7 @@ static void intel_mm_release(struct mmu_notifier *mn, struct mm_struct *mm)
>   	 */
>   	rcu_read_lock();
>   	list_for_each_entry_rcu(sdev, &svm->devs, list)
> -		intel_pasid_tear_down_entry(svm->iommu, sdev->dev,
> +		intel_pasid_tear_down_entry(sdev->iommu, sdev->dev,
>   					    svm->pasid, true);
>   	rcu_read_unlock();
>   
> @@ -363,6 +363,7 @@ int intel_svm_bind_gpasid(struct iommu_domain *domain, struct device *dev,
>   	}
>   	sdev->dev = dev;
>   	sdev->sid = PCI_DEVID(info->bus, info->devfn);
> +	sdev->iommu = iommu;
>   
>   	/* Only count users if device has aux domains */
>   	if (iommu_dev_feature_enabled(dev, IOMMU_DEV_FEAT_AUX))
> @@ -546,6 +547,7 @@ intel_svm_bind_mm(struct device *dev, unsigned int flags,
>   		goto out;
>   	}
>   	sdev->dev = dev;
> +	sdev->iommu = iommu;
>   
>   	ret = intel_iommu_enable_pasid(iommu, dev);
>   	if (ret) {
> @@ -575,7 +577,6 @@ intel_svm_bind_mm(struct device *dev, unsigned int flags,
>   			kfree(sdev);
>   			goto out;
>   		}
> -		svm->iommu = iommu;
>   
>   		if (pasid_max > intel_pasid_max_id)
>   			pasid_max = intel_pasid_max_id;
> diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
> index d956987ed032..94522685a0d9 100644
> --- a/include/linux/intel-iommu.h
> +++ b/include/linux/intel-iommu.h
> @@ -758,6 +758,7 @@ struct intel_svm_dev {
>   	struct list_head list;
>   	struct rcu_head rcu;
>   	struct device *dev;
> +	struct intel_iommu *iommu;
>   	struct svm_dev_ops *ops;
>   	struct iommu_sva sva;
>   	u32 pasid;
> @@ -771,7 +772,6 @@ struct intel_svm {
>   	struct mmu_notifier notifier;
>   	struct mm_struct *mm;
>   
> -	struct intel_iommu *iommu;
>   	unsigned int flags;
>   	u32 pasid;
>   	int gpasid; /* In case that guest PASID is different from host PASID */
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 2/3] iommu/vt-d: Track device aux-attach with subdevice_domain_info.

[Lu Baolu]

Hi,

On 2020/12/20 8:03, Liu Yi L wrote:
> In existing code, if wanting to loop all devices attached to a domain,
> current code can only loop the devices which are attached to the domain
> via normal manner. While for devices attached via auxiliary manner, this
> is subdevice, they are not tracked in the domain. This patch adds struct
> subdevice_domain_info which is created per domain attachment via auxiliary
> manner. So that such devices are also tracked in domain.
> 
> This was found by when I'm working on the belwo patch, There is no device
> in domain->devices, thus unable to get the cap and ecap of iommu unit. But
> this domain actually has one sub-device which is attached via aux-manner.
> This patch fixes the issue.
> 
> https://lore.kernel.org/kvm/1599734733-6431-17-git-send-email-yi.l.liu@intel.com/
> 
> But looks like, it doesn't affect me only. Such auxiliary track should be
> there for example if wanting to flush device_iotlb for a domain which has
> devices attached by auxiliray manner, then this fix is also necessary. This
> issue will also be fixed by another patch in this series with some additional
> changes based on the sudevice tracking framework introduced in this patch.
> 
> Co-developed-by: Xin Zeng <xin.zeng@intel.com>
> Signed-off-by: Xin Zeng <xin.zeng@intel.com>
> Co-developed-by: Liu Yi L <yi.l.liu@intel.com>
> Signed-off-by: Liu Yi L <yi.l.liu@intel.com>
> Co-developed-by: Lu Baolu <baolu.lu@linux.intel.com>
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>

Please remove my Signed-off-by. I need to review and test it.

Best regards,
baolu

> ---
>   drivers/iommu/intel/iommu.c | 92 ++++++++++++++++++++++++++++++++-----
>   include/linux/intel-iommu.h | 11 ++++-
>   2 files changed, 90 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index a49afa11673c..4274b4acc325 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -1881,6 +1881,7 @@ static struct dmar_domain *alloc_domain(int flags)
>   		domain->flags |= DOMAIN_FLAG_USE_FIRST_LEVEL;
>   	domain->has_iotlb_device = false;
>   	INIT_LIST_HEAD(&domain->devices);
> +	INIT_LIST_HEAD(&domain->sub_devices);
>   
>   	return domain;
>   }
> @@ -5172,33 +5173,79 @@ is_aux_domain(struct device *dev, struct iommu_domain *domain)
>   			domain->type == IOMMU_DOMAIN_UNMANAGED;
>   }
>   
> +static inline
> +void _auxiliary_link_device(struct dmar_domain *domain,
> +			    struct subdevice_domain_info *subinfo,
> +			    struct device *dev)
> +{
> +	subinfo->users++;
> +}
> +
> +static inline
> +int _auxiliary_unlink_device(struct dmar_domain *domain,
> +			     struct subdevice_domain_info *subinfo,
> +			     struct device *dev)
> +{
> +	subinfo->users--;
> +	return subinfo->users;
> +}
> +
>   static void auxiliary_link_device(struct dmar_domain *domain,
>   				  struct device *dev)
>   {
>   	struct device_domain_info *info = get_domain_info(dev);
> +	struct subdevice_domain_info *subinfo;
>   
>   	assert_spin_locked(&device_domain_lock);
>   	if (WARN_ON(!info))
>   		return;
>   
> +	subinfo = kzalloc(sizeof(*subinfo), GFP_ATOMIC);
> +	if (!subinfo)
> +		return;
> +
> +	subinfo->domain = domain;
> +	subinfo->dev = dev;
> +	list_add(&subinfo->link_domain, &info->auxiliary_domains);
> +	list_add(&subinfo->link_phys, &domain->sub_devices);
> +	_auxiliary_link_device(domain, subinfo, dev);
>   	domain->auxd_refcnt++;
> -	list_add(&domain->auxd, &info->auxiliary_domains);
>   }
>   
> -static void auxiliary_unlink_device(struct dmar_domain *domain,
> -				    struct device *dev)
> +static struct subdevice_domain_info *
> +subdevice_domain_info_lookup(struct dmar_domain *domain, struct device *dev)
> +{
> +	struct subdevice_domain_info *subinfo;
> +
> +	assert_spin_locked(&device_domain_lock);
> +
> +	list_for_each_entry(subinfo, &domain->sub_devices, link_phys)
> +		if (subinfo->dev == dev)
> +			return subinfo;
> +
> +	return NULL;
> +}
> +
> +static int auxiliary_unlink_device(struct dmar_domain *domain,
> +				   struct subdevice_domain_info *subinfo,
> +				   struct device *dev)
>   {
>   	struct device_domain_info *info = get_domain_info(dev);
> +	int ret;
>   
>   	assert_spin_locked(&device_domain_lock);
>   	if (WARN_ON(!info))
> -		return;
> +		return -EINVAL;
>   
> -	list_del(&domain->auxd);
> +	ret = _auxiliary_unlink_device(domain, subinfo, dev);
> +	if (ret == 0) {
> +		list_del(&subinfo->link_domain);
> +		list_del(&subinfo->link_phys);
> +		kfree(subinfo);
> +	}
>   	domain->auxd_refcnt--;
>   
> -	if (!domain->auxd_refcnt && domain->default_pasid > 0)
> -		ioasid_free(domain->default_pasid);
> +	return ret;
>   }
>   
>   static int aux_domain_add_dev(struct dmar_domain *domain,
> @@ -5207,6 +5254,8 @@ static int aux_domain_add_dev(struct dmar_domain *domain,
>   	int ret;
>   	unsigned long flags;
>   	struct intel_iommu *iommu;
> +	struct device_domain_info *info = get_domain_info(dev);
> +	struct subdevice_domain_info *subinfo;
>   
>   	iommu = device_to_iommu(dev, NULL, NULL);
>   	if (!iommu)
> @@ -5227,6 +5276,12 @@ static int aux_domain_add_dev(struct dmar_domain *domain,
>   	}
>   
>   	spin_lock_irqsave(&device_domain_lock, flags);
> +	subinfo = subdevice_domain_info_lookup(domain, dev);
> +	if (subinfo) {
> +		_auxiliary_link_device(domain, subinfo, dev);
> +		spin_unlock_irqrestore(&device_domain_lock, flags);
> +		return 0;
> +	}
>   	/*
>   	 * iommu->lock must be held to attach domain to iommu and setup the
>   	 * pasid entry for second level translation.
> @@ -5270,6 +5325,7 @@ static void aux_domain_remove_dev(struct dmar_domain *domain,
>   	struct device_domain_info *info;
>   	struct intel_iommu *iommu;
>   	unsigned long flags;
> +	struct subdevice_domain_info *subinfo;
>   
>   	if (!is_aux_domain(dev, &domain->domain))
>   		return;
> @@ -5278,14 +5334,26 @@ static void aux_domain_remove_dev(struct dmar_domain *domain,
>   	info = get_domain_info(dev);
>   	iommu = info->iommu;
>   
> -	auxiliary_unlink_device(domain, dev);
> +	subinfo = subdevice_domain_info_lookup(domain, dev);
> +	if (!subinfo) {
> +		spin_unlock_irqrestore(&device_domain_lock, flags);
> +		return;
> +	}
>   
> -	spin_lock(&iommu->lock);
> -	intel_pasid_tear_down_entry(iommu, dev, domain->default_pasid, false);
> -	domain_detach_iommu(domain, iommu);
> -	spin_unlock(&iommu->lock);
> +	if (auxiliary_unlink_device(domain, subinfo, dev) == 0) {
> +		spin_lock(&iommu->lock);
> +		intel_pasid_tear_down_entry(iommu,
> +					    dev,
> +					    domain->default_pasid,
> +					    false);
> +		domain_detach_iommu(domain, iommu);
> +		spin_unlock(&iommu->lock);
> +	}
>   
>   	spin_unlock_irqrestore(&device_domain_lock, flags);
> +
> +	if (!domain->auxd_refcnt && domain->default_pasid > 0)
> +		ioasid_free(domain->default_pasid);
>   }
>   
>   static int prepare_domain_attach_device(struct iommu_domain *domain,
> diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
> index 94522685a0d9..1fb3d6ab719a 100644
> --- a/include/linux/intel-iommu.h
> +++ b/include/linux/intel-iommu.h
> @@ -537,7 +537,7 @@ struct dmar_domain {
>   
>   	bool has_iotlb_device;
>   	struct list_head devices;	/* all devices' list */
> -	struct list_head auxd;		/* link to device's auxiliary list */
> +	struct list_head sub_devices;	/* all devices' list attached via aux-attach */
>   	struct iova_domain iovad;	/* iova's that belong to this domain */
>   
>   	struct dma_pte	*pgd;		/* virtual address */
> @@ -636,6 +636,15 @@ struct device_domain_info {
>   	struct pasid_table *pasid_table; /* pasid table */
>   };
>   
> +/* Aux attach device domain info */
> +struct subdevice_domain_info {
> +	struct device *dev;
> +	struct dmar_domain *domain;
> +	struct list_head link_phys;	/* link to phys device siblings */
> +	struct list_head link_domain;	/* link to domain siblings */
> +	int users;
> +};
> +
>   static inline void __iommu_flush_cache(
>   	struct intel_iommu *iommu, void *addr, int size)
>   {
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 3/3] iommu/vt-d: A fix to iommu_flush_dev_iotlb() for aux-domain

[Lu Baolu]

Hi,

On 2020/12/20 8:03, Liu Yi L wrote:
> iommu_flush_dev_iotlb() is called to invalidate caches on device. It only
> loops the devices which are full-attached to the domain. For sub-devices,
> this is ineffective. This results in invalid caching entries left on the
> device. Fix it by adding loop for subdevices as well. Also, update the
> domain->has_iotlb_device for both device/subdevice attach/detach and
> ATS enabling/disabling.
> 
> Signed-off-by: Liu Yi L <yi.l.liu@intel.com>
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>

The same here.

Best regards,
baolu

> ---
>   drivers/iommu/intel/iommu.c | 90 +++++++++++++++++++++++++++++--------
>   1 file changed, 72 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index 4274b4acc325..d9b6037b72b1 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -1437,6 +1437,10 @@ static void __iommu_flush_iotlb(struct intel_iommu *iommu, u16 did,
>   			(unsigned long long)DMA_TLB_IAIG(val));
>   }
>   
> +/**
> + * For a given bus/devfn, fetch its device_domain_info if it supports
> + * device tlb. Only needs to loop devices attached in normal manner.
> + */
>   static struct device_domain_info *
>   iommu_support_dev_iotlb (struct dmar_domain *domain, struct intel_iommu *iommu,
>   			 u8 bus, u8 devfn)
> @@ -1459,6 +1463,18 @@ iommu_support_dev_iotlb (struct dmar_domain *domain, struct intel_iommu *iommu,
>   	return NULL;
>   }
>   
> +static bool dev_iotlb_enabled(struct device *dev)
> +{
> +	struct pci_dev *pdev;
> +
> +	if (!dev || !dev_is_pci(dev))
> +		return false;
> +
> +	pdev = to_pci_dev(dev);
> +
> +	return !!pdev->ats_enabled;
> +}
> +
>   static void domain_update_iotlb(struct dmar_domain *domain)
>   {
>   	struct device_domain_info *info;
> @@ -1467,21 +1483,37 @@ static void domain_update_iotlb(struct dmar_domain *domain)
>   	assert_spin_locked(&device_domain_lock);
>   
>   	list_for_each_entry(info, &domain->devices, link) {
> -		struct pci_dev *pdev;
> -
> -		if (!info->dev || !dev_is_pci(info->dev))
> -			continue;
> -
> -		pdev = to_pci_dev(info->dev);
> -		if (pdev->ats_enabled) {
> +		if (dev_iotlb_enabled(info->dev)) {
>   			has_iotlb_device = true;
>   			break;
>   		}
>   	}
>   
> +	if (!has_iotlb_device) {
> +		struct subdevice_domain_info *subinfo;
> +
> +		list_for_each_entry(subinfo, &domain->sub_devices, link_phys) {
> +			if (dev_iotlb_enabled(subinfo->dev)) {
> +				has_iotlb_device = true;
> +				break;
> +			}
> +		}
> +	}
>   	domain->has_iotlb_device = has_iotlb_device;
>   }
>   
> +static void dev_update_domain_iotlb(struct device_domain_info *info)
> +{
> +	struct subdevice_domain_info *subinfo;
> +
> +	assert_spin_locked(&device_domain_lock);
> +
> +	domain_update_iotlb(info->domain);
> +
> +	list_for_each_entry(subinfo, &info->auxiliary_domains, link_domain)
> +		domain_update_iotlb(subinfo->domain);
> +}
> +
>   static void iommu_enable_dev_iotlb(struct device_domain_info *info)
>   {
>   	struct pci_dev *pdev;
> @@ -1524,7 +1556,7 @@ static void iommu_enable_dev_iotlb(struct device_domain_info *info)
>   	if (info->ats_supported && pci_ats_page_aligned(pdev) &&
>   	    !pci_enable_ats(pdev, VTD_PAGE_SHIFT)) {
>   		info->ats_enabled = 1;
> -		domain_update_iotlb(info->domain);
> +		dev_update_domain_iotlb(info);
>   		info->ats_qdep = pci_ats_queue_depth(pdev);
>   	}
>   }
> @@ -1543,7 +1575,7 @@ static void iommu_disable_dev_iotlb(struct device_domain_info *info)
>   	if (info->ats_enabled) {
>   		pci_disable_ats(pdev);
>   		info->ats_enabled = 0;
> -		domain_update_iotlb(info->domain);
> +		dev_update_domain_iotlb(info);
>   	}
>   #ifdef CONFIG_INTEL_IOMMU_SVM
>   	if (info->pri_enabled) {
> @@ -1557,26 +1589,43 @@ static void iommu_disable_dev_iotlb(struct device_domain_info *info)
>   #endif
>   }
>   
> +static void __iommu_flush_dev_iotlb(struct device_domain_info *info,
> +				    u64 addr, unsigned mask)
> +{
> +	u16 sid, qdep;
> +
> +	if (!info || !info->ats_enabled)
> +		return;
> +
> +	sid = info->bus << 8 | info->devfn;
> +	qdep = info->ats_qdep;
> +	qi_flush_dev_iotlb(info->iommu, sid, info->pfsid,
> +			   qdep, addr, mask);
> +}
> +
>   static void iommu_flush_dev_iotlb(struct dmar_domain *domain,
>   				  u64 addr, unsigned mask)
>   {
> -	u16 sid, qdep;
>   	unsigned long flags;
>   	struct device_domain_info *info;
> +	struct subdevice_domain_info *subinfo;
>   
>   	if (!domain->has_iotlb_device)
>   		return;
>   
>   	spin_lock_irqsave(&device_domain_lock, flags);
> -	list_for_each_entry(info, &domain->devices, link) {
> -		if (!info->ats_enabled)
> -			continue;
> +	list_for_each_entry(info, &domain->devices, link)
> +		__iommu_flush_dev_iotlb(info, addr, mask);
> +
> +	/*
> +	 * Besides looping all devices attached normally, also
> +	 * needs to loop all devices attached via auxiliary
> +	 * manner.
> +	 */
> +	list_for_each_entry(subinfo, &domain->sub_devices, link_phys)
> +		__iommu_flush_dev_iotlb(get_domain_info(subinfo->dev),
> +					addr, mask);
>   
> -		sid = info->bus << 8 | info->devfn;
> -		qdep = info->ats_qdep;
> -		qi_flush_dev_iotlb(info->iommu, sid, info->pfsid,
> -				qdep, addr, mask);
> -	}
>   	spin_unlock_irqrestore(&device_domain_lock, flags);
>   }
>   
> @@ -5208,6 +5257,9 @@ static void auxiliary_link_device(struct dmar_domain *domain,
>   	subinfo->dev = dev;
>   	list_add(&subinfo->link_domain, &info->auxiliary_domains);
>   	list_add(&subinfo->link_phys, &domain->sub_devices);
> +	if (dev_iotlb_enabled(dev))
> +		domain_update_iotlb(domain);
> +
>   	_auxiliary_link_device(domain, subinfo, dev);
>   	domain->auxd_refcnt++;
>   }
> @@ -5242,6 +5294,8 @@ static int auxiliary_unlink_device(struct dmar_domain *domain,
>   		list_del(&subinfo->link_domain);
>   		list_del(&subinfo->link_phys);
>   		kfree(subinfo);
> +		if (domain->has_iotlb_device)
> +			domain_update_iotlb(domain);
>   	}
>   	domain->auxd_refcnt--;
>   
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH 0/4] iommu/vtd-: Misc fixes on scalable mode

[Liu Yi L]

Hi,

This patchset aims to fix a bug regards to SVM usage on native, and
also several bugs around subdevice (attached to device via auxiliary
manner) tracking and ineffective device_tlb flush.

Regards,
Yi Liu

Liu Yi L (3):
  iommu/vt-d: Move intel_iommu info from struct intel_svm to struct
    intel_svm_dev
  iommu/vt-d: Track device aux-attach with subdevice_domain_info.
  iommu/vt-d: A fix to iommu_flush_dev_iotlb() for aux-domain

 drivers/iommu/intel/iommu.c | 182 ++++++++++++++++++++++++++++++------
 drivers/iommu/intel/svm.c   |   9 +-
 include/linux/intel-iommu.h |  13 ++-
 3 files changed, 168 insertions(+), 36 deletions(-)

-- 
2.25.1

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH 1/3] iommu/vt-d: Move intel_iommu info from struct intel_svm to struct intel_svm_dev

[Liu Yi L]

Current struct intel_svm has a field to record the struct intel_iommu
pointer for a PASID bind. And struct intel_svm will be shared by all
the devices bind to the same process. The devices may be behind different
DMAR units. As the iommu driver code uses the intel_iommu pointer stored
in intel_svm struct to do cache invalidations, it may only flush the cache
on a single DMAR unit, for others, the cache invalidation is missed.

As intel_svm struct already has a device list, this patch just moves the
intel_iommu pointer to be a field of intel_svm_dev struct.

Fixes: 2f26e0a9c986 ("iommu/vt-d: Add basic SVM PASID support")
Cc: Lu Baolu <baolu.lu@linux.intel.com>
Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
Cc: Raj Ashok <ashok.raj@intel.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Reported-by: Guo Kaijie <Kaijie.Guo@intel.com>
Reported-by: Xin Zeng <xin.zeng@intel.com>
Signed-off-by: Guo Kaijie <Kaijie.Guo@intel.com>
Signed-off-by: Xin Zeng <xin.zeng@intel.com>
Signed-off-by: Liu Yi L <yi.l.liu@intel.com>
---
 drivers/iommu/intel/svm.c   | 9 +++++----
 include/linux/intel-iommu.h | 2 +-
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
index 3242ebd0bca3..4a10c9ff368c 100644
--- a/drivers/iommu/intel/svm.c
+++ b/drivers/iommu/intel/svm.c
@@ -142,7 +142,7 @@ static void intel_flush_svm_range_dev (struct intel_svm *svm, struct intel_svm_d
 	}
 	desc.qw2 = 0;
 	desc.qw3 = 0;
-	qi_submit_sync(svm->iommu, &desc, 1, 0);
+	qi_submit_sync(sdev->iommu, &desc, 1, 0);
 
 	if (sdev->dev_iotlb) {
 		desc.qw0 = QI_DEV_EIOTLB_PASID(svm->pasid) |
@@ -166,7 +166,7 @@ static void intel_flush_svm_range_dev (struct intel_svm *svm, struct intel_svm_d
 		}
 		desc.qw2 = 0;
 		desc.qw3 = 0;
-		qi_submit_sync(svm->iommu, &desc, 1, 0);
+		qi_submit_sync(sdev->iommu, &desc, 1, 0);
 	}
 }
 
@@ -211,7 +211,7 @@ static void intel_mm_release(struct mmu_notifier *mn, struct mm_struct *mm)
 	 */
 	rcu_read_lock();
 	list_for_each_entry_rcu(sdev, &svm->devs, list)
-		intel_pasid_tear_down_entry(svm->iommu, sdev->dev,
+		intel_pasid_tear_down_entry(sdev->iommu, sdev->dev,
 					    svm->pasid, true);
 	rcu_read_unlock();
 
@@ -363,6 +363,7 @@ int intel_svm_bind_gpasid(struct iommu_domain *domain, struct device *dev,
 	}
 	sdev->dev = dev;
 	sdev->sid = PCI_DEVID(info->bus, info->devfn);
+	sdev->iommu = iommu;
 
 	/* Only count users if device has aux domains */
 	if (iommu_dev_feature_enabled(dev, IOMMU_DEV_FEAT_AUX))
@@ -546,6 +547,7 @@ intel_svm_bind_mm(struct device *dev, unsigned int flags,
 		goto out;
 	}
 	sdev->dev = dev;
+	sdev->iommu = iommu;
 
 	ret = intel_iommu_enable_pasid(iommu, dev);
 	if (ret) {
@@ -575,7 +577,6 @@ intel_svm_bind_mm(struct device *dev, unsigned int flags,
 			kfree(sdev);
 			goto out;
 		}
-		svm->iommu = iommu;
 
 		if (pasid_max > intel_pasid_max_id)
 			pasid_max = intel_pasid_max_id;
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index d956987ed032..94522685a0d9 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -758,6 +758,7 @@ struct intel_svm_dev {
 	struct list_head list;
 	struct rcu_head rcu;
 	struct device *dev;
+	struct intel_iommu *iommu;
 	struct svm_dev_ops *ops;
 	struct iommu_sva sva;
 	u32 pasid;
@@ -771,7 +772,6 @@ struct intel_svm {
 	struct mmu_notifier notifier;
 	struct mm_struct *mm;
 
-	struct intel_iommu *iommu;
 	unsigned int flags;
 	u32 pasid;
 	int gpasid; /* In case that guest PASID is different from host PASID */
-- 
2.25.1

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH 2/3] iommu/vt-d: Track device aux-attach with subdevice_domain_info.

[Liu Yi L]

In existing code, if wanting to loop all devices attached to a domain,
current code can only loop the devices which are attached to the domain
via normal manner. While for devices attached via auxiliary manner, this
is subdevice, they are not tracked in the domain. This patch adds struct
subdevice_domain_info which is created per domain attachment via auxiliary
manner. So that such devices are also tracked in domain.

This was found by when I'm working on the belwo patch, There is no device
in domain->devices, thus unable to get the cap and ecap of iommu unit. But
this domain actually has one sub-device which is attached via aux-manner.
This patch fixes the issue.

https://lore.kernel.org/kvm/1599734733-6431-17-git-send-email-yi.l.liu@intel.com/

But looks like, it doesn't affect me only. Such auxiliary track should be
there for example if wanting to flush device_iotlb for a domain which has
devices attached by auxiliray manner, then this fix is also necessary. This
issue will also be fixed by another patch in this series with some additional
changes based on the sudevice tracking framework introduced in this patch.

Co-developed-by: Xin Zeng <xin.zeng@intel.com>
Signed-off-by: Xin Zeng <xin.zeng@intel.com>
Co-developed-by: Liu Yi L <yi.l.liu@intel.com>
Signed-off-by: Liu Yi L <yi.l.liu@intel.com>
Co-developed-by: Lu Baolu <baolu.lu@linux.intel.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/intel/iommu.c | 92 ++++++++++++++++++++++++++++++++-----
 include/linux/intel-iommu.h | 11 ++++-
 2 files changed, 90 insertions(+), 13 deletions(-)

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index a49afa11673c..4274b4acc325 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -1881,6 +1881,7 @@ static struct dmar_domain *alloc_domain(int flags)
 		domain->flags |= DOMAIN_FLAG_USE_FIRST_LEVEL;
 	domain->has_iotlb_device = false;
 	INIT_LIST_HEAD(&domain->devices);
+	INIT_LIST_HEAD(&domain->sub_devices);
 
 	return domain;
 }
@@ -5172,33 +5173,79 @@ is_aux_domain(struct device *dev, struct iommu_domain *domain)
 			domain->type == IOMMU_DOMAIN_UNMANAGED;
 }
 
+static inline
+void _auxiliary_link_device(struct dmar_domain *domain,
+			    struct subdevice_domain_info *subinfo,
+			    struct device *dev)
+{
+	subinfo->users++;
+}
+
+static inline
+int _auxiliary_unlink_device(struct dmar_domain *domain,
+			     struct subdevice_domain_info *subinfo,
+			     struct device *dev)
+{
+	subinfo->users--;
+	return subinfo->users;
+}
+
 static void auxiliary_link_device(struct dmar_domain *domain,
 				  struct device *dev)
 {
 	struct device_domain_info *info = get_domain_info(dev);
+	struct subdevice_domain_info *subinfo;
 
 	assert_spin_locked(&device_domain_lock);
 	if (WARN_ON(!info))
 		return;
 
+	subinfo = kzalloc(sizeof(*subinfo), GFP_ATOMIC);
+	if (!subinfo)
+		return;
+
+	subinfo->domain = domain;
+	subinfo->dev = dev;
+	list_add(&subinfo->link_domain, &info->auxiliary_domains);
+	list_add(&subinfo->link_phys, &domain->sub_devices);
+	_auxiliary_link_device(domain, subinfo, dev);
 	domain->auxd_refcnt++;
-	list_add(&domain->auxd, &info->auxiliary_domains);
 }
 
-static void auxiliary_unlink_device(struct dmar_domain *domain,
-				    struct device *dev)
+static struct subdevice_domain_info *
+subdevice_domain_info_lookup(struct dmar_domain *domain, struct device *dev)
+{
+	struct subdevice_domain_info *subinfo;
+
+	assert_spin_locked(&device_domain_lock);
+
+	list_for_each_entry(subinfo, &domain->sub_devices, link_phys)
+		if (subinfo->dev == dev)
+			return subinfo;
+
+	return NULL;
+}
+
+static int auxiliary_unlink_device(struct dmar_domain *domain,
+				   struct subdevice_domain_info *subinfo,
+				   struct device *dev)
 {
 	struct device_domain_info *info = get_domain_info(dev);
+	int ret;
 
 	assert_spin_locked(&device_domain_lock);
 	if (WARN_ON(!info))
-		return;
+		return -EINVAL;
 
-	list_del(&domain->auxd);
+	ret = _auxiliary_unlink_device(domain, subinfo, dev);
+	if (ret == 0) {
+		list_del(&subinfo->link_domain);
+		list_del(&subinfo->link_phys);
+		kfree(subinfo);
+	}
 	domain->auxd_refcnt--;
 
-	if (!domain->auxd_refcnt && domain->default_pasid > 0)
-		ioasid_free(domain->default_pasid);
+	return ret;
 }
 
 static int aux_domain_add_dev(struct dmar_domain *domain,
@@ -5207,6 +5254,8 @@ static int aux_domain_add_dev(struct dmar_domain *domain,
 	int ret;
 	unsigned long flags;
 	struct intel_iommu *iommu;
+	struct device_domain_info *info = get_domain_info(dev);
+	struct subdevice_domain_info *subinfo;
 
 	iommu = device_to_iommu(dev, NULL, NULL);
 	if (!iommu)
@@ -5227,6 +5276,12 @@ static int aux_domain_add_dev(struct dmar_domain *domain,
 	}
 
 	spin_lock_irqsave(&device_domain_lock, flags);
+	subinfo = subdevice_domain_info_lookup(domain, dev);
+	if (subinfo) {
+		_auxiliary_link_device(domain, subinfo, dev);
+		spin_unlock_irqrestore(&device_domain_lock, flags);
+		return 0;
+	}
 	/*
 	 * iommu->lock must be held to attach domain to iommu and setup the
 	 * pasid entry for second level translation.
@@ -5270,6 +5325,7 @@ static void aux_domain_remove_dev(struct dmar_domain *domain,
 	struct device_domain_info *info;
 	struct intel_iommu *iommu;
 	unsigned long flags;
+	struct subdevice_domain_info *subinfo;
 
 	if (!is_aux_domain(dev, &domain->domain))
 		return;
@@ -5278,14 +5334,26 @@ static void aux_domain_remove_dev(struct dmar_domain *domain,
 	info = get_domain_info(dev);
 	iommu = info->iommu;
 
-	auxiliary_unlink_device(domain, dev);
+	subinfo = subdevice_domain_info_lookup(domain, dev);
+	if (!subinfo) {
+		spin_unlock_irqrestore(&device_domain_lock, flags);
+		return;
+	}
 
-	spin_lock(&iommu->lock);
-	intel_pasid_tear_down_entry(iommu, dev, domain->default_pasid, false);
-	domain_detach_iommu(domain, iommu);
-	spin_unlock(&iommu->lock);
+	if (auxiliary_unlink_device(domain, subinfo, dev) == 0) {
+		spin_lock(&iommu->lock);
+		intel_pasid_tear_down_entry(iommu,
+					    dev,
+					    domain->default_pasid,
+					    false);
+		domain_detach_iommu(domain, iommu);
+		spin_unlock(&iommu->lock);
+	}
 
 	spin_unlock_irqrestore(&device_domain_lock, flags);
+
+	if (!domain->auxd_refcnt && domain->default_pasid > 0)
+		ioasid_free(domain->default_pasid);
 }
 
 static int prepare_domain_attach_device(struct iommu_domain *domain,
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index 94522685a0d9..1fb3d6ab719a 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -537,7 +537,7 @@ struct dmar_domain {
 
 	bool has_iotlb_device;
 	struct list_head devices;	/* all devices' list */
-	struct list_head auxd;		/* link to device's auxiliary list */
+	struct list_head sub_devices;	/* all devices' list attached via aux-attach */
 	struct iova_domain iovad;	/* iova's that belong to this domain */
 
 	struct dma_pte	*pgd;		/* virtual address */
@@ -636,6 +636,15 @@ struct device_domain_info {
 	struct pasid_table *pasid_table; /* pasid table */
 };
 
+/* Aux attach device domain info */
+struct subdevice_domain_info {
+	struct device *dev;
+	struct dmar_domain *domain;
+	struct list_head link_phys;	/* link to phys device siblings */
+	struct list_head link_domain;	/* link to domain siblings */
+	int users;
+};
+
 static inline void __iommu_flush_cache(
 	struct intel_iommu *iommu, void *addr, int size)
 {
-- 
2.25.1

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH 3/3] iommu/vt-d: A fix to iommu_flush_dev_iotlb() for aux-domain

[Liu Yi L]

iommu_flush_dev_iotlb() is called to invalidate caches on device. It only
loops the devices which are full-attached to the domain. For sub-devices,
this is ineffective. This results in invalid caching entries left on the
device. Fix it by adding loop for subdevices as well. Also, update the
domain->has_iotlb_device for both device/subdevice attach/detach and
ATS enabling/disabling.

Signed-off-by: Liu Yi L <yi.l.liu@intel.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/intel/iommu.c | 90 +++++++++++++++++++++++++++++--------
 1 file changed, 72 insertions(+), 18 deletions(-)

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 4274b4acc325..d9b6037b72b1 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -1437,6 +1437,10 @@ static void __iommu_flush_iotlb(struct intel_iommu *iommu, u16 did,
 			(unsigned long long)DMA_TLB_IAIG(val));
 }
 
+/**
+ * For a given bus/devfn, fetch its device_domain_info if it supports
+ * device tlb. Only needs to loop devices attached in normal manner.
+ */
 static struct device_domain_info *
 iommu_support_dev_iotlb (struct dmar_domain *domain, struct intel_iommu *iommu,
 			 u8 bus, u8 devfn)
@@ -1459,6 +1463,18 @@ iommu_support_dev_iotlb (struct dmar_domain *domain, struct intel_iommu *iommu,
 	return NULL;
 }
 
+static bool dev_iotlb_enabled(struct device *dev)
+{
+	struct pci_dev *pdev;
+
+	if (!dev || !dev_is_pci(dev))
+		return false;
+
+	pdev = to_pci_dev(dev);
+
+	return !!pdev->ats_enabled;
+}
+
 static void domain_update_iotlb(struct dmar_domain *domain)
 {
 	struct device_domain_info *info;
@@ -1467,21 +1483,37 @@ static void domain_update_iotlb(struct dmar_domain *domain)
 	assert_spin_locked(&device_domain_lock);
 
 	list_for_each_entry(info, &domain->devices, link) {
-		struct pci_dev *pdev;
-
-		if (!info->dev || !dev_is_pci(info->dev))
-			continue;
-
-		pdev = to_pci_dev(info->dev);
-		if (pdev->ats_enabled) {
+		if (dev_iotlb_enabled(info->dev)) {
 			has_iotlb_device = true;
 			break;
 		}
 	}
 
+	if (!has_iotlb_device) {
+		struct subdevice_domain_info *subinfo;
+
+		list_for_each_entry(subinfo, &domain->sub_devices, link_phys) {
+			if (dev_iotlb_enabled(subinfo->dev)) {
+				has_iotlb_device = true;
+				break;
+			}
+		}
+	}
 	domain->has_iotlb_device = has_iotlb_device;
 }
 
+static void dev_update_domain_iotlb(struct device_domain_info *info)
+{
+	struct subdevice_domain_info *subinfo;
+
+	assert_spin_locked(&device_domain_lock);
+
+	domain_update_iotlb(info->domain);
+
+	list_for_each_entry(subinfo, &info->auxiliary_domains, link_domain)
+		domain_update_iotlb(subinfo->domain);
+}
+
 static void iommu_enable_dev_iotlb(struct device_domain_info *info)
 {
 	struct pci_dev *pdev;
@@ -1524,7 +1556,7 @@ static void iommu_enable_dev_iotlb(struct device_domain_info *info)
 	if (info->ats_supported && pci_ats_page_aligned(pdev) &&
 	    !pci_enable_ats(pdev, VTD_PAGE_SHIFT)) {
 		info->ats_enabled = 1;
-		domain_update_iotlb(info->domain);
+		dev_update_domain_iotlb(info);
 		info->ats_qdep = pci_ats_queue_depth(pdev);
 	}
 }
@@ -1543,7 +1575,7 @@ static void iommu_disable_dev_iotlb(struct device_domain_info *info)
 	if (info->ats_enabled) {
 		pci_disable_ats(pdev);
 		info->ats_enabled = 0;
-		domain_update_iotlb(info->domain);
+		dev_update_domain_iotlb(info);
 	}
 #ifdef CONFIG_INTEL_IOMMU_SVM
 	if (info->pri_enabled) {
@@ -1557,26 +1589,43 @@ static void iommu_disable_dev_iotlb(struct device_domain_info *info)
 #endif
 }
 
+static void __iommu_flush_dev_iotlb(struct device_domain_info *info,
+				    u64 addr, unsigned mask)
+{
+	u16 sid, qdep;
+
+	if (!info || !info->ats_enabled)
+		return;
+
+	sid = info->bus << 8 | info->devfn;
+	qdep = info->ats_qdep;
+	qi_flush_dev_iotlb(info->iommu, sid, info->pfsid,
+			   qdep, addr, mask);
+}
+
 static void iommu_flush_dev_iotlb(struct dmar_domain *domain,
 				  u64 addr, unsigned mask)
 {
-	u16 sid, qdep;
 	unsigned long flags;
 	struct device_domain_info *info;
+	struct subdevice_domain_info *subinfo;
 
 	if (!domain->has_iotlb_device)
 		return;
 
 	spin_lock_irqsave(&device_domain_lock, flags);
-	list_for_each_entry(info, &domain->devices, link) {
-		if (!info->ats_enabled)
-			continue;
+	list_for_each_entry(info, &domain->devices, link)
+		__iommu_flush_dev_iotlb(info, addr, mask);
+
+	/*
+	 * Besides looping all devices attached normally, also
+	 * needs to loop all devices attached via auxiliary
+	 * manner.
+	 */
+	list_for_each_entry(subinfo, &domain->sub_devices, link_phys)
+		__iommu_flush_dev_iotlb(get_domain_info(subinfo->dev),
+					addr, mask);
 
-		sid = info->bus << 8 | info->devfn;
-		qdep = info->ats_qdep;
-		qi_flush_dev_iotlb(info->iommu, sid, info->pfsid,
-				qdep, addr, mask);
-	}
 	spin_unlock_irqrestore(&device_domain_lock, flags);
 }
 
@@ -5208,6 +5257,9 @@ static void auxiliary_link_device(struct dmar_domain *domain,
 	subinfo->dev = dev;
 	list_add(&subinfo->link_domain, &info->auxiliary_domains);
 	list_add(&subinfo->link_phys, &domain->sub_devices);
+	if (dev_iotlb_enabled(dev))
+		domain_update_iotlb(domain);
+
 	_auxiliary_link_device(domain, subinfo, dev);
 	domain->auxd_refcnt++;
 }
@@ -5242,6 +5294,8 @@ static int auxiliary_unlink_device(struct dmar_domain *domain,
 		list_del(&subinfo->link_domain);
 		list_del(&subinfo->link_phys);
 		kfree(subinfo);
+		if (domain->has_iotlb_device)
+			domain_update_iotlb(domain);
 	}
 	domain->auxd_refcnt--;
 
-- 
2.25.1

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

RE: [PATCH 1/3] iommu/vt-d: Move intel_iommu info from struct intel_svm to struct intel_svm_dev

[Guo, Kaijie]

Hi Baolu,

I have verified on 5.9.13 this fix can resolve the error "DMAR: DRHD: handling fault status reg 40".

Regards,
Kaijie

-----Original Message-----
From: Lu Baolu <baolu.lu@linux.intel.com> 
Sent: Saturday, December 19, 2020 2:50 PM
To: Liu, Yi L <yi.l.liu@intel.com>; Pan, Jacob jun <jacob.jun.pan@intel.com>; Zeng, Xin <xin.zeng@intel.com>; Guo, Kaijie <kaijie.guo@intel.com>; will@kernel.org; joro@8bytes.org
Cc: baolu.lu@linux.intel.com; Tian, Kevin <kevin.tian@intel.com>; Tian, Jun J <jun.j.tian@intel.com>; Raj, Ashok <ashok.raj@intel.com>; iommu@lists.linux-foundation.org; Jacob Pan <jacob.jun.pan@linux.intel.com>; David Woodhouse <dwmw2@infradead.org>
Subject: Re: [PATCH 1/3] iommu/vt-d: Move intel_iommu info from struct intel_svm to struct intel_svm_dev

Hi,

On 2020/12/20 8:03, Liu Yi L wrote:
> Current struct intel_svm has a field to record the struct intel_iommu 
> pointer for a PASID bind. And struct intel_svm will be shared by all 
> the devices bind to the same process. The devices may be behind 
> different DMAR units. As the iommu driver code uses the intel_iommu 
> pointer stored in intel_svm struct to do cache invalidations, it may 
> only flush the cache on a single DMAR unit, for others, the cache invalidation is missed.
> 
> As intel_svm struct already has a device list, this patch just moves 
> the intel_iommu pointer to be a field of intel_svm_dev struct.
> 
> Fixes: 2f26e0a9c986 ("iommu/vt-d: Add basic SVM PASID support")
> Cc: Lu Baolu <baolu.lu@linux.intel.com>
> Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
> Cc: Raj Ashok <ashok.raj@intel.com>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Reported-by: Guo Kaijie <Kaijie.Guo@intel.com>
> Reported-by: Xin Zeng <xin.zeng@intel.com>

Kaijie or Xin, can you please confirm whether this fix work for you?

Best regards,
baolu

> Signed-off-by: Guo Kaijie <Kaijie.Guo@intel.com>
> Signed-off-by: Xin Zeng <xin.zeng@intel.com>
> Signed-off-by: Liu Yi L <yi.l.liu@intel.com>
> ---
>   drivers/iommu/intel/svm.c   | 9 +++++----
>   include/linux/intel-iommu.h | 2 +-
>   2 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c 
> index 3242ebd0bca3..4a10c9ff368c 100644
> --- a/drivers/iommu/intel/svm.c
> +++ b/drivers/iommu/intel/svm.c
> @@ -142,7 +142,7 @@ static void intel_flush_svm_range_dev (struct intel_svm *svm, struct intel_svm_d
>   	}
>   	desc.qw2 = 0;
>   	desc.qw3 = 0;
> -	qi_submit_sync(svm->iommu, &desc, 1, 0);
> +	qi_submit_sync(sdev->iommu, &desc, 1, 0);
>   
>   	if (sdev->dev_iotlb) {
>   		desc.qw0 = QI_DEV_EIOTLB_PASID(svm->pasid) | @@ -166,7 +166,7 @@ 
> static void intel_flush_svm_range_dev (struct intel_svm *svm, struct intel_svm_d
>   		}
>   		desc.qw2 = 0;
>   		desc.qw3 = 0;
> -		qi_submit_sync(svm->iommu, &desc, 1, 0);
> +		qi_submit_sync(sdev->iommu, &desc, 1, 0);
>   	}
>   }
>   
> @@ -211,7 +211,7 @@ static void intel_mm_release(struct mmu_notifier *mn, struct mm_struct *mm)
>   	 */
>   	rcu_read_lock();
>   	list_for_each_entry_rcu(sdev, &svm->devs, list)
> -		intel_pasid_tear_down_entry(svm->iommu, sdev->dev,
> +		intel_pasid_tear_down_entry(sdev->iommu, sdev->dev,
>   					    svm->pasid, true);
>   	rcu_read_unlock();
>   
> @@ -363,6 +363,7 @@ int intel_svm_bind_gpasid(struct iommu_domain *domain, struct device *dev,
>   	}
>   	sdev->dev = dev;
>   	sdev->sid = PCI_DEVID(info->bus, info->devfn);
> +	sdev->iommu = iommu;
>   
>   	/* Only count users if device has aux domains */
>   	if (iommu_dev_feature_enabled(dev, IOMMU_DEV_FEAT_AUX)) @@ -546,6 
> +547,7 @@ intel_svm_bind_mm(struct device *dev, unsigned int flags,
>   		goto out;
>   	}
>   	sdev->dev = dev;
> +	sdev->iommu = iommu;
>   
>   	ret = intel_iommu_enable_pasid(iommu, dev);
>   	if (ret) {
> @@ -575,7 +577,6 @@ intel_svm_bind_mm(struct device *dev, unsigned int flags,
>   			kfree(sdev);
>   			goto out;
>   		}
> -		svm->iommu = iommu;
>   
>   		if (pasid_max > intel_pasid_max_id)
>   			pasid_max = intel_pasid_max_id;
> diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h 
> index d956987ed032..94522685a0d9 100644
> --- a/include/linux/intel-iommu.h
> +++ b/include/linux/intel-iommu.h
> @@ -758,6 +758,7 @@ struct intel_svm_dev {
>   	struct list_head list;
>   	struct rcu_head rcu;
>   	struct device *dev;
> +	struct intel_iommu *iommu;
>   	struct svm_dev_ops *ops;
>   	struct iommu_sva sva;
>   	u32 pasid;
> @@ -771,7 +772,6 @@ struct intel_svm {
>   	struct mmu_notifier notifier;
>   	struct mm_struct *mm;
>   
> -	struct intel_iommu *iommu;
>   	unsigned int flags;
>   	u32 pasid;
>   	int gpasid; /* In case that guest PASID is different from host 
> PASID */
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 0/4] iommu/vtd-: Misc fixes on scalable mode

[Jacob Pan]

Hi Yi,

nit: The cover letter is 0/4, patches are 1/3 - 3/3. You also need to copy
LKML.

On Sun, 20 Dec 2020 08:03:49 +0800, Liu Yi L <yi.l.liu@intel.com> wrote:

> Hi,
> 
> This patchset aims to fix a bug regards to SVM usage on native, and
perhaps 'native SVM usage'
> also several bugs around subdevice (attached to device via auxiliary
> manner) tracking and ineffective device_tlb flush.
> 
> Regards,
> Yi Liu
> 
> Liu Yi L (3):
>   iommu/vt-d: Move intel_iommu info from struct intel_svm to struct
>     intel_svm_dev
>   iommu/vt-d: Track device aux-attach with subdevice_domain_info.
>   iommu/vt-d: A fix to iommu_flush_dev_iotlb() for aux-domain
> 
>  drivers/iommu/intel/iommu.c | 182 ++++++++++++++++++++++++++++++------
>  drivers/iommu/intel/svm.c   |   9 +-
>  include/linux/intel-iommu.h |  13 ++-
>  3 files changed, 168 insertions(+), 36 deletions(-)
> 


Thanks,

Jacob
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 2/3] iommu/vt-d: Track device aux-attach with subdevice_domain_info.

[Jacob Pan]

Hi Yi,

On Sun, 20 Dec 2020 08:03:51 +0800, Liu Yi L <yi.l.liu@intel.com> wrote:

> In existing code, if wanting to loop all devices attached to a domain,
> current code can only loop the devices which are attached to the domain
> via normal manner. While for devices attached via auxiliary manner, this
> is subdevice, they are not tracked in the domain. This patch adds struct
How about "In the existing code, loop all devices attached to a domain does
not include sub-devices attached via iommu_aux_attach_device()."

> subdevice_domain_info which is created per domain attachment via auxiliary
> manner. So that such devices are also tracked in domain.
> 
> This was found by when I'm working on the belwo patch, There is no device
> in domain->devices, thus unable to get the cap and ecap of iommu unit. But
> this domain actually has one sub-device which is attached via aux-manner.
> This patch fixes the issue.
> 
> https://lore.kernel.org/kvm/1599734733-6431-17-git-send-email-yi.l.liu@intel.com/
> 
> But looks like, it doesn't affect me only. Such auxiliary track should be
> there for example if wanting to flush device_iotlb for a domain which has
> devices attached by auxiliray manner, then this fix is also necessary.
Perhaps:
This fix goes beyond the patch above, such sub-device tracking is
necessary for other cases. For example, flushing device_iotlb for a domain
which has sub-devices attached by auxiliary manner.

> This issue will also be fixed by another patch in this series with some
> additional changes based on the sudevice tracking framework introduced in
> this patch.
> 
> Co-developed-by: Xin Zeng <xin.zeng@intel.com>
> Signed-off-by: Xin Zeng <xin.zeng@intel.com>
> Co-developed-by: Liu Yi L <yi.l.liu@intel.com>
> Signed-off-by: Liu Yi L <yi.l.liu@intel.com>
> Co-developed-by: Lu Baolu <baolu.lu@linux.intel.com>
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> ---
>  drivers/iommu/intel/iommu.c | 92 ++++++++++++++++++++++++++++++++-----
>  include/linux/intel-iommu.h | 11 ++++-
>  2 files changed, 90 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index a49afa11673c..4274b4acc325 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -1881,6 +1881,7 @@ static struct dmar_domain *alloc_domain(int flags)
>  		domain->flags |= DOMAIN_FLAG_USE_FIRST_LEVEL;
>  	domain->has_iotlb_device = false;
>  	INIT_LIST_HEAD(&domain->devices);
> +	INIT_LIST_HEAD(&domain->sub_devices);
>  
>  	return domain;
>  }
> @@ -5172,33 +5173,79 @@ is_aux_domain(struct device *dev, struct
> iommu_domain *domain) domain->type == IOMMU_DOMAIN_UNMANAGED;
>  }
>  
> +static inline
> +void _auxiliary_link_device(struct dmar_domain *domain,
> +			    struct subdevice_domain_info *subinfo,
> +			    struct device *dev)
> +{
> +	subinfo->users++;
> +}
> +
why pass in more arguments than subinfo? the function name does not match
what it does, seems just refcount inc.

> +static inline
> +int _auxiliary_unlink_device(struct dmar_domain *domain,
> +			     struct subdevice_domain_info *subinfo,
> +			     struct device *dev)
> +{
> +	subinfo->users--;
> +	return subinfo->users;
ditto. why not just
 	return subinfo->users--;

> +}
> +
>  static void auxiliary_link_device(struct dmar_domain *domain,
>  				  struct device *dev)
>  {
>  	struct device_domain_info *info = get_domain_info(dev);
> +	struct subdevice_domain_info *subinfo;
>  
>  	assert_spin_locked(&device_domain_lock);
>  	if (WARN_ON(!info))
>  		return;
>  
> +	subinfo = kzalloc(sizeof(*subinfo), GFP_ATOMIC);
> +	if (!subinfo)
> +		return;
> +
> +	subinfo->domain = domain;
> +	subinfo->dev = dev;
> +	list_add(&subinfo->link_domain, &info->auxiliary_domains);
> +	list_add(&subinfo->link_phys, &domain->sub_devices);
> +	_auxiliary_link_device(domain, subinfo, dev);
or just opencode subinfo->users++?
>  	domain->auxd_refcnt++;
> -	list_add(&domain->auxd, &info->auxiliary_domains);
>  }
>  
> -static void auxiliary_unlink_device(struct dmar_domain *domain,
> -				    struct device *dev)
> +static struct subdevice_domain_info *
> +subdevice_domain_info_lookup(struct dmar_domain *domain, struct device
> *dev) +{
> +	struct subdevice_domain_info *subinfo;
> +
> +	assert_spin_locked(&device_domain_lock);
> +
> +	list_for_each_entry(subinfo, &domain->sub_devices, link_phys)
> +		if (subinfo->dev == dev)
> +			return subinfo;
> +
> +	return NULL;
> +}
> +
> +static int auxiliary_unlink_device(struct dmar_domain *domain,
> +				   struct subdevice_domain_info *subinfo,
> +				   struct device *dev)
>  {
>  	struct device_domain_info *info = get_domain_info(dev);
> +	int ret;
>  
>  	assert_spin_locked(&device_domain_lock);
>  	if (WARN_ON(!info))
> -		return;
> +		return -EINVAL;
>  
> -	list_del(&domain->auxd);
> +	ret = _auxiliary_unlink_device(domain, subinfo, dev);
> +	if (ret == 0) {
> +		list_del(&subinfo->link_domain);
> +		list_del(&subinfo->link_phys);
> +		kfree(subinfo);
> +	}
>  	domain->auxd_refcnt--;
>  
> -	if (!domain->auxd_refcnt && domain->default_pasid > 0)
> -		ioasid_free(domain->default_pasid);
> +	return ret;
>  }
>  
>  static int aux_domain_add_dev(struct dmar_domain *domain,
> @@ -5207,6 +5254,8 @@ static int aux_domain_add_dev(struct dmar_domain
> *domain, int ret;
>  	unsigned long flags;
>  	struct intel_iommu *iommu;
> +	struct device_domain_info *info = get_domain_info(dev);
> +	struct subdevice_domain_info *subinfo;
>  
>  	iommu = device_to_iommu(dev, NULL, NULL);
>  	if (!iommu)
> @@ -5227,6 +5276,12 @@ static int aux_domain_add_dev(struct dmar_domain
> *domain, }
>  
>  	spin_lock_irqsave(&device_domain_lock, flags);
> +	subinfo = subdevice_domain_info_lookup(domain, dev);
> +	if (subinfo) {
> +		_auxiliary_link_device(domain, subinfo, dev);
> +		spin_unlock_irqrestore(&device_domain_lock, flags);
> +		return 0;
> +	}
>  	/*
>  	 * iommu->lock must be held to attach domain to iommu and setup
> the
>  	 * pasid entry for second level translation.
> @@ -5270,6 +5325,7 @@ static void aux_domain_remove_dev(struct
> dmar_domain *domain, struct device_domain_info *info;
>  	struct intel_iommu *iommu;
>  	unsigned long flags;
> +	struct subdevice_domain_info *subinfo;
>  
>  	if (!is_aux_domain(dev, &domain->domain))
>  		return;
> @@ -5278,14 +5334,26 @@ static void aux_domain_remove_dev(struct
> dmar_domain *domain, info = get_domain_info(dev);
>  	iommu = info->iommu;
>  
> -	auxiliary_unlink_device(domain, dev);
> +	subinfo = subdevice_domain_info_lookup(domain, dev);
> +	if (!subinfo) {
> +		spin_unlock_irqrestore(&device_domain_lock, flags);
> +		return;
> +	}
>  
> -	spin_lock(&iommu->lock);
> -	intel_pasid_tear_down_entry(iommu, dev, domain->default_pasid,
> false);
> -	domain_detach_iommu(domain, iommu);
> -	spin_unlock(&iommu->lock);
> +	if (auxiliary_unlink_device(domain, subinfo, dev) == 0) {
> +		spin_lock(&iommu->lock);
> +		intel_pasid_tear_down_entry(iommu,
> +					    dev,
> +					    domain->default_pasid,
> +					    false);
> +		domain_detach_iommu(domain, iommu);
> +		spin_unlock(&iommu->lock);
> +	}
>  
>  	spin_unlock_irqrestore(&device_domain_lock, flags);
> +
> +	if (!domain->auxd_refcnt && domain->default_pasid > 0)
> +		ioasid_free(domain->default_pasid);
>  }
>  
>  static int prepare_domain_attach_device(struct iommu_domain *domain,
> diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
> index 94522685a0d9..1fb3d6ab719a 100644
> --- a/include/linux/intel-iommu.h
> +++ b/include/linux/intel-iommu.h
> @@ -537,7 +537,7 @@ struct dmar_domain {
>  
>  	bool has_iotlb_device;
>  	struct list_head devices;	/* all devices' list */
> -	struct list_head auxd;		/* link to device's
> auxiliary list */
> +	struct list_head sub_devices;	/* all devices' list
> attached via aux-attach */ struct iova_domain iovad;	/* iova's
> that belong to this domain */ 
>  	struct dma_pte	*pgd;		/* virtual address */
> @@ -636,6 +636,15 @@ struct device_domain_info {
>  	struct pasid_table *pasid_table; /* pasid table */
>  };
>  
> +/* Aux attach device domain info */
> +struct subdevice_domain_info {
> +	struct device *dev;
> +	struct dmar_domain *domain;
> +	struct list_head link_phys;	/* link to phys device
> siblings */
> +	struct list_head link_domain;	/* link to domain siblings
> */
> +	int users;
> +};
> +
>  static inline void __iommu_flush_cache(
>  	struct intel_iommu *iommu, void *addr, int size)
>  {


Thanks,

Jacob
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

RE: [PATCH 0/4] iommu/vtd-: Misc fixes on scalable mode

[Liu, Yi L]

> From: Jacob Pan <jacob.jun.pan@intel.com>
> Sent: Wednesday, December 23, 2020 2:17 AM
> 
> Hi Yi,
> 
> nit: The cover letter is 0/4, patches are 1/3 - 3/3. You also need to copy
> LKML.
> 
> On Sun, 20 Dec 2020 08:03:49 +0800, Liu Yi L <yi.l.liu@intel.com> wrote:
> 
> > Hi,
> >
> > This patchset aims to fix a bug regards to SVM usage on native, and
> perhaps 'native SVM usage'

got it. thanks. will correct it.

Regards,
Yi Liu

> > also several bugs around subdevice (attached to device via auxiliary
> > manner) tracking and ineffective device_tlb flush.
> >
> > Regards,
> > Yi Liu
> >
> > Liu Yi L (3):
> >   iommu/vt-d: Move intel_iommu info from struct intel_svm to struct
> >     intel_svm_dev
> >   iommu/vt-d: Track device aux-attach with subdevice_domain_info.
> >   iommu/vt-d: A fix to iommu_flush_dev_iotlb() for aux-domain
> >
> >  drivers/iommu/intel/iommu.c | 182 ++++++++++++++++++++++++++++++---
> ---
> >  drivers/iommu/intel/svm.c   |   9 +-
> >  include/linux/intel-iommu.h |  13 ++-
> >  3 files changed, 168 insertions(+), 36 deletions(-)
> >
> 
> 
> Thanks,
> 
> Jacob
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

RE: [PATCH 2/3] iommu/vt-d: Track device aux-attach with subdevice_domain_info.

[Liu, Yi L]

Hi Jacob,

> From: Jacob Pan <jacob.jun.pan@intel.com>
> Sent: Wednesday, December 23, 2020 4:21 AM
>
> Hi Yi,
> 
> On Sun, 20 Dec 2020 08:03:51 +0800, Liu Yi L <yi.l.liu@intel.com> wrote:
> 
> > In existing code, if wanting to loop all devices attached to a domain,
> > current code can only loop the devices which are attached to the domain
> > via normal manner. While for devices attached via auxiliary manner, this
> > is subdevice, they are not tracked in the domain. This patch adds struct
> How about "In the existing code, loop all devices attached to a domain does
> not include sub-devices attached via iommu_aux_attach_device()."

looks good. will refine accordingly. 😊

> 
> > subdevice_domain_info which is created per domain attachment via
> auxiliary
> > manner. So that such devices are also tracked in domain.
> >
> > This was found by when I'm working on the belwo patch, There is no device
> > in domain->devices, thus unable to get the cap and ecap of iommu unit. But
> > this domain actually has one sub-device which is attached via aux-manner.
> > This patch fixes the issue.
> >
> > https://lore.kernel.org/kvm/1599734733-6431-17-git-send-email-
> yi.l.liu@intel.com/
> >
> > But looks like, it doesn't affect me only. Such auxiliary track should be
> > there for example if wanting to flush device_iotlb for a domain which has
> > devices attached by auxiliray manner, then this fix is also necessary.
> Perhaps:
> This fix goes beyond the patch above, such sub-device tracking is
> necessary for other cases. For example, flushing device_iotlb for a domain
> which has sub-devices attached by auxiliary manner.

yep. Baolu also suggested such refine. will tweak in next version.

Regards,
Yi Liu

> 
> > This issue will also be fixed by another patch in this series with some
> > additional changes based on the sudevice tracking framework introduced in
> > this patch.
> >
> > Co-developed-by: Xin Zeng <xin.zeng@intel.com>
> > Signed-off-by: Xin Zeng <xin.zeng@intel.com>
> > Co-developed-by: Liu Yi L <yi.l.liu@intel.com>
> > Signed-off-by: Liu Yi L <yi.l.liu@intel.com>
> > Co-developed-by: Lu Baolu <baolu.lu@linux.intel.com>
> > Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> > ---
> >  drivers/iommu/intel/iommu.c | 92 ++++++++++++++++++++++++++++++++--
> ---
> >  include/linux/intel-iommu.h | 11 ++++-
> >  2 files changed, 90 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> > index a49afa11673c..4274b4acc325 100644
> > --- a/drivers/iommu/intel/iommu.c
> > +++ b/drivers/iommu/intel/iommu.c
> > @@ -1881,6 +1881,7 @@ static struct dmar_domain *alloc_domain(int
> flags)
> >  		domain->flags |= DOMAIN_FLAG_USE_FIRST_LEVEL;
> >  	domain->has_iotlb_device = false;
> >  	INIT_LIST_HEAD(&domain->devices);
> > +	INIT_LIST_HEAD(&domain->sub_devices);
> >
> >  	return domain;
> >  }
> > @@ -5172,33 +5173,79 @@ is_aux_domain(struct device *dev, struct
> > iommu_domain *domain) domain->type ==
> IOMMU_DOMAIN_UNMANAGED;
> >  }
> >
> > +static inline
> > +void _auxiliary_link_device(struct dmar_domain *domain,
> > +			    struct subdevice_domain_info *subinfo,
> > +			    struct device *dev)
> > +{
> > +	subinfo->users++;
> > +}
> > +
> why pass in more arguments than subinfo? the function name does not match
> what it does, seems just refcount inc.
> 
> > +static inline
> > +int _auxiliary_unlink_device(struct dmar_domain *domain,
> > +			     struct subdevice_domain_info *subinfo,
> > +			     struct device *dev)
> > +{
> > +	subinfo->users--;
> > +	return subinfo->users;
> ditto. why not just
>  	return subinfo->users--;
> 
> > +}
> > +
> >  static void auxiliary_link_device(struct dmar_domain *domain,
> >  				  struct device *dev)
> >  {
> >  	struct device_domain_info *info = get_domain_info(dev);
> > +	struct subdevice_domain_info *subinfo;
> >
> >  	assert_spin_locked(&device_domain_lock);
> >  	if (WARN_ON(!info))
> >  		return;
> >
> > +	subinfo = kzalloc(sizeof(*subinfo), GFP_ATOMIC);
> > +	if (!subinfo)
> > +		return;
> > +
> > +	subinfo->domain = domain;
> > +	subinfo->dev = dev;
> > +	list_add(&subinfo->link_domain, &info->auxiliary_domains);
> > +	list_add(&subinfo->link_phys, &domain->sub_devices);
> > +	_auxiliary_link_device(domain, subinfo, dev);
> or just opencode subinfo->users++?
> >  	domain->auxd_refcnt++;
> > -	list_add(&domain->auxd, &info->auxiliary_domains);
> >  }
> >
> > -static void auxiliary_unlink_device(struct dmar_domain *domain,
> > -				    struct device *dev)
> > +static struct subdevice_domain_info *
> > +subdevice_domain_info_lookup(struct dmar_domain *domain, struct
> device
> > *dev) +{
> > +	struct subdevice_domain_info *subinfo;
> > +
> > +	assert_spin_locked(&device_domain_lock);
> > +
> > +	list_for_each_entry(subinfo, &domain->sub_devices, link_phys)
> > +		if (subinfo->dev == dev)
> > +			return subinfo;
> > +
> > +	return NULL;
> > +}
> > +
> > +static int auxiliary_unlink_device(struct dmar_domain *domain,
> > +				   struct subdevice_domain_info *subinfo,
> > +				   struct device *dev)
> >  {
> >  	struct device_domain_info *info = get_domain_info(dev);
> > +	int ret;
> >
> >  	assert_spin_locked(&device_domain_lock);
> >  	if (WARN_ON(!info))
> > -		return;
> > +		return -EINVAL;
> >
> > -	list_del(&domain->auxd);
> > +	ret = _auxiliary_unlink_device(domain, subinfo, dev);
> > +	if (ret == 0) {
> > +		list_del(&subinfo->link_domain);
> > +		list_del(&subinfo->link_phys);
> > +		kfree(subinfo);
> > +	}
> >  	domain->auxd_refcnt--;
> >
> > -	if (!domain->auxd_refcnt && domain->default_pasid > 0)
> > -		ioasid_free(domain->default_pasid);
> > +	return ret;
> >  }
> >
> >  static int aux_domain_add_dev(struct dmar_domain *domain,
> > @@ -5207,6 +5254,8 @@ static int aux_domain_add_dev(struct
> dmar_domain
> > *domain, int ret;
> >  	unsigned long flags;
> >  	struct intel_iommu *iommu;
> > +	struct device_domain_info *info = get_domain_info(dev);
> > +	struct subdevice_domain_info *subinfo;
> >
> >  	iommu = device_to_iommu(dev, NULL, NULL);
> >  	if (!iommu)
> > @@ -5227,6 +5276,12 @@ static int aux_domain_add_dev(struct
> dmar_domain
> > *domain, }
> >
> >  	spin_lock_irqsave(&device_domain_lock, flags);
> > +	subinfo = subdevice_domain_info_lookup(domain, dev);
> > +	if (subinfo) {
> > +		_auxiliary_link_device(domain, subinfo, dev);
> > +		spin_unlock_irqrestore(&device_domain_lock, flags);
> > +		return 0;
> > +	}
> >  	/*
> >  	 * iommu->lock must be held to attach domain to iommu and setup
> > the
> >  	 * pasid entry for second level translation.
> > @@ -5270,6 +5325,7 @@ static void aux_domain_remove_dev(struct
> > dmar_domain *domain, struct device_domain_info *info;
> >  	struct intel_iommu *iommu;
> >  	unsigned long flags;
> > +	struct subdevice_domain_info *subinfo;
> >
> >  	if (!is_aux_domain(dev, &domain->domain))
> >  		return;
> > @@ -5278,14 +5334,26 @@ static void aux_domain_remove_dev(struct
> > dmar_domain *domain, info = get_domain_info(dev);
> >  	iommu = info->iommu;
> >
> > -	auxiliary_unlink_device(domain, dev);
> > +	subinfo = subdevice_domain_info_lookup(domain, dev);
> > +	if (!subinfo) {
> > +		spin_unlock_irqrestore(&device_domain_lock, flags);
> > +		return;
> > +	}
> >
> > -	spin_lock(&iommu->lock);
> > -	intel_pasid_tear_down_entry(iommu, dev, domain->default_pasid,
> > false);
> > -	domain_detach_iommu(domain, iommu);
> > -	spin_unlock(&iommu->lock);
> > +	if (auxiliary_unlink_device(domain, subinfo, dev) == 0) {
> > +		spin_lock(&iommu->lock);
> > +		intel_pasid_tear_down_entry(iommu,
> > +					    dev,
> > +					    domain->default_pasid,
> > +					    false);
> > +		domain_detach_iommu(domain, iommu);
> > +		spin_unlock(&iommu->lock);
> > +	}
> >
> >  	spin_unlock_irqrestore(&device_domain_lock, flags);
> > +
> > +	if (!domain->auxd_refcnt && domain->default_pasid > 0)
> > +		ioasid_free(domain->default_pasid);
> >  }
> >
> >  static int prepare_domain_attach_device(struct iommu_domain *domain,
> > diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
> > index 94522685a0d9..1fb3d6ab719a 100644
> > --- a/include/linux/intel-iommu.h
> > +++ b/include/linux/intel-iommu.h
> > @@ -537,7 +537,7 @@ struct dmar_domain {
> >
> >  	bool has_iotlb_device;
> >  	struct list_head devices;	/* all devices' list */
> > -	struct list_head auxd;		/* link to device's
> > auxiliary list */
> > +	struct list_head sub_devices;	/* all devices' list
> > attached via aux-attach */ struct iova_domain iovad;	/* iova's
> > that belong to this domain */
> >  	struct dma_pte	*pgd;		/* virtual address */
> > @@ -636,6 +636,15 @@ struct device_domain_info {
> >  	struct pasid_table *pasid_table; /* pasid table */
> >  };
> >
> > +/* Aux attach device domain info */
> > +struct subdevice_domain_info {
> > +	struct device *dev;
> > +	struct dmar_domain *domain;
> > +	struct list_head link_phys;	/* link to phys device
> > siblings */
> > +	struct list_head link_domain;	/* link to domain siblings
> > */
> > +	int users;
> > +};
> > +
> >  static inline void __iommu_flush_cache(
> >  	struct intel_iommu *iommu, void *addr, int size)
> >  {
> 
> 
> Thanks,
> 
> Jacob
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu