[edk2-devel] [PATCH 0/3] update ArmSoftFloatLib to latest upstream version
Ard Biesheuvel
ard.biesheuvel at linaro.org
Fri May 24 21:32:39 UTC 2019
On Fri, 24 May 2019 at 22:57, Laszlo Ersek <lersek at redhat.com> wrote:
>
> 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 at 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.
>
Fair enough.
> (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.)
>
This version of ArmSoftFloatLib implements all __aeabi routines that
are listed in the spec. Only a few of those are referenced by OpenSSL,
and in practice this code never gets exercised. So unless we grow
another user of this library, I have no intention of doing lots of
maintenance work on this library and (in response to your point below)
this is the reason I simply imported the whole library - to make
future upgrades, in case they do occur, as painless and
straightforward as possible. So I think a git submodule is overkill,
especially given the fact that there does not seem to be an
authoritative git upstream for this library.
> > 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.
>
https://bugzilla.tianocore.org/show_bug.cgi?id=1750
> > - 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.
>
Sure. The only downside to that is that it increases the delta with
the upstream library, so let's hope that this rebases cleanly if we do
end up upgrading.
> >
> > Code can be found here:
> > https://github.com/ardbiesheuvel/edk2/tree/bz_1089_upgrade_to_openssl_1_1_1b_v4
> >
> > Cc: Laszlo Ersek <lersek at redhat.com>
> > Cc: "Gao, Liming" <liming.gao at intel.com>
> > Cc: "Wang, Jian J" <jian.j.wang at intel.com>
> > Cc: Leif Lindholm <leif.lindholm at linaro.org>
> > Cc: Michael D Kinney <michael.d.kinney at 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.
>
I can change that.
> > 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?)
>
I'll let Leif chime in here. I'd be fine with removing them, not
adding them in the first place or leaving them where they are.
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#41369): https://edk2.groups.io/g/devel/message/41369
Mute This Topic: https://groups.io/mt/31745496/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list