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 ---
tools/binman/bsection.py | 18 +++++++-- tools/binman/entry.py | 3 +- tools/binman/etype/fmap.py | 11 +++-- tools/binman/etype/u_boot_with_ucode_ptr.py | 12 +++--- tools/binman/fmap_util.py | 6 ++- tools/binman/ftest.py | 45 +++++++++++++++++++++ tools/binman/test/94_fmap_x86.dts | 20 +++++++++ tools/binman/test/95_fmap_x86_section.dts | 22 ++++++++++ 8 files changed, 121 insertions(+), 16 deletions(-) create mode 100644 tools/binman/test/94_fmap_x86.dts create mode 100644 tools/binman/test/95_fmap_x86_section.dts
diff --git a/tools/binman/bsection.py b/tools/binman/bsection.py index e4c1900b17f..c208029c25b 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 fd7223477eb..01be291b709 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 f1dd81ec493..bf35a5bbf4e 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 01f3513e7d3..da0e12417b5 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 7d520e33919..be3cbee87bd 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 2a9eea2a499..ff934d02222 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -1664,6 +1664,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 00000000000..613c5dab425 --- /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 00000000000..4cfce456705 --- /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>; + }; + }; + }; +};