Re: [PATCH v4 04/11] tools: add fdtsig.sh

[AKASHI Takahiro <takahiro.akashi@linaro.org>]

Simon,

On Thu, Oct 14, 2021 at 06:40:24PM -0600, Simon Glass wrote:
> Hi Takahiro,
> 
> On Mon, 11 Oct 2021 at 19:42, AKASHI Takahiro
> <takahiro.akashi@linaro.org> wrote:
> >
> > Simon,
> >
> > On Mon, Oct 11, 2021 at 08:54:09AM -0600, Simon Glass wrote:
> > > Hi Takahiro,
> > >
> > > On Thu, 7 Oct 2021 at 00:25, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
> > > >
> > > > With this script, a public key is added to a device tree blob
> > > > as the default efi_get_public_key_data() expects.
> > > >
> > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > > > ---
> > > >  MAINTAINERS     |  1 +
> > > >  tools/fdtsig.sh | 40 ++++++++++++++++++++++++++++++++++++++++
> > > >  2 files changed, 41 insertions(+)
> > > >  create mode 100755 tools/fdtsig.sh
> > >
> > > Instead of an ad-hoc script with no tests,
> >
> > Basically I intended to provide fdtsig.sh as a *sample* script so that
> > people may want to integrate the logic into their own build rule/systems.
> > But I could use this script in my 'capsule authentication' test
> > that is also added in patch#22.
> >
> > > could we use binman for
> > > putting the image together and inserting it?
> >
> > First, as you can see, the script is quite simple and secondly,
> > the purpose of binman, IIUC, is to help handle/manipulate U-Boot
> > image binaries.
> > So I'm not sure whether it is really useful to add such a feature to binman.
> 
> I'm not sure. The script seems very ad-hoc to me, for a feature that
> Linaro is pushing so hard.

To be honest, I've never used binman :) So I'm not sure whether binman
is the best place to add this feature. For example, README under tools/binman
says, "It seems better to use the mkimage tool to generate binaries and avoid
blurring the boundaries between building input files (mkimage) and packaging
then into a final image (binman)."
Obviously, dtb is not the final image.

> I don't see where the script is used in the tests or even mentioned in
> the documentation. Am I missing something?

Due to the history of submissions of this series, the current pytest
scenario doesn't use the script, but you can see the exact same
sequence of commands at test/py/tests/test_efi_capsule/conftest.py:
---8<---
        # Update dtb adding capsule certificate
        check_call('cd %s; cp %s/test/py/tests/test_efi_capsule/signature.dts .'
                   % (data_dir, u_boot_config.source_dir), shell=True)
        check_call('cd %s; dtc -@ -I dts -O dtb -o signature.dtbo signature.dts; fdtoverlay -i %s/arch/sandbox/dts/test.dtb -o test_sig.dtb signature.dtbo'
                   % (data_dir, u_boot_config.build_dir), shell=True)
--->8---
(Please see my patch#11.)

What I meant is that we can directly use fdtsig.sh here if your concern
is that the script is *not exercised* anywhere.

-Takahiro Akashi

> Regards,
> Simon