From: Simon Glass <sjg@chromium.org>
Date: Mon, 8 Jul 2019 20:25:28 +0000 (-0600)
Subject: binman: Add an image header
X-Git-Url: http://git.dujemihanovic.xyz/login.html?a=commitdiff_plain;h=cf2289435c7e1134c75d9217ef32238aec0ecdbf;p=u-boot.git

binman: Add an image header

It is useful to be able to quickly locate the FDT map in the image. An
easy way to do this is with a pointer at the start or end of the image.

Add an 'image header' entry, which places a magic number followed by a
pointer to the FDT map. This can be located at the start or end of the
image, or at a chosen location.

As part of this, update GetSiblingImagePos() to detect missing siblings.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

diff --git a/tools/binman/README b/tools/binman/README
index 8a5f3320dc..ef62d1f1ec 100644
--- a/tools/binman/README
+++ b/tools/binman/README
@@ -640,7 +640,9 @@ of each entry.
 
 Alternatively, an FDT map entry can be used to add a special FDT containing
 just the information about the image. This is preceded by a magic string so can
-be located anywhere in the image.
+be located anywhere in the image. An image header (typically at the start or end
+of the image) can be used to point to the FDT map. See fdtmap and image-header
+entries for more information.
 
 
 Compression
@@ -817,7 +819,6 @@ Some ideas:
 - Add an option to decode an image into the constituent binaries
 - Support building an image for a board (-b) more completely, with a
   configurable build directory
-- Support putting the FDT in an image with a suitable magic number
 - Support listing files in images
 - Support logging of binman's operations, with different levels of verbosity
 - Support updating binaries in an image (with no size change / repacking)
diff --git a/tools/binman/README.entries b/tools/binman/README.entries
index 7014d36f5f..598d8278a7 100644
--- a/tools/binman/README.entries
+++ b/tools/binman/README.entries
@@ -331,6 +331,25 @@ README.chromium for how to obtain the required keys and tools.
 
 
 
+Entry: image-header: An entry which contains a pointer to the FDT map
+---------------------------------------------------------------------
+
+Properties / Entry arguments:
+    location: Location of header ("start" or "end" of image). This is
+        optional. If omitted then the entry must have an offset property.
+
+This adds an 8-byte entry to the start or end of the image, pointing to the
+location of the FDT map. The format is a magic number followed by an offset
+from the start or end of the image, in twos-compliment format.
+
+This entry must be in the top-level part of the image.
+
+NOTE: If the location is at the start/end, you will probably need to specify
+sort-by-offset for the image, unless you actually put the image header
+first/last in the entry list.
+
+
+
 Entry: intel-cmc: Entry containing an Intel Chipset Micro Code (CMC) file
 -------------------------------------------------------------------------
 
diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index 7356c49c62..e1cd0d3a88 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -561,3 +561,14 @@ features to produce new behaviours.
                 else False
         """
         return name in self.section.GetEntries()
+
+    def GetSiblingImagePos(self, name):
+        """Return the image position of the given sibling
+
+        Returns:
+            Image position of sibling, or None if the sibling has no position,
+                or False if there is no such sibling
+        """
+        if not self.HasSibling(name):
+            return False
+        return self.section.GetEntries()[name].image_pos
diff --git a/tools/binman/etype/image_header.py b/tools/binman/etype/image_header.py
new file mode 100644
index 0000000000..9bc84ec01d
--- /dev/null
+++ b/tools/binman/etype/image_header.py
@@ -0,0 +1,76 @@
+# SPDX-License-Identifier: GPL-2.0+
+# Copyright (c) 2018 Google, Inc
+# Written by Simon Glass <sjg@chromium.org>
+
+"""Entry-type module for an image header which points to the FDT map
+
+This creates an 8-byte entry with a magic number and the offset of the FDT map
+(which is another entry in the image), relative to the start or end of the
+image.
+"""
+
+import struct
+
+from entry import Entry
+import fdt_util
+
+IMAGE_HEADER_MAGIC = b'BinM'
+
+class Entry_image_header(Entry):
+    """An entry which contains a pointer to the FDT map
+
+    Properties / Entry arguments:
+        location: Location of header ("start" or "end" of image). This is
+            optional. If omitted then the entry must have an offset property.
+
+    This adds an 8-byte entry to the start or end of the image, pointing to the
+    location of the FDT map. The format is a magic number followed by an offset
+    from the start or end of the image, in twos-compliment format.
+
+    This entry must be in the top-level part of the image.
+
+    NOTE: If the location is at the start/end, you will probably need to specify
+    sort-by-offset for the image, unless you actually put the image header
+    first/last in the entry list.
+    """
+    def __init__(self, section, etype, node):
+        Entry.__init__(self, section, etype, node)
+        self.location = fdt_util.GetString(self._node, 'location')
+
+    def _GetHeader(self):
+        image_pos = self.GetSiblingImagePos('fdtmap')
+        if image_pos == False:
+            self.Raise("'image_header' section must have an 'fdtmap' sibling")
+        elif image_pos is None:
+            # This will be available when called from ProcessContents(), but not
+            # when called from ObtainContents()
+            offset = 0xffffffff
+        else:
+            image_size = self.section.GetImageSize() or 0
+            base = (0 if self.location != 'end' else image_size)
+            offset = (image_pos - base) & 0xffffffff
+        data = IMAGE_HEADER_MAGIC + struct.pack('<I', offset)
+        return data
+
+    def ObtainContents(self):
+        """Obtain a placeholder for the header contents"""
+        self.SetContents(self._GetHeader())
+        return True
+
+    def Pack(self, offset):
+        """Special pack method to set the offset to start/end of image"""
+        if not self.offset:
+            if self.location not in ['start', 'end']:
+                self.Raise("Invalid location '%s', expected 'start' or 'end'" %
+                           self.location)
+            image_size = self.section.GetImageSize() or 0
+            self.offset = (0 if self.location != 'end' else image_size - 8)
+        return Entry.Pack(self, offset)
+
+    def ProcessContents(self):
+        """Write an updated version of the FDT map to this entry
+
+        This is necessary since image_pos is not available when ObtainContents()
+        is called, since by then the entries have not been packed in the image.
+        """
+        self.SetContents(self._GetHeader())
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 934145ca3c..6fdecb2f3b 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -2076,7 +2076,7 @@ class TestFunctional(unittest.TestCase):
         # Mangle the section name, which should cause a mismatch between the
         # correct FDT path and the one expected by the section
         image = control.images['image']
-        image._section._node.path += '-suffix'
+        image._node.path += '-suffix'
         entries = image.GetEntries()
         fdtmap = entries['fdtmap']
         with self.assertRaises(ValueError) as e:
@@ -2084,6 +2084,51 @@ class TestFunctional(unittest.TestCase):
         self.assertIn("Cannot locate node for path '/binman-suffix'",
                       str(e.exception))
 
+    def testFdtmapHeader(self):
+        """Test an FDT map and image header can be inserted in the image"""
+        data = self.data = self._DoReadFileRealDtb('116_fdtmap_hdr.dts')
+        fdtmap_pos = len(U_BOOT_DATA)
+        fdtmap_data = data[fdtmap_pos:]
+        fdt_data = fdtmap_data[16:]
+        dtb = fdt.Fdt.FromData(fdt_data)
+        fdt_size = dtb.GetFdtObj().totalsize()
+        hdr_data = data[-8:]
+        self.assertEqual('BinM', hdr_data[:4])
+        offset = struct.unpack('<I', hdr_data[4:])[0] & 0xffffffff
+        self.assertEqual(fdtmap_pos - 0x400, offset - (1 << 32))
+
+    def testFdtmapHeaderStart(self):
+        """Test an image header can be inserted at the image start"""
+        data = self.data = self._DoReadFileRealDtb('117_fdtmap_hdr_start.dts')
+        fdtmap_pos = 0x100 + len(U_BOOT_DATA)
+        hdr_data = data[:8]
+        self.assertEqual('BinM', hdr_data[:4])
+        offset = struct.unpack('<I', hdr_data[4:])[0]
+        self.assertEqual(fdtmap_pos, offset)
+
+    def testFdtmapHeaderPos(self):
+        """Test an image header can be inserted at a chosen position"""
+        data = self.data = self._DoReadFileRealDtb('118_fdtmap_hdr_pos.dts')
+        fdtmap_pos = 0x100 + len(U_BOOT_DATA)
+        hdr_data = data[0x80:0x88]
+        self.assertEqual('BinM', hdr_data[:4])
+        offset = struct.unpack('<I', hdr_data[4:])[0]
+        self.assertEqual(fdtmap_pos, offset)
+
+    def testHeaderMissingFdtmap(self):
+        """Test an image header requires an fdtmap"""
+        with self.assertRaises(ValueError) as e:
+            self.data = self._DoReadFileRealDtb('119_fdtmap_hdr_missing.dts')
+        self.assertIn("'image_header' section must have an 'fdtmap' sibling",
+                      str(e.exception))
+
+    def testHeaderNoLocation(self):
+        """Test an image header with a no specified location is detected"""
+        with self.assertRaises(ValueError) as e:
+            self.data = self._DoReadFileRealDtb('120_hdr_no_location.dts')
+        self.assertIn("Invalid location 'None', expected 'start' or 'end'",
+                      str(e.exception))
+
 
 if __name__ == "__main__":
     unittest.main()
diff --git a/tools/binman/test/116_fdtmap_hdr.dts b/tools/binman/test/116_fdtmap_hdr.dts
new file mode 100644
index 0000000000..77a2194b39
--- /dev/null
+++ b/tools/binman/test/116_fdtmap_hdr.dts
@@ -0,0 +1,17 @@
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		size = <0x400>;
+		u-boot {
+		};
+		fdtmap {
+		};
+		image-header {
+			location = "end";
+		};
+	};
+};
diff --git a/tools/binman/test/117_fdtmap_hdr_start.dts b/tools/binman/test/117_fdtmap_hdr_start.dts
new file mode 100644
index 0000000000..17b6be0047
--- /dev/null
+++ b/tools/binman/test/117_fdtmap_hdr_start.dts
@@ -0,0 +1,19 @@
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		size = <0x400>;
+		sort-by-offset;
+		u-boot {
+			offset = <0x100>;
+		};
+		fdtmap {
+		};
+		image-header {
+			location = "start";
+		};
+	};
+};
diff --git a/tools/binman/test/118_fdtmap_hdr_pos.dts b/tools/binman/test/118_fdtmap_hdr_pos.dts
new file mode 100644
index 0000000000..fd803f57fb
--- /dev/null
+++ b/tools/binman/test/118_fdtmap_hdr_pos.dts
@@ -0,0 +1,19 @@
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		size = <0x400>;
+		sort-by-offset;
+		u-boot {
+			offset = <0x100>;
+		};
+		fdtmap {
+		};
+		image-header {
+			offset = <0x80>;
+		};
+	};
+};
diff --git a/tools/binman/test/119_fdtmap_hdr_missing.dts b/tools/binman/test/119_fdtmap_hdr_missing.dts
new file mode 100644
index 0000000000..41bb680f08
--- /dev/null
+++ b/tools/binman/test/119_fdtmap_hdr_missing.dts
@@ -0,0 +1,16 @@
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		sort-by-offset;
+		u-boot {
+		};
+		image-header {
+			offset = <0x80>;
+			location = "start";
+		};
+	};
+};
diff --git a/tools/binman/test/120_hdr_no_location.dts b/tools/binman/test/120_hdr_no_location.dts
new file mode 100644
index 0000000000..585e21f456
--- /dev/null
+++ b/tools/binman/test/120_hdr_no_location.dts
@@ -0,0 +1,16 @@
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		sort-by-offset;
+		u-boot {
+		};
+		fdtmap {
+		};
+		image-header {
+		};
+	};
+};