Re: [edk2-devel] [PATCH] CryptoPkg/FltUsedLib: Add FltUsedLib for float.

["Guomin Jiang" <guomin.jiang@intel.com>]

Summarize current status:

Problem Statement:
Openssl require _fltused to be defined as a constant anywhere floating point is used.
It may use float out of edk2 tree and need _fltused, for example, Microsoft’s OnScreenKeyboard and UiToolKit.

Current Proposal as below:

Proposal 1: Add FltUsed.c into exist library
Detail: Add FltUsed.c into EntryPointLib(PEIM, DXE).
Recommend: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Approve: Laszlo Ersek lersek@redhat.com
Netual: Michael D Kinney <michael.d.kinney@intel.com>
Benefit: Doesn’t need modify every .dsc description file.
Defection: I test that it will fail because of /GL option, the error show fatal error LNK1237: during code generation, compiler introduced reference to symbol '_fltused' defined in module 'UefiApplicationEntryPoint.lib(FltUsed.obj)' compiled with /GL

Proposal 2: Define NULL library
Recommend: Michael D Kinney michael.d.kinney@intel.com
Oppose: Sean sean.brogan=microsoft.com@groups.io , Ard Biesheuvel <ard.biesheuvel@linaro.org>
Benefit: I test it and prove that it is executable.
Defection: It is required that modify every description file.
Defection: It need be very careful that it should only apply some module type(PEIM, DXE_DRIVER, UEFI_APPLICATION) rather than all.
Defection: Build break up.
Action Required: Need evaluate the affection on size.
Consideration: Add PCD to control the feature

Proposal 3: Define FltUsedLib
Detail: Define FltUsedLib and add dependence
Oppose: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Benefit: Doesn’t need care that which module will use it, we will explicitly point it in component file.
Defection: More complex, It is required that modify every description file and modify component meanwhile.
Defection: Build breakup

Thanks
guomin

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, Ray
> Sent: Tuesday, April 14, 2020 1:03 PM
> To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>;
> ard.biesheuvel@linaro.org; Kinney, Michael D <michael.d.kinney@intel.com>
> Cc: lersek@redhat.com; macarl@microsoft.com
> Subject: Re: [edk2-devel] [PATCH] CryptoPkg/FltUsedLib: Add FltUsedLib for
> float.
> 
> UEFI spec allows using float operation so asking module developers avoid
> using float may not make sense. Even UefiCpuPkg\Library\BaseUefiCpuLib\
> provides routine to initialize float support for X86.
> 
> Given ARM already uses NULL library class mechanism to supply the compiler
> stub, I prefer X86 aligns to the ARM style.
> 
> The only unsure thing is the size impact when a module is not using float. We
> expect there is no size impact when a module is not using float.
> 
> Thanks,
> Ray
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> Michael
> > D Kinney
> > Sent: Thursday, April 2, 2020 12:38 AM
> > To: devel@edk2.groups.io; ard.biesheuvel@linaro.org; Kinney, Michael D
> > <michael.d.kinney@intel.com>
> > Cc: lersek@redhat.com; macarl@microsoft.com
> > Subject: Re: [edk2-devel] [PATCH] CryptoPkg/FltUsedLib: Add FltUsedLib
> > for float.
> >
> > Hi Ard,
> >
> > I think adding a dependency in the module entry point libs is also
> > good way to guarantee an intrinsic lib is available.  I agree that it
> > can provide module type and arch specific mappings.  The NULL lib
> > instance can do the same if the NULL instance is listed in the
> > module/arch specific library classes sections. a few example section
> > names.
> >
> > [LibraryClasses.ARM.PEIM, LibraryClasses.AARCH64.PEIM]
> >
> > [LibraryClasses.IA32.UEFI_DRIVER]
> >
> > [LibraryClasses.X64.DXE_DRIVER]
> >
> > So either way, the DSC files need to provide the library mapping.  The
> > only difference between these
> > 2 approaches is that adding a dependency to the module entry point
> > libs uses a formally defined library class name for the intrinsic
> > functions vs.
> > the un-named NULL library instance.  A formally defined library class
> > name is better supported for things like unit tests from the
> > UnitTestFrameworkPkg.
> >
> > The fltused issue would not go away of we removed the float usage for
> > OpenSSL.  One or more other libs or the module could use float/double
> > types and this issue would pop up again.  I do agree it would be
> > cleaner if we could use OpenSSL without floating point.
> >
> > I think adding an intrinsic lib for IA32/X64 for VS20xx generation of
> > intrinsic functions would address fltused and other common C
> > implementation styles that generate intrinsic functions (e.g. 64-bit
> > int math on 32-bit, structure variable assignments, and loops that
> > fill a buffer with a const value) by VS20xx compilers.  An intrinsic
> > lib for GCC would also help with these same common C implementation
> > styles that generate intrinsic functions.
> >
> > Mike
> >
> > > -----Original Message-----
> > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ard
> > > Biesheuvel
> > > Sent: Tuesday, March 31, 2020 11:43 PM
> > > To: Kinney, Michael D <michael.d.kinney@intel.com>
> > > Cc: devel@edk2.groups.io; lersek@redhat.com; macarl@microsoft.com
> > > Subject: Re: [edk2-devel] [PATCH] CryptoPkg/FltUsedLib:
> > > Add FltUsedLib for float.
> > >
> > > On Tue, 31 Mar 2020 at 16:36, Kinney, Michael D
> > > <michael.d.kinney@intel.com> wrote:
> > > >
> > > > ARM and AARCH64 have a compiler intrinsic lib that is
> > > linked against all modules.
> > > >
> > > > [LibraryClasses.ARM, LibraryClasses.AARCH64]
> > > >   #
> > > >   # It is not possible to prevent ARM compiler calls to
> > > generic intrinsic functions.
> > > >   # This library provides the instrinsic functions
> > > generated by a given compiler.
> > > >   # [LibraryClasses.ARM] and NULL mean link this
> > > library into all ARM images.
> > > >   #
> > > >
> > > NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrins
> > > icsLib.inf
> > > >
> > > > Can we use this same technique for IA32/X64 VS builds?
> > > >
> > >
> > > In my opinion, having these intrinsics libraries added via NULL
> > > library class resolution was a mistake to begin with.
> > >
> > > Every component that we build incorporates some kind of startup code
> > > library that defines the _ModuleEntryPoint symbol, and it would be
> > > much better to make those libraries include an IntrinsicsLibrary
> > > dependency that can be satisfied by arch specific versions that also
> > > encapsulate the toolchain dependencies (such as this _fltused
> > > symbol, or memcpy/memset on ARM/GCC).
> > >
> > > On another note, it is still deeply disappointing that we need to
> > > jump through all of these hoops because the *only* single use of
> > > floating point in OpenSSL is the entropy estimate of an RNG, which
> > > is in the range of 0..1023 to begin with [IIRC). Remember that we
> > > also have a softfloat submodule for 32-bit ARM, for the same stupid
> > > reason. If we could stop pulling in that part of the code (or
> > > replace it with an
> > > UINT16 when building for the UEFI target), the whole floating point
> > > issue would mostly go away AFAICT.
> > >
> > >
> >
> >
> >
> 
> 
>