From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=66.187.233.73; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 8B42C21FD73C6 for ; Thu, 22 Feb 2018 00:28:08 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 9B2A9EAEAB; Thu, 22 Feb 2018 08:34:07 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-138.rdu2.redhat.com [10.10.120.138]) by smtp.corp.redhat.com (Postfix) with ESMTP id 38424213AEE2; Thu, 22 Feb 2018 08:34:06 +0000 (UTC) To: Leif Lindholm Cc: Meenakshi , michael.d.kinney@intel.com, edk2-devel@lists.01.org, ard.biesheuvel@linaro.org References: <1518771035-6733-1-git-send-email-meenakshi.aggarwal@nxp.com> <1518771035-6733-2-git-send-email-meenakshi.aggarwal@nxp.com> <20180221154601.nkbp2xmy3zb2xolm@bivouac.eciton.net> <20180221185818.arwfhombntutnt23@bivouac.eciton.net> From: Laszlo Ersek Message-ID: Date: Thu, 22 Feb 2018 09:34:05 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: <20180221185818.arwfhombntutnt23@bivouac.eciton.net> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.6 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.1]); Thu, 22 Feb 2018 08:34:07 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.1]); Thu, 22 Feb 2018 08:34:07 +0000 (UTC) for IP:'10.11.54.6' DOMAIN:'int-mx06.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'lersek@redhat.com' RCPT:'' Subject: Re: [PATCH edk2-platforms 01/39] Silicon/NXP: Add support for Big Endian Mmio APIs X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 22 Feb 2018 08:28:09 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit On 02/21/18 19:58, Leif Lindholm wrote: > On Wed, Feb 21, 2018 at 05:06:02PM +0100, Laszlo Ersek wrote: >> On 02/21/18 16:46, Leif Lindholm wrote: >>> Apologies for dropping the ball on this series during my sabbatical. >>> >>> For this particular patch, I would still like to see a core library >>> provide the needed functionality. I just sent out an RFC of a possible >>> implementation. >>> >>> Regardless, a key point is that this isn't about "big-endian", it is >>> about endianness opposite to the executing processor. >> >> I commented on just this aspect under your RFC. I think I disagree, for >> two reasons: >> >> - As long as the specs are LE-only, "endianness opposite to the >> executing processor" is needless complication / speculative generality >> in my eyes. > > HTON/NTOH? I don't understand this reference; host-to-net and net-to-host seem to support my suggestion. "net" is always BE, and "host" is whatever it is. So the API names are the same, but they work differently, dependent on CPU byte order. (Apologies if you mean something else by HTON/NTOH.) > The specs are not LE-only. > PI _is_ (at this point in time) LE-only. > UEFI leaves this entirely to architectural bindings. I disagree; from UEFI 2.7: 1 Introduction 1.9 Conventions Used in this Document 1.9.1 Data Structure Descriptions Supported processors are “little endian” machines. This distinction means that the low-order byte of a multibyte data item in memory is at the lowest address, while the high-order byte is at the highest address. Some supported 64-bit processors may be configured for both “little endian” and “big endian” operation. All implementations designed to conform to this specification use “little endian” operation. > For PI, this is mentioned in a single paragraph, repeated 4 times in > the PI 1.6 specification (due to it merging what was previously > separate documents). The same paragraph is present in UEFI 2.7. >> - Even if we supported multiple endiannesses on the CPU front, the API >> names should reflect the *device* byte order, not the CPU byte order. >> Think of the case when the same platform device is integrated on board >> B1 whose CPU is LE, and on board B2 whose CPU is BE. > > The actual watchdog code in this series, and comments made on the > list, suggests that there exists variants of this _device_ with BE > or LE byte order. > > If this is not the case, then yes, I agree that BE-naming makes sense. In my opinion, even if the same device is seen in the wild with both byte orders, the library class (the function names) should be designed independently of CPU byte order. The device driver should pick one set of functions dependent on device byte order (if indeed both byte orders are seen on variants of the device). How those APIs map to CPU byte order is a library instance detail. > So, Meenakshi - can you confirm that the Watchdog driver is expected > to be used against devices in both BE and LE mode? > > If it is the case, maybe this library would make more sense as the > non-standard protocol you suggested in > https://www.mail-archive.com/edk2-devel@lists.01.org/msg17869.html > ? Hmmm. Looking back at that email of mine, I think my "incomplete" argument no longer stands. Jordan and Mike have explained to me since that BaseLib (the class) has several APIs that are simply un-implementable on various CPUs, for example it has a number of Itanium-specific functions. Drivers that are not tied to IPF should simply not call them. This is supposed to be by design. Having skimmed your RFC, I also think I may have been wrong about "overkill" originally. If there are many BE registers, swapping bytes all the time explicitly is annoying, and people can mess up the word size. So right now I feel that having these APIs in a new lib class should be OK, as long as the function names are clear. (Clearly I've been wrong already on this topic, see above, so do take my points with a grain of salt :) ) >> If we name the APIs >> after the CPU byte order, then the same driver source code will be >> misleading on one of the boards. Whereas, if we name the APIs after >> device byte order, then the driver source code will be correct >> regardless of board / CPU, and only the internal workings of the APIs >> should change. For example, on a BE CPU / platform, the "normal" (LE) >> IoLib class should be resolved to an instance that byte-swaps >> internally, and the BE IoLib class should be resolved to an instance >> that is transparent internally. > > Right. > > But that brings back the complication as to how we have a driver that > needs an LE IO library to write output, and a BE IO library to > manipulate the hardware. Can you please explain the "write output" use case more precisely? My thinking would be this: - Use the IoLib class directly for "writing output" in little endian byte order (which is still unclear to me sorry). - Have some function pointers (global variables, or members of a global structure) that point to either IoLib or BeIoLib functions, dependent on the detected device byte order. A module can be linked against both IoLib and BeIoLib. - Use those function pointers to talk to the device. Thanks! Laszlo