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