From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2a00:1450:400c:c0c::244; helo=mail-wr0-x244.google.com; envelope-from=leif.lindholm@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-wr0-x244.google.com (mail-wr0-x244.google.com [IPv6:2a00:1450:400c:c0c::244]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 4E1F72034D8CC for ; Thu, 22 Feb 2018 03:46:27 -0800 (PST) Received: by mail-wr0-x244.google.com with SMTP id k9so10256339wre.9 for ; Thu, 22 Feb 2018 03:52:27 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=UYp+Hub2neeVxv1+xji5Bf7Y6BMMD3ak1K782YuHotA=; b=ENc7kwTTUaRDf9mw/Y9orQ0rXxWOCCWHtXz9RPKGDJFxghscstEYHeiicps3XwSPTi PBNYWRtdKEp5fmjymnHSETV4zJUOyQ/SfM1WY8fx9aIxZiixS4RP1OuyvrsthsQgGKau NqNquMX9NAtmMbAfHueBFQ7+GRXcKlqVjctMw= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=UYp+Hub2neeVxv1+xji5Bf7Y6BMMD3ak1K782YuHotA=; b=C7G1DMr6WXYDr9ZIPjbMaBD0EATgvK41p5SrE6cAFGHui7sJreQvEdznALLZ/3KCOt 6vFClppbc/SF8Bfz3n7H0BPA7xUhg6T4mRU3nbjl/nBvrH5JKSR4P+t7IxcvG7TLvA4q yT+aF1AzfevW4Bzv5XJQEnPdkhPUQimdnj8jgt3+r4kUfcNcmhi7B/cY32ZgxGQN6mT6 GvtEDyHoPdb7XHwmsaanG7jAoZv7V1TzgGzu8RXawq+uSaQMe9IzQ/BUvHOOeXDkduOt xm5My3Y9dBTMUTK85Fm0umnqRCdEmP8UShDnAuCRzbcwr4R9a3jkICPZ4lSAmV3tbrw1 1VYg== X-Gm-Message-State: APf1xPCLwtURDKggwAwc8LK5BP1yWzbTulCJpf0XlUV0sZnGEK6WpZrj PXp/E6gGCKDTIbURjTwjdafUPA== X-Google-Smtp-Source: AH8x227A0eM6B5JRBz1lkP0Vs7FYjn70hEyrwqx0SA98g1ShA/2Nci387hvPxWPvJmGsD57EF6jZUA== X-Received: by 10.223.135.231 with SMTP id c36mr6339542wrc.36.1519300346164; Thu, 22 Feb 2018 03:52:26 -0800 (PST) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id q12sm30661873wrg.37.2018.02.22.03.52.25 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 22 Feb 2018 03:52:25 -0800 (PST) Date: Thu, 22 Feb 2018 11:52:23 +0000 From: Leif Lindholm To: Laszlo Ersek Cc: Meenakshi , michael.d.kinney@intel.com, edk2-devel@lists.01.org, ard.biesheuvel@linaro.org Message-ID: <20180222115223.xtfpc7du22drfkju@bivouac.eciton.net> 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> MIME-Version: 1.0 In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) 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 11:46:28 -0000 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit On Thu, Feb 22, 2018 at 09:34:05AM +0100, Laszlo Ersek wrote: > >> - 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.) Well, they work differently depending on whether host and net byte order differ or not :) > > 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. Interestingly, that paragraph never appeared when I searched through the spec... (but I see it now you point it out). Nevertheless, the spec is reflecting the current state of things. This is UEFI, not ULEEFI, and is a BE-only architecture was ever added, the spec would need to change. > >> - 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. OK. > (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). If the IoLib class is mapped to a an instance that byte-swaps (hereto referred to as BeIoLib if IoLibSwap is unsuitable), would we not then end up mapping the non-swapping, currently implemented in BaseLibIoIntrinsic, variant as BeIoLib? Or if not, do we end up needing to duplicated all IoLib implementation .infs to provide an IoLib and a BeIoLib for each? It's at that point I burst an aneurysm. Am I overthinking/underthinking this? > - 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. Yes, that does sound like the best solution for the code in the driver regardless. / Leif