Re: [PATCH net-next 1/4] ixgbe: sparc: rename the ARCH_WANT_RELAX_ORDER to IXGBE_ALLOW_RELAXED_ORDER

Re: [PATCH net-next 1/4] ixgbe: sparc: rename the ARCH_WANT_RELAX_ORDER to IXGBE_ALLOW_RELAXED_ORDER

[Alexander Duyck]

On Wed, Apr 26, 2017 at 2:26 AM, Ding Tianhong <dingtianhong@huawei.com> wrote:
> Hi Amir:
>
> It is really glad to hear that the mlx5 will support RO mode this year, if so, do you agree that enable it dynamic by ethtool -s xxx,
> we have try it several month ago but there was only one drivers would use it at that time so the maintainer against it, it mlx5 would support RO,
> we could try to restart this solution, what do you think about it. :)
>
> Thanks
> Ding

Hi Ding,

Enabing relaxed ordering really doesn't have any place in ethtool. It
is a PCIe attribute that you are essentially wanting to enable.

It might be worth while to take a look at updating the PCIe code path
to handle this. Really what we should probably do is guarantee that
the architectures that need relaxed ordering are setting it in the
PCIe Device Control register and that the ones that don't are clearing
the bit. It's possible that this is already occurring, but I don't
know the state of handling those bits is in the kernel. Once we can
guarantee that we could use that to have the drivers determine their
behavior in regards to relaxed ordering. For example in the case of
igb/ixgbe we could probably change the behavior so that it will bey
default try to use relaxed ordering but if it is not enabled in PCIe
Device Control register the hardware should not request to use it. It
would simplify things in the drivers and allow for each architecture
to control things as needed in their PCIe code.

- Alex

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next 1/4] ixgbe: sparc: rename the ARCH_WANT_RELAX_ORDER to IXGBE_ALLOW_RELAXED_ORDER

[Bjorn Helgaas]

[+cc Casey]

On Wed, Apr 26, 2017 at 09:18:33AM -0700, Alexander Duyck wrote:
> On Wed, Apr 26, 2017 at 2:26 AM, Ding Tianhong <dingtianhong@huawei.com> wrote:
> > Hi Amir:
> >
> > It is really glad to hear that the mlx5 will support RO mode this year, if so, do you agree that enable it dynamic by ethtool -s xxx,
> > we have try it several month ago but there was only one drivers would use it at that time so the maintainer against it, it mlx5 would support RO,
> > we could try to restart this solution, what do you think about it. :)
> >
> > Thanks
> > Ding
> 
> Hi Ding,
> 
> Enabing relaxed ordering really doesn't have any place in ethtool. It
> is a PCIe attribute that you are essentially wanting to enable.
> 
> It might be worth while to take a look at updating the PCIe code path
> to handle this. Really what we should probably do is guarantee that
> the architectures that need relaxed ordering are setting it in the
> PCIe Device Control register and that the ones that don't are clearing
> the bit. It's possible that this is already occurring, but I don't
> know the state of handling those bits is in the kernel. Once we can
> guarantee that we could use that to have the drivers determine their
> behavior in regards to relaxed ordering. For example in the case of
> igb/ixgbe we could probably change the behavior so that it will bey
> default try to use relaxed ordering but if it is not enabled in PCIe
> Device Control register the hardware should not request to use it. It
> would simplify things in the drivers and allow for each architecture
> to control things as needed in their PCIe code.

I thought Relaxed Ordering was an optimization.  Are there cases where
it is actually required for correct behavior?

The PCI core doesn't currently do anything with Relaxed Ordering.
Some drivers enable/disable it directly.  I think it would probably be
better if the core provided an interface for this.  One reason is
because I think Casey has identified some systems where Relaxed
Ordering doesn't work correctly, and I'd rather deal with them once in
the core than in every driver.

Are you hinting that the PCI core or arch code could actually *enable*
Relaxed Ordering without the driver doing anything?  Is it safe to do
that?  Is there such a thing as a device that is capable of using RO,
but where the driver must be aware of it being enabled, so it programs
the device appropriately?

Bjorn

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next 1/4] ixgbe: sparc: rename the ARCH_WANT_RELAX_ORDER to IXGBE_ALLOW_RELAXED_ORDER

[Casey Leedom]

  Thanks for adding me to the Cc list Bjorn.  Hopefully my message will make
it out to the netdev and linux-pci lists -- I'm not currently subscribed to
them.  I've explicitly marked this message to be sent in plain text but
modern email agents suck with respect to this. (sigh) I have officially
become a curmudgeon. 

  So, officially, Relaxed Ordering should be a Semantic Noop as far as PCIe
transfers are concerned, as long as you don't care what order the PCIe
Transaction Layer Packets are processed in by the target PCIe Fabric End
Point.

  Basically, if you have some number of back-to-back PCIe TLPs between two
Fabric End Points {A} -> {B} which have the Relaxed Ordering Attribute set,
the End Point {B} receiving these RO TLPs may process them in any order it
likes.  When a TLP without Relaxed Ordering is sent {A} -> {B}, all
preceding TLPs with Relaxed Ordering set must be processed by {B} prior to
processing the TLP without Relaxed Ordering set.  In this sense, a TLP
without Relaxed Ordering set is something akin to a "memory barrier".

  All of this is covered in Section 2.4.1 of the PCIe 3.0 Specification (PCI
Express(r) Base Specification Revision 3.0 November 10, 2010).

  The advantage of using Relaxed Ordering (which is heavily used when
sending data to Graphics Cards as I understand it), is that the PCIe
Endpoint can potentially optimize the processing order of RO TLPs with
things like a local multi-channel Memory Controller in order to achieve the
highest transfer bandwidth possible.

  However, we have discovered at least two PCIe 3.0 Root Complex
implementations which have problems with TLPs directed at them with the
Relaxed Ordering Attribute set and I'm in the process of working up a Linux
Kernel PCI "Quirk" to allow those PCIe End Points to be marked as "not being
advisable to send RO TLPs to".  These problems range from "mere" Performance
Problems to outright Data Corruption.  I'm working with the vendors of these
...  "problematic" Root Complex implementations and hope to have this patch
submitted to the linux-pci list by tomorrow.

  By the way, it's important to note that just because, say, a Root Complex
has problems with RO TLPs directed at it, that doesn't mean that you want to
avoid all use of Relaxed Ordering within the PCIe Fabric.  For instance,
with the vendor whose Root Complex has a Performance Problem with RO TLPs
directed at it, it's perfectly reasonable -- and desired -- to use Relaxed
Ordering in Peer-to-Peer traffic.  Say for instance, with an NVMe <->
Ethernet application.

Casey

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next 1/4] ixgbe: sparc: rename the ARCH_WANT_RELAX_ORDER to IXGBE_ALLOW_RELAXED_ORDER

[Casey Leedom]

| From: Bjorn Helgaas <helgaas@kernel.org>
| Sent: Thursday, April 27, 2017 10:19 AM
|
| Are you hinting that the PCI core or arch code could actually *enable*
| Relaxed Ordering without the driver doing anything?  Is it safe to do that?
| Is there such a thing as a device that is capable of using RO, but where the
| driver must be aware of it being enabled, so it programs the device
| appropriately?

  I forgot to reply to this portion of Bjorn's email.

  The PCI Configuration Space PCI Capability Device Control[Enable Relaxed
Ordering] bit governs enabling the _ability_ for the PCIe Device to send
TLPs with the Relaxed Ordering Attribute set.  It does not _cause_ RO to be
set on TLPs.  Doing that would almost certainly cause Data Corruption Bugs
since you only want a subset of TLPs to have RO set.

  For instance, we typically use RO for Ingress Packet Data delivery but
non-RO for messages notifying the Host that an Ingress Packet has been
delivered.  This ensures that the "Ingress Packet Delivered" non-RO TLP is
processed _after_ any preceding RO TLPs delivering the actual Ingress Packet
Data.

  In the above scenario, if one were to turn off Enable Relaxed Ordering via
the PCIe Capability, then the on-chip PCIe engine would simply never send a
TLP with the Relaxed Ordering Attribute set, regardless of any other chip
programming.

  And finally, just to be absolutely clear, using Relaxed Ordering isn't and
"Architecture Thing".  It's a PCIe Fabric End Point Thing.  Many End Points
simply ignore the Relaxed Ordering Attribute (except to reflect it back in
Response TLPs).  In this sense, Relaxed Ordering simply provides
potentially useful optimization information to the PCIe End Point.

Casey

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH net-next 1/4] ixgbe: sparc: rename the ARCH_WANT_RELAX_ORDER to IXGBE_ALLOW_RELAXED_ORDER

[Gabriele Paoloni]

Hi Casey

Many thanks for the detailed explanation

> -----Original Message-----
> From: Casey Leedom [mailto:leedom@chelsio.com]
> Sent: 27 April 2017 21:35
> To: Bjorn Helgaas; Alexander Duyck
> Cc: Dingtianhong; Mark Rutland; Amir Ancel; Gabriele Paoloni; linux-
> pci@vger.kernel.org; Catalin Marinas; Will Deacon; Linuxarm; David
> Laight; jeffrey.t.kirsher@intel.com; netdev@vger.kernel.org; Robin
> Murphy; davem@davemloft.net; linux-arm-kernel@lists.infradead.org
> Subject: Re: [PATCH net-next 1/4] ixgbe: sparc: rename the
> ARCH_WANT_RELAX_ORDER to IXGBE_ALLOW_RELAXED_ORDER
> 
> | From: Bjorn Helgaas <helgaas@kernel.org>
> | Sent: Thursday, April 27, 2017 10:19 AM
> |
> | Are you hinting that the PCI core or arch code could actually
> *enable*
> | Relaxed Ordering without the driver doing anything?  Is it safe to do
> that?
> | Is there such a thing as a device that is capable of using RO, but
> where the
> | driver must be aware of it being enabled, so it programs the device
> | appropriately?
> 
>   I forgot to reply to this portion of Bjorn's email.
> 
>   The PCI Configuration Space PCI Capability Device Control[Enable
> Relaxed
> Ordering] bit governs enabling the _ability_ for the PCIe Device to
> send
> TLPs with the Relaxed Ordering Attribute set.  It does not _cause_ RO
> to be
> set on TLPs.  Doing that would almost certainly cause Data Corruption
> Bugs
> since you only want a subset of TLPs to have RO set.
> 
>   For instance, we typically use RO for Ingress Packet Data delivery
> but
> non-RO for messages notifying the Host that an Ingress Packet has been
> delivered.  This ensures that the "Ingress Packet Delivered" non-RO TLP
> is
> processed _after_ any preceding RO TLPs delivering the actual Ingress
> Packet
> Data.
> 
>   In the above scenario, if one were to turn off Enable Relaxed
> Ordering via
> the PCIe Capability, then the on-chip PCIe engine would simply never
> send a
> TLP with the Relaxed Ordering Attribute set, regardless of any other
> chip
> programming.
> 
>   And finally, just to be absolutely clear, using Relaxed Ordering
> isn't and
> "Architecture Thing".  It's a PCIe Fabric End Point Thing.  Many End
> Points
> simply ignore the Relaxed Ordering Attribute (except to reflect it back
> in
> Response TLPs).  In this sense, Relaxed Ordering simply provides
> potentially useful optimization information to the PCIe End Point.

I think your view matches what I found out about the current usage of the
"Enable Relaxed Ordering" bit in Linux mainline: i.e. looking at where and
why the other drivers set/clear the "Enable Relaxed Ordering" they do not
look for any global symbol, nor they look at the host architecture.

So with respect to this specific ixgbe driver I guess the main question is
why RO was disabled by default by Intel for this EP (commit 3d5c520727ce
mentions issues with "some chipsets"), then why it is safe to enable it back
on SPARC....?

Thanks
Gab

> 
> Casey

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next 1/4] ixgbe: sparc: rename the ARCH_WANT_RELAX_ORDER to IXGBE_ALLOW_RELAXED_ORDER

[Lucas Stach]

Am Donnerstag, den 27.04.2017, 12:19 -0500 schrieb Bjorn Helgaas:
> [+cc Casey]
> 
> On Wed, Apr 26, 2017 at 09:18:33AM -0700, Alexander Duyck wrote:
> > On Wed, Apr 26, 2017 at 2:26 AM, Ding Tianhong <dingtianhong@huawei.com> wrote:
> > > Hi Amir:
> > >
> > > It is really glad to hear that the mlx5 will support RO mode this year, if so, do you agree that enable it dynamic by ethtool -s xxx,
> > > we have try it several month ago but there was only one drivers would use it at that time so the maintainer against it, it mlx5 would support RO,
> > > we could try to restart this solution, what do you think about it. :)
> > >
> > > Thanks
> > > Ding
> > 
> > Hi Ding,
> > 
> > Enabing relaxed ordering really doesn't have any place in ethtool. It
> > is a PCIe attribute that you are essentially wanting to enable.
> > 
> > It might be worth while to take a look at updating the PCIe code path
> > to handle this. Really what we should probably do is guarantee that
> > the architectures that need relaxed ordering are setting it in the
> > PCIe Device Control register and that the ones that don't are clearing
> > the bit. It's possible that this is already occurring, but I don't
> > know the state of handling those bits is in the kernel. Once we can
> > guarantee that we could use that to have the drivers determine their
> > behavior in regards to relaxed ordering. For example in the case of
> > igb/ixgbe we could probably change the behavior so that it will bey
> > default try to use relaxed ordering but if it is not enabled in PCIe
> > Device Control register the hardware should not request to use it. It
> > would simplify things in the drivers and allow for each architecture
> > to control things as needed in their PCIe code.
> 
> I thought Relaxed Ordering was an optimization.  Are there cases where
> it is actually required for correct behavior?

Yes, at least the Tegra 2 TRM claims that RO needs to be enabled on the
device side for correct operation with the following language:

"Tegra 2 requires relaxed ordering for responses to downstream requests
(responses can pass writes). It is possible in some circumstances for
PCIe transfers from an external bus masters (i.e. upstream transfers) to
become blocked by a downstream read or non-posted write. The responses
to these downstream requests are blocked by upstream posted writes only
when PCIe strict ordering is imposed. It is therefore necessary to never
impose strict ordering that would block a response to a downstream
NPW/read request and always set the relaxed ordering bit to 1. Only
devices that are capable of relaxed ordering may be used with Tegra 2
devices."

Regards,
Lucas

Re: [PATCH net-next 1/4] ixgbe: sparc: rename the ARCH_WANT_RELAX_ORDER to IXGBE_ALLOW_RELAXED_ORDER

[Casey Leedom]

| From: Lucas Stach <l.stach@pengutronix.de>
| Sent: Friday, April 28, 2017 1:51 AM
|    =20
| Am Donnerstag, den 27.04.2017, 12:19 -0500 schrieb Bjorn Helgaas:
| >=20
| >=20
| > I thought Relaxed Ordering was an optimization.  Are there cases where
| > it is actually required for correct behavior?
|=20
| Yes, at least the Tegra 2 TRM claims that RO needs to be enabled on the
| device side for correct operation with the following language:
|=20
| "Tegra 2 requires relaxed ordering for responses to downstream requests
| (responses can pass writes). It is possible in some circumstances for PCI=
e
| transfers from an external bus masters (i.e. upstream transfers) to becom=
e
| blocked by a downstream read or non-posted write. The responses to these
| downstream requests are blocked by upstream posted writes only when PCIe
| strict ordering is imposed. It is therefore necessary to never impose str=
ict
| ordering that would block a response to a downstream NPW/read request and
| always set the relaxed ordering bit to 1. Only devices that are capable o=
f
| relaxed ordering may be used with Tegra 2 devices."

  (woof) Reading through the above paragraph is difficult because the autho=
r
seems to shift language and terminology mid sentence and isn't following
standard PCI terminology conventions.  The Root Complex is "Upstream", a
non-Root Complex Node in the PCIe Fabric is "Downstream", Requests that a
Downstream Device (End Point) send to the Root Complex are called "Upstream
Requests", responses that the Root Complex send to a Device are called
"Downstream Responses" (or, even more pedantically, "Responses sent
Downstream for an earlier Upstream Request").

  Because a Root Complex is Upstream, but the Requests it sent Downstream,
and Downstream Devices send their Requests Upstream, it's very important
that we use exceedingly precise language.

  So, it ~sounds like~ the nVidia Tegra 2 document is talking about the nee=
d
for Downstream Devices to echo the Relaxed Ordering Attribute in their
Responses directed Upstream to Requests sent Downstream from the Root
Complex.  Moreover, there's code in drivers/pci/host/pci-tegra.c:
tegra_pcie_relax_enable() which appears to set the PCIe Capability Device
Control[Enable Relaxed Ordering] bit on all PCIe Fabric Nodes.

  If my reading of the intent of the nVidia document is correct -- and
that's a Big If because of the extremely imprecise language used -- that
means that the tegra_pcie_relax_enable() is completely bogus.  The PCIe 3.0
Specification states that Responses MUST reflect the Relaxed Ordering and N=
o
Snoop Attributes of the Requests for which they are responding.  Section
2.2.9 of PCI Express(r) Base Specification Revision 3.0 November 10, 2010:
"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."

  And, specifically, the PCIe Capability Device Control[Enable Relaxed
Ordering] bit _only_ affects the ability of that Device to originate
Transaction Layer Packet Requests with the Relaxed Ordering Attribute set.
Thus, tegra_pcie_relax_enable() setting those bits on all the Downstream
Devices (and intervening Bridges) does not _cause_ those Devices to generat=
e
Requests with Relaxed Ordering set.  And, if the Devices are PCIe 3.0
compliant, it also doesn't affect the Responses that they send back Upstrea=
m
to the Root Complex.

  I apologize for the incredibly detailed nature of these responses, but
it's very easy for people new to PCIe to get these things wrong and/or
misinterpret the PCIe Specifications.

Casey