[PATCH] iommu: Remove stub functions for !CONFIG_IOMMU_API

[PATCH] iommu: Remove stub functions for !CONFIG_IOMMU_API

[Joerg Roedel]

Remove most of the stub functions because they are only
allowed to use when CONFIG_IOMMU_API is set anyway. This
will catch missing 'select' entries in kconfig at compile
time already.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 include/linux/iommu.h |   52 +-----------------------------------------------
 1 files changed, 2 insertions(+), 50 deletions(-)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 9940319..296f41a 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -19,6 +19,8 @@
 #ifndef __LINUX_IOMMU_H
 #define __LINUX_IOMMU_H
 
+#ifdef CONFIG_IOMMU_API
+
 #include <linux/errno.h>
 
 #define IOMMU_READ	(1)
@@ -49,8 +51,6 @@ struct iommu_ops {
 			      unsigned long cap);
 };
 
-#ifdef CONFIG_IOMMU_API
-
 extern void register_iommu(struct iommu_ops *ops);
 extern bool iommu_found(void);
 extern struct iommu_domain *iommu_domain_alloc(void);
@@ -70,59 +70,11 @@ extern int iommu_domain_has_cap(struct iommu_domain *domain,
 
 #else /* CONFIG_IOMMU_API */
 
-static inline void register_iommu(struct iommu_ops *ops)
-{
-}
-
 static inline bool iommu_found(void)
 {
 	return false;
 }
 
-static inline struct iommu_domain *iommu_domain_alloc(void)
-{
-	return NULL;
-}
-
-static inline void iommu_domain_free(struct iommu_domain *domain)
-{
-}
-
-static inline int iommu_attach_device(struct iommu_domain *domain,
-				      struct device *dev)
-{
-	return -ENODEV;
-}
-
-static inline void iommu_detach_device(struct iommu_domain *domain,
-				       struct device *dev)
-{
-}
-
-static inline int iommu_map(struct iommu_domain *domain, unsigned long iova,
-			    phys_addr_t paddr, int gfp_order, int prot)
-{
-	return -ENODEV;
-}
-
-static inline int iommu_unmap(struct iommu_domain *domain, unsigned long iova,
-			      int gfp_order)
-{
-	return -ENODEV;
-}
-
-static inline phys_addr_t iommu_iova_to_phys(struct iommu_domain *domain,
-					     unsigned long iova)
-{
-	return 0;
-}
-
-static inline int domain_has_cap(struct iommu_domain *domain,
-				 unsigned long cap)
-{
-	return 0;
-}
-
 #endif /* CONFIG_IOMMU_API */
 
 #endif /* __LINUX_IOMMU_H */
-- 
1.7.4.1

Re: [PATCH] iommu: Remove stub functions for !CONFIG_IOMMU_API

[Ohad Ben-Cohen]

On Fri, Aug 26, 2011 at 4:20 PM, Joerg Roedel <joerg.roedel@amd.com> wrote:
> Remove most of the stub functions because they are only
> allowed to use when CONFIG_IOMMU_API is set anyway. This
> will catch missing 'select' entries in kconfig at compile
> time already.

I'm not sure we want this; think about a generic framework/driver that
uses the IOMMU API only if the underlying hardware has an IOMMU and
otherwise will skip calling the IOMMU API altogether (e.g. this is
where remoteproc is headed).

With this patch, such generic code will have to "select IOMMU_API"
unconditionally, or it won't build when the hardware doesn't have an
IOMMU (e.g. the DSP in at least several of the DaVinci SoC isn't
behind an IOMMU). But doing so is a bit wasteful if there's no IOMMU
hardware...

Re: [PATCH] iommu: Remove stub functions for !CONFIG_IOMMU_API

[Roedel, Joerg]

On Sat, Aug 27, 2011 at 07:56:39AM -0400, Ohad Ben-Cohen wrote:
> On Fri, Aug 26, 2011 at 4:20 PM, Joerg Roedel <joerg.roedel@amd.com> wrote:
> > Remove most of the stub functions because they are only
> > allowed to use when CONFIG_IOMMU_API is set anyway. This
> > will catch missing 'select' entries in kconfig at compile
> > time already.
> 
> I'm not sure we want this; think about a generic framework/driver that
> uses the IOMMU API only if the underlying hardware has an IOMMU and
> otherwise will skip calling the IOMMU API altogether (e.g. this is
> where remoteproc is headed).

CONFIG_IOMMU_API would just compile in drivers/iommu/iommu.c to provide
the base-functionality of the api. You don't need to select and IOMMU
driver in the first place if your board doesn't have one.

On the other side this change makes it easy for a developer to find
kconfig problems already at compile time when he/she uses iommu-api
functions without selecting the api.

> With this patch, such generic code will have to "select IOMMU_API"
> unconditionally, or it won't build when the hardware doesn't have an
> IOMMU (e.g. the DSP in at least several of the DaVinci SoC isn't
> behind an IOMMU). But doing so is a bit wasteful if there's no IOMMU
> hardware...

Isn't the abstraction that the drivers use the dma-api? There should be
an implementation that doesn't require the iommu-api for such devices,
no?

	Joerg

-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632

Re: [PATCH] iommu: Remove stub functions for !CONFIG_IOMMU_API

[Ohad Ben-Cohen]

On Mon, Aug 29, 2011 at 1:15 PM, Roedel, Joerg <Joerg.Roedel@amd.com> wrote:
> Isn't the abstraction that the drivers use the dma-api? There should be
> an implementation that doesn't require the iommu-api for such devices,
> no?

Before we boot a remote processor device we often need to map it to
some (specific) bus addresses (e.g. to allow the processor access to
certain peripheral devices), and we're doing it directly with the
IOMMU API.

Do you think this kind of functionality should be added to the DMA API as well ?

IIRC, it was suggested awhile ago but you supported using the IOMMU
API directly in this case.

Thanks,
Ohad.

Re: [PATCH] iommu: Remove stub functions for !CONFIG_IOMMU_API

[Roedel, Joerg]

On Mon, Aug 29, 2011 at 06:59:05AM -0400, Ohad Ben-Cohen wrote:
> On Mon, Aug 29, 2011 at 1:15 PM, Roedel, Joerg <Joerg.Roedel@amd.com> wrote:
> > Isn't the abstraction that the drivers use the dma-api? There should be
> > an implementation that doesn't require the iommu-api for such devices,
> > no?
> 
> Before we boot a remote processor device we often need to map it to
> some (specific) bus addresses (e.g. to allow the processor access to
> certain peripheral devices), and we're doing it directly with the
> IOMMU API.
> 
> Do you think this kind of functionality should be added to the DMA API as well ?
> 
> IIRC, it was suggested awhile ago but you supported using the IOMMU
> API directly in this case.

Right, when the system has an iommu then the iommu-api is the right
choice to do that. But how to you handle this on systems without an
iommu?

	Joerg


-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632

Re: [PATCH] iommu: Remove stub functions for !CONFIG_IOMMU_API

[Ohad Ben-Cohen]

On Mon, Aug 29, 2011 at 3:25 PM, Roedel, Joerg <Joerg.Roedel@amd.com> wrote:
> Right, when the system has an iommu then the iommu-api is the right
> choice to do that. But how to you handle this on systems without an
> iommu?

On iommu-less systems this is completely moot, as the remote processor
can access the bus directly, and thus doesn't need any iommu
configuration to take place before it boots.

So in case there is an iommu - we configure it appropriately, and if
there isn't, we just skip that part.

Re: [PATCH] iommu: Remove stub functions for !CONFIG_IOMMU_API

[Roedel, Joerg]

On Mon, Aug 29, 2011 at 08:55:40AM -0400, Ohad Ben-Cohen wrote:
> On Mon, Aug 29, 2011 at 3:25 PM, Roedel, Joerg <Joerg.Roedel@amd.com> wrote:
> > Right, when the system has an iommu then the iommu-api is the right
> > choice to do that. But how to you handle this on systems without an
> > iommu?
> 
> On iommu-less systems this is completely moot, as the remote processor
> can access the bus directly, and thus doesn't need any iommu
> configuration to take place before it boots.
> 
> So in case there is an iommu - we configure it appropriately, and if
> there isn't, we just skip that part.

So in this case you can skip this whole part when CONFIG_IOMMU_API is
disabled (just don't compile the code in that requires the iommu-api).
This saves you even more .text size, no?

	Joerg

-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632

Re: [PATCH] iommu: Remove stub functions for !CONFIG_IOMMU_API

[Ohad Ben-Cohen]

On Mon, Aug 29, 2011 at 4:05 PM, Roedel, Joerg <Joerg.Roedel@amd.com> wrote:
> So in this case you can skip this whole part when CONFIG_IOMMU_API is
> disabled

Sure, we can do something like this in the driver:

#ifdef CONFIG_IOMMU_API

int do_the_iommu_thing(..)
{
   ... call the IOMMU API as needed...
}

#else

int do_the_iommu_thing(..)
{
    return 0;
}

#endif

Essentially, this means implementing the !CONFIG_IOMMU_API stubs in the driver.

Possible of course, but I think it's a bit neater to have that in one
place, where all drivers can use. Similar to !CONFIG_HWSPINLOCK,
!CONFIG_DEBUG_FS, ...

> This saves you even more .text size, no?

Not sure; I expect the code to be compiled out today when the IOMMU
API is called and CONFIG_IOMMU_API is not set.

Re: [PATCH] iommu: Remove stub functions for !CONFIG_IOMMU_API

[Roedel, Joerg]

On Mon, Aug 29, 2011 at 09:21:54AM -0400, Ohad Ben-Cohen wrote:
> On Mon, Aug 29, 2011 at 4:05 PM, Roedel, Joerg <Joerg.Roedel@amd.com> wrote:
> > So in this case you can skip this whole part when CONFIG_IOMMU_API is
> > disabled
> 
> Sure, we can do something like this in the driver:
> 
> #ifdef CONFIG_IOMMU_API
> 
> int do_the_iommu_thing(..)
> {
>    ... call the IOMMU API as needed...
> }
> 
> #else
> 
> int do_the_iommu_thing(..)
> {
>     return 0;
> }
> 
> #endif
> 
> Essentially, this means implementing the !CONFIG_IOMMU_API stubs in the driver.

I expect only very few drivers need that. Most drivers (like KVM and the
upcoming VFIO) rely on the iommu-api and I expect most other drivers
using the directly will also rely on an iommu. So this change makes
still sense to me.

> Possible of course, but I think it's a bit neater to have that in one
> place, where all drivers can use. Similar to !CONFIG_HWSPINLOCK,
> !CONFIG_DEBUG_FS, ...
> 
> > This saves you even more .text size, no?
> 
> Not sure; I expect the code to be compiled out today when the IOMMU
> API is called and CONFIG_IOMMU_API is not set.

But you even save all code that would use the iommu-api in your driver.
This change forces you to compile out this code too leading to smaller
.text-size.

	Joerg

-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632

Re: [PATCH] iommu: Remove stub functions for !CONFIG_IOMMU_API

[Laura Abbott]

On Wed, August 31, 2011 6:03 am, Roedel, Joerg wrote:
> On Mon, Aug 29, 2011 at 09:21:54AM -0400, Ohad Ben-Cohen wrote:
>> On Mon, Aug 29, 2011 at 4:05 PM, Roedel, Joerg <Joerg.Roedel@amd.com>
>> wrote:
>> > So in this case you can skip this whole part when CONFIG_IOMMU_API is
>> > disabled
>>
>> Sure, we can do something like this in the driver:
>>
>> #ifdef CONFIG_IOMMU_API
>>
>> int do_the_iommu_thing(..)
>> {
>>    ... call the IOMMU API as needed...
>> }
>>
>> #else
>>
>> int do_the_iommu_thing(..)
>> {
>>     return 0;
>> }
>>
>> #endif
>>
>> Essentially, this means implementing the !CONFIG_IOMMU_API stubs in the
>> driver.
>
> I expect only very few drivers need that. Most drivers (like KVM and the
> upcoming VFIO) rely on the iommu-api and I expect most other drivers
> using the directly will also rely on an iommu. So this change makes
> still sense to me.
>

At least with MSM code, the iommu calls are abstracted through an API so
drivers don't need to know if the platform has an iommu or not.
Essentially:

if (iommu_found())
    iommu_map(...)
else
    do something else reasonable.

With this change, all code that might potentially call any iommu_map
function must exist in a separate file or face linker errors.

This idiom of having stubbed out functions is all over the kernel. I'm not
really sure what is gained by not having stubbed out versions of the
functions. There are better ways to catch users not selecting
CONFIG_IOMMU.

>> Possible of course, but I think it's a bit neater to have that in one
>> place, where all drivers can use. Similar to !CONFIG_HWSPINLOCK,
>> !CONFIG_DEBUG_FS, ...
>>
>> > This saves you even more .text size, no?
>>
>> Not sure; I expect the code to be compiled out today when the IOMMU
>> API is called and CONFIG_IOMMU_API is not set.
>
> But you even save all code that would use the iommu-api in your driver.
> This change forces you to compile out this code too leading to smaller
> .text-size.
>
> 	Joerg
>
The .text size reduction doesn't seem like it would be that great compared
to the annoyance of not having stubbed out functions.

Laura

> --
> AMD Operating System Research Center
>
> Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
> General Managers: Alberto Bozzo, Andrew Bowd
> Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr.
> 43632
>
> _______________________________________________
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linux-foundation.org/mailman/listinfo/iommu
>


-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.