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 D824421F2E166 for ; Tue, 17 Apr 2018 01:01:17 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 2228484250; Tue, 17 Apr 2018 08:01:17 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-161.rdu2.redhat.com [10.10.120.161]) by smtp.corp.redhat.com (Postfix) with ESMTP id 374492026DFD; Tue, 17 Apr 2018 08:01:16 +0000 (UTC) To: Michael Brown , "Kinney, Michael D" , Leif Lindholm Cc: "edk2-devel@lists.01.org" , "Gao, Liming" References: <20180413174211.858-1-leif.lindholm@linaro.org> <20180413193143.t45tua3yi7sopk4d@bivouac.eciton.net> <20180416100712.6v642ycksvmoffvt@bivouac.eciton.net> <52ea5684-380d-0519-2545-6ef7f62198ae@ipxe.org> <09cc135a-c3b7-bcbd-1a71-6454e3ba7623@redhat.com> <52f131ae-ed51-9276-ac91-e3dded7472e2@ipxe.org> From: Laszlo Ersek Message-ID: <56ad661e-3f48-3889-0641-ae64e8da178a@redhat.com> Date: Tue, 17 Apr 2018 10:01:15 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: <52f131ae-ed51-9276-ac91-e3dded7472e2@ipxe.org> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.2]); Tue, 17 Apr 2018 08:01:17 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.2]); Tue, 17 Apr 2018 08:01:17 +0000 (UTC) for IP:'10.11.54.4' DOMAIN:'int-mx04.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'lersek@redhat.com' RCPT:'' Subject: Re: [PATCH] MdePkg: add big-endian MMIO BaseBeIoLib X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 17 Apr 2018 08:01:18 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 04/17/18 00:14, Michael Brown wrote: > On 16/04/18 21:42, Laszlo Ersek wrote: >> On 04/16/18 16:34, Michael Brown wrote: >>> On 16/04/18 15:10, Kinney, Michael D wrote: >>>> I think we only need a single lib class and lib >>>> Instance that does the byte swap and we should >>>> not use Le or Be in any of the names of the class, >>>> instance, or APIs. Just "Swap". >>> >>> I may have misunderstood, but wouldn't using "Swap" within the API >>> names effectively encode knowledge of the endianness of the _build_ >>> platform into the source code? This would prevent the same source >>> code being built for both little-endian and big-endian CPUs. >> >> Under this scenario, all drivers meant to be portable to both byte >> orders would have to: >> - link against both IoLib and IoSwapLib, >> - determine at device binding time, from CPU endianness and device >> endianness combined, whether swapping was needed for that device, >> - call the IoLib or IoSwapLib APIs through wrapper functions, or >> function pointers. > > Given that "all drivers meant to be portable to both byte orders" > would include almost the complete set of PCI device drivers, that > sounds like an incredibly large amount of unnecessarily duplicated > boilerplate code. I don't think any of the library classes under discussion are suitable for UEFI drivers that bind PCI devices. Such drivers should use the PciIo protocol members to access registers in MMIO BARs. Instead I think these lib classes target DXE (= platform) drivers, plus modules that run in earlier phases (PEI and maybe SEC). > Maybe we need some kind of wrapper library that provides an > abstraction layer to automatically determine whether or not swapping > is needed I agree. > and then call the appropriate IoLib or IoSwapLib API function. For > example, the wrapper library could provide a function > SwapIfNeededForBigEndianDeviceMmioRead16() (A 40-character function name; that would fit edk2 well :) ) > which would perform a runtime check on each call to determine the > current CPU endianness and then call MmioRead16() or SwapMmioRead16() > as appropriate. Right, that would be a suitable Base implementation, requiring no writeable global variables. The thing is, the BeIoLib and LeIoLib classes are already good for this -- they can be implemented as you suggest. So no need to call the function SwapIfNeededForBigEndianDeviceMmioRead16(), just call it BeMmioRead16(). Earlier I suggested the BaseBeIoLib and BaseBeIoLibSwap instances, through which the platform DSC would select swapping-or-not at build time. But that doesn't necessarily has to happen statically. We could have just one BaseBeIoLib instance that linked against both IoLib and IoSwapLib, and determined dynamically which one to delegate work to, on every call. Thanks Laszlo