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

Tom Rini
2022-05-26T14:15:41Z

[-- Attachment #1: Type: text/plain, Size: 13444 bytes --]

On Thu, May 26, 2022 at 10:28:45AM +0300, Roger Quadros wrote:
> 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";
> }
> }

Adding in Alper as well..

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

--
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]