Date: Tue, 1 Feb 2022 20:01:26 +0100
From: "Yann E. MORIN" <yann.morin.1998@free.fr>
To: Michael Nazzareno Trimarchi <michael@amarulasolutions.com>
Message-ID: <20220201190126.GW457876@scaer>
References: <20220201091342.143010-1-michael@amarulasolutions.com>
 <20220201171441.GV457876@scaer>
 <CAOf5uwmK-+jU3eRRPQ4NqKaFq=OUSBTK-_kRXTc6cEsGiwm=Gg@mail.gmail.com>
MIME-Version: 1.0
Content-Disposition: inline
In-Reply-To: <CAOf5uwmK-+jU3eRRPQ4NqKaFq=OUSBTK-_kRXTc6cEsGiwm=Gg@mail.gmail.com>
User-Agent: Mutt/1.5.22 (2013-10-16)
Subject: Re: [Buildroot] [PATCH] boot/uboot: add support for bundling TEE in
 BIN format into U-Boot
Precedence: list
Cc: Ariel D'Alessandro <ariel.dalessandro@collabora.com>,
 Jagan Teki <jagan@amarulasolutions.com>, buildroot@buildroot.org
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: buildroot-bounces@buildroot.org
Sender: "buildroot" <buildroot-bounces@buildroot.org>

Michael, All,

On 2022-02-01 18:57 +0100, Michael Nazzareno Trimarchi spake thusly:
> On Tue, Feb 1, 2022 at 6:14 PM Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> > On 2022-02-01 10:13 +0100, Michael Trimarchi spake thusly:
> > > Some U-Boot platforms (e.g. nxp) can bundle OPTEE's tee.bin
> > > into the U-Boot image using binman. This patch brings the necessary changes to
> > > enable this feature.

Also, comit log not nicely formatted: it should be wrapped at ~72 chars
on every lines.

> > > Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> > > ---
[--SNIP--]
> > >  ifeq ($(BR2_TARGET_UBOOT_NEEDS_OPTEE_TEE),y)
> > >  UBOOT_DEPENDENCIES += optee-os
> > > +ifeq ($(BR2_TARGET_UBOOT_NEEDS_OPTEE_ELF),y)
> > >  UBOOT_MAKE_OPTS += TEE=$(BINARIES_DIR)/tee.elf
                         ,^^^^^^^^^^^^^^^^^^^^^^^^^^^
Note this for later ----/

> > > +define UBOOT_COPY_TEE_FIRMWARE
> > > +     cp $(BINARIES_DIR)/tee.elf $(@D)/
> > Why do you need to copy it, when TEE is pointing to the original in
> > $(BINARIESD_DIR), just above, and that has worked well so far?
> The copy are needed to binman to include it

As far as I understand it, BR2_TARGET_UBOOT_NEEDS_OPTEE_ELF is supposed
to behave like the code we had so far, and so far we did not have the
copy, and that seems that it worked OK without the copy.

Now you are adding the copy: why was it not needed before, and why is it
needed now?

And if binman needs it in the source tree, then why do we point the TEE
variable to the original location in BINARIES_DIR, as I highlighted
above?

> > > +endef
> > > +UBOOT_PRE_BUILD_HOOKS += UBOOT_COPY_TEE_FIRMWARE
> > > +else
> > > +UBOOT_MAKE_OPTS += BL32=$(BINARIES_DIR)/tee.bin
> > > +define UBOOT_COPY_TEE_FIRMWARE
> > > +     cp $(BINARIES_DIR)/tee-raw.bin $(@D)/tee.bin
> > Ditto: why do you need to copy it, when TEE, just above, also points to
> > the original in $(BINARIES_DIR) ?
> Same as above

Ditto.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot