Re: [edk2-devel] [PATCH 0/3] update ArmSoftFloatLib to latest upstream version

["Laszlo Ersek" <lersek@redhat.com>]

Hi Ard,

On 05/24/19 17:11, Ard Biesheuvel wrote:
> Currently, our move to OpenSSL 1.1.1b is being blocked by an issue in
> the ARM software floating point library, which lacks some intrinsics
> that the ARM EABI spec defines.
>
> Since the code was in pretty sorry state, let's fix this by upgrading
> to the very latest version of the core library this code is based on,
> dated January 2018 (whereas the NetBSD fork of the old code dates back
> to 2002)

Thanks for this series!

I've fetched your branch noted below, and build-tested it with
ArmVirtQemu, ArmVirtQemuKernel, and ArmVirtXen. They all build fine.
And, AIUI, ArmSoftFloatLib is only needed for 32-bit ARM (not AArch64),
so I won't do other than build testing now.

Build-tested-by: Laszlo Ersek <lersek@redhat.com>

I'll make a number of comments below. I'm not requesting that *you* do
any of those, since you're already doing the community a favor, by
putting out this fire. I'll just list what I think should be done. If
there's agreement, I might take on a few of those.

(1) We should file a new TianoCore BZ (Feature Request) for this
ArmSoftFloatLib upgrade, and we should block TianoCore#1089 with that
new BZ.

(2) The new BZ should be referenced in all of the commit messages.

(3) The new BZ should be added to the release planning wiki page.

(4) In the longer term, we should investigate whether this (large)
library can be consumed as a git submodule. (Assuming that makes sense
-- if we don't expect another upgrade anytime soon, then this may not be
necessary.)

> A few notable issues that may require some discussion:
> - this code is made available under the 3-clause BSD license

That should be OK; "Readme.md" white-lists the 3-clause BSD License.

> - RVCT support is being dropped, since it is untested and nobody
>   appears to still care.

(5) I'm OK with that, but we should file a separate TianoCore BZ for
BaseTools, about RVCT removal.

> - no SPDX headers - this is left as an exercise for the steward.

(6) Right, I've noticed that -- separate BZ (dependent on the one from
(2)), or else it should be solved as the fourth patch in this series.

>
> Code can be found here:
> https://github.com/ardbiesheuvel/edk2/tree/bz_1089_upgrade_to_openssl_1_1_1b_v4
>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: "Gao, Liming" <liming.gao@intel.com>
> Cc: "Wang, Jian J" <jian.j.wang@intel.com>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>
> Ard Biesheuvel (3):
>   ArmPkg: import latest version (3e) of the Berkeley Softfloat library
>   ArmPkg/ArmSoftFloatLib: switch to new version of softfloat library

(7) This patch (patch#2) uses designated initializers (in the
initializers of the unions). I believe we never intend to build this
library with anything else than GCC, but I think the coding style still
requires us to avoid designated initializers.

>   ArmPkg/ArmSoftFloatLib: remove source files that are no longer used

OK, so here's a real scientific method that I used for determining
whether the result of this patch was "minimal". It relies on the
"strictatime" mount option -- I don't tolerate "relatime" or "noatime"
exactly because the POSIX atime behavior is so useful for debugging file
access.

So, a few minutes passed between my checking out your branch, and
starting the build tests. After the build tests above completed, I ran:

$ find ArmPkg/Library/ArmSoftFloatLib/ -type f -printf '%A+ %p\n' \
  | sort -r

which sorted the regular files in decreasing access time order (most
recent access near the top). The output suggests that the following
files are also not needed for the build(s) (with the
"ArmPkg/Library/ArmSoftFloatLib/SoftFloat-3e/" prefix stripped):

     1  COPYING.txt
     2  README.html
     3  README.txt
     4  build/Linux-386-GCC/Makefile
     5  build/Linux-386-GCC/platform.h
     6  build/Linux-386-SSE2-GCC/Makefile
     7  build/Linux-386-SSE2-GCC/platform.h
     8  build/Linux-ARM-VFPv2-GCC/Makefile
     9  build/Linux-x86_64-GCC/Makefile
    10  build/Linux-x86_64-GCC/platform.h
    11  build/Win32-MinGW/Makefile
    12  build/Win32-MinGW/platform.h
    13  build/Win32-SSE2-MinGW/Makefile
    14  build/Win32-SSE2-MinGW/platform.h
    15  build/Win64-MinGW-w64/Makefile
    16  build/Win64-MinGW-w64/platform.h
    17  build/template-FAST_INT64/Makefile
    18  build/template-FAST_INT64/platform.h
    19  build/template-not-FAST_INT64/Makefile
    20  build/template-not-FAST_INT64/platform.h
    21  doc/SoftFloat-history.html
    22  doc/SoftFloat-source.html
    23  doc/SoftFloat.html
    24  source/8086-SSE/extF80M_isSignalingNaN.c
    25  source/8086-SSE/f128M_isSignalingNaN.c
    26  source/8086-SSE/s_commonNaNToExtF80M.c
    27  source/8086-SSE/s_commonNaNToExtF80UI.c
    28  source/8086-SSE/s_commonNaNToF128M.c
    29  source/8086-SSE/s_commonNaNToF128UI.c
    30  source/8086-SSE/s_commonNaNToF16UI.c
    31  source/8086-SSE/s_commonNaNToF32UI.c
    32  source/8086-SSE/s_commonNaNToF64UI.c
    33  source/8086-SSE/s_extF80MToCommonNaN.c
    34  source/8086-SSE/s_extF80UIToCommonNaN.c
    35  source/8086-SSE/s_f128MToCommonNaN.c
    36  source/8086-SSE/s_f128UIToCommonNaN.c
    37  source/8086-SSE/s_f16UIToCommonNaN.c
    38  source/8086-SSE/s_f32UIToCommonNaN.c
    39  source/8086-SSE/s_f64UIToCommonNaN.c
    40  source/8086-SSE/s_propagateNaNExtF80M.c
    41  source/8086-SSE/s_propagateNaNExtF80UI.c
    42  source/8086-SSE/s_propagateNaNF128M.c
    43  source/8086-SSE/s_propagateNaNF128UI.c
    44  source/8086-SSE/s_propagateNaNF16UI.c
    45  source/8086-SSE/s_propagateNaNF32UI.c
    46  source/8086-SSE/s_propagateNaNF64UI.c
    47  source/8086-SSE/softfloat_raiseFlags.c
    48  source/8086-SSE/specialize.h
    49  source/8086/extF80M_isSignalingNaN.c
    50  source/8086/f128M_isSignalingNaN.c
    51  source/8086/s_commonNaNToExtF80M.c
    52  source/8086/s_commonNaNToExtF80UI.c
    53  source/8086/s_commonNaNToF128M.c
    54  source/8086/s_commonNaNToF128UI.c
    55  source/8086/s_commonNaNToF16UI.c
    56  source/8086/s_commonNaNToF32UI.c
    57  source/8086/s_commonNaNToF64UI.c
    58  source/8086/s_extF80MToCommonNaN.c
    59  source/8086/s_extF80UIToCommonNaN.c
    60  source/8086/s_f128MToCommonNaN.c
    61  source/8086/s_f128UIToCommonNaN.c
    62  source/8086/s_f16UIToCommonNaN.c
    63  source/8086/s_f32UIToCommonNaN.c
    64  source/8086/s_f64UIToCommonNaN.c
    65  source/8086/s_propagateNaNExtF80M.c
    66  source/8086/s_propagateNaNExtF80UI.c
    67  source/8086/s_propagateNaNF128M.c
    68  source/8086/s_propagateNaNF128UI.c
    69  source/8086/s_propagateNaNF16UI.c
    70  source/8086/s_propagateNaNF32UI.c
    71  source/8086/s_propagateNaNF64UI.c
    72  source/8086/softfloat_raiseFlags.c
    73  source/8086/specialize.h
    74  source/ARM-VFPv2-defaultNaN/extF80M_isSignalingNaN.c
    75  source/ARM-VFPv2-defaultNaN/f128M_isSignalingNaN.c
    76  source/ARM-VFPv2-defaultNaN/s_commonNaNToExtF80M.c
    77  source/ARM-VFPv2-defaultNaN/s_commonNaNToExtF80UI.c
    78  source/ARM-VFPv2-defaultNaN/s_commonNaNToF128M.c
    79  source/ARM-VFPv2-defaultNaN/s_commonNaNToF128UI.c
    80  source/ARM-VFPv2-defaultNaN/s_commonNaNToF16UI.c
    81  source/ARM-VFPv2-defaultNaN/s_commonNaNToF32UI.c
    82  source/ARM-VFPv2-defaultNaN/s_commonNaNToF64UI.c
    83  source/ARM-VFPv2-defaultNaN/s_extF80MToCommonNaN.c
    84  source/ARM-VFPv2-defaultNaN/s_extF80UIToCommonNaN.c
    85  source/ARM-VFPv2-defaultNaN/s_f128MToCommonNaN.c
    86  source/ARM-VFPv2-defaultNaN/s_f128UIToCommonNaN.c
    87  source/ARM-VFPv2-defaultNaN/s_f16UIToCommonNaN.c
    88  source/ARM-VFPv2-defaultNaN/s_f32UIToCommonNaN.c
    89  source/ARM-VFPv2-defaultNaN/s_f64UIToCommonNaN.c
    90  source/ARM-VFPv2-defaultNaN/s_propagateNaNExtF80M.c
    91  source/ARM-VFPv2-defaultNaN/s_propagateNaNExtF80UI.c
    92  source/ARM-VFPv2-defaultNaN/s_propagateNaNF128M.c
    93  source/ARM-VFPv2-defaultNaN/s_propagateNaNF128UI.c
    94  source/ARM-VFPv2-defaultNaN/s_propagateNaNF16UI.c
    95  source/ARM-VFPv2-defaultNaN/s_propagateNaNF32UI.c
    96  source/ARM-VFPv2-defaultNaN/s_propagateNaNF64UI.c
    97  source/ARM-VFPv2-defaultNaN/softfloat_raiseFlags.c
    98  source/ARM-VFPv2-defaultNaN/specialize.h
    99  source/ARM-VFPv2/extF80M_isSignalingNaN.c
   100  source/ARM-VFPv2/f128M_isSignalingNaN.c
   101  source/ARM-VFPv2/s_commonNaNToExtF80M.c
   102  source/ARM-VFPv2/s_commonNaNToExtF80UI.c
   103  source/ARM-VFPv2/s_commonNaNToF128M.c
   104  source/ARM-VFPv2/s_commonNaNToF128UI.c
   105  source/ARM-VFPv2/s_commonNaNToF16UI.c
   106  source/ARM-VFPv2/s_commonNaNToF32UI.c
   107  source/ARM-VFPv2/s_commonNaNToF64UI.c
   108  source/ARM-VFPv2/s_extF80MToCommonNaN.c
   109  source/ARM-VFPv2/s_extF80UIToCommonNaN.c
   110  source/ARM-VFPv2/s_f128MToCommonNaN.c
   111  source/ARM-VFPv2/s_f128UIToCommonNaN.c
   112  source/ARM-VFPv2/s_f16UIToCommonNaN.c
   113  source/ARM-VFPv2/s_f32UIToCommonNaN.c
   114  source/ARM-VFPv2/s_f64UIToCommonNaN.c
   115  source/ARM-VFPv2/s_propagateNaNExtF80M.c
   116  source/ARM-VFPv2/s_propagateNaNExtF80UI.c
   117  source/ARM-VFPv2/s_propagateNaNF128M.c
   118  source/ARM-VFPv2/s_propagateNaNF128UI.c
   119  source/ARM-VFPv2/s_propagateNaNF16UI.c
   120  source/ARM-VFPv2/s_propagateNaNF32UI.c

(8) Should we remove the "build/" (lines 4 through 20) and "source/"
(lines 24 through 120) subsets of this list, in patch #3? (Or maybe in a
totally separate patch?)

Thanks
Laszlo