ARM-software / tf-issues

Issue tracking for the ARM Trusted Firmware project
37 stars 16 forks source link

Memory map problem - Can't load atf-fastboot after increasing BL1_RO_SIZE on HiKey620 #538

Closed gitfineon closed 6 years ago

gitfineon commented 6 years ago

In former releases of ATF for Hikey it was possible to increase size of BL1 by shifting up the BL1_RO_LIMIT. This is necessary to integrate and debug features in this first stage, e.g. the TBB Feature. https://github.com/96boards/arm-trusted-firmware/blob/hikey/plat/hikey/include/platform_def.h

-#define PLATFORM_STACK_SIZE            0x800
+#define PLATFORM_STACK_SIZE            0x1000

+#if TRUSTED_BOARD_BOOT
+#define BL1_RO_LIMIT                   (XG2RAM0_BASE + 0x1D000) /* + 0xD000 */
+#else
 #define BL1_RO_LIMIT                   (XG2RAM0_BASE + 0x10000) /* 0xf981_0000 */
+#endif

+#if TRUSTED_BOARD_BOOT
+# define BL2_BASE                      (BL1_RW_BASE + 0x19000) /* + 0x11000 */
+#else
 # define BL2_BASE                      (BL1_RW_BASE + 0x8000)  /* 0xf981_8000 */
+#endif

I'm very happy that @hzhuang1 ported HiKey support back to the official ATF repo. https://github.com/ARM-software/arm-trusted-firmware/pull/950

But I tried to extend it the same way on the current version of ATF and make recovery stops before starting fastboot ... I'm pretty sure it has something to do with the entry point address, but I'm not able to fix it. https://github.com/ARM-software/arm-trusted-firmware/blob/master/plat/hisilicon/hikey/include/platform_def.h

-#define PLATFORM_STACK_SIZE            0x800
+#define PLATFORM_STACK_SIZE            0x1000

+#if TRUSTED_BOARD_BOOT
+#define TZROM_SIZE             0x00020000
+#else
+#define TZROM_SIZE             0x00010000
+#endif

 #define BL1_RO_BASE                    (XG2RAM0_BASE + BL1_XG2RAM0_OFFSET)
-#define BL1_RO_LIMIT                   (XG2RAM0_BASE + 0x10000)
+#define BL1_RO_LIMIT                   (XG2RAM0_BASE + TZROM_SIZE)

-#define BL1_RW_SIZE                    (0x00088000)
+#define BL1_RW_SIZE                    (BL31_LIMIT - BL1_RW_BASE)
-#define BL1_RW_LIMIT                   (0xF9898000)
+#define BL1_RW_LIMIT                   (BL31_LIMIT)

-#define BL31_LIMIT                     0xF9898000
+#define BL31_LIMIT                     (BL31_BASE + 0x40000)

Some of the flags are also used for atf-fastboot, so I updated this file, too. https://github.com/96boards-hikey/atf-fastboot/blob/master/plat/hikey/include/platform_def.h

+#if TRUSTED_BOARD_BOOT && (IMAGE_BL1 || IMAGE_BL2)
+#define PLATFORM_STACK_SIZE 0x1000
+#else
 #define PLATFORM_STACK_SIZE            0x800
+#endif

+#if TRUSTED_BOARD_BOOT
+/* Certificates */
+# define BL2_CERT_NAME                 "bl2.crt"
+# define TRUSTED_KEY_CERT_NAME         "trusted_key.crt"
+
+# define BL30_KEY_CERT_NAME            "bl30_key.crt"
+# define BL31_KEY_CERT_NAME            "bl31_key.crt"
+# define BL32_KEY_CERT_NAME            "bl32_key.crt"
+# define BL33_KEY_CERT_NAME            "bl33_key.crt"
+
+# define BL30_CERT_NAME                        "bl30.crt"
+# define BL31_CERT_NAME                        "bl31.crt"
+# define BL32_CERT_NAME                        "bl32.crt"
+# define BL33_CERT_NAME                        "bl33.crt"
+#endif /* TRUSTED_BOARD_BOOT */

+#if TRUSTED_BOARD_BOOT
+#define TZROM_SIZE             0x00020000
+#else
+#define TZROM_SIZE             0x00010000
+#endif

-#define BL1_RO_LIMIT                   (BL1_RO_BASE + 0x10000)
+#define BL1_RO_LIMIT                   (BL1_RO_BASE + TZROM_SIZE)

-#define BL1_RW_SIZE                    (0xf9898000 - BL1_RW_BASE)
+#define BL1_RW_SIZE                    (BL1_RW_LIMIT - BL1_RW_BASE)

I'm suffering from this issue more than 2 days now, tried a lot of combinations, wrote memory map on a paper and added debug messages.

 ...out of reset
NOTICE:  Booting Trusted Firmware
NOTICE:  BL1: v1.4(release):v1.1-2193-g9b3c221-dirty
NOTICE:  BL1: Built : 17:24:22, Dec 15 2017
INFO:    BL1: RAM 0xf9810000 - 0xf9815000
INFO:    BL1: Loading BL2
VERBOSE: FIP header looks OK.
VERBOSE: Using FIP
INFO:    Loading image id=1 at address 0xf9818000
INFO:    Image id=1 loaded: 0xf9818000 - 0xf982a6e0
NOTICE:  BL1: Booting BL2
VERBOSE: BL1: BL2 memory layout address = 0xf9810000
INFO:    Entry point address = 0xf9818000
INFO:    SPSR = 0x3c5
VERBOSE: Argument #0 = 0x0
VERBOSE: Argument #1 = 0xf9810000
VERBOSE: Argument #2 = 0x0
VERBOSE: Argument #3 = 0x0
VERBOSE: Argument #4 = 0x0
VERBOSE: Argument #5 = 0x0
VERBOSE: Argument #6 = 0x0
VERBOSE: Argument #7 = 0x0

I'm waiting for the INFO: Enter downloading mode. Please run fastboot command on Host. message, but even on LOG_LEVEL 50 I don't get the VERBOSE: Reserved xxx bytes :disappointed:.

Please help me @hzhuang1 to make the memory layout flexible enough to add features like TBB.

hzhuang1 commented 6 years ago

Excuse me that I didn't notice this.

This fastboot is used as NS BL1U that means non secure. It's mainly used to restore hikey from brick state. If we want to implement secure updater, why not just implement it in UEFI instead?

gitfineon commented 6 years ago

Sorry for this confusing issue description. I figured out that changing memory layout of atf-fastboot should not be necessary because with former ATF from the 96boards repo, fastboot image was precompiled, right?

One month ago, I accidentally solved my problem, but since than I was not able to reproduce it.

My goal is to extend BL1_RO e.g.

/*
 *  + XG2RAM +  Default         TBB
 *  ++++++++++  0xF980_0000     0xF980_0000
 *  + loader +          4KB             4KB
 *  ++++++++++  0xF980_1000     0xF980_1000
 *  + BL1_RO +         60KB           124KB
 *  ++++++++++  0xF981_0000     0xF982_0000
 *  + BL1_RW +         32KB            32KB
 *  ++++++++++  0xF981_8000     0xF982_8000
 *  + BL2    +        256KB           256KB
 *  ++++++++++  0xF985_8000     0xF986_8000
 *  + BL31   +        256KB           256KB
 *  ++++++++++  0xF989_8000     0xF98A_8000
 *  + space  +        176KB           112KB
 *  ++++++++++  0xF98C_4000     0xF98C_4000
 */

It is not about implementing a secure updater, it is about adding a feature to BL1, e.g. Trusted Board Boot (TBB).

What am I doing wrong?

/*
 * BL1 is stored in XG2RAM0_HIRQ that is 784KB large (0xF980_0000~0xF98C_4000).
 */
#define ONCHIPROM_PARAM_BASE    (XG2RAM0_BASE + 0x700)
#define LOADER_RAM_BASE         (XG2RAM0_BASE + 0x800)
#define BL1_XG2RAM0_OFFSET      0x1000

#ifndef TZROM_BASE
#define TZROM_BASE              (XG2RAM0_BASE + BL1_XG2RAM0_OFFSET)
#endif

/*
 * Default size of BL1 on HiKey is 64 KB, but TBB requires at least 80 KB in
 * debug mode. We can test TBB on HiKey using 128 KB of flash
 */
#if TRUSTED_BOARD_BOOT
#define TZROM_SIZE              (0x00020000 - BL1_XG2RAM0_OFFSET) /* 124KB */
#else
#define TZROM_SIZE              (0x00010000 - BL1_XG2RAM0_OFFSET) /*  60KB */
#endif

/*
 * BL1 specific defines.
 *
 * Both, loader and BL1_RO region stay in SRAM since they are used to simulate
 * ROM. Loader is used to switch Hi6220 SoC from 32-bit to 64-bit mode.
 */

#define BL1_RO_BASE             (TZROM_BASE)
#define BL1_RO_LIMIT            (TZROM_BASE + TZROM_SIZE)
#define BL1_RW_BASE             (BL1_RO_LIMIT)
#define BL1_RW_SIZE             (BL31_LIMIT - BL1_RW_BASE)
#define BL1_RW_LIMIT            (BL31_LIMIT)

/*
 * Non-Secure BL1U specific defines.
 */
#define NS_BL1U_BASE            (BL2_BASE)
#define NS_BL1U_SIZE            (0x10000)
#define NS_BL1U_LIMIT           (NS_BL1U_BASE + NS_BL1U_SIZE)

/*
 * BL2 specific defines.
 */
#define BL2_BASE                (BL1_RW_BASE + 0x8000)
#define BL2_LIMIT               (BL2_BASE + 0x40000)
hzhuang1 commented 6 years ago

I can't understand your issue clearly. Please help to share your branch to me. And share your log to me. Then I can understand it better.

gitfineon commented 6 years ago

Ok, minimal example would be: Only increase BL1_RO_LIMIT to have more space in this section

 #define BL1_RO_BASE                    (XG2RAM0_BASE + BL1_XG2RAM0_OFFSET)
-#define BL1_RO_LIMIT                   (XG2RAM0_BASE + 0x10000)
+#define BL1_RO_LIMIT                   (XG2RAM0_BASE + 0x11000)
 #define BL1_RW_BASE                    (BL1_RO_LIMIT)  /* 0xf981_0000 */

e.g. using optee/build

make cleaner
make lloader /* implicitly invokes optee_os, edk2, arm-tf, atf-fastboot and builds lloader.bin */
make recovery /* to flash lloader and fip */

Output would be:

INFO:    BL1: 0xf9811000 - 0xf9816000 [size = 20480]
NOTICE:  Booting Trusted Firmware
NOTICE:  BL1: v1.4(release):v1.4-673-gb13b11a-dirty
NOTICE:  BL1: Built : 11:24:54, Feb 23 2018
INFO:    BL1: RAM 0xf9811000 - 0xf9816000
NOTICE:  BL1-FWU: *******FWU Process Started*******
INFO:    cpacr_el1:0x300000
INFO:    Entry point address = 0xf9819000
INFO:    SPSR = 0x3c5

As you can see ... Entry point changed from 0xf9818000 to 0xf9819000, but atf-fastboot does not start to flash images.

Which additional steps are necessary to increase space for an image, e.g. BL1?

hzhuang1 commented 6 years ago

I suggest to hold it on. We're preparing to migrate BL2_EL3. When it's done, consider it again.

With your comment, it's not clear whether you want to change the memory map in recovery mode or normal boot mode. Do you want to change both?

gitfineon commented 6 years ago

I want to add a feature to BL1 in normal boot mode, which requires more memory than currently available. BL2_EL3 will not solve my issue ...

hzhuang1 commented 6 years ago

No. When we migrate to BL2_EL3, more memory space is spare since BL1 is dropped.

gitfineon commented 6 years ago

Dropped? In general or for the HiSilicon platforms? Can you give some more information about this plan? Is there a official document/roadmap? I just found this commit from @robertovargas-arm to the firmware-design document.

bl2-el3: Add documentation for BL2 at EL3

BootRom should be open source, too!

hzhuang1 commented 6 years ago

https://github.com/ARM-software/arm-trusted-firmware/pull/1277

It's in the queue to be merged.

ghost commented 6 years ago

Is this resolved?

hzhuang1 commented 6 years ago

It has been fixed already.

ghost commented 6 years ago

By the way, @hzhuang1, I've noticed that you are adding support for Cortex-A72 in the makefile, is it expected? https://github.com/ARM-software/arm-trusted-firmware/blob/210d90a985afe42c4d17f84f94e82f73a4ba306c/plat/hisilicon/hikey960/platform.mk#L96

Also, shouldn't all the files related to BL1 be removed? The platform uses BL2_AT_EL3 so BL1 is not needed.

vchong commented 4 years ago

Adding a note here for posterity, also in case I forget when coming back here for references in the future.

So this functionality (adding a feature to BL1, e.g. Trusted Board Boot (TBB)) has been implemented below: https://github.com/ARM-software/arm-trusted-firmware/pull/1449 https://github.com/ARM-software/arm-trusted-firmware/pull/1487 https://github.com/96boards-hikey/atf-fastboot/pull/10/files

But for this issue here, before the 3 PRs above were implemented, it appears that an increase in BL1_RO size in arm-trusted-firmware (as seen in https://github.com/ARM-software/arm-trusted-firmware/pull/1487/files#diff-4a534b6366574e5bce27c8f83839c78c) requires a corresponding decrease (NOT increase) in BL1_RW size in atf-fastboot (as seen in https://github.com/96boards-hikey/atf-fastboot/pull/10/files#diff-2ddb50e5d016d3d3c94787c713f965ed) to balance up the changes in size. Also NS_BL1U_BASE in arm-trusted-firmware seems to shifted as a result, which requires a corresponding change in BL1_RO_BASE in atf-fastboot, since these 2 have to match.