Re: [PATCH edk2-platforms 01/39] Silicon/NXP: Add support for Big Endian Mmio APIs

[Udit Kumar <udit.kumar@nxp.com>]

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Laszlo Ersek
> Sent: Thursday, February 22, 2018 7:26 PM
> To: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: michael.d.kinney@intel.com; edk2-devel@lists.01.org;
> ard.biesheuvel@linaro.org
> Subject: Re: [edk2] [PATCH edk2-platforms 01/39] Silicon/NXP: Add support
> for Big Endian Mmio APIs
> 
> On 02/22/18 12:52, Leif Lindholm wrote:
> > On Thu, Feb 22, 2018 at 09:34:05AM +0100, Laszlo Ersek wrote:
> 
> >>> 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?
> 
> We need two library classes, one for talking to LE devices and another to BE
> devices. These should be usable in a given module at the same time, as Ard
> says.
> 
> Both library classes need to work on both LE and BE CPUs (working from
> your suggestion that UEFI might grow BE CPU support at some point).
> Whether that is implemented by dumb, separate library instances (yielding
> in total 2*2=4 library instances), or by smart, CPU-endianness-agnostic
> library instances (in total, 2), is a different question.
> 
> Note that such "smarts" could be less than trivial to implement:
> - check CPU endianness in each library API?
> - or check in the lib constructor only, and flip some function pointers?
> - use a dynamic PCD for caching CPU endianness?
> - use a HOB for the same?
> - use a lib global variable (for caching only on the module level)?
> 
> 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 
Thanks
Udit
 
> Thanks,
> Laszlo
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.
> 01.org%2Fmailman%2Flistinfo%2Fedk2-
> devel&data=02%7C01%7Cudit.kumar%40nxp.com%7C930d96bb226d4491df2
> d08d579fc1717%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63654
> 9046016636482&sdata=0uPLjiDP60oNVSdbh44gostMx2id%2BzdLYjk8t%2BLwJ
> tU%3D&reserved=0