Re: [u-boot PATCH 3/3] k3-am642-evm-u-boot: Use binman to generate u-boot.img and tispl.bin

Roger Quadros
2022-05-26T07:29:01Z

On 25/05/2022 18:14, Andrew Davis wrote:
> On 5/25/22 3:30 AM, Roger Quadros wrote:
>> Hi Andrew,
>>
>> On 25/05/2022 01:03, Andrew Davis wrote:
>>> On 5/9/22 2:29 AM, Roger Quadros wrote:
>>>> Introduce k3-am642-evm-binman.dtsi to provide binman configuration.
>>>>
>>>> R5 build is still not converted to use binman so restrict binman.dtsi
>>>> to A53 builds only.
>>>>
>>>> This patch also take care of building Secure (HS) images using
>>>> binman instead of tools/k3_fit_atf.sh if CONFIG_BINMAN is set.
>>>>
>>>> Signed-off-by: Roger Quadros <rogerq@kernel.org>
>>>> ---
>>>>    arch/arm/dts/k3-am642-evm-binman.dtsi | 230 ++++++++++++++++++++++++++
>>>>    arch/arm/dts/k3-am642-evm-u-boot.dtsi |   3 +
>>>>    arch/arm/mach-k3/Kconfig              |   1 +
>>>>    arch/arm/mach-k3/config.mk            |   7 +
>>>>    4 files changed, 241 insertions(+)
>>>>    create mode 100644 arch/arm/dts/k3-am642-evm-binman.dtsi
>>>>
>>>> diff --git a/arch/arm/dts/k3-am642-evm-binman.dtsi b/arch/arm/dts/k3-am642-evm-binman.dtsi
>>>> new file mode 100644
>>>> index 0000000000..9e85ef41b0
>>>> --- /dev/null
>>>> +++ b/arch/arm/dts/k3-am642-evm-binman.dtsi
>>>> @@ -0,0 +1,230 @@
>>>> +// SPDX-License-Identifier: GPL-2.0
>>>> +/*
>>>> + * Copyright (C) 2021 Texas Instruments Incorporated - https://www.ti.com/
>>>> + */
>>>> +
>>>> +/ {
>>>> +    binman: binman {
>>>> +        multiple-images;
>>>> +    };
>>>> +};
>>>> +
>>>> +#ifdef CONFIG_TARGET_AM642_A53_EVM
>>>> +
>>>> +#ifdef CONFIG_TI_SECURE_DEVICE
>>>> +#define TISPL "tispl.bin_HS"
>>>> +#define UBOOT_IMG "u-boot.img_HS"
>>>> +#else
>>>> +#define TISPL "tispl.bin"
>>>> +#define UBOOT_IMG "u-boot.img"
>>>> +#endif
>>>> +
>>>> +#define SPL_NODTB "spl/u-boot-spl-nodtb.bin"
>>>> +#define SPL_AM642_EVM_DTB "spl/dts/k3-am642-evm.dtb"
>>>> +#define SPL_AM642_SK_DTB "spl/dts/k3-am642-sk.dtb"
>>>> +
>>>> +#define UBOOT_NODTB "u-boot-nodtb.bin"
>>>> +#define AM642_EVM_DTB "arch/arm/dts/k3-am642-evm.dtb"
>>>> +#define AM642_SK_DTB "arch/arm/dts/k3-am642-sk.dtb"
>>>> +
>>>> +&binman {
>>>> +    ti-spl {
>>>> +        filename = TISPL;
>>>> +        pad-byte = <0xff>;
>>>> +
>>>> +        fit {
>>>> +            description = "Configuration to load ATF and SPL";
>>>> +            #address-cells = <1>;
>>>> +
>>>> +            images {
>>>> +
>>>> +                atf {
>>>> +                    description = "ARM Trusted Firmware";
>>>> +                    type = "firmware";
>>>> +                    arch = "arm64";
>>>> +                    compression = "none";
>>>> +                    os = "arm-trusted-firmware";
>>>> +                    load = <CONFIG_K3_ATF_LOAD_ADDR>;
>>>> +                    entry = <CONFIG_K3_ATF_LOAD_ADDR>;
>>>> +                    atf-bl31 {
>>>> +                        filename = "bl31.bin";
>>>> +                    };
>>>
>>>
>>> On HS, bl31.bin and the below TEE and DM images must also be signed
>>> before being packaged into tispl.bin.
>>> Can we add signing here?
>>
>> I'm wondering how this is working as is on HS boards.
>>
>
>
> Today we manually sign those two before we feed them to U-Boot build.
> I'd like to fix that and have them signed along with all the other
> parts here when packaging them together.
>

OK. Then this is new feature. Do you mind if I make a separate patch for it?
But first I need to figure out what to do ;)

>
>> Another thing to note is that the atf and tee entries take into consideration
>> the below environment variables
>>                  -a atf-bl31-path=${BL31} \
>>                  -a tee-os-path=${TEE} \
>>
>> How do we continue to support that while adding the signing bits?
>>
>
>
> That's my question also, I'm not sure how we would make the type 'ti-secure'
> while also changing their path names, seems like a limitation currently
> of using etypes to do the signing, since we can do path renames from
> command line.

Simon,

Any thoughts on how to get the new ti-secure etype work with atf-bl31 and
tee-os etypes so that it can take the data output of those entries and create
a signed binary with filenames from those entries or atf-bl31-path and
tee-os-path?

Can something like this work?

ti-secure {
atf-bl31 {
filename = "bl31.bin";
};
}

We could probably get rid of filename property from ti-secure etype and use
blob for regular files.

ti-secure {
blob {
filename = "somefile.ext";
}
}

cheers,
-roger

>
> Andrew
>
>
>> cheers,
>> -roger
>>
>>>
>>> Andrew
>>>
>>>
>>>> +                };
>>>> +
>>>> +                tee {
>>>> +                    description = "OPTEE";
>>>> +                    type = "tee";
>>>> +                    arch = "arm64";
>>>> +                    compression = "none";
>>>> +                    os = "tee";
>>>> +                    load = <0x9e800000>;
>>>> +                    entry = <0x9e800000>;
>>>> +                    tee-os {
>>>> +                        filename = "tee-pager_v2.bin";
>>>> +                    };
>>>> +                };
>>>> +
>>>> +                dm {
>>>> +                    description = "DM binary";
>>>> +                    type = "firmware";
>>>> +                    arch = "arm32";
>>>> +                    compression = "none";
>>>> +                    os = "DM";
>>>> +                    load = <0x89000000>;
>>>> +                    entry = <0x89000000>;
>>>> +                    blob-ext {
>>>> +                        filename = "/dev/null";
>>>> +                    };
>>>> +                };
>>>> +
>>>> +                spl {
>>>> +                    description = "SPL (64-bit)";
>>>> +                    type = "standalone";
>>>> +                    os = "U-Boot";
>>>> +                    arch = "arm64";
>>>> +                    compression = "none";
>>>> +                    load = <0x80080000>;
>>>> +                    entry = <0x80080000>;
>>>> +#ifdef CONFIG_TI_SECURE_DEVICE
>>>> +                    ti-secure {
>>>> +#else
>>>> +                    blob {
>>>> +#endif
>>>> +                        filename = SPL_NODTB;
>>>> +                    };
>>>> +                };
>>>> +
>>>> +                fdt-1 {
>>>> +                    description = "k3-am642-evm";
>>>> +                    type = "flat_dt";
>>>> +                    arch = "arm";
>>>> +                    compression = "none";
>>>> +#ifdef CONFIG_TI_SECURE_DEVICE
>>>> +                    ti-secure {
>>>> +#else
>>>> +                    blob {
>>>> +#endif
>>>> +                        filename = SPL_AM642_EVM_DTB;
>>>> +                    };
>>>> +                };
>>>> +
>>>> +                fdt-2 {
>>>> +                    description = "k3-am642-sk";
>>>> +                    type = "flat_dt";
>>>> +                    arch = "arm";
>>>> +                    compression = "none";
>>>> +#ifdef CONFIG_TI_SECURE_DEVICE
>>>> +                    ti-secure {
>>>> +#else
>>>> +                    blob {
>>>> +#endif
>>>> +                        filename = SPL_AM642_SK_DTB;
>>>> +                    };
>>>> +                };
>>>> +            };
>>>> +
>>>> +            configurations {
>>>> +                default = "conf-1";
>>>> +
>>>> +                conf-1 {
>>>> +                    description = "k3-am642-evm";
>>>> +                    firmware = "atf";
>>>> +                    loadables = "tee", "dm", "spl";
>>>> +                    fdt = "fdt-1";
>>>> +                };
>>>> +
>>>> +                conf-2 {
>>>> +                    description = "k3-am642-sk";
>>>> +                    firmware = "atf";
>>>> +                    loadables = "tee", "dm", "spl";
>>>> +                    fdt = "fdt-2";
>>>> +                };
>>>> +            };
>>>> +        };
>>>> +    };
>>>> +};
>>>> +
>>>> +&binman {
>>>> +    u-boot {
>>>> +        filename = UBOOT_IMG;
>>>> +        pad-byte = <0xff>;
>>>> +
>>>> +        fit {
>>>> +            description = "FIT image with multiple configurations";
>>>> +
>>>> +            images {
>>>> +                uboot {
>>>> +                    description = "U-Boot for am64x board";
>>>> +                    type = "firmware";
>>>> +                    os = "u-boot";
>>>> +                    arch = "arm";
>>>> +                    compression = "none";
>>>> +                    load = <CONFIG_SYS_TEXT_BASE>;
>>>> +#ifdef CONFIG_TI_SECURE_DEVICE
>>>> +                    ti-secure {
>>>> +#else
>>>> +                    blob {
>>>> +#endif
>>>> +                        filename = UBOOT_NODTB;
>>>> +                    };
>>>> +                    hash {
>>>> +                        algo = "crc32";
>>>> +                    };
>>>> +                };
>>>> +
>>>> +                fdt-1 {
>>>> +                    description = "k3-am642-evm";
>>>> +                    type = "flat_dt";
>>>> +                    arch = "arm";
>>>> +                    compression = "none";
>>>> +#ifdef CONFIG_TI_SECURE_DEVICE
>>>> +                    ti-secure {
>>>> +#else
>>>> +                    blob {
>>>> +#endif
>>>> +                        filename = AM642_EVM_DTB;
>>>> +                    };
>>>> +                    hash {
>>>> +                        algo = "crc32";
>>>> +                    };
>>>> +                };
>>>> +
>>>> +                fdt-2 {
>>>> +                    description = "k3-am642-sk";
>>>> +                    type = "flat_dt";
>>>> +                    arch = "arm";
>>>> +                    compression = "none";
>>>> +#ifdef CONFIG_TI_SECURE_DEVICE
>>>> +                    ti-secure {
>>>> +#else
>>>> +                    blob {
>>>> +#endif
>>>> +                        filename = AM642_SK_DTB;
>>>> +                    };
>>>> +                    hash {
>>>> +                        algo = "crc32";
>>>> +                    };
>>>> +                };
>>>> +            };
>>>> +
>>>> +            configurations {
>>>> +                default = "conf-1";
>>>> +
>>>> +                conf-1 {
>>>> +                    description = "k3-am642-evm";
>>>> +                    firmware = "uboot";
>>>> +                    loadables = "uboot";
>>>> +                    fdt = "fdt-1";
>>>> +                };
>>>> +
>>>> +                conf-2 {
>>>> +                    description = "k3-am642-sk";
>>>> +                    firmware = "uboot";
>>>> +                    loadables = "uboot";
>>>> +                    fdt = "fdt-2";
>>>> +                };
>>>> +            };
>>>> +        };
>>>> +    };
>>>> +};
>>>> +#endif
>>>> diff --git a/arch/arm/dts/k3-am642-evm-u-boot.dtsi b/arch/arm/dts/k3-am642-evm-u-boot.dtsi
>>>> index 03688a51a3..db0a529f0f 100644
>>>> --- a/arch/arm/dts/k3-am642-evm-u-boot.dtsi
>>>> +++ b/arch/arm/dts/k3-am642-evm-u-boot.dtsi
>>>> @@ -2,6 +2,9 @@
>>>>    /*
>>>>     * Copyright (C) 2020-2021 Texas Instruments Incorporated - https://www.ti.com/
>>>>     */
>>>> +#include <config.h>
>>>> +
>>>> +#include "k3-am642-evm-binman.dtsi"
>>>>      / {
>>>>        chosen {
>>>> diff --git a/arch/arm/mach-k3/Kconfig b/arch/arm/mach-k3/Kconfig
>>>> index a01bf23514..a4c561254d 100644
>>>> --- a/arch/arm/mach-k3/Kconfig
>>>> +++ b/arch/arm/mach-k3/Kconfig
>>>> @@ -15,6 +15,7 @@ config SOC_K3_J721S2
>>>>      config SOC_K3_AM642
>>>>        bool "TI's K3 based AM642 SoC Family Support"
>>>> +    select BINMAN if TARGET_AM642_A53_EVM
>>>>      endchoice
>>>>    diff --git a/arch/arm/mach-k3/config.mk b/arch/arm/mach-k3/config.mk
>>>> index da458bcfb2..d2c490818a 100644
>>>> --- a/arch/arm/mach-k3/config.mk
>>>> +++ b/arch/arm/mach-k3/config.mk
>>>> @@ -47,6 +47,7 @@ tiboot3.bin: image_check FORCE
>>>>    INPUTS-y    += tiboot3.bin
>>>>    endif
>>>>    +ifndef CONFIG_BINMAN
>>>>    ifdef CONFIG_ARM64
>>>>      ifeq ($(CONFIG_SOC_K3_J721E),)
>>>> @@ -77,9 +78,11 @@ cmd_k3_mkits = \
>>>>    $(SPL_ITS): FORCE
>>>>        $(call cmd,k3_mkits)
>>>>    endif
>>>> +endif
>>>>      else
>>>>    +ifndef CONFIG_BINMAN
>>>>    ifeq ($(CONFIG_TI_SECURE_DEVICE),y)
>>>>    INPUTS-y    += u-boot.img_HS
>>>>    else
>>>> @@ -87,4 +90,8 @@ INPUTS-y    += u-boot.img
>>>>    endif
>>>>    endif
>>>>    +endif
>>>> +
>>>> +ifndef CONFIG_BINMAN
>>>>    include $(srctree)/arch/arm/mach-k3/config_secure.mk
>>>> +endif