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>

diff --git a/tools/binman/binman.rst b/tools/binman/binman.rst
index 990fc29577092dc9517c92dfeea201ccbd943aa2..84b1331df5c006f72167016b7f33de73f4a269fa 100644 (file)
--- 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
     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
 ----------------
 

diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index e506b552a8d7f6a7a08f76b3273830cb238dfbde..bdc60e47fcab7bc69907eb08de9b50e977934b74 100644 (file)
--- 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 3669d91a0bc7c7bb5045715ae7d5aba77274260b..35ca8490f79fab91c26d1bcb84bf729c86479903 100644 (file)
--- 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 020fd57d704e6a671a313e8a738742c2e569caf5..5e11cf58d28139e52cc055091c5ba5a0050cd40e 100644 (file)
--- 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 a553ca9e5642b685b908e817972b42bfba1d8a0a..733169b99f6b1755142ce9e0d8474309da18826f 100644 (file)
--- 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"""
@@ -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']