Hi all,
I've sent a few patches for fixing some compilation warnings or issues on Rockchip platforms [1][2][3][4][5] but I'm left with a few still...
[1] https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/33080 [2] https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/33081 [3] https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/33082 [4] https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/33083 [5] https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/33084
1) RK3588 symbol redefinition in assembly
======================================================================== commit fc3b98b2124c2b773322da0201e9ad7a697e6323 Author: Quentin Schulz quentin.schulz@cherry.de Date: Mon Oct 28 12:40:18 2024 +0100
fix(rk3588): pmu: fix assembly symbol redefinition
Somehow cpus_pd_req_enter_wfi() gets called multiple times and clang isn't happy about redefining the same label multiple times (it is an inline function).
An option could be to force the code to not be inlined (with __attribute__((noinline))) as removing the explicit inline still made the compiler inline the code.
Recreate the while loop without a label to fix the clang build issue: plat/rockchip/rk3588/drivers/pmu/pmu.c:763:7: error: symbol 'wfi_loop' is already defined 763 | "wfi_loop:\n" | ^ <inline asm>:5:1: note: instantiated into assembly here 5 | wfi_loop: | ^
Signed-off-by: Quentin Schulz quentin.schulz@cherry.de Change-Id: Ie9f55135b2f95a78deb7cbb94f9a62d3ba61e808
diff --git a/plat/rockchip/rk3588/drivers/pmu/pmu.c b/plat/rockchip/rk3588/drivers/pmu/pmu.c index f693dbdf0..f05d3e58f 100644 --- a/plat/rockchip/rk3588/drivers/pmu/pmu.c +++ b/plat/rockchip/rk3588/drivers/pmu/pmu.c @@ -760,10 +760,10 @@ static inline void cpus_pd_req_enter_wfi(void) "mrs x0, S3_0_C15_C2_7\n" "orr x0, x0, #0x1\n" "msr S3_0_C15_C2_7, x0\n" - "wfi_loop:\n" + "ldr x0, .+4\n" "isb\n" "wfi\n" - "b wfi_loop\n"); + "br x0\n"); }
static void nonboot_cpus_off(void) =========================================================================
This only happens for clang and I'm a bit confused as to why. Applying the above patch fixing that issue but inspecting the assembly code reveals something really odd.
**With** that patch, with gcc: 25c8c: d510149f msr dbgprcr_el1, xzr 25c90: d538f2e0 mrs x0, s3_0_c15_c2_7 25c94: b2400000 orr x0, x0, #0x1 25c98: d518f2e0 msr s3_0_c15_c2_7, x0 25c9c: 58000020 ldr x0, 0x25ca0 25ca0: d5033fdf isb 25ca4: d503207f wfi 25ca8: d61f0000 br x0
**with** that patch, with clang: 26d2c: d510149f msr dbgprcr_el1, xzr 26d30: d538f2e0 mrs x0, s3_0_c15_c2_7 26d34: b2400000 orr x0, x0, #0x1 26d38: d518f2e0 msr s3_0_c15_c2_7, x0 26d3c: 58000020 ldr x0, 0x26d40 <--- "my" ldr 26d40: d5033fdf isb 26d44: d503207f wfi 26d48: d61f0000 br x0 <--- "my" br 26d4c: 94000e76 bl 0x2a724 26d50: d510149f msr dbgprcr_el1, xzr 26d54: d538f2e0 mrs x0, s3_0_c15_c2_7 26d58: b2400000 orr x0, x0, #0x1 26d5c: d518f2e0 msr s3_0_c15_c2_7, x0 26d60: 58000020 ldr x0, 0x26d64 <--- "my" ldr 26d64: d5033fdf isb 26d68: d503207f wfi 26d6c: d61f0000 br x0 <--- "my" br
So I think the above patch is just a band-aid on what the actually is but I have no idea what could be going on.
If I use this patch instead, to match the other assembly instructions in this file
========================================================================= diff --git a/plat/rockchip/rk3588/drivers/pmu/pmu.c b/plat/rockchip/rk3588/drivers/pmu/pmu.c index f693dbdf0..a4128b214 100644 --- a/plat/rockchip/rk3588/drivers/pmu/pmu.c +++ b/plat/rockchip/rk3588/drivers/pmu/pmu.c @@ -760,10 +760,10 @@ static inline void cpus_pd_req_enter_wfi(void) "mrs x0, S3_0_C15_C2_7\n" "orr x0, x0, #0x1\n" "msr S3_0_C15_C2_7, x0\n" - "wfi_loop:\n" + "1:\n" "isb\n" "wfi\n" - "b wfi_loop\n"); + "b 1b\n"); }
static void nonboot_cpus_off(void) =========================================================================
It happily compiles, though the binary still seems to contain this loop twice.
Using __attribute__((noinline)) the code is not duplicated, but I assume this is expected as it would now simply be a function.
2) RK3399 warning array subscript 0 is outside array bounds of 'volatile unsigned int[0]' [-Warray-bounds=]
"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""" In file included from src/dram.c:12: src/dram.c: In function 'm0_main': include/rk3399_mcu.h:15:34: warning: array subscript 0 is outside array bounds of 'volatile unsigned int[0]' [-Warray-bounds=] 15 | (*(volatile unsigned int *)(c)); __v; }) | ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/rk3399_mcu.h:16:69: note: in definition of macro 'mmio_write_32' 16 | #define mmio_write_32(c, v) ((*(volatile unsigned int *)(c)) = (v)) | ^ src/dram.c:67:23: note: in expansion of macro 'mmio_read_32' 67 | mmio_read_32(PARAM_ADDR + PARAM_FREQ_SELECT)); | ^~~~~~~~~~~~ cc1: note: source object is likely at address zero In function 'ddr_set_pll', inlined from 'm0_main' at src/dram.c:71:2: include/rk3399_mcu.h:14:40: warning: array subscript 0 is outside array bounds of 'volatile unsigned int[0]' [-Warray-bounds=] 14 | #define mmio_read_32(c) ({unsigned int __v = \ | ^~~ include/rk3399_mcu.h:16:69: note: in definition of macro 'mmio_write_32' 16 | #define mmio_write_32(c, v) ((*(volatile unsigned int *)(c)) = (v)) | ^ src/dram.c:47:23: note: in expansion of macro 'mmio_read_32' 47 | mmio_read_32(PARAM_ADDR + PARAM_DPLL_CON0)); | ^~~~~~~~~~~~ In function 'm0_main': cc1: note: source object is likely at address zero In function 'ddr_set_pll', inlined from 'm0_main' at src/dram.c:71:2: include/rk3399_mcu.h:14:40: warning: array subscript 0 is outside array bounds of 'volatile unsigned int[0]' [-Warray-bounds=] 14 | #define mmio_read_32(c) ({unsigned int __v = \ | ^~~ include/rk3399_mcu.h:16:69: note: in definition of macro 'mmio_write_32' 16 | #define mmio_write_32(c, v) ((*(volatile unsigned int *)(c)) = (v)) | ^ src/dram.c:49:23: note: in expansion of macro 'mmio_read_32' 49 | mmio_read_32(PARAM_ADDR + PARAM_DPLL_CON1)); | ^~~~~~~~~~~~ In function 'm0_main': cc1: note: source object is likely at address zero include/rk3399_mcu.h:16:35: warning: array subscript 0 is outside array bounds of 'volatile unsigned int[0]' [-Warray-bounds=] 16 | #define mmio_write_32(c, v) ((*(volatile unsigned int *)(c)) = (v)) | ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ src/dram.c:80:9: note: in expansion of macro 'mmio_write_32' 80 | mmio_write_32(PARAM_ADDR + PARAM_M0_DONE, M0_DONE_FLAG); | ^~~~~~~~~~~~~ cc1: note: source object is likely at address zero """""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
This function is used in many places, but only in very specific places is it triggering a warning, when PARAM_ADDR is used, but only when accessed from m0 C files.
PARAM_ADDR is set to 0xc0. From !m0 files, access to PARAM_ADDR seems to be done through the M0_PARAM_ADDR which is M0_BINCODE_BASE + PARAM_ADDR. For m0 files, it's directly accessing PARAM_ADDR. I honestly don't really know why it's complaining for m0 files but not the !m0 ones.
3) RK3399 expected string in '.incbin' directive
This is only for clang.
""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""" <instantiation>:6:10: error: expected string in '.incbin' directive .incbin /home/qschulz/work/upstream/trusted-firmware-a/build/rk3399/release/m0/rk3399m0.bin ^ plat/rockchip/rk3399/drivers/pmu/pmu_fw.S:20:1: note: while in macro instantiation INCBIN """/home/qschulz/work/upstream/trusted-firmware-a/build/rk3399/release/m0/rk3399m0.bin""", "rk3399m0_bin", ".sram.incbin" ^ <instantiation>:6:10: error: expected string in '.incbin' directive .incbin /home/qschulz/work/upstream/trusted-firmware-a/build/rk3399/release/m0/rk3399m0pmu.bin ^ plat/rockchip/rk3399/drivers/pmu/pmu_fw.S:21:1: note: while in macro instantiation INCBIN """/home/qschulz/work/upstream/trusted-firmware-a/build/rk3399/release/m0/rk3399m0pmu.bin""", "rk3399m0pmu_bin", ".pmusram.incbin" ^ """""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
With the following patch:
========================================================================= diff --git a/plat/rockchip/rk3399/drivers/pmu/pmu_fw.S b/plat/rockchip/rk3399/drivers/pmu/pmu_fw.S index 26f331317..ed70cba1f 100644 --- a/plat/rockchip/rk3399/drivers/pmu/pmu_fw.S +++ b/plat/rockchip/rk3399/drivers/pmu/pmu_fw.S @@ -11,7 +11,7 @@ .type \sym, @object .align 4 \sym : - .incbin \file + .incbin "\file" .size \sym , .-\sym .global \sym()_end \sym()_end : =========================================================================
clang seems happy with that error but the linker triggers another one:
ld.lld: warning: ignoring memory region assignment for non-allocatable section '.incbin_sram' ld.lld: error: section '.pmusram' will not fit in region 'PMUSRAM': overflowed by 3928 bytes
Would appreciate some guidance there :)
Cheers, Quentin
Hi Quentin,
I dug around a bit and I think I have answers for most.
- RK3588 symbol redefinition in assembly
This only happens for clang and I'm a bit confused as to why. Applying the above patch fixing that issue but inspecting the assembly code reveals something really odd.
**With** that patch, with gcc: 25c8c: d510149f msr dbgprcr_el1, xzr 25c90: d538f2e0 mrs x0, s3_0_c15_c2_7 25c94: b2400000 orr x0, x0, #0x1 25c98: d518f2e0 msr s3_0_c15_c2_7, x0 25c9c: 58000020 ldr x0, 0x25ca0 25ca0: d5033fdf isb 25ca4: d503207f wfi 25ca8: d61f0000 br x0
**with** that patch, with clang: 26d2c: d510149f msr dbgprcr_el1, xzr 26d30: d538f2e0 mrs x0, s3_0_c15_c2_7 26d34: b2400000 orr x0, x0, #0x1 26d38: d518f2e0 msr s3_0_c15_c2_7, x0 26d3c: 58000020 ldr x0, 0x26d40 <--- "my" ldr 26d40: d5033fdf isb 26d44: d503207f wfi 26d48: d61f0000 br x0 <--- "my" br 26d4c: 94000e76 bl 0x2a724 26d50: d510149f msr dbgprcr_el1, xzr 26d54: d538f2e0 mrs x0, s3_0_c15_c2_7 26d58: b2400000 orr x0, x0, #0x1 26d5c: d518f2e0 msr s3_0_c15_c2_7, x0 26d60: 58000020 ldr x0, 0x26d64 <--- "my" ldr 26d64: d5033fdf isb 26d68: d503207f wfi 26d6c: d61f0000 br x0 <--- "my" br
So I think the above patch is just a band-aid on what the actually is but I have no idea what could be going on.
If I use this patch instead, to match the other assembly instructions in this file
========================================================================= diff --git a/plat/rockchip/rk3588/drivers/pmu/pmu.c b/plat/rockchip/rk3588/drivers/pmu/pmu.c index f693dbdf0..a4128b214 100644 --- a/plat/rockchip/rk3588/drivers/pmu/pmu.c +++ b/plat/rockchip/rk3588/drivers/pmu/pmu.c @@ -760,10 +760,10 @@ static inline void cpus_pd_req_enter_wfi(void) "mrs x0, S3_0_C15_C2_7\n" "orr x0, x0, #0x1\n" "msr S3_0_C15_C2_7, x0\n" - "wfi_loop:\n" + "1:\n" "isb\n" "wfi\n" - "b wfi_loop\n"); + "b 1b\n"); }
static void nonboot_cpus_off(void)
It happily compiles, though the binary still seems to contain this loop twice.
Using __attribute__((noinline)) the code is not duplicated, but I assume this is expected as it would now simply be a function.
So, the function only runs once, but it has its address taken elsewhere. For the actual call, it does get inlined, as expected. But the address taking bit, it is kept as a standalone function which happens to be places right after. Disassembling with symbols shows this a bit better. My guess would be that clang either isn't smart enough to deduplicate this, or it's doing it on purpose. Gcc must decide to not inline.
- RK3399 warning array subscript 0 is outside array bounds of 'volatile unsigned int[0]' [-Warray-bounds=]
"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""" In file included from src/dram.c:12: src/dram.c: In function 'm0_main': include/rk3399_mcu.h:15:34: warning: array subscript 0 is outside array bounds of 'volatile unsigned int[0]' [-Warray-bounds=] 15 | (*(volatile unsigned int *)(c)); __v; }) | ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/rk3399_mcu.h:16:69: note: in definition of macro 'mmio_write_32' 16 | #define mmio_write_32(c, v) ((*(volatile unsigned int *)(c)) = (v)) | ^ src/dram.c:67:23: note: in expansion of macro 'mmio_read_32' 67 | mmio_read_32(PARAM_ADDR + PARAM_FREQ_SELECT)); | ^~~~~~~~~~~~
This function is used in many places, but only in very specific places is it triggering a warning, when PARAM_ADDR is used, but only when accessed from m0 C files.
PARAM_ADDR is set to 0xc0. From !m0 files, access to PARAM_ADDR seems to be done through the M0_PARAM_ADDR which is M0_BINCODE_BASE + PARAM_ADDR. For m0 files, it's directly accessing PARAM_ADDR. I honestly don't really know why it's complaining for m0 files but not the !m0 ones.
This seems identical to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105523. The top-level makefile has a workaround for this and it sets --param=min-pagesize=0. However, this driver has a very peculiar way of building and it does not inherit most of tf-a's flags. Adding that flag to its own internal makefile fixes the issue, but I feel like it's bound to repeat. Can't it be built by the standard tfa build?
- RK3399 expected string in '.incbin' directive
clang seems happy with that error but the linker triggers another one:
ld.lld: warning: ignoring memory region assignment for non-allocatable section '.incbin_sram' ld.lld: error: section '.pmusram' will not fit in region 'PMUSRAM': overflowed by 3928 bytes
The overflow error seems legitimate, increasing PMUSRAM_RSIZE at the top of the linker script fixes it. There's funky alignment in the rule so maybe that's wasting space? I'm sadly not familiar enough with linkers and don't understand the last warning though, hopefully someone else can figure that one out.
Cheers, Boyan
Hi Boyan,
On 10/29/24 8:28 PM, Boyan Karatotev wrote:
[You don't often get email from boyan.karatotev@arm.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
Hi Quentin,
I dug around a bit and I think I have answers for most.
Thanks for looking into this :)
- RK3588 symbol redefinition in assembly
This only happens for clang and I'm a bit confused as to why. Applying the above patch fixing that issue but inspecting the assembly code reveals something really odd.
**With** that patch, with gcc: 25c8c: d510149f msr dbgprcr_el1, xzr 25c90: d538f2e0 mrs x0, s3_0_c15_c2_7 25c94: b2400000 orr x0, x0, #0x1 25c98: d518f2e0 msr s3_0_c15_c2_7, x0 25c9c: 58000020 ldr x0, 0x25ca0 25ca0: d5033fdf isb 25ca4: d503207f wfi 25ca8: d61f0000 br x0
**with** that patch, with clang: 26d2c: d510149f msr dbgprcr_el1, xzr 26d30: d538f2e0 mrs x0, s3_0_c15_c2_7 26d34: b2400000 orr x0, x0, #0x1 26d38: d518f2e0 msr s3_0_c15_c2_7, x0 26d3c: 58000020 ldr x0, 0x26d40 <--- "my" ldr 26d40: d5033fdf isb 26d44: d503207f wfi 26d48: d61f0000 br x0 <--- "my" br 26d4c: 94000e76 bl 0x2a724 26d50: d510149f msr dbgprcr_el1, xzr 26d54: d538f2e0 mrs x0, s3_0_c15_c2_7 26d58: b2400000 orr x0, x0, #0x1 26d5c: d518f2e0 msr s3_0_c15_c2_7, x0 26d60: 58000020 ldr x0, 0x26d64 <--- "my" ldr 26d64: d5033fdf isb 26d68: d503207f wfi 26d6c: d61f0000 br x0 <--- "my" br
So I think the above patch is just a band-aid on what the actually is but I have no idea what could be going on.
If I use this patch instead, to match the other assembly instructions in this file
========================================================================= diff --git a/plat/rockchip/rk3588/drivers/pmu/pmu.c b/plat/rockchip/rk3588/drivers/pmu/pmu.c index f693dbdf0..a4128b214 100644 --- a/plat/rockchip/rk3588/drivers/pmu/pmu.c +++ b/plat/rockchip/rk3588/drivers/pmu/pmu.c @@ -760,10 +760,10 @@ static inline void cpus_pd_req_enter_wfi(void) "mrs x0, S3_0_C15_C2_7\n" "orr x0, x0, #0x1\n" "msr S3_0_C15_C2_7, x0\n"
"wfi_loop:\n"
"1:\n" "isb\n" "wfi\n"
"b wfi_loop\n");
"b 1b\n");}
static void nonboot_cpus_off(void)
=========================================================================
It happily compiles, though the binary still seems to contain this loop twice.
Using __attribute__((noinline)) the code is not duplicated, but I assume this is expected as it would now simply be a function.
So, the function only runs once, but it has its address taken elsewhere. For the actual call, it does get inlined, as expected. But the address taking bit, it is kept as a standalone function which happens to be places right after. Disassembling with symbols shows this a bit better. My guess would be that clang either isn't smart enough to deduplicate this, or it's doing it on purpose. Gcc must decide to not inline.
But the function currently has the inline marker, doesn't it force the compiler to inline it?
Are you suggesting anything we could do here?
Would any of the three above suggestions be reasonably fit for upstreaming?
- RK3399 warning array subscript 0 is outside array bounds of 'volatile unsigned int[0]' [-Warray-bounds=]
"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""" In file included from src/dram.c:12: src/dram.c: In function 'm0_main': include/rk3399_mcu.h:15:34: warning: array subscript 0 is outside array bounds of 'volatile unsigned int[0]' [-Warray-bounds=] 15 | (*(volatile unsigned int *)(c)); __v; }) | ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/rk3399_mcu.h:16:69: note: in definition of macro 'mmio_write_32' 16 | #define mmio_write_32(c, v) ((*(volatile unsigned int *)(c)) = (v)) | ^ src/dram.c:67:23: note: in expansion of macro 'mmio_read_32' 67 | mmio_read_32(PARAM_ADDR + PARAM_FREQ_SELECT)); | ^~~~~~~~~~~~
This function is used in many places, but only in very specific places is it triggering a warning, when PARAM_ADDR is used, but only when accessed from m0 C files.
PARAM_ADDR is set to 0xc0. From !m0 files, access to PARAM_ADDR seems to be done through the M0_PARAM_ADDR which is M0_BINCODE_BASE + PARAM_ADDR. For m0 files, it's directly accessing PARAM_ADDR. I honestly don't really know why it's complaining for m0 files but not the !m0 ones.
This seems identical to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105523. The top-level makefile has a workaround for this and it sets --param=min-pagesize=0. However, this driver has a very peculiar way of building and it does not inherit most of tf-a's flags. Adding that flag to its own internal makefile fixes the issue, but I feel like it's bound to repeat. Can't it be built by the standard tfa build?
The only thing I know about TF-A on RK3399 is that it is indeed peculiar :)
As far as I know, some part of the binary runs on the Cortex-A core(s) and another part on one of the Cortex-M0, hence why we need an additional toolchain and different drivers and all that.
Not sure if there exists any platform today in TF-A that has the same behavior and is "built by the standard tfa build"? How would one go forward with that?
- RK3399 expected string in '.incbin' directive
clang seems happy with that error but the linker triggers another one:
ld.lld: warning: ignoring memory region assignment for non-allocatable section '.incbin_sram' ld.lld: error: section '.pmusram' will not fit in region 'PMUSRAM': overflowed by 3928 bytes
The overflow error seems legitimate, increasing PMUSRAM_RSIZE at the top of the linker script fixes it. There's funky alignment in the rule so maybe that's wasting space? I'm sadly not familiar enough with linkers and don't understand the last warning though, hopefully someone else can figure that one out.
Maybe once we figure out how to build properly as suggested in 2) this will just go away since we're probably missing a bunch of flags anyway.
Cheers, Quentin
Hi Quentin,
[You don't often get email from boyan.karatotev@arm.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
Hi Quentin,
I dug around a bit and I think I have answers for most.
Thanks for looking into this :)
No worries!
- RK3588 symbol redefinition in assembly
This only happens for clang and I'm a bit confused as to why. Applying the above patch fixing that issue but inspecting the assembly code reveals something really odd.
**With** that patch, with gcc: 25c8c: d510149f msr dbgprcr_el1, xzr 25c90: d538f2e0 mrs x0, s3_0_c15_c2_7 25c94: b2400000 orr x0, x0, #0x1 25c98: d518f2e0 msr s3_0_c15_c2_7, x0 25c9c: 58000020 ldr x0, 0x25ca0 25ca0: d5033fdf isb 25ca4: d503207f wfi 25ca8: d61f0000 br x0
**with** that patch, with clang: 26d2c: d510149f msr dbgprcr_el1, xzr 26d30: d538f2e0 mrs x0, s3_0_c15_c2_7 26d34: b2400000 orr x0, x0, #0x1 26d38: d518f2e0 msr s3_0_c15_c2_7, x0 26d3c: 58000020 ldr x0, 0x26d40 <--- "my" ldr 26d40: d5033fdf isb 26d44: d503207f wfi 26d48: d61f0000 br x0 <--- "my" br 26d4c: 94000e76 bl 0x2a724 26d50: d510149f msr dbgprcr_el1, xzr 26d54: d538f2e0 mrs x0, s3_0_c15_c2_7 26d58: b2400000 orr x0, x0, #0x1 26d5c: d518f2e0 msr s3_0_c15_c2_7, x0 26d60: 58000020 ldr x0, 0x26d64 <--- "my" ldr 26d64: d5033fdf isb 26d68: d503207f wfi 26d6c: d61f0000 br x0 <--- "my" br
So I think the above patch is just a band-aid on what the actually is but I have no idea what could be going on.
If I use this patch instead, to match the other assembly instructions in this file
========================================================================= diff --git a/plat/rockchip/rk3588/drivers/pmu/pmu.c b/plat/rockchip/rk3588/drivers/pmu/pmu.c index f693dbdf0..a4128b214 100644 --- a/plat/rockchip/rk3588/drivers/pmu/pmu.c +++ b/plat/rockchip/rk3588/drivers/pmu/pmu.c @@ -760,10 +760,10 @@ static inline void cpus_pd_req_enter_wfi(void) "mrs x0, S3_0_C15_C2_7\n" "orr x0, x0, #0x1\n" "msr S3_0_C15_C2_7, x0\n" - "wfi_loop:\n" + "1:\n" "isb\n" "wfi\n" - "b wfi_loop\n"); + "b 1b\n"); }
static void nonboot_cpus_off(void)
It happily compiles, though the binary still seems to contain this loop twice.
Using __attribute__((noinline)) the code is not duplicated, but I assume this is expected as it would now simply be a function.
So, the function only runs once, but it has its address taken elsewhere. For the actual call, it does get inlined, as expected. But the address taking bit, it is kept as a standalone function which happens to be places right after. Disassembling with symbols shows this a bit better. My guess would be that clang either isn't smart enough to deduplicate this, or it's doing it on purpose. Gcc must decide to not inline.
But the function currently has the inline marker, doesn't it force the compiler to inline it?
Apparently, it doesn't have to: https://www.kernel.org/doc/local/inline.html
Are you suggesting anything we could do here?
Would any of the three above suggestions be reasonably fit for upstreaming?
Well the numbered labels should let it compile and we already merged another patch for that. We do have __always_inline, ALWAYS_INLINE, or just __attribute__((__always_inline__)) which should force the inlining. But if there are two uses of the function, I'd imagine the named label would produce similar errors.
- RK3399 warning array subscript 0 is outside array bounds of 'volatile unsigned int[0]' [-Warray-bounds=]
"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""" In file included from src/dram.c:12: src/dram.c: In function 'm0_main': include/rk3399_mcu.h:15:34: warning: array subscript 0 is outside array bounds of 'volatile unsigned int[0]' [-Warray-bounds=] 15 | (*(volatile unsigned int *)(c)); __v; }) | ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/rk3399_mcu.h:16:69: note: in definition of macro 'mmio_write_32' 16 | #define mmio_write_32(c, v) ((*(volatile unsigned int *)(c)) = (v)) | ^ src/dram.c:67:23: note: in expansion of macro 'mmio_read_32' 67 | mmio_read_32(PARAM_ADDR + PARAM_FREQ_SELECT)); | ^~~~~~~~~~~~
This function is used in many places, but only in very specific places is it triggering a warning, when PARAM_ADDR is used, but only when accessed from m0 C files.
PARAM_ADDR is set to 0xc0. From !m0 files, access to PARAM_ADDR seems to be done through the M0_PARAM_ADDR which is M0_BINCODE_BASE + PARAM_ADDR. For m0 files, it's directly accessing PARAM_ADDR. I honestly don't really know why it's complaining for m0 files but not the !m0 ones.
This seems identical to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105523. The top-level makefile has a workaround for this and it sets --param=min-pagesize=0. However, this driver has a very peculiar way of building and it does not inherit most of tf-a's flags. Adding that flag to its own internal makefile fixes the issue, but I feel like it's bound to repeat. Can't it be built by the standard tfa build?
The only thing I know about TF-A on RK3399 is that it is indeed peculiar :)
As far as I know, some part of the binary runs on the Cortex-A core(s) and another part on one of the Cortex-M0, hence why we need an additional toolchain and different drivers and all that.
Ah, I hadn't though of that.
Not sure if there exists any platform today in TF-A that has the same behavior and is "built by the standard tfa build"? How would one go forward with that?
You're right, there probably isn't. I was thinking more along the lines of using the make_helpers/build_macros.mk helpers but that probably won't work either. Would propagating the TF_FLAGS be possible? Chris should probably weigh in on this.
- RK3399 expected string in '.incbin' directive
clang seems happy with that error but the linker triggers another one:
ld.lld: warning: ignoring memory region assignment for non-allocatable section '.incbin_sram' ld.lld: error: section '.pmusram' will not fit in region 'PMUSRAM': overflowed by 3928 bytes
The overflow error seems legitimate, increasing PMUSRAM_RSIZE at the top of the linker script fixes it. There's funky alignment in the rule so maybe that's wasting space? I'm sadly not familiar enough with linkers and don't understand the last warning though, hopefully someone else can figure that one out.
Maybe once we figure out how to build properly as suggested in 2) this will just go away since we're probably missing a bunch of flags anyway.
Cheers, Quentin
Boyan
Hi Boyan,
On 10/31/24 11:02 AM, Boyan Karatotev wrote:
[You don't often get email from boyan.karatotev@arm.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
Hi Quentin,
[You don't often get email from boyan.karatotev@arm.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
Hi Quentin,
I dug around a bit and I think I have answers for most.
Thanks for looking into this :)
No worries!
- RK3588 symbol redefinition in assembly
This only happens for clang and I'm a bit confused as to why. Applying the above patch fixing that issue but inspecting the assembly code reveals something really odd.
**With** that patch, with gcc: 25c8c: d510149f msr dbgprcr_el1, xzr 25c90: d538f2e0 mrs x0, s3_0_c15_c2_7 25c94: b2400000 orr x0, x0, #0x1 25c98: d518f2e0 msr s3_0_c15_c2_7, x0 25c9c: 58000020 ldr x0, 0x25ca0 25ca0: d5033fdf isb 25ca4: d503207f wfi 25ca8: d61f0000 br x0
**with** that patch, with clang: 26d2c: d510149f msr dbgprcr_el1, xzr 26d30: d538f2e0 mrs x0, s3_0_c15_c2_7 26d34: b2400000 orr x0, x0, #0x1 26d38: d518f2e0 msr s3_0_c15_c2_7, x0 26d3c: 58000020 ldr x0, 0x26d40 <--- "my" ldr 26d40: d5033fdf isb 26d44: d503207f wfi 26d48: d61f0000 br x0 <--- "my" br 26d4c: 94000e76 bl 0x2a724 26d50: d510149f msr dbgprcr_el1, xzr 26d54: d538f2e0 mrs x0, s3_0_c15_c2_7 26d58: b2400000 orr x0, x0, #0x1 26d5c: d518f2e0 msr s3_0_c15_c2_7, x0 26d60: 58000020 ldr x0, 0x26d64 <--- "my" ldr 26d64: d5033fdf isb 26d68: d503207f wfi 26d6c: d61f0000 br x0 <--- "my" br
So I think the above patch is just a band-aid on what the actually is but I have no idea what could be going on.
If I use this patch instead, to match the other assembly instructions in this file
========================================================================= diff --git a/plat/rockchip/rk3588/drivers/pmu/pmu.c b/plat/rockchip/rk3588/drivers/pmu/pmu.c index f693dbdf0..a4128b214 100644 --- a/plat/rockchip/rk3588/drivers/pmu/pmu.c +++ b/plat/rockchip/rk3588/drivers/pmu/pmu.c @@ -760,10 +760,10 @@ static inline void cpus_pd_req_enter_wfi(void) "mrs x0, S3_0_C15_C2_7\n" "orr x0, x0, #0x1\n" "msr S3_0_C15_C2_7, x0\n"
"wfi_loop:\n"
"1:\n" "isb\n" "wfi\n"
"b wfi_loop\n");
"b 1b\n");}
static void nonboot_cpus_off(void)
=========================================================================
It happily compiles, though the binary still seems to contain this loop twice.
Using __attribute__((noinline)) the code is not duplicated, but I assume this is expected as it would now simply be a function.
So, the function only runs once, but it has its address taken elsewhere. For the actual call, it does get inlined, as expected. But the address taking bit, it is kept as a standalone function which happens to be places right after. Disassembling with symbols shows this a bit better. My guess would be that clang either isn't smart enough to deduplicate this, or it's doing it on purpose. Gcc must decide to not inline.
But the function currently has the inline marker, doesn't it force the compiler to inline it?
Apparently, it doesn't have to: https://www.kernel.org/doc/local/inline.html
Are you suggesting anything we could do here?
Would any of the three above suggestions be reasonably fit for upstreaming?
Well the numbered labels should let it compile and we already merged another patch for that. We do have __always_inline, ALWAYS_INLINE, or just __attribute__((__always_inline__)) which should force the inlining. But if there are two uses of the function, I'd imagine the named label would produce similar errors.
Bingo!
If I force the inlining on GCC, I get the same error as in clang.
Using a local label works for GCC and clang, so will submit something like that instead. Note that for some reason I can only use numbered local labels even though named labels are supposedly supported by the Aarch64 ASM, but not GAS? Anyway, good enough for me, too far from my comfort zone already :)
Thanks for the hint!
I'm wondering now if it really makes sense to have inline functions if GCC just doesn't care about those? Should we remove the inline part or introduce the same macro as the kernel so that we force inlining even on GCC? I am not planning on submitting anything for that, just being curious where this puts us.
- RK3399 warning array subscript 0 is outside array bounds of 'volatile unsigned int[0]' [-Warray-bounds=]
"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""" In file included from src/dram.c:12: src/dram.c: In function 'm0_main': include/rk3399_mcu.h:15:34: warning: array subscript 0 is outside array bounds of 'volatile unsigned int[0]' [-Warray-bounds=] 15 | (*(volatile unsigned int *)(c)); __v; }) | ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/rk3399_mcu.h:16:69: note: in definition of macro 'mmio_write_32' 16 | #define mmio_write_32(c, v) ((*(volatile unsigned int *)(c)) = (v)) | ^ src/dram.c:67:23: note: in expansion of macro 'mmio_read_32' 67 | mmio_read_32(PARAM_ADDR + PARAM_FREQ_SELECT)); | ^~~~~~~~~~~~
This function is used in many places, but only in very specific places is it triggering a warning, when PARAM_ADDR is used, but only when accessed from m0 C files.
PARAM_ADDR is set to 0xc0. From !m0 files, access to PARAM_ADDR seems to be done through the M0_PARAM_ADDR which is M0_BINCODE_BASE + PARAM_ADDR. For m0 files, it's directly accessing PARAM_ADDR. I honestly don't really know why it's complaining for m0 files but not the !m0 ones.
This seems identical to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105523. The top-level makefile has a workaround for this and it sets --param=min-pagesize=0. However, this driver has a very peculiar way of building and it does not inherit most of tf-a's flags. Adding that flag to its own internal makefile fixes the issue, but I feel like it's bound to repeat. Can't it be built by the standard tfa build?
The only thing I know about TF-A on RK3399 is that it is indeed peculiar :)
As far as I know, some part of the binary runs on the Cortex-A core(s) and another part on one of the Cortex-M0, hence why we need an additional toolchain and different drivers and all that.
Ah, I hadn't though of that.
Not sure if there exists any platform today in TF-A that has the same behavior and is "built by the standard tfa build"? How would one go forward with that?
You're right, there probably isn't. I was thinking more along the lines of using the make_helpers/build_macros.mk helpers but that probably won't work either. Would propagating the TF_FLAGS be possible? Chris should probably weigh in on this.
Except if Chris's mail address is nb at arm.com, they weren't explicitly Cc'ed in your mail (if that was your intention :) ).
Cheers, Quentin
- RK3588 symbol redefinition in assembly
This only happens for clang and I'm a bit confused as to why. Applying the above patch fixing that issue but inspecting the assembly code reveals something really odd.
**With** that patch, with gcc: 25c8c: d510149f msr dbgprcr_el1, xzr 25c90: d538f2e0 mrs x0, s3_0_c15_c2_7 25c94: b2400000 orr x0, x0, #0x1 25c98: d518f2e0 msr s3_0_c15_c2_7, x0 25c9c: 58000020 ldr x0, 0x25ca0 25ca0: d5033fdf isb 25ca4: d503207f wfi 25ca8: d61f0000 br x0
**with** that patch, with clang: 26d2c: d510149f msr dbgprcr_el1, xzr 26d30: d538f2e0 mrs x0, s3_0_c15_c2_7 26d34: b2400000 orr x0, x0, #0x1 26d38: d518f2e0 msr s3_0_c15_c2_7, x0 26d3c: 58000020 ldr x0, 0x26d40 <--- "my" ldr 26d40: d5033fdf isb 26d44: d503207f wfi 26d48: d61f0000 br x0 <--- "my" br 26d4c: 94000e76 bl 0x2a724 26d50: d510149f msr dbgprcr_el1, xzr 26d54: d538f2e0 mrs x0, s3_0_c15_c2_7 26d58: b2400000 orr x0, x0, #0x1 26d5c: d518f2e0 msr s3_0_c15_c2_7, x0 26d60: 58000020 ldr x0, 0x26d64 <--- "my" ldr 26d64: d5033fdf isb 26d68: d503207f wfi 26d6c: d61f0000 br x0 <--- "my" br
So I think the above patch is just a band-aid on what the actually is but I have no idea what could be going on.
If I use this patch instead, to match the other assembly instructions in this file
========================================================================= diff --git a/plat/rockchip/rk3588/drivers/pmu/pmu.c b/plat/rockchip/rk3588/drivers/pmu/pmu.c index f693dbdf0..a4128b214 100644 --- a/plat/rockchip/rk3588/drivers/pmu/pmu.c +++ b/plat/rockchip/rk3588/drivers/pmu/pmu.c @@ -760,10 +760,10 @@ static inline void cpus_pd_req_enter_wfi(void) "mrs x0, S3_0_C15_C2_7\n" "orr x0, x0, #0x1\n" "msr S3_0_C15_C2_7, x0\n" - "wfi_loop:\n" + "1:\n" "isb\n" "wfi\n" - "b wfi_loop\n"); + "b 1b\n"); }
static void nonboot_cpus_off(void)
It happily compiles, though the binary still seems to contain this loop twice.
Using __attribute__((noinline)) the code is not duplicated, but I assume this is expected as it would now simply be a function.
So, the function only runs once, but it has its address taken elsewhere. For the actual call, it does get inlined, as expected. But the address taking bit, it is kept as a standalone function which happens to be places right after. Disassembling with symbols shows this a bit better. My guess would be that clang either isn't smart enough to deduplicate this, or it's doing it on purpose. Gcc must decide to not inline.
But the function currently has the inline marker, doesn't it force the compiler to inline it?
Apparently, it doesn't have to: https://www.kernel.org/doc/local/inline.html
Are you suggesting anything we could do here?
Would any of the three above suggestions be reasonably fit for upstreaming?
Well the numbered labels should let it compile and we already merged another patch for that. We do have __always_inline, ALWAYS_INLINE, or just __attribute__((__always_inline__)) which should force the inlining. But if there are two uses of the function, I'd imagine the named label would produce similar errors.
Bingo!
If I force the inlining on GCC, I get the same error as in clang.
Using a local label works for GCC and clang, so will submit something like that instead. Note that for some reason I can only use numbered local labels even though named labels are supposedly supported by the Aarch64 ASM, but not GAS? Anyway, good enough for me, too far from my comfort zone already :)> Thanks for the hint!
I'm wondering now if it really makes sense to have inline functions if GCC just doesn't care about those? Should we remove the inline part or introduce the same macro as the kernel so that we force inlining even on GCC? I am not planning on submitting anything for that, just being curious where this puts us.
Well it does care, it just may choose not to is the way I understand it. And we do have the same macro as the kernel, we just haven't needed it much thus far I suppose
- RK3399 warning array subscript 0 is outside array bounds of 'volatile unsigned int[0]' [-Warray-bounds=]
"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""" In file included from src/dram.c:12: src/dram.c: In function 'm0_main': include/rk3399_mcu.h:15:34: warning: array subscript 0 is outside array bounds of 'volatile unsigned int[0]' [-Warray-bounds=] 15 | (*(volatile unsigned int *)(c)); __v; }) | ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/rk3399_mcu.h:16:69: note: in definition of macro 'mmio_write_32' 16 | #define mmio_write_32(c, v) ((*(volatile unsigned int *)(c)) = (v)) | ^ src/dram.c:67:23: note: in expansion of macro 'mmio_read_32' 67 | mmio_read_32(PARAM_ADDR + PARAM_FREQ_SELECT)); | ^~~~~~~~~~~~
This function is used in many places, but only in very specific places is it triggering a warning, when PARAM_ADDR is used, but only when accessed from m0 C files.
PARAM_ADDR is set to 0xc0. From !m0 files, access to PARAM_ADDR seems to be done through the M0_PARAM_ADDR which is M0_BINCODE_BASE + PARAM_ADDR. For m0 files, it's directly accessing PARAM_ADDR. I honestly don't really know why it's complaining for m0 files but not the !m0 ones.
This seems identical to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105523. The top-level makefile has a workaround for this and it sets --param=min-pagesize=0. However, this driver has a very peculiar way of building and it does not inherit most of tf-a's flags. Adding that flag to its own internal makefile fixes the issue, but I feel like it's bound to repeat. Can't it be built by the standard tfa build?
The only thing I know about TF-A on RK3399 is that it is indeed peculiar :)
As far as I know, some part of the binary runs on the Cortex-A core(s) and another part on one of the Cortex-M0, hence why we need an additional toolchain and different drivers and all that.
Ah, I hadn't though of that.
Not sure if there exists any platform today in TF-A that has the same behavior and is "built by the standard tfa build"? How would one go forward with that?
You're right, there probably isn't. I was thinking more along the lines of using the make_helpers/build_macros.mk helpers but that probably won't work either. Would propagating the TF_FLAGS be possible? Chris should probably weigh in on this.
Except if Chris's mail address is nb at arm.com, they weren't explicitly Cc'ed in your mail (if that was your intention :) ).
Oops, sorry, that's separate. I've CCed him in case he has any input.
Boyan
Hi Quentin,
Sorry for the delay, I’ve spent the last few hours looking into this.
So, the semantics of the inline function specifier in C99 are as follows:
A function declared with an inline function specifier is an inline function. The function specifier may appear more than once; the behavior is the same as if it appeared only once. Making a function an inline function suggests that calls to the function be as fast as possible. The extent to which such suggestions are effective is implementation-defined.
To truly *enforce* inlining you need to use a GCC/Clang compiler extension like `__attribute__((always_inline))`.
In this specific case, I’m not quite sure I understand why the function is marked as `inline` at all - it is quite large. If the function is only ever used once then the compiler can (and almost always will) automatically inline it regardless of whether you use the `inline` hint, although because its address is taken elsewhere in the file the compiler must always generate a non-inlined version as well.
Additionally, the loop in the inline assembly does make it a bit harder for the compiler here – the compiler usually cannot inspect inline assembly very well, so it doesn’t automatically understand the internal infinite loop. I’ve posted a patch to resolve these issues in one swoop here: https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/33175
You're right, there probably isn't. I was thinking more along the lines of using the make_helpers/build_macros.mk helpers but that probably won't work either. Would propagating the TF_FLAGS be possible? Chris should probably weigh in on this.
I presume we’re talking about the RK3399 Cortex-M0 toolchain? The build macros are not set up to handle this sort of situation, I’m afraid. We can’t propagate `TF_CFLAGS` either because many of the flags are not compatible across the AArch64 and AArch32 toolchains, so it’s a manual process.
Hope that helps, Chris
From: Quentin Schulz via TF-A tf-a@lists.trustedfirmware.org Date: Thursday, 31 October 2024 at 13:53 To: Boyan Karatotev Boyan.Karatotev@arm.com, tf-a@lists.trustedfirmware.org tf-a@lists.trustedfirmware.org Cc: nb@arm.com nb@arm.com Subject: [TF-A] Re: TF-A: Compilation issues for Rockchip platforms Hi Boyan,
On 10/31/24 11:02 AM, Boyan Karatotev wrote:
[You don't often get email from boyan.karatotev@arm.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
Hi Quentin,
[You don't often get email from boyan.karatotev@arm.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
Hi Quentin,
I dug around a bit and I think I have answers for most.
Thanks for looking into this :)
No worries!
- RK3588 symbol redefinition in assembly
This only happens for clang and I'm a bit confused as to why. Applying the above patch fixing that issue but inspecting the assembly code reveals something really odd.
**With** that patch, with gcc: 25c8c: d510149f msr dbgprcr_el1, xzr 25c90: d538f2e0 mrs x0, s3_0_c15_c2_7 25c94: b2400000 orr x0, x0, #0x1 25c98: d518f2e0 msr s3_0_c15_c2_7, x0 25c9c: 58000020 ldr x0, 0x25ca0 25ca0: d5033fdf isb 25ca4: d503207f wfi 25ca8: d61f0000 br x0
**with** that patch, with clang: 26d2c: d510149f msr dbgprcr_el1, xzr 26d30: d538f2e0 mrs x0, s3_0_c15_c2_7 26d34: b2400000 orr x0, x0, #0x1 26d38: d518f2e0 msr s3_0_c15_c2_7, x0 26d3c: 58000020 ldr x0, 0x26d40 <--- "my" ldr 26d40: d5033fdf isb 26d44: d503207f wfi 26d48: d61f0000 br x0 <--- "my" br 26d4c: 94000e76 bl 0x2a724 26d50: d510149f msr dbgprcr_el1, xzr 26d54: d538f2e0 mrs x0, s3_0_c15_c2_7 26d58: b2400000 orr x0, x0, #0x1 26d5c: d518f2e0 msr s3_0_c15_c2_7, x0 26d60: 58000020 ldr x0, 0x26d64 <--- "my" ldr 26d64: d5033fdf isb 26d68: d503207f wfi 26d6c: d61f0000 br x0 <--- "my" br
So I think the above patch is just a band-aid on what the actually is but I have no idea what could be going on.
If I use this patch instead, to match the other assembly instructions in this file
========================================================================= diff --git a/plat/rockchip/rk3588/drivers/pmu/pmu.c b/plat/rockchip/rk3588/drivers/pmu/pmu.c index f693dbdf0..a4128b214 100644 --- a/plat/rockchip/rk3588/drivers/pmu/pmu.c +++ b/plat/rockchip/rk3588/drivers/pmu/pmu.c @@ -760,10 +760,10 @@ static inline void cpus_pd_req_enter_wfi(void) "mrs x0, S3_0_C15_C2_7\n" "orr x0, x0, #0x1\n" "msr S3_0_C15_C2_7, x0\n"
"wfi_loop:\n"
"1:\n" "isb\n" "wfi\n"
"b wfi_loop\n");
"b 1b\n");}
static void nonboot_cpus_off(void)
=========================================================================
It happily compiles, though the binary still seems to contain this loop twice.
Using __attribute__((noinline)) the code is not duplicated, but I assume this is expected as it would now simply be a function.
So, the function only runs once, but it has its address taken elsewhere. For the actual call, it does get inlined, as expected. But the address taking bit, it is kept as a standalone function which happens to be places right after. Disassembling with symbols shows this a bit better. My guess would be that clang either isn't smart enough to deduplicate this, or it's doing it on purpose. Gcc must decide to not inline.
But the function currently has the inline marker, doesn't it force the compiler to inline it?
Apparently, it doesn't have to: https://www.kernel.org/doc/local/inline.html
Are you suggesting anything we could do here?
Would any of the three above suggestions be reasonably fit for upstreaming?
Well the numbered labels should let it compile and we already merged another patch for that. We do have __always_inline, ALWAYS_INLINE, or just __attribute__((__always_inline__)) which should force the inlining. But if there are two uses of the function, I'd imagine the named label would produce similar errors.
Bingo!
If I force the inlining on GCC, I get the same error as in clang.
Using a local label works for GCC and clang, so will submit something like that instead. Note that for some reason I can only use numbered local labels even though named labels are supposedly supported by the Aarch64 ASM, but not GAS? Anyway, good enough for me, too far from my comfort zone already :)
Thanks for the hint!
I'm wondering now if it really makes sense to have inline functions if GCC just doesn't care about those? Should we remove the inline part or introduce the same macro as the kernel so that we force inlining even on GCC? I am not planning on submitting anything for that, just being curious where this puts us.
- RK3399 warning array subscript 0 is outside array bounds of 'volatile unsigned int[0]' [-Warray-bounds=]
"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""" In file included from src/dram.c:12: src/dram.c: In function 'm0_main': include/rk3399_mcu.h:15:34: warning: array subscript 0 is outside array bounds of 'volatile unsigned int[0]' [-Warray-bounds=] 15 | (*(volatile unsigned int *)(c)); __v; }) | ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/rk3399_mcu.h:16:69: note: in definition of macro 'mmio_write_32' 16 | #define mmio_write_32(c, v) ((*(volatile unsigned int *)(c)) = (v)) | ^ src/dram.c:67:23: note: in expansion of macro 'mmio_read_32' 67 | mmio_read_32(PARAM_ADDR + PARAM_FREQ_SELECT)); | ^~~~~~~~~~~~
This function is used in many places, but only in very specific places is it triggering a warning, when PARAM_ADDR is used, but only when accessed from m0 C files.
PARAM_ADDR is set to 0xc0. From !m0 files, access to PARAM_ADDR seems to be done through the M0_PARAM_ADDR which is M0_BINCODE_BASE + PARAM_ADDR. For m0 files, it's directly accessing PARAM_ADDR. I honestly don't really know why it's complaining for m0 files but not the !m0 ones.
This seems identical to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105523. The top-level makefile has a workaround for this and it sets --param=min-pagesize=0. However, this driver has a very peculiar way of building and it does not inherit most of tf-a's flags. Adding that flag to its own internal makefile fixes the issue, but I feel like it's bound to repeat. Can't it be built by the standard tfa build?
The only thing I know about TF-A on RK3399 is that it is indeed peculiar :)
As far as I know, some part of the binary runs on the Cortex-A core(s) and another part on one of the Cortex-M0, hence why we need an additional toolchain and different drivers and all that.
Ah, I hadn't though of that.
Not sure if there exists any platform today in TF-A that has the same behavior and is "built by the standard tfa build"? How would one go forward with that?
You're right, there probably isn't. I was thinking more along the lines of using the make_helpers/build_macros.mk helpers but that probably won't work either. Would propagating the TF_FLAGS be possible? Chris should probably weigh in on this.
Except if Chris's mail address is nb at arm.com, they weren't explicitly Cc'ed in your mail (if that was your intention :) ).
Cheers, Quentin -- TF-A mailing list -- tf-a@lists.trustedfirmware.org To unsubscribe send an email to tf-a-leave@lists.trustedfirmware.org
Hi Chris,
On 10/31/24 6:10 PM, Chris Kay wrote:
You don't often get email from chris.kay@arm.com. Learn why this is importanthttps://aka.ms/LearnAboutSenderIdentification Hi Quentin,
Sorry for the delay, I’ve spent the last few hours looking into this.
Absolutely no worries, very thankful for the help :)
So, the semantics of the inline function specifier in C99 are as follows:
A function declared with an inline function specifier is an inline function. The function specifier may appear more than once; the behavior is the same as if it appeared only once. Making a function an inline function suggests that calls to the function be as fast as possible. The extent to which such suggestions are effective is implementation-defined.
To truly *enforce* inlining you need to use a GCC/Clang compiler extension like `__attribute__((always_inline))`.
In this specific case, I’m not quite sure I understand why the function is marked as `inline` at all - it is quite large. If the function is only ever used once then the compiler can (and almost always will) automatically inline it regardless of whether you use the `inline` hint, although because its address is taken elsewhere in the file the compiler must always generate a non-inlined version as well.
Additionally, the loop in the inline assembly does make it a bit harder for the compiler here – the compiler usually cannot inspect inline assembly very well, so it doesn’t automatically understand the internal infinite loop. I’ve posted a patch to resolve these issues in one swoop here: https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/33175
FWIW, there are other isb+wfi loops in that file, so maybe it's worth changing those as well? c.f. ddr_resume and rockchip_cpu_reset_early.
You're right, there probably isn't. I was thinking more along the lines of using the make_helpers/build_macros.mk helpers but that probably won't work either. Would propagating the TF_FLAGS be possible? Chris should probably weigh in on this.
I presume we’re talking about the RK3399 Cortex-M0 toolchain? The build macros are not set up to handle this sort of situation, I’m afraid. We can’t propagate `TF_CFLAGS` either because many of the flags are not compatible across the AArch64 and AArch32 toolchains, so it’s a manual process.
That's what I was afraid of :/
I have another question related to the INCBIN macro with clang+GCC that I will send in a few minutes as an answer to another mail, will add you in Cc if you don't mind.
Thanks! Quentin
I’m afraid I’ve not been able to reproduce the .incbin error – it all links fine for me.
$ clang –version Ubuntu clang version 18.1.3 (1ubuntu1) Target: aarch64-unknown-linux-gnu Thread model: posix InstalledDir: /usr/bin
Hi Chris,
On 11/4/24 4:47 PM, Chris Kay wrote:
I’m afraid I’ve not been able to reproduce the .incbin error – it all links fine for me.
$ clang –version Ubuntu clang version 18.1.3 (1ubuntu1) Target: aarch64-unknown-linux-gnu Thread model: posix InstalledDir: /usr/bin
Can you share how you did it? Because I cannot even link it with Ubuntu 24.04 (which I assume is your distro).
The git repo is checked out at https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/33183/1 +
""" diff --git a/plat/rockchip/rk3399/drivers/pmu/pmu_fw.S b/plat/rockchip/rk3399/drivers/pmu/pmu_fw.S index 26f331317..ed70cba1f 100644 --- a/plat/rockchip/rk3399/drivers/pmu/pmu_fw.S +++ b/plat/rockchip/rk3399/drivers/pmu/pmu_fw.S @@ -11,7 +11,7 @@ .type \sym, @object .align 4 \sym : - .incbin \file + .incbin "\file" .size \sym , .-\sym .global \sym()_end \sym()_end : """
podman run -it --userns=keep-id -v "$PWD":"$PWD" -w $PWD --security-opt label=disable ubuntu:24.04 bash
in another terminal: podman exec -it --user root --latest sh -c 'apt-get update && apt-get install -qq --no-install-recommends clang make lld llvm gcc-arm-none-eabi'
from the first terminal, run make CROSS_COMPILE=aarch64-linux-gnu- PLAT=rk3399 CC=clang bl31 -j`nproc`
I get: ld.lld: warning: ignoring memory region assignment for non-allocatable section '.incbin_sram' ld.lld: error: section '.pmusram' will not fit in region 'PMUSRAM': overflowed by 3928 bytes
Whereas with Fedora41: podman run -it --userns=keep-id -v "$PWD":"$PWD" -w $PWD --security-opt label=disable fedora:41 bash
In a second terminal: podman exec -it --user root --latest sh -c 'dnf upgrade && dnf install --setopt=install_weak_deps=False -y make clang llvm arm-none-eabi-gcc lld'
I get: ld.lld: warning: ignoring memory region assignment for non-allocatable section '.incbin_sram'
clang --version clang version 19.1.0 (Fedora 19.1.0-1.fc41) Target: x86_64-redhat-linux-gnu Thread model: posix InstalledDir: /usr/bin Configuration file: /etc/clang/x86_64-redhat-linux-gnu-clang.cfg
Cheers, Quentin
Ah, sorry, I was building for RK3588 – got confused about what’s building where. Anyway, the issue is that the INCBIN macro creates sections without indicating their attributes, and .incbin does not provide enough information about what the binary data is to allow it to infer them. It’s generally reasonable to assume that the data is SHF_ALLOC, which I guess is what LD does, but evidently not LLD. The fix is quite simple:
diff --git a/plat/rockchip/rk3399/drivers/pmu/pmu_fw.S b/plat/rockchip/rk3399/drivers/pmu/pmu_fw.S index 26f331317..74f569402 100644 --- a/plat/rockchip/rk3399/drivers/pmu/pmu_fw.S +++ b/plat/rockchip/rk3399/drivers/pmu/pmu_fw.S @@ -6,7 +6,7 @@
/* convoluted way to make sure that the define is pasted just the right way */ .macro INCBIN file sym sec - .section \sec + .section \sec, "a" .global \sym .type \sym, @object .align 4
Hi Chris,
On 11/4/24 5:55 PM, Chris Kay wrote:
Ah, sorry, I was building for RK3588 – got confused about what’s building where. Anyway, the issue is that the INCBIN macro creates sections without indicating their attributes, and .incbin does not provide enough information about what the binary data is to allow it to infer them. It’s generally reasonable to assume that the data is SHF_ALLOC, which I guess is what LD does, but evidently not LLD. The fix is quite simple:
diff --git a/plat/rockchip/rk3399/drivers/pmu/pmu_fw.S b/plat/rockchip/rk3399/drivers/pmu/pmu_fw.S index 26f331317..74f569402 100644 --- a/plat/rockchip/rk3399/drivers/pmu/pmu_fw.S +++ b/plat/rockchip/rk3399/drivers/pmu/pmu_fw.S @@ -6,7 +6,7 @@
/* convoluted way to make sure that the define is pasted just the right way */ .macro INCBIN file sym sec
.section \sec
.section \sec, "a" .global \sym .type \sym, @object .align 4
That did the trick, thanks.
With the following changes:
""" diff --git a/plat/rockchip/rk3399/drivers/pmu/pmu_fw.S b/plat/rockchip/rk3399/drivers/pmu/pmu_fw.S index 26f331317..db2d4219a 100644 --- a/plat/rockchip/rk3399/drivers/pmu/pmu_fw.S +++ b/plat/rockchip/rk3399/drivers/pmu/pmu_fw.S @@ -6,16 +6,16 @@
/* convoluted way to make sure that the define is pasted just the right way */ .macro INCBIN file sym sec - .section \sec + .section \sec, "a" .global \sym .type \sym, @object .align 4 \sym : - .incbin \file + .incbin "\file" .size \sym , .-\sym .global \sym()_end \sym()_end : .endm
-INCBIN ""RK3399M0FW"", "rk3399m0_bin", ".sram.incbin" -INCBIN ""RK3399M0PMUFW"", "rk3399m0pmu_bin", ".pmusram.incbin" +INCBIN RK3399M0FW, "rk3399m0_bin", ".sram.incbin" +INCBIN RK3399M0PMUFW, "rk3399m0pmu_bin", ".pmusram.incbin" """
RK3399 (the cortex-A part only!) now builds with GCC and clang.
I'm not entirely sure the INCBIN changes are safe, but the RK3399M0*FW are all strings with escaped quotes, so maybe that's safe enough?
Are you going to send a patch or should I send it? I don't really understand the fix so I would add the sentence in your previous mail in the commit log instead of coming up with something on my own. Let me know!
Thanks for the help and fixes!
Cheers, Quentin
Hi Boyan, Chris,
On 10/31/24 11:02 AM, Boyan Karatotev wrote: [...]
- RK3399 warning array subscript 0 is outside array bounds of 'volatile unsigned int[0]' [-Warray-bounds=]
"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""" In file included from src/dram.c:12: src/dram.c: In function 'm0_main': include/rk3399_mcu.h:15:34: warning: array subscript 0 is outside array bounds of 'volatile unsigned int[0]' [-Warray-bounds=] 15 | (*(volatile unsigned int *)(c)); __v; }) | ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/rk3399_mcu.h:16:69: note: in definition of macro 'mmio_write_32' 16 | #define mmio_write_32(c, v) ((*(volatile unsigned int *)(c)) = (v)) | ^ src/dram.c:67:23: note: in expansion of macro 'mmio_read_32' 67 | mmio_read_32(PARAM_ADDR + PARAM_FREQ_SELECT)); | ^~~~~~~~~~~~
This function is used in many places, but only in very specific places is it triggering a warning, when PARAM_ADDR is used, but only when accessed from m0 C files.
PARAM_ADDR is set to 0xc0. From !m0 files, access to PARAM_ADDR seems to be done through the M0_PARAM_ADDR which is M0_BINCODE_BASE + PARAM_ADDR. For m0 files, it's directly accessing PARAM_ADDR. I honestly don't really know why it's complaining for m0 files but not the !m0 ones.
This seems identical to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105523. The top-level makefile has a workaround for this and it sets --param=min-pagesize=0. However, this driver has a very peculiar way of building and it does not inherit most of tf-a's flags. Adding that flag to its own internal makefile fixes the issue, but I feel like it's bound to repeat. Can't it be built by the standard tfa build?
The only thing I know about TF-A on RK3399 is that it is indeed peculiar :)
As far as I know, some part of the binary runs on the Cortex-A core(s) and another part on one of the Cortex-M0, hence why we need an additional toolchain and different drivers and all that.
Ah, I hadn't though of that.
Not sure if there exists any platform today in TF-A that has the same behavior and is "built by the standard tfa build"? How would one go forward with that?
You're right, there probably isn't. I was thinking more along the lines of using the make_helpers/build_macros.mk helpers but that probably won't work either. Would propagating the TF_FLAGS be possible? Chris should probably weigh in on this.
I did some updates to the Makefile to match closer to what the top Makefile has and it is now happier. Will send a patch, let's see what the reviewers will say. Thanks for the hints!
- RK3399 expected string in '.incbin' directive
clang seems happy with that error but the linker triggers another one:
ld.lld: warning: ignoring memory region assignment for non-allocatable section '.incbin_sram' ld.lld: error: section '.pmusram' will not fit in region 'PMUSRAM': overflowed by 3928 bytes
The overflow error seems legitimate, increasing PMUSRAM_RSIZE at the top of the linker script fixes it. There's funky alignment in the rule so maybe that's wasting space? I'm sadly not familiar enough with linkers and don't understand the last warning though, hopefully someone else can figure that one out.
Maybe once we figure out how to build properly as suggested in 2) this will just go away since we're probably missing a bunch of flags anyway.
Ok so I found another "issue" with the custom Makefile for the M0 on RK3399. If one sets CC=clang on the command line, the part running on the Cortex-A is compiled with clang but the part running on the M0 is compiled with GCC. I think we're missing something like the CC definition in the M0 Makefile but it's a bit confusing to me how it's done in the top Makefile so not sure how to go with that. But once that's done (I just hardcoded the compiler in the M0 Makefile), clang (wrongly?) complains about unused functions:
src/startup.c:64:13: warning: unused function 'default_reset_handler' [-Wunused-function] 64 | static void default_reset_handler(void) | ^~~~~~~~~~~~~~~~~~~~~ src/startup.c:87:13: warning: unused function 'default_handler' [-Wunused-function] 87 | static void default_handler(void) | ^~~~~~~~~~~~~~~ 2 warnings generated.
but those are part of the g_pfnVectors through the use of #pragma weak, so probably a false positive?
Anyway, this INCBIN macro seems to be difficult to make it work with both clang and GCC. I swear I found some way to make it work on both but I didn't save my work and now I cannot reproduce the successful try :/
With:
=============================================================== diff --git a/plat/rockchip/rk3399/drivers/pmu/pmu_fw.S b/plat/rockchip/rk3399/drivers/pmu/pmu_fw.S index 26f331317..ed70cba1f 100644 --- a/plat/rockchip/rk3399/drivers/pmu/pmu_fw.S +++ b/plat/rockchip/rk3399/drivers/pmu/pmu_fw.S @@ -11,7 +11,7 @@ .type \sym, @object .align 4 \sym : - .incbin \file + .incbin "\file" .size \sym , .-\sym .global \sym()_end \sym()_end : ===============================================================
I can compile with clang but I get a warning:
ld.lld: warning: ignoring memory region assignment for non-allocatable section '.incbin_sram'
I assume this is bad but the system still reaches userspace, so maybe that's fine? What's the consensus on that?
Cheers, Quentin
Hi Quentin,
To allow the usual multi-toolchain mechanisms to kick in for the M0 toolchain you’ll need to add `rk3399-m0-{as,cc,ld,..}-parameter` variables to `make_helpers/toolchains/rk3399-m0.mk`, a la:
``` diff --git a/make_helpers/toolchains/rk3399-m0.mk b/make_helpers/toolchains/rk3399-m0.mk index 3a7f173a3..c9902b955 100644 --- a/make_helpers/toolchains/rk3399-m0.mk +++ b/make_helpers/toolchains/rk3399-m0.mk @@ -6,24 +6,31 @@ rk3399-m0-name := RK3399 M0 +rk3399-m0-cc-parameter := M0_CC rk3399-m0-cc-default-id := gnu-gcc rk3399-m0-cc-default := $(or $(M0_CROSS_COMPILE),arm-none-eabi-)gcc +rk3399-m0-cpp-parameter := M0_CPP rk3399-m0-cpp-default-id := gnu-gcc rk3399-m0-cpp-default := $(or $(M0_CROSS_COMPILE),arm-none-eabi-)gcc +rk3399-m0-as-parameter := M0_AS rk3399-m0-as-default-id := gnu-gcc rk3399-m0-as-default := $(or $(M0_CROSS_COMPILE),arm-none-eabi-)gcc +rk3399-m0-ld-parameter := M0_LD rk3399-m0-ld-default-id := gnu-gcc rk3399-m0-ld-default := $(or $(M0_CROSS_COMPILE),arm-none-eabi-)gcc +rk3399-m0-oc-parameter := M0_OC rk3399-m0-oc-default-id := gnu-objcopy rk3399-m0-oc-default := $(or $(M0_CROSS_COMPILE),arm-none-eabi-)objcopy +rk3399-m0-od-parameter := M0_OD rk3399-m0-od-default-id := gnu-objdump rk3399-m0-od-default := $(or $(M0_CROSS_COMPILE),arm-none-eabi-)objdump +rk3399-m0-ar-parameter := M0_AR rk3399-m0-ar-default-id := gnu-ar rk3399-m0-ar-default := $(or $(M0_CROSS_COMPILE),arm-none-eabi-)gcc-ar ```
The warning about the `.incbin_sram` section is suspicious; it should definitely be `SHF_ALLOC`, so it being not suggests that it might be zero-sized, and that the `.incbin` directive is not behaving as expected... I’ll try to take a look a bit later.
Chris
From: Quentin Schulz quentin.schulz@cherry.de Date: Thursday, 31 October 2024 at 17:38 To: Boyan Karatotev Boyan.Karatotev@arm.com, tf-a@lists.trustedfirmware.org tf-a@lists.trustedfirmware.org Cc: nb@arm.com nb@arm.com, Chris Kay Chris.Kay@arm.com Subject: Re: [TF-A] TF-A: Compilation issues for Rockchip platforms Hi Boyan, Chris,
On 10/31/24 11:02 AM, Boyan Karatotev wrote: [...]
- RK3399 warning array subscript 0 is outside array bounds of 'volatile unsigned int[0]' [-Warray-bounds=]
"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""" In file included from src/dram.c:12: src/dram.c: In function 'm0_main': include/rk3399_mcu.h:15:34: warning: array subscript 0 is outside array bounds of 'volatile unsigned int[0]' [-Warray-bounds=] 15 | (*(volatile unsigned int *)(c)); __v; }) | ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/rk3399_mcu.h:16:69: note: in definition of macro 'mmio_write_32' 16 | #define mmio_write_32(c, v) ((*(volatile unsigned int *)(c)) = (v)) | ^ src/dram.c:67:23: note: in expansion of macro 'mmio_read_32' 67 | mmio_read_32(PARAM_ADDR + PARAM_FREQ_SELECT)); | ^~~~~~~~~~~~
This function is used in many places, but only in very specific places is it triggering a warning, when PARAM_ADDR is used, but only when accessed from m0 C files.
PARAM_ADDR is set to 0xc0. From !m0 files, access to PARAM_ADDR seems to be done through the M0_PARAM_ADDR which is M0_BINCODE_BASE + PARAM_ADDR. For m0 files, it's directly accessing PARAM_ADDR. I honestly don't really know why it's complaining for m0 files but not the !m0 ones.
This seems identical to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105523. The top-level makefile has a workaround for this and it sets --param=min-pagesize=0. However, this driver has a very peculiar way of building and it does not inherit most of tf-a's flags. Adding that flag to its own internal makefile fixes the issue, but I feel like it's bound to repeat. Can't it be built by the standard tfa build?
The only thing I know about TF-A on RK3399 is that it is indeed peculiar :)
As far as I know, some part of the binary runs on the Cortex-A core(s) and another part on one of the Cortex-M0, hence why we need an additional toolchain and different drivers and all that.
Ah, I hadn't though of that.
Not sure if there exists any platform today in TF-A that has the same behavior and is "built by the standard tfa build"? How would one go forward with that?
You're right, there probably isn't. I was thinking more along the lines of using the make_helpers/build_macros.mk helpers but that probably won't work either. Would propagating the TF_FLAGS be possible? Chris should probably weigh in on this.
I did some updates to the Makefile to match closer to what the top Makefile has and it is now happier. Will send a patch, let's see what the reviewers will say. Thanks for the hints!
- RK3399 expected string in '.incbin' directive
clang seems happy with that error but the linker triggers another one:
ld.lld: warning: ignoring memory region assignment for non-allocatable section '.incbin_sram' ld.lld: error: section '.pmusram' will not fit in region 'PMUSRAM': overflowed by 3928 bytes
The overflow error seems legitimate, increasing PMUSRAM_RSIZE at the top of the linker script fixes it. There's funky alignment in the rule so maybe that's wasting space? I'm sadly not familiar enough with linkers and don't understand the last warning though, hopefully someone else can figure that one out.
Maybe once we figure out how to build properly as suggested in 2) this will just go away since we're probably missing a bunch of flags anyway.
Ok so I found another "issue" with the custom Makefile for the M0 on RK3399. If one sets CC=clang on the command line, the part running on the Cortex-A is compiled with clang but the part running on the M0 is compiled with GCC. I think we're missing something like the CC definition in the M0 Makefile but it's a bit confusing to me how it's done in the top Makefile so not sure how to go with that. But once that's done (I just hardcoded the compiler in the M0 Makefile), clang (wrongly?) complains about unused functions:
src/startup.c:64:13: warning: unused function 'default_reset_handler' [-Wunused-function] 64 | static void default_reset_handler(void) | ^~~~~~~~~~~~~~~~~~~~~ src/startup.c:87:13: warning: unused function 'default_handler' [-Wunused-function] 87 | static void default_handler(void) | ^~~~~~~~~~~~~~~ 2 warnings generated.
but those are part of the g_pfnVectors through the use of #pragma weak, so probably a false positive?
Anyway, this INCBIN macro seems to be difficult to make it work with both clang and GCC. I swear I found some way to make it work on both but I didn't save my work and now I cannot reproduce the successful try :/
With:
=============================================================== diff --git a/plat/rockchip/rk3399/drivers/pmu/pmu_fw.S b/plat/rockchip/rk3399/drivers/pmu/pmu_fw.S index 26f331317..ed70cba1f 100644 --- a/plat/rockchip/rk3399/drivers/pmu/pmu_fw.S +++ b/plat/rockchip/rk3399/drivers/pmu/pmu_fw.S @@ -11,7 +11,7 @@ .type \sym, @object .align 4 \sym : - .incbin \file + .incbin "\file" .size \sym , .-\sym .global \sym()_end \sym()_end : ===============================================================
I can compile with clang but I get a warning:
ld.lld: warning: ignoring memory region assignment for non-allocatable section '.incbin_sram'
I assume this is bad but the system still reaches userspace, so maybe that's fine? What's the consensus on that?
Cheers, Quentin
Hi Chris,
On 11/1/24 1:00 PM, Chris Kay wrote:
Hi Quentin,
To allow the usual multi-toolchain mechanisms to kick in for the M0 toolchain you’ll need to add `rk3399-m0-{as,cc,ld,..}-parameter` variables to `make_helpers/toolchains/rk3399-m0.mk`, a la:
diff --git a/make_helpers/toolchains/rk3399-m0.mk b/make_helpers/toolchains/rk3399-m0.mk index 3a7f173a3..c9902b955 100644 --- a/make_helpers/toolchains/rk3399-m0.mk +++ b/make_helpers/toolchains/rk3399-m0.mk @@ -6,24 +6,31 @@ rk3399-m0-name := RK3399 M0 +rk3399-m0-cc-parameter := M0_CC rk3399-m0-cc-default-id := gnu-gcc rk3399-m0-cc-default := $(or $(M0_CROSS_COMPILE),arm-none-eabi-)gcc +rk3399-m0-cpp-parameter := M0_CPP rk3399-m0-cpp-default-id := gnu-gcc rk3399-m0-cpp-default := $(or $(M0_CROSS_COMPILE),arm-none-eabi-)gcc +rk3399-m0-as-parameter := M0_AS rk3399-m0-as-default-id := gnu-gcc rk3399-m0-as-default := $(or $(M0_CROSS_COMPILE),arm-none-eabi-)gcc +rk3399-m0-ld-parameter := M0_LD rk3399-m0-ld-default-id := gnu-gcc rk3399-m0-ld-default := $(or $(M0_CROSS_COMPILE),arm-none-eabi-)gcc +rk3399-m0-oc-parameter := M0_OC rk3399-m0-oc-default-id := gnu-objcopy rk3399-m0-oc-default := $(or $(M0_CROSS_COMPILE),arm-none-eabi-)objcopy +rk3399-m0-od-parameter := M0_OD rk3399-m0-od-default-id := gnu-objdump rk3399-m0-od-default := $(or $(M0_CROSS_COMPILE),arm-none-eabi-)objdump +rk3399-m0-ar-parameter := M0_AR rk3399-m0-ar-default-id := gnu-ar rk3399-m0-ar-default := $(or $(M0_CROSS_COMPILE),arm-none-eabi-)gcc-ar
Ah, nice... though I basically end up needing to reimplement all the compiler and linker checks we have in the top Makefile for LLVM/clang-specific options vs GCC-specific ones. It'd be nice if we could move some of those in some make_helpers maybe?
Seems like a big rework though, so I'm fine with "too much work for this one SoC with a peculiar setup". To come clean, I don't use clang for compiling this piece of software but some people using meta-rockchip Yocto layer do, and I hate having -dirty in my version number because of local patches to fix clang support, hence why I pushed most commits for reviews and fixing the warnings I could.
Thanks a lot for the help!
Cheers, Quentin
tf-a@lists.trustedfirmware.org
-
Boyan Karatotev -
Chris Kay -
Quentin Schulz