ArmPkg/Library/CompilerIntrinsicsLib/Arm/rtdiv.asm | 255 ++++++++++++++++++++ ArmPkg/Library/CompilerIntrinsicsLib/Arm/rtsrsh.asm | 45 ++++ ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf | 13 +- ArmPkg/Library/CompilerIntrinsicsLib/memcpy_ms.c | 34 +++ ArmPkg/Library/CompilerIntrinsicsLib/memset_ms.c | 33 +++ BaseTools/Conf/build_rule.template | 31 ++- BaseTools/Conf/tools_def.template | 28 +++ MdePkg/Include/Arm/ProcessorBind.h | 96 ++++++-- MdePkg/Include/Base.h | 13 + MdePkg/Library/BaseLib/Arm/CpuBreakpoint.asm | 5 +- MdePkg/Library/BaseLib/BaseLib.inf | 16 +- MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf | 5 +- MdePkg/Library/BaseStackCheckLib/BaseStackCheckNull.c | 18 ++ 13 files changed, 562 insertions(+), 30 deletions(-) create mode 100644 ArmPkg/Library/CompilerIntrinsicsLib/Arm/rtdiv.asm create mode 100644 ArmPkg/Library/CompilerIntrinsicsLib/Arm/rtsrsh.asm create mode 100644 ArmPkg/Library/CompilerIntrinsicsLib/memcpy_ms.c create mode 100644 ArmPkg/Library/CompilerIntrinsicsLib/memset_ms.c create mode 100644 MdePkg/Library/BaseStackCheckLib/BaseStackCheckNull.c
This is a v2 of the previous patch, that takes into account the alignment of suppressed level 4 warnings between IA32, X64 and ARM, and that also removes compiler options that weren't actually needed. The following series adds ARM compilation support for the VS2017 toolchain. * PATCH 1 targets the disabling of VS Level 4 warnings. The disabled warnings for ARM are now aligned with IA32 and X64. * PATCH 2 adds a NULL handler for the base stack check, since this is a GCC functionality. * PATCH 3 updates MdePkg/Library/BaseLib so that the RVCT assembly sources are also used for MSFT. * PATCH 4 adds the required compiler intrinsics replacements for division, shift and memset/memcpy. * PATCH 5 adds variable argument handlers for print output. Note that this is done without relying on any external headers, with the VA_ARG macro having been reverse engineered from MSFT ARM assembly output. * PATCH 6 enables the selection of ARM in the conf templates. With these patches, VS2017 toolchain users should be able to compile regular UEFI ARM applications using EDK2. Note that, unlike ARM64 support, ARM support does not require a specific update of Visual Studio 2017, as the ARM toolchain has been available from the very first release. Additional notes: We tested compiling and running the full UEFI Shell with this series, as well as a small set of applications and drivers, and found no issues. With an additional patch [1], it is also possible to use this proposal to compile a complete QEMU ARM firmware. As the patch shows, the changes that need to be applied to the EDK2 sources to achieve this are actually very minimal. However, the generated firmware does not currently boot, possibly because of the following warnings being generated by the MS compiler: - ArmCpuDxe.dll : warning LNK4072: section count 118 exceeds max (96); image may not run - UiApp.dll : warning LNK4072: section count 113 exceeds max (96); image may not run As far as I could see, the section count max is hardcoded so a workaround would be needed to address those. Also, because the VS2017 ARM compiler forces a section alignment of 4096 bytes (which in turn forces use to use /FILEALIGN:4096 as a linker option for the firmware generation), the generated firmware exceeds 2MB and we had to double its size to 4MB. At this stage, since the goal of this series is to allow users to compile regular ARM UEFI applications using the VS2017 toolchain, I have non plans to spend more time on the QEMU firmware issues, especially as I suspect that reducing the firmware size back to 2 MB may not be achievable without Microsoft altering their compiler. I am however hopeful that ARM specialists can take this matter over eventually... Regards, /Pete [1] https://github.com/pbatard/edk2/commit/c4ce41094a46f4f3dc7ccc64a90604813f037b13 Pete Batard (6): MdePkg: Disable some Level 4 warnings for VS2017/ARM MdePkg/Library/BaseStackCheckLib: Add Null handler for VS2017/ARM MdePkg/Library/BaseLib: Enable VS2017/ARM builds ArmPkg/Library/CompilerIntrinsicsLib: Enable VS2017/ARM builds MdePkg/Include: Add VA list support for VS2017/ARM BaseTools/Conf: Add VS2017/ARM support ArmPkg/Library/CompilerIntrinsicsLib/Arm/rtdiv.asm | 255 ++++++++++++++++++++ ArmPkg/Library/CompilerIntrinsicsLib/Arm/rtsrsh.asm | 45 ++++ ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf | 13 +- ArmPkg/Library/CompilerIntrinsicsLib/memcpy_ms.c | 34 +++ ArmPkg/Library/CompilerIntrinsicsLib/memset_ms.c | 33 +++ BaseTools/Conf/build_rule.template | 31 ++- BaseTools/Conf/tools_def.template | 28 +++ MdePkg/Include/Arm/ProcessorBind.h | 96 ++++++-- MdePkg/Include/Base.h | 13 + MdePkg/Library/BaseLib/Arm/CpuBreakpoint.asm | 5 +- MdePkg/Library/BaseLib/BaseLib.inf | 16 +- MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf | 5 +- MdePkg/Library/BaseStackCheckLib/BaseStackCheckNull.c | 18 ++ 13 files changed, 562 insertions(+), 30 deletions(-) create mode 100644 ArmPkg/Library/CompilerIntrinsicsLib/Arm/rtdiv.asm create mode 100644 ArmPkg/Library/CompilerIntrinsicsLib/Arm/rtsrsh.asm create mode 100644 ArmPkg/Library/CompilerIntrinsicsLib/memcpy_ms.c create mode 100644 ArmPkg/Library/CompilerIntrinsicsLib/memset_ms.c create mode 100644 MdePkg/Library/BaseStackCheckLib/BaseStackCheckNull.c -- 2.9.3.windows.2 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Pete: This is the base step to enable VS2017 ARM tool chain. Then, user could use it and build ARM application first. Here, I have one question. VS2017 ARM requires 4096 alignment. So, why not add /FILEALIGN:4096 into tools_def.txt as the default linker option for ARM? Thanks Liming >-----Original Message----- >From: Pete Batard [mailto:pete@akeo.ie] >Sent: Wednesday, December 06, 2017 7:15 PM >To: edk2-devel@lists.01.org >Cc: Gao, Liming <liming.gao@intel.com> >Subject: [PATCH v2 0/6] Add ARM support for VS2017 > >This is a v2 of the previous patch, that takes into account the alignment >of suppressed level 4 warnings between IA32, X64 and ARM, and that also >removes compiler options that weren't actually needed. > >The following series adds ARM compilation support for the VS2017 toolchain. >* PATCH 1 targets the disabling of VS Level 4 warnings. The disabled warnings > for ARM are now aligned with IA32 and X64. >* PATCH 2 adds a NULL handler for the base stack check, since this is a GCC > functionality. >* PATCH 3 updates MdePkg/Library/BaseLib so that the RVCT assembly >sources > are also used for MSFT. >* PATCH 4 adds the required compiler intrinsics replacements for division, > shift and memset/memcpy. >* PATCH 5 adds variable argument handlers for print output. Note that this > is done without relying on any external headers, with the VA_ARG macro > having been reverse engineered from MSFT ARM assembly output. >* PATCH 6 enables the selection of ARM in the conf templates. > >With these patches, VS2017 toolchain users should be able to compile >regular UEFI ARM applications using EDK2. Note that, unlike ARM64 support, >ARM support does not require a specific update of Visual Studio 2017, as >the ARM toolchain has been available from the very first release. > >Additional notes: > >We tested compiling and running the full UEFI Shell with this series, as >well as a small set of applications and drivers, and found no issues. >With an additional patch [1], it is also possible to use this proposal to >compile a complete QEMU ARM firmware. As the patch shows, the changes >that >need to be applied to the EDK2 sources to achieve this are actually very >minimal. > >However, the generated firmware does not currently boot, possibly because >of the following warnings being generated by the MS compiler: >- ArmCpuDxe.dll : warning LNK4072: section count 118 exceeds max (96); >image may not run >- UiApp.dll : warning LNK4072: section count 113 exceeds max (96); image may >not run > >As far as I could see, the section count max is hardcoded so a workaround >would be needed to address those. > >Also, because the VS2017 ARM compiler forces a section alignment of 4096 >bytes (which in turn forces use to use /FILEALIGN:4096 as a linker option >for the firmware generation), the generated firmware exceeds 2MB and we >had to double its size to 4MB. > >At this stage, since the goal of this series is to allow users to compile >regular ARM UEFI applications using the VS2017 toolchain, I have non plans >to spend more time on the QEMU firmware issues, especially as I suspect >that reducing the firmware size back to 2 MB may not be achievable without >Microsoft altering their compiler. I am however hopeful that ARM >specialists can take this matter over eventually... > >Regards, > >/Pete > >[1] >https://github.com/pbatard/edk2/commit/c4ce41094a46f4f3dc7ccc64a906048 >13f037b13 > > >Pete Batard (6): > MdePkg: Disable some Level 4 warnings for VS2017/ARM > MdePkg/Library/BaseStackCheckLib: Add Null handler for VS2017/ARM > MdePkg/Library/BaseLib: Enable VS2017/ARM builds > ArmPkg/Library/CompilerIntrinsicsLib: Enable VS2017/ARM builds > MdePkg/Include: Add VA list support for VS2017/ARM > BaseTools/Conf: Add VS2017/ARM support > > ArmPkg/Library/CompilerIntrinsicsLib/Arm/rtdiv.asm | 255 >++++++++++++++++++++ > ArmPkg/Library/CompilerIntrinsicsLib/Arm/rtsrsh.asm | 45 ++++ > ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf | 13 +- > ArmPkg/Library/CompilerIntrinsicsLib/memcpy_ms.c | 34 +++ > ArmPkg/Library/CompilerIntrinsicsLib/memset_ms.c | 33 +++ > BaseTools/Conf/build_rule.template | 31 ++- > BaseTools/Conf/tools_def.template | 28 +++ > MdePkg/Include/Arm/ProcessorBind.h | 96 ++++++-- > MdePkg/Include/Base.h | 13 + > MdePkg/Library/BaseLib/Arm/CpuBreakpoint.asm | 5 +- > MdePkg/Library/BaseLib/BaseLib.inf | 16 +- > MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf | 5 +- > MdePkg/Library/BaseStackCheckLib/BaseStackCheckNull.c | 18 ++ > 13 files changed, 562 insertions(+), 30 deletions(-) > create mode 100644 ArmPkg/Library/CompilerIntrinsicsLib/Arm/rtdiv.asm > create mode 100644 ArmPkg/Library/CompilerIntrinsicsLib/Arm/rtsrsh.asm > create mode 100644 ArmPkg/Library/CompilerIntrinsicsLib/memcpy_ms.c > create mode 100644 ArmPkg/Library/CompilerIntrinsicsLib/memset_ms.c > create mode 100644 >MdePkg/Library/BaseStackCheckLib/BaseStackCheckNull.c > >-- >2.9.3.windows.2 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Hi Liming, On 2017.12.07 03:02, Gao, Liming wrote: > This is the base step to enable VS2017 ARM tool chain. Then, user could use it and build ARM application first. Yes. > Here, I have one question. VS2017 ARM requires 4096 alignment. So, why not add /FILEALIGN:4096 into tools_def.txt as the default linker option for ARM? I don't think we want to do that on account that this option does not need to be specified when building regular applications, and is currently only needed when building the QEMU firmware. So it's probably better to only specify it in the context where it is needed, rather than globally, especially as the documentation for /FILEALIGN [1] indicates that "By default, the linker does not use a fixed alignment size" which would tend to indicate that we might be better off letting the compiler decide what it should use on its own. Especially, forcing /FILEALIGN to 4K everywhere does appear to increase the size of the generated binaries. For instance, I'm seeing a RELEASE Shell.efi with a size of 762 KB without /FILEALIGN, vs. 792 KB with /FILEALIGN:4096. Thus, I think we should be conservative with regards to the global compiler options we add: if an option only seems to be needed for a single module (and especially, if that module has not yet been patched and reviewed for compilation), it should probably be specified for that module alone. As such, unless you or other people on this list have a strong opinion that /FILEALIGN:4096 should be global, I am currently not planning to resubmit a new patch that adds that option. Please let me know if you still think the patch should be altered. Regards, /Pete [1] https://docs.microsoft.com/en-gb/cpp/build/reference/filealign _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Pete: I get your point. I suggest to add it in VS2017 tool chain comment. To build XIP firmware image, /FILEALIGN:4096 are required. Now, SEC, PEI_CORE and PEIM module require it. Thanks Liming >-----Original Message----- >From: Pete Batard [mailto:pete@akeo.ie] >Sent: Thursday, December 07, 2017 11:56 PM >To: edk2-devel@lists.01.org; Gao, Liming <liming.gao@intel.com> >Subject: Re: [PATCH v2 0/6] Add ARM support for VS2017 > >Hi Liming, > >On 2017.12.07 03:02, Gao, Liming wrote: >> This is the base step to enable VS2017 ARM tool chain. Then, user could >use it and build ARM application first. > >Yes. > >> Here, I have one question. VS2017 ARM requires 4096 alignment. So, why >not add /FILEALIGN:4096 into tools_def.txt as the default linker option for >ARM? > >I don't think we want to do that on account that this option does not >need to be specified when building regular applications, and is >currently only needed when building the QEMU firmware. > >So it's probably better to only specify it in the context where it is >needed, rather than globally, especially as the documentation for >/FILEALIGN [1] indicates that "By default, the linker does not use a >fixed alignment size" which would tend to indicate that we might be >better off letting the compiler decide what it should use on its own. > >Especially, forcing /FILEALIGN to 4K everywhere does appear to increase >the size of the generated binaries. For instance, I'm seeing a RELEASE >Shell.efi with a size of 762 KB without /FILEALIGN, vs. 792 KB with >/FILEALIGN:4096. > >Thus, I think we should be conservative with regards to the global >compiler options we add: if an option only seems to be needed for a >single module (and especially, if that module has not yet been patched >and reviewed for compilation), it should probably be specified for that >module alone. > >As such, unless you or other people on this list have a strong opinion >that /FILEALIGN:4096 should be global, I am currently not planning to >resubmit a new patch that adds that option. > >Please let me know if you still think the patch should be altered. > >Regards, > >/Pete > >[1] https://docs.microsoft.com/en-gb/cpp/build/reference/filealign _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
© 2016 - 2024 Red Hat, Inc.