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

30 Jun 2023

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

    