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

[Laszlo Ersek <lersek@redhat.com>]

On 04/17/18 10:24, Michael Brown wrote:
> On 17/04/18 09:01, Laszlo Ersek wrote:
>> 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().
> 
> I know.  I thought that suggesting a 40-character function name

Yes, I noticed that (and commented on it).

> and a
> runtime check in case the CPU endianness changed mid-execution would be
> sufficiently obviously ridiculous,

Linking BaseBeIoLib (and BaseLeIoLib) against both IoLib and IoSwapLib,
and then deciding with an "if" in the code which one to call, would not
expect the CPU to change endianness mid-execution. The controlling
expression of the "if" would be evaluable at compile time, and then
link-time optimization at the latest could fully eliminate one of IoLib
/ IoSwapLib. This was discussed last time the topic was alive:

http://mid.mail-archive.com/2a1fa56f-98db-a1c1-d973-7e84cc7dc1fa@redhat.com

    //
    // at file scope
    //
    STATIC CONST UINT16 mOne = 1;

    //
    // at function scope
    //
    if (*(CONST UINT8 *)&mOne == 1) {

Keeping the "if" in genuine C source code provides better source code
coverage (for compiling anyway) than conditional compilation via macros
(this is a practice the SeaBIOS project follows as well).

> but I fear that it may have sounded
> too plausible for EDK2.

Please don't troll? I've spent some time on this because I was asked for
my opinion. You may find the edk2 coding practices ridiculous (and we're
totally not blind to their shortcomings either), and/or my thoughts on
the matter painfully naive, but trolling just wastes our time.

I guess I'm out of this thread.
Laszlo