Re: [PATCH RFC v2 11/11] ti: dtsi: j721e: Use binman to package tispl.bin

From: Roger Quadros <rogerq@kernel.org>
To: Andrew Davis <afd@ti.com>, Neha Malcom Francis <n-francis@ti.com>,
	u-boot@lists.denx.de
Cc: Alper Nebi Yasak <alpernebiyasak@gmail.com>
Subject: Re: [PATCH RFC v2 11/11] ti: dtsi: j721e: Use binman to package tispl.bin
Date: Fri, 3 Jun 2022 11:49:19 +0300	[thread overview]
Message-ID: <86cd403e-8b48-6878-ac29-7959aec2321d@kernel.org> (raw)
In-Reply-To: <248e9917-6086-5ca4-d76e-0077368009c0@ti.com>

On 01/06/2022 15:47, Andrew Davis wrote:
> On 6/1/22 5:55 AM, Roger Quadros wrote:
>>
>>
>> On 01/06/2022 13:42, Neha Malcom Francis wrote:
>>> Hi Roger,
>>>
>>> On 01/06/22 14:53, Roger Quadros wrote:
>>>> Hi,
>>>>
>>>> On 01/06/2022 09:08, Neha Malcom Francis wrote:
>>>>> Hi Roger,
>>>>>
>>>>> On 31/05/22 16:32, Roger Quadros wrote:
>>>>>>
>>>>>>
>>>>>> On 06/05/2022 07:37, Neha Malcom Francis wrote:
>>>>>>> Explicit make commands were earlier used to generate tispl.bin image,
>>>>>>> now it is replaced using binman.
>>>>>>>
>>>>>>> Binman picks up and packages entries according to the description of
>>>>>>> entries given in the binman node in the device tree. The make commands
>>>>>>> that were earlier responsible for generating tispl.bin has been removed.
>>>>>>>
>>>>>>> k3-j721e-a72-binman.dtsi has been introduced for A72 specific binman node.
>>>>>>> It can be included in files that require it like
>>>>>>> k3-j721e-common-proc-board-u-boot.dtsi.
>>>>>>>
>>>>>>> Note that make commands for secure devices has also been removed as
>>>>>>> focus is on general purpose devices at present time.
>>>>>>>
>>>>>>> Signed-off-by: Tarun Sahu <t-sahu@ti.com>
>>>>>>> [n-francis@ti.com: prepared patch for upstreaming]
>>>>>>> Signed-off-by: Neha Malcom Francis <n-francis@ti.com>
>>>>>>> ---
>>>>>>>     arch/arm/dts/k3-j721e-a72-binman.dtsi         | 86 +++++++++++++++++++
>>>>>>>     .../k3-j721e-common-proc-board-u-boot.dtsi    |  1 +
>>>>>>>     arch/arm/mach-k3/config.mk                    | 33 -------
>>>>>>>     board/ti/j721e/Kconfig                        |  1 +
>>>>>>>     scripts/Makefile.spl                          |  4 -
>>>>>>>     5 files changed, 88 insertions(+), 37 deletions(-)
>>>>>>>     create mode 100644 arch/arm/dts/k3-j721e-a72-binman.dtsi
>>>>>>>
>>>>>>> diff --git a/arch/arm/dts/k3-j721e-a72-binman.dtsi b/arch/arm/dts/k3-j721e-a72-binman.dtsi
>>>>>>> new file mode 100644
>>>>>>> index 0000000000..beb3424bb9
>>>>>>> --- /dev/null
>>>>>>> +++ b/arch/arm/dts/k3-j721e-a72-binman.dtsi
>>>>>>> @@ -0,0 +1,86 @@
>>>>>>> +// SPDX-License-Identifier: GPL-2.0+
>>>>>>> +// Copyright (C) 2022 Texas Instruments Incorporated - https://www.ti.com/
>>>>>>> +
>>>>>>> +#include <config.h>
>>>>>>> +
>>>>>>> +#ifdef CONFIG_ARM64
>>>>>>> +/ {
>>>>>>> +    binman: binman {
>>>>>>> +        multiple-images;
>>>>>>> +    };
>>>>>>> +};
>>>>>>> +
>>>>>>> +&binman {
>>>>>>> +    tispl {
>>>>>>> +        filename = "tispl.bin";
>>>>>>> +        fit {
>>>>>>> +            description = "FIT IMAGE";
>>>>>>> +            #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 {
>>>>>>> +                    };
>>>>>>> +                };
>>>>>>> +                tee {
>>>>>>> +                    description = "OPTEE";
>>>>>>> +                    type = "tee";
>>>>>>> +                    arch = "arm64";
>>>>>>> +                    compression = "none";
>>>>>>> +                    os = "tee";
>>>>>>> +                    load = <0x9e800000>;
>>>>>>> +                    entry = <0x9e800000>;
>>>>>>> +                    tee-os {
>>>>>>> +                    };
>>>>>>> +                };
>>>>>>> +                dm {
>>>>>>> +                    description = "DM binary";
>>>>>>> +                    type = "firmware";
>>>>>>> +                    arch = "arm32";
>>>>>>> +                    compression = "none";
>>>>>>> +                    os = "DM";
>>>>>>> +                    load = <0x89000000>;
>>>>>>> +                    entry = <0x89000000>;
>>>>>>> +                    ti-dm {
>>>>>>> +                    };
>>>>>>> +                };
>>>>>>> +                spl {
>>>>>>> +                    description = "SPL (64-bit)";
>>>>>>> +                    type = "standalone";
>>>>>>> +                    os = "U-Boot";
>>>>>>> +                    arch = "arm64";
>>>>>>> +                    compression = "none";
>>>>>>> +                    load = <CONFIG_SPL_TEXT_BASE>;
>>>>>>> +                    entry = <CONFIG_SPL_TEXT_BASE>;
>>>>>>> +                    u-boot-spl-nodtb {
>>>>>>> +                    };
>>>>>>> +                };
>>>>>>> +                k3-j721e-common-proc-board.dtb {
>>>>>>> +                    description = "k3-j721e-common-proc-board";
>>>>>>> +                    type = "flat_dt";
>>>>>>> +                    arch = "arm";
>>>>>>> +                    compression = "none";
>>>>>>> +                    blob-ext {
>>>>>>> +                    filename = "spl/dts/k3-j721e-common-proc-board.dtb";
>>>>>>> +                    };
>>>>>>> +                };
>>>>>>> +            };
>>>>>>> +            configurations {
>>>>>>> +                default = "conf";
>>>>>>> +                conf {
>>>>>>> +                    description = "k3-j721e-common-proc-board";
>>>>>>> +                    firmware = "atf";
>>>>>>> +                    loadables = "tee", "dm", "spl";
>>>>>>> +                    fdt = "k3-j721e-common-proc-board.dtb";
>>>>>>> +                };
>>>>>>> +            };
>>>>>>> +        };
>>>>>>> +    };
>>>>>>> +};
>>>>>>> +#endif
>>>>>>> diff --git a/arch/arm/dts/k3-j721e-common-proc-board-u-boot.dtsi b/arch/arm/dts/k3-j721e-common-proc-board-u-boot.dtsi
>>>>>>> index 677a72d2a2..6490d71f7e 100644
>>>>>>> --- a/arch/arm/dts/k3-j721e-common-proc-board-u-boot.dtsi
>>>>>>> +++ b/arch/arm/dts/k3-j721e-common-proc-board-u-boot.dtsi
>>>>>>> @@ -4,6 +4,7 @@
>>>>>>>      */
>>>>>>>       #include <dt-bindings/net/ti-dp83867.h>
>>>>>>> +#include "k3-j721e-a72-binman.dtsi"
>>>>>>>       / {
>>>>>>>         chosen {
>>>>>>> diff --git a/arch/arm/mach-k3/config.mk b/arch/arm/mach-k3/config.mk
>>>>>>> index 49f80ae79b..e0d9ea97c9 100644
>>>>>>> --- a/arch/arm/mach-k3/config.mk
>>>>>>> +++ b/arch/arm/mach-k3/config.mk
>>>>>>> @@ -52,38 +52,5 @@ ifeq ($(CONFIG_SOC_K3_J721E),)
>>>>>>>     export DM := /dev/null
>>>>>>>     endif
>>>>>>>     -ifeq ($(CONFIG_TI_SECURE_DEVICE),y)
>>>>>>> -SPL_ITS := u-boot-spl-k3_HS.its
>>>>>>> -$(SPL_ITS): export IS_HS=1
>>>>>>> -INPUTS-y    += tispl.bin_HS
>>>>>>> -else
>>>>>>> -SPL_ITS := u-boot-spl-k3.its
>>>>>>> -INPUTS-y    += tispl.bin
>>>>>>> -endif
>>>>>>> -
>>>>>>> -ifeq ($(CONFIG_SPL_OF_LIST),)
>>>>>>> -LIST_OF_DTB := $(CONFIG_DEFAULT_DEVICE_TREE)
>>>>>>> -else
>>>>>>> -LIST_OF_DTB := $(CONFIG_SPL_OF_LIST)
>>>>>>>     endif
>>>>>>> -
>>>>>>> -quiet_cmd_k3_mkits = MKITS   $@
>>>>>>> -cmd_k3_mkits = \
>>>>>>> -    $(srctree)/tools/k3_fit_atf.sh \
>>>>>>> -    $(CONFIG_K3_ATF_LOAD_ADDR) \
>>>>>>> -    $(patsubst %,$(obj)/dts/%.dtb,$(subst ",,$(LIST_OF_DTB))) > $@
>>>>>>> -
>>>>>>> -$(SPL_ITS): FORCE
>>>>>>> -    $(call cmd,k3_mkits)
>>>>>>>     endif
>>>>>>> -
>>>>>>> -else
>>>>>>> -
>>>>>>> -ifeq ($(CONFIG_TI_SECURE_DEVICE),y)
>>>>>>> -INPUTS-y    += u-boot.img_HS
>>>>>>> -else
>>>>>>> -INPUTS-y    += u-boot.img
>>>>>>> -endif
>>>>>>> -endif
>>>>>>> -
>>>>>>> -include $(srctree)/arch/arm/mach-k3/config_secure.mk
>>>>>>
>>>>>> You can't yet remove these as all K3 platforms are not converted to use binman.
>>>>>> Please include them under #ifndef CONFIG_BINMAN...#endif
>>>>>>
>>>>>> One more question I have is that how are _HS binaries being created with binman?
>>>>>>
>>>>>
>>>>> I will make the changes accordingly that do not affect any of the other builds. _HS binaries are not being created with binman in this series.
>>>>>
>>>>> But there is a patch series that does create HS images (tispl.bin and u-boot.img) using binman for AM64x: https://patchwork.ozlabs.org/project/uboot/patch/20220509072936.12899-4-rogerq@kernel.org/
>>>>
>>>> That series still depends on "TI_SECURE_DEV_PKG//scripts/secure-binary-image.sh"
>>>> to sign the image.
>>>>
>>>> Funny thing is that I'm no longer able to download the above package from ti.com
>>>> to see what it is doing. Any pointers on how to get it?
>>>>
>>>> cheers,
>>>> -roger
>>>>
>>>
>>> Did you try to access using the browser using https link?
>>>
>>> I am able to access the script by cloning git://git.ti.com/security-development-tools/core-secdev-k3.git
>>>
>>
>> Aha! sure I can access that.
>> But according to doc/README.ti-secure I should be getting it from
>> http://www.ti.com/mysecuresoftware
>> And I couldn't get anything there.
>>
>> Looks like we need to update the doc to point to the git repo instead?
>>
> 
> 
> The mysecuresoftware is for the AM335x-AM57x class devices, that is still
> the place to get the tools for those. For K3 forward the git.ti.com is
> the right spot. We can add a new doc for K3 devices.
> 

Understood now. Thanks for the clarification.

cheers,
-roger