From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753706AbdGJKud (ORCPT ); Mon, 10 Jul 2017 06:50:33 -0400 Received: from szxga03-in.huawei.com ([45.249.212.189]:8888 "EHLO szxga03-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753649AbdGJKub (ORCPT ); Mon, 10 Jul 2017 06:50:31 -0400 Subject: Re: [PATCH v6 0/3] Add new PCI_DEV_FLAGS_NO_RELAXED_ORDERING flag To: Casey Leedom , Alexander Duyck References: <1498133721-21152-1-git-send-email-dingtianhong@huawei.com> CC: "ashok.raj@intel.com" , "helgaas@kernel.org" , Michael Werner , Ganesh GR , "asit.k.mallick@intel.com" , "patrick.j.cramer@intel.com" , "Suravee.Suthikulpanit@amd.com" , "Bob.Shaw@amd.com" , "l.stach@pengutronix.de" , "amira@mellanox.com" , "gabriele.paoloni@huawei.com" , "David.Laight@aculab.com" , "jeffrey.t.kirsher@intel.com" , "catalin.marinas@arm.com" , "will.deacon@arm.com" , "mark.rutland@arm.com" , "robin.murphy@arm.com" , "davem@davemloft.net" , "linux-arm-kernel@lists.infradead.org" , "netdev@vger.kernel.org" , "linux-pci@vger.kernel.org" , "linux-kernel@vger.kernel.org" From: Ding Tianhong Message-ID: <0df11d6f-c9cb-3c6b-ba08-d5e5c45754ae@huawei.com> Date: Mon, 10 Jul 2017 18:49:02 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.177.23.32] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A020204.59635BAD.0012,ss=1,re=0.000,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0, ip=0.0.0.0, so=2014-11-16 11:51:01, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: 4d68fe2a8e86e65fcbf89d1fb3981d0f Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Casey: On 2017/7/8 10:04, Casey Leedom wrote: > Okay, thanks for the note Alexander. I'll have to look more closely at > the patch on Monday and try it out on one of the targeted systems to verify > the semantics you describe. > All the modification is only clearing the device's Device Control{Relaxed Ordering Enable]bit when distinguish that the platform should not support RO and did nothing to the RC configuration, so I don't think it will break anything compare to the first version from yours. > However, that said, there is no way to tell a priori where a device will > send TLPs. To simply assume that all TLPs will be directed towards the Root > Complex is a big assumption. Only the device and the code controlling it > know where the TLPs will be directed. That's why there are changes required > in the cxgb4 driver. For instance, the code in > drivers/net/ethernet/chelsio./cxgb4/sge.c: t4_sge_alloc_rxq() knows that > it's allocating Free List Buffers in Host Memory and that the RX Queues that > it's allocating in the Hardware will eventually send Ingress Data to those > Free List Buffers. (And similarly for the Free List Buffer Pointer Queue > with respect to DMA Reads from the host.) In that routine we explicitly > configure the Hardware to use/not-use the Relaxed Ordering Attribute via the > FW_IQ_CMD_FL0FETCHRO and FW_IQ_CMD_FL0DATARO flags. Basically we're > conditionally setting them based on the desirability of sending Relaxed > Ordering TLPs to the Root Complex. (And we would perform the same kind of > check for an nVME application ... which brings us to ...) > > And what would be the code using these patch APIs to set up a Peer-to-Peer > nVME-style application? In that case we'd need the Chelsio adapter's PCIe > Capability Device Control[Relaxed Ordering Enable] set for the nVME > application ... and we would avoid programming the Chelsio Hardware to use > Relaxed Ordering for TLPs directed at the Root Complex. Thus we would be in > a position where some TLPs being emitted by the device to Peer devices would > have Relaxed Ordering set and some directed at the Root Complex would not. > And the only way for that to work is if the source device's Device > Control[Relaxed Ordering Enable] is set ... > > Finally, setting aside my disagreements with the patch, we still have the > code in the cxgb4 driver which explicitly turns on its own Device > Control[Relaxed Ordering Enable] in cxgb4_main.c: > enable_pcie_relaxed_ordering(). So the patch is something of a loop if all > we're doing is testing our own Relaxed Ordering Enable state ... > > Casey > > . >