On Tuesday 22 November 2022 12:31:56 Tom Rini wrote:
It is a bad idea, and more modern toolchains will fail, if you declare an assembly function to be global and then weak, instead of declaring it weak to start with. Update assorted assembly files to use the WEAK macro directly.
Signed-off-by: Tom Rini trini@konsulko.com
During debugging of Nokia N900 code I was looking at this and I originally thought that this redefinition is the issue why N900 u-boot did not work... (but as we now know, the n900 issue was somewhere else).
So I agree with this change, feel free to add my:
Reviewed-by: Pali Rohár pali@kernel.org
... but even after this change, linked u-boot.bin binary is not-so-correct. It works but has an issue: In final u-boot.bin binary there is both weak and non-weak version of every weak function. You can verify it for example by looking at "save_boot_params" code (really code, not just symbol) in u-boot ELF binary.
The reason for this is that linker (even LTO enabled) cannot eliminate code for weak version of function because it does not know how to "drop" it from binary/assembly code. So linker just set that non-weak version of function is active and let non-weak version present in binary as probably dead code.
This is affected only by assembly files, not by C files, because gcc is called with -ffunction-sections -fdata-sections flags which cause that every (weak) function is in its separate section (so C function "int abc() { ... }" is put into the section ".text.abc" instead of ".text") and linker's flag --gc-sections (or LTO optimization) then drop all unreferenced sections.
I do not know how fix this issue in assembly files. But cannot be WEAK macro modified to change section to ".text.<entry_name>" to mimic C compiler behavior? Would this cause any issues?
arch/arm/cpu/armv7/nonsec_virt.S | 3 +- arch/arm/cpu/armv7/psci.S | 77 +++++++++++--------------------- arch/arm/cpu/armv7/start.S | 6 +-- arch/arm/cpu/armv8/psci.S | 12 ++--- arch/arm/lib/relocate.S | 3 +- 5 files changed, 34 insertions(+), 67 deletions(-)
diff --git a/arch/arm/cpu/armv7/nonsec_virt.S b/arch/arm/cpu/armv7/nonsec_virt.S index 39aeeb423f08..5ee6577c2c3c 100644 --- a/arch/arm/cpu/armv7/nonsec_virt.S +++ b/arch/arm/cpu/armv7/nonsec_virt.S @@ -207,7 +207,7 @@ ENDPROC(_nonsec_init)
#ifdef CONFIG_SMP_PEN_ADDR /* void __weak smp_waitloop(unsigned previous_address); */ -ENTRY(smp_waitloop) +WEAK(smp_waitloop) wfi ldr r1, =CONFIG_SMP_PEN_ADDR @ load start address ldr r1, [r1] @@ -219,7 +219,6 @@ ENTRY(smp_waitloop) mov r0, r1 b _do_nonsec_entry ENDPROC(smp_waitloop) -.weak smp_waitloop #endif
.popsection diff --git a/arch/arm/cpu/armv7/psci.S b/arch/arm/cpu/armv7/psci.S index 983cd9044295..6c066e50d91e 100644 --- a/arch/arm/cpu/armv7/psci.S +++ b/arch/arm/cpu/armv7/psci.S @@ -36,34 +36,32 @@ _psci_vectors: b default_psci_vector @ irq b psci_fiq_enter @ fiq
-ENTRY(psci_fiq_enter) +WEAK(psci_fiq_enter) movs pc, lr ENDPROC(psci_fiq_enter) -.weak psci_fiq_enter
-ENTRY(default_psci_vector) +WEAK(default_psci_vector) movs pc, lr ENDPROC(default_psci_vector) -.weak default_psci_vector
-ENTRY(psci_version) -ENTRY(psci_cpu_suspend) -ENTRY(psci_cpu_off) -ENTRY(psci_cpu_on) -ENTRY(psci_affinity_info) -ENTRY(psci_migrate) -ENTRY(psci_migrate_info_type) -ENTRY(psci_migrate_info_up_cpu) -ENTRY(psci_system_off) -ENTRY(psci_system_reset) -ENTRY(psci_features) -ENTRY(psci_cpu_freeze) -ENTRY(psci_cpu_default_suspend) -ENTRY(psci_node_hw_state) -ENTRY(psci_system_suspend) -ENTRY(psci_set_suspend_mode) -ENTRY(psi_stat_residency) -ENTRY(psci_stat_count)
+WEAK(psci_version) +WEAK(psci_cpu_suspend) +WEAK(psci_cpu_off) +WEAK(psci_cpu_on) +WEAK(psci_affinity_info) +WEAK(psci_migrate) +WEAK(psci_migrate_info_type) +WEAK(psci_migrate_info_up_cpu) +WEAK(psci_system_off) +WEAK(psci_system_reset) +WEAK(psci_features) +WEAK(psci_cpu_freeze) +WEAK(psci_cpu_default_suspend) +WEAK(psci_node_hw_state) +WEAK(psci_system_suspend) +WEAK(psci_set_suspend_mode) +WEAK(psi_stat_residency) +WEAK(psci_stat_count) mov r0, #ARM_PSCI_RET_NI @ Return -1 (Not Implemented) mov pc, lr ENDPROC(psci_stat_count) @@ -84,24 +82,6 @@ ENDPROC(psci_cpu_on) ENDPROC(psci_cpu_off) ENDPROC(psci_cpu_suspend) ENDPROC(psci_version) -.weak psci_version -.weak psci_cpu_suspend -.weak psci_cpu_off -.weak psci_cpu_on -.weak psci_affinity_info -.weak psci_migrate -.weak psci_migrate_info_type -.weak psci_migrate_info_up_cpu -.weak psci_system_off -.weak psci_system_reset -.weak psci_features -.weak psci_cpu_freeze -.weak psci_cpu_default_suspend -.weak psci_node_hw_state -.weak psci_system_suspend -.weak psci_set_suspend_mode -.weak psi_stat_residency -.weak psci_stat_count
_psci_table: .word ARM_PSCI_FN_CPU_SUSPEND @@ -179,12 +159,11 @@ _smc_psci: movs pc, lr @ Return to the kernel
@ Requires dense and single-cluster CPU ID space -ENTRY(psci_get_cpu_id) +WEAK(psci_get_cpu_id) mrc p15, 0, r0, c0, c0, 5 /* read MPIDR */ and r0, r0, #0xff /* return CPU ID in cluster */ bx lr ENDPROC(psci_get_cpu_id) -.weak psci_get_cpu_id
/* Imported from Linux kernel */ ENTRY(psci_v7_flush_dcache_all) @@ -236,7 +215,7 @@ finished: bx lr ENDPROC(psci_v7_flush_dcache_all)
-ENTRY(psci_disable_smp) +WEAK(psci_disable_smp) mrc p15, 0, r0, c1, c0, 1 @ ACTLR bic r0, r0, #(1 << 6) @ Clear SMP bit mcr p15, 0, r0, c1, c0, 1 @ ACTLR @@ -244,16 +223,14 @@ ENTRY(psci_disable_smp) dsb bx lr ENDPROC(psci_disable_smp) -.weak psci_disable_smp
-ENTRY(psci_enable_smp) +WEAK(psci_enable_smp) mrc p15, 0, r0, c1, c0, 1 @ ACTLR orr r0, r0, #(1 << 6) @ Set SMP bit mcr p15, 0, r0, c1, c0, 1 @ ACTLR isb bx lr ENDPROC(psci_enable_smp) -.weak psci_enable_smp
ENTRY(psci_cpu_off_common) push {lr} @@ -316,15 +293,13 @@ ENTRY(psci_stack_setup) bx r6 ENDPROC(psci_stack_setup)
-ENTRY(psci_arch_init) +WEAK(psci_arch_init) mov pc, lr ENDPROC(psci_arch_init) -.weak psci_arch_init
-ENTRY(psci_arch_cpu_entry) +WEAK(psci_arch_cpu_entry) mov pc, lr ENDPROC(psci_arch_cpu_entry) -.weak psci_arch_cpu_entry
ENTRY(psci_cpu_entry) bl psci_enable_smp diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S index 4f6327fe3ab7..7d7aac021e22 100644 --- a/arch/arm/cpu/armv7/start.S +++ b/arch/arm/cpu/armv7/start.S @@ -151,16 +151,14 @@ ENDPROC(c_runtime_cpu_setup)
- Don't save anything to stack even if compiled with -O0
*************************************************************************/ -ENTRY(save_boot_params) +WEAK(save_boot_params) b save_boot_params_ret @ back to my caller ENDPROC(save_boot_params)
- .weak save_boot_params
#ifdef CONFIG_ARMV7_LPAE -ENTRY(switch_to_hypervisor) +WEAK(switch_to_hypervisor) b switch_to_hypervisor_ret ENDPROC(switch_to_hypervisor)
- .weak switch_to_hypervisor
#endif
/************************************************************************* diff --git a/arch/arm/cpu/armv8/psci.S b/arch/arm/cpu/armv8/psci.S index 7ffc8dbadbe0..6aece1198712 100644 --- a/arch/arm/cpu/armv8/psci.S +++ b/arch/arm/cpu/armv8/psci.S @@ -12,11 +12,10 @@
/* Default PSCI function, return -1, Not Implemented */ #define PSCI_DEFAULT(__fn) \
- ENTRY(__fn); \
- WEAK(__fn); \ mov w0, #ARM_PSCI_RET_NI; \ ret; \ ENDPROC(__fn); \
- .weak __fn
/* PSCI function and ID table definition*/ #define PSCI_TABLE(__id, __fn) \ @@ -207,7 +206,7 @@ handle_smc64:
- used for the return value, while in this PSCI environment, X0 usually holds
- the SMC function identifier, so X0 should be saved by caller function.
*/ -ENTRY(psci_get_cpu_id) +WEAK(psci_get_cpu_id) #ifdef CONFIG_ARMV8_PSCI_CPUS_PER_CLUSTER mrs x9, MPIDR_EL1 ubfx x9, x9, #8, #8 @@ -221,7 +220,6 @@ ENTRY(psci_get_cpu_id) add x0, x10, x9 ret ENDPROC(psci_get_cpu_id) -.weak psci_get_cpu_id
/* CPU ID input in x0, stack top output in x0*/ LENTRY(psci_get_cpu_stack_top) @@ -261,10 +259,9 @@ handle_sync:
- Override this function if custom error handling is
- needed for asynchronous aborts
*/ -ENTRY(plat_error_handler) +WEAK(plat_error_handler) ret ENDPROC(plat_error_handler) -.weak plat_error_handler
handle_error: bl psci_get_cpu_id @@ -323,9 +320,8 @@ ENTRY(psci_setup_vectors) ret ENDPROC(psci_setup_vectors)
-ENTRY(psci_arch_init) +WEAK(psci_arch_init) ret ENDPROC(psci_arch_init) -.weak psci_arch_init
.popsection diff --git a/arch/arm/lib/relocate.S b/arch/arm/lib/relocate.S index dd6f2e3bd5e0..345e282e3e65 100644 --- a/arch/arm/lib/relocate.S +++ b/arch/arm/lib/relocate.S @@ -23,9 +23,8 @@ */
.section .text.relocate_vectors,"ax",%progbits
- .weak relocate_vectors
-ENTRY(relocate_vectors) +WEAK(relocate_vectors)
#ifdef CONFIG_CPU_V7M /* -- 2.25.1