[PATCH v5 01/13] binman: Ignore noload segments for image numbering

[Simon Glass]

Hi Varadarajan,

On 2026-06-17T10:17:59, Varadarajan Narayanan
<varadarajan.narayanan at oss.qualcomm.com> wrote:
> binman: Ignore noload segments for image numbering
>
> While including the partial images of a split-elf, binman gives the
> segment number as found in the input ELF. However this results in 'gaps'
> if the NOLOAD segments appear in between LOAD segments. Hence track the
> LOAD segments separately and number the partial images accordingly.
>
> Update the read segments test to verify this.
>
> Signed-off-by: Varadarajan Narayanan <varadarajan.narayanan at oss.qualcomm.com>
>
> tools/binman/elf.py      | 4 +++-
>  tools/binman/elf_test.py | 4 ++++
>  2 files changed, 7 insertions(+), 1 deletion(-)

> diff --git a/tools/binman/elf_test.py b/tools/binman/elf_test.py
> @@ -288,6 +288,10 @@ class TestElf(unittest.TestCase):
>              self.skipTest('Python elftools not available')
>          fname = self.ElfTestFile('embed_data')
>          segments, entry = elf.read_loadable_segments(tools.read_file(fname))
> +        n = 0
> +        for (i, start, data) in segments:
> +            self.assertEqual(i, n)
> +            n = n + 1

This doesn't exercise the change. The embed_data ELF has a single
PT_LOAD segment (see test/embed_data.lds), so the old code already
returns 0, 1, 2, ... — the loop index i and the LOAD count coincide.
To regression-test the fix you need an ELF with a non-PT_LOAD program
header between two PT_LOAD ones, so the old code produces a gap and
the new code does not. Please add a fixture (or extend embed_data.lds
with a NOTE/NOLOAD segment) that demonstrates the previous behaviour
was wrong.

> diff --git a/tools/binman/elf.py b/tools/binman/elf.py
> @@ -551,6 +551,7 @@ def read_loadable_segments(data):
>              raise ValueError(err)
>          entry = elf.header['e_entry']
>          segments = []
> +        n = 0
>          for i in range(elf.num_segments()):
>              segment = elf.get_segment(i)
>              if segment['p_type'] != 'PT_LOAD' or not segment['p_memsz']:
> @@ -558,7 +559,8 @@ def read_loadable_segments(data):
>                  continue
>              start = segment['p_offset']
>              rend = start + segment['p_filesz']
> -            segments.append((i, segment['p_paddr'], data[start:rend]))
> +            segments.append((n, segment['p_paddr'], data[start:rend]))
> +            n = n + 1

The docstring (lines 534-540) describes the first tuple element as
'Segment number (0 = first)', which is ambiguous on whether gaps are
allowed. Please update it to make clear this is the index into the
returned LOAD segments, not the program-header index. Also,
enumerate() over the filtered iteration would read more naturally;
drop n and use enumerate at the append point. Minor: n += 1 is the
usual style.

I also suggest a sentence in the commit message noting this is a
behaviour change for split-elf FIT nodes (SEQ substitution now
produces contiguous numbers when NOLOAD segments are present).

Regards,
Simon