linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alexander Duyck <alexander.duyck@gmail.com>
To: Casey Leedom <leedom@chelsio.com>
Cc: Ding Tianhong <dingtianhong@huawei.com>,
	"ashok.raj@intel.com" <ashok.raj@intel.com>,
	"helgaas@kernel.org" <helgaas@kernel.org>,
	Michael Werner <werner@chelsio.com>,
	Ganesh GR <ganeshgr@chelsio.com>,
	"asit.k.mallick@intel.com" <asit.k.mallick@intel.com>,
	"patrick.j.cramer@intel.com" <patrick.j.cramer@intel.com>,
	"Suravee.Suthikulpanit@amd.com" <Suravee.Suthikulpanit@amd.com>,
	"Bob.Shaw@amd.com" <Bob.Shaw@amd.com>,
	"l.stach@pengutronix.de" <l.stach@pengutronix.de>,
	"amira@mellanox.com" <amira@mellanox.com>,
	"gabriele.paoloni@huawei.com" <gabriele.paoloni@huawei.com>,
	"David.Laight@aculab.com" <David.Laight@aculab.com>,
	"jeffrey.t.kirsher@intel.com" <jeffrey.t.kirsher@intel.com>,
	"catalin.marinas@arm.com" <catalin.marinas@arm.com>,
	"will.deacon@arm.com" <will.deacon@arm.com>,
	"mark.rutland@arm.com" <mark.rutland@arm.com>,
	"robin.murphy@arm.com" <robin.murphy@arm.com>,
	"davem@davemloft.net" <davem@davemloft.net>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v6 0/3] Add new PCI_DEV_FLAGS_NO_RELAXED_ORDERING flag
Date: Fri, 7 Jul 2017 17:47:21 -0700	[thread overview]
Message-ID: <CAKgT0Uc5KSJTj4A=9pSudiJ28AW0krc7FZpdW4upwY+FgpeArA@mail.gmail.com> (raw)
In-Reply-To: <MWHPR12MB160006024D920ED58126048AC8AB0@MWHPR12MB1600.namprd12.prod.outlook.com>

On Fri, Jul 7, 2017 at 5:30 PM, Casey Leedom <leedom@chelsio.com> wrote:
>   By the way, it ~seems~ like the patch set confuses the idea of the PCIe Capability Device Control[Relaxed Ordering Enable] with the device's ability to handle incoming TLPs with the Relaxed Ordering Attribute set.  These are completely different things.  The PCIe Capability Device Control[Relaxed Ordering Enable] solely governs the ability of the device to _send_ TLPs with the Relaxed Ordering Attribute set.  It has nothing whatsoever to do with the handling of incoming TLPs with the Relaxed Ordering Attribute set.  In fact, there is any standard way to disable receipt processing of such TLPs.  That's kind of the whole point of the majority of the patch.  If there was a separate "Ignore Incoming Relaxed Ordering Attributes" then we'd mostly just have a quirk which would set that for problematic devices.
>
>   Worse yet, if I'm reading the patch right, it _is_ turning off the PCIe Capability Device Control[Relaxed Ordering Enable] for the devices that we've identified with problematic receive handling.  Not only does this not solve the problem that we've been talking about, it actually almost certainly introduces a huge Graphics Performance Bug.  The Relaxed Ordering Attribute was originally developed for Graphics Device support in order to download textures, etc. to a graphics devices as fast as possible and only ensure ordering at points where needed.  For instance, as far as I know, the Intel Root Complexes that we've been talking about have no issues whatsoever in generating downstream TLPs with the Relaxed Ordering Attribute set.  By turning off the PCIe Capability Device Control[Relaxed Ordering Enable] on these Root Complexes, this patch prevents the generation of downstream TLPs with the Relaxed Ordering Attribute set targeted at devices like graphics adapters.
>
> Casey

The patches should be disabling the relaxed ordering on upstream
facing ports that would be sending requests to the root complex. So if
the root complex cannot handle receiving a message with relaxed
ordering enabled we should be clearing the relaxed ordering bit in the
PCIe configuration on those device that could send TLPs to that root
complex. By doing that the devices hosted on that root complex cannot
generate requests that contain relaxed ordering TLPs.

I'll have to do some double checking of the patches, but I thought we
were only clearing the relaxed ordering bits on the devices that would
be sending requests to the root complex, not the downstream ports of
the root complex itself. As such we should be able to generate relaxed
ordering TLPs from the root complex, but the devices hosted on the bus
shouldn't be able to generate relaxed ordering TLPs. The bit in the
configuration space controls the ability to generate content, not the
ability to receive it so clearing the bit on the device should be the
correct behavior for this.

As far as your suggestions for the cxgb4 patch it has the same problem
it originally had. We want to disable the relaxed ordering bit on the
device since the device should not be generating relaxed ordering
requests. This is why I was trying to get your input on this as I know
you have a peer-to-peer configuration that you wanted to support. What
we need to do is identify what platforms cannot support relaxed
ordering to the root complex and prevent that from happening which is
why we clear the relaxed ordering bit on the device. In that setup we
need to have a good way to identify when this has occurred and instead
setup a side channel type configuration where we then re-enable
relaxed ordering, but only allow for sending directly to the peer and
not the root complex.

- Alex

  reply	other threads:[~2017-07-08  0:47 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-22 12:15 [PATCH v6 0/3] Add new PCI_DEV_FLAGS_NO_RELAXED_ORDERING flag Ding Tianhong
2017-06-22 12:15 ` [PATCH v6 1/3] PCI: Add new PCIe Fabric End Node flag, PCI_DEV_FLAGS_NO_RELAXED_ORDERING Ding Tianhong
2017-06-22 12:15 ` [PATCH v6 2/3] PCI: Enable PCIe Relaxed Ordering if supported Ding Tianhong
2017-06-22 12:15 ` [PATCH v6 3/3] net/cxgb4: Use new PCI_DEV_FLAGS_NO_RELAXED_ORDERING flag Ding Tianhong
2017-06-29  5:47 ` [PATCH v6 0/3] Add " Ding Tianhong
2017-07-06 12:58   ` Ding Tianhong
2017-07-06 17:17     ` Bjorn Helgaas
2017-07-07  1:03       ` Ding Tianhong
2017-07-07 21:48 ` Casey Leedom
2017-07-08  0:30   ` Casey Leedom
2017-07-08  0:47     ` Alexander Duyck [this message]
2017-07-08  2:04       ` Casey Leedom
2017-07-08  3:37         ` Alexander Duyck
2017-07-11  0:01           ` Casey Leedom
2017-07-11 20:33             ` Alexander Duyck
2017-07-12  9:46             ` Ding Tianhong
2017-07-13  0:52               ` Casey Leedom
2017-07-13  1:18                 ` Ding Tianhong
2017-07-13  2:28                   ` Casey Leedom
2017-07-10 10:49         ` Ding Tianhong

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='CAKgT0Uc5KSJTj4A=9pSudiJ28AW0krc7FZpdW4upwY+FgpeArA@mail.gmail.com' \
    --to=alexander.duyck@gmail.com \
    --cc=Bob.Shaw@amd.com \
    --cc=David.Laight@aculab.com \
    --cc=Suravee.Suthikulpanit@amd.com \
    --cc=amira@mellanox.com \
    --cc=ashok.raj@intel.com \
    --cc=asit.k.mallick@intel.com \
    --cc=catalin.marinas@arm.com \
    --cc=davem@davemloft.net \
    --cc=dingtianhong@huawei.com \
    --cc=gabriele.paoloni@huawei.com \
    --cc=ganeshgr@chelsio.com \
    --cc=helgaas@kernel.org \
    --cc=jeffrey.t.kirsher@intel.com \
    --cc=l.stach@pengutronix.de \
    --cc=leedom@chelsio.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=netdev@vger.kernel.org \
    --cc=patrick.j.cramer@intel.com \
    --cc=robin.murphy@arm.com \
    --cc=werner@chelsio.com \
    --cc=will.deacon@arm.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;
as well as URLs for NNTP newsgroup(s).