From: "JWM" <jwm@systemfabricworks.com>
To: <linux-pci@atrey.karlin.mff.cuni.cz>,
<linux-ia64@vger.kernel.org>, "Roland Dreier" <rdreier@cisco.com>
Cc: <linux-kernel@vger.kernel.org>, <openib-general@openib.org>
Subject: Re: [openib-general] Ordering between PCI config space writes and MMIO reads?
Date: Tue, 24 Oct 2006 16:01:25 -0500 [thread overview]
Message-ID: <00df01c6f7af$91af9aa0$7401a8c0@Maelstrom> (raw)
In-Reply-To: adafyddcysw.fsf@cisco.com
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@cisco.com>
To: <linux-pci@atrey.karlin.mff.cuni.cz>; <linux-ia64@vger.kernel.org>
Cc: <linux-kernel@vger.kernel.org>; <openib-general@openib.org>
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@sgi.com>
> 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@sgi.com>
> Signed-off-by: Roland Dreier <rolandd@cisco.com>
>
> 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@openib.org
> http://openib.org/mailman/listinfo/openib-general
>
> To unsubscribe, please visit
> http://openib.org/mailman/listinfo/openib-general
>
>
next prev parent reply other threads:[~2006-10-24 21:02 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2006-10-24 19:13 Ordering between PCI config space writes and MMIO reads? Roland Dreier
2006-10-24 19:22 ` Jeff Garzik
2006-10-24 21:47 ` Matthew Wilcox
2006-10-24 21:51 ` Roland Dreier
2006-10-24 22:12 ` John Partridge
2006-10-24 22:36 ` Matthew Wilcox
2006-10-24 22:43 ` David Miller
2006-10-25 14:15 ` Roland Dreier
2006-10-31 19:02 ` Roland Dreier
2006-10-31 19:53 ` Michael S. Tsirkin
2006-10-31 19:53 ` Roland Dreier
2006-10-31 19:58 ` Matthew Wilcox
2006-10-31 20:28 ` Michael S. Tsirkin
2006-10-31 20:34 ` Richard B. Johnson
2006-10-31 20:47 ` Matthew Wilcox
2006-10-31 22:30 ` Roland Dreier
2006-11-01 16:27 ` John Partridge
2006-11-01 16:46 ` Matthew Wilcox
2006-11-01 17:08 ` John Partridge
2006-11-01 17:14 ` Matthew Wilcox
2006-11-01 23:04 ` David Miller
2006-11-02 1:08 ` John Partridge
2006-10-31 20:50 ` Michael S. Tsirkin
2006-10-24 22:59 ` [openib-general] " Jason Gunthorpe
2006-10-25 14:04 ` Roland Dreier
2006-10-24 23:09 ` Michael S. Tsirkin
2006-10-24 23:27 ` Jack Steiner
2006-10-25 14:05 ` Roland Dreier
2006-11-02 3:05 ` Jeremy Higdon
2006-10-24 21:01 ` JWM [this message]
2006-10-24 21:24 ` Alan Cox
2006-10-24 21:29 ` Roland Dreier
2006-10-24 21:37 ` Jeff Garzik
2006-10-25 6:30 ` Grant Grundler
2006-10-25 14:11 ` Roland Dreier
2006-10-25 14:18 ` Matthew Wilcox
2006-10-25 17:15 ` [openib-general] " Jason Gunthorpe
2006-10-25 18:22 ` Michael S. Tsirkin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='00df01c6f7af$91af9aa0$7401a8c0@Maelstrom' \
--to=jwm@systemfabricworks.com \
--cc=linux-ia64@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@atrey.karlin.mff.cuni.cz \
--cc=openib-general@openib.org \
--cc=rdreier@cisco.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox