[PATCH] pci: Add pci quirk to turnoff Nosnoop and Relaxed Ordering

[PATCH] pci: Add pci quirk to turnoff Nosnoop and Relaxed Ordering

[Hariprasad Shenai]

Some devices violate the PCI Specification regarding echoing the Root
Port Transaction Layer Packet Request (TLP) No Snoop and Relaxed
Ordering Attributes into the TLP Response. The PCI Specification
"encourages" compliant Root Port implementation to drop such
malformed TLP Responses leading to device access timeouts. Many Root Port
implementations accept such malformed TLP Responses and a few more
strict implementations do drop them.

For devices which fail this part of the PCI Specification, we need to
traverse up the PCI Chain to the Root Port and turn off the Enable No
Snoop and Enable Relaxed Ordering bits in the Root Port's PCI-Express
Device Control register. This does affect all other devices which
are downstream of that Root Port.

Note that Configuration Space accesses are never supposed to have TLP
Attributes, so we're safe waiting till after any Configuration Space
accesses to do the Root Port "fixup".

Based on original work by Casey Leedom <leedom@chelsio.com>

Signed-off-by: Hariprasad Shenai <hariprasad@chelsio.com>
---
 drivers/pci/pci.c    | 28 ++++++++++++++++++++++++++
 drivers/pci/quirks.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pci.h  |  1 +
 3 files changed, 85 insertions(+)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 6a9a111..3ce202b 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -458,6 +458,34 @@ struct resource *pci_find_parent_resource(const struct pci_dev *dev,
 EXPORT_SYMBOL(pci_find_parent_resource);
 
 /**
+ * pci_find_root_pcie_port - return PCI-E Root Port
+ * @dev: PCI device to query
+ *
+ * Traverses up the parent chain and return the PCI-E Root Port PCI Device
+ * for a given PCI Device.
+ */
+struct pci_dev *pci_find_root_pcie_port(const struct pci_dev *dev)
+{
+	struct pci_bus *bus = dev->bus;
+	struct pci_dev *highest_pcie_bridge = NULL;
+
+	while (bus) {
+		struct pci_dev *bridge = bus->self;
+
+		if (!bridge || !bridge->pcie_cap)
+			break;
+		highest_pcie_bridge = bridge;
+		bus = bus->parent;
+	}
+
+	if (!highest_pcie_bridge)
+		dev_warn(&dev->dev, "Can't find Root Port\n");
+
+	return highest_pcie_bridge;
+}
+EXPORT_SYMBOL(pci_find_root_pcie_port);
+
+/**
  * pci_wait_for_pending - wait for @mask bit(s) to clear in status word @pos
  * @dev: the PCI device to operate on
  * @pos: config space offset of status word
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 6a30252..f860956 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3692,6 +3692,62 @@ DECLARE_PCI_FIXUP_CLASS_EARLY(0x1797, 0x6869, PCI_CLASS_NOT_DEFINED, 8,
 			      quirk_tw686x_class);
 
 /*
+ * Some devices violate the PCI Specification regarding echoing the Root
+ * Port Transaction Layer Packet Request (TLP) No Snoop and Relaxed
+ * Ordering Attributes into the TLP Response.  The PCI Specification
+ * "encourages" compliant Root Port implementation to drop such malformed
+ * TLP Responses leading to device access timeouts.  Many Root Port
+ * implementations accept such malformed TLP Responses and a few more strict
+ * implementations do drop them.
+ *
+ * For devices which fail this part of the PCI Specification, we need to
+ * traverse up the PCI Chain to the Root Port and turn off the Enable No
+ * Snoop and Enable Relaxed Ordering bits in the Root Port's PCI-Express
+ * Device Control register.  This does affect all other devices which are
+ * downstream of that Root Port but since No Snoop and Relaxed ordering are
+ * "Performance Hints," we're okay with that ...
+ *
+ * Note that Configuration Space accesses are never supposed to have TLP
+ * Attributes, so we're safe waiting till after any Configuration Space
+ * accesses to do the Root Port "fixup" ...
+ */
+static void quirk_disable_root_port_attributes(struct pci_dev *pdev)
+{
+	struct pci_dev *root_port = pci_find_root_pcie_port(pdev);
+
+	if (!root_port) {
+		dev_warn(&pdev->dev, "Can't find Root Port to disable No Snoop/Relaxed Ordering\n");
+		return;
+	}
+
+	dev_info(&pdev->dev, "Disabling No Snoop/Relaxed Ordering on Root Port %s\n",
+		 dev_name(&root_port->dev));
+	pcie_capability_clear_and_set_word(root_port,
+					   PCI_EXP_DEVCTL,
+					   PCI_EXP_DEVCTL_RELAX_EN |
+					   PCI_EXP_DEVCTL_NOSNOOP_EN,
+					   0);
+}
+
+/*
+ * The Chelsio T5 chip fails to return the Root Port's TLP Attributes in
+ * its TLP responses to the Root Port.
+ */
+static void quirk_chelsio_T5_disable_root_port_attributes(struct pci_dev *pdev)
+{
+	/*
+	 * This mask/compare operation selects for Physical Function 4 on a
+	 * T5.  We only need to fix up the Root Port once for any of the
+	 * PFs.  PF[0..3] have PCI Device IDs of 0x50xx, but PF4 is uniquely
+	 * 0x54xx so we use that one,
+	 */
+	if ((pdev->device & 0xff00) == 0x5400)
+		quirk_disable_root_port_attributes(pdev);
+}
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_CHELSIO, PCI_ANY_ID,
+			 quirk_chelsio_T5_disable_root_port_attributes);
+
+/*
  * AMD has indicated that the devices below do not support peer-to-peer
  * in any system where they are found in the southbridge with an AMD
  * IOMMU in the system.  Multifunction devices that do not support
diff --git a/include/linux/pci.h b/include/linux/pci.h
index e90eb22..5b4d7cc 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -820,6 +820,7 @@ void pci_bus_add_device(struct pci_dev *dev);
 void pci_read_bridge_bases(struct pci_bus *child);
 struct resource *pci_find_parent_resource(const struct pci_dev *dev,
 					  struct resource *res);
+struct pci_dev *pci_find_root_pcie_port(const struct pci_dev *dev);
 u8 pci_swizzle_interrupt_pin(const struct pci_dev *dev, u8 pin);
 int pci_get_interrupt_pin(struct pci_dev *dev, struct pci_dev **bridge);
 u8 pci_common_swizzle(struct pci_dev *dev, u8 *pinp);
-- 
2.3.4

Re: [PATCH] pci: Add pci quirk to turnoff Nosnoop and Relaxed Ordering

[Bjorn Helgaas]

Hi Hariprasad & Casey,

On Sun, Oct 18, 2015 at 07:55:04PM +0530, Hariprasad Shenai wrote:
> Some devices violate the PCI Specification regarding echoing the Root
> Port Transaction Layer Packet Request (TLP) No Snoop and Relaxed
> Ordering Attributes into the TLP Response. The PCI Specification
> "encourages" compliant Root Port implementation to drop such
> malformed TLP Responses leading to device access timeouts. Many Root Port
> implementations accept such malformed TLP Responses and a few more
> strict implementations do drop them.
> 
> For devices which fail this part of the PCI Specification, we need to
> traverse up the PCI Chain to the Root Port and turn off the Enable No
> Snoop and Enable Relaxed Ordering bits in the Root Port's PCI-Express
> Device Control register. This does affect all other devices which
> are downstream of that Root Port.
> 
> Note that Configuration Space accesses are never supposed to have TLP
> Attributes, so we're safe waiting till after any Configuration Space
> accesses to do the Root Port "fixup".
> 
> Based on original work by Casey Leedom <leedom@chelsio.com>
> 
> Signed-off-by: Hariprasad Shenai <hariprasad@chelsio.com>

I reworked this and applied the patch below to pci/misc for v4.4.

Please try it out to make sure I didn't break something.  Here's what I
changed:

  - Changelog and comments to include specific spec references (they
    were in Casey's original posts but got lost).

  - Renamed to pci_find_pcie_root_port() to keep "root_port" together.

  - Reworked to use pci_upstream_bridge() to avoid issues if we're
    under a "virtual bus" used for SR-IOV (this isn't likely to be a
    problem, but it's hard to prove it's not).

  - Added a check for "Root Port" device type, with the intent to use
    this other places, e.g., to replace pcie_find_root_port().

  - Removed the "Can't find Root Port" message; if we use
    pci_find_pcie_root_port() generically, the caller should be
    responsible for deciding if a message is appropriate.

  - Reworded quirk "Can't find Root Port to disable No Snoop/Relaxed
    Ordering" message to make it clear that we're trying to work
    around a device erratum.  I don't think the original message had
    enough context to be useful.

  - Reworded "Disabling No Snoop/Relaxed Ordering on Root Port %s'
    message, again to make the reason clear (device erratum) and to
    use the root port as the affected device.

Bjorn


commit 4cdcda8244d4c7b8a0e37fe2ce8d11d4633c6687
Author: Hariprasad Shenai <hariprasad@chelsio.com>
Date:   Sun Oct 18 19:55:04 2015 +0530

    PCI: Turn off Request Attributes to avoid Chelsio T5 Completion erratum
    
    The Chelsio T5 has a PCIe compliance erratum that causes Malformed TLP or
    Unexpected Completion errors in some systems, which may cause device access
    timeouts.
    
    Per PCIe r3.0, sec 2.2.9, "Completion headers must supply the same values
    for the Attribute as were supplied in the header of the corresponding
    Request, except as explicitly allowed when IDO is used."
    
    Instead of copying the Attributes from the Request to the Completion, the
    T5 always generates Completions with zero Attributes.  The receiver of a
    Completion whose Attributes don't match the Request may accept it (which
    itself seems non-compliant based on sec 2.3.2), or it may handle it as a
    Malformed TLP or an Unexpected Completion, which will probably lead to a
    device access timeout.
    
    Work around this by disabling "Relaxed Ordering" and "No Snoop" in the Root
    Port so it always generate Requests with zero Attributes.
    
    This does affect all other devices which are downstream of that Root Port,
    but these are performance optimizations that should not make a functional
    difference.
    
    Note that Configuration Space accesses are never supposed to have TLP
    Attributes, so we're safe waiting till after any Configuration Space
    accesses to do the Root Port "fixup".
    
    Based on original work by Casey Leedom <leedom@chelsio.com>
    
    [bhelgaas: changelog, comments, rename to pci_find_pcie_root_port(), rework
    to use pci_upstream_bridge() and check for Root Port device type, edit
    diagnostics to clarify intent and devices affected]
    Signed-off-by: Hariprasad Shenai <hariprasad@chelsio.com>
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 6a9a111..3dacbeb 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -458,6 +458,30 @@ struct resource *pci_find_parent_resource(const struct pci_dev *dev,
 EXPORT_SYMBOL(pci_find_parent_resource);
 
 /**
+ * pci_find_pcie_root_port - return PCIe Root Port
+ * @dev: PCI device to query
+ *
+ * Traverse up the parent chain and return the PCIe Root Port PCI Device
+ * for a given PCI Device.
+ */
+struct pci_dev *pci_find_pcie_root_port(const struct pci_dev *dev)
+{
+	struct pci_dev *bridge, *highest_pcie_bridge = NULL;
+
+	bridge = pci_upstream_bridge(dev);
+	while (bridge && pci_is_pcie(bridge)) {
+		highest_pcie_bridge = bridge;
+		bridge = pci_upstream_bridge(bridge);
+	}
+
+	if (pci_pcie_type(highest_pcie_bridge) != PCI_EXP_TYPE_ROOT_PORT)
+		return NULL;
+
+	return highest_pcie_bridge;
+}
+EXPORT_SYMBOL(pci_find_pcie_root_port);
+
+/**
  * pci_wait_for_pending - wait for @mask bit(s) to clear in status word @pos
  * @dev: the PCI device to operate on
  * @pos: config space offset of status word
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 6a30252..eb3c98e 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3692,6 +3692,63 @@ DECLARE_PCI_FIXUP_CLASS_EARLY(0x1797, 0x6869, PCI_CLASS_NOT_DEFINED, 8,
 			      quirk_tw686x_class);
 
 /*
+ * Per PCIe r3.0, sec 2.2.9, "Completion headers must supply the same
+ * values for the Attribute as were supplied in the header of the
+ * corresponding Request, except as explicitly allowed when IDO is used."
+ *
+ * If a non-compliant device generates a completion with a different
+ * attribute than the request, the receiver may accept it (which itself
+ * seems non-compliant based on sec 2.3.2), or it may handle it as a
+ * Malformed TLP or an Unexpected Completion, which will probably lead to a
+ * device access timeout.
+ *
+ * If the non-compliant device generates completions with zero attributes
+ * (instead of copying the attributes from the request), we can work around
+ * this by disabling the "Relaxed Ordering" and "No Snoop" attributes in
+ * upstream devices so they always generate requests with zero attributes.
+ *
+ * This affects other devices under the same Root Port, but since these
+ * attributes are performance hints, there should be no functional problem.
+ *
+ * Note that Configuration Space accesses are never supposed to have TLP
+ * Attributes, so we're safe waiting till after any Configuration Space
+ * accesses to do the Root Port fixup.
+ */
+static void quirk_disable_root_port_attributes(struct pci_dev *pdev)
+{
+	struct pci_dev *root_port = pci_find_pcie_root_port(pdev);
+
+	if (!root_port) {
+		dev_warn(&pdev->dev, "PCIe Completion erratum may cause device errors\n");
+		return;
+	}
+
+	dev_info(&root_port->dev, "Disabling No Snoop/Relaxed Ordering Attributes to avoid PCIe Completion erratum in %s\n",
+		 dev_name(&pdev->dev));
+	pcie_capability_clear_and_set_word(root_port, PCI_EXP_DEVCTL,
+					   PCI_EXP_DEVCTL_RELAX_EN |
+					   PCI_EXP_DEVCTL_NOSNOOP_EN, 0);
+}
+
+/*
+ * The Chelsio T5 chip fails to copy TLP Attributes from a Request to the
+ * Completion it generates.
+ */
+static void quirk_chelsio_T5_disable_root_port_attributes(struct pci_dev *pdev)
+{
+	/*
+	 * This mask/compare operation selects for Physical Function 4 on a
+	 * T5.  We only need to fix up the Root Port once for any of the
+	 * PFs.  PF[0..3] have PCI Device IDs of 0x50xx, but PF4 is uniquely
+	 * 0x54xx so we use that one,
+	 */
+	if ((pdev->device & 0xff00) == 0x5400)
+		quirk_disable_root_port_attributes(pdev);
+}
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_CHELSIO, PCI_ANY_ID,
+			 quirk_chelsio_T5_disable_root_port_attributes);
+
+/*
  * AMD has indicated that the devices below do not support peer-to-peer
  * in any system where they are found in the southbridge with an AMD
  * IOMMU in the system.  Multifunction devices that do not support
diff --git a/include/linux/pci.h b/include/linux/pci.h
index b54fbf1..0f3696c 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -820,6 +820,7 @@ void pci_bus_add_device(struct pci_dev *dev);
 void pci_read_bridge_bases(struct pci_bus *child);
 struct resource *pci_find_parent_resource(const struct pci_dev *dev,
 					  struct resource *res);
+struct pci_dev *pci_find_root_pcie_port(const struct pci_dev *dev);
 u8 pci_swizzle_interrupt_pin(const struct pci_dev *dev, u8 pin);
 int pci_get_interrupt_pin(struct pci_dev *dev, struct pci_dev **bridge);
 u8 pci_common_swizzle(struct pci_dev *dev, u8 *pinp);

Re: [PATCH] pci: Add pci quirk to turnoff Nosnoop and Relaxed Ordering

[Casey Leedom]

  Thanks Bjorn!  We really appreciate your help in getting this in.  We’ll check your version of the patch ASAP.

Casey

> On Oct 22, 2015, at 3:13 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:
> 
> Hi Hariprasad & Casey,
> 
> On Sun, Oct 18, 2015 at 07:55:04PM +0530, Hariprasad Shenai wrote:
>> Some devices violate the PCI Specification regarding echoing the Root
>> Port Transaction Layer Packet Request (TLP) No Snoop and Relaxed
>> Ordering Attributes into the TLP Response. The PCI Specification
>> "encourages" compliant Root Port implementation to drop such
>> malformed TLP Responses leading to device access timeouts. Many Root Port
>> implementations accept such malformed TLP Responses and a few more
>> strict implementations do drop them.
>> 
>> For devices which fail this part of the PCI Specification, we need to
>> traverse up the PCI Chain to the Root Port and turn off the Enable No
>> Snoop and Enable Relaxed Ordering bits in the Root Port's PCI-Express
>> Device Control register. This does affect all other devices which
>> are downstream of that Root Port.
>> 
>> Note that Configuration Space accesses are never supposed to have TLP
>> Attributes, so we're safe waiting till after any Configuration Space
>> accesses to do the Root Port "fixup".
>> 
>> Based on original work by Casey Leedom <leedom@chelsio.com>
>> 
>> Signed-off-by: Hariprasad Shenai <hariprasad@chelsio.com>
> 
> I reworked this and applied the patch below to pci/misc for v4.4.
> 
> Please try it out to make sure I didn't break something.  Here's what I
> changed:
> 
>  - Changelog and comments to include specific spec references (they
>    were in Casey's original posts but got lost).
> 
>  - Renamed to pci_find_pcie_root_port() to keep "root_port" together.
> 
>  - Reworked to use pci_upstream_bridge() to avoid issues if we're
>    under a "virtual bus" used for SR-IOV (this isn't likely to be a
>    problem, but it's hard to prove it's not).
> 
>  - Added a check for "Root Port" device type, with the intent to use
>    this other places, e.g., to replace pcie_find_root_port().
> 
>  - Removed the "Can't find Root Port" message; if we use
>    pci_find_pcie_root_port() generically, the caller should be
>    responsible for deciding if a message is appropriate.
> 
>  - Reworded quirk "Can't find Root Port to disable No Snoop/Relaxed
>    Ordering" message to make it clear that we're trying to work
>    around a device erratum.  I don't think the original message had
>    enough context to be useful.
> 
>  - Reworded "Disabling No Snoop/Relaxed Ordering on Root Port %s'
>    message, again to make the reason clear (device erratum) and to
>    use the root port as the affected device.
> 
> Bjorn
> 
> 
> commit 4cdcda8244d4c7b8a0e37fe2ce8d11d4633c6687
> Author: Hariprasad Shenai <hariprasad@chelsio.com>
> Date:   Sun Oct 18 19:55:04 2015 +0530
> 
>    PCI: Turn off Request Attributes to avoid Chelsio T5 Completion erratum
> 
>    The Chelsio T5 has a PCIe compliance erratum that causes Malformed TLP or
>    Unexpected Completion errors in some systems, which may cause device access
>    timeouts.
> 
>    Per PCIe r3.0, sec 2.2.9, "Completion headers must supply the same values
>    for the Attribute as were supplied in the header of the corresponding
>    Request, except as explicitly allowed when IDO is used."
> 
>    Instead of copying the Attributes from the Request to the Completion, the
>    T5 always generates Completions with zero Attributes.  The receiver of a
>    Completion whose Attributes don't match the Request may accept it (which
>    itself seems non-compliant based on sec 2.3.2), or it may handle it as a
>    Malformed TLP or an Unexpected Completion, which will probably lead to a
>    device access timeout.
> 
>    Work around this by disabling "Relaxed Ordering" and "No Snoop" in the Root
>    Port so it always generate Requests with zero Attributes.
> 
>    This does affect all other devices which are downstream of that Root Port,
>    but these are performance optimizations that should not make a functional
>    difference.
> 
>    Note that Configuration Space accesses are never supposed to have TLP
>    Attributes, so we're safe waiting till after any Configuration Space
>    accesses to do the Root Port "fixup".
> 
>    Based on original work by Casey Leedom <leedom@chelsio.com>
> 
>    [bhelgaas: changelog, comments, rename to pci_find_pcie_root_port(), rework
>    to use pci_upstream_bridge() and check for Root Port device type, edit
>    diagnostics to clarify intent and devices affected]
>    Signed-off-by: Hariprasad Shenai <hariprasad@chelsio.com>
>    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 6a9a111..3dacbeb 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -458,6 +458,30 @@ struct resource *pci_find_parent_resource(const struct pci_dev *dev,
> EXPORT_SYMBOL(pci_find_parent_resource);
> 
> /**
> + * pci_find_pcie_root_port - return PCIe Root Port
> + * @dev: PCI device to query
> + *
> + * Traverse up the parent chain and return the PCIe Root Port PCI Device
> + * for a given PCI Device.
> + */
> +struct pci_dev *pci_find_pcie_root_port(const struct pci_dev *dev)
> +{
> +	struct pci_dev *bridge, *highest_pcie_bridge = NULL;
> +
> +	bridge = pci_upstream_bridge(dev);
> +	while (bridge && pci_is_pcie(bridge)) {
> +		highest_pcie_bridge = bridge;
> +		bridge = pci_upstream_bridge(bridge);
> +	}
> +
> +	if (pci_pcie_type(highest_pcie_bridge) != PCI_EXP_TYPE_ROOT_PORT)
> +		return NULL;
> +
> +	return highest_pcie_bridge;
> +}
> +EXPORT_SYMBOL(pci_find_pcie_root_port);
> +
> +/**
>  * pci_wait_for_pending - wait for @mask bit(s) to clear in status word @pos
>  * @dev: the PCI device to operate on
>  * @pos: config space offset of status word
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 6a30252..eb3c98e 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -3692,6 +3692,63 @@ DECLARE_PCI_FIXUP_CLASS_EARLY(0x1797, 0x6869, PCI_CLASS_NOT_DEFINED, 8,
> 			      quirk_tw686x_class);
> 
> /*
> + * Per PCIe r3.0, sec 2.2.9, "Completion headers must supply the same
> + * values for the Attribute as were supplied in the header of the
> + * corresponding Request, except as explicitly allowed when IDO is used."
> + *
> + * If a non-compliant device generates a completion with a different
> + * attribute than the request, the receiver may accept it (which itself
> + * seems non-compliant based on sec 2.3.2), or it may handle it as a
> + * Malformed TLP or an Unexpected Completion, which will probably lead to a
> + * device access timeout.
> + *
> + * If the non-compliant device generates completions with zero attributes
> + * (instead of copying the attributes from the request), we can work around
> + * this by disabling the "Relaxed Ordering" and "No Snoop" attributes in
> + * upstream devices so they always generate requests with zero attributes.
> + *
> + * This affects other devices under the same Root Port, but since these
> + * attributes are performance hints, there should be no functional problem.
> + *
> + * Note that Configuration Space accesses are never supposed to have TLP
> + * Attributes, so we're safe waiting till after any Configuration Space
> + * accesses to do the Root Port fixup.
> + */
> +static void quirk_disable_root_port_attributes(struct pci_dev *pdev)
> +{
> +	struct pci_dev *root_port = pci_find_pcie_root_port(pdev);
> +
> +	if (!root_port) {
> +		dev_warn(&pdev->dev, "PCIe Completion erratum may cause device errors\n");
> +		return;
> +	}
> +
> +	dev_info(&root_port->dev, "Disabling No Snoop/Relaxed Ordering Attributes to avoid PCIe Completion erratum in %s\n",
> +		 dev_name(&pdev->dev));
> +	pcie_capability_clear_and_set_word(root_port, PCI_EXP_DEVCTL,
> +					   PCI_EXP_DEVCTL_RELAX_EN |
> +					   PCI_EXP_DEVCTL_NOSNOOP_EN, 0);
> +}
> +
> +/*
> + * The Chelsio T5 chip fails to copy TLP Attributes from a Request to the
> + * Completion it generates.
> + */
> +static void quirk_chelsio_T5_disable_root_port_attributes(struct pci_dev *pdev)
> +{
> +	/*
> +	 * This mask/compare operation selects for Physical Function 4 on a
> +	 * T5.  We only need to fix up the Root Port once for any of the
> +	 * PFs.  PF[0..3] have PCI Device IDs of 0x50xx, but PF4 is uniquely
> +	 * 0x54xx so we use that one,
> +	 */
> +	if ((pdev->device & 0xff00) == 0x5400)
> +		quirk_disable_root_port_attributes(pdev);
> +}
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_CHELSIO, PCI_ANY_ID,
> +			 quirk_chelsio_T5_disable_root_port_attributes);
> +
> +/*
>  * AMD has indicated that the devices below do not support peer-to-peer
>  * in any system where they are found in the southbridge with an AMD
>  * IOMMU in the system.  Multifunction devices that do not support
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index b54fbf1..0f3696c 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -820,6 +820,7 @@ void pci_bus_add_device(struct pci_dev *dev);
> void pci_read_bridge_bases(struct pci_bus *child);
> struct resource *pci_find_parent_resource(const struct pci_dev *dev,
> 					  struct resource *res);
> +struct pci_dev *pci_find_root_pcie_port(const struct pci_dev *dev);
> u8 pci_swizzle_interrupt_pin(const struct pci_dev *dev, u8 pin);
> int pci_get_interrupt_pin(struct pci_dev *dev, struct pci_dev **bridge);
> u8 pci_common_swizzle(struct pci_dev *dev, u8 *pinp);

Re: [PATCH] pci: Add pci quirk to turnoff Nosnoop and Relaxed Ordering

[Bjorn Helgaas]

On Thu, Oct 22, 2015 at 05:07:38PM -0700, Casey Leedom wrote:
>   Thanks Bjorn!  We really appreciate your help in getting this in.  We’ll check your version of the patch ASAP.

How embarrassing.  The patch below doesn't even compile because of a typo
and some const issues.  I updated it and repushed pci/misc.

> > On Oct 22, 2015, at 3:13 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:
> > 
> > Hi Hariprasad & Casey,
> > 
> > On Sun, Oct 18, 2015 at 07:55:04PM +0530, Hariprasad Shenai wrote:
> >> Some devices violate the PCI Specification regarding echoing the Root
> >> Port Transaction Layer Packet Request (TLP) No Snoop and Relaxed
> >> Ordering Attributes into the TLP Response. The PCI Specification
> >> "encourages" compliant Root Port implementation to drop such
> >> malformed TLP Responses leading to device access timeouts. Many Root Port
> >> implementations accept such malformed TLP Responses and a few more
> >> strict implementations do drop them.
> >> 
> >> For devices which fail this part of the PCI Specification, we need to
> >> traverse up the PCI Chain to the Root Port and turn off the Enable No
> >> Snoop and Enable Relaxed Ordering bits in the Root Port's PCI-Express
> >> Device Control register. This does affect all other devices which
> >> are downstream of that Root Port.
> >> 
> >> Note that Configuration Space accesses are never supposed to have TLP
> >> Attributes, so we're safe waiting till after any Configuration Space
> >> accesses to do the Root Port "fixup".
> >> 
> >> Based on original work by Casey Leedom <leedom@chelsio.com>
> >> 
> >> Signed-off-by: Hariprasad Shenai <hariprasad@chelsio.com>
> > 
> > I reworked this and applied the patch below to pci/misc for v4.4.
> > 
> > Please try it out to make sure I didn't break something.  Here's what I
> > changed:
> > 
> >  - Changelog and comments to include specific spec references (they
> >    were in Casey's original posts but got lost).
> > 
> >  - Renamed to pci_find_pcie_root_port() to keep "root_port" together.
> > 
> >  - Reworked to use pci_upstream_bridge() to avoid issues if we're
> >    under a "virtual bus" used for SR-IOV (this isn't likely to be a
> >    problem, but it's hard to prove it's not).
> > 
> >  - Added a check for "Root Port" device type, with the intent to use
> >    this other places, e.g., to replace pcie_find_root_port().
> > 
> >  - Removed the "Can't find Root Port" message; if we use
> >    pci_find_pcie_root_port() generically, the caller should be
> >    responsible for deciding if a message is appropriate.
> > 
> >  - Reworded quirk "Can't find Root Port to disable No Snoop/Relaxed
> >    Ordering" message to make it clear that we're trying to work
> >    around a device erratum.  I don't think the original message had
> >    enough context to be useful.
> > 
> >  - Reworded "Disabling No Snoop/Relaxed Ordering on Root Port %s'
> >    message, again to make the reason clear (device erratum) and to
> >    use the root port as the affected device.
> > 
> > Bjorn
> > 
> > 
> > commit 4cdcda8244d4c7b8a0e37fe2ce8d11d4633c6687
> > Author: Hariprasad Shenai <hariprasad@chelsio.com>
> > Date:   Sun Oct 18 19:55:04 2015 +0530
> > 
> >    PCI: Turn off Request Attributes to avoid Chelsio T5 Completion erratum
> > 
> >    The Chelsio T5 has a PCIe compliance erratum that causes Malformed TLP or
> >    Unexpected Completion errors in some systems, which may cause device access
> >    timeouts.
> > 
> >    Per PCIe r3.0, sec 2.2.9, "Completion headers must supply the same values
> >    for the Attribute as were supplied in the header of the corresponding
> >    Request, except as explicitly allowed when IDO is used."
> > 
> >    Instead of copying the Attributes from the Request to the Completion, the
> >    T5 always generates Completions with zero Attributes.  The receiver of a
> >    Completion whose Attributes don't match the Request may accept it (which
> >    itself seems non-compliant based on sec 2.3.2), or it may handle it as a
> >    Malformed TLP or an Unexpected Completion, which will probably lead to a
> >    device access timeout.
> > 
> >    Work around this by disabling "Relaxed Ordering" and "No Snoop" in the Root
> >    Port so it always generate Requests with zero Attributes.
> > 
> >    This does affect all other devices which are downstream of that Root Port,
> >    but these are performance optimizations that should not make a functional
> >    difference.
> > 
> >    Note that Configuration Space accesses are never supposed to have TLP
> >    Attributes, so we're safe waiting till after any Configuration Space
> >    accesses to do the Root Port "fixup".
> > 
> >    Based on original work by Casey Leedom <leedom@chelsio.com>
> > 
> >    [bhelgaas: changelog, comments, rename to pci_find_pcie_root_port(), rework
> >    to use pci_upstream_bridge() and check for Root Port device type, edit
> >    diagnostics to clarify intent and devices affected]
> >    Signed-off-by: Hariprasad Shenai <hariprasad@chelsio.com>
> >    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> > 
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index 6a9a111..3dacbeb 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -458,6 +458,30 @@ struct resource *pci_find_parent_resource(const struct pci_dev *dev,
> > EXPORT_SYMBOL(pci_find_parent_resource);
> > 
> > /**
> > + * pci_find_pcie_root_port - return PCIe Root Port
> > + * @dev: PCI device to query
> > + *
> > + * Traverse up the parent chain and return the PCIe Root Port PCI Device
> > + * for a given PCI Device.
> > + */
> > +struct pci_dev *pci_find_pcie_root_port(const struct pci_dev *dev)
> > +{
> > +	struct pci_dev *bridge, *highest_pcie_bridge = NULL;
> > +
> > +	bridge = pci_upstream_bridge(dev);
> > +	while (bridge && pci_is_pcie(bridge)) {
> > +		highest_pcie_bridge = bridge;
> > +		bridge = pci_upstream_bridge(bridge);
> > +	}
> > +
> > +	if (pci_pcie_type(highest_pcie_bridge) != PCI_EXP_TYPE_ROOT_PORT)
> > +		return NULL;
> > +
> > +	return highest_pcie_bridge;
> > +}
> > +EXPORT_SYMBOL(pci_find_pcie_root_port);
> > +
> > +/**
> >  * pci_wait_for_pending - wait for @mask bit(s) to clear in status word @pos
> >  * @dev: the PCI device to operate on
> >  * @pos: config space offset of status word
> > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> > index 6a30252..eb3c98e 100644
> > --- a/drivers/pci/quirks.c
> > +++ b/drivers/pci/quirks.c
> > @@ -3692,6 +3692,63 @@ DECLARE_PCI_FIXUP_CLASS_EARLY(0x1797, 0x6869, PCI_CLASS_NOT_DEFINED, 8,
> > 			      quirk_tw686x_class);
> > 
> > /*
> > + * Per PCIe r3.0, sec 2.2.9, "Completion headers must supply the same
> > + * values for the Attribute as were supplied in the header of the
> > + * corresponding Request, except as explicitly allowed when IDO is used."
> > + *
> > + * If a non-compliant device generates a completion with a different
> > + * attribute than the request, the receiver may accept it (which itself
> > + * seems non-compliant based on sec 2.3.2), or it may handle it as a
> > + * Malformed TLP or an Unexpected Completion, which will probably lead to a
> > + * device access timeout.
> > + *
> > + * If the non-compliant device generates completions with zero attributes
> > + * (instead of copying the attributes from the request), we can work around
> > + * this by disabling the "Relaxed Ordering" and "No Snoop" attributes in
> > + * upstream devices so they always generate requests with zero attributes.
> > + *
> > + * This affects other devices under the same Root Port, but since these
> > + * attributes are performance hints, there should be no functional problem.
> > + *
> > + * Note that Configuration Space accesses are never supposed to have TLP
> > + * Attributes, so we're safe waiting till after any Configuration Space
> > + * accesses to do the Root Port fixup.
> > + */
> > +static void quirk_disable_root_port_attributes(struct pci_dev *pdev)
> > +{
> > +	struct pci_dev *root_port = pci_find_pcie_root_port(pdev);
> > +
> > +	if (!root_port) {
> > +		dev_warn(&pdev->dev, "PCIe Completion erratum may cause device errors\n");
> > +		return;
> > +	}
> > +
> > +	dev_info(&root_port->dev, "Disabling No Snoop/Relaxed Ordering Attributes to avoid PCIe Completion erratum in %s\n",
> > +		 dev_name(&pdev->dev));
> > +	pcie_capability_clear_and_set_word(root_port, PCI_EXP_DEVCTL,
> > +					   PCI_EXP_DEVCTL_RELAX_EN |
> > +					   PCI_EXP_DEVCTL_NOSNOOP_EN, 0);
> > +}
> > +
> > +/*
> > + * The Chelsio T5 chip fails to copy TLP Attributes from a Request to the
> > + * Completion it generates.
> > + */
> > +static void quirk_chelsio_T5_disable_root_port_attributes(struct pci_dev *pdev)
> > +{
> > +	/*
> > +	 * This mask/compare operation selects for Physical Function 4 on a
> > +	 * T5.  We only need to fix up the Root Port once for any of the
> > +	 * PFs.  PF[0..3] have PCI Device IDs of 0x50xx, but PF4 is uniquely
> > +	 * 0x54xx so we use that one,
> > +	 */
> > +	if ((pdev->device & 0xff00) == 0x5400)
> > +		quirk_disable_root_port_attributes(pdev);
> > +}
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_CHELSIO, PCI_ANY_ID,
> > +			 quirk_chelsio_T5_disable_root_port_attributes);
> > +
> > +/*
> >  * AMD has indicated that the devices below do not support peer-to-peer
> >  * in any system where they are found in the southbridge with an AMD
> >  * IOMMU in the system.  Multifunction devices that do not support
> > diff --git a/include/linux/pci.h b/include/linux/pci.h
> > index b54fbf1..0f3696c 100644
> > --- a/include/linux/pci.h
> > +++ b/include/linux/pci.h
> > @@ -820,6 +820,7 @@ void pci_bus_add_device(struct pci_dev *dev);
> > void pci_read_bridge_bases(struct pci_bus *child);
> > struct resource *pci_find_parent_resource(const struct pci_dev *dev,
> > 					  struct resource *res);
> > +struct pci_dev *pci_find_root_pcie_port(const struct pci_dev *dev);
> > u8 pci_swizzle_interrupt_pin(const struct pci_dev *dev, u8 pin);
> > int pci_get_interrupt_pin(struct pci_dev *dev, struct pci_dev **bridge);
> > u8 pci_common_swizzle(struct pci_dev *dev, u8 *pinp);
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] pci: Add pci quirk to turnoff Nosnoop and Relaxed Ordering

[Bjorn Helgaas]

On Sun, Oct 18, 2015 at 07:55:04PM +0530, Hariprasad Shenai wrote:
> Some devices violate the PCI Specification regarding echoing the Root
> Port Transaction Layer Packet Request (TLP) No Snoop and Relaxed
> Ordering Attributes into the TLP Response. The PCI Specification
> "encourages" compliant Root Port implementation to drop such
> malformed TLP Responses leading to device access timeouts. Many Root Port
> implementations accept such malformed TLP Responses and a few more
> strict implementations do drop them.
> 
> For devices which fail this part of the PCI Specification, we need to
> traverse up the PCI Chain to the Root Port and turn off the Enable No
> Snoop and Enable Relaxed Ordering bits in the Root Port's PCI-Express
> Device Control register. This does affect all other devices which
> are downstream of that Root Port.

While researching another thread about RO [1], I got concerned about
setting RO for root ports.

Setting RO for *endpoints* makes sense: that allows (but does not
require) the endpoint to issue writes that don't require strong
ordering.

Setting RO for *root ports* seems more problematic.  It allows the
root port to issue PCIe writes that don't require strong ordering.
These would be CPU MMIO writes to devices.  But Linux currently does
not have a way for drivers to indicate that some MMIO writes need to
be ordered while others do not, and I think drivers assume that all
MMIO writes are performed in order.

I don't think Linux ever enables RO in Root Ports, but this patch
suggests that firmware might enable it.  I don't understand how that
could *ever* be safe, unless we had some mechanism like a separate
MMIO window that generated writes with relaxed ordering.

Did you trip over firmware that enables RO, or is this a preventive
thing in case firmware or Linux ever *did* enable RO in the Root Port?

[1] https://lore.kernel.org/r/20210830123704.221494-2-verdre@v0yd.nl

> Note that Configuration Space accesses are never supposed to have TLP
> Attributes, so we're safe waiting till after any Configuration Space
> accesses to do the Root Port "fixup".
> 
> Based on original work by Casey Leedom <leedom@chelsio.com>
> 
> Signed-off-by: Hariprasad Shenai <hariprasad@chelsio.com>
> ---
>  drivers/pci/pci.c    | 28 ++++++++++++++++++++++++++
>  drivers/pci/quirks.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/pci.h  |  1 +
>  3 files changed, 85 insertions(+)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 6a9a111..3ce202b 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -458,6 +458,34 @@ struct resource *pci_find_parent_resource(const struct pci_dev *dev,
>  EXPORT_SYMBOL(pci_find_parent_resource);
>  
>  /**
> + * pci_find_root_pcie_port - return PCI-E Root Port
> + * @dev: PCI device to query
> + *
> + * Traverses up the parent chain and return the PCI-E Root Port PCI Device
> + * for a given PCI Device.
> + */
> +struct pci_dev *pci_find_root_pcie_port(const struct pci_dev *dev)
> +{
> +	struct pci_bus *bus = dev->bus;
> +	struct pci_dev *highest_pcie_bridge = NULL;
> +
> +	while (bus) {
> +		struct pci_dev *bridge = bus->self;
> +
> +		if (!bridge || !bridge->pcie_cap)
> +			break;
> +		highest_pcie_bridge = bridge;
> +		bus = bus->parent;
> +	}
> +
> +	if (!highest_pcie_bridge)
> +		dev_warn(&dev->dev, "Can't find Root Port\n");
> +
> +	return highest_pcie_bridge;
> +}
> +EXPORT_SYMBOL(pci_find_root_pcie_port);
> +
> +/**
>   * pci_wait_for_pending - wait for @mask bit(s) to clear in status word @pos
>   * @dev: the PCI device to operate on
>   * @pos: config space offset of status word
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 6a30252..f860956 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -3692,6 +3692,62 @@ DECLARE_PCI_FIXUP_CLASS_EARLY(0x1797, 0x6869, PCI_CLASS_NOT_DEFINED, 8,
>  			      quirk_tw686x_class);
>  
>  /*
> + * Some devices violate the PCI Specification regarding echoing the Root
> + * Port Transaction Layer Packet Request (TLP) No Snoop and Relaxed
> + * Ordering Attributes into the TLP Response.  The PCI Specification
> + * "encourages" compliant Root Port implementation to drop such malformed
> + * TLP Responses leading to device access timeouts.  Many Root Port
> + * implementations accept such malformed TLP Responses and a few more strict
> + * implementations do drop them.
> + *
> + * For devices which fail this part of the PCI Specification, we need to
> + * traverse up the PCI Chain to the Root Port and turn off the Enable No
> + * Snoop and Enable Relaxed Ordering bits in the Root Port's PCI-Express
> + * Device Control register.  This does affect all other devices which are
> + * downstream of that Root Port but since No Snoop and Relaxed ordering are
> + * "Performance Hints," we're okay with that ...
> + *
> + * Note that Configuration Space accesses are never supposed to have TLP
> + * Attributes, so we're safe waiting till after any Configuration Space
> + * accesses to do the Root Port "fixup" ...
> + */
> +static void quirk_disable_root_port_attributes(struct pci_dev *pdev)
> +{
> +	struct pci_dev *root_port = pci_find_root_pcie_port(pdev);
> +
> +	if (!root_port) {
> +		dev_warn(&pdev->dev, "Can't find Root Port to disable No Snoop/Relaxed Ordering\n");
> +		return;
> +	}
> +
> +	dev_info(&pdev->dev, "Disabling No Snoop/Relaxed Ordering on Root Port %s\n",
> +		 dev_name(&root_port->dev));
> +	pcie_capability_clear_and_set_word(root_port,
> +					   PCI_EXP_DEVCTL,
> +					   PCI_EXP_DEVCTL_RELAX_EN |
> +					   PCI_EXP_DEVCTL_NOSNOOP_EN,
> +					   0);
> +}
> +
> +/*
> + * The Chelsio T5 chip fails to return the Root Port's TLP Attributes in
> + * its TLP responses to the Root Port.
> + */
> +static void quirk_chelsio_T5_disable_root_port_attributes(struct pci_dev *pdev)
> +{
> +	/*
> +	 * This mask/compare operation selects for Physical Function 4 on a
> +	 * T5.  We only need to fix up the Root Port once for any of the
> +	 * PFs.  PF[0..3] have PCI Device IDs of 0x50xx, but PF4 is uniquely
> +	 * 0x54xx so we use that one,
> +	 */
> +	if ((pdev->device & 0xff00) == 0x5400)
> +		quirk_disable_root_port_attributes(pdev);
> +}
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_CHELSIO, PCI_ANY_ID,
> +			 quirk_chelsio_T5_disable_root_port_attributes);
> +
> +/*
>   * AMD has indicated that the devices below do not support peer-to-peer
>   * in any system where they are found in the southbridge with an AMD
>   * IOMMU in the system.  Multifunction devices that do not support
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index e90eb22..5b4d7cc 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -820,6 +820,7 @@ void pci_bus_add_device(struct pci_dev *dev);
>  void pci_read_bridge_bases(struct pci_bus *child);
>  struct resource *pci_find_parent_resource(const struct pci_dev *dev,
>  					  struct resource *res);
> +struct pci_dev *pci_find_root_pcie_port(const struct pci_dev *dev);
>  u8 pci_swizzle_interrupt_pin(const struct pci_dev *dev, u8 pin);
>  int pci_get_interrupt_pin(struct pci_dev *dev, struct pci_dev **bridge);
>  u8 pci_common_swizzle(struct pci_dev *dev, u8 *pinp);
> -- 
> 2.3.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] pci: Add pci quirk to turnoff Nosnoop and Relaxed Ordering

[Keith Busch]

On Wed, Sep 01, 2021 at 05:23:53PM -0500, Bjorn Helgaas wrote:
> On Sun, Oct 18, 2015 at 07:55:04PM +0530, Hariprasad Shenai wrote:
> > Some devices violate the PCI Specification regarding echoing the Root
> > Port Transaction Layer Packet Request (TLP) No Snoop and Relaxed
> > Ordering Attributes into the TLP Response. The PCI Specification
> > "encourages" compliant Root Port implementation to drop such
> > malformed TLP Responses leading to device access timeouts. Many Root Port
> > implementations accept such malformed TLP Responses and a few more
> > strict implementations do drop them.
> > 
> > For devices which fail this part of the PCI Specification, we need to
> > traverse up the PCI Chain to the Root Port and turn off the Enable No
> > Snoop and Enable Relaxed Ordering bits in the Root Port's PCI-Express
> > Device Control register. This does affect all other devices which
> > are downstream of that Root Port.
> 
> While researching another thread about RO [1], I got concerned about
> setting RO for root ports.
> 
> Setting RO for *endpoints* makes sense: that allows (but does not
> require) the endpoint to issue writes that don't require strong
> ordering.
> 
> Setting RO for *root ports* seems more problematic.  It allows the
> root port to issue PCIe writes that don't require strong ordering.
> These would be CPU MMIO writes to devices.  But Linux currently does
> not have a way for drivers to indicate that some MMIO writes need to
> be ordered while others do not, and I think drivers assume that all
> MMIO writes are performed in order.

Is that not what writel_relaxed() is for? While it appears that most
archs just have that call the generic writel(), it does let drivers
indicate which writes are not strongly ordered.

Re: [PATCH] pci: Add pci quirk to turnoff Nosnoop and Relaxed Ordering

[Bjorn Helgaas]

On Wed, Sep 01, 2021 at 04:22:12PM -0700, Keith Busch wrote:
> On Wed, Sep 01, 2021 at 05:23:53PM -0500, Bjorn Helgaas wrote:
> > On Sun, Oct 18, 2015 at 07:55:04PM +0530, Hariprasad Shenai wrote:
> > > Some devices violate the PCI Specification regarding echoing the Root
> > > Port Transaction Layer Packet Request (TLP) No Snoop and Relaxed
> > > Ordering Attributes into the TLP Response. The PCI Specification
> > > "encourages" compliant Root Port implementation to drop such
> > > malformed TLP Responses leading to device access timeouts. Many Root Port
> > > implementations accept such malformed TLP Responses and a few more
> > > strict implementations do drop them.
> > > 
> > > For devices which fail this part of the PCI Specification, we need to
> > > traverse up the PCI Chain to the Root Port and turn off the Enable No
> > > Snoop and Enable Relaxed Ordering bits in the Root Port's PCI-Express
> > > Device Control register. This does affect all other devices which
> > > are downstream of that Root Port.
> > 
> > While researching another thread about RO [1], I got concerned about
> > setting RO for root ports.
> > 
> > Setting RO for *endpoints* makes sense: that allows (but does not
> > require) the endpoint to issue writes that don't require strong
> > ordering.
> > 
> > Setting RO for *root ports* seems more problematic.  It allows the
> > root port to issue PCIe writes that don't require strong ordering.
> > These would be CPU MMIO writes to devices.  But Linux currently does
> > not have a way for drivers to indicate that some MMIO writes need to
> > be ordered while others do not, and I think drivers assume that all
> > MMIO writes are performed in order.
> 
> Is that not what writel_relaxed() is for? While it appears that most
> archs just have that call the generic writel(), it does let drivers
> indicate which writes are not strongly ordered.

Sheesh, I hate when I spend a whole afternoon making a fool of myself.

I was thinking just from the PCI core perspective, expecting a
separate window or something for relaxed ordering, but I'm sure
writel_relaxed() could be implemented using different CPU bus
transactions that the Root Complex knows how to interpret, and that
would be essentially invisible to the PCI core.

Sorry for the noise and thanks for the hint :)