Re: [bug report] iommu/vt-d: Add SVA domain support

[Baolu Lu <baolu.lu@linux.intel.com>]

On 11/15/22 9:04 PM, Dan Carpenter wrote:
> Hello Lu Baolu,
> 

Hi Dan,

> The patch eaca8889a1ef: "iommu/vt-d: Add SVA domain support" from Oct
> 31, 2022, leads to the following Smatch static checker warning:
> 
> drivers/gpu/drm/msm/adreno/adreno_gpu.c:213 adreno_iommu_create_address_space() warn: passing zero to 'ERR_CAST'
> drivers/gpu/drm/msm/adreno/adreno_gpu.c:216 adreno_iommu_create_address_space() warn: 'geometry' isn't an ERR_PTR
> drivers/iommu/intel/iommu.c:4721 intel_iommu_remove_dev_pasid() error: 'domain' dereferencing possible ERR_PTR()
> drivers/iommu/io-pgfault.c:82 iopf_handler() error: 'domain' dereferencing possible ERR_PTR()
> 
> drivers/iommu/intel/iommu.c
>      4713 static void intel_iommu_remove_dev_pasid(struct device *dev, ioasid_t pasid)
>      4714 {
>      4715         struct intel_iommu *iommu = device_to_iommu(dev, NULL, NULL);
>      4716         struct iommu_domain *domain;
>      4717
>      4718         /* Domain type specific cleanup: */
>      4719         domain = iommu_get_domain_for_dev_pasid(dev, pasid, 0);
>      4720         if (domain) {
> 
> The iommu_get_domain_for_dev_pasid() function returns both error pointers
> and NULL.  The comments say that it's supposed to only return NULL.  I
> don't really understand what's going with these iommu functions.

The iommu_get_domain_for_dev_pasid() function returns ERR_PTR(-EBUSY) if
the requested domain is not the same type as the one stored in the iommu
group.

But as the kernel-doc of iommu_get_domain_for_dev_pasid() states, type 0
means any match, hence, here @domain is either a valid pointer or NULL.

The same thing for iopf_handler().

Best regards,
baolu

> 
> --> 4721                 switch (domain->type) {
>                                   ^^^^^^^^^^^^
>      4722                 case IOMMU_DOMAIN_SVA:
>      4723                         intel_svm_remove_dev_pasid(dev, pasid);
>      4724                         break;
>      4725                 default:
>      4726                         /* should never reach here */
>      4727                         WARN_ON(1);
>      4728                         break;
>      4729                 }
>      4730         }
>      4731
>      4732         intel_pasid_tear_down_entry(iommu, dev, pasid, false);
>      4733 }
> 
> regards,
> dan carpenter