Dependency on Amlogic's fip_create

[afaerber]

The amlbootsig tool depends on pre-aligned input specific to the fip_create tool in the Hardkernel fork.

The tool should be amended to do the necessary alignment itself, based on an upstream fiptool input (modulo the Amlogic BL3-0-1 UUID).

That would avoid distros needing to build and package a custom fip_create tool.

[xypron]

Hardkernel's fip_create is based on the tool of the same name in ARM Trusted Platform, version 0.4 https://github.com/ARM-software/arm-trusted-firmware

Amlogic has made the following changes:

image_offset and toc_size have hard coded values (0x4000).

toc_entry_lookup_list contains an Amlogic specific entry:

{ "SCP Firmware BL3-0-1", UUID_SCP_FIRMWARE_BL301, "bl301", NULL, FLAG_FILENAME}, 
#define UUID_SCP_FIRMWARE_BL301 {0xAABBCCDD, 0xABCD, 0xEFEF, 0xAB, 0xCD, {0x12, 0x34, 0x56, 0x78, 0xAB, 0xCD} }

I agree it would be preferable to rely on the original ARM code enhanced by the missing UUID.

Thank you for working in this direction.

Best regards

Heinrich Schuchardt

[afaerber]

Thanks, I am well aware of arm-trusted-firmware, that's what "fork" referred to. You will find comments of mine related to the introduction of fiptool complicating forward-porting of fip_create patches.

As pointed out in bdo#860834, that assessment about a fixed 0x4000 is incorrect. Here is my last diff from around the introduction of fiptool:

diff --git a/tools/fiptool/fiptool.c b/tools/fiptool/fiptool.c
index b3f02f6c..bc0ddce4 100644
--- a/tools/fiptool/fiptool.c
+++ b/tools/fiptool/fiptool.c
@@ -389,6 +389,8 @@ static int info_cmd(int argc, char *argv[])
 
        image_offset = sizeof(fip_toc_header_t) +
            (sizeof(fip_toc_entry_t) * (nr_images + 1));
+       if (1)
+               image_offset = 0x4000;
 
        for (i = 0; i < nr_images; i++) {
                image = images[i];
@@ -411,7 +413,10 @@ static int info_cmd(int argc, char *argv[])
                        md_print(md, sizeof(md));
                }
                putchar('\n');
-               image_offset += image_size;
+               if (1)
+                       image_offset += 0x4000 * ((image_size / 0x4000) + 1);
+               else
+                       image_offset += image_size;
        }
 
        free_images();

You are overlooking the second hunk. Only the first hunk hardcodes 0x4000, in the knowledge that the TOC will be smaller.

Note how that is different from a simple ROUND_UP(image_size, 0x4000). This has to do with the headers being inserted by aml_encrypt_gxb needing to fit into 0x4000 as well, which a pure 0x4000 alignment by fiptool --align 0x4000 will not handle. So what's really needed here, I think, is a ROUND_UP(image_size + sizeof(struct AmlogicHeader), 0x4000). (Which in turn would mean that Hardkernel's fip_create is buggy for certain image_size values...)

An optimized implementation of amlbootsig would differ in 0x4000 bytes in most cases, losing the hex diff comparability that I have relied on for verifying the implementation. I.e., we'd need some command line switch (or at least a preprocessor define) to turn this feature on/off. => v0.2 material

On second thoughts it might be easier to add some --align-reserve 64 option to upstream fiptool than to rewrite my amlbootsig logic. That would still lead to output differences due to a saner calculation.

[afaerber]

When using fiptool --align 16384 together with the --blob uuid=AABBCCDD-ABCD-EFEF-ABCD-12345678ABCD,file=... option, it places the bl301 last, after bl33. (I recall there were upstream discussions about order, but can't find it right now...) This leads to boot failure:

BL2 Built : 11:44:26, Nov 25 2015. 
gxb gfb13a3b-c2 - jcao@wonton

Board ID = 8
set vcck to 1100 mv
set vddee to 1050 mv
CPU clk: 1536MHz
DDR channel setting: DDR0 Rank0+1 same
DDR0: 2048MB(auto) @ 912MHz(2T)-13
DataBus test pass!
AddrBus test pass!
Load fip header from SD, src: 0x0000c200, des: 0x01400000, size: 0x000000b0
Load bl30 from SD, src: 0x00010200, des: 0x01000000, size: 0x00009ef0
Sending bl30........................................OK. 
Run bl30...
Load bl301 from SD, src: 0x0001c200, des: 0x01000000, size: 0x000017c0
Wait bl30...Done
Sending bl301......OK. 
Run bl301...
 from SD, src: 0x00020200, des: 0x10100000, size: 0x00011130

When implementing bl301 in fiptool in between bl2 and bl31 it succeeds, so there appear to be hardcoded offset (0x0001c200) and size (0x000017c0) assumptions for bl301.

BL2 Built : 11:44:26, Nov 25 2015. 
gxb gfb13a3b-c2 - jcao@wonton

Board ID = 8
set vcck to 1100 mv
set vddee to 1050 mv
CPU clk: 1536MHz
DDR channel setting: DDR0 Rank0+1 same
DDR0: 2048MB(auto) @ 912MHz(2T)-13
DataBus test pass!
AddrBus test pass!
Load fip header from SD, src: 0x0000c200, des: 0x01400000, size: 0x000000b0
Load bl30 from SD, src: 0x00010200, des: 0x01000000, size: 0x00009ef0
Sending bl30........................................OK. 
Run bl30...
Load bl301 from SD, src: 0x0001c200, des: 0x01000000, size: 0x000017c0
Wait bl30...Done
Sending bl301......OK. 
Run bl301...
 from SD, src: 0x00020200, des: 0x10100000, size: 0x00011130


--- UART initialized after reboot ---
[Reset cause: unknown]
[Image: unknown, amlogic_v1.1.3046-00db630 2015-10-28 15:24:31 xiaobo.gu@droid05]
bl30: check_permit, count is 1
bl30: check_permit: ok!
chipid: ef be ad de d f0 aLoad bl33 from SD, src: 0x00034200, des: 0x01000000, size: 0x00056710
d ba ef be ad de not ES chip
[0.338122 Inits done]
secure task start!
high task start!
low task start!
NOTICE:  BL3-1: v1.0(debug):4d2e34d
NOTICE:  BL3-1: Built : 17:08:35, Oct 29 2015
INFO:    BL3-1: Initializing runtime services
INFO:    BL3-1: Preparing for EL3 exit to normal world
INFO:    BL3-1: Next image address = 0x1000000
INFO:    BL3-1: Next image spsr = 0x3c9


U-Boot 2017.05-rc1-00308-gcdf670e (Apr 15 2017 - 06:44:41 +0200) odroid-c2

[xypron]

Does your test really prove that the address is hard coded? Or does it only imply that the code does not care about the UUIDs in the tasble of contents and assumes a certain sequence of the binaries?

I guess first we need a signing tool that can work around the 0x4000 alignment to figure that out.

Best regards

Heinrich Schuchardt

[afaerber]

The size proves that it is not taking the n-th TOC entry, otherwise it would be larger.

I've pushed patches against latest arm-trusted-firmware - the last test result above was already using an earlier version of the UUID addition (even without 64 byte alignment tweak). With those fiptool-side changes no complicated resizing should be necessary in amlbootsig any more. Did I miss something?