Patchwork [RFC,2/3] iommu/fsl: Add iommu domain attributes required by fsl PAMU driver.

login
register
mail settings
Submitter Sethi Varun-B16395
Date Sept. 19, 2012, 1:17 p.m.
Message ID <1348060632-12997-3-git-send-email-b16395@freescale.com>
Download mbox | patch
Permalink /patch/185036/
State Superseded
Headers show

Comments

Sethi Varun-B16395 - Sept. 19, 2012, 1:17 p.m.
From: Varun Sethi <Varun.Sethi@freescale.com>

Added the following domain attributes required by FSL PAMU driver:
1. Subwindows field added to the iommu domain geometry attribute.
2. Added new iommu stash attribute, which allows setting of the
   LIODN specific stash id parameter through IOMMU API.
3. Added an attribute for enabling/disabling DMA to a particular
   memory window.

Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com>
---
 include/linux/iommu.h |   30 ++++++++++++++++++++++++++++++
 1 files changed, 30 insertions(+), 0 deletions(-)
Kumar Gala - Sept. 19, 2012, 1:52 p.m.
On Sep 19, 2012, at 8:17 AM, <b16395@freescale.com> <b16395@freescale.com> wrote:

> From: Varun Sethi <Varun.Sethi@freescale.com>
> 
> Added the following domain attributes required by FSL PAMU driver:
> 1. Subwindows field added to the iommu domain geometry attribute.
> 2. Added new iommu stash attribute, which allows setting of the
>   LIODN specific stash id parameter through IOMMU API.
> 3. Added an attribute for enabling/disabling DMA to a particular
>   memory window.
> 
> Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com>
> ---
> include/linux/iommu.h |   30 ++++++++++++++++++++++++++++++
> 1 files changed, 30 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 7e83370..eaa40c6 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -44,6 +44,28 @@ struct iommu_domain_geometry {
> 	dma_addr_t aperture_start; /* First address that can be mapped    */
> 	dma_addr_t aperture_end;   /* Last address that can be mapped     */
> 	bool force_aperture;       /* DMA only allowed in mappable range? */
> +
> +	/* The subwindows field indicates number of DMA subwindows supported
> +	 * by the geometry. Following is the interpretation of
> +	 * values for this field:
> +	 * 0 : This implies that the supported geometry size is 1 MB
> +         * with each subwindow size being 4KB. Thus number of subwindows
> +	 * being = 1MB/4KB = 256.
> +	 * 1 : Only one DMA window i.e. no subwindows.
> +	 * value other than 0 or 1 would indicate actual number of subwindows.
> +	 */
> +	u32 subwindows;
> +};
> +
> +/* This attribute corresponds to IOMMUs capable of generating
> + * a stash transaction. A stash transaction is typically a
> + * hardware initiated prefetch of data from memory to cache.
> + * This attribute allows configuring stashig specific parameters
> + * in the IOMMU hardware.
> + */
> +struct iommu_stash_attribute {
> +	u32 	cpu;	/* cpu number */
> +	u32 	cache;	/* cache to stash to: L1,L2,L3 */

seems like this should be enum instead of u32 for cache

With enum being something like:

enum iommu_attr_stash_cache {
	IOMMU_ATTR_CACHE_L1,
	IOMMU_ATTR_CACHE_L2,
	IOMMU_ATTR_CACHE_L3,
};

> };
> 
> struct iommu_domain {
> @@ -60,6 +82,14 @@ struct iommu_domain {
> enum iommu_attr {
> 	DOMAIN_ATTR_MAX,
> 	DOMAIN_ATTR_GEOMETRY,
> +	/* Set the IOMMU hardware stashing
> +	 * parameters.
> +	 */
> +	DOMAIN_ATTR_STASH,
> +	/* Explicity enable/disable DMA for a
> +         * particular memory window.
> +         */
> +	DOMAIN_ATTR_ENABLE,
> };
> 
> #ifdef CONFIG_IOMMU_API
> -- 
> 1.7.4.1
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
Scott Wood - Sept. 20, 2012, 12:12 a.m.
On 09/19/2012 08:52:27 AM, Kumar Gala wrote:
> 
> On Sep 19, 2012, at 8:17 AM, <b16395@freescale.com>  
> <b16395@freescale.com> wrote:
> 
> > From: Varun Sethi <Varun.Sethi@freescale.com>
> >
> > Added the following domain attributes required by FSL PAMU driver:
> > 1. Subwindows field added to the iommu domain geometry attribute.
> > 2. Added new iommu stash attribute, which allows setting of the
> >   LIODN specific stash id parameter through IOMMU API.
> > 3. Added an attribute for enabling/disabling DMA to a particular
> >   memory window.
> >
> > Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com>
> > ---
> > include/linux/iommu.h |   30 ++++++++++++++++++++++++++++++
> > 1 files changed, 30 insertions(+), 0 deletions(-)
> >
> > diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> > index 7e83370..eaa40c6 100644
> > --- a/include/linux/iommu.h
> > +++ b/include/linux/iommu.h
> > @@ -44,6 +44,28 @@ struct iommu_domain_geometry {
> > 	dma_addr_t aperture_start; /* First address that can be  
> mapped    */
> > 	dma_addr_t aperture_end;   /* Last address that can be  
> mapped     */
> > 	bool force_aperture;       /* DMA only allowed in mappable  
> range? */
> > +
> > +	/* The subwindows field indicates number of DMA subwindows  
> supported
> > +	 * by the geometry. Following is the interpretation of
> > +	 * values for this field:
> > +	 * 0 : This implies that the supported geometry size is 1 MB
> > +         * with each subwindow size being 4KB. Thus number of  
> subwindows
> > +	 * being = 1MB/4KB = 256.
> > +	 * 1 : Only one DMA window i.e. no subwindows.
> > +	 * value other than 0 or 1 would indicate actual number of  
> subwindows.
> > +	 */
> > +	u32 subwindows;
> > +};
> > +
> > +/* This attribute corresponds to IOMMUs capable of generating
> > + * a stash transaction. A stash transaction is typically a
> > + * hardware initiated prefetch of data from memory to cache.
> > + * This attribute allows configuring stashig specific parameters
> > + * in the IOMMU hardware.
> > + */
> > +struct iommu_stash_attribute {
> > +	u32 	cpu;	/* cpu number */
> > +	u32 	cache;	/* cache to stash to: L1,L2,L3 */
> 
> seems like this should be enum instead of u32 for cache
> 
> With enum being something like:
> 
> enum iommu_attr_stash_cache {
> 	IOMMU_ATTR_CACHE_L1,
> 	IOMMU_ATTR_CACHE_L2,
> 	IOMMU_ATTR_CACHE_L3,
> };

Don't we want these structs to be usable via some VFIO ioctl?  In that  
case they need to use fixed size types.

-Scott
Sethi Varun-B16395 - Sept. 20, 2012, 9:46 a.m.
> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Thursday, September 20, 2012 5:42 AM
> To: Kumar Gala
> Cc: Sethi Varun-B16395; joerg.roedel@amd.com; iommu@lists.linux-
> foundation.org; linuxppc-dev@lists.ozlabs.org; linux-
> kernel@vger.kernel.org; Sethi Varun-B16395
> Subject: Re: [RFC][PATCH 2/3] iommu/fsl: Add iommu domain attributes
> required by fsl PAMU driver.
> 
> On 09/19/2012 08:52:27 AM, Kumar Gala wrote:
> >
> > On Sep 19, 2012, at 8:17 AM, <b16395@freescale.com>
> > <b16395@freescale.com> wrote:
> >
> > > From: Varun Sethi <Varun.Sethi@freescale.com>
> > >
> > > Added the following domain attributes required by FSL PAMU driver:
> > > 1. Subwindows field added to the iommu domain geometry attribute.
> > > 2. Added new iommu stash attribute, which allows setting of the
> > >   LIODN specific stash id parameter through IOMMU API.
> > > 3. Added an attribute for enabling/disabling DMA to a particular
> > >   memory window.
> > >
> > > Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com>
> > > ---
> > > include/linux/iommu.h |   30 ++++++++++++++++++++++++++++++
> > > 1 files changed, 30 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/include/linux/iommu.h b/include/linux/iommu.h index
> > > 7e83370..eaa40c6 100644
> > > --- a/include/linux/iommu.h
> > > +++ b/include/linux/iommu.h
> > > @@ -44,6 +44,28 @@ struct iommu_domain_geometry {
> > > 	dma_addr_t aperture_start; /* First address that can be
> > mapped    */
> > > 	dma_addr_t aperture_end;   /* Last address that can be
> > mapped     */
> > > 	bool force_aperture;       /* DMA only allowed in mappable
> > range? */
> > > +
> > > +	/* The subwindows field indicates number of DMA subwindows
> > supported
> > > +	 * by the geometry. Following is the interpretation of
> > > +	 * values for this field:
> > > +	 * 0 : This implies that the supported geometry size is 1 MB
> > > +         * with each subwindow size being 4KB. Thus number of
> > subwindows
> > > +	 * being = 1MB/4KB = 256.
> > > +	 * 1 : Only one DMA window i.e. no subwindows.
> > > +	 * value other than 0 or 1 would indicate actual number of
> > subwindows.
> > > +	 */
> > > +	u32 subwindows;
> > > +};
> > > +
> > > +/* This attribute corresponds to IOMMUs capable of generating
> > > + * a stash transaction. A stash transaction is typically a
> > > + * hardware initiated prefetch of data from memory to cache.
> > > + * This attribute allows configuring stashig specific parameters
> > > + * in the IOMMU hardware.
> > > + */
> > > +struct iommu_stash_attribute {
> > > +	u32 	cpu;	/* cpu number */
> > > +	u32 	cache;	/* cache to stash to: L1,L2,L3 */
> >
> > seems like this should be enum instead of u32 for cache
> >
> > With enum being something like:
> >
> > enum iommu_attr_stash_cache {
> > 	IOMMU_ATTR_CACHE_L1,
> > 	IOMMU_ATTR_CACHE_L2,
> > 	IOMMU_ATTR_CACHE_L3,
> > };
> 
> Don't we want these structs to be usable via some VFIO ioctl?  In that
> case they need to use fixed size types.
> 
Yes, this would be usable via vfio ioctl. But, then the caller should be
aware of supported stash targets. May be I should add an interface for the caller,
to query supported stash targets.

-Varun
Kumar Gala - Sept. 20, 2012, 1:19 p.m.
On Sep 20, 2012, at 4:46 AM, Sethi Varun-B16395 wrote:

> 
> 
>> -----Original Message-----
>> From: Wood Scott-B07421
>> Sent: Thursday, September 20, 2012 5:42 AM
>> To: Kumar Gala
>> Cc: Sethi Varun-B16395; joerg.roedel@amd.com; iommu@lists.linux-
>> foundation.org; linuxppc-dev@lists.ozlabs.org; linux-
>> kernel@vger.kernel.org; Sethi Varun-B16395
>> Subject: Re: [RFC][PATCH 2/3] iommu/fsl: Add iommu domain attributes
>> required by fsl PAMU driver.
>> 
>> On 09/19/2012 08:52:27 AM, Kumar Gala wrote:
>>> 
>>> On Sep 19, 2012, at 8:17 AM, <b16395@freescale.com>
>>> <b16395@freescale.com> wrote:
>>> 
>>>> From: Varun Sethi <Varun.Sethi@freescale.com>
>>>> 
>>>> Added the following domain attributes required by FSL PAMU driver:
>>>> 1. Subwindows field added to the iommu domain geometry attribute.
>>>> 2. Added new iommu stash attribute, which allows setting of the
>>>>  LIODN specific stash id parameter through IOMMU API.
>>>> 3. Added an attribute for enabling/disabling DMA to a particular
>>>>  memory window.
>>>> 
>>>> Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com>
>>>> ---
>>>> include/linux/iommu.h |   30 ++++++++++++++++++++++++++++++
>>>> 1 files changed, 30 insertions(+), 0 deletions(-)
>>>> 
>>>> diff --git a/include/linux/iommu.h b/include/linux/iommu.h index
>>>> 7e83370..eaa40c6 100644
>>>> --- a/include/linux/iommu.h
>>>> +++ b/include/linux/iommu.h
>>>> @@ -44,6 +44,28 @@ struct iommu_domain_geometry {
>>>> 	dma_addr_t aperture_start; /* First address that can be
>>> mapped    */
>>>> 	dma_addr_t aperture_end;   /* Last address that can be
>>> mapped     */
>>>> 	bool force_aperture;       /* DMA only allowed in mappable
>>> range? */
>>>> +
>>>> +	/* The subwindows field indicates number of DMA subwindows
>>> supported
>>>> +	 * by the geometry. Following is the interpretation of
>>>> +	 * values for this field:
>>>> +	 * 0 : This implies that the supported geometry size is 1 MB
>>>> +         * with each subwindow size being 4KB. Thus number of
>>> subwindows
>>>> +	 * being = 1MB/4KB = 256.
>>>> +	 * 1 : Only one DMA window i.e. no subwindows.
>>>> +	 * value other than 0 or 1 would indicate actual number of
>>> subwindows.
>>>> +	 */
>>>> +	u32 subwindows;
>>>> +};
>>>> +
>>>> +/* This attribute corresponds to IOMMUs capable of generating
>>>> + * a stash transaction. A stash transaction is typically a
>>>> + * hardware initiated prefetch of data from memory to cache.
>>>> + * This attribute allows configuring stashig specific parameters
>>>> + * in the IOMMU hardware.
>>>> + */
>>>> +struct iommu_stash_attribute {
>>>> +	u32 	cpu;	/* cpu number */
>>>> +	u32 	cache;	/* cache to stash to: L1,L2,L3 */
>>> 
>>> seems like this should be enum instead of u32 for cache
>>> 
>>> With enum being something like:
>>> 
>>> enum iommu_attr_stash_cache {
>>> 	IOMMU_ATTR_CACHE_L1,
>>> 	IOMMU_ATTR_CACHE_L2,
>>> 	IOMMU_ATTR_CACHE_L3,
>>> };
>> 
>> Don't we want these structs to be usable via some VFIO ioctl?  In that
>> case they need to use fixed size types.
>> 
> Yes, this would be usable via vfio ioctl. But, then the caller should be
> aware of supported stash targets. May be I should add an interface for the caller,
> to query supported stash targets.

Guess the caller probably knows, but thinking we should move the #defines for valid values into this file out of pamu specific files.

- k

Patch

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 7e83370..eaa40c6 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -44,6 +44,28 @@  struct iommu_domain_geometry {
 	dma_addr_t aperture_start; /* First address that can be mapped    */
 	dma_addr_t aperture_end;   /* Last address that can be mapped     */
 	bool force_aperture;       /* DMA only allowed in mappable range? */
+
+	/* The subwindows field indicates number of DMA subwindows supported
+	 * by the geometry. Following is the interpretation of
+	 * values for this field:
+	 * 0 : This implies that the supported geometry size is 1 MB
+         * with each subwindow size being 4KB. Thus number of subwindows
+	 * being = 1MB/4KB = 256.
+	 * 1 : Only one DMA window i.e. no subwindows.
+	 * value other than 0 or 1 would indicate actual number of subwindows.
+	 */
+	u32 subwindows;
+};
+
+/* This attribute corresponds to IOMMUs capable of generating
+ * a stash transaction. A stash transaction is typically a
+ * hardware initiated prefetch of data from memory to cache.
+ * This attribute allows configuring stashig specific parameters
+ * in the IOMMU hardware.
+ */
+struct iommu_stash_attribute {
+	u32 	cpu;	/* cpu number */
+	u32 	cache;	/* cache to stash to: L1,L2,L3 */
 };
 
 struct iommu_domain {
@@ -60,6 +82,14 @@  struct iommu_domain {
 enum iommu_attr {
 	DOMAIN_ATTR_MAX,
 	DOMAIN_ATTR_GEOMETRY,
+	/* Set the IOMMU hardware stashing
+	 * parameters.
+	 */
+	DOMAIN_ATTR_STASH,
+	/* Explicity enable/disable DMA for a
+         * particular memory window.
+         */
+	DOMAIN_ATTR_ENABLE,
 };
 
 #ifdef CONFIG_IOMMU_API