[10/12] binman: btool: Add Xilinx Bootgen btool

From: Lukas Funke <lukas.funke@weidmueller.com>

Add the Xilinx Bootgen as bintool. Xilinx Bootgen is used to create
bootable SPL (FSBL in Xilinx terms) images for Zynq/ZynqMP devices. The
btool creates a signed version of the SPL.

Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>
---

 tools/binman/btool/bootgen.py | 82 +++++++++++++++++++++++++++++++++++
 1 file changed, 82 insertions(+)
 create mode 100644 tools/binman/btool/bootgen.py

Hi,

On Thu, 29 Jun 2023 at 15:59, <lukas.funke-oss@weidmueller.com> wrote:
>
> From: Lukas Funke <lukas.funke@weidmueller.com>
>
> Add the Xilinx Bootgen as bintool. Xilinx Bootgen is used to create
> bootable SPL (FSBL in Xilinx terms) images for Zynq/ZynqMP devices. The
> btool creates a signed version of the SPL.
>
> Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>
> ---
>
>  tools/binman/btool/bootgen.py | 82 +++++++++++++++++++++++++++++++++++
>  1 file changed, 82 insertions(+)
>  create mode 100644 tools/binman/btool/bootgen.py
>
> diff --git a/tools/binman/btool/bootgen.py b/tools/binman/btool/bootgen.py
> new file mode 100644
> index 0000000000..8bc727a54f
> --- /dev/null
> +++ b/tools/binman/btool/bootgen.py
> @@ -0,0 +1,82 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +# Copyright (C) 2023 Weidmüller Interface GmbH & Co. KG
> +# Lukas Funke <lukas.funke@weidmueller.com>
> +#
> +"""Bintool implementation for bootgen
> +
> +bootgen allows creating bootable SPL for Zynq(MP)
> +
> +Documentation is available via::
> +https://www.xilinx.com/support/documents/sw_manuals/xilinx2022_1/ug1283-bootgen-user-guide.pdf

But you need to create some info here. It is good to have that link,
but we also need some about of info about it here.

Overall this whole patch needs more docs and detail.

> +
> +Source code is available at:
> +
> +https://github.com/Xilinx/bootgen
> +
> +"""
> +import tempfile
> +
> +from binman import bintool
> +from u_boot_pylib import tools
> +
> +# pylint: disable=C0103
> +class Bintoolbootgen(bintool.Bintool):
> +    """Generate bootable fsbl image for zynq/zynqmp
> +
> +    This bintools supports running Xilinx "bootgen" in order
> +    to generate a bootable, authenticated image form an SPL.
> +
> +    """
> +    def __init__(self, name):
> +        super().__init__(name, 'Xilinx Bootgen',
> +                         version_regex=r'^\*\*\*\*\*\* Xilinx Bootgen',
> +                         version_args='')
> +
> +    # pylint: disable=R0913
> +    def sign(self, arch, spl_elf_fname, pmufw_elf_fname,
> +             psk_fname, ssk_fname, fsbl_config, auth_params, output_fname):
> +        """ Sign FSBL(SPL) elf file and bundle it with pmu firmware
> +            to a bootable image
> +
> +        Args:
> +            arch (str): Xilinx SoC architecture

what options are valid?

> +            spl_elf_fname (str): Filename of FSBL ELF file

what is FSBL?

> +            pmufw_elf_fname (str): Filename pmu firmware

what is pmu?

> +            psk_fname (str): Filename of the primary secret key
> +            ssk_fname (str): Filename of the secondary secret key

why are there two keys? What format is used for these keys?

> +            fsbl_config (str): FSBL config options

what options are available? What are valid vaulues for this arg?

> +            auth_params (str): Authentication parameter

what are valid values?

> +            output_fname (str): Filename where bootgen should write the result

This should really be handled automatically, I think. See how the
mkimage etype creates an output filename.

> +        """
> +
> +        _fsbl_config = f"[fsbl_config] {fsbl_config}" if fsbl_config else ""
> +        _auth_params = f"[auth_params] {auth_params}" if auth_params else ""
> +
> +        bif_template = f"""u_boot_spl_aes_rsa: {{
> +            [pskfile] {psk_fname}
> +            [sskfile] {ssk_fname}
> +            {_fsbl_config}
> +            {_auth_params}
> +            [ bootloader,
> +              authentication = rsa,
> +              destination_cpu=a53-0] {spl_elf_fname}
> +            [pmufw_image] {pmufw_elf_fname}
> +        }}"""
> +        args = ["-arch", arch]
> +
> +        with tempfile.NamedTemporaryFile(suffix=".bif",
> +                                         dir=tools.get_output_dir()) as bif:

Please use a deterministic name - see mkimage etype for an example.

> +            tools.write_file(bif.name, bif_template, binary=False)
> +            args += ["-image", bif.name, '-w', '-o', output_fname]
> +            self.run_cmd(*args)
> +
> +    def fetch(self, method):
> +        """Fetch bootgen from git"""
> +        if method != bintool.FETCH_BUILD:
> +            return None
> +
> +        result = self.build_from_git(
> +            'https://github.com/Xilinx/bootgen',
> +            'all',
> +            'build/bootgen/bootgen')
> +        return result
> --
> 2.30.2
>

Regards,
Simon

On 30.06.2023 06:18, Simon Glass wrote:
> Hi,
> 
> On Thu, 29 Jun 2023 at 15:59, <lukas.funke-oss@weidmueller.com> wrote:
>>
>> From: Lukas Funke <lukas.funke@weidmueller.com>
>>
>> Add the Xilinx Bootgen as bintool. Xilinx Bootgen is used to create
>> bootable SPL (FSBL in Xilinx terms) images for Zynq/ZynqMP devices. The
>> btool creates a signed version of the SPL.
>>
>> Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>
>> ---
>>
>>   tools/binman/btool/bootgen.py | 82 +++++++++++++++++++++++++++++++++++
>>   1 file changed, 82 insertions(+)
>>   create mode 100644 tools/binman/btool/bootgen.py
>>
>> diff --git a/tools/binman/btool/bootgen.py b/tools/binman/btool/bootgen.py
>> new file mode 100644
>> index 0000000000..8bc727a54f
>> --- /dev/null
>> +++ b/tools/binman/btool/bootgen.py
>> @@ -0,0 +1,82 @@
>> +# SPDX-License-Identifier: GPL-2.0+
>> +# Copyright (C) 2023 Weidmüller Interface GmbH & Co. KG
>> +# Lukas Funke <lukas.funke@weidmueller.com>
>> +#
>> +"""Bintool implementation for bootgen
>> +
>> +bootgen allows creating bootable SPL for Zynq(MP)
>> +
>> +Documentation is available via::
>> +https://www.xilinx.com/support/documents/sw_manuals/xilinx2022_1/ug1283-bootgen-user-guide.pdf
> 
> But you need to create some info here. It is good to have that link,
> but we also need some about of info about it here.
> 
> Overall this whole patch needs more docs and detail.
> 
>> +
>> +Source code is available at:
>> +
>> +https://github.com/Xilinx/bootgen
>> +
>> +"""
>> +import tempfile
>> +
>> +from binman import bintool
>> +from u_boot_pylib import tools
>> +
>> +# pylint: disable=C0103
>> +class Bintoolbootgen(bintool.Bintool):
>> +    """Generate bootable fsbl image for zynq/zynqmp
>> +
>> +    This bintools supports running Xilinx "bootgen" in order
>> +    to generate a bootable, authenticated image form an SPL.
>> +
>> +    """
>> +    def __init__(self, name):
>> +        super().__init__(name, 'Xilinx Bootgen',
>> +                         version_regex=r'^\*\*\*\*\*\* Xilinx Bootgen',
>> +                         version_args='')
>> +
>> +    # pylint: disable=R0913
>> +    def sign(self, arch, spl_elf_fname, pmufw_elf_fname,
>> +             psk_fname, ssk_fname, fsbl_config, auth_params, output_fname):
>> +        """ Sign FSBL(SPL) elf file and bundle it with pmu firmware
>> +            to a bootable image
>> +
>> +        Args:
>> +            arch (str): Xilinx SoC architecture
> 
> what options are valid?
> 
>> +            spl_elf_fname (str): Filename of FSBL ELF file
> 
> what is FSBL?
> 
>> +            pmufw_elf_fname (str): Filename pmu firmware
> 
> what is pmu?
> 
>> +            psk_fname (str): Filename of the primary secret key
>> +            ssk_fname (str): Filename of the secondary secret key
> 
> why are there two keys? What format is used for these keys?
> 
>> +            fsbl_config (str): FSBL config options
> 
> what options are available? What are valid vaulues for this arg?
> 
>> +            auth_params (str): Authentication parameter
> 
> what are valid values?
> 
>> +            output_fname (str): Filename where bootgen should write the result
> 
> This should really be handled automatically, I think. See how the
> mkimage etype creates an output filename.

Simon, thanks for the review!

What to you mean by automatically? The mkimage btool also has an 
'output_fname' argument as well. And the output filename is passed down 
from the mkimage etype to the mkimage btool. Should the bootgen btool 
generate the output_fname by itself and pass it back to the caller?

> 
>> +        """
>> +
>> +        _fsbl_config = f"[fsbl_config] {fsbl_config}" if fsbl_config else ""
>> +        _auth_params = f"[auth_params] {auth_params}" if auth_params else ""
>> +
>> +        bif_template = f"""u_boot_spl_aes_rsa: {{
>> +            [pskfile] {psk_fname}
>> +            [sskfile] {ssk_fname}
>> +            {_fsbl_config}
>> +            {_auth_params}
>> +            [ bootloader,
>> +              authentication = rsa,
>> +              destination_cpu=a53-0] {spl_elf_fname}
>> +            [pmufw_image] {pmufw_elf_fname}
>> +        }}"""
>> +        args = ["-arch", arch]
>> +
>> +        with tempfile.NamedTemporaryFile(suffix=".bif",
>> +                                         dir=tools.get_output_dir()) as bif:
> 
> Please use a deterministic name - see mkimage etype for an example.

The .bif file is a temporary input file passed to 'bootgen'.

My intention was to make sure that the file is deleted afterwards. Would 
you prefer that the intermediate files are kept and the output file is 
deterministically created with "uniq = self.GetUniqueName()" and 
"output_fname = tools.get_output_filename('foo.%s' % uniq)"?

If so, I would also change the way the ELF file is created in order to 
keep the intermediate results.

> 
>> +            tools.write_file(bif.name, bif_template, binary=False)
>> +            args += ["-image", bif.name, '-w', '-o', output_fname]
>> +            self.run_cmd(*args)
>> +
>> +    def fetch(self, method):
>> +        """Fetch bootgen from git"""
>> +        if method != bintool.FETCH_BUILD:
>> +            return None
>> +
>> +        result = self.build_from_git(
>> +            'https://github.com/Xilinx/bootgen',
>> +            'all',
>> +            'build/bootgen/bootgen')
>> +        return result
>> --
>> 2.30.2
>>
> 
> Regards,
> Simon

Regards,
Lukas

Hi Lukas,

On Fri, 30 Jun 2023 at 13:28, Lukas Funke
<lukas.funke-oss@weidmueller.com> wrote:
>
> On 30.06.2023 06:18, Simon Glass wrote:
> > Hi,
> >
> > On Thu, 29 Jun 2023 at 15:59, <lukas.funke-oss@weidmueller.com> wrote:
> >>
> >> From: Lukas Funke <lukas.funke@weidmueller.com>
> >>
> >> Add the Xilinx Bootgen as bintool. Xilinx Bootgen is used to create
> >> bootable SPL (FSBL in Xilinx terms) images for Zynq/ZynqMP devices. The
> >> btool creates a signed version of the SPL.
> >>
> >> Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>
> >> ---
> >>
> >>   tools/binman/btool/bootgen.py | 82 +++++++++++++++++++++++++++++++++++
> >>   1 file changed, 82 insertions(+)
> >>   create mode 100644 tools/binman/btool/bootgen.py
> >>
> >> diff --git a/tools/binman/btool/bootgen.py b/tools/binman/btool/bootgen.py
> >> new file mode 100644
> >> index 0000000000..8bc727a54f
> >> --- /dev/null
> >> +++ b/tools/binman/btool/bootgen.py
> >> @@ -0,0 +1,82 @@
> >> +# SPDX-License-Identifier: GPL-2.0+
> >> +# Copyright (C) 2023 Weidmüller Interface GmbH & Co. KG
> >> +# Lukas Funke <lukas.funke@weidmueller.com>
> >> +#
> >> +"""Bintool implementation for bootgen
> >> +
> >> +bootgen allows creating bootable SPL for Zynq(MP)
> >> +
> >> +Documentation is available via::
> >> +https://www.xilinx.com/support/documents/sw_manuals/xilinx2022_1/ug1283-bootgen-user-guide.pdf
> >
> > But you need to create some info here. It is good to have that link,
> > but we also need some about of info about it here.
> >
> > Overall this whole patch needs more docs and detail.
> >
> >> +
> >> +Source code is available at:
> >> +
> >> +https://github.com/Xilinx/bootgen
> >> +
> >> +"""
> >> +import tempfile
> >> +
> >> +from binman import bintool
> >> +from u_boot_pylib import tools
> >> +
> >> +# pylint: disable=C0103
> >> +class Bintoolbootgen(bintool.Bintool):
> >> +    """Generate bootable fsbl image for zynq/zynqmp
> >> +
> >> +    This bintools supports running Xilinx "bootgen" in order
> >> +    to generate a bootable, authenticated image form an SPL.
> >> +
> >> +    """
> >> +    def __init__(self, name):
> >> +        super().__init__(name, 'Xilinx Bootgen',
> >> +                         version_regex=r'^\*\*\*\*\*\* Xilinx Bootgen',
> >> +                         version_args='')
> >> +
> >> +    # pylint: disable=R0913
> >> +    def sign(self, arch, spl_elf_fname, pmufw_elf_fname,
> >> +             psk_fname, ssk_fname, fsbl_config, auth_params, output_fname):
> >> +        """ Sign FSBL(SPL) elf file and bundle it with pmu firmware
> >> +            to a bootable image
> >> +
> >> +        Args:
> >> +            arch (str): Xilinx SoC architecture
> >
> > what options are valid?
> >
> >> +            spl_elf_fname (str): Filename of FSBL ELF file
> >
> > what is FSBL?
> >
> >> +            pmufw_elf_fname (str): Filename pmu firmware
> >
> > what is pmu?
> >
> >> +            psk_fname (str): Filename of the primary secret key
> >> +            ssk_fname (str): Filename of the secondary secret key
> >
> > why are there two keys? What format is used for these keys?
> >
> >> +            fsbl_config (str): FSBL config options
> >
> > what options are available? What are valid vaulues for this arg?
> >
> >> +            auth_params (str): Authentication parameter
> >
> > what are valid values?
> >
> >> +            output_fname (str): Filename where bootgen should write the result
> >
> > This should really be handled automatically, I think. See how the
> > mkimage etype creates an output filename.
>
> Simon, thanks for the review!
>
> What to you mean by automatically? The mkimage btool also has an
> 'output_fname' argument as well. And the output filename is passed down
> from the mkimage etype to the mkimage btool. Should the bootgen btool
> generate the output_fname by itself and pass it back to the caller?

Oh I think I was getting confused with the 'filename' property used by
the mkimage entry type. What you are doing here looks fine to me.

>
> >
> >> +        """
> >> +
> >> +        _fsbl_config = f"[fsbl_config] {fsbl_config}" if fsbl_config else ""
> >> +        _auth_params = f"[auth_params] {auth_params}" if auth_params else ""
> >> +
> >> +        bif_template = f"""u_boot_spl_aes_rsa: {{
> >> +            [pskfile] {psk_fname}
> >> +            [sskfile] {ssk_fname}
> >> +            {_fsbl_config}
> >> +            {_auth_params}
> >> +            [ bootloader,
> >> +              authentication = rsa,
> >> +              destination_cpu=a53-0] {spl_elf_fname}
> >> +            [pmufw_image] {pmufw_elf_fname}
> >> +        }}"""
> >> +        args = ["-arch", arch]
> >> +
> >> +        with tempfile.NamedTemporaryFile(suffix=".bif",
> >> +                                         dir=tools.get_output_dir()) as bif:
> >
> > Please use a deterministic name - see mkimage etype for an example.
>
> The .bif file is a temporary input file passed to 'bootgen'.
>
> My intention was to make sure that the file is deleted afterwards. Would
> you prefer that the intermediate files are kept and the output file is
> deterministically created with "uniq = self.GetUniqueName()" and
> "output_fname = tools.get_output_filename('foo.%s' % uniq)"?
>
> If so, I would also change the way the ELF file is created in order to
> keep the intermediate results.
>
> >
> >> +            tools.write_file(bif.name, bif_template, binary=False)
> >> +            args += ["-image", bif.name, '-w', '-o', output_fname]
> >> +            self.run_cmd(*args)
> >> +
> >> +    def fetch(self, method):
> >> +        """Fetch bootgen from git"""
> >> +        if method != bintool.FETCH_BUILD:
> >> +            return None
> >> +
> >> +        result = self.build_from_git(
> >> +            'https://github.com/Xilinx/bootgen',
> >> +            'all',
> >> +            'build/bootgen/bootgen')
> >> +        return result
> >> --
> >> 2.30.2
> >>
Regards,
Simon