Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1711062 > unrolled thread

[PATCH v10 0/5] Add new PCI_DEV_FLAGS_NO_RELAXED_ORDERING flag

Started byDing Tianhong <dingtianhong@huawei.com>
First post2017-08-14 17:50 +0200
Last post2017-08-14 20:20 +0200
Articles 6 — 4 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH v10 0/5] Add new PCI_DEV_FLAGS_NO_RELAXED_ORDERING flag Ding Tianhong <dingtianhong@huawei.com> - 2017-08-14 17:50 +0200
    [PATCH v10 3/5] PCI: Disable Relaxed Ordering Attributes for AMD A1100 Ding Tianhong <dingtianhong@huawei.com> - 2017-08-14 18:10 +0200
      Re: [PATCH v10 3/5] PCI: Disable Relaxed Ordering Attributes for AMD  A1100 "Raj, Ashok" <ashok.raj@intel.com> - 2017-08-14 19:50 +0200
        Re: [PATCH v10 3/5] PCI: Disable Relaxed Ordering Attributes for AMD  A1100 Casey Leedom <leedom@chelsio.com> - 2017-08-14 20:10 +0200
        Re: [PATCH v10 3/5] PCI: Disable Relaxed Ordering Attributes for AMD  A1100 Ding Tianhong <dingtianhong@huawei.com> - 2017-08-15 03:50 +0200
    Re: [PATCH v10 0/5] Add new PCI_DEV_FLAGS_NO_RELAXED_ORDERING flag David Miller <davem@davemloft.net> - 2017-08-14 20:20 +0200

#1711062 — [PATCH v10 0/5] Add new PCI_DEV_FLAGS_NO_RELAXED_ORDERING flag

FromDing Tianhong <dingtianhong@huawei.com>
Date2017-08-14 17:50 +0200
Subject[PATCH v10 0/5] Add new PCI_DEV_FLAGS_NO_RELAXED_ORDERING flag
Message-ID<uepMR-5WA-9@gated-at.bofh.it>
Some devices have problems with Transaction Layer Packets with the Relaxed
Ordering Attribute set.  This patch set adds a new PCIe Device Flag,
PCI_DEV_FLAGS_NO_RELAXED_ORDERING, a set of PCI Quirks to catch some known
devices with Relaxed Ordering issues, and a use of this new flag by the
cxgb4 driver to avoid using Relaxed Ordering with problematic Root Complex
Ports.

It's been years since I've submitted kernel.org patches, I appolgise for the
almost certain submission errors.

v2: Alexander point out that the v1 was only a part of the whole solution,
    some platform which has some issues could use the new flag to indicate
    that it is not safe to enable relaxed ordering attribute, then we need
    to clear the relaxed ordering enable bits in the PCI configuration when
    initializing the device. So add a new second patch to modify the PCI
    initialization code to clear the relaxed ordering enable bit in the
    event that the root complex doesn't want relaxed ordering enabled.

    The third patch was base on the v1's second patch and only be changed
    to query the relaxed ordering enable bit in the PCI configuration space
    to allow the Chelsio NIC to send TLPs with the relaxed ordering attributes
    set.

    This version didn't plan to drop the defines for Intel Drivers to use the
    new checking way to enable relaxed ordering because it is not the hardest
    part of the moment, we could fix it in next patchset when this patches
    reach the goal.

v3: Redesigned the logic for pci_configure_relaxed_ordering when configuration,
    If a PCIe device didn't enable the relaxed ordering attribute default,
    we should not do anything in the PCIe configuration, otherwise we
    should check if any of the devices above us do not support relaxed
    ordering by the PCI_DEV_FLAGS_NO_RELAXED_ORDERING flag, then base on
    the result if we get a return that indicate that the relaxed ordering
    is not supported we should update our device to disable relaxed ordering
    in configuration space. If the device above us doesn't exist or isn't
    the PCIe device, we shouldn't do anything and skip updating relaxed ordering
    because we are probably running in a guest.

v4: Rename the functions pcie_get_relaxed_ordering and pcie_disable_relaxed_ordering
    according John's suggestion, and modify the description, use the true/false
    as the return value.

    We shouldn't enable relaxed ordering attribute by the setting in the root
    complex configuration space for PCIe device, so fix it for cxgb4.

    Fix some format issues.

v5: Removed the unnecessary code for some function which only return the bool
    value, and add the check for VF device.

    Make this patch set base on 4.12-rc5.

v6: Fix the logic error in the need to enable the relaxed ordering attribute for cxgb4.

v7: The cxgb4 drivers will enable the PCIe Capability Device Control[Relaxed
    Ordering Enable] in PCI Probe() routine, this will break our current
    solution for some platform which has problematic when enable the relaxed
    ordering attribute. According to the latest recommendations, remove the
    enable_pcie_relaxed_ordering(), although it could not cover the Peer-to-Peer
    scene, but we agree to leave this problem until we really trigger it.

    Make this patch set base on 4.12 release version.

v8: Change the second patch title and description to make it more reasonable,
    add the acked-by from Alex and Ashok.

    Add a new patch to enable the Relaxed Ordering Attribute for cxgb4vf driver.

    Make this patch set base on 4.13-rc2.

v9: The document (https://software.intel.com/sites/default/files/managed/9e/
    bc/64-ia-32-architectures-optimization-manual.pdf) indicate that the Xeon
    processors based on Broadwell/Haswell microarchitecture has the problem
    with Relaxed Ordering Attribute enabled, so add the whole list Device ID
    from Intel to the patch.

v10: Significant rework based on Bjorn's feedback, reorganize the first 2 patches,
     now the Intel and AMD erratum soc has been divided to the different patches,
     rename the pcie_relaxed_ordering_supported() to pcie_relaxed_ordering_enabled(),
     and no need to check every intervening switch except the root ports, update
     some commits.

Casey Leedom (2):
  net/cxgb4: Use new PCI_DEV_FLAGS_NO_RELAXED_ORDERING flag
  net/cxgb4vf: Use new PCI_DEV_FLAGS_NO_RELAXED_ORDERING flag

Ding Tianhong (3):
  PCI: Disable PCIe Relaxed Ordering if unsupported
  PCI: Disable Relaxed Ordering for some Intel processors
  PCI: Disable Relaxed Ordering Attributes for AMD A1100

 drivers/net/ethernet/chelsio/cxgb4/cxgb4.h         |  1 +
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c    | 23 ++++--
 drivers/net/ethernet/chelsio/cxgb4/sge.c           |  5 +-
 drivers/net/ethernet/chelsio/cxgb4vf/adapter.h     |  1 +
 .../net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c    | 18 +++++
 drivers/net/ethernet/chelsio/cxgb4vf/sge.c         |  3 +
 drivers/pci/probe.c                                | 43 +++++++++++
 drivers/pci/quirks.c                               | 89 ++++++++++++++++++++++
 include/linux/pci.h                                |  3 +
 9 files changed, 178 insertions(+), 8 deletions(-)

-- 
1.8.3.1

[toc] | [next] | [standalone]

#1711111 — [PATCH v10 3/5] PCI: Disable Relaxed Ordering Attributes for AMD A1100

FromDing Tianhong <dingtianhong@huawei.com>
Date2017-08-14 18:10 +0200
Subject[PATCH v10 3/5] PCI: Disable Relaxed Ordering Attributes for AMD A1100
Message-ID<ueq6e-6jm-13@gated-at.bofh.it>
In reply to#1711062
Casey reported that the AMD ARM A1100 SoC has a bug in its PCIe
Root Port where Upstream Transaction Layer Packets with the Relaxed
Ordering Attribute clear are allowed to bypass earlier TLPs with
Relaxed Ordering set, it would cause Data Corruption, so we need
to disable Relaxed Ordering Attribute when Upstream TLPs to the
Root Port.

Signed-off-by: Casey Leedom <leedom@chelsio.com>
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
Acked-by: Alexander Duyck <alexander.h.duyck@intel.com>
Acked-by: Ashok Raj <ashok.raj@intel.com>
---
 drivers/pci/quirks.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 1272f7e..1407604 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -4089,6 +4089,22 @@ static void quirk_relaxedordering_disable(struct pci_dev *dev)
 			      quirk_relaxedordering_disable);
 
 /*
+ * The AMD ARM A1100 (AKA "SEATTLE") SoC has a bug in its PCIe Root Complex
+ * where Upstream Transaction Layer Packets with the Relaxed Ordering
+ * Attribute clear are allowed to bypass earlier TLPs with Relaxed Ordering
+ * set.  This is a violation of the PCIe 3.0 Transaction Ordering Rules
+ * outlined in Section 2.4.1 (PCI Express(r) Base Specification Revision 3.0
+ * November 10, 2010).  As a result, on this platform we can't use Relaxed
+ * Ordering for Upstream TLPs.
+ */
+DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_VENDOR_ID_AMD, 0x1a00, PCI_CLASS_NOT_DEFINED, 8,
+			      quirk_relaxedordering_disable);
+DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_VENDOR_ID_AMD, 0x1a01, PCI_CLASS_NOT_DEFINED, 8,
+			      quirk_relaxedordering_disable);
+DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_VENDOR_ID_AMD, 0x1a02, PCI_CLASS_NOT_DEFINED, 8,
+			      quirk_relaxedordering_disable);
+
+/*
  * 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."
-- 
1.8.3.1

[toc] | [prev] | [next] | [standalone]

#1711280 — Re: [PATCH v10 3/5] PCI: Disable Relaxed Ordering Attributes for AMD A1100

From"Raj, Ashok" <ashok.raj@intel.com>
Date2017-08-14 19:50 +0200
SubjectRe: [PATCH v10 3/5] PCI: Disable Relaxed Ordering Attributes for AMD A1100
Message-ID<uerF0-76t-3@gated-at.bofh.it>
In reply to#1711111
On Mon, Aug 14, 2017 at 11:44:57PM +0800, Ding Tianhong wrote:
> Casey reported that the AMD ARM A1100 SoC has a bug in its PCIe
> Root Port where Upstream Transaction Layer Packets with the Relaxed
> Ordering Attribute clear are allowed to bypass earlier TLPs with
> Relaxed Ordering set, it would cause Data Corruption, so we need
> to disable Relaxed Ordering Attribute when Upstream TLPs to the
> Root Port.
> 
> Signed-off-by: Casey Leedom <leedom@chelsio.com>
> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
> Acked-by: Alexander Duyck <alexander.h.duyck@intel.com>
> Acked-by: Ashok Raj <ashok.raj@intel.com>

I can't ack this patch :-).. must be someone from AMD. Please remove my
signature from this.

> ---
>  drivers/pci/quirks.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 1272f7e..1407604 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -4089,6 +4089,22 @@ static void quirk_relaxedordering_disable(struct pci_dev *dev)
>  			      quirk_relaxedordering_disable);
>  
>  /*
> + * The AMD ARM A1100 (AKA "SEATTLE") SoC has a bug in its PCIe Root Complex
> + * where Upstream Transaction Layer Packets with the Relaxed Ordering
> + * Attribute clear are allowed to bypass earlier TLPs with Relaxed Ordering
> + * set.  This is a violation of the PCIe 3.0 Transaction Ordering Rules
> + * outlined in Section 2.4.1 (PCI Express(r) Base Specification Revision 3.0
> + * November 10, 2010).  As a result, on this platform we can't use Relaxed
> + * Ordering for Upstream TLPs.
> + */
> +DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_VENDOR_ID_AMD, 0x1a00, PCI_CLASS_NOT_DEFINED, 8,
> +			      quirk_relaxedordering_disable);
> +DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_VENDOR_ID_AMD, 0x1a01, PCI_CLASS_NOT_DEFINED, 8,
> +			      quirk_relaxedordering_disable);
> +DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_VENDOR_ID_AMD, 0x1a02, PCI_CLASS_NOT_DEFINED, 8,
> +			      quirk_relaxedordering_disable);
> +
> +/*
>   * 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."
> -- 
> 1.8.3.1
> 
>

[toc] | [prev] | [next] | [standalone]

#1711318 — Re: [PATCH v10 3/5] PCI: Disable Relaxed Ordering Attributes for AMD A1100

FromCasey Leedom <leedom@chelsio.com>
Date2017-08-14 20:10 +0200
SubjectRe: [PATCH v10 3/5] PCI: Disable Relaxed Ordering Attributes for AMD A1100
Message-ID<uerYm-7s4-15@gated-at.bofh.it>
In reply to#1711280
| From: Raj, Ashok <ashok.raj@intel.com>
| Sent: Monday, August 14, 2017 10:19 AM
|     
| On Mon, Aug 14, 2017 at 11:44:57PM +0800, Ding Tianhong wrote:
| > Casey reported that the AMD ARM A1100 SoC has a bug in its PCIe
| > Root Port where Upstream Transaction Layer Packets with the Relaxed
| > Ordering Attribute clear are allowed to bypass earlier TLPs with
| > Relaxed Ordering set, it would cause Data Corruption, so we need
| > to disable Relaxed Ordering Attribute when Upstream TLPs to the
| > Root Port.
| > 
| > Signed-off-by: Casey Leedom <leedom@chelsio.com>
| > Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
| > Acked-by: Alexander Duyck <alexander.h.duyck@intel.com>
| > Acked-by: Ashok Raj <ashok.raj@intel.com>
| 
| I can't ack this patch :-).. must be someone from AMD. Please remove my
| signature from this.

  You can go ahead and leave my name on since I'm the person who found and diagnosed the problem (with help from others inside Chelsio).

  If anyone on the Linux PCI List knows anyone at AMD who'd be willing to respond it would be great, but as I noted earlier, I think that AMD has effectively abandoned the A1100 ("Seattle") ARM SoC, so I doubt if we'll get anyone from AMD to even comment now.

Casey

[toc] | [prev] | [next] | [standalone]

#1711692 — Re: [PATCH v10 3/5] PCI: Disable Relaxed Ordering Attributes for AMD A1100

FromDing Tianhong <dingtianhong@huawei.com>
Date2017-08-15 03:50 +0200
SubjectRe: [PATCH v10 3/5] PCI: Disable Relaxed Ordering Attributes for AMD A1100
Message-ID<uez9w-3gj-15@gated-at.bofh.it>
In reply to#1711280

On 2017/8/15 1:19, Raj, Ashok wrote:
> On Mon, Aug 14, 2017 at 11:44:57PM +0800, Ding Tianhong wrote:
>> Casey reported that the AMD ARM A1100 SoC has a bug in its PCIe
>> Root Port where Upstream Transaction Layer Packets with the Relaxed
>> Ordering Attribute clear are allowed to bypass earlier TLPs with
>> Relaxed Ordering set, it would cause Data Corruption, so we need
>> to disable Relaxed Ordering Attribute when Upstream TLPs to the
>> Root Port.
>>
>> Signed-off-by: Casey Leedom <leedom@chelsio.com>
>> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>> Acked-by: Alexander Duyck <alexander.h.duyck@intel.com>
>> Acked-by: Ashok Raj <ashok.raj@intel.com>
> 
> I can't ack this patch :-).. must be someone from AMD. Please remove my
> signature from this.
> 

Sorry for funny mistake :)  I will fix it.

Ding

>> ---
>>  drivers/pci/quirks.c | 16 ++++++++++++++++
>>  1 file changed, 16 insertions(+)
>>
>> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
>> index 1272f7e..1407604 100644
>> --- a/drivers/pci/quirks.c
>> +++ b/drivers/pci/quirks.c
>> @@ -4089,6 +4089,22 @@ static void quirk_relaxedordering_disable(struct pci_dev *dev)
>>  			      quirk_relaxedordering_disable);
>>  
>>  /*
>> + * The AMD ARM A1100 (AKA "SEATTLE") SoC has a bug in its PCIe Root Complex
>> + * where Upstream Transaction Layer Packets with the Relaxed Ordering
>> + * Attribute clear are allowed to bypass earlier TLPs with Relaxed Ordering
>> + * set.  This is a violation of the PCIe 3.0 Transaction Ordering Rules
>> + * outlined in Section 2.4.1 (PCI Express(r) Base Specification Revision 3.0
>> + * November 10, 2010).  As a result, on this platform we can't use Relaxed
>> + * Ordering for Upstream TLPs.
>> + */
>> +DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_VENDOR_ID_AMD, 0x1a00, PCI_CLASS_NOT_DEFINED, 8,
>> +			      quirk_relaxedordering_disable);
>> +DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_VENDOR_ID_AMD, 0x1a01, PCI_CLASS_NOT_DEFINED, 8,
>> +			      quirk_relaxedordering_disable);
>> +DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_VENDOR_ID_AMD, 0x1a02, PCI_CLASS_NOT_DEFINED, 8,
>> +			      quirk_relaxedordering_disable);
>> +
>> +/*
>>   * 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."
>> -- 
>> 1.8.3.1
>>
>>
> 
> .
>

[toc] | [prev] | [next] | [standalone]

#1711325

FromDavid Miller <davem@davemloft.net>
Date2017-08-14 20:20 +0200
Message-ID<ues81-7vo-11@gated-at.bofh.it>
In reply to#1711062
It looks like we are really close on this.

Please just remove the Intel signoff from the AMD patch, and I don't
think it's wise to wait for an AMD person to signoff on it given
the state of affairs for that chipset.

Thanks.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web