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::230; helo=mail-wr0-x230.google.com; envelope-from=leif.lindholm@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-wr0-x230.google.com (mail-wr0-x230.google.com [IPv6:2a00:1450:400c:c0c::230]) (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 982BF2034D8F7 for ; Wed, 28 Feb 2018 05:13:02 -0800 (PST) Received: by mail-wr0-x230.google.com with SMTP id u49so2370551wrc.10 for ; Wed, 28 Feb 2018 05:19:09 -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:in-reply-to:user-agent; bh=OfCtkwX/oIET0+2UIuJ4VB57KSv5GLzHJ3Bisa+PcQY=; b=bnKsvVI6vR5XY29pyKZ+JJuGHhQOU1yxBYljJqW7j/ngzVHA5kx+Ogf4h0pgKYbdsp a01ZbBGmZRvZPoC9GvX/fE8y8aX2vtFOZzWC78mnQuHoMhSxF6EihwJS94KCTTzslc4/ AjymCAjaBOfw1Mgbspy0R7qNB8YNQq8/WH0ps= 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:in-reply-to:user-agent; bh=OfCtkwX/oIET0+2UIuJ4VB57KSv5GLzHJ3Bisa+PcQY=; b=ZM31YmW6bDXCTcgQiqgrSbMafMzJTYa6VefyzGWtaRljnEnthF42ZHVwYfXM7Df3JX jdHjFvAtwTIn7cytjDhzy7shkh1qTGp4lDCBL1cCL/ajTWR35TTrNJha+yEzzAuKPJqU i9RzOOC++vSOuEK7v9KXrauHwuNQuZ5ampEEIa1vDjQes7BHqrju0v7RVEjRjdmshTcL PeVmfiyXjz3fJ6REQsVj4ZqmgvSqGat4UlklXUJN18l8han9JfAqy7IbnxgKAdq8xNqr lKDegP6eGpzgTLibVO4ALXefu1E/3TWlH96XEF70iPj6XFbysCMmSWXGWMBnTm6+2J3T HxBQ== X-Gm-Message-State: APf1xPCbRTHW5xMbxIkfSdblhn/ClhWDdLoTU5Yqazh/IIcuMvfXBp9w vTX/UFL9gWb517g8pLP484WJOQ== X-Google-Smtp-Source: AH8x2246t50Z+RBIsaqYxAXk4RaSyyuSp5lslXLr9OfP+SfX3GMdjum7m4qT7DXIgMKy8SCEF/Yy2Q== X-Received: by 10.223.209.11 with SMTP id a11mr16236397wri.122.1519823948275; Wed, 28 Feb 2018 05:19:08 -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 p21sm1727651wmc.43.2018.02.28.05.19.06 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 28 Feb 2018 05:19:07 -0800 (PST) Date: Wed, 28 Feb 2018 13:19:05 +0000 From: Leif Lindholm To: Laszlo Ersek Cc: Udit Kumar , "michael.d.kinney@intel.com" , "edk2-devel@lists.01.org" , "ard.biesheuvel@linaro.org" Message-ID: <20180228131905.ix2kbhdslnz66e54@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> <20180222115223.xtfpc7du22drfkju@bivouac.eciton.net> <2a1fa56f-98db-a1c1-d973-7e84cc7dc1fa@redhat.com> <58b7fe3f-8e39-02a2-76a9-cf904280d298@redhat.com> MIME-Version: 1.0 In-Reply-To: <58b7fe3f-8e39-02a2-76a9-cf904280d298@redhat.com> 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: Wed, 28 Feb 2018 13:13:03 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Fri, Feb 23, 2018 at 11:47:28AM +0100, Laszlo Ersek wrote: > >> I think the solution that saves the most on the *source* code size > >> is: > >> - introduce the BeIoLib class > >> - duplicate the MMIO functions from BaseIoLibIntrinsic to the one > >> BeIoLib instance that we introduce > >> - modify the MMIO functions in *both* lib instances (original LE, and > >> new BE), like this: > >> > >> - If the CPU architecture is known to be bound to a single > >> endianness, then hardcode the appropriate operation. This can be > >> done with preprocessor macros, or with the architecture support > >> of INF files / separate source files. For example, on IA32 and > >> X64, the IoLib instance should work transparently, > >> unconditionally, and the BeIoLib instance should byte-swap, > >> unconditionally. > >> > >> - On other CPU arches, all the wider-than-byte MMIO functions, in > >> *both* lib instances should do something like this: > >> > >> // > >> // at file scope > >> // > >> STATIC CONST UINT16 mOne = 1; > >> > >> // > >> // at function scope > >> // > >> if (*(CONST UINT8 *)&mOne == 1) { > >> // > >> // CPU in LE mode: > >> // - work transparently in the IoLib instance > >> // - byte-swap in the BeIoLib instance > >> // > >> } else { > >> // > >> // CPU in BE mode: > >> // - byte-swap in the IoLib instance > >> // - work transparently in the BeIoLib instance > >> // > >> } > > > > You meant, sort of cpu_to_le and cpu_to_be sort of APIs > > I'm lost. I don't know how to put it any clearer. Let me try with actual > code. > > (a) Suggested for "MdePkg/Library/BaseIoLibIntrinsic/IoLib.c", which is > used on IA32 and X64, therefore CPU byte order is little endian only: > > > UINT32 > > EFIAPI > > MmioWrite32 ( > > IN UINTN Address, > > IN UINT32 Value > > ) > > { > > ASSERT ((Address & 3) == 0); > > > > MemoryFence (); > > *(volatile UINT32*)Address = Value; > > MemoryFence (); > > > > return Value; > > } > > In other words, no change to the current implementation. > > > (b) Suggested for "MdePkg/Library/BaseBeIoLibIntrinsic/IoLib.c", also to > be used on IA32 and X64. Because the CPU byte order is LE only, this > variant will byte-swap unconditionally. > > > UINT32 > > EFIAPI > > BeMmioWrite32 ( > > IN UINTN Address, > > IN UINT32 Value > > ) > > { > > ASSERT ((Address & 3) == 0); > > > > MemoryFence (); > > *(volatile UINT32*)Address = SwapBytes32 (Value); > > MemoryFence (); > > > > return Value; > > } > > (c) Suggested for "MdePkg/Library/BaseIoLibIntrinsic/IoLibArm.c", which > is used on ARM and AARCH64. And here I'll assume that the CPU byte order > on those can be either LE or BE. We badly need to reduce the number of architecture-specific libraries for doing mmio accesses from C code, rather than increasing them. So, I realise I've confused the situation somewhat here by talking about big-endian CPUs. We are not looking to support big-endian CPUs today. All I wanted was to make sure we don't unneccesarily build in assumptions in the codebase about things that could change in the future with fairly minor changes to the specifications. What I do see as an absolute is that a single _UEFI_ architecture could never be more than one possible endianness. I.e., if someone wanted to (and this _really_ isn't me being *nudge* *nudge*, *wink* *wink*) bring in a BE-port of AArch64, that wouldn't be AARCH64, that would be AARCH64BE. Which also means I don't think there is any need for any sort of runtime detection of this. Your proposal of a set of function-pointers in the driver being mapped to appropriate device endianness on initialization seems sufficient to resolve the situation posed. And the situation feels sufficiently esoteric to motivate that level of clunkiness. Regards, Leif