This:
commit fae9a0d8ca68a14da8d2351ad3e0bf42f3b29899
Author: Glauber Costa <[email protected]>
Date: Tue Apr 8 13:20:56 2008 -0300
x86: merge iommu initialization parameters
Moved the forbid_dac parameter into pci-dma.c but forgot that it's a PCI
only symbol thus causing a compile failure if CONFIG_PCI=N. Fix by
surrounding the set clause in iommu_setup with #ifdef CONFIG_PCI
Signed-off-by: James Bottomley <[email protected]>
---
diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c
index 1972266..47c5a7a 100644
--- a/arch/x86/kernel/pci-dma.c
+++ b/arch/x86/kernel/pci-dma.c
@@ -202,6 +202,7 @@ static __init int iommu_setup(char *p)
iommu_merge = 0;
if (!strncmp(p, "forcesac", 8))
iommu_sac_force = 1;
+#ifdef CONFIG_PCI
if (!strncmp(p, "allowdac", 8))
forbid_dac = 0;
if (!strncmp(p, "nodac", 5))
@@ -210,6 +211,7 @@ static __init int iommu_setup(char *p)
forbid_dac = -1;
return 1;
}
+#endif
#ifdef CONFIG_SWIOTLB
if (!strncmp(p, "soft", 4))
swiotlb = 1;
On Tue, 28 Oct 2008 16:22:23 -0500
James Bottomley <[email protected]> wrote:
> This:
>
> commit fae9a0d8ca68a14da8d2351ad3e0bf42f3b29899
> Author: Glauber Costa <[email protected]>
> Date: Tue Apr 8 13:20:56 2008 -0300
>
> x86: merge iommu initialization parameters
>
> Moved the forbid_dac parameter into pci-dma.c but forgot that it's a PCI
> only symbol thus causing a compile failure if CONFIG_PCI=N. Fix by
> surrounding the set clause in iommu_setup with #ifdef CONFIG_PCI
>
> Signed-off-by: James Bottomley <[email protected]>
http://marc.info/?l=linux-kernel&m=122480590627590&w=2
I guess that this patch is in the PCI tree.
Can you please send this quickly? I've seen bug reports due to this
too many times.
* James Bottomley <[email protected]> wrote:
> This:
>
> commit fae9a0d8ca68a14da8d2351ad3e0bf42f3b29899
> Author: Glauber Costa <[email protected]>
> Date: Tue Apr 8 13:20:56 2008 -0300
>
> x86: merge iommu initialization parameters
>
> Moved the forbid_dac parameter into pci-dma.c but forgot that it's a
> PCI only symbol thus causing a compile failure if CONFIG_PCI=N. Fix
> by surrounding the set clause in iommu_setup with #ifdef CONFIG_PCI
uhm, that's wrong James, the above commit you refer to is half a year
old and has been released in v2.6.26 and v2.6.27 ;-)
The patch that broke the !CONFIG_PCI build in this cycle is this one:
| From 5b6985ce8ec7127b4d60ad450b64ca8b82748a3b Mon Sep 17 00:00:00 2001
| From: Fenghua Yu <[email protected]>
| Date: Thu, 16 Oct 2008 18:02:32 -0700
| Subject: [PATCH] intel-iommu: IA64 support
|
| The current Intel IOMMU code assumes that both host page size and
| Intel IOMMU page size are 4KiB. The first patch supports variable
| page size. This provides support for IA64 which has multiple page
| sizes.
|
| This patch also adds some other code hooks for IA64 platform
| including DMAR_OPERATION_TIMEOUT definition.
|
| [dwmw2: some cleanup]
| Signed-off-by: Fenghua Yu <[email protected]>
| Signed-off-by: Tony Luck <[email protected]>
| Signed-off-by: David Woodhouse <[email protected]>
> Signed-off-by: James Bottomley <[email protected]>
>
> ---
>
> diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c
> index 1972266..47c5a7a 100644
> --- a/arch/x86/kernel/pci-dma.c
> +++ b/arch/x86/kernel/pci-dma.c
> @@ -202,6 +202,7 @@ static __init int iommu_setup(char *p)
> iommu_merge = 0;
> if (!strncmp(p, "forcesac", 8))
> iommu_sac_force = 1;
> +#ifdef CONFIG_PCI
> if (!strncmp(p, "allowdac", 8))
> forbid_dac = 0;
> if (!strncmp(p, "nodac", 5))
> @@ -210,6 +211,7 @@ static __init int iommu_setup(char *p)
> forbid_dac = -1;
> return 1;
> }
> +#endif
that's the wrong fix, the right fix from Fenghua Yu (from about a week
ago) is at:
http://marc.info/?l=linux-kernel&m=122480590627590&w=2
and it is in the PCI tree already.
Ingo
On Wed, 2008-10-29 at 08:36 +0100, Ingo Molnar wrote:
> * James Bottomley <[email protected]> wrote:
>
> > This:
> >
> > commit fae9a0d8ca68a14da8d2351ad3e0bf42f3b29899
> > Author: Glauber Costa <[email protected]>
> > Date: Tue Apr 8 13:20:56 2008 -0300
> >
> > x86: merge iommu initialization parameters
> >
> > Moved the forbid_dac parameter into pci-dma.c but forgot that it's a
> > PCI only symbol thus causing a compile failure if CONFIG_PCI=N. Fix
> > by surrounding the set clause in iommu_setup with #ifdef CONFIG_PCI
>
> uhm, that's wrong James, the above commit you refer to is half a year
> old and has been released in v2.6.26 and v2.6.27 ;-)
>
> The patch that broke the !CONFIG_PCI build in this cycle is this one:
>
> | From 5b6985ce8ec7127b4d60ad450b64ca8b82748a3b Mon Sep 17 00:00:00 2001
> | From: Fenghua Yu <[email protected]>
> | Date: Thu, 16 Oct 2008 18:02:32 -0700
> | Subject: [PATCH] intel-iommu: IA64 support
> |
> | The current Intel IOMMU code assumes that both host page size and
> | Intel IOMMU page size are 4KiB. The first patch supports variable
> | page size. This provides support for IA64 which has multiple page
> | sizes.
> |
> | This patch also adds some other code hooks for IA64 platform
> | including DMAR_OPERATION_TIMEOUT definition.
> |
> | [dwmw2: some cleanup]
> | Signed-off-by: Fenghua Yu <[email protected]>
> | Signed-off-by: Tony Luck <[email protected]>
> | Signed-off-by: David Woodhouse <[email protected]>
Hmm, well, the triage could be wrong ... however the prior patch seemed
to move forbid_dac out from under the PCI defines.
> > Signed-off-by: James Bottomley <[email protected]>
> >
> > ---
> >
> > diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c
> > index 1972266..47c5a7a 100644
> > --- a/arch/x86/kernel/pci-dma.c
> > +++ b/arch/x86/kernel/pci-dma.c
> > @@ -202,6 +202,7 @@ static __init int iommu_setup(char *p)
> > iommu_merge = 0;
> > if (!strncmp(p, "forcesac", 8))
> > iommu_sac_force = 1;
> > +#ifdef CONFIG_PCI
> > if (!strncmp(p, "allowdac", 8))
> > forbid_dac = 0;
> > if (!strncmp(p, "nodac", 5))
> > @@ -210,6 +211,7 @@ static __init int iommu_setup(char *p)
> > forbid_dac = -1;
> > return 1;
> > }
> > +#endif
>
> that's the wrong fix, the right fix from Fenghua Yu (from about a week
> ago) is at:
>
> http://marc.info/?l=linux-kernel&m=122480590627590&w=2
>
> and it is in the PCI tree already.
This is obviously some strange definition of the word "right" of which I
was previously unaware. That patch moves forbid_dac plus a load of
quirk processing (also for a PCI bus) out from under CONFIG_PCI only ...
which will fix the compile error, sure.
However, if you'd be so kind, please explain how a DAC (meaning Dual
Addressing Cycle on the PCI bus) is useful (or even can be effected)
without a PCI bus?
All its really doing is contaminating pci-dma.c with clutter that only
needs to be there because someone can't get the separation right.
James
On Wed, 29 Oct 2008 09:01:46 -0500
James Bottomley <[email protected]> wrote:
> This is obviously some strange definition of the word "right" of
> which I was previously unaware. That patch moves forbid_dac plus a
> load of quirk processing (also for a PCI bus) out from under
> CONFIG_PCI only ... which will fix the compile error, sure.
>
> However, if you'd be so kind, please explain how a DAC (meaning Dual
> Addressing Cycle on the PCI bus) is useful (or even can be effected)
> without a PCI bus?
>
> All its really doing is contaminating pci-dma.c with clutter that only
> needs to be there because someone can't get the separation right.o
>
why do you even want that file for CONFIG_PCI=n ??
--
Arjan van de Ven Intel Open Source Technology Centre
For development, discussion and tips for power savings,
visit http://www.lesswatts.org
On Wed, 2008-10-29 at 07:08 -0700, Arjan van de Ven wrote:
> On Wed, 29 Oct 2008 09:01:46 -0500
> James Bottomley <[email protected]> wrote:
>
> > This is obviously some strange definition of the word "right" of
> > which I was previously unaware. That patch moves forbid_dac plus a
> > load of quirk processing (also for a PCI bus) out from under
> > CONFIG_PCI only ... which will fix the compile error, sure.
> >
> > However, if you'd be so kind, please explain how a DAC (meaning Dual
> > Addressing Cycle on the PCI bus) is useful (or even can be effected)
> > without a PCI bus?
> >
> > All its really doing is contaminating pci-dma.c with clutter that only
> > needs to be there because someone can't get the separation right.o
> >
>
> why do you even want that file for CONFIG_PCI=n ??
Um, my fault from long ago. It's contaminated with the dma_ API pieces
that are bus generic.
The correct solution, I think, is to split it out into a bus generic
piece and a PCI specific piece. I think all the intel/amd IOMMU stuff
should be in the PCI specific piece ... although I know there's a
theoretical case where the AMD iommu can be ht bus only with no PCI bus,
I don't think anyone's built such a beast, in which case it's safe to
condition iommu presence on CONFIG_PCI?
James
On Wed, 29 Oct 2008 09:18:03 -0500
James Bottomley <[email protected]> wrote:
> On Wed, 2008-10-29 at 07:08 -0700, Arjan van de Ven wrote:
> > On Wed, 29 Oct 2008 09:01:46 -0500
> > James Bottomley <[email protected]> wrote:
> >
> > > This is obviously some strange definition of the word "right" of
> > > which I was previously unaware. That patch moves forbid_dac plus
> > > a load of quirk processing (also for a PCI bus) out from under
> > > CONFIG_PCI only ... which will fix the compile error, sure.
> > >
> > > However, if you'd be so kind, please explain how a DAC (meaning
> > > Dual Addressing Cycle on the PCI bus) is useful (or even can be
> > > effected) without a PCI bus?
> > >
> > > All its really doing is contaminating pci-dma.c with clutter that
> > > only needs to be there because someone can't get the separation
> > > right.o
> > >
> >
> > why do you even want that file for CONFIG_PCI=n ??
>
> Um, my fault from long ago. It's contaminated with the dma_ API
> pieces that are bus generic.
>
> The correct solution, I think, is to split it out into a bus generic
> piece and a PCI specific piece.
agreed
> I think all the intel/amd IOMMU stuff
> should be in the PCI specific piece ... although I know there's a
> theoretical case where the AMD iommu can be ht bus only with no PCI
> bus, I don't think anyone's built such a beast, in which case it's
> safe to condition iommu presence on CONFIG_PCI?
I would think that's not relevant; AMD seems to have PCI in it's cpu
package already, never mind what it is connected to.
--
Arjan van de Ven Intel Open Source Technology Centre
For development, discussion and tips for power savings,
visit http://www.lesswatts.org
This splits pci-dma.c into two pieces (a PCI specific pci-dma.c and a
generic dma.c) and conditions the compile of the PCI specific piece on
CONFIG_PCI.
This has the benefit of sweeping all the IOMMU glue into PCI specific
files.
Signed-off-by: James Bottomley <[email protected]>
---
Note: it's possible to merge pci-nommu into this if desired, since that
file really has nothing to do with PCI and everything to do with generic
dma operations.
Also, I used the CONFIG_PCI conditioning dma_supported() rather than
weak symbol trickery because of the EXPORT_SYMBOL() requirements.
James
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 58814cc..be44178 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -34,7 +34,8 @@ obj-$(CONFIG_X86_32) += sys_i386_32.o i386_ksyms_32.o
obj-$(CONFIG_X86_64) += sys_x86_64.o x8664_ksyms_64.o
obj-$(CONFIG_X86_64) += syscall_64.o vsyscall_64.o
obj-y += bootflag.o e820.o
-obj-y += pci-dma.o quirks.o i8237.o topology.o kdebugfs.o
+obj-$(CONFIG_PCI) += pci-dma.o
+obj-y += dma.o quirks.o i8237.o topology.o kdebugfs.o
obj-y += alternative.o i8253.o pci-nommu.o
obj-y += tsc.o io_delay.o rtc.o
diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c
index 1926248..8cbf10c 100644
--- a/arch/x86/kernel/pci-dma.c
+++ b/arch/x86/kernel/pci-dma.c
@@ -11,9 +11,6 @@
static int forbid_dac __read_mostly;
-struct dma_mapping_ops *dma_ops;
-EXPORT_SYMBOL(dma_ops);
-
static int iommu_sac_force __read_mostly;
#ifdef CONFIG_IOMMU_DEBUG
@@ -35,30 +32,6 @@ int iommu_detected __read_mostly = 0;
int iommu_bio_merge __read_mostly = 0;
EXPORT_SYMBOL(iommu_bio_merge);
-dma_addr_t bad_dma_address __read_mostly = 0;
-EXPORT_SYMBOL(bad_dma_address);
-
-/* Dummy device used for NULL arguments (normally ISA). Better would
- be probably a smaller DMA mask, but this is bug-to-bug compatible
- to older i386. */
-struct device x86_dma_fallback_dev = {
- .bus_id = "fallback device",
- .coherent_dma_mask = DMA_32BIT_MASK,
- .dma_mask = &x86_dma_fallback_dev.coherent_dma_mask,
-};
-EXPORT_SYMBOL(x86_dma_fallback_dev);
-
-int dma_set_mask(struct device *dev, u64 mask)
-{
- if (!dev->dma_mask || !dma_supported(dev, mask))
- return -EIO;
-
- *dev->dma_mask = mask;
-
- return 0;
-}
-EXPORT_SYMBOL(dma_set_mask);
-
#ifdef CONFIG_X86_64
static __initdata void *dma32_bootmem_ptr;
static unsigned long dma32_bootmem_size __initdata = (128ULL<<20);
@@ -134,37 +107,6 @@ unsigned long iommu_nr_pages(unsigned long addr, unsigned long len)
EXPORT_SYMBOL(iommu_nr_pages);
#endif
-void *dma_generic_alloc_coherent(struct device *dev, size_t size,
- dma_addr_t *dma_addr, gfp_t flag)
-{
- unsigned long dma_mask;
- struct page *page;
- dma_addr_t addr;
-
- dma_mask = dma_alloc_coherent_mask(dev, flag);
-
- flag |= __GFP_ZERO;
-again:
- page = alloc_pages_node(dev_to_node(dev), flag, get_order(size));
- if (!page)
- return NULL;
-
- addr = page_to_phys(page);
- if (!is_buffer_dma_capable(dma_mask, addr, size)) {
- __free_pages(page, get_order(size));
-
- if (dma_mask < DMA_32BIT_MASK && !(flag & GFP_DMA)) {
- flag = (flag & ~GFP_DMA32) | GFP_DMA;
- goto again;
- }
-
- return NULL;
- }
-
- *dma_addr = addr;
- return page_address(page);
-}
-
/*
* See <Documentation/x86_64/boot-options.txt> for the iommu kernel parameter
* documentation.
@@ -232,16 +174,15 @@ static __init int iommu_setup(char *p)
}
early_param("iommu", iommu_setup);
+/* the non PCI version of this file is in dma.c */
int dma_supported(struct device *dev, u64 mask)
{
struct dma_mapping_ops *ops = get_dma_ops(dev);
-#ifdef CONFIG_PCI
if (mask > 0xffffffff && forbid_dac > 0) {
dev_info(dev, "PCI: Disallowing DAC for device\n");
return 0;
}
-#endif
if (ops->dma_supported)
return ops->dma_supported(dev, mask);
@@ -273,7 +214,7 @@ int dma_supported(struct device *dev, u64 mask)
}
EXPORT_SYMBOL(dma_supported);
-static int __init pci_iommu_init(void)
+void __init pci_iommu_init(void)
{
calgary_iommu_init();
@@ -283,18 +224,15 @@ static int __init pci_iommu_init(void)
gart_iommu_init();
- no_iommu_init();
- return 0;
+ if (!dma_ops)
+ force_iommu = 0;
}
void pci_iommu_shutdown(void)
{
gart_iommu_shutdown();
}
-/* Must execute after PCI subsystem */
-fs_initcall(pci_iommu_init);
-#ifdef CONFIG_PCI
/* Many VIA bridges seem to corrupt data for DAC. Disable it here */
static __devinit void via_no_dac(struct pci_dev *dev)
@@ -306,4 +244,3 @@ static __devinit void via_no_dac(struct pci_dev *dev)
}
}
DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_VIA, PCI_ANY_ID, via_no_dac);
-#endif
diff --git a/arch/x86/kernel/pci-nommu.c b/arch/x86/kernel/pci-nommu.c
index c70ab5a..5e7dd85 100644
--- a/arch/x86/kernel/pci-nommu.c
+++ b/arch/x86/kernel/pci-nommu.c
@@ -90,7 +90,5 @@ void __init no_iommu_init(void)
{
if (dma_ops)
return;
-
- force_iommu = 0; /* no HW IOMMU */
dma_ops = &nommu_dma_ops;
}
diff --git a/dev/null b/arch/x86/kernel/dma.c
new file mode 100644
index 0000000..0896152
--- /dev/null
+++ b/arch/x86/kernel/dma.c
@@ -0,0 +1,104 @@
+#include <linux/dma-mapping.h>
+#include <linux/dmar.h>
+#include <linux/bootmem.h>
+
+#include <asm/proto.h>
+#include <asm/dma.h>
+#include <asm/iommu.h>
+
+struct dma_mapping_ops *dma_ops;
+EXPORT_SYMBOL(dma_ops);
+
+dma_addr_t bad_dma_address __read_mostly = 0;
+EXPORT_SYMBOL(bad_dma_address);
+
+/* Dummy device used for NULL arguments (normally ISA). Better would
+ be probably a smaller DMA mask, but this is bug-to-bug compatible
+ to older i386. */
+struct device x86_dma_fallback_dev = {
+ .bus_id = "fallback device",
+ .coherent_dma_mask = DMA_32BIT_MASK,
+ .dma_mask = &x86_dma_fallback_dev.coherent_dma_mask,
+};
+EXPORT_SYMBOL(x86_dma_fallback_dev);
+
+int dma_set_mask(struct device *dev, u64 mask)
+{
+ if (!dev->dma_mask || !dma_supported(dev, mask))
+ return -EIO;
+
+ *dev->dma_mask = mask;
+
+ return 0;
+}
+EXPORT_SYMBOL(dma_set_mask);
+
+void *dma_generic_alloc_coherent(struct device *dev, size_t size,
+ dma_addr_t *dma_addr, gfp_t flag)
+{
+ unsigned long dma_mask;
+ struct page *page;
+ dma_addr_t addr;
+
+ dma_mask = dma_alloc_coherent_mask(dev, flag);
+
+ flag |= __GFP_ZERO;
+again:
+ page = alloc_pages_node(dev_to_node(dev), flag, get_order(size));
+ if (!page)
+ return NULL;
+
+ addr = page_to_phys(page);
+ if (!is_buffer_dma_capable(dma_mask, addr, size)) {
+ __free_pages(page, get_order(size));
+
+ if (dma_mask < DMA_32BIT_MASK && !(flag & GFP_DMA)) {
+ flag = (flag & ~GFP_DMA32) | GFP_DMA;
+ goto again;
+ }
+
+ return NULL;
+ }
+
+ *dma_addr = addr;
+ return page_address(page);
+}
+
+#ifndef CONFIG_PCI
+int dma_supported(struct device *dev, u64 mask)
+{
+ struct dma_mapping_ops *ops = get_dma_ops(dev);
+
+ if (ops->dma_supported)
+ return ops->dma_supported(dev, mask);
+
+ /* Copied from i386. Doesn't make much sense, because it will
+ only work for pci_alloc_coherent.
+ The caller just has to use GFP_DMA in this case. */
+ if (mask < DMA_24BIT_MASK)
+ return 0;
+
+ return 1;
+}
+EXPORT_SYMBOL(dma_supported);
+#endif
+
+/* dummy weak initialisation function. If CONFIG_PCI is selected
+ * pci-dma.c has a strong version of this which gets called instead */
+void __init __weak pci_iommu_init(void)
+{
+}
+
+static __init int dma_init(void)
+{
+ pci_iommu_init();
+
+ no_iommu_init();
+
+ return 0;
+}
+
+/* Must execute after PCI subsystem */
+fs_initcall(dma_init);
+
+