[PATCH 1/2] iommu: Always define struct iommu_ops

[PATCH 1/2] iommu: Always define struct iommu_ops

[Thierry Reding]

From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

With this structure always defined, drivers can be always compiled,
irrespective of whether or not IOMMU_API is enabled. This helps to
increase compile coverage without having to build with two separate
configurations.

Unused code can still be discarded by the compiler if the IS_ENABLED()
macro is properly used in an if condition, such as shown below:

	static const struct iommu_ops iommu_ops = {
		...
	};

	static int iommu_setup(...)
	{
		...

		bus_set_iommu(..., &iommu_ops);

		...
	}

	...

	static int iommu_probe(...)
	{
		...

		if (IS_ENABLED(CONFIG_IOMMU_API)) {
			err = iommu_setup(...);
			...
		}

		...
	}

That pattern will allow the compiler to notice that setup_iommu() is
never called if IOMMU_API is not selected, which will allow it to
discard its code and all data referenced from it.

Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 include/linux/iommu.h | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 20f9a527922a..0b5222c7ff97 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -83,8 +83,6 @@ enum iommu_attr {
 	DOMAIN_ATTR_MAX,
 };
 
-#ifdef CONFIG_IOMMU_API
-
 /**
  * struct iommu_ops - iommu ops and capabilities
  * @domain_init: init iommu domain
@@ -140,6 +138,8 @@ struct iommu_ops {
 #define IOMMU_GROUP_NOTIFY_UNBIND_DRIVER	5 /* Pre Driver unbind */
 #define IOMMU_GROUP_NOTIFY_UNBOUND_DRIVER	6 /* Post Driver unbind */
 
+#ifdef CONFIG_IOMMU_API
+
 extern int bus_set_iommu(struct bus_type *bus, const struct iommu_ops *ops);
 extern bool iommu_present(struct bus_type *bus);
 extern struct iommu_domain *iommu_domain_alloc(struct bus_type *bus);
@@ -242,9 +242,6 @@ static inline int report_iommu_fault(struct iommu_domain *domain,
 
 #else /* CONFIG_IOMMU_API */
 
-struct iommu_ops {};
-struct iommu_group {};
-
 static inline bool iommu_present(struct bus_type *bus)
 {
 	return false;
-- 
2.0.3

[PATCH 2/2] iommu: Implement a dummy bus_set_iommu()

[Thierry Reding]

From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

This allows IOMMU drivers to compile even if IOMMU_API is not selected
and helps improve compile coverage.

Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 include/linux/iommu.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 0b5222c7ff97..415608ea2999 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -242,6 +242,12 @@ static inline int report_iommu_fault(struct iommu_domain *domain,
 
 #else /* CONFIG_IOMMU_API */
 
+static inline int bus_set_iommu(struct bus_type *bus,
+				const struct iommu_ops *ops)
+{
+	return -ENOSYS;
+}
+
 static inline bool iommu_present(struct bus_type *bus)
 {
 	return false;
-- 
2.0.3

Re: [PATCH 2/2] iommu: Implement a dummy bus_set_iommu()

[Joerg Roedel]

On Fri, Aug 01, 2014 at 02:45:13PM +0200, Thierry Reding wrote:
> From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> 
> This allows IOMMU drivers to compile even if IOMMU_API is not selected
> and helps improve compile coverage.

IOMMU drivers usually implement the IOMMU-API, so they have a strong
dependency to it. Why do you want to compile-test an iommu driver
without testing the iommu core-code as well?


	Joerg

Re: [PATCH 2/2] iommu: Implement a dummy bus_set_iommu()

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1159 bytes --]

On Mon, Aug 18, 2014 at 06:57:50PM +0200, Joerg Roedel wrote:
> On Fri, Aug 01, 2014 at 02:45:13PM +0200, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > This allows IOMMU drivers to compile even if IOMMU_API is not selected
> > and helps improve compile coverage.
> 
> IOMMU drivers usually implement the IOMMU-API, so they have a strong
> dependency to it. Why do you want to compile-test an iommu driver
> without testing the iommu core-code as well?

It seems like I never got back to you on this. The reason here is that
for Tegra the IOMMU is part of a larger IP block. The IP block is
primarily a memory controller with a bunch of configuration knobs for
arbitration, prioritization and whatnot. A smaller part of the device
does the IOMMU translation. Therefore the IOMMU implementation would
be part of a larger driver, and having this dummy would reduce the
amount of #ifdefs required.

bus_set_iommu() is also the only function in include/linux/iommu.h that
doesn't have a dummy, so it seemed like a natural thing to do.

That said, I can work around this being absent for !IOMMU_API.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 2/2] iommu: Implement a dummy bus_set_iommu()

[Joerg Roedel]

On Wed, Oct 15, 2014 at 11:10:44AM +0200, Thierry Reding wrote:
> It seems like I never got back to you on this. The reason here is that
> for Tegra the IOMMU is part of a larger IP block. The IP block is
> primarily a memory controller with a bunch of configuration knobs for
> arbitration, prioritization and whatnot. A smaller part of the device
> does the IOMMU translation. Therefore the IOMMU implementation would
> be part of a larger driver, and having this dummy would reduce the
> amount of #ifdefs required.
> 
> bus_set_iommu() is also the only function in include/linux/iommu.h that
> doesn't have a dummy, so it seemed like a natural thing to do.
> 
> That said, I can work around this being absent for !IOMMU_API.

Or you make this "larger driver" just depend on IOMMU_API? Otherwise you
need the #ifdefs anyway to not carry around the iommu-ops functions in
the driver without having the iommu core enabled, no?


	Joerg

Re: [PATCH 2/2] iommu: Implement a dummy bus_set_iommu()

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 1467 bytes --]

On Wed, Oct 22, 2014 at 04:29:22PM +0200, Joerg Roedel wrote:
> On Wed, Oct 15, 2014 at 11:10:44AM +0200, Thierry Reding wrote:
> > It seems like I never got back to you on this. The reason here is that
> > for Tegra the IOMMU is part of a larger IP block. The IP block is
> > primarily a memory controller with a bunch of configuration knobs for
> > arbitration, prioritization and whatnot. A smaller part of the device
> > does the IOMMU translation. Therefore the IOMMU implementation would
> > be part of a larger driver, and having this dummy would reduce the
> > amount of #ifdefs required.
> > 
> > bus_set_iommu() is also the only function in include/linux/iommu.h that
> > doesn't have a dummy, so it seemed like a natural thing to do.
> > 
> > That said, I can work around this being absent for !IOMMU_API.
> 
> Or you make this "larger driver" just depend on IOMMU_API? Otherwise you
> need the #ifdefs anyway to not carry around the iommu-ops functions in
> the driver without having the iommu core enabled, no?

I can do that using IS_ENABLED() magic, which will cause the iommu_ops
to be thrown away automatically when IOMMU_API is disabled. That allows
the driver to be compiled with full coverage and still only include the
needed parts in the final binary.

Then again, going forward it doesn't seem likely that we'll ever want to
disable IOMMU on Tegra once it's been enabled, so this isn't really a
big issue.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]