[edk2-devel] [PATCH 0/3] update ArmSoftFloatLib to latest upstream version
Xiaoyu Lu
xiaoyux.lu at intel.com
Mon May 27 09:37:25 UTC 2019
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 <xiaoyux.lu at intel.com>
> -----Original Message-----
> From: devel at edk2.groups.io [mailto:devel at edk2.groups.io] On Behalf Of
> Ard Biesheuvel
> Sent: Saturday, May 25, 2019 5:33 AM
> To: Laszlo Ersek <lersek at redhat.com>
> Cc: edk2-devel-groups-io <devel at edk2.groups.io>; Gao, Liming
> <liming.gao at intel.com>; Wang, Jian J <jian.j.wang at intel.com>; Leif Lindholm
> <leif.lindholm at linaro.org>; Kinney, Michael D <michael.d.kinney at intel.com>
> Subject: Re: [edk2-devel] [PATCH 0/3] update ArmSoftFloatLib to latest
> upstream version
>
> 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 (#41416): https://edk2.groups.io/g/devel/message/41416
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