[PATCH RESEND] binman: fix mkmbn integration and tests

[Casey Connolly]

On 28/10/2025 09:19, Neil Armstrong via groups.io wrote:
> Hi,
> 
> On 10/23/25 15:38, Casey Connolly wrote:
>> Add the missing cryptography requirement to binman so that mkmbn
>> will work correctly, fix a pylint error, and adjust mkmbn to properly
>> integrate into binman rather than writing the output file directly.
>>
>> For now skip testing u_boot_mbn since it has some weird requirements due
>> to how mkmbn works and can't be easily integrated into binman.
>>
>> Signed-off-by: Casey Connolly <casey.connolly at linaro.org>
>> ---
>> Resent to fix my name
>> ---
>>   tools/binman/btool/mkmbn.py      | 9 ++-------
>>   tools/binman/etype/u_boot_mbn.py | 5 +++--
>>   tools/binman/main.py             | 4 +++-
>>   tools/binman/requirements.txt    | 1 +
>>   tools/qcom/mkmbn/elf.py          | 6 +++---
>>   5 files changed, 12 insertions(+), 13 deletions(-)
>>
>> diff --git a/tools/binman/btool/mkmbn.py b/tools/binman/btool/mkmbn.py
>> index 232e2b442a16..c1439977833e 100644
>> --- a/tools/binman/btool/mkmbn.py
>> +++ b/tools/binman/btool/mkmbn.py
>> @@ -12,18 +12,13 @@ class Bintoolmkmbn(bintool.Bintool):
>>       def __init__(self, name):
>>           super().__init__(name, 'Generate MBN (signed ELF) image for
>> U-Boot')
>>         # pylint: disable=R0913
>> -    def run(self, infile: str) -> CommandResult:
>> +    def run(self, infile: str, outfile:  str) -> CommandResult:
>>           """Run mkmbn.py
>>           """
>> -        args = [infile]
>> +        args = ['-o', outfile, infile]
>>           return self.run_cmd_result(*args, raise_on_error=False)
>>     -    def get_path(self):
>> -        path = super().get_path()
>> -        return path
>> -
>> -
>>       def fetch(self, _method):
>>           return True
>> diff --git a/tools/binman/etype/u_boot_mbn.py b/tools/binman/etype/
>> u_boot_mbn.py
>> index e470b31fa318..6268397fd122 100644
>> --- a/tools/binman/etype/u_boot_mbn.py
>> +++ b/tools/binman/etype/u_boot_mbn.py
>> @@ -31,10 +31,11 @@ class Entry_u_boot_mbn(Entry_blob):
>>       def __init__(self, section, etype, node):
>>           super().__init__(section, etype, node)
>>         def ObtainContents(self):
>> -        self._pathname = tools.get_input_filename('u-boot.bin')
>> -        res = self.mkmbn.run(self._pathname)
>> +        pathname = tools.get_input_filename('u-boot.bin')
>> +        self._pathname = tools.get_output_filename('u-boot.mbn')
>> +        res = self.mkmbn.run(pathname, self._pathname)
>>           if res is None:
>>               self.Raise('mkmbn not found or failed!')
>>           # Exit code 61 (ENODATA) indicates that an MBN file can't be
>> built
>>           # for this board. This is non-fatal since it may not be
>> required.
>> diff --git a/tools/binman/main.py b/tools/binman/main.py
>> index fa5ad79ca0e8..bfd6e990d2e4 100755
>> --- a/tools/binman/main.py
>> +++ b/tools/binman/main.py
>> @@ -102,9 +102,11 @@ def RunTestCoverage(toolpath, build_dir, args):
>>                'tools/u_boot_pylib/*'],
>>               build_dir, all_set, extra_args or None, args=args,
>>               allow_failures=['tools/binman/btool/cst.py',
>>                               'tools/binman/etype/nxp_imx8mcst.py',
>> -                            'tools/binman/etype/nxp_imx8mimage.py'])
>> +                            'tools/binman/etype/nxp_imx8mimage.py',
>> +                            'tools/binman/etype/u_boot_mbn.py',
>> +                            'tools/binman/btool/mkmbn.py'])
>>     def RunBinman(args):
>>       """Main entry point to binman once arguments are parsed
>>   diff --git a/tools/binman/requirements.txt b/tools/binman/
>> requirements.txt
>> index 7db72e888e3e..51b8ecd73478 100644
>> --- a/tools/binman/requirements.txt
>> +++ b/tools/binman/requirements.txt
>> @@ -3,4 +3,5 @@ importlib_resources==6.5.2
>>   jsonschema==4.23.0
>>   pycryptodomex==3.21.0
>>   pyelftools==0.31
>>   yamllint==1.35.1
>> +cryptography==46.0.3 # for mkmbn
>> diff --git a/tools/qcom/mkmbn/elf.py b/tools/qcom/mkmbn/elf.py
>> index 86f12d1f4e5d..7737adc6a4e2 100644
>> --- a/tools/qcom/mkmbn/elf.py
>> +++ b/tools/qcom/mkmbn/elf.py
>> @@ -171,11 +171,11 @@ def _align(i: int, alignment: int) -> int:
>>   class Elf:
>>       ehdr: Ehdr
>>       phdrs: List[Phdr]
>>   -    def __init__(self):
>> -        self.ehdr = Ehdr()
>> -        self.phdrs: List[Phdr] = []
>> +    def __init__(self, ehdr: Ehdr | None = None, phdrs: List[Phdr] |
>> None = None):
>> +        self.ehdr = ehdr or Ehdr()
>> +        self.phdrs: List[Phdr] = phdrs or []
>>         def total_header_size(self):
>>           return self.ehdr.e_phoff + len(self.phdrs) *
>> self.ehdr.e_phentsize
>>   
> 
> I understand all the changes.. except this last one.
> 
> Could you elaborate ?

Ah yeah i missed that in the commit message. In ("tools: qcom: add
mkmbn.py") we add the missing constructor for the Elf class but I forgot
to add the parameters which are passed in when constructed with the
parse() staticmethod, this fixes that. It should probably be a separate
commit, I'll split it out.

Kind regards,
> 
> Thanks,
> Neil
> 
> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#2386): https://groups.io/g/u-boot-qcom/message/2386
> Mute This Topic: https://groups.io/mt/115910537/8413174
> Group Owner: u-boot-qcom+owner at groups.io
> Unsubscribe: https://groups.io/g/u-boot-qcom/
> leave/13197796/8413174/1557304080/xyzzy [casey.connolly at linaro.org]
> -=-=-=-=-=-=-=-=-=-=-=-
> 
> 

-- 
// Casey (she/her)