binman: Correct fmap output on x86

Normally x86 platforms use the end-at-4gb option. This currently produces
an FMAP with positions which have a large offset. The use of end-at-4gb is
a useful convenience within binman, but we don't really want to export
a map with these offsets.

Fix this by subtracting the 'skip at start' parameter.

Also put the code which convers names to fmap format, for clarity.

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

8 files changed, 121 insertions, 16 deletions

diff --git a/tools/binman/bsection.py b/tools/binman/bsection.py
index e4c1900b17..c208029c25 100644
--- a/tools/binman/bsection.py
+++ b/tools/binman/bsection.py
@@ -68,6 +68,7 @@ class Section(object):
         self._end_4gb = False
         self._name_prefix = ''
         self._entries = OrderedDict()
+        self._image_pos = None
         if not test:
             self._ReadNode()
             self._ReadEntries()
@@ -124,7 +125,10 @@ class Section(object):
     def SetCalculatedProperties(self):
         state.SetInt(self._node, 'offset', self._offset)
         state.SetInt(self._node, 'size', self._size)
-        state.SetInt(self._node, 'image-pos', self._image_pos)
+        image_pos = self._image_pos
+        if self._parent_section:
+            image_pos -= self._parent_section.GetRootSkipAtStart()
+        state.SetInt(self._node, 'image-pos', image_pos)
         for entry in self._entries.values():
             entry.SetCalculatedProperties()
 
@@ -437,11 +441,17 @@ class Section(object):
             source_entry.Raise("Cannot find node for phandle %d" % phandle)
         for entry in self._entries.values():
             if entry._node == node:
-                if entry.data is None:
-                    return None
-                return entry.data
+                return entry.GetData()
         source_entry.Raise("Cannot find entry for node '%s'" % node.name)
 
     def ExpandSize(self, size):
         if size != self._size:
             self._size = size
+
+    def GetRootSkipAtStart(self):
+        if self._parent_section:
+            return self._parent_section.GetRootSkipAtStart()
+        return self._skip_at_start
+
+    def GetImageSize(self):
+        return self._image._size
diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index fd7223477e..01be291b70 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -197,7 +197,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)
-        state.SetInt(self._node, 'image-pos', self.image_pos)
+        state.SetInt(self._node, 'image-pos',
+                       self.image_pos - self.section.GetRootSkipAtStart())
         state.CheckSetHashValue(self._node, self.GetData)
 
     def ProcessFdt(self, fdt):
diff --git a/tools/binman/etype/fmap.py b/tools/binman/etype/fmap.py
index f1dd81ec49..bf35a5bbf4 100644
--- a/tools/binman/etype/fmap.py
+++ b/tools/binman/etype/fmap.py
@@ -42,14 +42,17 @@ class Entry_fmap(Entry):
                 for subentry in entries.values():
                     _AddEntries(areas, subentry)
             else:
-                areas.append(fmap_util.FmapArea(entry.image_pos or 0,
-                                                entry.size or 0, entry.name, 0))
+                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, 0))
 
-        entries = self.section.GetEntries()
+        entries = self.section._image.GetEntries()
         areas = []
         for entry in entries.values():
             _AddEntries(areas, entry)
-        return fmap_util.EncodeFmap(self.section.GetSize() or 0, self.name,
+        return fmap_util.EncodeFmap(self.section.GetImageSize() or 0, self.name,
                                     areas)
 
     def ObtainContents(self):
diff --git a/tools/binman/etype/u_boot_with_ucode_ptr.py b/tools/binman/etype/u_boot_with_ucode_ptr.py
index 01f3513e7d..da0e12417b 100644
--- a/tools/binman/etype/u_boot_with_ucode_ptr.py
+++ b/tools/binman/etype/u_boot_with_ucode_ptr.py
@@ -66,11 +66,11 @@ class Entry_u_boot_with_ucode_ptr(Entry_blob):
         # the U-Boot region must start at offset 7MB in the section. In this
         # case the ROM starts at 0xff800000, so the offset of the first
         # entry in the section corresponds to that.
-        if (self.target_offset < self.offset or
-                self.target_offset >= self.offset + self.size):
-            self.Raise('Microcode pointer _dt_ucode_base_size at %08x is '
-                'outside the section ranging from %08x to %08x' %
-                (self.target_offset, self.offset, self.offset + self.size))
+        if (self.target_offset < self.image_pos or
+                self.target_offset >= self.image_pos + self.size):
+            self.Raise('Microcode pointer _dt_ucode_base_size at %08x is outside the section ranging from %08x to %08x' %
+                (self.target_offset, self.image_pos,
+                 self.image_pos + self.size))
 
         # Get the microcode, either from u-boot-ucode or u-boot-dtb-with-ucode.
         # If we have left the microcode in the device tree, then it will be
@@ -90,7 +90,7 @@ class Entry_u_boot_with_ucode_ptr(Entry_blob):
 
         # Write the microcode offset and size into the entry
         offset_and_size = struct.pack('<2L', offset, size)
-        self.target_offset -= self.offset
+        self.target_offset -= self.image_pos
         self.ProcessContentsUpdate(self.data[:self.target_offset] +
                                    offset_and_size +
                                    self.data[self.target_offset + 8:])
diff --git a/tools/binman/fmap_util.py b/tools/binman/fmap_util.py
index 7d520e3391..be3cbee87b 100644
--- a/tools/binman/fmap_util.py
+++ b/tools/binman/fmap_util.py
@@ -49,6 +49,9 @@ FmapHeader = collections.namedtuple('FmapHeader', FMAP_HEADER_NAMES)
 FmapArea = collections.namedtuple('FmapArea', FMAP_AREA_NAMES)
 
 
+def NameToFmap(name):
+    return name.replace('\0', '').replace('-', '_').upper()
+
 def ConvertName(field_names, fields):
     """Convert a name to something flashrom likes
 
@@ -62,7 +65,7 @@ def ConvertName(field_names, fields):
             value: value of that field (string for the ones we support)
     """
     name_index = field_names.index('name')
-    fields[name_index] = fields[name_index].replace('\0', '').replace('-', '_').upper()
+    fields[name_index] = NameToFmap(fields[name_index])
 
 def DecodeFmap(data):
     """Decode a flashmap into a header and list of areas
@@ -100,6 +103,7 @@ def EncodeFmap(image_size, name, areas):
     """
     def _FormatBlob(fmt, names, obj):
         params = [getattr(obj, name) for name in names]
+        ConvertName(names, params)
         return struct.pack(fmt, *params)
 
     values = FmapHeader(FMAP_SIGNATURE, 1, 0, 0, image_size, name, len(areas))
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index f8faef1893..1abb768e64 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -1663,6 +1663,51 @@ class TestFunctional(unittest.TestCase):
         self.assertEqual('tplnodtb with microc' + pos_and_size +
                          'ter somewhere in here', first)
 
+    def testFmapX86(self):
+        """Basic test of generation of a flashrom fmap"""
+        data = self._DoReadFile('94_fmap_x86.dts')
+        fhdr, fentries = fmap_util.DecodeFmap(data[32:])
+        expected = U_BOOT_DATA + MRC_DATA + 'a' * (32 - 7)
+        self.assertEqual(expected, data[:32])
+        fhdr, fentries = fmap_util.DecodeFmap(data[32:])
+
+        self.assertEqual(0x100, fhdr.image_size)
+
+        self.assertEqual(0, fentries[0].offset)
+        self.assertEqual(4, fentries[0].size)
+        self.assertEqual('U_BOOT', fentries[0].name)
+
+        self.assertEqual(4, fentries[1].offset)
+        self.assertEqual(3, fentries[1].size)
+        self.assertEqual('INTEL_MRC', fentries[1].name)
+
+        self.assertEqual(32, fentries[2].offset)
+        self.assertEqual(fmap_util.FMAP_HEADER_LEN +
+                         fmap_util.FMAP_AREA_LEN * 3, fentries[2].size)
+        self.assertEqual('FMAP', fentries[2].name)
+
+    def testFmapX86Section(self):
+        """Basic test of generation of a flashrom fmap"""
+        data = self._DoReadFile('95_fmap_x86_section.dts')
+        expected = U_BOOT_DATA + MRC_DATA + 'b' * (32 - 7)
+        self.assertEqual(expected, data[:32])
+        fhdr, fentries = fmap_util.DecodeFmap(data[36:])
+
+        self.assertEqual(0x100, fhdr.image_size)
+
+        self.assertEqual(0, fentries[0].offset)
+        self.assertEqual(4, fentries[0].size)
+        self.assertEqual('U_BOOT', fentries[0].name)
+
+        self.assertEqual(4, fentries[1].offset)
+        self.assertEqual(3, fentries[1].size)
+        self.assertEqual('INTEL_MRC', fentries[1].name)
+
+        self.assertEqual(36, fentries[2].offset)
+        self.assertEqual(fmap_util.FMAP_HEADER_LEN +
+                         fmap_util.FMAP_AREA_LEN * 3, fentries[2].size)
+        self.assertEqual('FMAP', fentries[2].name)
+
 
 if __name__ == "__main__":
     unittest.main()
diff --git a/tools/binman/test/94_fmap_x86.dts b/tools/binman/test/94_fmap_x86.dts
new file mode 100644
index 0000000000..613c5dab42
--- /dev/null
+++ b/tools/binman/test/94_fmap_x86.dts
@@ -0,0 +1,20 @@
+// SPDX-License-Identifier: GPL-2.0+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		end-at-4gb;
+		size = <0x100>;
+		pad-byte = <0x61>;
+		u-boot {
+		};
+		intel-mrc {
+		};
+		fmap {
+			offset = <0xffffff20>;
+		};
+	};
+};
diff --git a/tools/binman/test/95_fmap_x86_section.dts b/tools/binman/test/95_fmap_x86_section.dts
new file mode 100644
index 0000000000..4cfce45670
--- /dev/null
+++ b/tools/binman/test/95_fmap_x86_section.dts
@@ -0,0 +1,22 @@
+// SPDX-License-Identifier: GPL-2.0+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		end-at-4gb;
+		size = <0x100>;
+		u-boot {
+		};
+		section {
+			pad-byte = <0x62>;
+			intel-mrc {
+			};
+			fmap {
+				offset = <0x20>;
+			};
+		};
+	};
+};