From patchwork Tue Jul 2 00:24:44 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 1125733 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=lists.denx.de (client-ip=81.169.180.215; helo=lists.denx.de; envelope-from=u-boot-bounces@lists.denx.de; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=chromium.org Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=chromium.org header.i=@chromium.org header.b="O6T5CEV+"; dkim-atps=neutral Received: from lists.denx.de (dione.denx.de [81.169.180.215]) by ozlabs.org (Postfix) with ESMTP id 45d4vc0r9Tz9s4V for ; Tue, 2 Jul 2019 10:32:44 +1000 (AEST) Received: by lists.denx.de (Postfix, from userid 105) id CEC75C21DA2; Tue, 2 Jul 2019 00:30:10 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on lists.denx.de X-Spam-Level: X-Spam-Status: No, score=0.0 required=5.0 tests=T_DKIM_INVALID autolearn=unavailable autolearn_force=no version=3.4.0 Received: from lists.denx.de (localhost [IPv6:::1]) by lists.denx.de (Postfix) with ESMTP id 83D41C21DA6; Tue, 2 Jul 2019 00:25:37 +0000 (UTC) Received: by lists.denx.de (Postfix, from userid 105) id C721CC21DA1; Tue, 2 Jul 2019 00:25:23 +0000 (UTC) Received: from mail-io1-f43.google.com (mail-io1-f43.google.com [209.85.166.43]) by lists.denx.de (Postfix) with ESMTPS id 1511AC21E2B for ; Tue, 2 Jul 2019 00:25:23 +0000 (UTC) Received: by mail-io1-f43.google.com with SMTP id k8so33076022iot.1 for ; Mon, 01 Jul 2019 17:25:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=Lbbhi6fu0aa4ES3Li0LvrwYpeT/xKy4nzDqbd3vaW2U=; b=O6T5CEV+o7IRKkcMvFEkGlsf3ifijp1z18Jm2mF2B5ZnhjwSUTbdJLQHpR+kTL83PU tWwZndegvkM+pR8qSi77jkdk/UKXBZM688oXA3+qjaI3JBbaxPRXG+3Y7EwP8F0bZSDd 6q0EAphIKda8JUKSQJ77CZx8jgcIjiUMKwXmo= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=Lbbhi6fu0aa4ES3Li0LvrwYpeT/xKy4nzDqbd3vaW2U=; b=HSlA8aNnKG0iF4e21h/YOhzdGerMFaZjuEuqXuAnyIoNTGeHZVC2haRHUOrRO7Kavm m1WoX/X7x7ECNTTndKE0ApIz9o1TwS47obUsxG1s7LBkErh0H8Nw4NE3oJZAm8McKOUR pC5o4q7IDZn8BJvz9tHXdhAKOORHyQm6ecotN2+orzFxlGrAsYXf50t2QpBum5x6qnfu uoyLjfzvuWAPYqkpj8eGh8R13HpmtqktKvkTYrEY62HpWlKuzj9HdGAOEHKbO0uQIIEq AQQDzkan6JbddoMybo11UGet0uCnv5ce+4PQPlcOiTwefA0Al1UfDSHCOD+GUIZ5WQGd PHSw== X-Gm-Message-State: APjAAAXUggxqhja+ugLZvIypkB/qesZxvPYuGTebd6nsHvtj2RzJzGhg tO2rTgkmfMfQzCooFhQnJgmTstyG0w4= X-Google-Smtp-Source: APXvYqwBP6mP9a+C/uNfodTCf3ed6iW2378uv9E2uMg719XVEcsc3KRdXW+SGPwLN/Tah3lsZKoRSg== X-Received: by 2002:a05:6638:38a:: with SMTP id y10mr33476916jap.104.1562027121603; Mon, 01 Jul 2019 17:25:21 -0700 (PDT) Received: from kiwi.bld.corp.google.com ([2620:15c:183:0:8223:87c:a681:66aa]) by smtp.gmail.com with ESMTPSA id h19sm10539202iol.65.2019.07.01.17.25.21 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Mon, 01 Jul 2019 17:25:21 -0700 (PDT) From: Simon Glass To: U-Boot Mailing List Date: Mon, 1 Jul 2019 18:24:44 -0600 Message-Id: <20190702002455.996-16-sjg@chromium.org> X-Mailer: git-send-email 2.22.0.410.gd8fdbe21b5-goog In-Reply-To: <20190702002455.996-1-sjg@chromium.org> References: <20190702002455.996-1-sjg@chromium.org> MIME-Version: 1.0 Cc: Tom Rini Subject: [U-Boot] [PATCH 15/26] binman: Allow entries to expand after packing X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.18 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" Add support for detecting entries that change size after they have already been packed, and re-running packing when it happens. This removes the limitation that entry size cannot change after PackEntries() is called. Signed-off-by: Simon Glass --- tools/binman/README | 3 +- tools/binman/bsection.py | 11 ++++++ tools/binman/control.py | 39 ++++++++++++------- tools/binman/entry.py | 18 ++++++++- tools/binman/etype/_testing.py | 11 +++++- tools/binman/etype/section.py | 5 +++ tools/binman/etype/u_boot_with_ucode_ptr.py | 2 +- tools/binman/ftest.py | 33 ++++++++++++++-- tools/binman/image.py | 8 ++++ tools/binman/test/121_entry_expand.dts | 20 ++++++++++ tools/binman/test/122_entry_expand_twice.dts | 21 ++++++++++ .../binman/test/123_entry_expand_section.dts | 22 +++++++++++ 12 files changed, 168 insertions(+), 25 deletions(-) create mode 100644 tools/binman/test/121_entry_expand.dts create mode 100644 tools/binman/test/122_entry_expand_twice.dts create mode 100644 tools/binman/test/123_entry_expand_section.dts diff --git a/tools/binman/README b/tools/binman/README index 150d94c65b8..7e745a2d466 100644 --- a/tools/binman/README +++ b/tools/binman/README @@ -566,7 +566,8 @@ tree. This sets the correct 'offset' and 'size' vaues, for example. The default implementatoin does nothing. This can be overriden to adjust the contents of an entry in some way. For example, it would be possible to create an entry containing a hash of the contents of some other entries. At this -stage the offset and size of entries should not be adjusted. +stage the offset and size of entries should not be adjusted unless absolutely +necessary, since it requires a repack (going back to PackEntries()). 10. WriteSymbols() - write the value of symbols into the U-Boot SPL binary. See 'Access to binman entry offsets at run time' below for a description of diff --git a/tools/binman/bsection.py b/tools/binman/bsection.py index f49a6e93bc7..d368e3f6baa 100644 --- a/tools/binman/bsection.py +++ b/tools/binman/bsection.py @@ -45,6 +45,8 @@ class Section(object): _name_prefix: Prefix to add to the name of all entries within this section _entries: OrderedDict() of entries + _orig_offset: Original offset value read from node + _orig_size: Original size value read from node """ def __init__(self, name, parent_section, node, image, test=False): global entry @@ -76,6 +78,8 @@ class Section(object): """Read properties from the section node""" self._offset = fdt_util.GetInt(self._node, 'offset') self._size = fdt_util.GetInt(self._node, 'size') + self._orig_offset = self._offset + self._orig_size = self._size self._align_size = fdt_util.GetInt(self._node, 'align-size') if tools.NotPowerOfTwo(self._align_size): self._Raise("Alignment size %s must be a power of two" % @@ -257,6 +261,13 @@ class Section(object): for name, info in offset_dict.items(): self._SetEntryOffsetSize(name, *info) + def ResetForPack(self): + """Reset offset/size fields so that packing can be done again""" + self._offset = self._orig_offset + self._size = self._orig_size + for entry in self._entries.values(): + entry.ResetForPack() + def PackEntries(self): """Pack all entries into the section""" offset = self._skip_at_start diff --git a/tools/binman/control.py b/tools/binman/control.py index 9022cf76e99..d48e7ac0070 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -170,21 +170,30 @@ def Binman(args): # completed and written, but that does not seem important. image.GetEntryContents() image.GetEntryOffsets() - try: - image.PackEntries() - image.CheckSize() - image.CheckEntries() - except Exception as e: - if args.map: - fname = image.WriteMap() - print("Wrote map file '%s' to show errors" % fname) - raise - image.SetImagePos() - if args.update_fdt: - image.SetCalculatedProperties() - for dtb_item in state.GetFdts(): - dtb_item.Sync() - image.ProcessEntryContents() + passes = 2 + for pack_pass in range(passes): + try: + image.PackEntries() + image.CheckSize() + image.CheckEntries() + except Exception as e: + if args.map: + fname = image.WriteMap() + print("Wrote map file '%s' to show errors" % fname) + raise + image.SetImagePos() + if args.update_fdt: + image.SetCalculatedProperties() + for dtb_item in state.GetFdts(): + dtb_item.Sync() + sizes_ok = image.ProcessEntryContents() + if sizes_ok: + break + image.ResetForPack() + if not sizes_ok: + image.Raise('Entries expanded after packing (tried %s passes' % + passes) + image.WriteSymbols() image.BuildImage() if args.map: diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 7db1402b84f..a9a9d119e1d 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -61,6 +61,8 @@ class Entry(object): pad_after: Number of pad bytes after the contents, 0 if none data: Contents of entry (string of bytes) compress: Compression algoithm used (e.g. 'lz4'), 'none' if none + orig_offset: Original offset value read from node + orig_size: Original size value read from node """ def __init__(self, section, etype, node, read_node=True, name_prefix=''): self.section = section @@ -153,6 +155,7 @@ class Entry(object): self.Raise("Please use 'offset' instead of 'pos'") self.offset = fdt_util.GetInt(self._node, 'offset') self.size = fdt_util.GetInt(self._node, 'size') + self.orig_offset, self.orig_size = self.offset, self.size self.align = fdt_util.GetInt(self._node, 'align') if tools.NotPowerOfTwo(self.align): raise ValueError("Node '%s': Alignment %s must be a power of two" % @@ -255,9 +258,16 @@ class Entry(object): ValueError if the new data size is not the same as the old """ size_ok = True - if len(data) != self.contents_size: + new_size = len(data) + if state.AllowEntryExpansion(): + if new_size > self.contents_size: + print("Entry '%s' size change from %#x to %#x" % ( + self._node.path, self.contents_size, new_size)) + # self.data will indicate the new size needed + size_ok = False + elif new_size != self.contents_size: self.Raise('Cannot update entry size from %d to %d' % - (self.contents_size, len(data))) + (self.contents_size, new_size)) self.SetContents(data) return size_ok @@ -271,6 +281,10 @@ class Entry(object): # No contents by default: subclasses can implement this return True + def ResetForPack(self): + """Reset offset/size fields so that packing can be done again""" + self.offset, self.size = self.orig_offset, self.orig_size + def Pack(self, offset): """Figure out how to pack the entry into the section diff --git a/tools/binman/etype/_testing.py b/tools/binman/etype/_testing.py index 2204362281c..ae24fe8105a 100644 --- a/tools/binman/etype/_testing.py +++ b/tools/binman/etype/_testing.py @@ -50,6 +50,8 @@ class Entry__testing(Entry): 'bad-update-contents') self.return_contents_once = fdt_util.GetBool(self._node, 'return-contents-once') + self.bad_update_contents_twice = fdt_util.GetBool(self._node, + 'bad-update-contents-twice') # Set to True when the entry is ready to process the FDT. self.process_fdt_ready = False @@ -71,11 +73,12 @@ class Entry__testing(Entry): if self.force_bad_datatype: self.GetEntryArgsOrProps([EntryArg('test-bad-datatype-arg', bool)]) self.return_contents = True + self.contents = b'a' def ObtainContents(self): if self.return_unknown_contents or not self.return_contents: return False - self.data = b'a' + self.data = self.contents self.contents_size = len(self.data) if self.return_contents_once: self.return_contents = False @@ -90,7 +93,11 @@ class Entry__testing(Entry): if self.bad_update_contents: # Request to update the contents with something larger, to cause a # failure. - return self.ProcessContentsUpdate('aa') + if self.bad_update_contents_twice: + self.contents += b'a' + else: + self.contents = b'aa' + return self.ProcessContentsUpdate(self.contents) return True def ProcessFdt(self, fdt): diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index 51eddcd995a..23bf22113d4 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -64,6 +64,11 @@ class Entry_section(Entry): self._section.GetEntryOffsets() return {} + def ResetForPack(self): + """Reset offset/size fields so that packing can be done again""" + self._section.ResetForPack() + Entry.ResetForPack(self) + def Pack(self, offset): """Pack all entries into the section""" self._section.PackEntries() diff --git a/tools/binman/etype/u_boot_with_ucode_ptr.py b/tools/binman/etype/u_boot_with_ucode_ptr.py index 4104bf8bf13..cb7dbc68dbb 100644 --- a/tools/binman/etype/u_boot_with_ucode_ptr.py +++ b/tools/binman/etype/u_boot_with_ucode_ptr.py @@ -49,7 +49,7 @@ class Entry_u_boot_with_ucode_ptr(Entry_blob): def ProcessContents(self): # If the image does not need microcode, there is nothing to do if not self.target_offset: - return + return True # Get the offset of the microcode ucode_entry = self.section.FindEntryType('u-boot-ucode') diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 8c9942982ba..0ff8b5e2de8 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -1210,10 +1210,14 @@ class TestFunctional(unittest.TestCase): def testBadChangeSize(self): """Test that trying to change the size of an entry fails""" - with self.assertRaises(ValueError) as e: - self._DoReadFile('059_change_size.dts', True) - self.assertIn("Node '/binman/_testing': Cannot update entry size from " - '1 to 2', str(e.exception)) + try: + state.SetAllowEntryExpansion(False) + with self.assertRaises(ValueError) as e: + self._DoReadFile('059_change_size.dts', True) + self.assertIn("Node '/binman/_testing': Cannot update entry size from 1 to 2", + str(e.exception)) + finally: + state.SetAllowEntryExpansion(True) def testUpdateFdt(self): """Test that we can update the device tree with offset/size info""" @@ -2108,6 +2112,27 @@ class TestFunctional(unittest.TestCase): self.assertIn("Invalid location 'None', expected 'start' or 'end'", str(e.exception)) + def testEntryExpand(self): + """Test expanding an entry after it is packed""" + data = self._DoReadFile('121_entry_expand.dts') + self.assertEqual('aa', data[:2]) + self.assertEqual(U_BOOT_DATA, data[2:2 + len(U_BOOT_DATA)]) + self.assertEqual('aa', data[-2:]) + + def testEntryExpandBad(self): + """Test expanding an entry after it is packed, twice""" + with self.assertRaises(ValueError) as e: + self._DoReadFile('122_entry_expand_twice.dts') + self.assertIn("Image '/binman': Entries expanded after packing", + str(e.exception)) + + def testEntryExpandSection(self): + """Test expanding an entry within a section after it is packed""" + data = self._DoReadFile('123_entry_expand_section.dts') + self.assertEqual('aa', data[:2]) + self.assertEqual(U_BOOT_DATA, data[2:2 + len(U_BOOT_DATA)]) + self.assertEqual('aa', data[-2:]) + if __name__ == "__main__": unittest.main() diff --git a/tools/binman/image.py b/tools/binman/image.py index c8bce394aa1..6339d020e76 100644 --- a/tools/binman/image.py +++ b/tools/binman/image.py @@ -55,6 +55,10 @@ class Image: self._filename = filename self._section = bsection.Section('main-section', None, self._node, self) + def Raise(self, msg): + """Convenience function to raise an error referencing an image""" + raise ValueError("Image '%s': %s" % (self._node.path, msg)) + def GetFdtSet(self): """Get the set of device tree files used by this image""" return self._section.GetFdtSet() @@ -100,6 +104,10 @@ class Image: """ self._section.GetEntryOffsets() + def ResetForPack(self): + """Reset offset/size fields so that packing can be done again""" + self._section.ResetForPack() + def PackEntries(self): """Pack all entries into the image""" self._section.PackEntries() diff --git a/tools/binman/test/121_entry_expand.dts b/tools/binman/test/121_entry_expand.dts new file mode 100644 index 00000000000..ebb7816db90 --- /dev/null +++ b/tools/binman/test/121_entry_expand.dts @@ -0,0 +1,20 @@ +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + _testing { + bad-update-contents; + }; + + u-boot { + }; + + _testing2 { + type = "_testing"; + bad-update-contents; + }; + }; +}; diff --git a/tools/binman/test/122_entry_expand_twice.dts b/tools/binman/test/122_entry_expand_twice.dts new file mode 100644 index 00000000000..258cf859f4b --- /dev/null +++ b/tools/binman/test/122_entry_expand_twice.dts @@ -0,0 +1,21 @@ +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + _testing { + bad-update-contents; + bad-update-contents-twice; + }; + + u-boot { + }; + + _testing2 { + type = "_testing"; + bad-update-contents; + }; + }; +}; diff --git a/tools/binman/test/123_entry_expand_section.dts b/tools/binman/test/123_entry_expand_section.dts new file mode 100644 index 00000000000..046f7234348 --- /dev/null +++ b/tools/binman/test/123_entry_expand_section.dts @@ -0,0 +1,22 @@ +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + _testing { + bad-update-contents; + }; + + u-boot { + }; + + section { + _testing2 { + type = "_testing"; + bad-update-contents; + }; + }; + }; +};