binman: Honour the skip-at-start property more faithfully

A discussion on the mailing list about dealing with block offsets and
binman symbols made me think that something is wrong with how Binman
deals with the skip-at-start property.

The feature was originally designed to handle x86 ROMs, which are mapped
at the top of the address space. That seemed too specific, whereas
skipping some space at the start seemed more generally useful.

It has proved useful. For example, rockchip images start at block 64,
so a skip-at-start of 0x8000 deals with this.

But it doesn't actually work correctly, since the image_pos value does
not give the actual position on the media.

Fix this and update the documentation, moving it into the 'section'
section.

Signed-off-by: Simon Glass <sjg@chromium.org>
This commit is contained in:
Simon Glass 2025-02-26 09:26:17 -07:00 committed by Tom Rini
parent 91fa9d9ed2
commit 2c252d36b8
5 changed files with 46 additions and 36 deletions

View file

@ -823,24 +823,6 @@ multiple-images:
};
};
end-at-4gb:
For x86 machines the ROM offsets start just before 4GB and extend
up so that the image finished at the 4GB boundary. This boolean
option can be enabled to support this. The image size must be
provided so that binman knows when the image should start. For an
8MB ROM, the offset of the first entry would be 0xfff80000 with
this option, instead of 0 without this option.
skip-at-start:
This property specifies the entry offset of the first entry.
For PowerPC mpc85xx based CPU, CONFIG_TEXT_BASE is the entry
offset of the first entry. It can be 0xeff40000 or 0xfff40000 for
nor flash boot, 0x201000 for sd boot etc.
'end-at-4gb' property is not applicable where CONFIG_TEXT_BASE +
Image size != 4gb.
align-default:
Specifies the default alignment for entries in this section if they do
not specify an alignment. Note that this only applies to top-level entries
@ -957,6 +939,35 @@ filename:
section in different image, since there is currently no way to share data
between images other than through files.
end-at-4gb:
For x86 machines the ROM offsets start just before 4GB and extend
up so that the image finished at the 4GB boundary. This boolean
option can be enabled to support this. The image size must be
provided so that binman knows when the image should start. For an
8MB ROM, the offset of the first entry would be 0xfff80000 with
this option, instead of 0 without this option.
skip-at-start:
This property specifies the entry offset of the first entry in the section.
It is useful when the Binman image is written to a particular offset in the
media. It allows the offset of the first entry to be the media offset, even
though it is at the start of the image. It effectively creates a hole at the
start of the image, an implied, empty area.
For example, if the image is written to offset 4K on the media, set
skip-at-start to 0x1000. At runtime, the Binman image will assume that it
has be written at offset 4K and all symbols and offsets will take account of
that. The image-pos values will also be adjusted. The effect is similar to
adding an empty 4K region at the start, except that Binman does not actually
output it.
For PowerPC mpc85xx based CPU, CONFIG_TEXT_BASE is the entry
offset of the first entry. It can be 0xeff40000 or 0xfff40000 for
nor flash boot, 0x201000 for sd boot etc.
'end-at-4gb' property is not applicable where CONFIG_TEXT_BASE +
Image size != 4gb.
Image Properties
----------------

View file

@ -392,9 +392,8 @@ class Entry(object):
"""Set the value of device-tree properties calculated by binman"""
state.SetInt(self._node, 'offset', self.offset)
state.SetInt(self._node, 'size', self.size)
base = self.section.GetRootSkipAtStart() if self.section else 0
if self.image_pos is not None:
state.SetInt(self._node, 'image-pos', self.image_pos - base)
state.SetInt(self._node, 'image-pos', self.image_pos)
if self.GetImage().allow_repack:
if self.orig_offset is not None:
state.SetInt(self._node, 'orig-offset', self.orig_offset, True)

View file

@ -65,7 +65,7 @@ class Entry_fmap(Entry):
if entry.image_pos is None:
pos = 0
else:
pos = entry.image_pos - entry.GetRootSkipAtStart()
pos = entry.image_pos
# Drop @ symbols in name
name = entry.name.replace('@', '')
@ -75,8 +75,6 @@ class Entry_fmap(Entry):
_AddEntries(areas, subentry)
else:
pos = entry.image_pos
if pos is not None:
pos -= entry.section.GetRootSkipAtStart()
areas.append(fmap_util.FmapArea(pos or 0, entry.size or 0,
entry.name, flags))

View file

@ -801,7 +801,7 @@ class Entry_section(Entry):
if not entry:
self._Raise("Unable to set offset/size for unknown entry '%s'" %
name)
entry.SetOffsetSize(self._skip_at_start + offset if offset is not None
entry.SetOffsetSize(offset + self._skip_at_start if offset is not None
else None, size)
def GetEntryOffsets(self):

View file

@ -2297,16 +2297,17 @@ class TestFunctional(unittest.TestCase):
fhdr, fentries = fmap_util.DecodeFmap(data[32:])
self.assertEqual(0x100, fhdr.image_size)
base = (1 << 32) - 0x100
self.assertEqual(0, fentries[0].offset)
self.assertEqual(base, fentries[0].offset)
self.assertEqual(4, fentries[0].size)
self.assertEqual(b'U_BOOT', fentries[0].name)
self.assertEqual(4, fentries[1].offset)
self.assertEqual(base + 4, fentries[1].offset)
self.assertEqual(3, fentries[1].size)
self.assertEqual(b'INTEL_MRC', fentries[1].name)
self.assertEqual(32, fentries[2].offset)
self.assertEqual(base + 32, fentries[2].offset)
self.assertEqual(fmap_util.FMAP_HEADER_LEN +
fmap_util.FMAP_AREA_LEN * 3, fentries[2].size)
self.assertEqual(b'FMAP', fentries[2].name)
@ -2319,27 +2320,28 @@ class TestFunctional(unittest.TestCase):
fhdr, fentries = fmap_util.DecodeFmap(data[36:])
self.assertEqual(0x180, fhdr.image_size)
base = (1 << 32) - 0x180
expect_size = fmap_util.FMAP_HEADER_LEN + fmap_util.FMAP_AREA_LEN * 4
fiter = iter(fentries)
fentry = next(fiter)
self.assertEqual(b'U_BOOT', fentry.name)
self.assertEqual(0, fentry.offset)
self.assertEqual(base, fentry.offset)
self.assertEqual(4, fentry.size)
fentry = next(fiter)
self.assertEqual(b'SECTION', fentry.name)
self.assertEqual(4, fentry.offset)
self.assertEqual(base + 4, fentry.offset)
self.assertEqual(0x20 + expect_size, fentry.size)
fentry = next(fiter)
self.assertEqual(b'INTEL_MRC', fentry.name)
self.assertEqual(4, fentry.offset)
self.assertEqual(base + 4, fentry.offset)
self.assertEqual(3, fentry.size)
fentry = next(fiter)
self.assertEqual(b'FMAP', fentry.name)
self.assertEqual(36, fentry.offset)
self.assertEqual(base + 36, fentry.offset)
self.assertEqual(expect_size, fentry.size)
def testElf(self):
@ -3535,8 +3537,8 @@ class TestFunctional(unittest.TestCase):
image = control.images['image']
entries = image.GetEntries()
desc = entries['intel-descriptor']
self.assertEqual(0xff800000, desc.offset);
self.assertEqual(0xff800000, desc.image_pos);
self.assertEqual(0xff800000, desc.offset)
self.assertEqual(0xff800000, desc.image_pos)
def testReplaceCbfs(self):
"""Test replacing a single file in CBFS without changing the size"""
@ -3778,8 +3780,8 @@ class TestFunctional(unittest.TestCase):
image = control.images['image']
entries = image.GetEntries()
expected_ptr = entries['intel-fit'].image_pos - (1 << 32)
self.assertEqual(expected_ptr, ptr)
expected_ptr = entries['intel-fit'].image_pos #- (1 << 32)
self.assertEqual(expected_ptr, ptr + (1 << 32))
def testPackIntelFitMissing(self):
"""Test detection of a FIT pointer with not FIT region"""
@ -4773,7 +4775,7 @@ class TestFunctional(unittest.TestCase):
entry = image.GetEntries()['fdtmap']
self.assertEqual(orig_entry.offset, entry.offset)
self.assertEqual(orig_entry.size, entry.size)
self.assertEqual(16, entry.image_pos)
self.assertEqual((1 << 32) - 0x400 + 16, entry.image_pos)
u_boot = image.GetEntries()['section'].GetEntries()['u-boot']