[PATCH 0 of 4] x86-64: Calgary IOMMU updates

[PATCH 0 of 4] x86-64: Calgary IOMMU updates

[Muli Ben-Yehuda]

Hi Andi,

[resending with proper CC's].

Please apply this Calgary patchset. It fixes one bug
(https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=203971, should
go to -stable as well) and makes several other updates all of which
are safe for 2.6.19. Each patch has a detailed description.

Thanks,
Muli

2 files changed, 25 insertions(+), 13 deletions(-)
MAINTAINERS                      |    2 +-
arch/x86_64/kernel/pci-calgary.c |   36 ++++++++++++++++++++++++------------

[PATCH 1 of 4] x86-64: Calgary IOMMU: print PCI bus numbers in hex

[Muli Ben-Yehuda]

1 files changed, 4 insertions(+), 4 deletions(-)
arch/x86_64/kernel/pci-calgary.c |    8 ++++----


# HG changeset patch
# User Muli Ben-Yehuda <muli@il.ibm.com>
# Date 1159604172 -10800
# Node ID 28658cf477bc8c6adc5a5335363a4d1428f58273
# Parent  585e6c1736a2a8b419ae9b8dc5055ee8774ba57f
x86-64: Calgary IOMMU: print PCI bus numbers in hex

From: Jon Mason <jdmason@kudzu.us>

Make the references to the bus number in hex instead of decimal, as
that is the way that lspci prints out the bus numbers.

Signed-off-by: Jon Mason <jdmason@kudzu.us>
Signed-off-by: Muli Ben-Yehuda <muli@il.ibm.com>

diff -r 585e6c1736a2 -r 28658cf477bc arch/x86_64/kernel/pci-calgary.c
--- a/arch/x86_64/kernel/pci-calgary.c	Sat Sep 30 11:13:54 2006 +0300
+++ b/arch/x86_64/kernel/pci-calgary.c	Sat Sep 30 11:16:12 2006 +0300
@@ -715,7 +715,7 @@ static void calgary_watchdog(unsigned lo
 
 	/* If no error, the agent ID in the CSR is not valid */
 	if (val32 & CSR_AGENT_MASK) {
-		printk(KERN_EMERG "calgary_watchdog: DMA error on bus %d, "
+		printk(KERN_EMERG "calgary_watchdog: DMA error on PHB %#x, "
 				  "CSR = %#x\n", dev->bus->number, val32);
 		writel(0, target);
 
@@ -749,7 +749,7 @@ static void __init calgary_enable_transl
 	val32 = be32_to_cpu(readl(target));
 	val32 |= PHB_TCE_ENABLE | PHB_DAC_DISABLE | PHB_MCSR_ENABLE;
 
-	printk(KERN_INFO "Calgary: enabling translation on PHB %d\n", busnum);
+	printk(KERN_INFO "Calgary: enabling translation on PHB %#x\n", busnum);
 	printk(KERN_INFO "Calgary: errant DMAs will now be prevented on this "
 	       "bus.\n");
 
@@ -779,7 +779,7 @@ static void __init calgary_disable_trans
 	val32 = be32_to_cpu(readl(target));
 	val32 &= ~(PHB_TCE_ENABLE | PHB_DAC_DISABLE | PHB_MCSR_ENABLE);
 
-	printk(KERN_INFO "Calgary: disabling translation on PHB %d!\n", busnum);
+	printk(KERN_INFO "Calgary: disabling translation on PHB %#x!\n", busnum);
 	writel(cpu_to_be32(val32), target);
 	readl(target); /* flush */
 
@@ -1053,7 +1053,7 @@ static int __init calgary_parse_options(
 
 			if (bridge < MAX_PHB_BUS_NUM) {
 				printk(KERN_INFO "Calgary: disabling "
-				       "translation for PHB 0x%x\n", bridge);
+				       "translation for PHB %#x\n", bridge);
 				bus_info[bridge].translation_disabled = 1;
 			}
 		}

[PATCH 2 of 4] x86-64: Calgary IOMMU: deobfuscate calgary_init

[Muli Ben-Yehuda]

1 files changed, 7 insertions(+), 5 deletions(-)
arch/x86_64/kernel/pci-calgary.c |   12 +++++++-----


# HG changeset patch
# User Muli Ben-Yehuda <muli@il.ibm.com>
# Date 1159604311 -10800
# Node ID e0562474cf16b13d8c3c815fce3159ba7cd0f540
# Parent  28658cf477bc8c6adc5a5335363a4d1428f58273
x86-64: Calgary IOMMU: deobfuscate calgary_init

From: Jon Mason <jdmason@kudzu.us>

calgary_init's for loop does not correspond to the actual device being
checked, which makes its upperbound check for array overflow useless.
Changing this to a do-while loop is the correct way of doing this.
There should be no possibility of spinning forever in this loop, as
pci_get_device states that it will go through all iterations, then
return NULL (thus breaking the loop).

Signed-off-by: Jon Mason <jdmason@kudzu.us>
Signed-off-by: Muli Ben-Yehuda <muli@il.ibm.com>

diff -r 28658cf477bc -r e0562474cf16 arch/x86_64/kernel/pci-calgary.c
--- a/arch/x86_64/kernel/pci-calgary.c	Sat Sep 30 11:16:12 2006 +0300
+++ b/arch/x86_64/kernel/pci-calgary.c	Sat Sep 30 11:18:31 2006 +0300
@@ -817,6 +817,8 @@ static int __init calgary_init_one(struc
 	void __iomem *bbar;
 	int ret;
 
+	BUG_ON(dev->bus->number >= MAX_PHB_BUS_NUM);
+
 	address = locate_register_space(dev);
 	/* map entire 1MB of Calgary config space */
 	bbar = ioremap_nocache(address, 1024 * 1024);
@@ -843,10 +845,10 @@ done:
 
 static int __init calgary_init(void)
 {
-	int i, ret = -ENODEV;
+	int ret = -ENODEV;
 	struct pci_dev *dev = NULL;
 
-	for (i = 0; i < MAX_PHB_BUS_NUM; i++) {
+	do {
 		dev = pci_get_device(PCI_VENDOR_ID_IBM,
 				     PCI_DEVICE_ID_IBM_CALGARY,
 				     dev);
@@ -862,12 +864,12 @@ static int __init calgary_init(void)
 		ret = calgary_init_one(dev);
 		if (ret)
 			goto error;
-	}
+	} while (1);
 
 	return ret;
 
 error:
-	for (i--; i >= 0; i--) {
+	do {
 		dev = pci_find_device_reverse(PCI_VENDOR_ID_IBM,
 					      PCI_DEVICE_ID_IBM_CALGARY,
 					      dev);
@@ -883,7 +885,7 @@ error:
 		calgary_disable_translation(dev);
 		calgary_free_bus(dev);
 		pci_dev_put(dev); /* Undo calgary_init_one()'s pci_dev_get() */
-	}
+	} while (1);
 
 	return ret;
 }

[PATCH 3 of 4] x86-64: Calgary IOMMU: Update Jon's contact info

[Muli Ben-Yehuda]

2 files changed, 3 insertions(+), 2 deletions(-)
MAINTAINERS                      |    2 +-
arch/x86_64/kernel/pci-calgary.c |    3 ++-


# HG changeset patch
# User Muli Ben-Yehuda <muli@il.ibm.com>
# Date 1159604379 -10800
# Node ID 7aff757cee0ea0566ae4016b43a0284769b0b27a
# Parent  e0562474cf16b13d8c3c815fce3159ba7cd0f540
x86-64: Calgary IOMMU: Update Jon's contact info

From: Jon Mason <jdmason@kudzu.us>

Also add copyright for work done after leaving IBM.

Signed-off-by: Jon Mason <jdmason@kudzu.us>
Signed-off-by: Muli Ben-Yehuda <muli@il.ibm.com>

diff -r e0562474cf16 -r 7aff757cee0e MAINTAINERS
--- a/MAINTAINERS	Sat Sep 30 11:18:31 2006 +0300
+++ b/MAINTAINERS	Sat Sep 30 11:19:39 2006 +0300
@@ -643,7 +643,7 @@ P:	Muli Ben-Yehuda
 P:	Muli Ben-Yehuda
 M:	muli@il.ibm.com
 P:	Jon D. Mason
-M:	jdmason@us.ibm.com
+M:	jdmason@kudzu.us
 L:	linux-kernel@vger.kernel.org
 L:	discuss@x86-64.org
 S:	Maintained
diff -r e0562474cf16 -r 7aff757cee0e arch/x86_64/kernel/pci-calgary.c
--- a/arch/x86_64/kernel/pci-calgary.c	Sat Sep 30 11:18:31 2006 +0300
+++ b/arch/x86_64/kernel/pci-calgary.c	Sat Sep 30 11:19:39 2006 +0300
@@ -2,8 +2,9 @@
  * Derived from arch/powerpc/kernel/iommu.c
  *
  * Copyright (C) IBM Corporation, 2006
+ * Copyright (C) 2006  Jon Mason <jdmason@kudzu.us>
  *
- * Author: Jon Mason <jdmason@us.ibm.com>
+ * Author: Jon Mason <jdmason@kudzu.us>
  * Author: Muli Ben-Yehuda <muli@il.ibm.com>
 
  * This program is free software; you can redistribute it and/or modify

[PATCH 4 of 4] x86-64: Calgary IOMMU: Fix off by one when calculating register space

[Muli Ben-Yehuda]

1 files changed, 11 insertions(+), 2 deletions(-)
arch/x86_64/kernel/pci-calgary.c |   13 +++++++++++--


# HG changeset patch
# User Muli Ben-Yehuda <muli@il.ibm.com>
# Date 1159604463 -10800
# Node ID 1e2a3d57541a0d31894c179ef45dd4c5a21ca308
# Parent  7aff757cee0ea0566ae4016b43a0284769b0b27a
x86-64: Calgary IOMMU: Fix off by one when calculating register space
location

From: Jon Mason <jdmason@kudzu.us>

The purpose of the code being modified is to determine the location
of the calgary chip address space.  This is done by a magical formula
of FE0MB-8MB*OneBasedChassisNumber+1MB*(RioNodeId-ChassisBase) to
find the offset where BIOS puts it.  In this formula,
OneBasedChassisNumber corresponds to the NUMA node, and rionodeid is
always 2 or 3 depending on which chip in the system it is.  The
problem was that we had an off by one error that caused us to account
some busses to the wrong chip and thus give them the wrong address
space.

Fixes RH bugzilla #203971.

Signed-off-by: Jon Mason <jdmason@kudzu.us>
Signed-off-bu: Muli Ben-Yehuda <muli@il.ibm.com>

diff -r 7aff757cee0e -r 1e2a3d57541a arch/x86_64/kernel/pci-calgary.c
--- a/arch/x86_64/kernel/pci-calgary.c	Sat Sep 30 11:19:39 2006 +0300
+++ b/arch/x86_64/kernel/pci-calgary.c	Sat Sep 30 11:21:03 2006 +0300
@@ -792,7 +792,16 @@ static inline unsigned int __init locate
 	int rionodeid;
 	u32 address;
 
-	rionodeid = (dev->bus->number % 15 > 4) ? 3 : 2;
+	/*
+	 * Each Calgary has four busses. The first four busses (first Calgary)
+	 * have RIO node ID 2, then the next four (second Calgary) have RIO
+	 * node ID 3, the next four (third Calgary) have node ID 2 again, etc.
+	 * We use a gross hack - relying on the dev->bus->number ordering,
+	 * modulo 14 - to decide which Calgary a given bus is on. Busses 0, 1,
+	 * 2 and 4 are on the first Calgary (id 2), 6, 8, a and c are on the
+	 * second (id 3), and then it repeats modulo 14.
+ 	 */
+	rionodeid = (dev->bus->number % 14 > 4) ? 3 : 2;
 	/*
 	 * register space address calculation as follows:
 	 * FE0MB-8MB*OneBasedChassisNumber+1MB*(RioNodeId-ChassisBase)
@@ -800,7 +809,7 @@ static inline unsigned int __init locate
 	 * RioNodeId is 2 for first Calgary, 3 for second Calgary
 	 */
 	address = START_ADDRESS	-
-		(0x800000 * (ONE_BASED_CHASSIS_NUM + dev->bus->number / 15)) +
+		(0x800000 * (ONE_BASED_CHASSIS_NUM + dev->bus->number / 14)) +
 		(0x100000) * (rionodeid - CHASSIS_BASE);
 	return address;
 }

Re: [PATCH 4 of 4] x86-64: Calgary IOMMU: Fix off by one when calculating register space

[Valdis.Kletnieks]

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

On Sat, 30 Sep 2006 11:43:32 +0300, Muli Ben-Yehuda said:

> +	 * Each Calgary has four busses. The first four busses (first Calgary)
> +	 * have RIO node ID 2, then the next four (second Calgary) have RIO
> +	 * node ID 3, the next four (third Calgary) have node ID 2 again, etc.
> +	 * We use a gross hack - relying on the dev->bus->number ordering,
> +	 * modulo 14 - to decide which Calgary a given bus is on. Busses 0, 1,
> +	 * 2 and 4 are on the first Calgary (id 2), 6, 8, a and c are on the
> +	 * second (id 3), and then it repeats modulo 14.
> + 	 */
> +	rionodeid = (dev->bus->number % 14 > 4) ? 3 : 2;

A quick perusal of the pci-calgary.c in 2.6.18-mm2 doesn't find a single
comment explaining where "6, 8, a, c" comes from, which makes that 14 seem
"magical" - are the 3rd Calgary's busses e,f, 10, and 12? It makes me wonder
why they didn't just blow 2 "reserved" numbers to make it mod 16 and an easier
decode. ;)

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

Re: [PATCH 0 of 4] x86-64: Calgary IOMMU updates

[Andi Kleen]

On Saturday 30 September 2006 10:43, Muli Ben-Yehuda wrote:
> Hi Andi,
> 
> [resending with proper CC's].
> 
> Please apply this Calgary patchset. It fixes one bug
> (https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=203971, should
> go to -stable as well) and makes several other updates all of which
> are safe for 2.6.19. Each patch has a detailed description.

Merged thanks.

It would be good if you could use a less weird patch format in the future 
though. In particular diffstat should be at the end and any other weird
metadata too. And the Subject shouldn't be wrapped.

Thanks,

-Andi

Re: [PATCH 0 of 4] x86-64: Calgary IOMMU updates

[Muli Ben-Yehuda]

On Sat, Sep 30, 2006 at 06:07:25PM +0200, Andi Kleen wrote:
> On Saturday 30 September 2006 10:43, Muli Ben-Yehuda wrote:
> > Hi Andi,
> > 
> > [resending with proper CC's].
> > 
> > Please apply this Calgary patchset. It fixes one bug
> > (https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=203971, should
> > go to -stable as well) and makes several other updates all of which
> > are safe for 2.6.19. Each patch has a detailed description.
> 
> Merged thanks.
> 
> It would be good if you could use a less weird patch format in the future 
> though. In particular diffstat should be at the end and any other weird
> metadata too. And the Subject shouldn't be wrapped.

Hmpf, silly hg email. Will fix.

Cheers,
Muli