[PATCH 1/4] x86-64: Calgary IOMMU - introduce iommu_detected

[PATCH 1/4] x86-64: Calgary IOMMU - introduce iommu_detected

[Jon Mason]

swiotlb relies on the gart specific iommu_aperture variable to know if
we discovered a hardware IOMMU before swiotlb initialization.  Introduce
iommu_detected to do the same thing, but in a HW IOMMU neutral manner,
in preparation for adding the Calgary HW IOMMU.

Thanks,
Jon

Signed-Off-By: Muli Ben-Yehuda <muli@il.ibm.com>
Signed-Off-By: Jon Mason <jdmason@us.ibm.com>

diff -r 82f66cc5a33b arch/x86_64/kernel/aperture.c
--- a/arch/x86_64/kernel/aperture.c	Tue May 23 18:57:22 2006
+++ b/arch/x86_64/kernel/aperture.c	Tue May 23 14:05:16 2006
@@ -212,6 +212,7 @@
 		if (read_pci_config(0, num, 3, 0x00) != NB_ID_3) 
 			continue;	
 
+		iommu_detected = 1;
 		iommu_aperture = 1; 
 
 		aper_order = (read_pci_config(0, num, 3, 0x90) >> 1) & 7; 
diff -r 82f66cc5a33b arch/x86_64/kernel/pci-dma.c
--- a/arch/x86_64/kernel/pci-dma.c	Tue May 23 18:57:22 2006
+++ b/arch/x86_64/kernel/pci-dma.c	Tue May 23 14:05:16 2006
@@ -32,6 +32,9 @@
 int panic_on_overflow __read_mostly = 0;
 int force_iommu __read_mostly= 0;
 #endif
+
+/* Set this to 1 if there is a HW IOMMU in the system */
+int iommu_detected __read_mostly = 0;
 
 /* Dummy device used for NULL arguments (normally ISA). Better would
    be probably a smaller DMA mask, but this is bug-to-bug compatible
diff -r 82f66cc5a33b arch/x86_64/kernel/pci-gart.c
--- a/arch/x86_64/kernel/pci-gart.c	Tue May 23 18:57:22 2006
+++ b/arch/x86_64/kernel/pci-gart.c	Tue May 23 14:05:16 2006
@@ -624,6 +624,10 @@
 	if (swiotlb)
 		return -1; 
 
+	/* Did we detect a different HW IOMMU? */
+	if (iommu_detected && !iommu_aperture)
+		return -1;
+
 	if (no_iommu ||
 	    (!force_iommu && end_pfn <= MAX_DMA32_PFN) ||
 	    !iommu_aperture ||
diff -r 82f66cc5a33b arch/x86_64/kernel/pci-swiotlb.c
--- a/arch/x86_64/kernel/pci-swiotlb.c	Tue May 23 18:57:22 2006
+++ b/arch/x86_64/kernel/pci-swiotlb.c	Tue May 23 14:05:16 2006
@@ -31,7 +31,7 @@
 void pci_swiotlb_init(void)
 {
 	/* don't initialize swiotlb if iommu=off (no_iommu=1) */
-	if (!iommu_aperture && !no_iommu &&
+	if (!iommu_detected && !no_iommu &&
 	    (end_pfn > MAX_DMA32_PFN || force_iommu))
 	       swiotlb = 1;
 	if (swiotlb) {
diff -r 82f66cc5a33b include/asm-x86_64/proto.h
--- a/include/asm-x86_64/proto.h	Tue May 23 18:57:22 2006
+++ b/include/asm-x86_64/proto.h	Tue May 23 14:05:16 2006
@@ -116,6 +116,7 @@
 extern int acpi_ht;
 extern int acpi_disabled;
 
+extern int iommu_detected;
 #ifdef CONFIG_GART_IOMMU
 extern int fallback_aper_order;
 extern int fallback_aper_force;

Re: [PATCH 1/4] x86-64: Calgary IOMMU - introduce iommu_detected

[Andi Kleen]

On Thursday 25 May 2006 05:34, Jon Mason wrote:
> swiotlb relies on the gart specific iommu_aperture variable to know if
> we discovered a hardware IOMMU before swiotlb initialization.  Introduce
> iommu_detected to do the same thing, but in a HW IOMMU neutral manner,
> in preparation for adding the Calgary HW IOMMU.

I applied them all.

But I think you broke the aperture setup. iommu_setup really
needs to be called early, otherwise aperture.c doesn't get
the right parameters.  I undid that change.

And please next time send against the latest tree. It required
quite some tweaking to apply.

-Andi

Re: [PATCH 1/4] x86-64: Calgary IOMMU - introduce iommu_detected

[Jon Mason]

On Thu, May 25, 2006 at 05:54:23AM +0200, Andi Kleen wrote:
> On Thursday 25 May 2006 05:34, Jon Mason wrote:
> > swiotlb relies on the gart specific iommu_aperture variable to know if
> > we discovered a hardware IOMMU before swiotlb initialization.  Introduce
> > iommu_detected to do the same thing, but in a HW IOMMU neutral manner,
> > in preparation for adding the Calgary HW IOMMU.
> 
> I applied them all.

Fantastic!

> But I think you broke the aperture setup. iommu_setup really
> needs to be called early, otherwise aperture.c doesn't get
> the right parameters.  I undid that change.

I'll take a look at that, but I did boot test these patches on my
opteron system and didn't notice anything wrong.  Are the patches
available for me to look at on firstfloor?

> And please next time send against the latest tree. It required
> quite some tweaking to apply.

Sorry, I pulled a the latest mercurial tree this morning, but I guess
that one is stale (or became stale over the day).  

> -Andi

Re: [PATCH 1/4] x86-64: Calgary IOMMU - introduce iommu_detected

[Muli Ben-Yehuda]

On Wed, May 24, 2006 at 11:19:56PM -0500, Jon Mason wrote:
> On Thu, May 25, 2006 at 05:54:23AM +0200, Andi Kleen wrote:
> > On Thursday 25 May 2006 05:34, Jon Mason wrote:
> > > swiotlb relies on the gart specific iommu_aperture variable to know if
> > > we discovered a hardware IOMMU before swiotlb initialization.  Introduce
> > > iommu_detected to do the same thing, but in a HW IOMMU neutral manner,
> > > in preparation for adding the Calgary HW IOMMU.
> > 
> > I applied them all.
> 
> Fantastic!

Seconded :-)

> > But I think you broke the aperture setup. iommu_setup really
> > needs to be called early, otherwise aperture.c doesn't get
> > the right parameters.  I undid that change.
> 
> I'll take a look at that, but I did boot test these patches on my
> opteron system and didn't notice anything wrong.  Are the patches
> available for me to look at on firstfloor?

ftp://ftp.firstfloor.org/pub/ak/x86_64/quilt/patches/

> > And please next time send against the latest tree. It required
> > quite some tweaking to apply.
> 
> Sorry, I pulled a the latest mercurial tree this morning, but I guess
> that one is stale (or became stale over the day).  

I guess Andi means the firstfloor tree, which has a number of other
IOMMU related changes that are not in mainline yet. Let's take it for
a spin.

Cheers,
Muli

Re: [PATCH 1/4] x86-64: Calgary IOMMU - introduce iommu_detected

[Jon Mason]

On Thu, May 25, 2006 at 05:54:23AM +0200, Andi Kleen wrote:
> On Thursday 25 May 2006 05:34, Jon Mason wrote:
> > swiotlb relies on the gart specific iommu_aperture variable to know if
> > we discovered a hardware IOMMU before swiotlb initialization.  Introduce
> > iommu_detected to do the same thing, but in a HW IOMMU neutral manner,
> > in preparation for adding the Calgary HW IOMMU.
> 
> I applied them all.
> 
> But I think you broke the aperture setup. iommu_setup really
> needs to be called early, otherwise aperture.c doesn't get
> the right parameters.  I undid that change.

Actually, I didn't break aperture.c by moving iommu_setup because I
moved iommu_hole_init after __setup.  This change also enabled me to
move back the calgary specific bootarg parsing (like you asked for in
the original version of the path).  

Also, in the reworked version of iommu-abstraction on
ftp://ftp.firstfloor.org/pub/ak/x86_64/quilt/patches has a bug.  When
iommu_setup was kept in it's original form, the "+__setup("iommu=",
iommu_setup);" wasn't removed, which gives 2 calls to the same function.
I'll send updated versions of the patches here shortly which will apply
cleanly inplace to that tree.

> 
> And please next time send against the latest tree. It required
> quite some tweaking to apply.

Sorry, I'll be happy to do it next time :)

Thanks,
Jon

Re: [PATCH 1/4] x86-64: Calgary IOMMU - introduce iommu_detected

[Andi Kleen]

> Also, in the reworked version of iommu-abstraction on
> ftp://ftp.firstfloor.org/pub/ak/x86_64/quilt/patches has a bug.  When
> iommu_setup was kept in it's original form, the "+__setup("iommu=",
> iommu_setup);" wasn't removed, which gives 2 calls to the same function.
> I'll send updated versions of the patches here shortly which will apply
> cleanly inplace to that tree.

That was intentional. All early boot options need a __setup too, otherwise
they will appear as variables in init's environment.

It would be only a problem if the code couldn't run twice, but that is 
normally not the case.

-Andi