Messages in this thread | | | Subject | Re: [PATCH 1/2] iommu: Fix iommu_unmap and iommu_unmap_fast return type | From | Robin Murphy <> | Date | Wed, 31 Jan 2018 18:02:47 +0000 |
| |
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; > } >
| |