[PATCH 0/2] iommu / vfio: Clean up iommu_map[_fast] interface

[PATCH 0/2] iommu / vfio: Clean up iommu_map[_fast] interface

[Suravee Suthikulpanit]

Change iommu_unmap[_fast] interfaces return type to ssize_t since
it can also return error code.

Cc: Joerg Roedel <joro@8bytes.org>
Cc: Alex Williamson <alex.williamson@redhat.com>

Suravee Suthikulpanit (2):
  iommu: Fix iommu_unmap and iommu_unmap_fast return type
  vfio/type1: Add iommu_unmap error check when vfio_unmap_unpin

 drivers/iommu/amd_iommu.c       |  6 +++---
 drivers/iommu/intel-iommu.c     |  4 ++--
 drivers/iommu/iommu.c           | 16 ++++++++--------
 drivers/vfio/vfio_iommu_type1.c |  5 +++--
 include/linux/iommu.h           | 20 ++++++++++----------
 5 files changed, 26 insertions(+), 25 deletions(-)

-- 
1.8.3.1

[PATCH 1/2] iommu: Fix iommu_unmap and iommu_unmap_fast return type

[Suravee Suthikulpanit]

Currently, iommu_unmap and iommu_unmap_fast return unmapped
pages with size_t.  However, the actual value returned could
be error codes (< 0), which can be misinterpreted as large
number of unmapped pages. Therefore, change the return type to ssize_t.

Cc: Joerg Roedel <joro@8bytes.org>
Cc: Alex Williamson <alex.williamson@redhat.com>
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
---
 drivers/iommu/amd_iommu.c   |  6 +++---
 drivers/iommu/intel-iommu.c |  4 ++--
 drivers/iommu/iommu.c       | 16 ++++++++--------
 include/linux/iommu.h       | 20 ++++++++++----------
 4 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 7d5eb00..3609f51 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -3030,11 +3030,11 @@ static int amd_iommu_map(struct iommu_domain *dom, unsigned long iova,
 	return ret;
 }
 
-static size_t amd_iommu_unmap(struct iommu_domain *dom, unsigned long iova,
-			   size_t page_size)
+static ssize_t amd_iommu_unmap(struct iommu_domain *dom, unsigned long iova,
+			       size_t page_size)
 {
 	struct protection_domain *domain = to_pdomain(dom);
-	size_t unmap_size;
+	ssize_t unmap_size;
 
 	if (domain->mode == PAGE_MODE_NONE)
 		return -EINVAL;
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 4a2de34..15ba866 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -5068,8 +5068,8 @@ static int intel_iommu_map(struct iommu_domain *domain,
 	return ret;
 }
 
-static size_t intel_iommu_unmap(struct iommu_domain *domain,
-				unsigned long iova, size_t size)
+static ssize_t intel_iommu_unmap(struct iommu_domain *domain,
+				 unsigned long iova, size_t size)
 {
 	struct dmar_domain *dmar_domain = to_dmar_domain(domain);
 	struct page *freelist = NULL;
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 3de5c0b..8f7da8a 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1557,12 +1557,12 @@ int iommu_map(struct iommu_domain *domain, unsigned long iova,
 }
 EXPORT_SYMBOL_GPL(iommu_map);
 
-static size_t __iommu_unmap(struct iommu_domain *domain,
-			    unsigned long iova, size_t size,
-			    bool sync)
+static ssize_t __iommu_unmap(struct iommu_domain *domain,
+			     unsigned long iova, size_t size,
+			     bool sync)
 {
 	const struct iommu_ops *ops = domain->ops;
-	size_t unmapped_page, unmapped = 0;
+	ssize_t unmapped_page, unmapped = 0;
 	unsigned long orig_iova = iova;
 	unsigned int min_pagesz;
 
@@ -1617,15 +1617,15 @@ static size_t __iommu_unmap(struct iommu_domain *domain,
 	return unmapped;
 }
 
-size_t iommu_unmap(struct iommu_domain *domain,
-		   unsigned long iova, size_t size)
+ssize_t iommu_unmap(struct iommu_domain *domain,
+		    unsigned long iova, size_t size)
 {
 	return __iommu_unmap(domain, iova, size, true);
 }
 EXPORT_SYMBOL_GPL(iommu_unmap);
 
-size_t iommu_unmap_fast(struct iommu_domain *domain,
-			unsigned long iova, size_t size)
+ssize_t iommu_unmap_fast(struct iommu_domain *domain,
+			 unsigned long iova, size_t size)
 {
 	return __iommu_unmap(domain, iova, size, false);
 }
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 41b8c57..78df048 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -199,8 +199,8 @@ struct iommu_ops {
 	void (*detach_dev)(struct iommu_domain *domain, struct device *dev);
 	int (*map)(struct iommu_domain *domain, unsigned long iova,
 		   phys_addr_t paddr, size_t size, int prot);
-	size_t (*unmap)(struct iommu_domain *domain, unsigned long iova,
-		     size_t size);
+	ssize_t (*unmap)(struct iommu_domain *domain, unsigned long iova,
+			 size_t size);
 	size_t (*map_sg)(struct iommu_domain *domain, unsigned long iova,
 			 struct scatterlist *sg, unsigned int nents, int prot);
 	void (*flush_iotlb_all)(struct iommu_domain *domain);
@@ -299,10 +299,10 @@ extern void iommu_detach_device(struct iommu_domain *domain,
 extern struct iommu_domain *iommu_get_domain_for_dev(struct device *dev);
 extern int iommu_map(struct iommu_domain *domain, unsigned long iova,
 		     phys_addr_t paddr, size_t size, int prot);
-extern size_t iommu_unmap(struct iommu_domain *domain, unsigned long iova,
-			  size_t size);
-extern size_t iommu_unmap_fast(struct iommu_domain *domain,
-			       unsigned long iova, size_t size);
+extern ssize_t iommu_unmap(struct iommu_domain *domain, unsigned long iova,
+			   size_t size);
+extern ssize_t iommu_unmap_fast(struct iommu_domain *domain,
+				unsigned long iova, size_t size);
 extern size_t default_iommu_map_sg(struct iommu_domain *domain, unsigned long iova,
 				struct scatterlist *sg,unsigned int nents,
 				int prot);
@@ -465,14 +465,14 @@ static inline int iommu_map(struct iommu_domain *domain, unsigned long iova,
 	return -ENODEV;
 }
 
-static inline int iommu_unmap(struct iommu_domain *domain, unsigned long iova,
-			      size_t size)
+static inline ssize_t iommu_unmap(struct iommu_domain *domain,
+				  unsigned long iova, size_t size)
 {
 	return -ENODEV;
 }
 
-static inline int iommu_unmap_fast(struct iommu_domain *domain, unsigned long iova,
-				   int gfp_order)
+static inline ssize_t iommu_unmap_fast(struct iommu_domain *domain,
+				       unsigned long iova, int gfp_order)
 {
 	return -ENODEV;
 }
-- 
1.8.3.1

Re: [PATCH 1/2] iommu: Fix iommu_unmap and iommu_unmap_fast return type

[Robin Murphy]

Hi Suravee,

On 31/01/18 01:48, Suravee Suthikulpanit wrote:
> Currently, iommu_unmap and iommu_unmap_fast return unmapped
> pages with size_t.  However, the actual value returned could
> be error codes (< 0), which can be misinterpreted as large
> number of unmapped pages. Therefore, change the return type to ssize_t.
> 
> Cc: Joerg Roedel <joro@8bytes.org>
> Cc: Alex Williamson <alex.williamson@redhat.com>
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> ---
>   drivers/iommu/amd_iommu.c   |  6 +++---
>   drivers/iommu/intel-iommu.c |  4 ++--

Er, there are a few more drivers than that implementing iommu_ops ;)

It seems like it might be more sensible to fix the single instance of a 
driver returning -EINVAL (which appears to be a "should never happen if 
used correctly" kinda thing anyway) and leave the API-internal callback 
prototype as-is. I do agree the inconsistency of iommu_unmap() itself 
wants sorting, though (particularly the !IOMMU_API stubs which are wrong 
either way).

Robin.

>   drivers/iommu/iommu.c       | 16 ++++++++--------
>   include/linux/iommu.h       | 20 ++++++++++----------
>   4 files changed, 23 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
> index 7d5eb00..3609f51 100644
> --- a/drivers/iommu/amd_iommu.c
> +++ b/drivers/iommu/amd_iommu.c
> @@ -3030,11 +3030,11 @@ static int amd_iommu_map(struct iommu_domain *dom, unsigned long iova,
>   	return ret;
>   }
>   
> -static size_t amd_iommu_unmap(struct iommu_domain *dom, unsigned long iova,
> -			   size_t page_size)
> +static ssize_t amd_iommu_unmap(struct iommu_domain *dom, unsigned long iova,
> +			       size_t page_size)
>   {
>   	struct protection_domain *domain = to_pdomain(dom);
> -	size_t unmap_size;
> +	ssize_t unmap_size;
>   
>   	if (domain->mode == PAGE_MODE_NONE)
>   		return -EINVAL;
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> index 4a2de34..15ba866 100644
> --- a/drivers/iommu/intel-iommu.c
> +++ b/drivers/iommu/intel-iommu.c
> @@ -5068,8 +5068,8 @@ static int intel_iommu_map(struct iommu_domain *domain,
>   	return ret;
>   }
>   
> -static size_t intel_iommu_unmap(struct iommu_domain *domain,
> -				unsigned long iova, size_t size)
> +static ssize_t intel_iommu_unmap(struct iommu_domain *domain,
> +				 unsigned long iova, size_t size)
>   {
>   	struct dmar_domain *dmar_domain = to_dmar_domain(domain);
>   	struct page *freelist = NULL;
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 3de5c0b..8f7da8a 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -1557,12 +1557,12 @@ int iommu_map(struct iommu_domain *domain, unsigned long iova,
>   }
>   EXPORT_SYMBOL_GPL(iommu_map);
>   
> -static size_t __iommu_unmap(struct iommu_domain *domain,
> -			    unsigned long iova, size_t size,
> -			    bool sync)
> +static ssize_t __iommu_unmap(struct iommu_domain *domain,
> +			     unsigned long iova, size_t size,
> +			     bool sync)
>   {
>   	const struct iommu_ops *ops = domain->ops;
> -	size_t unmapped_page, unmapped = 0;
> +	ssize_t unmapped_page, unmapped = 0;
>   	unsigned long orig_iova = iova;
>   	unsigned int min_pagesz;
>   
> @@ -1617,15 +1617,15 @@ static size_t __iommu_unmap(struct iommu_domain *domain,
>   	return unmapped;
>   }
>   
> -size_t iommu_unmap(struct iommu_domain *domain,
> -		   unsigned long iova, size_t size)
> +ssize_t iommu_unmap(struct iommu_domain *domain,
> +		    unsigned long iova, size_t size)
>   {
>   	return __iommu_unmap(domain, iova, size, true);
>   }
>   EXPORT_SYMBOL_GPL(iommu_unmap);
>   
> -size_t iommu_unmap_fast(struct iommu_domain *domain,
> -			unsigned long iova, size_t size)
> +ssize_t iommu_unmap_fast(struct iommu_domain *domain,
> +			 unsigned long iova, size_t size)
>   {
>   	return __iommu_unmap(domain, iova, size, false);
>   }
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 41b8c57..78df048 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -199,8 +199,8 @@ struct iommu_ops {
>   	void (*detach_dev)(struct iommu_domain *domain, struct device *dev);
>   	int (*map)(struct iommu_domain *domain, unsigned long iova,
>   		   phys_addr_t paddr, size_t size, int prot);
> -	size_t (*unmap)(struct iommu_domain *domain, unsigned long iova,
> -		     size_t size);
> +	ssize_t (*unmap)(struct iommu_domain *domain, unsigned long iova,
> +			 size_t size);
>   	size_t (*map_sg)(struct iommu_domain *domain, unsigned long iova,
>   			 struct scatterlist *sg, unsigned int nents, int prot);
>   	void (*flush_iotlb_all)(struct iommu_domain *domain);
> @@ -299,10 +299,10 @@ extern void iommu_detach_device(struct iommu_domain *domain,
>   extern struct iommu_domain *iommu_get_domain_for_dev(struct device *dev);
>   extern int iommu_map(struct iommu_domain *domain, unsigned long iova,
>   		     phys_addr_t paddr, size_t size, int prot);
> -extern size_t iommu_unmap(struct iommu_domain *domain, unsigned long iova,
> -			  size_t size);
> -extern size_t iommu_unmap_fast(struct iommu_domain *domain,
> -			       unsigned long iova, size_t size);
> +extern ssize_t iommu_unmap(struct iommu_domain *domain, unsigned long iova,
> +			   size_t size);
> +extern ssize_t iommu_unmap_fast(struct iommu_domain *domain,
> +				unsigned long iova, size_t size);
>   extern size_t default_iommu_map_sg(struct iommu_domain *domain, unsigned long iova,
>   				struct scatterlist *sg,unsigned int nents,
>   				int prot);
> @@ -465,14 +465,14 @@ static inline int iommu_map(struct iommu_domain *domain, unsigned long iova,
>   	return -ENODEV;
>   }
>   
> -static inline int iommu_unmap(struct iommu_domain *domain, unsigned long iova,
> -			      size_t size)
> +static inline ssize_t iommu_unmap(struct iommu_domain *domain,
> +				  unsigned long iova, size_t size)
>   {
>   	return -ENODEV;
>   }
>   
> -static inline int iommu_unmap_fast(struct iommu_domain *domain, unsigned long iova,
> -				   int gfp_order)
> +static inline ssize_t iommu_unmap_fast(struct iommu_domain *domain,
> +				       unsigned long iova, int gfp_order)
>   {
>   	return -ENODEV;
>   }
> 

Re: [PATCH 1/2] iommu: Fix iommu_unmap and iommu_unmap_fast return type

[Suravee Suthikulpanit]

Hi Robin,

On 2/1/18 1:02 AM, Robin Murphy wrote:
> Hi Suravee,
> 
> On 31/01/18 01:48, Suravee Suthikulpanit wrote:
>> Currently, iommu_unmap and iommu_unmap_fast return unmapped
>> pages with size_t.  However, the actual value returned could
>> be error codes (< 0), which can be misinterpreted as large
>> number of unmapped pages. Therefore, change the return type to ssize_t.
>>
>> Cc: Joerg Roedel <joro@8bytes.org>
>> Cc: Alex Williamson <alex.williamson@redhat.com>
>> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
>> ---
>>   drivers/iommu/amd_iommu.c   |  6 +++---
>>   drivers/iommu/intel-iommu.c |  4 ++--
> 
> Er, there are a few more drivers than that implementing iommu_ops ;)

Ahh right.
> 
> It seems like it might be more sensible to fix the single instance of a driver returning -EINVAL (which appears to be a "should never happen if used correctly" kinda thing anyway) and leave the API-internal callback prototype as-is. I do agree the inconsistency of iommu_unmap() itself wants sorting, though (particularly the !IOMMU_API stubs which are wrong either way).
> 
> Robin.

Make sense. I'll leave the API alone, and change the code to not returning error then.
There are a few places to fix.

Thanks,
Suravee

Re: [PATCH 1/2] iommu: Fix iommu_unmap and iommu_unmap_fast return type

[Robin Murphy]

On 01/02/18 05:03, Suravee Suthikulpanit wrote:
> Hi Robin,
> 
> On 2/1/18 1:02 AM, Robin Murphy wrote:
>> Hi Suravee,
>>
>> On 31/01/18 01:48, Suravee Suthikulpanit wrote:
>>> Currently, iommu_unmap and iommu_unmap_fast return unmapped
>>> pages with size_t.  However, the actual value returned could
>>> be error codes (< 0), which can be misinterpreted as large
>>> number of unmapped pages. Therefore, change the return type to ssize_t.
>>>
>>> Cc: Joerg Roedel <joro@8bytes.org>
>>> Cc: Alex Williamson <alex.williamson@redhat.com>
>>> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
>>> ---
>>>   drivers/iommu/amd_iommu.c   |  6 +++---
>>>   drivers/iommu/intel-iommu.c |  4 ++--
>>
>> Er, there are a few more drivers than that implementing iommu_ops ;)
> 
> Ahh right.
>>
>> It seems like it might be more sensible to fix the single instance of 
>> a driver returning -EINVAL (which appears to be a "should never happen 
>> if used correctly" kinda thing anyway) and leave the API-internal 
>> callback prototype as-is. I do agree the inconsistency of 
>> iommu_unmap() itself wants sorting, though (particularly the 
>> !IOMMU_API stubs which are wrong either way).
>>
>> Robin.
> 
> Make sense. I'll leave the API alone, and change the code to not 
> returning error then.

Actually, on a second look I think that check in amd_iommu is 99% 
redundant anyway, since PAGE_MODE_NONE is only normally set for 
IOMMU_DOMAIN_IDENTITY domains, thus iommu_unmap() would have bailed out 
from the __IOMMU_DOMAIN_PAGING check before ops->unmap could be called. 
AFAICS the only way to hit it at all is if a caller somehow managed to 
get hold of the dev_state->domain set up in amd_iommu_init_device(), 
then tried to unmap something from that, which seems such a very wrong 
thing to do it should probably just crash and burn with extreme 
prejudice anyway.

Cheers,
Robin.

Re: [PATCH 1/2] iommu: Fix iommu_unmap and iommu_unmap_fast return type

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2806 bytes --]

Hi Suravee,

I love your patch! Yet something to improve:

[auto build test ERROR on iommu/next]
[also build test ERROR on v4.15 next-20180202]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Suravee-Suthikulpanit/iommu-Fix-iommu_unmap-and-iommu_unmap_fast-return-type/20180203-015316
base:   https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next
config: s390-allyesconfig (attached as .config)
compiler: s390x-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=s390 

All errors (new ones prefixed by >>):

>> drivers//iommu/s390-iommu.c:373:11: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
     .unmap = s390_iommu_unmap,
              ^~~~~~~~~~~~~~~~
   drivers//iommu/s390-iommu.c:373:11: note: (near initialization for 's390_iommu_ops.unmap')
   cc1: some warnings being treated as errors

vim +373 drivers//iommu/s390-iommu.c

f42c22351 Joerg Roedel    2017-04-27  365  
cceb84519 Arvind Yadav    2017-08-28  366  static const struct iommu_ops s390_iommu_ops = {
8128f23c4 Gerald Schaefer 2015-08-27  367  	.capable = s390_iommu_capable,
8128f23c4 Gerald Schaefer 2015-08-27  368  	.domain_alloc = s390_domain_alloc,
8128f23c4 Gerald Schaefer 2015-08-27  369  	.domain_free = s390_domain_free,
8128f23c4 Gerald Schaefer 2015-08-27  370  	.attach_dev = s390_iommu_attach_device,
8128f23c4 Gerald Schaefer 2015-08-27  371  	.detach_dev = s390_iommu_detach_device,
8128f23c4 Gerald Schaefer 2015-08-27  372  	.map = s390_iommu_map,
8128f23c4 Gerald Schaefer 2015-08-27 @373  	.unmap = s390_iommu_unmap,
8128f23c4 Gerald Schaefer 2015-08-27  374  	.iova_to_phys = s390_iommu_iova_to_phys,
8128f23c4 Gerald Schaefer 2015-08-27  375  	.add_device = s390_iommu_add_device,
8128f23c4 Gerald Schaefer 2015-08-27  376  	.remove_device = s390_iommu_remove_device,
0929deca4 Joerg Roedel    2017-06-15  377  	.device_group = generic_device_group,
8128f23c4 Gerald Schaefer 2015-08-27  378  	.pgsize_bitmap = S390_IOMMU_PGSIZES,
8128f23c4 Gerald Schaefer 2015-08-27  379  };
8128f23c4 Gerald Schaefer 2015-08-27  380  

:::::: The code at line 373 was first introduced by commit
:::::: 8128f23c436d0dd4f72412e1bf9256e424479dc3 iommu/s390: Add iommu api for s390 pci devices

:::::: TO: Gerald Schaefer <gerald.schaefer@de.ibm.com>
:::::: CC: Joerg Roedel <jroedel@suse.de>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 48730 bytes --]

Re: [PATCH 1/2] iommu: Fix iommu_unmap and iommu_unmap_fast return type

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3082 bytes --]

Hi Suravee,

I love your patch! Yet something to improve:

[auto build test ERROR on iommu/next]
[also build test ERROR on v4.15 next-20180202]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Suravee-Suthikulpanit/iommu-Fix-iommu_unmap-and-iommu_unmap_fast-return-type/20180203-015316
base:   https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next
config: sparc64-allyesconfig (attached as .config)
compiler: sparc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=sparc64 

All errors (new ones prefixed by >>):

>> drivers/iommu/qcom_iommu.c:592:12: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
     .unmap  = qcom_iommu_unmap,
               ^~~~~~~~~~~~~~~~
   drivers/iommu/qcom_iommu.c:592:12: note: (near initialization for 'qcom_iommu_ops.unmap')
   cc1: some warnings being treated as errors

vim +592 drivers/iommu/qcom_iommu.c

0ae349a0f3 Rob Clark    2017-08-09  584  
0ae349a0f3 Rob Clark    2017-08-09  585  static const struct iommu_ops qcom_iommu_ops = {
0ae349a0f3 Rob Clark    2017-08-09  586  	.capable	= qcom_iommu_capable,
0ae349a0f3 Rob Clark    2017-08-09  587  	.domain_alloc	= qcom_iommu_domain_alloc,
0ae349a0f3 Rob Clark    2017-08-09  588  	.domain_free	= qcom_iommu_domain_free,
0ae349a0f3 Rob Clark    2017-08-09  589  	.attach_dev	= qcom_iommu_attach_dev,
0ae349a0f3 Rob Clark    2017-08-09  590  	.detach_dev	= qcom_iommu_detach_dev,
0ae349a0f3 Rob Clark    2017-08-09  591  	.map		= qcom_iommu_map,
0ae349a0f3 Rob Clark    2017-08-09 @592  	.unmap		= qcom_iommu_unmap,
0ae349a0f3 Rob Clark    2017-08-09  593  	.map_sg		= default_iommu_map_sg,
4d689b6194 Robin Murphy 2017-09-28  594  	.flush_iotlb_all = qcom_iommu_iotlb_sync,
4d689b6194 Robin Murphy 2017-09-28  595  	.iotlb_sync	= qcom_iommu_iotlb_sync,
0ae349a0f3 Rob Clark    2017-08-09  596  	.iova_to_phys	= qcom_iommu_iova_to_phys,
0ae349a0f3 Rob Clark    2017-08-09  597  	.add_device	= qcom_iommu_add_device,
0ae349a0f3 Rob Clark    2017-08-09  598  	.remove_device	= qcom_iommu_remove_device,
0ae349a0f3 Rob Clark    2017-08-09  599  	.device_group	= generic_device_group,
0ae349a0f3 Rob Clark    2017-08-09  600  	.of_xlate	= qcom_iommu_of_xlate,
0ae349a0f3 Rob Clark    2017-08-09  601  	.pgsize_bitmap	= SZ_4K | SZ_64K | SZ_1M | SZ_16M,
0ae349a0f3 Rob Clark    2017-08-09  602  };
0ae349a0f3 Rob Clark    2017-08-09  603  

:::::: The code at line 592 was first introduced by commit
:::::: 0ae349a0f33fb040a2bc228fdc6d60111455feab iommu/qcom: Add qcom_iommu

:::::: TO: Rob Clark <robdclark@gmail.com>
:::::: CC: Joerg Roedel <jroedel@suse.de>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 53022 bytes --]

Re: [PATCH 1/2] iommu: Fix iommu_unmap and iommu_unmap_fast return type

[kbuild test robot]

Hi Suravee,

I love your patch! Perhaps something to improve:

[auto build test WARNING on iommu/next]
[also build test WARNING on v4.15 next-20180202]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Suravee-Suthikulpanit/iommu-Fix-iommu_unmap-and-iommu_unmap_fast-return-type/20180203-015316
base:   https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next
reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

>> drivers/iommu/qcom_iommu.c:592:27: sparse: incorrect type in initializer (different signedness) @@ expected long ( )( ... ) @@ got unsigned long ( )( ... ) @@
   drivers/iommu/qcom_iommu.c:592:27: expected long ( )( ... )
   drivers/iommu/qcom_iommu.c:592:27: got unsigned long ( )( ... )
   drivers/iommu/qcom_iommu.c:592:12: error: initialization from incompatible pointer type
    .unmap = qcom_iommu_unmap,
    ^~~~~~~~~~~~~~~~
   drivers/iommu/qcom_iommu.c:592:12: note: (near initialization for 'qcom_iommu_ops.unmap')
   cc1: some warnings being treated as errors

vim +592 drivers/iommu/qcom_iommu.c

0ae349a0f3 Rob Clark    2017-08-09  584  
0ae349a0f3 Rob Clark    2017-08-09  585  static const struct iommu_ops qcom_iommu_ops = {
0ae349a0f3 Rob Clark    2017-08-09  586  	.capable	= qcom_iommu_capable,
0ae349a0f3 Rob Clark    2017-08-09  587  	.domain_alloc	= qcom_iommu_domain_alloc,
0ae349a0f3 Rob Clark    2017-08-09  588  	.domain_free	= qcom_iommu_domain_free,
0ae349a0f3 Rob Clark    2017-08-09  589  	.attach_dev	= qcom_iommu_attach_dev,
0ae349a0f3 Rob Clark    2017-08-09  590  	.detach_dev	= qcom_iommu_detach_dev,
0ae349a0f3 Rob Clark    2017-08-09  591  	.map		= qcom_iommu_map,
0ae349a0f3 Rob Clark    2017-08-09 @592  	.unmap		= qcom_iommu_unmap,
0ae349a0f3 Rob Clark    2017-08-09  593  	.map_sg		= default_iommu_map_sg,
4d689b6194 Robin Murphy 2017-09-28  594  	.flush_iotlb_all = qcom_iommu_iotlb_sync,
4d689b6194 Robin Murphy 2017-09-28  595  	.iotlb_sync	= qcom_iommu_iotlb_sync,
0ae349a0f3 Rob Clark    2017-08-09  596  	.iova_to_phys	= qcom_iommu_iova_to_phys,
0ae349a0f3 Rob Clark    2017-08-09  597  	.add_device	= qcom_iommu_add_device,
0ae349a0f3 Rob Clark    2017-08-09  598  	.remove_device	= qcom_iommu_remove_device,
0ae349a0f3 Rob Clark    2017-08-09  599  	.device_group	= generic_device_group,
0ae349a0f3 Rob Clark    2017-08-09  600  	.of_xlate	= qcom_iommu_of_xlate,
0ae349a0f3 Rob Clark    2017-08-09  601  	.pgsize_bitmap	= SZ_4K | SZ_64K | SZ_1M | SZ_16M,
0ae349a0f3 Rob Clark    2017-08-09  602  };
0ae349a0f3 Rob Clark    2017-08-09  603  

:::::: The code at line 592 was first introduced by commit
:::::: 0ae349a0f33fb040a2bc228fdc6d60111455feab iommu/qcom: Add qcom_iommu

:::::: TO: Rob Clark <robdclark@gmail.com>
:::::: CC: Joerg Roedel <jroedel@suse.de>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[PATCH 2/2] vfio/type1: Add iommu_unmap error check when vfio_unmap_unpin

[Suravee Suthikulpanit]

Besides zero check the number of unmapped page, also check
and handle iommu_unmap errors.

Cc: Alex Williamson <alex.williamson@redhat.com>
Cc: Joerg Roedel <joro@8bytes.org>
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
---
 drivers/vfio/vfio_iommu_type1.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index e30e29a..c580518 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -677,7 +677,8 @@ static long vfio_unmap_unpin(struct vfio_iommu *iommu, struct vfio_dma *dma,
 	}
 
 	while (iova < end) {
-		size_t unmapped, len;
+		ssize_t unmapped;
+		size_t len;
 		phys_addr_t phys, next;
 
 		phys = iommu_iova_to_phys(domain->domain, iova);
@@ -699,7 +700,7 @@ static long vfio_unmap_unpin(struct vfio_iommu *iommu, struct vfio_dma *dma,
 		}
 
 		unmapped = iommu_unmap(domain->domain, iova, len);
-		if (WARN_ON(!unmapped))
+		if (WARN_ON(unmapped <= 0))
 			break;
 
 		unlocked += vfio_unpin_pages_remote(dma, iova,
-- 
1.8.3.1