Re: [PATCH v1 1/3] iommu/vt-d: Fix prq reporting issues

[Yi Sun <yi.y.sun@linux.intel.com>]

Thanks! Will do the modifications according to your comments in next
version.

On 20-10-28 12:50:37, Lu Baolu wrote:
> Hi Yi,
> 
> On 10/28/20 9:36 AM, Yi Sun wrote:
> >From: Liu Yi L <yi.l.liu@intel.com>
> >
> >Should get correct sid and set it into sdev. Because we execute
> >'sdev->sid != req->rid' in the loop of prq_event_thread().
> 
> How about making the title more accurate, how about something like:
> 
> iommu/vt-d: Fix use before set issue in intel_svm_bind_gpasid()
> 
> >
> >Signed-off-by: Liu Yi L <yi.l.liu@intel.com>
> >Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
> >---
> >  drivers/iommu/intel/svm.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> >diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
> >index f1861fa..75d9dc9 100644
> >--- a/drivers/iommu/intel/svm.c
> >+++ b/drivers/iommu/intel/svm.c
> >@@ -277,6 +277,7 @@ int intel_svm_bind_gpasid(struct iommu_domain *domain, struct device *dev,
> >  			  struct iommu_gpasid_bind_data *data)
> >  {
> >  	struct intel_iommu *iommu = device_to_iommu(dev, NULL, NULL);
> >+	struct device_domain_info *info;
> >  	struct intel_svm_dev *sdev = NULL;
> >  	struct dmar_domain *dmar_domain;
> >  	struct intel_svm *svm = NULL;
> >@@ -357,6 +358,8 @@ int intel_svm_bind_gpasid(struct iommu_domain *domain, struct device *dev,
> >  		goto out;
> >  	}
> >  	sdev->dev = dev;
> >+	info = get_domain_info(dev);
> >+	sdev->sid = PCI_DEVID(info->bus, info->devfn);
> >  	/* Only count users if device has aux domains */
> >  	if (iommu_dev_feature_enabled(dev, IOMMU_DEV_FEAT_AUX))
> >
> 
> How about moving get_domain_info() up to the sanity check part?
> 
> diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
> index f1861fa3d0e4..75846692f2f2 100644
> --- a/drivers/iommu/intel/svm.c
> +++ b/drivers/iommu/intel/svm.c
> @@ -279,6 +279,7 @@ int intel_svm_bind_gpasid(struct iommu_domain
> *domain, struct device *dev,
>         struct intel_iommu *iommu = device_to_iommu(dev, NULL, NULL);
>         struct intel_svm_dev *sdev = NULL;
>         struct dmar_domain *dmar_domain;
> +       struct device_domain_info *info;
>         struct intel_svm *svm = NULL;
>         int ret = 0;
> 
> @@ -310,6 +311,10 @@ int intel_svm_bind_gpasid(struct iommu_domain
> *domain, struct device *dev,
>         if (data->hpasid <= 0 || data->hpasid >= PASID_MAX)
>                 return -EINVAL;
> 
> +       info = get_domain_info(dev);
> +       if (!info)
> +               return -EINVAL;
> +
>         dmar_domain = to_dmar_domain(domain);
> 
>         mutex_lock(&pasid_mutex);
> @@ -357,6 +362,7 @@ int intel_svm_bind_gpasid(struct iommu_domain
> *domain, struct device *dev,
>                 goto out;
>         }
>         sdev->dev = dev;
> +       sdev->sid = PCI_DEVID(info->bus, info->devfn);
> 
>         /* Only count users if device has aux domains */
>         if (iommu_dev_feature_enabled(dev, IOMMU_DEV_FEAT_AUX))
> 
> Best regards,
> baolu
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu