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 ++++++++++++++++++++++++------------
1 files changed, 7 insertions(+), 5 deletions(-)
arch/x86_64/kernel/pci-calgary.c | 12 +++++++-----
# HG changeset patch
# User Muli Ben-Yehuda <[email protected]>
# Date 1159604311 -10800
# Node ID e0562474cf16b13d8c3c815fce3159ba7cd0f540
# Parent 28658cf477bc8c6adc5a5335363a4d1428f58273
x86-64: Calgary IOMMU: deobfuscate calgary_init
From: Jon Mason <[email protected]>
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 <[email protected]>
Signed-off-by: Muli Ben-Yehuda <[email protected]>
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;
}
1 files changed, 4 insertions(+), 4 deletions(-)
arch/x86_64/kernel/pci-calgary.c | 8 ++++----
# HG changeset patch
# User Muli Ben-Yehuda <[email protected]>
# Date 1159604172 -10800
# Node ID 28658cf477bc8c6adc5a5335363a4d1428f58273
# Parent 585e6c1736a2a8b419ae9b8dc5055ee8774ba57f
x86-64: Calgary IOMMU: print PCI bus numbers in hex
From: Jon Mason <[email protected]>
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 <[email protected]>
Signed-off-by: Muli Ben-Yehuda <[email protected]>
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;
}
}
2 files changed, 3 insertions(+), 2 deletions(-)
MAINTAINERS | 2 +-
arch/x86_64/kernel/pci-calgary.c | 3 ++-
# HG changeset patch
# User Muli Ben-Yehuda <[email protected]>
# Date 1159604379 -10800
# Node ID 7aff757cee0ea0566ae4016b43a0284769b0b27a
# Parent e0562474cf16b13d8c3c815fce3159ba7cd0f540
x86-64: Calgary IOMMU: Update Jon's contact info
From: Jon Mason <[email protected]>
Also add copyright for work done after leaving IBM.
Signed-off-by: Jon Mason <[email protected]>
Signed-off-by: Muli Ben-Yehuda <[email protected]>
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: [email protected]
P: Jon D. Mason
-M: [email protected]
+M: [email protected]
L: [email protected]
L: [email protected]
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 <[email protected]>
*
- * Author: Jon Mason <[email protected]>
+ * Author: Jon Mason <[email protected]>
* Author: Muli Ben-Yehuda <[email protected]>
* This program is free software; you can redistribute it and/or modify
1 files changed, 11 insertions(+), 2 deletions(-)
arch/x86_64/kernel/pci-calgary.c | 13 +++++++++++--
# HG changeset patch
# User Muli Ben-Yehuda <[email protected]>
# Date 1159604463 -10800
# Node ID 1e2a3d57541a0d31894c179ef45dd4c5a21ca308
# Parent 7aff757cee0ea0566ae4016b43a0284769b0b27a
x86-64: Calgary IOMMU: Fix off by one when calculating register space
location
From: Jon Mason <[email protected]>
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 <[email protected]>
Signed-off-bu: Muli Ben-Yehuda <[email protected]>
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;
}
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. ;)
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
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