[PATCH v2] firmware: remove copy-base relocation

Mon Mar 11 10:14:38 PDT 2024

On 11 Mar 2024, at 16:03, Xiang W <wxjstz at 126.com> wrote:
> 
> Remove copy-base relocations that are no longer needed.
> 
> Signed-off-by: Xiang W <wxjstz at 126.com>
> ---
> v2 changes:
> - Split from "[PATCH v5 0/8] Improvements to fw_base.S"
> - rebase to the latest master branch
> 
> Makefile            |  8 +++-
> README.md           |  5 ---
> docs/firmware/fw.md |  6 ---
> firmware/fw_base.S  | 98 ++-------------------------------------------
> firmware/objects.mk | 11 -----
> 5 files changed, 10 insertions(+), 118 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 680c19a..d8cffa6 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -179,6 +179,10 @@ CC_SUPPORT_STRICT_ALIGN := $(shell $(CC) $(CLANG_TARGET) $(RELAX_FLAG) -nostdlib
> # Check whether the assembler and the compiler support the Zicsr and Zifencei extensions
> CC_SUPPORT_ZICSR_ZIFENCEI := $(shell $(CC) $(CLANG_TARGET) $(RELAX_FLAG) -nostdlib -march=rv$(OPENSBI_CC_XLEN)imafd_zicsr_zifencei -x c /dev/null -o /dev/null 2>&1 | grep "zicsr\|zifencei" > /dev/null && echo n || echo y)
> 
> +ifneq ($(OPENSBI_LD_PIE),y)
> +$(error Your linker does not support creating PIEs, opensbi requires this.)
> +endif
> +
> # Build Info:
> # OPENSBI_BUILD_TIME_STAMP -- the compilation time stamp
> # OPENSBI_BUILD_COMPILER_VERSION -- the compiler version info
> @@ -356,7 +360,7 @@ CFLAGS += -mcmodel=$(PLATFORM_RISCV_CODE_MODEL)
> CFLAGS += $(RELAX_FLAG)
> CFLAGS += $(GENFLAGS)
> CFLAGS += $(platform-cflags-y)
> -CFLAGS += -fno-pie -no-pie
> +CFLAGS += -fPIE -pie
> CFLAGS += $(firmware-cflags-y)
> 
> CPPFLAGS += $(GENFLAGS)
> @@ -365,6 +369,7 @@ CPPFLAGS += $(firmware-cppflags-y)
> 
> ASFLAGS = -g -Wall -nostdlib
> ASFLAGS += -fno-omit-frame-pointer -fno-optimize-sibling-calls
> +ASFLAGS += -fpic

This isn’t your code, as you’re just moving it around, but this should
be -fPIE too. Otherwise la will unnecessarily be lga rather than lla.

> # Optionally supported flags
> ifeq ($(CC_SUPPORT_SAVE_RESTORE),y)
> ASFLAGS += -mno-save-restore
> @@ -391,6 +396,7 @@ ifeq ($(OPENSBI_LD_EXCLUDE_LIBS),y)
> ELFFLAGS += -Wl,--exclude-libs,ALL
> endif
> ELFFLAGS += -Wl,--build-id=none
> +ELFFLAGS += -Wl,--no-dynamic-linker -Wl,-pie
> ELFFLAGS += $(platform-ldflags-y)
> ELFFLAGS += $(firmware-ldflags-y)
> 
> diff --git a/README.md b/README.md
> index 73de8ea..7a24801 100644
> --- a/README.md
> +++ b/README.md
> @@ -274,11 +274,6 @@ make CC=riscv64-linux-gnu-gcc LLVM=1
> These variables must be passed for all the make invocations described in this
> document.
> 
> -NOTE: Using Clang with a `riscv*-linux-gnu` GNU binutils linker has been seen
> -to produce broken binaries with missing relocations; it is therefore currently
> -recommended that this combination be avoided or *FW_PIC=n* be used to disable
> -building OpenSBI as a position-independent binary.
> -

The first part of this is still true.

Jess

> Building with timestamp and compiler info
> -----------------------------------------
> 
> diff --git a/docs/firmware/fw.md b/docs/firmware/fw.md
> index 38351c8..2f4deb5 100644
> --- a/docs/firmware/fw.md
> +++ b/docs/firmware/fw.md
> @@ -69,12 +69,6 @@ parameters:
>   argument by the prior booting stage.
> * **FW_FDT_PADDING** - Optional zero bytes padding to the embedded flattened
>   device tree binary file specified by **FW_FDT_PATH** option.
> -* **FW_PIC** - "FW_PIC=y" generates position independent executable firmware
> -  images. OpenSBI can run at arbitrary address with appropriate alignment.
> -  Therefore, the original relocation mechanism ("FW_PIC=n") will be skipped.
> -  In other words, OpenSBI will directly run at the load address without any
> -  code movement. This option requires a toolchain with PIE support, and it
> -  is on by default.
> 
> Additionally, each firmware type as a set of type specific configuration
> parameters. Detailed information for each firmware type can be found in the
> diff --git a/firmware/fw_base.S b/firmware/fw_base.S
> index 126b067..6013db3 100644
> --- a/firmware/fw_base.S
> +++ b/firmware/fw_base.S
> @@ -15,8 +15,7 @@
> #include <sbi/sbi_trap.h>
> 
> #define BOOT_STATUS_LOTTERY_DONE 1
> -#define BOOT_STATUS_RELOCATE_DONE 2
> -#define BOOT_STATUS_BOOT_HART_DONE 3
> +#define BOOT_STATUS_BOOT_HART_DONE 2
> 
> .macro MOV_3R __d0, __s0, __d1, __s1, __d2, __s2
> add \__d0, \__s0, zero
> @@ -32,17 +31,6 @@
> add \__d4, \__s4, zero
> .endm
> 
> -/*
> - * If __start_reg <= __check_reg and __check_reg < __end_reg then
> - *   jump to __pass
> - */
> -.macro BRANGE __start_reg, __end_reg, __check_reg, __jump_lable
> - blt \__check_reg, \__start_reg, 999f
> - bge \__check_reg, \__end_reg, 999f
> - j \__jump_lable
> -999:
> -.endm
> -
> .section .entry, "ax", %progbits
> .align 3
> .globl _start
> @@ -56,15 +44,14 @@ _start:
> li a7, -1
> beq a6, a7, _try_lottery
> /* Jump to relocation wait loop if we are not boot hart */
> - bne a0, a6, _wait_relocate_copy_done
> + bne a0, a6, _wait_for_boot_hart
> _try_lottery:
> /* Jump to relocation wait loop if we don't get relocation lottery */
> lla a6, _boot_status
> li a7, BOOT_STATUS_LOTTERY_DONE
> amoswap.w a6, a7, (a6)
> - bnez a6, _wait_relocate_copy_done
> + bnez a6, _wait_for_boot_hart
> 
> -#ifdef FW_PIC
> /* relocate the global table content */
> li t0, FW_TEXT_START /* link start */
> lla t1, _fw_start /* load start */
> @@ -85,86 +72,7 @@ _try_lottery:
> 3:
> addi t0, t0, (REGBYTES * 3)
> blt t0, t1, 2b
> - j _relocate_done
> -_wait_relocate_copy_done:
> - j _wait_for_boot_hart
> -#else
> - /* Relocate if load address != link address */
> -_relocate:
> - li t0, FW_TEXT_START /* link start */
> - lla t2, _fw_start /* load start */
> - lla t3, _fw_reloc_end /* load end */
> - sub t6, t2, t0 /* load offset */
> - sub t1, t3, t6 /* link end */
> - beq t0, t2, _relocate_done
> - lla t4, _relocate_done
> - sub t4, t4, t6
> - blt t2, t0, _relocate_copy_to_upper
> -_relocate_copy_to_lower:
> - ble t1, t2, _relocate_copy_to_lower_loop
> - lla t3, _boot_status
> - BRANGE t2, t1, t3, _start_hang
> - lla t3, _relocate
> - lla t5, _relocate_done
> - BRANGE t2, t1, t3, _start_hang
> - BRANGE t2, t1, t5, _start_hang
> - BRANGE  t3, t5, t2, _start_hang
> -_relocate_copy_to_lower_loop:
> - REG_L t3, 0(t2)
> - REG_S t3, 0(t0)
> - add t0, t0, __SIZEOF_POINTER__
> - add t2, t2, __SIZEOF_POINTER__
> - blt t0, t1, _relocate_copy_to_lower_loop
> - jr t4
> -_relocate_copy_to_upper:
> - ble t3, t0, _relocate_copy_to_upper_loop
> - lla t2, _boot_status
> - BRANGE t0, t3, t2, _start_hang
> - lla t2, _relocate
> - lla t5, _relocate_done
> - BRANGE t0, t3, t2, _start_hang
> - BRANGE t0, t3, t5, _start_hang
> - BRANGE t2, t5, t0, _start_hang
> -_relocate_copy_to_upper_loop:
> - add t3, t3, -__SIZEOF_POINTER__
> - add t1, t1, -__SIZEOF_POINTER__
> - REG_L t2, 0(t3)
> - REG_S t2, 0(t1)
> - blt t0, t1, _relocate_copy_to_upper_loop
> - jr t4
> -_wait_relocate_copy_done:
> - lla t0, _fw_start
> - li t1, FW_TEXT_START
> - beq t0, t1, _wait_for_boot_hart
> - lla t2, _boot_status
> - lla t3, _wait_for_boot_hart
> - sub t3, t3, t0
> - add t3, t3, t1
> -1:
> - /* waitting for relocate copy done (_boot_status == 1) */
> - li t4, BOOT_STATUS_RELOCATE_DONE
> - REG_L t5, 0(t2)
> - /* Reduce the bus traffic so that boot hart may proceed faster */
> - nop
> - nop
> - nop
> - bgt     t4, t5, 1b
> - jr t3
> -#endif
> _relocate_done:
> -
> - /*
> - * Mark relocate copy done
> - * Use _boot_status copy relative to the load address
> - */
> - lla t0, _boot_status
> -#ifndef FW_PIC
> - add t0, t0, t6
> -#endif
> - li t1, BOOT_STATUS_RELOCATE_DONE
> - REG_S t1, 0(t0)
> - fence rw, rw
> -
> /* At this point we are running from link address */
> 
> /* Reset all registers except ra, a0, a1, a2, a3 and a4 for boot HART */
> diff --git a/firmware/objects.mk b/firmware/objects.mk
> index 3ae0a28..e6b364b 100644
> --- a/firmware/objects.mk
> +++ b/firmware/objects.mk
> @@ -13,17 +13,6 @@ firmware-cflags-y +=
> firmware-asflags-y +=
> firmware-ldflags-y +=
> 
> -ifndef FW_PIC
> -FW_PIC := $(OPENSBI_LD_PIE)
> -endif
> -
> -ifeq ($(FW_PIC),y)
> -firmware-genflags-y += -DFW_PIC
> -firmware-asflags-y  += -fpic
> -firmware-cflags-y   += -fPIE -pie
> -firmware-ldflags-y  += -Wl,--no-dynamic-linker -Wl,-pie
> -endif
> -
> ifdef FW_TEXT_START
> firmware-genflags-y += -DFW_TEXT_START=$(FW_TEXT_START)
> endif
> -- 
> 2.43.0
> 
> 
> -- 
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi