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:
[...]
>>>> 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));
>>>>         |                       ^~~~~~~~~~~~
>>>>
>>>> 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!

>>>> 3) 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