Patchwork [1/6,v8] iommu/fsl: Store iommu domain information pointer in archdata.

login
register
mail settings
Submitter Varun Sethi
Date Feb. 18, 2013, 12:52 p.m.
Message ID <1361191939-21260-2-git-send-email-Varun.Sethi@freescale.com>
Download mbox | patch
Permalink /patch/221374/
State Superseded
Delegated to: Kumar Gala
Headers show

Comments

Varun Sethi - Feb. 18, 2013, 12:52 p.m.
Add a new field in the device (powerpc) archdata structure for storing iommu domain
information pointer. This pointer is stored when the device is attached to a particular
domain.


Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com>
---
- no change.
 arch/powerpc/include/asm/device.h |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
Joerg Roedel - Feb. 27, 2013, 11:30 a.m.
On Mon, Feb 18, 2013 at 06:22:14PM +0530, Varun Sethi wrote:
> Add a new field in the device (powerpc) archdata structure for storing iommu domain
> information pointer. This pointer is stored when the device is attached to a particular
> domain.
> 
> 
> Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com>
> ---
> - no change.
>  arch/powerpc/include/asm/device.h |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/device.h b/arch/powerpc/include/asm/device.h
> index 77e97dd..6dc79fe 100644
> --- a/arch/powerpc/include/asm/device.h
> +++ b/arch/powerpc/include/asm/device.h
> @@ -28,6 +28,10 @@ struct dev_archdata {
>  		void		*iommu_table_base;
>  	} dma_data;
>  
> +	/* IOMMU domain information pointer. This would be set
> +	 * when this device is attached to an iommu_domain.
> +	 */
> +	void			*iommu_domain;

Please Cc the PowerPC Maintainers on this, so that they can have a look
at it. This also must be put this into an #ifdef CONFIG_IOMMU_API.


	Joerg
Sethi Varun-B16395 - Feb. 27, 2013, 12:04 p.m.
Hi Kumar,Ben,
I am implementing the Freescale PAMU (IOMMU) driver using the Linux IOMMU API. In this particular patch, I have added a new field to dev_archdata structure to store the dma domain information.
This field is updated whenever we attach a device to an iommu domain.

Regards
Varun

> -----Original Message-----
> From: Joerg Roedel [mailto:joro@8bytes.org]
> Sent: Wednesday, February 27, 2013 5:01 PM
> To: Sethi Varun-B16395
> Cc: iommu@lists.linux-foundation.org; linuxppc-dev@lists.ozlabs.org;
> linux-kernel@vger.kernel.org; Wood Scott-B07421; Yoder Stuart-B08248
> Subject: Re: [PATCH 1/6 v8] iommu/fsl: Store iommu domain information
> pointer in archdata.
> 
> On Mon, Feb 18, 2013 at 06:22:14PM +0530, Varun Sethi wrote:
> > Add a new field in the device (powerpc) archdata structure for storing
> > iommu domain information pointer. This pointer is stored when the
> > device is attached to a particular domain.
> >
> >
> > Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com>
> > ---
> > - no change.
> >  arch/powerpc/include/asm/device.h |    4 ++++
> >  1 files changed, 4 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/device.h
> > b/arch/powerpc/include/asm/device.h
> > index 77e97dd..6dc79fe 100644
> > --- a/arch/powerpc/include/asm/device.h
> > +++ b/arch/powerpc/include/asm/device.h
> > @@ -28,6 +28,10 @@ struct dev_archdata {
> >  		void		*iommu_table_base;
> >  	} dma_data;
> >
> > +	/* IOMMU domain information pointer. This would be set
> > +	 * when this device is attached to an iommu_domain.
> > +	 */
> > +	void			*iommu_domain;
> 
> Please Cc the PowerPC Maintainers on this, so that they can have a look
> at it. This also must be put this into an #ifdef CONFIG_IOMMU_API.
> 
> 
> 	Joerg
> 
>
Kumar Gala - Feb. 28, 2013, 3:51 p.m.
On Feb 27, 2013, at 6:04 AM, Sethi Varun-B16395 wrote:

> Hi Kumar,Ben,
> I am implementing the Freescale PAMU (IOMMU) driver using the Linux IOMMU API. In this particular patch, I have added a new field to dev_archdata structure to store the dma domain information.
> This field is updated whenever we attach a device to an iommu domain.
> 
> Regards
> Varun

Would be good to see if this overlaps with Alexey's work for IOMMU driver for powernv.

- k

> 
>> -----Original Message-----
>> From: Joerg Roedel [mailto:joro@8bytes.org]
>> Sent: Wednesday, February 27, 2013 5:01 PM
>> To: Sethi Varun-B16395
>> Cc: iommu@lists.linux-foundation.org; linuxppc-dev@lists.ozlabs.org;
>> linux-kernel@vger.kernel.org; Wood Scott-B07421; Yoder Stuart-B08248
>> Subject: Re: [PATCH 1/6 v8] iommu/fsl: Store iommu domain information
>> pointer in archdata.
>> 
>> On Mon, Feb 18, 2013 at 06:22:14PM +0530, Varun Sethi wrote:
>>> Add a new field in the device (powerpc) archdata structure for storing
>>> iommu domain information pointer. This pointer is stored when the
>>> device is attached to a particular domain.
>>> 
>>> 
>>> Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com>
>>> ---
>>> - no change.
>>> arch/powerpc/include/asm/device.h |    4 ++++
>>> 1 files changed, 4 insertions(+), 0 deletions(-)
>>> 
>>> diff --git a/arch/powerpc/include/asm/device.h
>>> b/arch/powerpc/include/asm/device.h
>>> index 77e97dd..6dc79fe 100644
>>> --- a/arch/powerpc/include/asm/device.h
>>> +++ b/arch/powerpc/include/asm/device.h
>>> @@ -28,6 +28,10 @@ struct dev_archdata {
>>> 		void		*iommu_table_base;
>>> 	} dma_data;
>>> 
>>> +	/* IOMMU domain information pointer. This would be set
>>> +	 * when this device is attached to an iommu_domain.
>>> +	 */
>>> +	void			*iommu_domain;
>> 
>> Please Cc the PowerPC Maintainers on this, so that they can have a look
>> at it. This also must be put this into an #ifdef CONFIG_IOMMU_API.
>> 
>> 
>> 	Joerg
>> 
>> 
> 
> 
> --
> 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/
Alexey Kardashevskiy - March 1, 2013, 1:24 a.m.
Hi!

On POWERNV we use only the part of IOMMU API which handles devices and 
groups. We do not use IOMMU domains as VFIO containers do everything we 
need for VFIO and we do not implement iommu_ops as it is not very relevant 
to our architecture (does not give dma window properties, etc).

So your work does not overlap with my work :)


On 01/03/13 02:51, Kumar Gala wrote:
>
> On Feb 27, 2013, at 6:04 AM, Sethi Varun-B16395 wrote:
>
>> Hi Kumar,Ben,
>> I am implementing the Freescale PAMU (IOMMU) driver using the Linux IOMMU API. In this particular patch, I have added a new field to dev_archdata structure to store the dma domain information.
>> This field is updated whenever we attach a device to an iommu domain.
>>
>> Regards
>> Varun
>
> Would be good to see if this overlaps with Alexey's work for IOMMU driver for powernv.
>
> - k
>
>>
>>> -----Original Message-----
>>> From: Joerg Roedel [mailto:joro@8bytes.org]
>>> Sent: Wednesday, February 27, 2013 5:01 PM
>>> To: Sethi Varun-B16395
>>> Cc: iommu@lists.linux-foundation.org; linuxppc-dev@lists.ozlabs.org;
>>> linux-kernel@vger.kernel.org; Wood Scott-B07421; Yoder Stuart-B08248
>>> Subject: Re: [PATCH 1/6 v8] iommu/fsl: Store iommu domain information
>>> pointer in archdata.
>>>
>>> On Mon, Feb 18, 2013 at 06:22:14PM +0530, Varun Sethi wrote:
>>>> Add a new field in the device (powerpc) archdata structure for storing
>>>> iommu domain information pointer. This pointer is stored when the
>>>> device is attached to a particular domain.
>>>>
>>>>
>>>> Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com>
>>>> ---
>>>> - no change.
>>>> arch/powerpc/include/asm/device.h |    4 ++++
>>>> 1 files changed, 4 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/arch/powerpc/include/asm/device.h
>>>> b/arch/powerpc/include/asm/device.h
>>>> index 77e97dd..6dc79fe 100644
>>>> --- a/arch/powerpc/include/asm/device.h
>>>> +++ b/arch/powerpc/include/asm/device.h
>>>> @@ -28,6 +28,10 @@ struct dev_archdata {
>>>> 		void		*iommu_table_base;
>>>> 	} dma_data;
>>>>
>>>> +	/* IOMMU domain information pointer. This would be set
>>>> +	 * when this device is attached to an iommu_domain.
>>>> +	 */
>>>> +	void			*iommu_domain;
>>>
>>> Please Cc the PowerPC Maintainers on this, so that they can have a look
>>> at it. This also must be put this into an #ifdef CONFIG_IOMMU_API.
Sethi Varun-B16395 - March 1, 2013, 8:55 a.m.
Thanks for the clarification Alexey.

Kumar,
We are using this new field (for PAMU) to store the iommu domain (for iommu API) information for a device.

Regards
Varun

> -----Original Message-----
> From: Alexey Kardashevskiy [mailto:aik@ozlabs.ru]
> Sent: Friday, March 01, 2013 6:55 AM
> To: Kumar Gala
> Cc: Sethi Varun-B16395; Benjamin Herrenschmidt; iommu@lists.linux-
> foundation.org; linuxppc-dev@lists.ozlabs.org list; linux-
> kernel@vger.kernel.org list; Wood Scott-B07421; Yoder Stuart-B08248;
> Joerg Roedel; Paul Mackerras; David Gibson; Alex Williamson
> Subject: Re: [PATCH 1/6 v8] iommu/fsl: Store iommu domain information
> pointer in archdata.
> 
> Hi!
> 
> On POWERNV we use only the part of IOMMU API which handles devices and
> groups. We do not use IOMMU domains as VFIO containers do everything we
> need for VFIO and we do not implement iommu_ops as it is not very
> relevant to our architecture (does not give dma window properties, etc).
> 
> So your work does not overlap with my work :)
> 
> 
> On 01/03/13 02:51, Kumar Gala wrote:
> >
> > On Feb 27, 2013, at 6:04 AM, Sethi Varun-B16395 wrote:
> >
> >> Hi Kumar,Ben,
> >> I am implementing the Freescale PAMU (IOMMU) driver using the Linux
> IOMMU API. In this particular patch, I have added a new field to
> dev_archdata structure to store the dma domain information.
> >> This field is updated whenever we attach a device to an iommu domain.
> >>
> >> Regards
> >> Varun
> >
> > Would be good to see if this overlaps with Alexey's work for IOMMU
> driver for powernv.
> >
> > - k
> >
> >>
> >>> -----Original Message-----
> >>> From: Joerg Roedel [mailto:joro@8bytes.org]
> >>> Sent: Wednesday, February 27, 2013 5:01 PM
> >>> To: Sethi Varun-B16395
> >>> Cc: iommu@lists.linux-foundation.org; linuxppc-dev@lists.ozlabs.org;
> >>> linux-kernel@vger.kernel.org; Wood Scott-B07421; Yoder Stuart-B08248
> >>> Subject: Re: [PATCH 1/6 v8] iommu/fsl: Store iommu domain
> >>> information pointer in archdata.
> >>>
> >>> On Mon, Feb 18, 2013 at 06:22:14PM +0530, Varun Sethi wrote:
> >>>> Add a new field in the device (powerpc) archdata structure for
> >>>> storing iommu domain information pointer. This pointer is stored
> >>>> when the device is attached to a particular domain.
> >>>>
> >>>>
> >>>> Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com>
> >>>> ---
> >>>> - no change.
> >>>> arch/powerpc/include/asm/device.h |    4 ++++
> >>>> 1 files changed, 4 insertions(+), 0 deletions(-)
> >>>>
> >>>> diff --git a/arch/powerpc/include/asm/device.h
> >>>> b/arch/powerpc/include/asm/device.h
> >>>> index 77e97dd..6dc79fe 100644
> >>>> --- a/arch/powerpc/include/asm/device.h
> >>>> +++ b/arch/powerpc/include/asm/device.h
> >>>> @@ -28,6 +28,10 @@ struct dev_archdata {
> >>>> 		void		*iommu_table_base;
> >>>> 	} dma_data;
> >>>>
> >>>> +	/* IOMMU domain information pointer. This would be set
> >>>> +	 * when this device is attached to an iommu_domain.
> >>>> +	 */
> >>>> +	void			*iommu_domain;
> >>>
> >>> Please Cc the PowerPC Maintainers on this, so that they can have a
> >>> look at it. This also must be put this into an #ifdef
> CONFIG_IOMMU_API.
> 
> 
> --
> Alexey
Alexey Kardashevskiy - March 1, 2013, 10:07 a.m.
btw the device struct already has a pointer to its iommu_group, and the 
iommu_group struct itself has a pointer void *iommu_data which you could 
use for anything you want (iommu_group_get_iommudata(), 
iommu_group_set_iommudata()).

By design you are expected to add iommu groups to a domain but not devices 
so I am not so sure that you really need a pointer to domain in the device 
struct.


On 01/03/13 19:55, Sethi Varun-B16395 wrote:
> Thanks for the clarification Alexey.
>
> Kumar,
> We are using this new field (for PAMU) to store the iommu domain (for iommu API) information for a device.
>
> Regards
> Varun
>
>> -----Original Message-----
>> From: Alexey Kardashevskiy [mailto:aik@ozlabs.ru]
>> Sent: Friday, March 01, 2013 6:55 AM
>> To: Kumar Gala
>> Cc: Sethi Varun-B16395; Benjamin Herrenschmidt; iommu@lists.linux-
>> foundation.org; linuxppc-dev@lists.ozlabs.org list; linux-
>> kernel@vger.kernel.org list; Wood Scott-B07421; Yoder Stuart-B08248;
>> Joerg Roedel; Paul Mackerras; David Gibson; Alex Williamson
>> Subject: Re: [PATCH 1/6 v8] iommu/fsl: Store iommu domain information
>> pointer in archdata.
>>
>> Hi!
>>
>> On POWERNV we use only the part of IOMMU API which handles devices and
>> groups. We do not use IOMMU domains as VFIO containers do everything we
>> need for VFIO and we do not implement iommu_ops as it is not very
>> relevant to our architecture (does not give dma window properties, etc).
>>
>> So your work does not overlap with my work :)
>>
>>
>> On 01/03/13 02:51, Kumar Gala wrote:
>>>
>>> On Feb 27, 2013, at 6:04 AM, Sethi Varun-B16395 wrote:
>>>
>>>> Hi Kumar,Ben,
>>>> I am implementing the Freescale PAMU (IOMMU) driver using the Linux
>> IOMMU API. In this particular patch, I have added a new field to
>> dev_archdata structure to store the dma domain information.
>>>> This field is updated whenever we attach a device to an iommu domain.
>>>>
>>>> Regards
>>>> Varun
>>>
>>> Would be good to see if this overlaps with Alexey's work for IOMMU
>> driver for powernv.
>>>
>>> - k
>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Joerg Roedel [mailto:joro@8bytes.org]
>>>>> Sent: Wednesday, February 27, 2013 5:01 PM
>>>>> To: Sethi Varun-B16395
>>>>> Cc: iommu@lists.linux-foundation.org; linuxppc-dev@lists.ozlabs.org;
>>>>> linux-kernel@vger.kernel.org; Wood Scott-B07421; Yoder Stuart-B08248
>>>>> Subject: Re: [PATCH 1/6 v8] iommu/fsl: Store iommu domain
>>>>> information pointer in archdata.
>>>>>
>>>>> On Mon, Feb 18, 2013 at 06:22:14PM +0530, Varun Sethi wrote:
>>>>>> Add a new field in the device (powerpc) archdata structure for
>>>>>> storing iommu domain information pointer. This pointer is stored
>>>>>> when the device is attached to a particular domain.
>>>>>>
>>>>>>
>>>>>> Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com>
>>>>>> ---
>>>>>> - no change.
>>>>>> arch/powerpc/include/asm/device.h |    4 ++++
>>>>>> 1 files changed, 4 insertions(+), 0 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/powerpc/include/asm/device.h
>>>>>> b/arch/powerpc/include/asm/device.h
>>>>>> index 77e97dd..6dc79fe 100644
>>>>>> --- a/arch/powerpc/include/asm/device.h
>>>>>> +++ b/arch/powerpc/include/asm/device.h
>>>>>> @@ -28,6 +28,10 @@ struct dev_archdata {
>>>>>> 		void		*iommu_table_base;
>>>>>> 	} dma_data;
>>>>>>
>>>>>> +	/* IOMMU domain information pointer. This would be set
>>>>>> +	 * when this device is attached to an iommu_domain.
>>>>>> +	 */
>>>>>> +	void			*iommu_domain;
>>>>>
>>>>> Please Cc the PowerPC Maintainers on this, so that they can have a
>>>>> look at it. This also must be put this into an #ifdef
>> CONFIG_IOMMU_API.
Yoder Stuart-B08248 - March 1, 2013, 4:21 p.m.
> -----Original Message-----
> From: Alexey Kardashevskiy [mailto:aik@ozlabs.ru]
> Sent: Friday, March 01, 2013 4:07 AM
> To: Sethi Varun-B16395
> Cc: Kumar Gala; Benjamin Herrenschmidt; iommu@lists.linux-foundation.org; linuxppc-dev@lists.ozlabs.org
> list; linux-kernel@vger.kernel.org list; Wood Scott-B07421; Yoder Stuart-B08248; Joerg Roedel; Paul
> Mackerras; David Gibson; Alex Williamson
> Subject: Re: [PATCH 1/6 v8] iommu/fsl: Store iommu domain information pointer in archdata.
> 
> btw the device struct already has a pointer to its iommu_group, and the
> iommu_group struct itself has a pointer void *iommu_data which you could
> use for anything you want (iommu_group_get_iommudata(),
> iommu_group_set_iommudata()).
>
> By design you are expected to add iommu groups to a domain but not devices
> so I am not so sure that you really need a pointer to domain in the device
> struct.

Well, at the lowest level the IOMMU API does attach devices to domains-- i.e.
API attach_dev().  So, it seems to conceptually make sense to have a ptr
from the device to the associated domain.  When you implement attach_dev()
you need to be able to see whether the device is already attached to
a domain.   Adding a couple of levels of indirection...from device to
group to domain...doesn't seems to make things simpler or better IMHO.

x86 keeps a pointer to the domain in device archdata and since there is
a direct correlation between a device and domain I'd rather see it
where this patch has it.

Stuart
Sethi Varun-B16395 - March 4, 2013, 6:35 a.m.
> -----Original Message-----
> From: Yoder Stuart-B08248
> Sent: Friday, March 01, 2013 9:52 PM
> To: Alexey Kardashevskiy; Sethi Varun-B16395
> Cc: Kumar Gala; Benjamin Herrenschmidt; iommu@lists.linux-foundation.org;
> linuxppc-dev@lists.ozlabs.org list; linux-kernel@vger.kernel.org list;
> Wood Scott-B07421; Joerg Roedel; Paul Mackerras; David Gibson; Alex
> Williamson
> Subject: RE: [PATCH 1/6 v8] iommu/fsl: Store iommu domain information
> pointer in archdata.
> 
> 
> 
> > -----Original Message-----
> > From: Alexey Kardashevskiy [mailto:aik@ozlabs.ru]
> > Sent: Friday, March 01, 2013 4:07 AM
> > To: Sethi Varun-B16395
> > Cc: Kumar Gala; Benjamin Herrenschmidt;
> > iommu@lists.linux-foundation.org; linuxppc-dev@lists.ozlabs.org list;
> > linux-kernel@vger.kernel.org list; Wood Scott-B07421; Yoder
> > Stuart-B08248; Joerg Roedel; Paul Mackerras; David Gibson; Alex
> > Williamson
> > Subject: Re: [PATCH 1/6 v8] iommu/fsl: Store iommu domain information
> pointer in archdata.
> >
> > btw the device struct already has a pointer to its iommu_group, and
> > the iommu_group struct itself has a pointer void *iommu_data which you
> > could use for anything you want (iommu_group_get_iommudata(),
> > iommu_group_set_iommudata()).
> >
> > By design you are expected to add iommu groups to a domain but not
> > devices so I am not so sure that you really need a pointer to domain
> > in the device struct.
> 
> Well, at the lowest level the IOMMU API does attach devices to domains--
> i.e.
> API attach_dev().  So, it seems to conceptually make sense to have a ptr
> from the device to the associated domain.  When you implement
> attach_dev() you need to be able to see whether the device is already
> attached to
> a domain.   Adding a couple of levels of indirection...from device to
> group to domain...doesn't seems to make things simpler or better IMHO.
> 
> x86 keeps a pointer to the domain in device archdata and since there is a
> direct correlation between a device and domain I'd rather see it where
> this patch has it.
> 

As Stuart stated this allows us to maintain the device <-> domain relationship at the lowest level.

-Varun

Patch

diff --git a/arch/powerpc/include/asm/device.h b/arch/powerpc/include/asm/device.h
index 77e97dd..6dc79fe 100644
--- a/arch/powerpc/include/asm/device.h
+++ b/arch/powerpc/include/asm/device.h
@@ -28,6 +28,10 @@  struct dev_archdata {
 		void		*iommu_table_base;
 	} dma_data;
 
+	/* IOMMU domain information pointer. This would be set
+	 * when this device is attached to an iommu_domain.
+	 */
+	void			*iommu_domain;
 #ifdef CONFIG_SWIOTLB
 	dma_addr_t		max_direct_dma_addr;
 #endif