RE: [RFC PATCH v6 1/5] iommu: add an exported function to get minimum page size for a domain

2019-06-16 Thread Yoshihiro Shimoda
Hi Robin,

> From: Robin Murphy, Sent: Friday, June 14, 2019 6:41 PM
> 
> On 13/06/2019 11:20, Yoshihiro Shimoda wrote:
> > This patch adds an exported function to get minimum page size for
> > a domain. This patch also modifies similar codes on the iommu.c.
> 
> Heh, seeing this gave me a genuine déjà vu moment...
> 
> ...but it turns out I actually *have* reviewed this patch before :)
> 
> https://lore.kernel.org/lkml/05eca601-0264-8141-ceeb-7ef7ad5d5...@arm.com/

Thank you for the information :)
I realized my patch should have taken care of the CONFIG_IPMMU_API=n case.

However, the latest patch series doesn't have such a patch. So, I'll keep this
my patch on next patch version.
https://lore.kernel.org/lkml/20190603011620.31999-1-baolu...@linux.intel.com/

Best regards,
Yoshihiro Shimoda

> Robin.
> 
> > Signed-off-by: Yoshihiro Shimoda 
> > ---
> >   drivers/iommu/iommu.c | 18 +++---
> >   include/linux/iommu.h |  1 +
> >   2 files changed, 16 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> > index 2a90638..7ed16af 100644
> > --- a/drivers/iommu/iommu.c
> > +++ b/drivers/iommu/iommu.c
> > @@ -280,6 +280,18 @@ iommu_insert_device_resv_regions(struct list_head 
> > *dev_resv_regions,
> > return ret;
> >   }
> >
> > +/**
> > + * iommu_get_minimum_page_size - get minimum page size for a domain
> > + * @domain: the domain
> > + *
> > + * Allow iommu driver to get a minimum page size for a domain.
> > + */
> > +unsigned long iommu_get_minimum_page_size(struct iommu_domain *domain)
> > +{
> > +   return 1UL << __ffs(domain->pgsize_bitmap);
> > +}
> > +EXPORT_SYMBOL_GPL(iommu_get_minimum_page_size);
> > +
> >   int iommu_get_group_resv_regions(struct iommu_group *group,
> >  struct list_head *head)
> >   {
> > @@ -558,7 +570,7 @@ static int iommu_group_create_direct_mappings(struct 
> > iommu_group *group,
> >
> > BUG_ON(!domain->pgsize_bitmap);
> >
> > -   pg_size = 1UL << __ffs(domain->pgsize_bitmap);
> > +   pg_size = iommu_get_minimum_page_size(domain);
> > INIT_LIST_HEAD();
> >
> > iommu_get_resv_regions(dev, );
> > @@ -1595,7 +1607,7 @@ int iommu_map(struct iommu_domain *domain, unsigned 
> > long iova,
> > return -EINVAL;
> >
> > /* find out the minimum page size supported */
> > -   min_pagesz = 1 << __ffs(domain->pgsize_bitmap);
> > +   min_pagesz = iommu_get_minimum_page_size(domain);
> >
> > /*
> >  * both the virtual address and the physical one, as well as
> > @@ -1655,7 +1667,7 @@ static size_t __iommu_unmap(struct iommu_domain 
> > *domain,
> > return 0;
> >
> > /* find out the minimum page size supported */
> > -   min_pagesz = 1 << __ffs(domain->pgsize_bitmap);
> > +   min_pagesz = iommu_get_minimum_page_size(domain);
> >
> > /*
> >  * The virtual address, as well as the size of the mapping, must be
> > diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> > index 91af22a..7e53b43 100644
> > --- a/include/linux/iommu.h
> > +++ b/include/linux/iommu.h
> > @@ -366,6 +366,7 @@ extern int iommu_request_dma_domain_for_dev(struct 
> > device *dev);
> >   extern struct iommu_resv_region *
> >   iommu_alloc_resv_region(phys_addr_t start, size_t length, int prot,
> > enum iommu_resv_type type);
> > +extern unsigned long iommu_get_minimum_page_size(struct iommu_domain 
> > *domain);
> >   extern int iommu_get_group_resv_regions(struct iommu_group *group,
> > struct list_head *head);
> >
> >


RE: [RFC PATCH v6 1/5] iommu: add an exported function to get minimum page size for a domain

2019-06-16 Thread Yoshihiro Shimoda
Hi Wolfram-san,

> From: Wolfram Sang, Sent: Friday, June 14, 2019 4:38 AM
> 
> On Thu, Jun 13, 2019 at 07:20:11PM +0900, Yoshihiro Shimoda wrote:
> > This patch adds an exported function to get minimum page size for
> > a domain. This patch also modifies similar codes on the iommu.c.
> >
> > Signed-off-by: Yoshihiro Shimoda 
> > ---
> >  drivers/iommu/iommu.c | 18 +++---
> >  include/linux/iommu.h |  1 +
> >  2 files changed, 16 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> > index 2a90638..7ed16af 100644
> > --- a/drivers/iommu/iommu.c
> > +++ b/drivers/iommu/iommu.c
> > @@ -280,6 +280,18 @@ iommu_insert_device_resv_regions(struct list_head 
> > *dev_resv_regions,
> > return ret;
> >  }
> >
> > +/**
> > + * iommu_get_minimum_page_size - get minimum page size for a domain
> > + * @domain: the domain
> > + *
> > + * Allow iommu driver to get a minimum page size for a domain.
> > + */
> > +unsigned long iommu_get_minimum_page_size(struct iommu_domain *domain)
> > +{
> > +   return 1UL << __ffs(domain->pgsize_bitmap);
> > +}
> > +EXPORT_SYMBOL_GPL(iommu_get_minimum_page_size);
> 
> What about making this a 'static inline' in the iommu header file? I'd
> think it is simple enough and would save us the EXPORT symbol.

Thank you for the review. I think so. I'll use 'static inline' instead of
EXPORT symbol.

Best regards,
Yoshihiro Shimoda



Re: [RFC PATCH v6 1/5] iommu: add an exported function to get minimum page size for a domain

2019-06-14 Thread Robin Murphy

On 13/06/2019 11:20, Yoshihiro Shimoda wrote:

This patch adds an exported function to get minimum page size for
a domain. This patch also modifies similar codes on the iommu.c.


Heh, seeing this gave me a genuine déjà vu moment...

...but it turns out I actually *have* reviewed this patch before :)

https://lore.kernel.org/lkml/05eca601-0264-8141-ceeb-7ef7ad5d5...@arm.com/

Robin.


Signed-off-by: Yoshihiro Shimoda 
---
  drivers/iommu/iommu.c | 18 +++---
  include/linux/iommu.h |  1 +
  2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 2a90638..7ed16af 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -280,6 +280,18 @@ iommu_insert_device_resv_regions(struct list_head 
*dev_resv_regions,
return ret;
  }
  
+/**

+ * iommu_get_minimum_page_size - get minimum page size for a domain
+ * @domain: the domain
+ *
+ * Allow iommu driver to get a minimum page size for a domain.
+ */
+unsigned long iommu_get_minimum_page_size(struct iommu_domain *domain)
+{
+   return 1UL << __ffs(domain->pgsize_bitmap);
+}
+EXPORT_SYMBOL_GPL(iommu_get_minimum_page_size);
+
  int iommu_get_group_resv_regions(struct iommu_group *group,
 struct list_head *head)
  {
@@ -558,7 +570,7 @@ static int iommu_group_create_direct_mappings(struct 
iommu_group *group,
  
  	BUG_ON(!domain->pgsize_bitmap);
  
-	pg_size = 1UL << __ffs(domain->pgsize_bitmap);

+   pg_size = iommu_get_minimum_page_size(domain);
INIT_LIST_HEAD();
  
  	iommu_get_resv_regions(dev, );

@@ -1595,7 +1607,7 @@ int iommu_map(struct iommu_domain *domain, unsigned long 
iova,
return -EINVAL;
  
  	/* find out the minimum page size supported */

-   min_pagesz = 1 << __ffs(domain->pgsize_bitmap);
+   min_pagesz = iommu_get_minimum_page_size(domain);
  
  	/*

 * both the virtual address and the physical one, as well as
@@ -1655,7 +1667,7 @@ static size_t __iommu_unmap(struct iommu_domain *domain,
return 0;
  
  	/* find out the minimum page size supported */

-   min_pagesz = 1 << __ffs(domain->pgsize_bitmap);
+   min_pagesz = iommu_get_minimum_page_size(domain);
  
  	/*

 * The virtual address, as well as the size of the mapping, must be
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 91af22a..7e53b43 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -366,6 +366,7 @@ extern int iommu_request_dma_domain_for_dev(struct device 
*dev);
  extern struct iommu_resv_region *
  iommu_alloc_resv_region(phys_addr_t start, size_t length, int prot,
enum iommu_resv_type type);
+extern unsigned long iommu_get_minimum_page_size(struct iommu_domain *domain);
  extern int iommu_get_group_resv_regions(struct iommu_group *group,
struct list_head *head);
  



Re: [RFC PATCH v6 1/5] iommu: add an exported function to get minimum page size for a domain

2019-06-14 Thread Christoph Hellwig
On Thu, Jun 13, 2019 at 09:37:59PM +0200, Wolfram Sang wrote:
> What about making this a 'static inline' in the iommu header file? I'd
> think it is simple enough and would save us the EXPORT symbol.

Agreed, this seems simple enought for an inline.


Re: [RFC PATCH v6 1/5] iommu: add an exported function to get minimum page size for a domain

2019-06-13 Thread Wolfram Sang
On Thu, Jun 13, 2019 at 07:20:11PM +0900, Yoshihiro Shimoda wrote:
> This patch adds an exported function to get minimum page size for
> a domain. This patch also modifies similar codes on the iommu.c.
> 
> Signed-off-by: Yoshihiro Shimoda 
> ---
>  drivers/iommu/iommu.c | 18 +++---
>  include/linux/iommu.h |  1 +
>  2 files changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 2a90638..7ed16af 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -280,6 +280,18 @@ iommu_insert_device_resv_regions(struct list_head 
> *dev_resv_regions,
>   return ret;
>  }
>  
> +/**
> + * iommu_get_minimum_page_size - get minimum page size for a domain
> + * @domain: the domain
> + *
> + * Allow iommu driver to get a minimum page size for a domain.
> + */
> +unsigned long iommu_get_minimum_page_size(struct iommu_domain *domain)
> +{
> + return 1UL << __ffs(domain->pgsize_bitmap);
> +}
> +EXPORT_SYMBOL_GPL(iommu_get_minimum_page_size);

What about making this a 'static inline' in the iommu header file? I'd
think it is simple enough and would save us the EXPORT symbol.



signature.asc
Description: PGP signature