2011-06-08 08:35:56

by Ohad Ben Cohen

[permalink] [raw]
Subject: [PATCH 0/4] drivers/iommu/ relocations

Create a dedicated iommu drivers folder, put the base iommu code there,
and move the existing IOMMU API users as well (msm-iommu, amd_iommu and
intel-iommu).

Putting all iommu drivers together will ease finding similarities
between different platforms, with the intention of solving problems once,
in a generic framework, which everyone can use.

OMAP's iommu will be moved too as soon as it's migrated.

For previous discussions on this, please see:
https://lkml.org/lkml/2011/6/2/369

Ohad Ben-Cohen (4):
drivers: iommu: move to a dedicated folder
msm: iommu: move to drivers/iommu/
x86: amd_iommu: move to drivers/iommu/
x86: intel-iommu: move to drivers/iommu/

arch/arm/mach-msm/Kconfig | 15 ------
arch/arm/mach-msm/Makefile | 2 +-
arch/x86/Kconfig | 40 ---------------
arch/x86/kernel/Makefile | 2 +-
drivers/Kconfig | 2 +
drivers/Makefile | 1 +
drivers/base/Makefile | 1 -
drivers/iommu/Kconfig | 53 ++++++++++++++++++++
drivers/iommu/Makefile | 4 ++
{arch/x86/kernel => drivers/iommu}/amd_iommu.c | 0
drivers/{pci => iommu}/intel-iommu.c | 1 -
drivers/{base => iommu}/iommu.c | 0
.../mach-msm/iommu.c => drivers/iommu/msm-iommu.c | 0
drivers/pci/Makefile | 2 +-
drivers/pci/pci.h | 2 -
include/linux/pci.h | 11 ++++
16 files changed, 74 insertions(+), 62 deletions(-)
create mode 100644 drivers/iommu/Kconfig
create mode 100644 drivers/iommu/Makefile
rename {arch/x86/kernel => drivers/iommu}/amd_iommu.c (100%)
rename drivers/{pci => iommu}/intel-iommu.c (99%)
rename drivers/{base => iommu}/iommu.c (100%)
rename arch/arm/mach-msm/iommu.c => drivers/iommu/msm-iommu.c (100%)


2011-06-08 08:36:00

by Ohad Ben Cohen

[permalink] [raw]
Subject: [PATCH 1/4] drivers: iommu: move to a dedicated folder

Create a dedicated folder for iommu drivers, and move the base
iommu implementation over there.

Grouping the various iommu drivers in a single location will help
finding similar problems shared by different platforms, so they
could be solved once, in the iommu framework, instead of solved
differently (or duplicated) in each driver.

Signed-off-by: Ohad Ben-Cohen <[email protected]>
---
arch/arm/mach-msm/Kconfig | 3 ---
arch/x86/Kconfig | 5 ++---
drivers/Kconfig | 2 ++
drivers/Makefile | 1 +
drivers/base/Makefile | 1 -
drivers/iommu/Kconfig | 3 +++
drivers/iommu/Makefile | 1 +
drivers/{base => iommu}/iommu.c | 0
8 files changed, 9 insertions(+), 7 deletions(-)
create mode 100644 drivers/iommu/Kconfig
create mode 100644 drivers/iommu/Makefile
rename drivers/{base => iommu}/iommu.c (100%)

diff --git a/arch/arm/mach-msm/Kconfig b/arch/arm/mach-msm/Kconfig
index 1516896..efb7b7d 100644
--- a/arch/arm/mach-msm/Kconfig
+++ b/arch/arm/mach-msm/Kconfig
@@ -205,9 +205,6 @@ config MSM_GPIOMUX
config MSM_V2_TLMM
bool

-config IOMMU_API
- bool
-
config MSM_SCM
bool
endif
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index da34972..460d573 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -685,6 +685,7 @@ config AMD_IOMMU
select SWIOTLB
select PCI_MSI
select PCI_IOV
+ select IOMMU_API
depends on X86_64 && PCI && ACPI
---help---
With this option you can enable support for AMD IOMMU hardware in
@@ -720,9 +721,6 @@ config SWIOTLB
config IOMMU_HELPER
def_bool (CALGARY_IOMMU || GART_IOMMU || SWIOTLB || AMD_IOMMU)

-config IOMMU_API
- def_bool (AMD_IOMMU || DMAR)
-
config MAXSMP
bool "Enable Maximum number of SMP Processors and NUMA Nodes"
depends on X86_64 && SMP && DEBUG_KERNEL && EXPERIMENTAL
@@ -1945,6 +1943,7 @@ config PCI_CNB20LE_QUIRK
config DMAR
bool "Support for DMA Remapping Devices (EXPERIMENTAL)"
depends on PCI_MSI && ACPI && EXPERIMENTAL
+ select IOMMU_API
help
DMA remapping (DMAR) devices support enables independent address
translations for Direct Memory Access (DMA) from devices.
diff --git a/drivers/Kconfig b/drivers/Kconfig
index 3bb154d..9d51318 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -126,4 +126,6 @@ source "drivers/hwspinlock/Kconfig"

source "drivers/clocksource/Kconfig"

+source "drivers/iommu/Kconfig"
+
endmenu
diff --git a/drivers/Makefile b/drivers/Makefile
index 09f3232..2f7a71a 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -122,3 +122,4 @@ obj-y += ieee802154/
obj-y += clk/

obj-$(CONFIG_HWSPINLOCK) += hwspinlock/
+obj-$(CONFIG_IOMMU_API) += iommu/
diff --git a/drivers/base/Makefile b/drivers/base/Makefile
index 4c5701c..5ab0d07 100644
--- a/drivers/base/Makefile
+++ b/drivers/base/Makefile
@@ -13,7 +13,6 @@ obj-$(CONFIG_FW_LOADER) += firmware_class.o
obj-$(CONFIG_NUMA) += node.o
obj-$(CONFIG_MEMORY_HOTPLUG_SPARSE) += memory.o
obj-$(CONFIG_SMP) += topology.o
-obj-$(CONFIG_IOMMU_API) += iommu.o
ifeq ($(CONFIG_SYSFS),y)
obj-$(CONFIG_MODULES) += module.o
endif
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
new file mode 100644
index 0000000..2c5dfb4
--- /dev/null
+++ b/drivers/iommu/Kconfig
@@ -0,0 +1,3 @@
+# IOMMU_API always gets selected by whoever wants it.
+config IOMMU_API
+ bool
diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
new file mode 100644
index 0000000..241ba4c
--- /dev/null
+++ b/drivers/iommu/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_IOMMU_API) += iommu.o
diff --git a/drivers/base/iommu.c b/drivers/iommu/iommu.c
similarity index 100%
rename from drivers/base/iommu.c
rename to drivers/iommu/iommu.c
--
1.7.1

2011-06-08 08:36:53

by Ohad Ben Cohen

[permalink] [raw]
Subject: [PATCH 2/4] msm: iommu: move to drivers/iommu/

This should ease finding similarities with different platforms,
with the intention of solving problems once in a generic framework
which everyone can use.

Compile-tested for MSM8X60.

Signed-off-by: Ohad Ben-Cohen <[email protected]>
---
arch/arm/mach-msm/Kconfig | 12 ------------
arch/arm/mach-msm/Makefile | 2 +-
drivers/iommu/Kconfig | 11 +++++++++++
drivers/iommu/Makefile | 1 +
.../mach-msm/iommu.c => drivers/iommu/msm-iommu.c | 0
5 files changed, 13 insertions(+), 13 deletions(-)
rename arch/arm/mach-msm/iommu.c => drivers/iommu/msm-iommu.c (100%)

diff --git a/arch/arm/mach-msm/Kconfig b/arch/arm/mach-msm/Kconfig
index efb7b7d..14ebda1 100644
--- a/arch/arm/mach-msm/Kconfig
+++ b/arch/arm/mach-msm/Kconfig
@@ -148,18 +148,6 @@ config MACH_MSM8960_RUMI3

endmenu

-config MSM_IOMMU
- bool "MSM IOMMU Support"
- depends on ARCH_MSM8X60 || ARCH_MSM8960
- select IOMMU_API
- default n
- help
- Support for the IOMMUs found on certain Qualcomm SOCs.
- These IOMMUs allow virtualization of the address space used by most
- cores within the multimedia subsystem.
-
- If unsure, say N here.
-
config IOMMU_PGTABLES_L2
def_bool y
depends on MSM_IOMMU && MMU && SMP && CPU_DCACHE_DISABLE=n
diff --git a/arch/arm/mach-msm/Makefile b/arch/arm/mach-msm/Makefile
index 9519fd2..0bf4648 100644
--- a/arch/arm/mach-msm/Makefile
+++ b/arch/arm/mach-msm/Makefile
@@ -3,7 +3,7 @@ obj-y += clock.o
obj-$(CONFIG_DEBUG_FS) += clock-debug.o

obj-$(CONFIG_MSM_VIC) += irq-vic.o
-obj-$(CONFIG_MSM_IOMMU) += iommu.o iommu_dev.o devices-iommu.o
+obj-$(CONFIG_MSM_IOMMU) += iommu_dev.o devices-iommu.o

obj-$(CONFIG_ARCH_MSM7X00A) += dma.o irq.o acpuclock-arm11.o
obj-$(CONFIG_ARCH_MSM7X30) += dma.o
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 2c5dfb4..c6d667c 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -1,3 +1,14 @@
# IOMMU_API always gets selected by whoever wants it.
config IOMMU_API
bool
+
+config MSM_IOMMU
+ bool "MSM IOMMU Support"
+ depends on ARCH_MSM8X60 || ARCH_MSM8960
+ select IOMMU_API
+ help
+ Support for the IOMMUs found on certain Qualcomm SOCs.
+ These IOMMUs allow virtualization of the address space used by most
+ cores within the multimedia subsystem.
+
+ If unsure, say N here.
diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index 241ba4c..a7e66fa 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -1 +1,2 @@
obj-$(CONFIG_IOMMU_API) += iommu.o
+obj-$(CONFIG_MSM_IOMMU) += msm-iommu.o
diff --git a/arch/arm/mach-msm/iommu.c b/drivers/iommu/msm-iommu.c
similarity index 100%
rename from arch/arm/mach-msm/iommu.c
rename to drivers/iommu/msm-iommu.c
--
1.7.1

2011-06-08 08:36:06

by Ohad Ben Cohen

[permalink] [raw]
Subject: [PATCH 3/4] x86: amd_iommu: move to drivers/iommu/

This should ease finding similarities with different platforms,
with the intention of solving problems once in a generic framework
which everyone can use.

Compile-tested on x86_64.

Signed-off-by: Ohad Ben-Cohen <[email protected]>
---
arch/x86/Kconfig | 28 ------------------------
arch/x86/kernel/Makefile | 2 +-
drivers/iommu/Kconfig | 28 ++++++++++++++++++++++++
drivers/iommu/Makefile | 1 +
{arch/x86/kernel => drivers/iommu}/amd_iommu.c | 0
5 files changed, 30 insertions(+), 29 deletions(-)
rename {arch/x86/kernel => drivers/iommu}/amd_iommu.c (100%)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 460d573..1b6a2e2 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -680,34 +680,6 @@ config CALGARY_IOMMU_ENABLED_BY_DEFAULT
Calgary anyway, pass 'iommu=calgary' on the kernel command line.
If unsure, say Y.

-config AMD_IOMMU
- bool "AMD IOMMU support"
- select SWIOTLB
- select PCI_MSI
- select PCI_IOV
- select IOMMU_API
- depends on X86_64 && PCI && ACPI
- ---help---
- With this option you can enable support for AMD IOMMU hardware in
- your system. An IOMMU is a hardware component which provides
- remapping of DMA memory accesses from devices. With an AMD IOMMU you
- can isolate the the DMA memory of different devices and protect the
- system from misbehaving device drivers or hardware.
-
- You can find out if your system has an AMD IOMMU if you look into
- your BIOS for an option to enable it or if you have an IVRS ACPI
- table.
-
-config AMD_IOMMU_STATS
- bool "Export AMD IOMMU statistics to debugfs"
- depends on AMD_IOMMU
- select DEBUG_FS
- ---help---
- This option enables code in the AMD IOMMU driver to collect various
- statistics about whats happening in the driver and exports that
- information to userspace via debugfs.
- If unsure, say N.
-
# need this always selected by IOMMU for the VIA workaround
config SWIOTLB
def_bool y if X86_64
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 90b06d4..aef0298 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -123,7 +123,7 @@ ifeq ($(CONFIG_X86_64),y)

obj-$(CONFIG_GART_IOMMU) += amd_gart_64.o aperture_64.o
obj-$(CONFIG_CALGARY_IOMMU) += pci-calgary_64.o tce_64.o
- obj-$(CONFIG_AMD_IOMMU) += amd_iommu_init.o amd_iommu.o
+ obj-$(CONFIG_AMD_IOMMU) += amd_iommu_init.o

obj-$(CONFIG_PCI_MMCONFIG) += mmconf-fam10h_64.o
obj-y += vsmp_64.o
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index c6d667c..555f5a8 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -12,3 +12,31 @@ config MSM_IOMMU
cores within the multimedia subsystem.

If unsure, say N here.
+
+config AMD_IOMMU
+ bool "AMD IOMMU support"
+ select SWIOTLB
+ select PCI_MSI
+ select PCI_IOV
+ select IOMMU_API
+ depends on X86_64 && PCI && ACPI
+ ---help---
+ With this option you can enable support for AMD IOMMU hardware in
+ your system. An IOMMU is a hardware component which provides
+ remapping of DMA memory accesses from devices. With an AMD IOMMU you
+ can isolate the the DMA memory of different devices and protect the
+ system from misbehaving device drivers or hardware.
+
+ You can find out if your system has an AMD IOMMU if you look into
+ your BIOS for an option to enable it or if you have an IVRS ACPI
+ table.
+
+config AMD_IOMMU_STATS
+ bool "Export AMD IOMMU statistics to debugfs"
+ depends on AMD_IOMMU
+ select DEBUG_FS
+ ---help---
+ This option enables code in the AMD IOMMU driver to collect various
+ statistics about whats happening in the driver and exports that
+ information to userspace via debugfs.
+ If unsure, say N.
diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index a7e66fa..d099d39 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -1,2 +1,3 @@
obj-$(CONFIG_IOMMU_API) += iommu.o
obj-$(CONFIG_MSM_IOMMU) += msm-iommu.o
+obj-$(CONFIG_AMD_IOMMU) += amd_iommu.o
diff --git a/arch/x86/kernel/amd_iommu.c b/drivers/iommu/amd_iommu.c
similarity index 100%
rename from arch/x86/kernel/amd_iommu.c
rename to drivers/iommu/amd_iommu.c
--
1.7.1

2011-06-08 08:36:31

by Ohad Ben Cohen

[permalink] [raw]
Subject: [PATCH 4/4] x86: intel-iommu: move to drivers/iommu/

This should ease finding similarities with different platforms,
with the intention of solving problems once in a generic framework
which everyone can use.

Note: to move intel-iommu.c, the declaration of pci_find_upstream_pcie_bridge()
has to move from drivers/pci/pci.h to include/linux/pci.h. This is handled
in this patch, too.

Compile-tested on x86_64.

Signed-off-by: Ohad Ben-Cohen <[email protected]>
---
arch/x86/Kconfig | 11 -----------
drivers/iommu/Kconfig | 11 +++++++++++
drivers/iommu/Makefile | 1 +
drivers/{pci => iommu}/intel-iommu.c | 1 -
drivers/pci/Makefile | 2 +-
drivers/pci/pci.h | 2 --
include/linux/pci.h | 11 +++++++++++
7 files changed, 24 insertions(+), 15 deletions(-)
rename drivers/{pci => iommu}/intel-iommu.c (99%)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 1b6a2e2..d22662c 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1912,17 +1912,6 @@ config PCI_CNB20LE_QUIRK

You should say N unless you know you need this.

-config DMAR
- bool "Support for DMA Remapping Devices (EXPERIMENTAL)"
- depends on PCI_MSI && ACPI && EXPERIMENTAL
- select IOMMU_API
- help
- DMA remapping (DMAR) devices support enables independent address
- translations for Direct Memory Access (DMA) from devices.
- These DMA remapping devices are reported via ACPI tables
- and include PCI device scope covered by these DMA
- remapping devices.
-
config DMAR_DEFAULT_ON
def_bool y
prompt "Enable DMA Remapping Devices by default"
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 555f5a8..b03a980 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -40,3 +40,14 @@ config AMD_IOMMU_STATS
statistics about whats happening in the driver and exports that
information to userspace via debugfs.
If unsure, say N.
+
+config DMAR
+ bool "Support for DMA Remapping Devices (EXPERIMENTAL)"
+ depends on PCI_MSI && ACPI && EXPERIMENTAL
+ select IOMMU_API
+ help
+ DMA remapping (DMAR) devices support enables independent address
+ translations for Direct Memory Access (DMA) from devices.
+ These DMA remapping devices are reported via ACPI tables
+ and include PCI device scope covered by these DMA
+ remapping devices.
diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index d099d39..aaac56d 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -1,3 +1,4 @@
obj-$(CONFIG_IOMMU_API) += iommu.o
obj-$(CONFIG_MSM_IOMMU) += msm-iommu.o
obj-$(CONFIG_AMD_IOMMU) += amd_iommu.o
+obj-$(CONFIG_DMAR) += intel-iommu.o
diff --git a/drivers/pci/intel-iommu.c b/drivers/iommu/intel-iommu.c
similarity index 99%
rename from drivers/pci/intel-iommu.c
rename to drivers/iommu/intel-iommu.c
index 59f17ac..fd7a055 100644
--- a/drivers/pci/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -42,7 +42,6 @@
#include <linux/pci-ats.h>
#include <asm/cacheflush.h>
#include <asm/iommu.h>
-#include "pci.h"

#define ROOT_SIZE VTD_PAGE_SIZE
#define CONTEXT_SIZE VTD_PAGE_SIZE
diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
index c85f744..7826920 100644
--- a/drivers/pci/Makefile
+++ b/drivers/pci/Makefile
@@ -30,7 +30,7 @@ obj-$(CONFIG_PCI_MSI) += msi.o
obj-$(CONFIG_HT_IRQ) += htirq.o

# Build Intel IOMMU support
-obj-$(CONFIG_DMAR) += dmar.o iova.o intel-iommu.o
+obj-$(CONFIG_DMAR) += dmar.o iova.o

obj-$(CONFIG_INTR_REMAP) += dmar.o intr_remapping.o

diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 731e202..b7bf11d 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -184,8 +184,6 @@ pci_match_one_device(const struct pci_device_id *id, const struct pci_dev *dev)
return NULL;
}

-struct pci_dev *pci_find_upstream_pcie_bridge(struct pci_dev *pdev);
-
/* PCI slot sysfs helper code */
#define to_pci_slot(s) container_of(s, struct pci_slot, kobj)

diff --git a/include/linux/pci.h b/include/linux/pci.h
index c446b5c..970bfe0 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1589,5 +1589,16 @@ int pci_vpd_find_tag(const u8 *buf, unsigned int off, unsigned int len, u8 rdt);
int pci_vpd_find_info_keyword(const u8 *buf, unsigned int off,
unsigned int len, const char *kw);

+/**
+ * pci_find_upstream_pcie_bridge - find upstream PCIe-to-PCI bridge of a device
+ * @pdev: the PCI device
+ *
+ * if the device is PCIE, return NULL
+ * if the device isn't connected to a PCIe bridge (that is its parent is a
+ * legacy PCI bridge and the bridge is directly connected to bus 0), return its
+ * parent
+ */
+struct pci_dev *pci_find_upstream_pcie_bridge(struct pci_dev *pdev);
+
#endif /* __KERNEL__ */
#endif /* LINUX_PCI_H */
--
1.7.1

2011-06-08 09:17:47

by David Woodhouse

[permalink] [raw]
Subject: Re: [PATCH 4/4] x86: intel-iommu: move to drivers/iommu/

On Wed, 2011-06-08 at 11:34 +0300, Ohad Ben-Cohen wrote:
> --- a/drivers/pci/Makefile
> +++ b/drivers/pci/Makefile
> @@ -30,7 +30,7 @@ obj-$(CONFIG_PCI_MSI) += msi.o
> obj-$(CONFIG_HT_IRQ) += htirq.o
>
> # Build Intel IOMMU support
> -obj-$(CONFIG_DMAR) += dmar.o iova.o intel-iommu.o
> +obj-$(CONFIG_DMAR) += dmar.o iova.o
>
> obj-$(CONFIG_INTR_REMAP) += dmar.o intr_remapping.o
>

At least iova.o wants to go with it. That's one of the parts that is a
candidate for harmonisation across IOMMU implementations, either by
removing it or by having others use it too. It's how we allocate virtual
I/O address space.

I suspect the interrupt remapping support may well want to move with it
too. It's no more out-of-place in drivers/iommu than it is in
drivers/pci. And then you can certainly move dmar.o too.

--
David Woodhouse Open Source Technology Centre
[email protected] Intel Corporation

2011-06-08 09:35:32

by Joerg Roedel

[permalink] [raw]
Subject: Re: [PATCH 0/4] drivers/iommu/ relocations

(Cc'ing Ingo)

On Wed, Jun 08, 2011 at 04:34:18AM -0400, Ohad Ben-Cohen wrote:
> Create a dedicated iommu drivers folder, put the base iommu code there,
> and move the existing IOMMU API users as well (msm-iommu, amd_iommu and
> intel-iommu).
>
> Putting all iommu drivers together will ease finding similarities
> between different platforms, with the intention of solving problems once,
> in a generic framework, which everyone can use.
>
> OMAP's iommu will be moved too as soon as it's migrated.

Great, thanks. I'll apply the patches as soon as the relevant ACKs come
in. Looking at the MAINTAINERS file David Brown needs to ACK the MSM
patch and David Woodhouse the VT-d patch.

David B., David W., is this direction ok for both of you?

A more important question is how we handle the IOMMU tree. Currently the
situation is as follows:

* The AMD IOMMU changes go upstream through Ingo
* David Woohouse has his own tree which he sents directly to
Linus
* Not sure about the ARM IOMMU code
* And to comlicate things further there is the upcoming ARM
integration tree which may contain code that depends on IOMMU
changes

My suggestion is that the ARM tree pulls in the necessary changes from
the IOMMU tree and the IOMMU code goes upstream through Ingo or directly
to Linus (with some time in linux-next, of course). Thoughts?

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

2011-06-08 09:37:47

by Joerg Roedel

[permalink] [raw]
Subject: Re: [PATCH 4/4] x86: intel-iommu: move to drivers/iommu/

On Wed, Jun 08, 2011 at 05:17:38AM -0400, David Woodhouse wrote:
> On Wed, 2011-06-08 at 11:34 +0300, Ohad Ben-Cohen wrote:
> > --- a/drivers/pci/Makefile
> > +++ b/drivers/pci/Makefile
> > @@ -30,7 +30,7 @@ obj-$(CONFIG_PCI_MSI) += msi.o
> > obj-$(CONFIG_HT_IRQ) += htirq.o
> >
> > # Build Intel IOMMU support
> > -obj-$(CONFIG_DMAR) += dmar.o iova.o intel-iommu.o
> > +obj-$(CONFIG_DMAR) += dmar.o iova.o
> >
> > obj-$(CONFIG_INTR_REMAP) += dmar.o intr_remapping.o
> >
>
> At least iova.o wants to go with it. That's one of the parts that is a
> candidate for harmonisation across IOMMU implementations, either by
> removing it or by having others use it too. It's how we allocate virtual
> I/O address space.
>
> I suspect the interrupt remapping support may well want to move with it
> too. It's no more out-of-place in drivers/iommu than it is in
> drivers/pci. And then you can certainly move dmar.o too.

Interrupt remapping certainly makes sense too. I am not sure yet how to
generalize it because the AMD version of it is significantly different
from VT-d, but we'll see.

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

2011-06-08 10:33:33

by Ohad Ben Cohen

[permalink] [raw]
Subject: Re: [PATCH 4/4] x86: intel-iommu: move to drivers/iommu/

On Wed, Jun 8, 2011 at 12:17 PM, David Woodhouse <[email protected]> wrote:
> At least iova.o wants to go with it. That's one of the parts that is a
> candidate for harmonisation across IOMMU implementations, either by
> removing it or by having others use it too. It's how we allocate virtual
> I/O address space.
>
> I suspect the interrupt remapping support may well want to move with it
> too. It's no more out-of-place in drivers/iommu than it is in
> drivers/pci. And then you can certainly move dmar.o too.

Sounds good, thanks.

I'll wait a bit to see if there're more comments, and then re-send this one

2011-06-08 13:05:27

by Matthew Wilcox

[permalink] [raw]
Subject: Re: [PATCH 0/4] drivers/iommu/ relocations

On Wed, Jun 08, 2011 at 11:34:18AM +0300, Ohad Ben-Cohen wrote:
> Create a dedicated iommu drivers folder, put the base iommu code there,
> and move the existing IOMMU API users as well (msm-iommu, amd_iommu and
> intel-iommu).
>
> Putting all iommu drivers together will ease finding similarities
> between different platforms, with the intention of solving problems once,
> in a generic framework, which everyone can use.
>
> OMAP's iommu will be moved too as soon as it's migrated.
>
> For previous discussions on this, please see:
> https://lkml.org/lkml/2011/6/2/369
>
> Ohad Ben-Cohen (4):
> drivers: iommu: move to a dedicated folder
> msm: iommu: move to drivers/iommu/
> x86: amd_iommu: move to drivers/iommu/
> x86: intel-iommu: move to drivers/iommu/

You've missed at least parisc, ia64, alpha, sparc, powerpc and sh which
have IOMMUs. Not that they necessarily all need to be moved across in
one patchset, but saying "all iommu drivers" is clearly false.

--
Matthew Wilcox Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."

2011-06-08 13:11:40

by Ohad Ben Cohen

[permalink] [raw]
Subject: Re: [PATCH 0/4] drivers/iommu/ relocations

On Wed, Jun 8, 2011 at 4:05 PM, Matthew Wilcox <[email protected]> wrote:
> You've missed at least parisc, ia64, alpha, sparc, powerpc and sh which
> have IOMMUs.

None of these seem to call register_iommu.

> Not that they necessarily all need to be moved across in
> one patchset, but saying "all iommu drivers" is clearly false.

I've moved only the existing IOMMU API users.

2011-06-08 13:35:43

by Joerg Roedel

[permalink] [raw]
Subject: Re: [PATCH 0/4] drivers/iommu/ relocations

On Wed, Jun 08, 2011 at 09:11:16AM -0400, Ohad Ben-Cohen wrote:
> On Wed, Jun 8, 2011 at 4:05 PM, Matthew Wilcox <[email protected]> wrote:
> > You've missed at least parisc, ia64, alpha, sparc, powerpc and sh which
> > have IOMMUs.
>
> None of these seem to call register_iommu.
>
> > Not that they necessarily all need to be moved across in
> > one patchset, but saying "all iommu drivers" is clearly false.
>
> I've moved only the existing IOMMU API users.

And I think that is good for now. The iommu-api needs to be extended to
cover all kinds of iommus (like the iommus available on other
architectures). It is definitly the plan to do that and have a common
dma_ops implementation for all of them. But lets start small by now and
move forward step by step :-)

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

2011-06-08 14:33:09

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 0/4] drivers/iommu/ relocations


* Roedel, Joerg <[email protected]> wrote:

> (Cc'ing Ingo)
>
> On Wed, Jun 08, 2011 at 04:34:18AM -0400, Ohad Ben-Cohen wrote:
> > Create a dedicated iommu drivers folder, put the base iommu code there,
> > and move the existing IOMMU API users as well (msm-iommu, amd_iommu and
> > intel-iommu).
> >
> > Putting all iommu drivers together will ease finding similarities
> > between different platforms, with the intention of solving problems once,
> > in a generic framework, which everyone can use.
> >
> > OMAP's iommu will be moved too as soon as it's migrated.
>
> Great, thanks. I'll apply the patches as soon as the relevant ACKs come
> in. Looking at the MAINTAINERS file David Brown needs to ACK the MSM
> patch and David Woodhouse the VT-d patch.
>
> David B., David W., is this direction ok for both of you?
>
> A more important question is how we handle the IOMMU tree. Currently the
> situation is as follows:
>
> * The AMD IOMMU changes go upstream through Ingo
> * David Woohouse has his own tree which he sents directly to
> Linus
> * Not sure about the ARM IOMMU code
> * And to comlicate things further there is the upcoming ARM
> integration tree which may contain code that depends on IOMMU
> changes
>
> My suggestion is that the ARM tree pulls in the necessary changes from
> the IOMMU tree and the IOMMU code goes upstream through Ingo or directly
> to Linus (with some time in linux-next, of course). Thoughts?

I can certainly pull from you trees you pull from elsewhere.

David could help keep things tidier by sending the Intel IOMMU bits
to me as well.

In any case the tip:core/iommu tree is in linux-next so whatever you
send me shows up there on the next day or so.

Thanks,

Ingo

2011-06-08 17:49:29

by Chris Wright

[permalink] [raw]
Subject: Re: [PATCH 4/4] x86: intel-iommu: move to drivers/iommu/

* Ohad Ben-Cohen ([email protected]) wrote:
> +
> +config DMAR
> + bool "Support for DMA Remapping Devices (EXPERIMENTAL)"
> + depends on PCI_MSI && ACPI && EXPERIMENTAL

You may want to further restrict to x86 and ia64
And I think you'll need to fixup arch/ia64/Kconfig

BTW, I think EXPERIMENTAL can be dropped by now.

2011-06-08 19:35:46

by Ohad Ben Cohen

[permalink] [raw]
Subject: Re: [PATCH 4/4] x86: intel-iommu: move to drivers/iommu/

On Wed, Jun 8, 2011 at 8:47 PM, Chris Wright <[email protected]> wrote:
> You may want to further restrict to x86 and ia64
> And I think you'll need to fixup arch/ia64/Kconfig
>
> BTW, I think EXPERIMENTAL can be dropped by now.

Sounds all good, thanks a lot !

2011-06-08 22:23:13

by David Brown

[permalink] [raw]
Subject: Re: [PATCH 0/4] drivers/iommu/ relocations

On Wed, Jun 08 2011, Roedel, Joerg wrote:

> Great, thanks. I'll apply the patches as soon as the relevant ACKs come
> in. Looking at the MAINTAINERS file David Brown needs to ACK the MSM
> patch and David Woodhouse the VT-d patch.
>
> David B., David W., is this direction ok for both of you?

I think it is a good direction to move in. I'll reply to the patch
directly to comment on it.

> My suggestion is that the ARM tree pulls in the necessary changes from
> the IOMMU tree and the IOMMU code goes upstream through Ingo or directly
> to Linus (with some time in linux-next, of course). Thoughts?

This seems the cleanest to me.

David

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

2011-06-08 22:27:15

by David Brown

[permalink] [raw]
Subject: Re: [PATCH 2/4] msm: iommu: move to drivers/iommu/

On Wed, Jun 08 2011, Ohad Ben-Cohen wrote:

> This should ease finding similarities with different platforms,
> with the intention of solving problems once in a generic framework
> which everyone can use.
>
> Compile-tested for MSM8X60.
>
> Signed-off-by: Ohad Ben-Cohen <[email protected]>
> ---
> arch/arm/mach-msm/Kconfig | 12 ------------
> arch/arm/mach-msm/Makefile | 2 +-
> drivers/iommu/Kconfig | 11 +++++++++++
> drivers/iommu/Makefile | 1 +
> .../mach-msm/iommu.c => drivers/iommu/msm-iommu.c | 0
> 5 files changed, 13 insertions(+), 13 deletions(-)
> rename arch/arm/mach-msm/iommu.c => drivers/iommu/msm-iommu.c (100%)

Moving the driver is good. I'm wondering what we should do with
iommu_dev.c, though. Wouldn't it make sense to move this file as well?

I've added the author, Stepan Moskovchenko, in case he has any feedback.

David

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

2011-06-10 16:24:28

by Ohad Ben Cohen

[permalink] [raw]
Subject: Re: [PATCH 2/4] msm: iommu: move to drivers/iommu/

On Thu, Jun 9, 2011 at 1:27 AM, David Brown <[email protected]> wrote:
> Moving the driver is good. ?I'm wondering what we should do with
> iommu_dev.c, though. ?Wouldn't it make sense to move this file as well?

Yeah, it does make sense. I'll move that one too, thanks.