[PATCH 2/2] binman: Honour the skip-at-start property more faithfully
Simon Glass
sjg at chromium.org
Sun Jan 26 00:38:12 CET 2025
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 at chromium.org>
---
tools/binman/binman.rst | 47 +++++++++++++++++++++--------------
tools/binman/entry.py | 3 +--
tools/binman/etype/fmap.py | 4 +--
tools/binman/etype/section.py | 2 +-
tools/binman/ftest.py | 26 ++++++++++---------
5 files changed, 46 insertions(+), 36 deletions(-)
diff --git a/tools/binman/binman.rst b/tools/binman/binman.rst
index 381e55686f9..e27054a9c67 100644
--- a/tools/binman/binman.rst
+++ b/tools/binman/binman.rst
@@ -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
beteen 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
----------------
diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index e506b552a8d..bdc60e47fca 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -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)
diff --git a/tools/binman/etype/fmap.py b/tools/binman/etype/fmap.py
index 3669d91a0bc..35ca8490f79 100644
--- a/tools/binman/etype/fmap.py
+++ b/tools/binman/etype/fmap.py
@@ -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))
diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py
index 020fd57d704..5e11cf58d28 100644
--- a/tools/binman/etype/section.py
+++ b/tools/binman/etype/section.py
@@ -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):
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 156567ace77..fb99c185c80 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -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"""
@@ -4751,7 +4753,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']
--
2.43.0
More information about the U-Boot
mailing list