From patchwork Wed Jun 6 00:46:40 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 925671 X-Patchwork-Delegate: sjg@chromium.org 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" (2048-bit key; unprotected) header.d=google.com header.i=@google.com header.b="POeJjQqC"; dkim-atps=neutral Received: from lists.denx.de (dione.denx.de [81.169.180.215]) by ozlabs.org (Postfix) with ESMTP id 410qrg4CF5z9s02 for ; Wed, 6 Jun 2018 10:52:19 +1000 (AEST) Received: by lists.denx.de (Postfix, from userid 105) id 3DEA3C21FCE; Wed, 6 Jun 2018 00:48:37 +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=RCVD_IN_DNSWL_BLOCKED, RCVD_IN_MSPIKE_H2, 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 EFA35C21F84; Wed, 6 Jun 2018 00:47:42 +0000 (UTC) Received: by lists.denx.de (Postfix, from userid 105) id 48BD1C21F6E; Wed, 6 Jun 2018 00:47:26 +0000 (UTC) Received: from mail-io0-f177.google.com (mail-io0-f177.google.com [209.85.223.177]) by lists.denx.de (Postfix) with ESMTPS id 7AE82C21EF0 for ; Wed, 6 Jun 2018 00:47:21 +0000 (UTC) Received: by mail-io0-f177.google.com with SMTP id t6-v6so5634108iob.10 for ; Tue, 05 Jun 2018 17:47:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=sender:from:to:cc:subject:date:message-id:in-reply-to:references; bh=VFAZYc0jDX6xUnMb+r6O8Yok+UP8lT2I+cFJDCnmcY4=; b=POeJjQqC5jMRyvSEUD/zTgJw3dc50Kpc+zDJlvAaxdYK3T8K7JrsNc3sIRU4yZpsut jXAEi8zVqn4PB0lxpCcGKC4yfNTS9h3qrMxvPQLqR9w8N5y9mRUEyHN80yo/ZzjOzfDd MbWq65WL3SYQx0KxgAYEhEqDQ2am2ZsX9rjAfhbzq+oWaLflBuQ3kuNJmqEUoWQB0ZDu o7Q4jbiRV3U19PC7M4FIO6bNueHEBYuhrtKLc4/s6aZ2R5AFzULKOGFaZQCyiRdJ2ADj +dGQb6aNyHmWUf9+/0tvWZZ3pAn0N9oZk/mra5kyc2Ia9sD/LOxlY84aZrOdMBeLQa2m TkZA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:from:to:cc:subject:date:message-id :in-reply-to:references; bh=VFAZYc0jDX6xUnMb+r6O8Yok+UP8lT2I+cFJDCnmcY4=; b=kYxlql2Hq/nX0oAtkptrIXuXVagTIeMTPptHrf74sQYxinVcTDZlOy3MNaSnrjB2T+ zMFMMoVaL1NJrP21c2hc1dgu3bfRk8JwDx+mTVtsFYCPhh4ZZHrSjCg9c6Ukp6/9zEH8 CAL18x+wPqZoFPudDeHinbG1+Ze2u5DxQ4sug9VdhHSqX/9jFTOhUtang+t/cw1+NIkE xavrvh0K2buER3nXUjC2EIA+w6I6nWlyW31NJUR73ZVZlr5Z9JF1mSmSP46vD339BouB zHDS5A0BNvh+6pHMMNP23IVGR0lGIybOCxOND71FdPFUjhLYwKSlqpeiynI+N7XstvX9 TYNQ== X-Gm-Message-State: APt69E2FXWmlyL29RRCuhgjZklTtorUiut9Tfzq7aDcOgx0jiJwdCRJA iaLKzcXdcaW37RaUlpff1+nu3g== X-Google-Smtp-Source: ADUXVKJFzm7Vvi2QJFfCof0wctOuTPbRpZ/DYPkKVhxAfZhuvf5qIqFsomNxV9GJu05JKxSRzbZVtg== X-Received: by 2002:a6b:a510:: with SMTP id o16-v6mr788481ioe.271.1528246039732; Tue, 05 Jun 2018 17:47:19 -0700 (PDT) Received: from kiwi.bld.corp.google.com ([100.67.80.24]) by smtp.gmail.com with ESMTPSA id u8-v6sm6085287iol.6.2018.06.05.17.47.17 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 05 Jun 2018 17:47:17 -0700 (PDT) Received: by kiwi.bld.corp.google.com (Postfix, from userid 121222) id 226D8140700; Tue, 5 Jun 2018 18:47:17 -0600 (MDT) From: Simon Glass To: U-Boot Mailing List Date: Tue, 5 Jun 2018 18:46:40 -0600 Message-Id: <20180606004705.79641-5-sjg@chromium.org> X-Mailer: git-send-email 2.17.1.1185.g55be947832-goog In-Reply-To: <20180606004705.79641-1-sjg@chromium.org> References: <20180606004705.79641-1-sjg@chromium.org> Cc: Tom Rini Subject: [U-Boot] [PATCH 04/29] binman: Correct operation of ObtainContents() 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: , MIME-Version: 1.0 Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" This method is supposed to return the contents of an entry. However at present there is no check that it actually does. Also some implementations do not return 'True' to indicate success, as required. Add a check for things working as expected, and correct the implementations. This requires some additional test cases to cover things which were missed originally. Add these at the same time. Signed-off-by: Simon Glass --- tools/binman/bsection.py | 4 ++ tools/binman/etype/_testing.py | 5 ++ tools/binman/etype/section.py | 2 +- tools/binman/etype/u_boot_spl_bss_pad.py | 1 + tools/binman/etype/u_boot_ucode.py | 9 ++- tools/binman/ftest.py | 57 +++++++++++++++---- tools/binman/test/57_unknown_contents.dts | 14 +++++ .../test/58_x86_ucode_spl_needs_retry.dts | 36 ++++++++++++ 8 files changed, 114 insertions(+), 14 deletions(-) create mode 100644 tools/binman/test/57_unknown_contents.dts create mode 100644 tools/binman/test/58_x86_ucode_spl_needs_retry.dts diff --git a/tools/binman/bsection.py b/tools/binman/bsection.py index 3f30f6e4fe7..06a6711350a 100644 --- a/tools/binman/bsection.py +++ b/tools/binman/bsection.py @@ -162,6 +162,10 @@ class Section(object): todo = next_todo if not todo: break + if todo: + self._Raise('Internal error: Could not complete processing of ' + 'contents: remaining %s' % todo) + return True def _SetEntryPosSize(self, name, pos, size): """Set the position and size of an entry diff --git a/tools/binman/etype/_testing.py b/tools/binman/etype/_testing.py index 0b1eaefc3ce..c075c3ff0d9 100644 --- a/tools/binman/etype/_testing.py +++ b/tools/binman/etype/_testing.py @@ -9,6 +9,7 @@ from entry import Entry import fdt_util import tools + class Entry__testing(Entry): """A fake entry used for testing @@ -19,8 +20,12 @@ class Entry__testing(Entry): Entry.__init__(self, section, etype, node) self.return_invalid_entry = fdt_util.GetBool(self._node, 'return-invalid-entry') + self.return_unknown_contents = fdt_util.GetBool(self._node, + 'return-unknown-contents') def ObtainContents(self): + if self.return_unknown_contents: + return False self.data = 'a' self.contents_size = len(self.data) return True diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index 139fcad51af..36b31a849f8 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -18,7 +18,7 @@ class Entry_section(Entry): self._section = bsection.Section(node.name, node) def ObtainContents(self): - self._section.GetEntryContents() + return self._section.GetEntryContents() def GetData(self): return self._section.GetData() diff --git a/tools/binman/etype/u_boot_spl_bss_pad.py b/tools/binman/etype/u_boot_spl_bss_pad.py index 3d2dea2e0d8..6c397957e30 100644 --- a/tools/binman/etype/u_boot_spl_bss_pad.py +++ b/tools/binman/etype/u_boot_spl_bss_pad.py @@ -24,3 +24,4 @@ class Entry_u_boot_spl_bss_pad(Entry_blob): self.Raise('Expected __bss_size symbol in spl/u-boot-spl') self.data = chr(0) * bss_size self.contents_size = bss_size + return True diff --git a/tools/binman/etype/u_boot_ucode.py b/tools/binman/etype/u_boot_ucode.py index 3a0cff7c3a3..8cae7deed3c 100644 --- a/tools/binman/etype/u_boot_ucode.py +++ b/tools/binman/etype/u_boot_ucode.py @@ -64,9 +64,14 @@ class Entry_u_boot_ucode(Entry_blob): self.data = '' return True - # Get the microcode from the device tree entry + # Get the microcode from the device tree entry. If it is not available + # yet, return False so we will be called later. If the section simply + # doesn't exist, then we may as well return True, since we are going to + # get an error anyway. fdt_entry = self.section.FindEntryType('u-boot-dtb-with-ucode') - if not fdt_entry or not fdt_entry.ucode_data: + if not fdt_entry: + return True + if not fdt_entry.ucode_data: return False if not fdt_entry.collate: diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 80eadeffab8..ca9d158eef3 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -688,12 +688,14 @@ class TestFunctional(unittest.TestCase): data = self._DoReadFile('33_x86-start16.dts') self.assertEqual(X86_START16_DATA, data[:len(X86_START16_DATA)]) - def _RunMicrocodeTest(self, dts_fname, nodtb_data): + def _RunMicrocodeTest(self, dts_fname, nodtb_data, ucode_second=False): """Handle running a test for insertion of microcode Args: dts_fname: Name of test .dts file nodtb_data: Data that we expect in the first section + ucode_second: True if the microsecond entry is second instead of + third Returns: Tuple: @@ -704,10 +706,16 @@ class TestFunctional(unittest.TestCase): data = self._DoReadFile(dts_fname, True) # Now check the device tree has no microcode - dtb_with_ucode = data[len(nodtb_data):] - fdt_len = self.GetFdtLen(dtb_with_ucode) - ucode_content = dtb_with_ucode[fdt_len:] - ucode_pos = len(nodtb_data) + fdt_len + if ucode_second: + ucode_content = data[len(nodtb_data):] + ucode_pos = len(nodtb_data) + dtb_with_ucode = ucode_content[16:] + fdt_len = self.GetFdtLen(dtb_with_ucode) + else: + dtb_with_ucode = data[len(nodtb_data):] + fdt_len = self.GetFdtLen(dtb_with_ucode) + ucode_content = dtb_with_ucode[fdt_len:] + ucode_pos = len(nodtb_data) + fdt_len fname = tools.GetOutputFilename('test.dtb') with open(fname, 'wb') as fd: fd.write(dtb_with_ucode) @@ -728,8 +736,8 @@ class TestFunctional(unittest.TestCase): # expected position and size pos_and_size = struct.pack('<2L', 0xfffffe00 + ucode_pos, len(ucode_data)) - first = data[:len(nodtb_data)] - return first, pos_and_size + u_boot = data[:len(nodtb_data)] + return u_boot, pos_and_size def testPackUbootMicrocode(self): """Test that x86 microcode can be handled correctly @@ -892,23 +900,42 @@ class TestFunctional(unittest.TestCase): data = self._DoReadFile('48_x86-start16-spl.dts') self.assertEqual(X86_START16_SPL_DATA, data[:len(X86_START16_SPL_DATA)]) - def testPackUbootSplMicrocode(self): - """Test that x86 microcode can be handled correctly in SPL + def _PackUbootSplMicrocode(self, dts, ucode_second=False): + """Helper function for microcode tests We expect to see the following in the image, in order: u-boot-spl-nodtb.bin with a microcode pointer inserted at the correct place u-boot.dtb with the microcode removed the microcode + + Args: + dts: Device tree file to use for test + ucode_second: True if the microsecond entry is second instead of + third """ # ELF file with a '_dt_ucode_base_size' symbol with open(self.TestFile('u_boot_ucode_ptr')) as fd: TestFunctional._MakeInputFile('spl/u-boot-spl', fd.read()) - first, pos_and_size = self._RunMicrocodeTest('49_x86_ucode_spl.dts', - U_BOOT_SPL_NODTB_DATA) + first, pos_and_size = self._RunMicrocodeTest(dts, U_BOOT_SPL_NODTB_DATA, + ucode_second=ucode_second) self.assertEqual('splnodtb with microc' + pos_and_size + 'ter somewhere in here', first) + def testPackUbootSplMicrocode(self): + """Test that x86 microcode can be handled correctly in SPL""" + self._PackUbootSplMicrocode('49_x86_ucode_spl.dts') + + def testPackUbootSplMicrocodeReorder(self): + """Test that order doesn't matter for microcode entries + + This is the same as testPackUbootSplMicrocode but when we process the + u-boot-ucode entry we have not yet seen the u-boot-dtb-with-ucode + entry, so we reply on binman to try later. + """ + self._PackUbootSplMicrocode('58_x86_ucode_spl_needs_retry.dts', + ucode_second=True) + def testPackMrc(self): """Test that an image with an MRC binary can be created""" data = self._DoReadFile('50_intel_mrc.dts') @@ -971,5 +998,13 @@ class TestFunctional(unittest.TestCase): 00000000 00000004 rw-u-boot ''', map_data) + def testUnknownContents(self): + """Test that obtaining the contents works as expected""" + with self.assertRaises(ValueError) as e: + self._DoReadFile('57_unknown_contents.dts', True) + self.assertIn("Section '/binman': Internal error: Could not complete " + "processing of contents: remaining [<_testing.Entry__testing ", + str(e.exception)) + if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/57_unknown_contents.dts b/tools/binman/test/57_unknown_contents.dts new file mode 100644 index 00000000000..6ea98d7cab6 --- /dev/null +++ b/tools/binman/test/57_unknown_contents.dts @@ -0,0 +1,14 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + _testing { + return-unknown-contents; + }; + }; +}; diff --git a/tools/binman/test/58_x86_ucode_spl_needs_retry.dts b/tools/binman/test/58_x86_ucode_spl_needs_retry.dts new file mode 100644 index 00000000000..e2cb80cf6e2 --- /dev/null +++ b/tools/binman/test/58_x86_ucode_spl_needs_retry.dts @@ -0,0 +1,36 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + sort-by-pos; + end-at-4gb; + size = <0x200>; + u-boot-spl-with-ucode-ptr { + }; + + /* + * Microcode goes before the DTB which contains it, so binman + * will need to obtain the contents of the next section before + * obtaining the contents of this one. + */ + u-boot-ucode { + }; + + u-boot-dtb-with-ucode { + }; + }; + + microcode { + update@0 { + data = <0x12345678 0x12345679>; + }; + update@1 { + data = <0xabcd0000 0x78235609>; + }; + }; +};