Re: [openib-general] Ordering between PCI config space writes and MMIO reads?

From: JWM
Date: Tue Oct 24 2006 - 17:02:30 EST

On IA64 there are two memory barriers mf and mf.a. To protect against the scenario below mf.a (slower of course) would be required.
....JW
----- Original Message ----- From: "Roland Dreier" <rdreier@xxxxxxxxx>
To: <linux-pci@xxxxxxxxxxxxxxxxxxxxxxxx>; <linux-ia64@xxxxxxxxxxxxxxx>
Cc: <linux-kernel@xxxxxxxxxxxxxxx>; <openib-general@xxxxxxxxxx>
Sent: Tuesday, October 24, 2006 2:13 PM
Subject: [openib-general] Ordering between PCI config space writes and MMIO reads?


John Partridge found an interesting bug involving mthca (Mellanox
InfiniBand HCA driver) on IA64/Altix systems. Basically, during
initialization, mthca does:

- do some config writes, including enabling BARs
- then start a firmware command
- read an MMIO register from a BAR (to check if FW is busy)

However, John found that the Altix PCI-X bridge was allowing the MMIO
read to start before the config write was done (which is allowed by
the PCI spec). The PCI trace looked like:

23454: Config Write REG = 01 TYPE = 1 BE = 0000 Req = (0,0,0) Tag = 1 Bus = 1 Device = 0 Function = 0 WAIT = 2
23462: Memory Rd DW A = 00280698 BE = 0000 Req = (0,0,0) Tag = 0 WAIT = 2
23470: Split compl. Lower A = 00 Req = (0,0,0) Tag = 0 Comp = (0,2,0) WAIT = 1 (Error completion)
23476: Split compl. Lower A = 00 Req = (0,0,0) Tag = 1 Comp = (0,2,0) WAIT = 1 (Normal completion of WRITE)

and that "Error completion" leads to a crash.

John proposed the following patch to fix this, which looks good to
me. However, I have a couple of questions about this situation:

1) Is this something that should be fixed in the driver? The PCI
spec allows MMIO cycles to start before an earlier config cycle
completed, but do we want to expose this fact to drivers? Would
it be better for ia64 to use some sort of barrier to make sure
pci_write_config_xxx() is strongly ordered with MMIO?

2) Is this issue lurking in other drivers?

Thanks,
Roland

commit 424b50b6360b325ce642ece687756a600c25d28a
Author: John Partridge <johnip@xxxxxxx>
Date: Tue Oct 24 11:54:16 2006 -0700

IB/mthca: Make sure all PCI config writes reach device before doing MMIO

During initialization, mthca writes some PCI config space registers
and then does an MMIO read from one of the BARs it just enabled. This
MMIO read sometimes failed and caused a crash on SGI Altix machines,
because the PCI-X host bridge (legitimately, according to the PCI
spec) allowed the MMIO read to start before the config write completed.

To fix this, add a config read after all config writes to make sure
they are all done before starting the MMIO read.

Signed-off-by: John Partridge <johnip@xxxxxxx>
Signed-off-by: Roland Dreier <rolandd@xxxxxxxxx>

diff --git a/drivers/infiniband/hw/mthca/mthca_reset.c b/drivers/infiniband/hw/mthca/mthca_reset.c
index 91934f2..578dc7c 100644
--- a/drivers/infiniband/hw/mthca/mthca_reset.c
+++ b/drivers/infiniband/hw/mthca/mthca_reset.c
@@ -281,6 +281,20 @@ good:
goto out;
}

+ /*
+ * Perform a "flush" of the PCI config writes here by reading
+ * the PCI_COMMAND register. This is needed to make sure that
+ * we don't try to touch other PCI BARs before the config
+ * writes are done -- otherwise an MMIO cycle could start
+ * before the config writes are done and reach the HCA before
+ * the BAR is actually enabled.
+ */
+ if (pci_read_config_dword(mdev->pdev, PCI_COMMAND, hca_header)) {
+ err = -ENODEV;
+ mthca_err(mdev, "Couldn't access HCA memory after restoring, "
+ "aborting.\n");
+ }
+
out:
if (bridge)
pci_dev_put(bridge);

_______________________________________________
openib-general mailing list
openib-general@xxxxxxxxxx
http://openib.org/mailman/listinfo/openib-general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general



-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/