Re: [edk2-devel] [edk2-staging/RISC-V-V2 PATCH v2 24/29] BaseTools: BaseTools changes for RISC-V platform.

["Leif Lindholm" <leif.lindholm@linaro.org>]

On Mon, Sep 23, 2019 at 08:31:50AM +0800, Abner Chang wrote:
> BaseTools changes for building EDK2 RISC-V platform.
> The changes made to build_rule.template is to avoid build errors
> cause by GCC711RISCV tool chain.

Thank you, this is much cleaner.
There are however some issues in this patch that prevent building on
any platform. Please ensure to give a local build test before
submitting a 3.

First of all, this still does not contain the addition to
BaseTools/Source/Python/Common/buildoptions.py that I mentioned in
https://edk2.groups.io/g/devel/message/47036 - meaning that attempting
to build anything for RISCV64 gives an error.

Other minor issues reviewed inline:

> Signed-off-by: Abner Chang <abner.chang@hpe.com>
> ---
>  BaseTools/Conf/build_rule.template                 |  62 ++---
>  BaseTools/Conf/tools_def.template                  |  64 ++++-
>  BaseTools/Source/C/Common/BasePeCoff.c             |  15 +-
>  BaseTools/Source/C/Common/PeCoffLoaderEx.c         |  95 ++++++++
>  BaseTools/Source/C/GenFv/GenFvInternalLib.c        | 128 +++++++++-
>  BaseTools/Source/C/GenFw/Elf32Convert.c            |   5 +-
>  BaseTools/Source/C/GenFw/Elf64Convert.c            | 260 ++++++++++++++++++++-
>  BaseTools/Source/C/GenFw/elf_common.h              |  62 +++++
>  .../Source/C/Include/IndustryStandard/PeImage.h    |   6 +
>  BaseTools/Source/Python/Common/DataType.py         |   7 +-
>  10 files changed, 659 insertions(+), 45 deletions(-)
> 
> diff --git a/BaseTools/Conf/build_rule.template b/BaseTools/Conf/build_rule.template
> index db06d3a..fab3926 100755
> --- a/BaseTools/Conf/build_rule.template
> +++ b/BaseTools/Conf/build_rule.template
> @@ -1,6 +1,7 @@
>  #
>  #  Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.<BR>
>  #  Portions copyright (c) 2008 - 2010, Apple Inc. All rights reserved.<BR>
> +#  Portions Copyright (c) 2019, Hewlett Packard Enterprise Development LP. All rights reserved.<BR>
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
>  
> @@ -145,14 +146,6 @@
>      <Command.GCC, Command.RVCT>
>          "$(CC)" $(CC_FLAGS) $(CC_XIPFLAGS) -c -o ${dst} $(INC) ${src}
>  
> -[C-Header-File]
> -    <InputFile>
> -        *.h, *.H
> -
> -    <OutputFile>
> -
> -    <Command>
> -

Header files are good, please don't delete them.

>  [Assembly-Code-File.COMMON.COMMON]
>      <InputFile.MSFT, InputFile.INTEL, InputFile.RVCT>
>          ?.asm, ?.Asm, ?.ASM
> @@ -266,10 +259,10 @@
>      <Command.GCC>
>          $(RM) ${dst}
>          "$(SLINK)" cr ${dst} $(SLINK_FLAGS) @$(OBJECT_FILES_LIST)
> -    
> +

Please address these line ending issues..

>      <Command.RVCT>
>          "$(SLINK)" $(SLINK_FLAGS) ${dst} --via $(OBJECT_FILES_LIST)
> -    
> +
>      <Command.RVCTCYGWIN>
>          # $(OBJECT_FILES_LIST) has wrong paths for cygwin
>          "$(SLINK)" $(SLINK_FLAGS) ${dst} $(OBJECT_FILES)
> @@ -304,8 +297,8 @@
>  
>      <Command.XCODE>
>          "$(DLINK)" $(DLINK_FLAGS) -o ${dst} $(DLINK_SPATH) -filelist $(STATIC_LIBRARY_FILES_LIST)  $(DLINK2_FLAGS)
> -    
> -    
> +
> +
>  [Static-Library-File.SEC.AARCH64, Static-Library-File.PEI_CORE.AARCH64, Static-Library-File.PEIM.AARCH64,Static-Library-File.SEC.ARM, Static-Library-File.PEI_CORE.ARM, Static-Library-File.PEIM.ARM]
>      <InputFile>
>          *.lib
> @@ -321,6 +314,21 @@
>          "$(OBJCOPY)" $(OBJCOPY_FLAGS) ${dst}
>  
>  
> +[Static-Library-File.COMMON.RISCV64, Static-Library-File.COMMON.RISCV32]
> +    <InputFile>
> +        *.lib
> +
> +    <ExtraDependency>
> +        $(MAKE_FILE)
> +
> +    <OutputFile>
> +        $(DEBUG_DIR)(+)$(MODULE_NAME).dll
> +
> +    <Command.GCC>
> +        "$(DLINK)" -o ${dst} $(DLINK_FLAGS) --start-group $(DLINK_SPATH) @$(STATIC_LIBRARY_FILES_LIST) --end-group $(DLINK2_FLAGS)

This line looks to me like the only thing that is actually changed
here, and I am not convinced it is necessary.
          "$(DLINK)" -o ${dst} $(DLINK_FLAGS) -Wl,--start-group,@$(STATIC_LIBRARY_FILES_LIST),--end-group $(CC_FLAGS) $(DLINK2_FLAGS)

On the ARM/AARCH64 side, we use gcc as the DLINK, and pass the
required flags through to the linker with -Wl. Please have a look and
try to rework at that end rather than fundamentally revamping the
basic build rules differently for RISCV than other architectures.

Basically, please discard all changes to this file, apply the below
diff, and rework the flags to resolve the builds. (Basically, add a
bunch of -Wl,)

diff --git a/BaseTools/Conf/tools_def.template b/BaseTools/Conf/tools_def.template
index b96b394dc441..b6d5c25ba5b5 100755
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -2286,11 +2286,10 @@ RELEASE_GCC5_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20
 ##################
 # GCC5 RISCV64 definitions
 ##################
-*_GCC5_RISCV64_OBJCOPY_PATH         = ENV(GCC5_RISCV64_PREFIX)objcopy
 *_GCC5_RISCV64_CC_PATH              = ENV(GCC5_RISCV64_PREFIX)gcc
-*_GCC5_RISCV64_SLINK_PATH           = ENV(GCC5_RISCV64_PREFIX)gcc-ar
-*_GCC5_RISCV64_DLINK_PATH           = ENV(GCC5_RISCV64_PREFIX)ld
-*_GCC5_RISCV64_ASLDLINK_PATH        = ENV(GCC5_RISCV64_PREFIX)ld
+*_GCC5_RISCV64_SLINK_PATH           = ENV(GCC5_RISCV64_PREFIX)ar
+*_GCC5_RISCV64_DLINK_PATH           = ENV(GCC5_RISCV64_PREFIX)gcc
+*_GCC5_RISCV64_ASLDLINK_PATH        = ENV(GCC5_RISCV64_PREFIX)gcc
 *_GCC5_RISCV64_ASM_PATH             = ENV(GCC5_RISCV64_PREFIX)gcc
 *_GCC5_RISCV64_PP_PATH              = ENV(GCC5_RISCV64_PREFIX)gcc
 *_GCC5_RISCV64_VFRPP_PATH           = ENV(GCC5_RISCV64_PREFIX)gcc


> +        "$(OBJCOPY)" $(OBJCOPY_FLAGS) ${dst}
> +
> +
>  [Static-Library-File.USER_DEFINED, Static-Library-File.HOST_APPLICATION]
>      <InputFile>
>          *.lib
> @@ -346,8 +354,8 @@
>  
>      <Command.XCODE>
>          "$(DLINK)" -o ${dst} $(DLINK_FLAGS)  $(DLINK_SPATH) -filelist $(STATIC_LIBRARY_FILES_LIST)  $(DLINK2_FLAGS)
> -      
> -      
> +
> +
>  [Dynamic-Library-File]
>      <InputFile>
>          ?.dll
> @@ -360,7 +368,7 @@
>          $(CP) ${dst} $(DEBUG_DIR)
>          $(CP) ${dst} $(BIN_DIR)(+)$(MODULE_NAME_GUID).efi
>          -$(CP) $(DEBUG_DIR)(+)*.map $(OUTPUT_DIR)
> -        -$(CP) $(DEBUG_DIR)(+)*.pdb $(OUTPUT_DIR) 
> +        -$(CP) $(DEBUG_DIR)(+)*.pdb $(OUTPUT_DIR)
>      <Command.GCC>
>          $(CP) ${src} $(DEBUG_DIR)(+)$(MODULE_NAME).debug
>          $(OBJCOPY) --strip-unneeded -R .eh_frame ${src}
> @@ -375,7 +383,7 @@
>          $(CP) ${dst} $(DEBUG_DIR)
>          $(CP) ${dst} $(BIN_DIR)(+)$(MODULE_NAME_GUID).efi
>          -$(CP) $(DEBUG_DIR)(+)*.map $(OUTPUT_DIR)
> -        
> +
>      <Command.XCODE>
>          # tool to convert Mach-O to PE/COFF
>          "$(MTOC)" -subsystem $(MODULE_TYPE)  $(MTOC_FLAGS)  ${src}  $(DEBUG_DIR)(+)$(MODULE_NAME).pecoff
> @@ -414,13 +422,13 @@
>      <Command.MSFT, Command.INTEL>
>          Trim --asl-file -o $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.i -i $(INC_LIST) ${src}
>          "$(ASLPP)" $(ASLPP_FLAGS) $(INC) /I${s_path} $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.i > $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.iii
> -        Trim --source-code -l -o $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.iiii $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.iii 
> +        Trim --source-code -l -o $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.iiii $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.iii
>          "$(ASL)" $(ASL_FLAGS) $(ASL_OUTFLAGS)${dst} $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.iiii
>  
>      <Command.GCC>
>          Trim --asl-file -o $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.i -i $(INC_LIST) ${src}
>          "$(ASLPP)" $(ASLPP_FLAGS) $(INC) -I${s_path} $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.i > $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.iii
> -        Trim --source-code -l -o $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.iiii $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.iii 
> +        Trim --source-code -l -o $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.iiii $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.iii
>          "$(ASL)" $(ASL_FLAGS) $(ASL_OUTFLAGS)${dst} $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.iiii
>  
>  [C-Code-File.AcpiTable]
> @@ -462,14 +470,14 @@
>          "$(ASLCC)" -c -o $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.obj $(CC_FLAGS) $(ASLCC_FLAGS) $(INC) ${src}
>          "$(ASLDLINK)" -o $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.dll $(ASLDLINK_FLAGS) $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.obj
>          "$(GENFW)" -o ${dst} -c $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.dll $(GENFW_FLAGS)
> -        
> -    <Command.XCODE>        
> +
> +    <Command.XCODE>
>          "$(ASLCC)" -o $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.obj  $(ASLCC_FLAGS) $(INC) ${src}
>          "$(ASLDLINK)" -o $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.dll $(ASLDLINK_FLAGS) $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.obj
>          "$(MTOC)" -subsystem $(MODULE_TYPE)  $(MTOC_FLAGS) $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.dll $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.pecoff
>          "$(GENFW)" -o ${dst} -c $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.pecoff $(GENFW_FLAGS)
> -      
> -      
> +
> +
>  [Masm16-Code-File]
>      <InputFile>
>          ?.asm16, ?.Asm16, ?.ASM16, ?.s16, ?.S16
> @@ -492,14 +500,14 @@
>        Trim --source-code -o ${d_path}(+)${s_base}.iii ${d_path}(+)${s_base}.i
>        "$(ASM)" -o $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.obj $(ASM_FLAGS) $(INC) ${d_path}(+)${s_base}.iii
>        "$(DLINK)" -o ${dst} $(DLINK_FLAGS) --start-group $(DLINK_SPATH) $(LIBS) $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.obj --end-group
> -     
> +
>      <Command.XCODE>
>        "$(PP)" $(PP_FLAGS) $(INC) ${src} > ${d_path}(+)${s_base}.i
>        Trim --source-code -o ${d_path}(+)${s_base}.iii ${d_path}(+)${s_base}.i
>        "$(ASM)" -o $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.obj $(ASM_FLAGS) $(INC) ${d_path}(+)${s_base}.iii
>        "$(SLINK)" $(SLINK_FLAGS) $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.slib $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.obj
>        otool -t $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.slib | hex2bin.py ${dst}
> -      
> +
>  
>  [Nasm-to-Binary-Code-File]
>      <InputFile>
> @@ -635,8 +643,8 @@
>      <Command.GCC>
>          "$(GENFW)" -o $(OUTPUT_DIR)(+)$(MODULE_NAME)hii.rc -g $(MODULE_GUID) --hiibinpackage $(HII_BINARY_PACKAGES) $(GENFW_FLAGS)
>          "$(RC)" $(RC_FLAGS) $(OUTPUT_DIR)(+)$(MODULE_NAME)hii.rc ${dst}
> -        
> +
>      <Command.XCODE, Command.RVCT>
>          GenFw -o $(OUTPUT_DIR)(+)$(MODULE_NAME)hii.rc -g $(MODULE_GUID) --hiibinpackage $(HII_BINARY_PACKAGES)
> -        
> -        
> +
> +
> diff --git a/BaseTools/Conf/tools_def.template b/BaseTools/Conf/tools_def.template
> index 8f0e6cb..54c3dc5 100755
> --- a/BaseTools/Conf/tools_def.template
> +++ b/BaseTools/Conf/tools_def.template
> @@ -3,7 +3,7 @@
>  #  Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR>
>  #  Portions copyright (c) 2011 - 2014, ARM Ltd. All rights reserved.<BR>
>  #  Copyright (c) 2015, Hewlett-Packard Development Company, L.P.<BR>
> -#  (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
> +#  (C) Copyright 2016-2019 Hewlett Packard Enterprise Development LP<BR>
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> @@ -231,11 +231,12 @@ DEFINE DTC_BIN                 = ENV(DTC_PREFIX)dtc
>  #                               Intel(r) ACPI Compiler from
>  #                               https://acpica.org/downloads
>  #   GCC5        -Linux,Windows-  Requires:
> -#                             GCC 5 with LTO support, targeting x86_64-linux-gnu, aarch64-linux-gnu, or arm-linux-gnueabi
> +#                             GCC 5 with LTO support, targeting x86_64-linux-gnu, aarch64-linux-gnu, arm-linux-gnueabi or riscv64-linux-gnu
>  #                        Optional:
>  #                             Required to build platforms or ACPI tables:
>  #                               Intel(r) ACPI Compiler from
>  #                               https://acpica.org/downloads
> +#
>  #   CLANG35     -Linux,Windows-  Requires:
>  #                             Clang v3.5 or later, and GNU binutils targeting aarch64-linux-gnu or arm-linux-gnueabi
>  #                        Optional:
> @@ -1735,6 +1736,7 @@ DEFINE GCC_IA32_RC_FLAGS           = -I binary -O elf32-i386          -B i386
>  DEFINE GCC_X64_RC_FLAGS            = -I binary -O elf64-x86-64        -B i386    --rename-section .data=.hii
>  DEFINE GCC_ARM_RC_FLAGS            = -I binary -O elf32-littlearm     -B arm     --rename-section .data=.hii
>  DEFINE GCC_AARCH64_RC_FLAGS        = -I binary -O elf64-littleaarch64 -B aarch64 --rename-section .data=.hii
> +DEFINE GCC_RISCV64_RC_FLAGS        = -I binary -O elf64-littleriscv -  B riscv64 --rename-section .data=.hii

The above line requires two changes in order to work. The first one is
the two spaces between the - and the B. The second is that "riscv64"
is not accepted by the toolchain. "riscv" works.

>  
>  DEFINE GCC48_ALL_CC_FLAGS            = -g -fshort-wchar -fno-builtin -fno-strict-aliasing -Wall -Werror -Wno-array-bounds -ffunction-sections -fdata-sections -include AutoGen.h -fno-common -DSTRING_ARRAY_NAME=$(BASE_NAME)Strings
>  DEFINE GCC48_IA32_X64_DLINK_COMMON   = -nostdlib -Wl,-n,-q,--gc-sections -z common-page-size=0x20
> @@ -1806,6 +1808,21 @@ DEFINE GCC5_ARM_ASLDLINK_FLAGS       = DEF(GCC49_ARM_ASLDLINK_FLAGS)
>  DEFINE GCC5_AARCH64_ASLDLINK_FLAGS   = DEF(GCC49_AARCH64_ASLDLINK_FLAGS)
>  DEFINE GCC5_ASLCC_FLAGS              = DEF(GCC49_ASLCC_FLAGS) -fno-lto
>  
> +DEFINE GCC5_RISCV_ALL_CC_FLAGS                    = -g -fshort-wchar -fno-strict-aliasing -Wall -Werror -Wno-array-bounds -ffunction-sections -fdata-sections -c -include AutoGen.h -fno-common -DSTRING_ARRAY_NAME=$(BASE_NAME)Strings
> +DEFINE GCC5_RISCV_ALL_DLINK_COMMON                = -nostdlib -n -q --gc-sections -z common-page-size=0x40
> +DEFINE GCC5_RISCV_ALL_DLINK_FLAGS                 = DEF(GCC5_RISCV_ALL_DLINK_COMMON) --entry $(IMAGE_ENTRY_POINT) -u $(IMAGE_ENTRY_POINT) -Map $(DEST_DIR_DEBUG)/$(BASE_NAME).map
> +DEFINE GCC5_RISCV_ALL_DLINK2_FLAGS                = --defsym=PECOFF_HEADER_SIZE=0x220 --script=$(EDK_TOOLS_PATH)/Scripts/GccBaseRiscV.lds
> +DEFINE GCC5_RISCV_ALL_ASM_FLAGS                   = -c -x assembler -imacros $(DEST_DIR_DEBUG)/AutoGen.h
> +DEFINE GCC5_RISCV_ALL_CC_FLAGS_WARNING_DISABLE    = -Wno-tautological-compare -Wno-pointer-compare
> +
> +DEFINE GCC5_RISCV64_ARCH                   = rv64imafdc
> +DEFINE GCC5_RISCV32_RISCV64_ASLDLINK_FLAGS = DEF(GCC5_RISCV_ALL_DLINK_COMMON) --entry ReferenceAcpiTable -u ReferenceAcpiTable
> +DEFINE GCC5_RISCV32_RISCV64_DLINK_FLAGS    = DEF(GCC5_RISCV_ALL_DLINK_COMMON) --entry $(IMAGE_ENTRY_POINT) -u $(IMAGE_ENTRY_POINT) -Map $(DEST_DIR_DEBUG)/$(BASE_NAME).map
> +DEFINE GCC5_RISCV64_CC_FLAGS               = DEF(GCC5_RISCV_ALL_CC_FLAGS) DEF(GCC5_RISCV_ALL_CC_FLAGS_WARNING_DISABLE) -march=DEF(GCC5_RISCV64_ARCH) -fno-builtin -fno-builtin-memcpy -fno-stack-protector -Wno-address -fno-asynchronous-unwind-tables -Wno-unused-but-set-variable -fpack-struct=8 -mcmodel=medany -mabi=lp64
> +DEFINE GCC5_RISCV64_DLINK_FLAGS            = DEF(GCC5_RISCV_ALL_DLINK_FLAGS)  -melf64lriscv --oformat=elf64-littleriscv --no-relax
> +DEFINE GCC5_RISCV64_DLINK2_FLAGS           = DEF(GCC5_RISCV_ALL_DLINK2_FLAGS)
> +DEFINE GCC5_ASM_FLAGS                      = DEF(GCC5_RISCV_ALL_ASM_FLAGS) -march=DEF(GCC5_RISCV64_ARCH) -mcmodel=medany -mabi=lp64
> +
>  ####################################################################################
>  #
>  # GCC 4.8 - This configuration is used to compile under Linux to produce
> @@ -2247,6 +2264,49 @@ RELEASE_GCC5_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20
>    NOOPT_GCC5_AARCH64_DLINK_FLAGS = DEF(GCC5_AARCH64_DLINK_FLAGS) -O0
>    NOOPT_GCC5_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20 -O0
>  
> +###################################################################################
> +####################################################################################
> +#
> +# GCC RISC-V This configuration is used to compile under Linux to produce
> +#             PE/COFF binaries using GCC RISC-V tool chain
> +#
> +####################################################################################
> +
> +#*_GCC5_*_*_FAMILY                   = GCC
> +
> +#*_GCC5_*_MAKE_PATH                  = DEF(GCC49_IA32_PREFIX)make
> +#*_GCC5_*_PP_FLAGS                   = DEF(GCC_PP_FLAGS)
> +#*_GCC5_*_ASLPP_FLAGS                = DEF(GCC_ASLPP_FLAGS)
> +#*_GCC5_*_ASLCC_FLAGS                = DEF(GCC_ASLCC_FLAGS)
> +#*_GCC5_*_VFRPP_FLAGS                = DEF(GCC_VFRPP_FLAGS)
> +#*_GCC5_*_APP_FLAGS                  =
> +#*_GCC5_*_ASL_FLAGS                  = DEF(IASL_FLAGS)
> +#*_GCC5_*_ASL_OUTFLAGS               = DEF(IASL_OUTFLAGS)
> +
> +##################
> +# GCC5 RISCV64 definitions
> +##################
> +*_GCC5_RISCV64_OBJCOPY_PATH         = ENV(GCC5_RISCV64_PREFIX)objcopy
> +*_GCC5_RISCV64_CC_PATH              = ENV(GCC5_RISCV64_PREFIX)gcc
> +*_GCC5_RISCV64_SLINK_PATH           = ENV(GCC5_RISCV64_PREFIX)gcc-ar
> +*_GCC5_RISCV64_DLINK_PATH           = ENV(GCC5_RISCV64_PREFIX)ld
> +*_GCC5_RISCV64_ASLDLINK_PATH        = ENV(GCC5_RISCV64_PREFIX)ld
> +*_GCC5_RISCV64_ASM_PATH             = ENV(GCC5_RISCV64_PREFIX)gcc
> +*_GCC5_RISCV64_PP_PATH              = ENV(GCC5_RISCV64_PREFIX)gcc
> +*_GCC5_RISCV64_VFRPP_PATH           = ENV(GCC5_RISCV64_PREFIX)gcc
> +*_GCC5_RISCV64_ASLCC_PATH           = ENV(GCC5_RISCV64_PREFIX)gcc
> +*_GCC5_RISCV64_ASLPP_PATH           = ENV(GCC5_RISCV64_PREFIX)gcc
> +*_GCC5_RISCV64_RC_PATH              = ENV(GCC5_RISCV64_PREFIX)objcopy
> +
> +*_GCC5_RISCV64_ASLCC_FLAGS          = DEF(GCC_ASLCC_FLAGS)
> +*_GCC5_RISCV64_ASLDLINK_FLAGS       = DEF(GCC5_RISCV32_RISCV64_ASLDLINK_FLAGS)
> +*_GCC5_RISCV64_ASM_FLAGS            = DEF(GCC5_ASM_FLAGS)
> +*_GCC5_RISCV64_CC_FLAGS             = DEF(GCC5_RISCV64_CC_FLAGS) -save-temps
> +*_GCC5_RISCV64_DLINK_FLAGS          = DEF(GCC5_RISCV64_DLINK_FLAGS)
> +*_GCC5_RISCV64_DLINK2_FLAGS         = DEF(GCC5_RISCV64_DLINK2_FLAGS)
> +*_GCC5_RISCV64_RC_FLAGS             = DEF(GCC_RISCV64_RC_FLAGS)
> +*_GCC5_RISCV64_OBJCOPY_FLAGS        =
> +
>  ####################################################################################
>  #
>  # CLANG35   - This configuration is used to compile under Linux to produce
> diff --git a/BaseTools/Source/C/Common/BasePeCoff.c b/BaseTools/Source/C/Common/BasePeCoff.c
> index e7566b3..640f7a1 100644
> --- a/BaseTools/Source/C/Common/BasePeCoff.c
> +++ b/BaseTools/Source/C/Common/BasePeCoff.c
> @@ -4,6 +4,7 @@
>  
>  Copyright (c) 2004 - 2018, Intel Corporation. All rights reserved.<BR>
>  Portions Copyright (c) 2011 - 2013, ARM Ltd. All rights reserved.<BR>
> +Portions Copyright (c) 2016 - 2019, Hewlett Packard Enterprise Development LP. All rights reserved.<BR>
>  SPDX-License-Identifier: BSD-2-Clause-Patent
>  
>  **/
> @@ -59,6 +60,14 @@ PeCoffLoaderRelocateArmImage (
>    IN UINT64      Adjust
>    );
>  
> +RETURN_STATUS
> +PeCoffLoaderRelocateRiscVImage (
> +  IN UINT16      *Reloc,
> +  IN OUT CHAR8   *Fixup,
> +  IN OUT CHAR8   **FixupData,
> +  IN UINT64      Adjust
> +  );
> +
>  STATIC
>  RETURN_STATUS
>  PeCoffLoaderGetPeHeader (
> @@ -174,7 +183,8 @@ Returns:
>        ImageContext->Machine != EFI_IMAGE_MACHINE_X64  && \
>        ImageContext->Machine != EFI_IMAGE_MACHINE_ARMT && \
>        ImageContext->Machine != EFI_IMAGE_MACHINE_EBC  && \
> -      ImageContext->Machine != EFI_IMAGE_MACHINE_AARCH64) {
> +      ImageContext->Machine != EFI_IMAGE_MACHINE_AARCH64 && \
> +      ImageContext->Machine != EFI_IMAGE_MACHINE_RISCV64) {
>      if (ImageContext->Machine == IMAGE_FILE_MACHINE_ARM) {
>        //
>        // There are two types of ARM images. Pure ARM and ARM/Thumb.
> @@ -802,6 +812,9 @@ Returns:
>          case EFI_IMAGE_MACHINE_ARMT:
>            Status = PeCoffLoaderRelocateArmImage (&Reloc, Fixup, &FixupData, Adjust);
>            break;
> +        case EFI_IMAGE_MACHINE_RISCV64:
> +          Status = PeCoffLoaderRelocateRiscVImage (Reloc, Fixup, &FixupData, Adjust);
> +          break;
>          default:
>            Status = RETURN_UNSUPPORTED;
>            break;
> diff --git a/BaseTools/Source/C/Common/PeCoffLoaderEx.c b/BaseTools/Source/C/Common/PeCoffLoaderEx.c
> index e367836..36797d9 100644
> --- a/BaseTools/Source/C/Common/PeCoffLoaderEx.c
> +++ b/BaseTools/Source/C/Common/PeCoffLoaderEx.c
> @@ -3,6 +3,7 @@ IA32 and X64 Specific relocation fixups
>  
>  Copyright (c) 2004 - 2018, Intel Corporation. All rights reserved.<BR>
>  Portions Copyright (c) 2011 - 2013, ARM Ltd. All rights reserved.<BR>
> +Copyright (c) 2016 - 2019, Hewlett Packard Enterprise Development LP. All rights reserved.<BR>
>  SPDX-License-Identifier: BSD-2-Clause-Patent
>  
>  --*/
> @@ -61,6 +62,17 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #define IMM64_SIGN_INST_WORD_POS_X      27
>  #define IMM64_SIGN_VAL_POS_X            63
>  
> +//
> +// RISC-V definition.
> +//
> +#define RV_X(x, s, n) (((x) >> (s)) & ((1<<(n))-1))
> +#define RISCV_IMM_BITS 12
> +#define RISCV_IMM_REACH (1LL<<RISCV_IMM_BITS)
> +#define RISCV_CONST_HIGH_PART(VALUE) \
> +  (((VALUE) + (RISCV_IMM_REACH/2)) & ~(RISCV_IMM_REACH-1))
> +
> +UINT32 *RiscVHi20Fixup = NULL;
> +
>  RETURN_STATUS
>  PeCoffLoaderRelocateIa32Image (
>    IN UINT16      *Reloc,
> @@ -93,6 +105,89 @@ Returns:
>    return RETURN_UNSUPPORTED;
>  }
>  
> +/*++
> +
> +Routine Description:
> +
> +  Performs an RISC-V specific relocation fixup
> +
> +Arguments:
> +
> +  Reloc      - Pointer to the relocation record
> +
> +  Fixup      - Pointer to the address to fix up
> +
> +  FixupData  - Pointer to a buffer to log the fixups
> +
> +  Adjust     - The offset to adjust the fixup
> +
> +Returns:
> +
> +  Status code
> +
> +--*/
> +RETURN_STATUS
> +PeCoffLoaderRelocateRiscVImage (
> +  IN UINT16      *Reloc,
> +  IN OUT CHAR8   *Fixup,
> +  IN OUT CHAR8   **FixupData,
> +  IN UINT64      Adjust
> +  )
> +{
> +  UINT32 Value;
> +  UINT32 Value2;
> +  UINT32 OrgValue;
> +
> +  OrgValue = *(UINT32 *) Fixup;
> +  OrgValue = OrgValue;
> +  switch ((*Reloc) >> 12) {
> +  case EFI_IMAGE_REL_BASED_RISCV_HI20:
> +      RiscVHi20Fixup = (UINT32 *) Fixup;
> +      break;
> +
> +  case EFI_IMAGE_REL_BASED_RISCV_LOW12I:
> +      if (RiscVHi20Fixup != NULL) {
> +        Value = (UINT32)(RV_X(*RiscVHi20Fixup, 12, 20) << 12);
> +        Value2 = (UINT32)(RV_X(*(UINT32 *)Fixup, 20, 12));
> +        if (Value2 & (RISCV_IMM_REACH/2)) {
> +          Value2 |= ~(RISCV_IMM_REACH-1);
> +        }
> +        Value += Value2;
> +        Value += (UINT32)Adjust;
> +        Value2 = RISCV_CONST_HIGH_PART (Value);
> +        *(UINT32 *)RiscVHi20Fixup = (RV_X (Value2, 12, 20) << 12) | \
> +                                           (RV_X (*(UINT32 *)RiscVHi20Fixup, 0, 12));
> +        *(UINT32 *)Fixup = (RV_X (Value, 0, 12) << 20) | \
> +                           (RV_X (*(UINT32 *)Fixup, 0, 20));
> +      }
> +      RiscVHi20Fixup = NULL;
> +      break;
> +
> +  case EFI_IMAGE_REL_BASED_RISCV_LOW12S:
> +      if (RiscVHi20Fixup != NULL) {
> +        Value = (UINT32)(RV_X(*RiscVHi20Fixup, 12, 20) << 12);
> +        Value2 = (UINT32)(RV_X(*(UINT32 *)Fixup, 7, 5) | (RV_X(*(UINT32 *)Fixup, 25, 7) << 5));
> +        if (Value2 & (RISCV_IMM_REACH/2)) {
> +          Value2 |= ~(RISCV_IMM_REACH-1);
> +        }
> +        Value += Value2;
> +        Value += (UINT32)Adjust;
> +        Value2 = RISCV_CONST_HIGH_PART (Value);
> +        *(UINT32 *)RiscVHi20Fixup = (RV_X (Value2, 12, 20) << 12) | \
> +                                           (RV_X (*(UINT32 *)RiscVHi20Fixup, 0, 12));
> +        Value2 = *(UINT32 *)Fixup & 0x01fff07f;
> +        Value &= RISCV_IMM_REACH - 1;
> +        *(UINT32 *)Fixup = Value2 | (UINT32)(((RV_X(Value, 0, 5) << 7) | (RV_X(Value, 5, 7) << 25)));
> +      }
> +      RiscVHi20Fixup = NULL;
> +      break;
> +
> +  default:
> +      return EFI_UNSUPPORTED;
> +
> +  }
> +  return RETURN_SUCCESS;
> +}
>  
>  /**
>    Pass in a pointer to an ARM MOVT or MOVW immediate instruction and
> diff --git a/BaseTools/Source/C/GenFv/GenFvInternalLib.c b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
> index 908740d..fdbdd42 100644
> --- a/BaseTools/Source/C/GenFv/GenFvInternalLib.c
> +++ b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
> @@ -4,6 +4,7 @@ This file contains the internal functions required to generate a Firmware Volume
>  Copyright (c) 2004 - 2018, Intel Corporation. All rights reserved.<BR>
>  Portions Copyright (c) 2011 - 2013, ARM Ltd. All rights reserved.<BR>
>  Portions Copyright (c) 2016 HP Development Company, L.P.<BR>
> +Portions Copyright (c) 2016 - 2019, Hewlett Packard Enterprise Development LP. All rights reserved.<BR>
>  SPDX-License-Identifier: BSD-2-Clause-Patent
>  
>  **/
> @@ -37,6 +38,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #define ARM64_UNCONDITIONAL_JUMP_INSTRUCTION      0x14000000
>  
>  BOOLEAN mArm = FALSE;
> +BOOLEAN mRiscV = FALSE;
>  STATIC UINT32   MaxFfsAlignment = 0;
>  BOOLEAN VtfFileFlag = FALSE;
>  
> @@ -2274,6 +2276,104 @@ Returns:
>  }
>  
>  EFI_STATUS
> +UpdateRiscvResetVectorIfNeeded (
> +  MEMORY_FILE            *FvImage,
> +  FV_INFO                *FvInfo
> +  )
> +/*++
> +
> +Routine Description:
> +  This parses the FV looking for SEC and patches that address into the
> +  beginning of the FV header.
> +
> +  For RISC-V ISA, the reset vector is at 0xfff~ff00h or 200h
> +
> +Arguments:
> +  FvImage       Memory file for the FV memory image/
> +  FvInfo        Information read from INF file.
> +
> +Returns:
> +
> +  EFI_SUCCESS             Function Completed successfully.
> +  EFI_ABORTED             Error encountered.
> +  EFI_INVALID_PARAMETER   A required parameter was NULL.
> +  EFI_NOT_FOUND           PEI Core file not found.
> +
> +--*/
> +{
> +  EFI_STATUS                Status;
> +  UINT16                    MachineType;
> +  EFI_FILE_SECTION_POINTER  SecPe32;
> +  EFI_PHYSICAL_ADDRESS      SecCoreEntryAddress;
> +
> +  UINT32 bSecCore;
> +  UINT32 tmp;
> +
> +
> +  //
> +  // Verify input parameters
> +  //
> +  if (FvImage == NULL || FvInfo == NULL) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +  //
> +  // Initialize FV library
> +  //
> +  InitializeFvLib (FvImage->FileImage, FvInfo->Size);
> +
> +  //
> +  // Find the Sec Core
> +  //
> +  Status = FindCorePeSection(FvImage->FileImage, FvInfo->Size, EFI_FV_FILETYPE_SECURITY_CORE, &SecPe32);
> +  if(EFI_ERROR(Status)) {
> +    printf("skip because Secutiry Core not found\n");
> +    return EFI_SUCCESS;
> +  }
> +
> +  DebugMsg (NULL, 0, 9, "Update SEC core in FV Header", NULL);
> +
> +  Status = GetCoreMachineType(SecPe32, &MachineType);
> +  if(EFI_ERROR(Status)) {
> +    Error(NULL, 0, 3000, "Invalid", "Could not get the PE32 machine type for SEC core.");
> +    return EFI_ABORTED;
> +  }
> +
> +  if (MachineType != EFI_IMAGE_MACHINE_RISCV64) {
> +    Error(NULL, 0, 3000, "Invalid", "Could not update SEC core because Machine type is not RiscV.");
> +    return EFI_ABORTED;
> +  }
> +
> +  Status = GetCoreEntryPointAddress(FvImage->FileImage, FvInfo, SecPe32, &SecCoreEntryAddress);
> +  if(EFI_ERROR(Status)) {
> +    Error(NULL, 0, 3000, "Invalid", "Could not get the PE32 entry point address for SEC Core.");
> +    return EFI_ABORTED;
> +  }
> +
> +  VerboseMsg("SecCore entry point Address = 0x%llX", (unsigned long long) SecCoreEntryAddress);
> +  VerboseMsg("BaseAddress = 0x%llX", (unsigned long long) FvInfo->BaseAddress);
> +  bSecCore = (SecCoreEntryAddress - FvInfo->BaseAddress);
> +  VerboseMsg("offset = 0x%llX", bSecCore);
> +
> +  if(bSecCore > 0x0fffff) {
> +    Error(NULL, 0, 3000, "Invalid", "SEC Entry point must be within 1MB of start of the FV");
> +    return EFI_ABORTED;
> +  }
> +
> +  tmp = bSecCore;
> +  bSecCore = 0;
> +  //J-type
> +  bSecCore  = (tmp&0x100000)<<11; //imm[20]    at bit[31]
> +  bSecCore |= (tmp&0x0007FE)<<20; //imm[10:1]  at bit[30:21]
> +  bSecCore |= (tmp&0x000800)<<9;  //imm[11]    at bit[20]
> +  bSecCore |= (tmp&0x0FF000);     //imm[19:12] at bit[19:12]
> +  bSecCore |= 0x6F; //JAL opcode
> +
> +  memcpy(FvImage->FileImage, &bSecCore, sizeof(bSecCore));
> +
> +  return EFI_SUCCESS;
> +}
> +
> +EFI_STATUS
>  GetPe32Info (
>    IN UINT8                  *Pe32,
>    OUT UINT32                *EntryPoint,
> @@ -2365,7 +2465,8 @@ Returns:
>    // Verify machine type is supported
>    //
>    if ((*MachineType != EFI_IMAGE_MACHINE_IA32) &&  (*MachineType != EFI_IMAGE_MACHINE_X64) && (*MachineType != EFI_IMAGE_MACHINE_EBC) &&
> -      (*MachineType != EFI_IMAGE_MACHINE_ARMT) && (*MachineType != EFI_IMAGE_MACHINE_AARCH64)) {
> +      (*MachineType != EFI_IMAGE_MACHINE_ARMT) && (*MachineType != EFI_IMAGE_MACHINE_AARCH64) &&
> +      (*MachineType != EFI_IMAGE_MACHINE_RISCV64)) {
>      Error (NULL, 0, 3000, "Invalid", "Unrecognized machine type in the PE32 file.");
>      return EFI_UNSUPPORTED;
>    }
> @@ -2808,7 +2909,8 @@ Returns:
>        Error (NULL, 0, 4002, "Resource", "FV space is full, cannot add pad file between the last file and the VTF file.");
>        goto Finish;
>      }
> -    if (!mArm) {
> +
> +    if (!mArm && !mRiscV) {
>        //
>        // Update reset vector (SALE_ENTRY for IPF)
>        // Now for IA32 and IA64 platform, the fv which has bsf file must have the
> @@ -2843,6 +2945,22 @@ Returns:
>      FvHeader->Checksum = CalculateChecksum16 ((UINT16 *) FvHeader, FvHeader->HeaderLength / sizeof (UINT16));
>    }
>  
> +  if (mRiscV) {
> +     //
> +     // Update RISCV reset vector.
> +     //
> +     Status = UpdateRiscvResetVectorIfNeeded (&FvImageMemoryFile, &mFvDataInfo);
> +     if (EFI_ERROR (Status)) {
> +       Error (NULL, 0, 3000, "Invalid", "Could not update the reset vector for RISC-V.");
> +       goto Finish;
> +    }
> +    //
> +    // Update Checksum for FvHeader
> +    //
> +    FvHeader->Checksum = 0;
> +    FvHeader->Checksum = CalculateChecksum16 ((UINT16 *) FvHeader, FvHeader->HeaderLength / sizeof (UINT16));
> +  }
> +
>    //
>    // Update FV Alignment attribute to the largest alignment of all the FFS files in the FV
>    //
> @@ -3430,6 +3548,10 @@ Returns:
>        mArm = TRUE;
>      }
>  
> +    if (ImageContext.Machine == EFI_IMAGE_MACHINE_RISCV64) {
> +      mRiscV = TRUE;
> +    }
> +
>      //
>      // Keep Image Context for PE image in FV
>      //
> @@ -3583,7 +3705,7 @@ Returns:
>      ImageContext.DestinationAddress = NewPe32BaseAddress;
>      Status                          = PeCoffLoaderRelocateImage (&ImageContext);
>      if (EFI_ERROR (Status)) {
> -      Error (NULL, 0, 3000, "Invalid", "RelocateImage() call failed on rebase of %s", FileName);
> +      Error (NULL, 0, 3000, "Invalid", "RelocateImage() call failed on rebase of %s Status=%d", FileName, Status);
>        free ((VOID *) MemoryImagePointer);
>        return Status;
>      }
> diff --git a/BaseTools/Source/C/GenFw/Elf32Convert.c b/BaseTools/Source/C/GenFw/Elf32Convert.c
> index 46089ff..4095b7c 100644
> --- a/BaseTools/Source/C/GenFw/Elf32Convert.c
> +++ b/BaseTools/Source/C/GenFw/Elf32Convert.c
> @@ -3,6 +3,7 @@ Elf32 Convert solution
>  
>  Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.<BR>
>  Portions copyright (c) 2013, ARM Ltd. All rights reserved.<BR>
> +Portions Copyright (c) 2016 - 2019, Hewlett Packard Enterprise Development LP. All rights reserved.<BR>
>  
>  SPDX-License-Identifier: BSD-2-Clause-Patent
>  
> @@ -141,8 +142,8 @@ InitializeElf32 (
>      Error (NULL, 0, 3000, "Unsupported", "ELF e_type not ET_EXEC or ET_DYN");
>      return FALSE;
>    }
> -  if (!((mEhdr->e_machine == EM_386) || (mEhdr->e_machine == EM_ARM))) {
> -    Error (NULL, 0, 3000, "Unsupported", "ELF e_machine not EM_386 or EM_ARM");
> +  if (!((mEhdr->e_machine == EM_386) || (mEhdr->e_machine == EM_ARM) || (mEhdr->e_machine == EM_RISCV))) {
> +    Error (NULL, 0, 3000, "Unsupported", "ELF e_machine is not Elf32 machine.");
>      return FALSE;
>    }
>    if (mEhdr->e_version != EV_CURRENT) {
> diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c b/BaseTools/Source/C/GenFw/Elf64Convert.c
> index 3d6319c..2aa09fd 100644
> --- a/BaseTools/Source/C/GenFw/Elf64Convert.c
> +++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
> @@ -3,6 +3,7 @@ Elf64 convert solution
>  
>  Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.<BR>
>  Portions copyright (c) 2013-2014, ARM Ltd. All rights reserved.<BR>
> +Portions Copyright (c) 2016 - 2019 Hewlett Packard Enterprise Development LP. All rights reserved.<BR>
>  
>  SPDX-License-Identifier: BSD-2-Clause-Patent
>  
> @@ -31,6 +32,12 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #include "ElfConvert.h"
>  #include "Elf64Convert.h"
>  
> +#define RV_X(x, s, n) (((x) >> (s)) & ((1<<(n))-1))
> +#define RISCV_IMM_BITS 12
> +#define RISCV_IMM_REACH (1LL<<RISCV_IMM_BITS)
> +#define RISCV_CONST_HIGH_PART(VALUE) \
> +  (((VALUE) + (RISCV_IMM_REACH/2)) & ~(RISCV_IMM_REACH-1))
> +
>  STATIC
>  VOID
>  ScanSections64 (
> @@ -153,8 +160,8 @@ InitializeElf64 (
>      Error (NULL, 0, 3000, "Unsupported", "ELF e_type not ET_EXEC or ET_DYN");
>      return FALSE;
>    }
> -  if (!((mEhdr->e_machine == EM_X86_64) || (mEhdr->e_machine == EM_AARCH64))) {
> -    Error (NULL, 0, 3000, "Unsupported", "ELF e_machine not EM_X86_64 or EM_AARCH64");
> +  if (!((mEhdr->e_machine == EM_X86_64) || (mEhdr->e_machine == EM_AARCH64) || (mEhdr->e_machine == EM_RISCV64))) {
> +    Error (NULL, 0, 3000, "Unsupported", "ELF e_machine is not Elf64 machine.");
>      return FALSE;
>    }
>    if (mEhdr->e_version != EV_CURRENT) {
> @@ -481,6 +488,7 @@ ScanSections64 (
>    switch (mEhdr->e_machine) {
>    case EM_X86_64:
>    case EM_AARCH64:
> +  case EM_RISCV64:
>      mCoffOffset += sizeof (EFI_IMAGE_NT_HEADERS64);
>    break;
>    default:
> @@ -690,6 +698,11 @@ ScanSections64 (
>      NtHdr->Pe32Plus.FileHeader.Machine = EFI_IMAGE_MACHINE_AARCH64;
>      NtHdr->Pe32Plus.OptionalHeader.Magic = EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
>      break;
> +  case EM_RISCV64:
> +    NtHdr->Pe32Plus.FileHeader.Machine = EFI_IMAGE_MACHINE_RISCV64;
> +    NtHdr->Pe32Plus.OptionalHeader.Magic = EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
> +    break;
> +
>    default:
>      VerboseMsg ("%s unknown e_machine type. Assume X64", (UINTN)mEhdr->e_machine);
>      NtHdr->Pe32Plus.FileHeader.Machine = EFI_IMAGE_MACHINE_X64;
> @@ -769,6 +782,11 @@ WriteSections64 (
>    Elf_Shdr    *SecShdr;
>    UINT32      SecOffset;
>    BOOLEAN     (*Filter)(Elf_Shdr *);
> +  UINT32      Value;
> +  UINT32      Value2;
> +  UINT8       *Pass1Targ = NULL;
> +  Elf_Shdr    *Pass1Sym = NULL;
> +  Elf64_Half  Pass1SymSecIndex = 0;
>    Elf64_Addr  GOTEntryRva;
>  
>    //
> @@ -893,13 +911,14 @@ WriteSections64 (
>            if (SymName == NULL) {
>              SymName = (const UINT8 *)"<unknown>";
>            }
> +          if (mEhdr->e_machine != EM_RISCV64) {

This needs a comment explaining why this does not apply to RISCV.

> +            Error (NULL, 0, 3000, "Invalid",
> +                   "%s: Bad definition for symbol '%s'@%#llx or unsupported symbol type.  "
> +                   "For example, absolute and undefined symbols are not supported.",
> +                   mInImageName, SymName, Sym->st_value);
>  
> -          Error (NULL, 0, 3000, "Invalid",
> -                 "%s: Bad definition for symbol '%s'@%#llx or unsupported symbol type.  "
> -                 "For example, absolute and undefined symbols are not supported.",
> -                 mInImageName, SymName, Sym->st_value);
> -
> -          exit(EXIT_FAILURE);
> +            exit(EXIT_FAILURE);
> +          }
>          }
>          SymShdr = GetShdrByIndex(Sym->st_shndx);
>  
> @@ -1114,6 +1133,128 @@ WriteSections64 (
>            default:
>              Error (NULL, 0, 3000, "Invalid", "WriteSections64(): %s unsupported ELF EM_AARCH64 relocation 0x%x.", mInImageName, (unsigned) ELF_R_TYPE(Rel->r_info));
>            }
> +        } else if (mEhdr->e_machine == EM_RISCV64) {

Yeah, this code block is just *waaaay* too big.
Please break it out into its own helper function.

> +          switch (ELF_R_TYPE(Rel->r_info)) {
> +          case R_RISCV_NONE:
> +            break;
> +          case R_RISCV_32:
> +            *(UINT32 *)Targ = (UINT32)((UINT64)(*(UINT32 *)Targ) - SymShdr->sh_addr + mCoffSectionsOffset[Sym->st_shndx]);
> +            break;
> +          case R_RISCV_64:
> +            *(UINT64 *)Targ = *(UINT64 *)Targ - SymShdr->sh_addr + mCoffSectionsOffset[Sym->st_shndx];
> +            break;
> +          case R_RISCV_HI20:
> +              Pass1Targ = Targ;
> +              Pass1Sym = SymShdr;
> +              Pass1SymSecIndex = Sym->st_shndx;
> +            break;
> +          case R_RISCV_LO12_I:
> +            if (Pass1Sym == SymShdr && Pass1Targ != NULL && Pass1SymSecIndex == Sym->st_shndx && Pass1SymSecIndex != 0) {
> +              Value = (UINT32)(RV_X(*(UINT32 *)Pass1Targ, 12, 20) << 12);
> +              Value2 = (UINT32)(RV_X(*(UINT32 *)Targ, 20, 12));
> +              if (Value2 & (RISCV_IMM_REACH/2)) {
> +                Value2 |= ~(RISCV_IMM_REACH-1);
> +              }
> +              Value += Value2;
> +              Value = Value - SymShdr->sh_addr + mCoffSectionsOffset[Sym->st_shndx];
> +              Value2 = RISCV_CONST_HIGH_PART (Value);
> +              *(UINT32 *)Pass1Targ = (RV_X (Value2, 12, 20) << 12) | \
> +                                         (RV_X (*(UINT32 *)Pass1Targ, 0, 12));
> +              *(UINT32 *)Targ = (RV_X (Value, 0, 12) << 20) | \
> +                                (RV_X (*(UINT32 *)Targ, 0, 20));
> +            }
> +            Pass1Sym = NULL;
> +            Pass1Targ = NULL;
> +            Pass1SymSecIndex = 0;
> +            break;
> +
> +          case R_RISCV_LO12_S:
> +            if (Pass1Sym == SymShdr && Pass1Targ != NULL && Pass1SymSecIndex == Sym->st_shndx && Pass1SymSecIndex != 0) {
> +              Value = (UINT32)(RV_X(*(UINT32 *)Pass1Targ, 12, 20) << 12);
> +              Value2 = (UINT32)(RV_X(*(UINT32 *)Targ, 7, 5) | (RV_X(*(UINT32 *)Targ, 25, 7) << 5));
> +              if (Value2 & (RISCV_IMM_REACH/2)) {
> +                Value2 |= ~(RISCV_IMM_REACH-1);
> +              }
> +              Value += Value2;
> +              Value = Value - SymShdr->sh_addr + mCoffSectionsOffset[Sym->st_shndx];
> +              Value2 = RISCV_CONST_HIGH_PART (Value);
> +              *(UINT32 *)Pass1Targ = (RV_X (Value2, 12, 20) << 12) | \
> +                                         (RV_X (*(UINT32 *)Pass1Targ, 0, 12));
> +
> +              Value2 = *(UINT32 *)Targ & 0x01fff07f;
> +              Value &= RISCV_IMM_REACH - 1;
> +              *(UINT32 *)Targ = Value2 | (UINT32)(((RV_X(Value, 0, 5) << 7) | (RV_X(Value, 5, 7) << 25)));
> +            }
> +            Pass1Sym = NULL;
> +            Pass1Targ = NULL;
> +            Pass1SymSecIndex = 0;
> +            break;
> +
> +          case R_RISCV_PCREL_HI20:
> +            Pass1Targ = Targ;
> +            Pass1Sym = SymShdr;
> +            Pass1SymSecIndex = Sym->st_shndx;
> +
> +            Value = (UINT32)(RV_X(*(UINT32 *)Pass1Targ, 12, 20));
> +            break;
> +          case R_RISCV_PCREL_LO12_I:
> +            if (Pass1Targ != NULL && Pass1Sym != NULL && Pass1SymSecIndex != 0) {
> +              int i;
> +              Value2 = (UINT32)(RV_X(*(UINT32 *)Pass1Targ, 12, 20));
> +              Value = (UINT32)(RV_X(*(UINT32 *)Targ, 20, 12));
> +              if(Value & (RISCV_IMM_REACH/2)) {
> +                Value |= ~(RISCV_IMM_REACH-1);
> +              }
> +              Value = Value - Pass1Sym->sh_addr + mCoffSectionsOffset[Pass1SymSecIndex];
> +              if(-2048 > (INT32)Value) {
> +                i = (-Value / 4096);
> +                Value2 -= i;
> +                Value += 4096 * i;
> +                if(-2048 > (INT32)Value) {
> +                  Value2 -= 1;
> +                  Value += 4096;
> +                }
> +              }
> +              else if( 2047 < (INT32)Value) {
> +                i = (Value / 4096);
> +                Value2 += i;
> +                Value -= 4096 * i;
> +                if(2047 < (INT32)Value) {
> +                  Value2 += 1;
> +                  Value -= 4096;
> +                }
> +              }
> +
> +              *(UINT32 *)Targ = (RV_X(Value, 0, 12) << 20) | (RV_X(*(UINT32*)Targ, 0, 20));
> +              *(UINT32 *)Pass1Targ = (RV_X(Value2, 0, 20)<<12) | (RV_X(*(UINT32 *)Pass1Targ, 0, 12));
> +            }
> +            Pass1Sym = NULL;
> +            Pass1Targ = NULL;
> +            Pass1SymSecIndex = 0;
> +            break;
> +
> +          case R_RISCV_ADD64:
> +          case R_RISCV_SUB64:
> +          case R_RISCV_ADD32:
> +          case R_RISCV_SUB32:
> +          case R_RISCV_BRANCH:
> +          case R_RISCV_JAL:
> +          case R_RISCV_GPREL_I:
> +          case R_RISCV_GPREL_S:
> +          case R_RISCV_CALL:
> +          case R_RISCV_RVC_BRANCH:
> +          case R_RISCV_RVC_JUMP:
> +          case R_RISCV_RELAX:
> +          case R_RISCV_SUB6:
> +          case R_RISCV_SET6:
> +          case R_RISCV_SET8:
> +          case R_RISCV_SET16:
> +          case R_RISCV_SET32:
> +            break;
> +
> +          default:
> +            Error (NULL, 0, 3000, "Invalid", "WriteSections64(): %s unsupported ELF EM_RISCV64 relocation 0x%x.", mInImageName, (unsigned) ELF_R_TYPE(Rel->r_info));
> +          }
>          } else {
>            Error (NULL, 0, 3000, "Invalid", "Not a supported machine type");
>          }
> @@ -1133,6 +1274,7 @@ WriteRelocations64 (
>    UINT32                           Index;
>    EFI_IMAGE_OPTIONAL_HEADER_UNION  *NtHdr;
>    EFI_IMAGE_DATA_DIRECTORY         *Dir;
> +  UINT32 RiscVRelType;
>  
>    for (Index = 0; Index < mEhdr->e_shnum; Index++) {
>      Elf_Shdr *RelShdr = GetShdrByIndex(Index);
> @@ -1237,6 +1379,108 @@ WriteRelocations64 (
>              default:
>                  Error (NULL, 0, 3000, "Invalid", "WriteRelocations64(): %s unsupported ELF EM_AARCH64 relocation 0x%x.", mInImageName, (unsigned) ELF_R_TYPE(Rel->r_info));
>              }
> +          } else if (mEhdr->e_machine == EM_RISCV64) {
> +            RiscVRelType = ELF_R_TYPE(Rel->r_info);
> +            switch (RiscVRelType) {
> +            case R_RISCV_NONE:
> +              break;
> +
> +            case R_RISCV_32:
> +              CoffAddFixup(
> +                (UINT32) ((UINT64) mCoffSectionsOffset[RelShdr->sh_info]
> +                + (Rel->r_offset - SecShdr->sh_addr)),
> +                EFI_IMAGE_REL_BASED_HIGHLOW);
> +              break;
> +
> +            case R_RISCV_64:
> +              CoffAddFixup(
> +                (UINT32) ((UINT64) mCoffSectionsOffset[RelShdr->sh_info]
> +                + (Rel->r_offset - SecShdr->sh_addr)),
> +                EFI_IMAGE_REL_BASED_DIR64);
> +              break;
> +
> +            case R_RISCV_HI20:
> +              CoffAddFixup(
> +                (UINT32) ((UINT64) mCoffSectionsOffset[RelShdr->sh_info]
> +                + (Rel->r_offset - SecShdr->sh_addr)),
> +                EFI_IMAGE_REL_BASED_RISCV_HI20);
> +              break;
> +
> +            case R_RISCV_LO12_I:
> +              CoffAddFixup(
> +                (UINT32) ((UINT64) mCoffSectionsOffset[RelShdr->sh_info]
> +                + (Rel->r_offset - SecShdr->sh_addr)),
> +                EFI_IMAGE_REL_BASED_RISCV_LOW12I);
> +              break;
> +
> +            case R_RISCV_LO12_S:
> +              CoffAddFixup(
> +                (UINT32) ((UINT64) mCoffSectionsOffset[RelShdr->sh_info]
> +                + (Rel->r_offset - SecShdr->sh_addr)),
> +                EFI_IMAGE_REL_BASED_RISCV_LOW12S);
> +              break;
> +
> +            case R_RISCV_ADD64:
> +              CoffAddFixup(
> +                (UINT32) ((UINT64) mCoffSectionsOffset[RelShdr->sh_info]
> +                + (Rel->r_offset - SecShdr->sh_addr)),
> +                EFI_IMAGE_REL_BASED_ABSOLUTE);
> +              break;
> +
> +            case R_RISCV_SUB64:
> +              CoffAddFixup(
> +                (UINT32) ((UINT64) mCoffSectionsOffset[RelShdr->sh_info]
> +                + (Rel->r_offset - SecShdr->sh_addr)),
> +                EFI_IMAGE_REL_BASED_ABSOLUTE);
> +              break;
> +
> +            case R_RISCV_ADD32:
> +              CoffAddFixup(
> +                (UINT32) ((UINT64) mCoffSectionsOffset[RelShdr->sh_info]
> +                + (Rel->r_offset - SecShdr->sh_addr)),
> +                EFI_IMAGE_REL_BASED_ABSOLUTE);
> +              break;
> +
> +            case R_RISCV_SUB32:
> +              CoffAddFixup(
> +                (UINT32) ((UINT64) mCoffSectionsOffset[RelShdr->sh_info]
> +                + (Rel->r_offset - SecShdr->sh_addr)),
> +                EFI_IMAGE_REL_BASED_ABSOLUTE);
> +              break;
> +
> +            case R_RISCV_BRANCH:
> +              CoffAddFixup(
> +                (UINT32) ((UINT64) mCoffSectionsOffset[RelShdr->sh_info]
> +                + (Rel->r_offset - SecShdr->sh_addr)),
> +                EFI_IMAGE_REL_BASED_ABSOLUTE);
> +              break;
> +
> +            case R_RISCV_JAL:
> +              CoffAddFixup(
> +                (UINT32) ((UINT64) mCoffSectionsOffset[RelShdr->sh_info]
> +                + (Rel->r_offset - SecShdr->sh_addr)),
> +                EFI_IMAGE_REL_BASED_ABSOLUTE);
> +              break;
> +
> +            case R_RISCV_GPREL_I:
> +            case R_RISCV_GPREL_S:
> +            case R_RISCV_CALL:
> +            case R_RISCV_RVC_BRANCH:
> +            case R_RISCV_RVC_JUMP:
> +            case R_RISCV_RELAX:
> +            case R_RISCV_SUB6:
> +            case R_RISCV_SET6:
> +            case R_RISCV_SET8:
> +            case R_RISCV_SET16:
> +            case R_RISCV_SET32:
> +            case R_RISCV_PCREL_HI20:
> +            case R_RISCV_PCREL_LO12_I:
> +              break;
> +
> +            default:
> +              printf ("Unsupported RISCV64 ELF relocation type 0x%x, offset: %lx\n", RiscVRelType, Rel->r_offset);

printf is not a supported output function in this file. Please use Error.

> +              Error (NULL, 0, 3000, "Invalid", "WriteRelocations64(): %s unsupported ELF EM_RISCV64 relocation 0x%x.", mInImageName, (unsigned) ELF_R_TYPE(Rel->r_info));
> +            }
>            } else {
>              Error (NULL, 0, 3000, "Not Supported", "This tool does not support relocations for ELF with e_machine %u (processor type).", (unsigned) mEhdr->e_machine);
>            }
> diff --git a/BaseTools/Source/C/GenFw/elf_common.h b/BaseTools/Source/C/GenFw/elf_common.h
> index 15c9e33..1321f78 100644
> --- a/BaseTools/Source/C/GenFw/elf_common.h
> +++ b/BaseTools/Source/C/GenFw/elf_common.h
> @@ -3,6 +3,7 @@ Ported ELF include files from FreeBSD
>  
>  Copyright (c) 2009 - 2010, Apple Inc. All rights reserved.<BR>
>  Portions Copyright (c) 2011 - 2013, ARM Ltd. All rights reserved.<BR>
> +Portion Copyright (c) 2016 - 2019, Hewlett Packard Enterprise Development LP. All rights reserved.<BR>
>  SPDX-License-Identifier: BSD-2-Clause-Patent
>  
>  
> @@ -178,6 +179,8 @@ typedef struct {
>  #define EM_X86_64  62  /* Advanced Micro Devices x86-64 */
>  #define  EM_AMD64  EM_X86_64  /* Advanced Micro Devices x86-64 (compat) */
>  #define EM_AARCH64  183  /* ARM 64bit Architecture */
> +#define EM_RISCV64  243 /* 64bit RISC-V Architecture */
> +#define EM_RISCV    244 /* 32bit RISC-V Architecture */
>  
>  /* Non-standard or deprecated. */
>  #define EM_486    6  /* Intel i486. */
> @@ -979,5 +982,64 @@ typedef struct {
>  #define  R_X86_64_GOTPCRELX  41  /* Load from 32 bit signed pc relative offset to GOT entry without REX prefix, relaxable. */
>  #define  R_X86_64_REX_GOTPCRELX  42  /* Load from 32 bit signed pc relative offset to GOT entry with REX prefix, relaxable. */
>  
> +/*
> + * RISC-V relocation types
> + */
> +
> +/* Relocation types used by the dynamic linker */
> +#define R_RISCV_NONE        0
> +#define R_RISCV_32          1
> +#define R_RISCV_64          2
> +#define R_RISCV_RELATIVE    3
> +#define R_RISCV_COPY        4
> +#define R_RISCV_JUMP_SLOT   5
> +#define R_RISCV_TLS_DTPMOD32    6
> +#define R_RISCV_TLS_DTPMOD64    7
> +#define R_RISCV_TLS_DTPREL32    8
> +#define R_RISCV_TLS_DTPREL64    9
> +#define R_RISCV_TLS_TPREL32     10
> +#define R_RISCV_TLS_TPREL64     11
>  
> +/* Relocation types not used by the dynamic linker */
> +#define R_RISCV_BRANCH      16
> +#define R_RISCV_JAL         17
> +#define R_RISCV_CALL        18
> +#define R_RISCV_CALL_PLT    19
> +#define R_RISCV_GOT_HI20    20
> +#define R_RISCV_TLS_GOT_HI20    21
> +#define R_RISCV_TLS_GD_HI20     22
> +#define R_RISCV_PCREL_HI20      23
> +#define R_RISCV_PCREL_LO12_I    24
> +#define R_RISCV_PCREL_LO12_S    25
> +#define R_RISCV_HI20            26
> +#define R_RISCV_LO12_I          27
> +#define R_RISCV_LO12_S          28
> +#define R_RISCV_TPREL_HI20      29
> +#define R_RISCV_TPREL_LO12_I    30
> +#define R_RISCV_TPREL_LO12_S    31
> +#define R_RISCV_TPREL_ADD       32
> +#define R_RISCV_ADD8            33
> +#define R_RISCV_ADD16           34
> +#define R_RISCV_ADD32           35
> +#define R_RISCV_ADD64           36
> +#define R_RISCV_SUB8            37
> +#define R_RISCV_SUB16           38
> +#define R_RISCV_SUB32           39
> +#define R_RISCV_SUB64           40
> +#define R_RISCV_GNU_VTINHERIT   41
> +#define R_RISCV_GNU_VTENTRY     42
> +#define R_RISCV_ALIGN           43
> +#define R_RISCV_RVC_BRANCH      44
> +#define R_RISCV_RVC_JUMP        45
> +#define R_RISCV_RVC_LUI         46
> +#define R_RISCV_GPREL_I         47
> +#define R_RISCV_GPREL_S         48
> +#define R_RISCV_TPREL_I         49
> +#define R_RISCV_TPREL_S         50
> +#define R_RISCV_RELAX           51
> +#define R_RISCV_SUB6            52
> +#define R_RISCV_SET6            53
> +#define R_RISCV_SET8            54
> +#define R_RISCV_SET16           55
> +#define R_RISCV_SET32           56
>  #endif /* !_SYS_ELF_COMMON_H_ */
> diff --git a/BaseTools/Source/C/Include/IndustryStandard/PeImage.h b/BaseTools/Source/C/Include/IndustryStandard/PeImage.h
> index 44037d1..2ed3008 100644
> --- a/BaseTools/Source/C/Include/IndustryStandard/PeImage.h
> +++ b/BaseTools/Source/C/Include/IndustryStandard/PeImage.h
> @@ -6,6 +6,7 @@
>  
>    Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
>    Portions copyright (c) 2011 - 2013, ARM Ltd. All rights reserved.<BR>
> +  Copyright (c) 2016 - 2019, Hewlett Packard Enterprise Development LP. All rights reserved.<BR>
>  
>    SPDX-License-Identifier: BSD-2-Clause-Patent
>  
> @@ -41,6 +42,7 @@
>  #define IMAGE_FILE_MACHINE_ARM      0x01c0  // Thumb only
>  #define IMAGE_FILE_MACHINE_ARMT     0x01c2  // 32bit Mixed ARM and Thumb/Thumb 2  Little Endian
>  #define IMAGE_FILE_MACHINE_ARM64    0xAA64  // 64bit ARM Architecture, Little Endian
> +#define IMAGE_FILE_MACHINE_RISCV64  0x5064  // 64bit RISC-V ISA
>  
>  //
>  // Support old names for backward compatible
> @@ -50,6 +52,7 @@
>  #define EFI_IMAGE_MACHINE_X64       IMAGE_FILE_MACHINE_X64
>  #define EFI_IMAGE_MACHINE_ARMT      IMAGE_FILE_MACHINE_ARMT
>  #define EFI_IMAGE_MACHINE_AARCH64   IMAGE_FILE_MACHINE_ARM64
> +#define EFI_IMAGE_MACHINE_RISCV64   IMAGE_FILE_MACHINE_RISCV64
>  
>  #define EFI_IMAGE_DOS_SIGNATURE     0x5A4D      // MZ
>  #define EFI_IMAGE_OS2_SIGNATURE     0x454E      // NE
> @@ -504,7 +507,10 @@ typedef struct {
>  #define EFI_IMAGE_REL_BASED_HIGHADJ       4
>  #define EFI_IMAGE_REL_BASED_MIPS_JMPADDR  5
>  #define EFI_IMAGE_REL_BASED_ARM_MOV32A    5
> +#define EFI_IMAGE_REL_BASED_RISCV_HI20    5
>  #define EFI_IMAGE_REL_BASED_ARM_MOV32T    7
> +#define EFI_IMAGE_REL_BASED_RISCV_LOW12I  7
> +#define EFI_IMAGE_REL_BASED_RISCV_LOW12S  8
>  #define EFI_IMAGE_REL_BASED_IA64_IMM64    9
>  #define EFI_IMAGE_REL_BASED_DIR64         10
>  
> diff --git a/BaseTools/Source/Python/Common/DataType.py b/BaseTools/Source/Python/Common/DataType.py
> index 8ae1bd2..fc23e8c 100644
> --- a/BaseTools/Source/Python/Common/DataType.py
> +++ b/BaseTools/Source/Python/Common/DataType.py
> @@ -3,6 +3,7 @@
>  #
>  # Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.<BR>
>  # Portions copyright (c) 2011 - 2013, ARM Ltd. All rights reserved.<BR>
> +# Portions Copyright (c) 2016 - 2019, Hewlett Packard Enterprise Development LP. All rights reserved.<BR>
>  # SPDX-License-Identifier: BSD-2-Clause-Patent
>  
>  ##
> @@ -52,7 +53,9 @@ TAB_ARCH_ARM = 'ARM'
>  TAB_ARCH_EBC = 'EBC'
>  TAB_ARCH_AARCH64 = 'AARCH64'
>  
> -ARCH_SET_FULL = {TAB_ARCH_IA32, TAB_ARCH_X64, TAB_ARCH_ARM, TAB_ARCH_EBC, TAB_ARCH_AARCH64, TAB_ARCH_COMMON}
> +TAB_ARCH_RISCV64 = 'RISCV64'
> +
> +ARCH_SET_FULL = {TAB_ARCH_IA32, TAB_ARCH_X64, TAB_ARCH_ARM, TAB_ARCH_EBC, TAB_ARCH_AARCH64, TAB_ARCH_RISCV64, TAB_ARCH_COMMON}
>  
>  SUP_MODULE_BASE = 'BASE'
>  SUP_MODULE_SEC = 'SEC'
> @@ -532,4 +535,4 @@ PACK_CODE_BY_SIZE = {8:'=Q',
>                       0:'=B',
>                      16:""}
>  
> -TAB_COMPILER_MSFT = 'MSFT'
> \ No newline at end of file
> +TAB_COMPILER_MSFT = 'MSFT'

Please either fix in a separate patch or leave out altogether.

/
    Leif

> -- 
> 2.7.4
> 
> 
> 
>