Re: [PATCH] MdePkg: add big-endian MMIO BaseBeIoLib

[Michael Brown <mcb30@ipxe.org>]

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.

Maybe we need some kind of wrapper library that provides an abstraction 
layer to automatically determine whether or not swapping is needed and 
then call the appropriate IoLib or IoSwapLib API function.  For example, 
the wrapper library could provide a function 
SwapIfNeededForBigEndianDeviceMmioRead16() which would perform a runtime 
check on each call to determine the current CPU endianness and then call 
MmioRead16() or SwapMmioRead16() as appropriate.

Michael