From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932267AbeAXHbk (ORCPT ); Wed, 24 Jan 2018 02:31:40 -0500 Received: from mail-lf0-f66.google.com ([209.85.215.66]:46225 "EHLO mail-lf0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932095AbeAXHbj (ORCPT ); Wed, 24 Jan 2018 02:31:39 -0500 X-Google-Smtp-Source: AH8x224vHChaD6FlERPMuLPAeTf+NsVhv8Wb/nHOC0Md4n4edWAoVn8RrKoAYX+vJK+lxvRbIgwJ7g== Date: Wed, 24 Jan 2018 10:31:51 +0300 From: Serge Semin To: Jon Mason Cc: Arnd Bergmann , Dave Jiang , Allen.Hubbe@gmail.com, "Hook, Gary" , Sergey.Semin@t-platforms.ru, linux-ntb@googlegroups.com, Linux Kernel Mailing List Subject: Re: [PATCH] NTB: ntb_perf: fix cast to restricted __le32 Message-ID: <20180124073151.GB31120@mobilestation> References: <20180119173044.8013-1-fancer.lancer@gmail.com> <20180119210310.GA2284@mobilestation> <20180124041805.GD20474@kudzu.us> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180124041805.GD20474@kudzu.us> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Jan 23, 2018 at 11:18:06PM -0500, Jon Mason wrote: > On Fri, Jan 19, 2018 at 10:26:37PM +0100, Arnd Bergmann wrote: > > On Fri, Jan 19, 2018 at 10:03 PM, Serge Semin wrote: > > > > > > Actually the provided patch is the best solution I could come up with. > > > The thing is, that the methods can't be changed. Those functions are > > > the part of the NTB API methods used by many drivers. So basically they > > > are like pci_{read,write}_config_{byte,word,dword}() methods. We can't > > > change their prototypes only because it's suit some driver. The methods > > > give an access to the NTB device dummy u32-sized registers, nothing > > > else. So endianness is the transmitted data settings in this case. > > > > > > NTB is the technology to interconnect some two systems with possibly > > > different endianness (unlike PCI, which interconnect CPU with LE devices). > > > In this case I'd need to set some agreement up between two systems about > > > the endianness of the exchanged data like host and network types in > > > Linux networking. I've chosen the network data to be little-endian, > > > that's why I needed first to convert them from CPU to le32, then on > > > remote side convert them back from le32 to CPU. > > > > > > If you have any better suggestion how the warning can be fixed, I'd > > > be glad to stick to it. > > > > I don't think your description matches what you actually do: The > > underlying ntb hardware drivers (amd, idt, intel, mscc) all treat the > > incoming data as CPU-endian and convert it to little-endian on > > the register side, so the framework already assumes that whatever > > you do here uses a little-endian wire-level protocol. > > > > On a little-endian kernel/CPU, nothing is ever swapped here, neither > > in the ntb_perf front-end nor in the back-ends. On a big-endian > > kernel/CPU, they both swap, so you end up with CPU-endian > > data on the wire, so it should be impossible for a big-endian > > system to talk to a little-endian one. Have you actually tried that > > combination with the current code? > > I do not believe anyone has every tried NTB on a big endien system, > let alone tried it with one side LE and the other BE. To my > knowledge, this has only ever been used on x86 to x86. > > > If my interpretation is correct, then the best solution would be to > > completely remove the cpu_to_le32/le32_to_cpu conversions > > from ntb_perf, and just define that it works like any other PCI > > device, exchanging little-endian data. > > Yes, this would be the best solution. Thank you for the insight. > > Serge, when you get a chance, please make this change and resumbit. > Ok. I'll do it in an hour. Regards, -Sergey > Thanks, > Jon > > > > > > There are two interesting cases to consider though: > > > > - if someone wants to implement an NTB based protocol > > using big-endian data on the wire, you probably want to add > > a ntb_peer_spad_read_be()/ntb_peer_msg_write_be() > > set of interfaces, to go along with ioread32_be()/iowrite32_be() > > the same way that ntb_peer_spad_read()/ntb_peer_msg_write() > > ends up doing ioread32()/iowrite32() with the implied little-endian > > behavior. > > > > - memcpy_toio()/memcpy_fromio() and ioread32_rep()/iowrite32_rep > > importantly do not do any byteswap, they are meant to > > transfer byte streams. > > > > Arnd