Hi Ard, Thanks for these patches.
I did some function tests for it. Here is the results: --------------- Test 1: --------------- test uint32_to_f64 4026531839: Mem: 00 00 E0 FF FF FF ED 41 test uint32_to_f64 4294967295: Mem: 00 00 E0 FF FF FF EF 41 ------------ Test 2: --------------- Test f64_to_uint32 5294967295.1: 4294967295 Test f64_to_uint32 4294967295.1: 4294967295 Test f64_to_uint32 4294967294.8: 4294967294 Test f64_to_uint32 4294967295.2: 4294967295 Test f64_to_uint32 4294967295.8: 4294967295 Test f64_to_uint32 -40.0: 0 Test f64_to_uint32 0.0: 0 Test f64_to_uint32 0.8: 0 Test f64_to_uint32 -0.1: 0 looks fine for me. I also did CryptoPkg tests with OpenSSL_1_1_1b, it works too. Tested-by: Xiaoyu Lu <[email protected]> > -----Original Message----- > From: [email protected] [mailto:[email protected]] On Behalf Of > Ard Biesheuvel > Sent: Saturday, May 25, 2019 5:33 AM > To: Laszlo Ersek <[email protected]> > Cc: edk2-devel-groups-io <[email protected]>; Gao, Liming > <[email protected]>; Wang, Jian J <[email protected]>; Leif Lindholm > <[email protected]>; Kinney, Michael D <[email protected]> > Subject: Re: [edk2-devel] [PATCH 0/3] update ArmSoftFloatLib to latest > upstream version > > On Fri, 24 May 2019 at 22:57, Laszlo Ersek <[email protected]> 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 <[email protected]> > > > > 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 <[email protected]> > > > Cc: "Gao, Liming" <[email protected]> > > > Cc: "Wang, Jian J" <[email protected]> > > > Cc: Leif Lindholm <[email protected]> > > > Cc: Michael D Kinney <[email protected]> > > > > > > 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 (#41416): https://edk2.groups.io/g/devel/message/41416 Mute This Topic: https://groups.io/mt/31745496/21656 Group Owner: [email protected] Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
