From ee813c86f99006bf37826d195e7212cf98803de6 Mon Sep 17 00:00:00 2001 From: Alper Nebi Yasak Date: Wed, 9 Feb 2022 22:02:35 +0300 Subject: binman: Skip processing "hash" subnodes of FIT subsections Binman's FIT entry type can have image subentries with "hash" subnodes intended to be processed by mkimage, but not binman. However, the Entry class and any subclass that reuses its implementation tries to process these unconditionally. This can lead to an error when boards specify hash algorithms that binman doesn't support, but mkimage supports. Let entries skip processing these "hash" subnodes based on an instance variable, and set this instance variable for FIT subsections. Also re-enable processing of calculated and missing properties of FIT entries which was disabled to mitigate this issue. Signed-off-by: Alper Nebi Yasak Reviewed-by: Simon Glass --- tools/binman/entry.py | 23 ++++++++++--- tools/binman/etype/fit.py | 12 ++----- tools/binman/ftest.py | 24 +++++++++++++ tools/binman/test/221_fit_subentry_hash.dts | 52 +++++++++++++++++++++++++++++ 4 files changed, 97 insertions(+), 14 deletions(-) create mode 100644 tools/binman/test/221_fit_subentry_hash.dts diff --git a/tools/binman/entry.py b/tools/binman/entry.py index dc26f8f167b..631215dfc88 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -78,6 +78,8 @@ class Entry(object): external: True if this entry contains an external binary blob bintools: Bintools used by this entry (only populated for Image) missing_bintools: List of missing bintools for this entry + update_hash: True if this entry's "hash" subnode should be + updated with a hash of the entry contents """ def __init__(self, section, etype, node, name_prefix=''): # Put this here to allow entry-docs and help to work without libfdt @@ -111,6 +113,7 @@ class Entry(object): self.allow_fake = False self.bintools = {} self.missing_bintools = [] + self.update_hash = True @staticmethod def FindEntryClass(etype, expanded): @@ -315,9 +318,11 @@ class Entry(object): if self.compress != 'none': state.AddZeroProp(self._node, 'uncomp-size') - err = state.CheckAddHashProp(self._node) - if err: - self.Raise(err) + + if self.update_hash: + err = state.CheckAddHashProp(self._node) + if err: + self.Raise(err) def SetCalculatedProperties(self): """Set the value of device-tree properties calculated by binman""" @@ -333,7 +338,9 @@ class Entry(object): state.SetInt(self._node, 'orig-size', self.orig_size, True) if self.uncomp_size is not None: state.SetInt(self._node, 'uncomp-size', self.uncomp_size) - state.CheckSetHashValue(self._node, self.GetData) + + if self.update_hash: + state.CheckSetHashValue(self._node, self.GetData) def ProcessFdt(self, fdt): """Allow entries to adjust the device tree @@ -1108,3 +1115,11 @@ features to produce new behaviours. btool = bintool.Bintool.create(name) tools[name] = btool return btool + + def SetUpdateHash(self, update_hash): + """Set whether this entry's "hash" subnode should be updated + + Args: + update_hash: True if hash should be updated, False if not + """ + self.update_hash = update_hash diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py index a56b0564f9a..cd7ebc571e2 100644 --- a/tools/binman/etype/fit.py +++ b/tools/binman/etype/fit.py @@ -186,6 +186,8 @@ class Entry_fit(Entry_section): # 'data' property later. entry = Entry.Create(self.section, node, etype='section') entry.ReadNode() + # The hash subnodes here are for mkimage, not binman. + entry.SetUpdateHash(False) self._entries[rel_path] = entry for subnode in node.subnodes: @@ -294,13 +296,3 @@ class Entry_fit(Entry_section): def AddBintools(self, tools): super().AddBintools(tools) self.mkimage = self.AddBintool(tools, 'mkimage') - - def AddMissingProperties(self, have_image_pos): - # We don't want to interfere with any hash properties in the FIT, so - # disable this for now. - pass - - def SetCalculatedProperties(self): - # We don't want to interfere with any hash properties in the FIT, so - # disable this for now. - pass diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 59b6d52fbe4..b6801b72756 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -5160,5 +5160,29 @@ fdt fdtmap Extract the devicetree blob from the fdtmap self.assertRegex(err, "Image 'main-section'.*missing bintools.*: futility") + def testFitSubentryHashSubnode(self): + """Test an image with a FIT inside""" + data, _, _, out_dtb_name = self._DoReadFileDtb( + '221_fit_subentry_hash.dts', use_real_dtb=True, update_dtb=True) + + mkimage_dtb = fdt.Fdt.FromData(data) + mkimage_dtb.Scan() + binman_dtb = fdt.Fdt(out_dtb_name) + binman_dtb.Scan() + + # Check that binman didn't add hash values + fnode = binman_dtb.GetNode('/binman/fit/images/kernel/hash') + self.assertNotIn('value', fnode.props) + + fnode = binman_dtb.GetNode('/binman/fit/images/fdt-1/hash') + self.assertNotIn('value', fnode.props) + + # Check that mkimage added hash values + fnode = mkimage_dtb.GetNode('/images/kernel/hash') + self.assertIn('value', fnode.props) + + fnode = mkimage_dtb.GetNode('/images/fdt-1/hash') + self.assertIn('value', fnode.props) + if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/221_fit_subentry_hash.dts b/tools/binman/test/221_fit_subentry_hash.dts new file mode 100644 index 00000000000..2cb04f96d08 --- /dev/null +++ b/tools/binman/test/221_fit_subentry_hash.dts @@ -0,0 +1,52 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + fit { + description = "test-desc"; + #address-cells = <1>; + + images { + kernel { + description = "Vanilla Linux kernel"; + type = "kernel"; + arch = "ppc"; + os = "linux"; + compression = "gzip"; + load = <00000000>; + entry = <00000000>; + hash { + algo = "sha1"; + }; + u-boot { + }; + }; + fdt-1 { + description = "Flattened Device Tree blob"; + type = "flat_dt"; + arch = "ppc"; + compression = "none"; + hash { + algo = "crc32"; + }; + u-boot-spl-dtb { + }; + }; + }; + + configurations { + default = "conf-1"; + conf-1 { + description = "Boot Linux kernel with FDT blob"; + kernel = "kernel"; + fdt = "fdt-1"; + }; + }; + }; + }; +}; -- cgit v1.3.1 From 730922205b107acb80e51ee3c9e2e244ba8968b8 Mon Sep 17 00:00:00 2001 From: Alper Nebi Yasak Date: Tue, 8 Feb 2022 01:08:08 +0300 Subject: binman: Update image positions of FIT subentries Binman keeps track of positions of each entry in the final image, but currently this data is wrong for things included in FIT entries, especially since a previous patch makes FIT a subclass of Section and inherit its implementation. There are three ways to put data into a FIT image. It can be directly included as a "data" property, or it can be external to the FIT image represented by an offset-size pair of properties. This external offset is either "data-position" from the start of the FIT or "data-offset" from the end of the FIT, and the size is "data-size" for both. However, binman doesn't use the "data-offset" method while building FIT entries. According to the Section docstring, its subclasses should calculate and set the correct offsets and sizes in SetImagePos() method. Do this for FIT subentries for the three ways mentioned above, and add tests for the two ways binman can pack them in. Signed-off-by: Alper Nebi Yasak Reviewed-by: Simon Glass --- tools/binman/etype/fit.py | 51 +++++++++++++++++++++ tools/binman/ftest.py | 112 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 163 insertions(+) diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py index cd7ebc571e2..1e957023f35 100644 --- a/tools/binman/etype/fit.py +++ b/tools/binman/etype/fit.py @@ -293,6 +293,57 @@ class Entry_fit(Entry_section): data = fdt.GetContents() return data + def SetImagePos(self, image_pos): + """Set the position in the image + + This sets each subentry's offsets, sizes and positions-in-image + according to where they ended up in the packed FIT file. + + Args: + image_pos: Position of this entry in the image + """ + super().SetImagePos(image_pos) + + # If mkimage is missing we'll have empty data, + # which will cause a FDT_ERR_BADMAGIC error + if self.mkimage in self.missing_bintools: + return + + fdt = Fdt.FromData(self.GetData()) + fdt.Scan() + + for path, section in self._entries.items(): + node = fdt.GetNode(path) + + data_prop = node.props.get("data") + data_pos = fdt_util.GetInt(node, "data-position") + data_offset = fdt_util.GetInt(node, "data-offset") + data_size = fdt_util.GetInt(node, "data-size") + + # Contents are inside the FIT + if data_prop is not None: + # GetOffset() returns offset of a fdt_property struct, + # which has 3 fdt32_t members before the actual data. + offset = data_prop.GetOffset() + 12 + size = len(data_prop.bytes) + + # External offset from the base of the FIT + elif data_pos is not None: + offset = data_pos + size = data_size + + # External offset from the end of the FIT, not used in binman + elif data_offset is not None: # pragma: no cover + offset = fdt.GetFdtObj().totalsize() + data_offset + size = data_size + + # This should never happen + else: # pragma: no cover + self.Raise("%s: missing data properties" % (path)) + + section.SetOffsetSize(offset, size) + section.SetImagePos(self.image_pos) + def AddBintools(self, tools): super().AddBintools(tools) self.mkimage = self.AddBintool(tools, 'mkimage') diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index b6801b72756..f8e73c6f9bf 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -3770,6 +3770,62 @@ class TestFunctional(unittest.TestCase): self._CheckSimpleFitData(fit_data, U_BOOT_EXP_DATA, U_BOOT_SPL_DTB_DATA) + def testSimpleFitImagePos(self): + """Test that we have correct image-pos for FIT subentries""" + data, _, _, out_dtb_fname = self._DoReadFileDtb('161_fit.dts', + update_dtb=True) + dtb = fdt.Fdt(out_dtb_fname) + dtb.Scan() + props = self._GetPropTree(dtb, BASE_DTB_PROPS + REPACK_DTB_PROPS) + + self.assertEqual({ + 'image-pos': 0, + 'offset': 0, + 'size': 1890, + + 'u-boot:image-pos': 0, + 'u-boot:offset': 0, + 'u-boot:size': 4, + + 'fit:image-pos': 4, + 'fit:offset': 4, + 'fit:size': 1840, + + 'fit/images/kernel:image-pos': 160, + 'fit/images/kernel:offset': 156, + 'fit/images/kernel:size': 4, + + 'fit/images/kernel/u-boot:image-pos': 160, + 'fit/images/kernel/u-boot:offset': 0, + 'fit/images/kernel/u-boot:size': 4, + + 'fit/images/fdt-1:image-pos': 456, + 'fit/images/fdt-1:offset': 452, + 'fit/images/fdt-1:size': 6, + + 'fit/images/fdt-1/u-boot-spl-dtb:image-pos': 456, + 'fit/images/fdt-1/u-boot-spl-dtb:offset': 0, + 'fit/images/fdt-1/u-boot-spl-dtb:size': 6, + + 'u-boot-nodtb:image-pos': 1844, + 'u-boot-nodtb:offset': 1844, + 'u-boot-nodtb:size': 46, + }, props) + + # Actually check the data is where we think it is + for node, expected in [ + ("u-boot", U_BOOT_DATA), + ("fit/images/kernel", U_BOOT_DATA), + ("fit/images/kernel/u-boot", U_BOOT_DATA), + ("fit/images/fdt-1", U_BOOT_SPL_DTB_DATA), + ("fit/images/fdt-1/u-boot-spl-dtb", U_BOOT_SPL_DTB_DATA), + ("u-boot-nodtb", U_BOOT_NODTB_DATA), + ]: + image_pos = props[f"{node}:image-pos"] + size = props[f"{node}:size"] + self.assertEqual(len(expected), size) + self.assertEqual(expected, data[image_pos:image_pos+size]) + def testFitExternal(self): """Test an image with an FIT with external images""" data = self._DoReadFile('162_fit_external.dts') @@ -3798,6 +3854,62 @@ class TestFunctional(unittest.TestCase): self.assertEqual(U_BOOT_DATA + b'aa', data[actual_pos:actual_pos + external_data_size]) + def testFitExternalImagePos(self): + """Test that we have correct image-pos for external FIT subentries""" + data, _, _, out_dtb_fname = self._DoReadFileDtb('162_fit_external.dts', + update_dtb=True) + dtb = fdt.Fdt(out_dtb_fname) + dtb.Scan() + props = self._GetPropTree(dtb, BASE_DTB_PROPS + REPACK_DTB_PROPS) + + self.assertEqual({ + 'image-pos': 0, + 'offset': 0, + 'size': 1082, + + 'u-boot:image-pos': 0, + 'u-boot:offset': 0, + 'u-boot:size': 4, + + 'fit:size': 1032, + 'fit:offset': 4, + 'fit:image-pos': 4, + + 'fit/images/kernel:size': 4, + 'fit/images/kernel:offset': 1024, + 'fit/images/kernel:image-pos': 1028, + + 'fit/images/kernel/u-boot:size': 4, + 'fit/images/kernel/u-boot:offset': 0, + 'fit/images/kernel/u-boot:image-pos': 1028, + + 'fit/images/fdt-1:size': 2, + 'fit/images/fdt-1:offset': 1028, + 'fit/images/fdt-1:image-pos': 1032, + + 'fit/images/fdt-1/_testing:size': 2, + 'fit/images/fdt-1/_testing:offset': 0, + 'fit/images/fdt-1/_testing:image-pos': 1032, + + 'u-boot-nodtb:image-pos': 1036, + 'u-boot-nodtb:offset': 1036, + 'u-boot-nodtb:size': 46, + }, props) + + # Actually check the data is where we think it is + for node, expected in [ + ("u-boot", U_BOOT_DATA), + ("fit/images/kernel", U_BOOT_DATA), + ("fit/images/kernel/u-boot", U_BOOT_DATA), + ("fit/images/fdt-1", b'aa'), + ("fit/images/fdt-1/_testing", b'aa'), + ("u-boot-nodtb", U_BOOT_NODTB_DATA), + ]: + image_pos = props[f"{node}:image-pos"] + size = props[f"{node}:size"] + self.assertEqual(len(expected), size) + self.assertEqual(expected, data[image_pos:image_pos+size]) + def testFitMissing(self): """Test that binman still produces a FIT image if mkimage is missing""" with test_util.capture_sys_output() as (_, stderr): -- cgit v1.3.1 From 8db1f9958ff7dfc54ef673607d47694dd672dae7 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 8 Feb 2022 10:59:44 -0700 Subject: binman: Correct the error message for a bad hash algorithm This shows an internal type at present, rather than the algorithm name. Fix it and update the test to catch this. Signed-off-by: Simon Glass Reviewed-by: Alper Nebi Yasak --- tools/binman/ftest.py | 2 +- tools/binman/state.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index f8e73c6f9bf..4616a29deb6 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -2076,7 +2076,7 @@ class TestFunctional(unittest.TestCase): def testHashBadAlgo(self): with self.assertRaises(ValueError) as e: self._DoReadFileDtb('092_hash_bad_algo.dts', update_dtb=True) - self.assertIn("Node '/binman/u-boot': Unknown hash algorithm", + self.assertIn("Node '/binman/u-boot': Unknown hash algorithm 'invalid'", str(e.exception)) def testHashSection(self): diff --git a/tools/binman/state.py b/tools/binman/state.py index 8cd8a483182..a302e1f00eb 100644 --- a/tools/binman/state.py +++ b/tools/binman/state.py @@ -397,7 +397,7 @@ def CheckAddHashProp(node): if algo.value == 'sha256': size = 32 else: - return "Unknown hash algorithm '%s'" % algo + return "Unknown hash algorithm '%s'" % algo.value for n in GetUpdateNodes(hash_node): n.AddEmptyProp('value', size) -- cgit v1.3.1 From b8d11da0d09aed23c880e572144148d8c9d4cd64 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 8 Feb 2022 11:49:45 -0700 Subject: moveconfig: Show the config name rather than the defconfig The _defconfig suffix is unnecessary when showing matching boards. Drop it. Signed-off-by: Simon Glass --- tools/moveconfig.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/tools/moveconfig.py b/tools/moveconfig.py index 1bcf58caf14..5ef5a95eb6d 100755 --- a/tools/moveconfig.py +++ b/tools/moveconfig.py @@ -91,7 +91,20 @@ SIZES = { 'SZ_4G': 0x100000000 } +RE_REMOVE_DEFCONFIG = re.compile(r'(.*)_defconfig') + ### helper functions ### +def remove_defconfig(defc): + """Drop the _defconfig suffix on a string + + Args: + defc (str): String to convert + + Returns: + str: string with the '_defconfig' suffix removed + """ + return RE_REMOVE_DEFCONFIG.match(defc)[1] + def check_top_directory(): """Exit if we are not at the top of source directory.""" for fname in 'README', 'Licenses': @@ -1638,7 +1651,7 @@ def do_find_config(config_list): print(f"Error: Not in Kconfig: %s" % ' '.join(adhoc)) else: print(f'{len(out)} matches') - print(' '.join(out)) + print(' '.join([remove_defconfig(item) for item in out])) def prefix_config(cfg): -- cgit v1.3.1 From 941671a19cfb8e0fad3bd2a11ab5137e43c2ddc5 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 8 Feb 2022 11:49:46 -0700 Subject: moveconfig: Allow regex matches when finding combinations It is useful to be able to search for CONFIG options that match a regex, such as this, which lists boards which define SPL_FIT_GENERATOR and anything not starting with ROCKCHIP: ./tools/moveconfig.py -f SPL_FIT_GENERATOR ~ROCKCHIP.* Add support for this. Signed-off-by: Simon Glass --- tools/moveconfig.py | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/tools/moveconfig.py b/tools/moveconfig.py index 5ef5a95eb6d..cff1e306581 100755 --- a/tools/moveconfig.py +++ b/tools/moveconfig.py @@ -1606,12 +1606,31 @@ def do_imply_config(config_list, add_imply, imply_flags, skip_added, for linenum in sorted(linenums, reverse=True): add_imply_rule(config[CONFIG_LEN:], fname, linenum) +def defconfig_matches(configs, re_match): + """Check if any CONFIG option matches a regex + + The match must be complete, i.e. from the start to end of the CONFIG option. + + Args: + configs (dict): Dict of CONFIG options: + key: CONFIG option + value: Value of option + re_match (re.Pattern): Match to check + + Returns: + bool: True if any CONFIG matches the regex + """ + for cfg in configs: + m_cfg = re_match.match(cfg) + if m_cfg and m_cfg.span()[1] == len(cfg): + return True + return False def do_find_config(config_list): """Find boards with a given combination of CONFIGs Params: - config_list: List of CONFIG options to check (each a string consisting + config_list: List of CONFIG options to check (each a regex consisting of a config option, with or without a CONFIG_ prefix. If an option is preceded by a tilde (~) then it must be false, otherwise it must be true) @@ -1643,8 +1662,9 @@ def do_find_config(config_list): # running for the next stage in_list = out out = set() + re_match = re.compile(cfg) for defc in in_list: - has_cfg = cfg in config_db[defc] + has_cfg = defconfig_matches(config_db[defc], re_match) if has_cfg == want: out.add(defc) if adhoc: -- cgit v1.3.1 From 00959d871cc8264d9e06c8cd0ee23c324699ab9e Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 8 Feb 2022 11:49:47 -0700 Subject: spl: x86: Correct the binman symbols for SPL These symbols are incorrect, meaning that binman cannot find the associated entry. This leads to errors like: binman: Section '/binman/simple-bin': Symbol '_binman_spl_prop_size' in entry '/binman/simple-bin/u-boot-spl/u-boot-spl-nodtb': Entry 'spl' not found in list (mkimage,u-boot-spl-nodtb, u-boot-spl-bss-pad,u-boot-spl-dtb,u-boot-spl,u-boot-img,main-section) Fix it. Signed-off-by: Simon Glass --- arch/x86/dts/u-boot.dtsi | 2 +- common/spl/spl.c | 8 ++++---- include/spl.h | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/arch/x86/dts/u-boot.dtsi b/arch/x86/dts/u-boot.dtsi index ca84d18ad90..24e692f988d 100644 --- a/arch/x86/dts/u-boot.dtsi +++ b/arch/x86/dts/u-boot.dtsi @@ -37,7 +37,7 @@ u-boot-tpl-dtb { }; #endif - spl { + u-boot-spl { type = "u-boot-spl"; offset = ; }; diff --git a/common/spl/spl.c b/common/spl/spl.c index 884102bdea4..444907432c1 100644 --- a/common/spl/spl.c +++ b/common/spl/spl.c @@ -54,8 +54,8 @@ binman_sym_declare(ulong, u_boot_any, image_pos); binman_sym_declare(ulong, u_boot_any, size); #ifdef CONFIG_TPL -binman_sym_declare(ulong, spl, image_pos); -binman_sym_declare(ulong, spl, size); +binman_sym_declare(ulong, u_boot_spl, image_pos); +binman_sym_declare(ulong, u_boot_spl, size); #endif /* Define board data structure */ @@ -143,14 +143,14 @@ void spl_fixup_fdt(void *fdt_blob) ulong spl_get_image_pos(void) { return spl_phase() == PHASE_TPL ? - binman_sym(ulong, spl, image_pos) : + binman_sym(ulong, u_boot_spl, image_pos) : binman_sym(ulong, u_boot_any, image_pos); } ulong spl_get_image_size(void) { return spl_phase() == PHASE_TPL ? - binman_sym(ulong, spl, size) : + binman_sym(ulong, u_boot_spl, size) : binman_sym(ulong, u_boot_any, size); } diff --git a/include/spl.h b/include/spl.h index bb92bc6ec6c..8ceb3c0f095 100644 --- a/include/spl.h +++ b/include/spl.h @@ -269,8 +269,8 @@ struct spl_load_info { */ binman_sym_extern(ulong, u_boot_any, image_pos); binman_sym_extern(ulong, u_boot_any, size); -binman_sym_extern(ulong, spl, image_pos); -binman_sym_extern(ulong, spl, size); +binman_sym_extern(ulong, u_boot_spl, image_pos); +binman_sym_extern(ulong, u_boot_spl, size); /** * spl_get_image_pos() - get the image position of the next phase -- cgit v1.3.1 From 38c04d8e065c42254dbaca760a9e6d200321c15b Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 8 Feb 2022 11:49:48 -0700 Subject: spl: Allow disabling binman symbols in SPL When CONFIG_SPL_FIT is enabled we do not access U-Boot directly in the image, since it is embedded in a FIT which is parsed at runtime. Provide a CONFIG option to drop the symbols in this case. Signed-off-by: Simon Glass --- common/spl/Kconfig | 24 ++++++++++++++++++++++++ common/spl/spl.c | 4 ++++ 2 files changed, 28 insertions(+) diff --git a/common/spl/Kconfig b/common/spl/Kconfig index e0d0a6f77b5..9418d37b2e2 100644 --- a/common/spl/Kconfig +++ b/common/spl/Kconfig @@ -101,6 +101,18 @@ config SPL_SHOW_ERRORS This adds a small amount to SPL code size, perhaps 100 bytes. +config SPL_BINMAN_SYMBOLS + bool "Declare binman symbols in SPL" + depends on SPL_FRAMEWORK && BINMAN + default y + help + This enables use of symbols in SPL which refer to U-Boot, enabling SPL + to obtain the location of U-Boot simply by calling spl_get_image_pos() + and spl_get_image_size(). + + For this to work, you must have a U-Boot image in the binman image, so + binman can update SPL with the location of it. + menu "PowerPC and LayerScape SPL Boot options" config SPL_NAND_BOOT @@ -1321,6 +1333,18 @@ config TPL_SIZE_LIMIT Specifies the maximum length of the U-Boot TPL image. If this value is zero, it is ignored. +config TPL_BINMAN_SYMBOLS + bool "Declare binman symbols in SPL" + depends on SPL_FRAMEWORK && BINMAN + default y + help + This enables use of symbols in TPL which refer to U-Boot, enabling SPL + to obtain the location of U-Boot simply by calling spl_get_image_pos() + and spl_get_image_size(). + + For this to work, you must have a U-Boot image in the binman image, so + binman can update SPL with the location of it. + config TPL_FRAMEWORK bool "Support TPL based upon the common SPL framework" default y if SPL_FRAMEWORK diff --git a/common/spl/spl.c b/common/spl/spl.c index 444907432c1..b452d4feeb2 100644 --- a/common/spl/spl.c +++ b/common/spl/spl.c @@ -49,9 +49,11 @@ DECLARE_GLOBAL_DATA_PTR; u32 *boot_params_ptr = NULL; +#if CONFIG_IS_ENABLED(BINMAN_SYMBOLS) /* See spl.h for information about this */ binman_sym_declare(ulong, u_boot_any, image_pos); binman_sym_declare(ulong, u_boot_any, size); +#endif #ifdef CONFIG_TPL binman_sym_declare(ulong, u_boot_spl, image_pos); @@ -140,6 +142,7 @@ void spl_fixup_fdt(void *fdt_blob) #endif } +#if CONFIG_IS_ENABLED(BINMAN_SYMBOLS) ulong spl_get_image_pos(void) { return spl_phase() == PHASE_TPL ? @@ -153,6 +156,7 @@ ulong spl_get_image_size(void) binman_sym(ulong, u_boot_spl, size) : binman_sym(ulong, u_boot_any, size); } +#endif /* BINMAN_SYMBOLS */ ulong spl_get_image_text_base(void) { -- cgit v1.3.1 From bc116029c0ad25b2631629062a70a4d36157d43c Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 8 Feb 2022 11:49:50 -0700 Subject: dtoc: Support adding a string list to a device tree Add a new function to add a string list. Signed-off-by: Simon Glass --- tools/dtoc/fdt.py | 18 ++++++++++++++++++ tools/dtoc/test_fdt.py | 8 ++++++++ 2 files changed, 26 insertions(+) diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py index 7e13757a1be..026f7a273f9 100644 --- a/tools/dtoc/fdt.py +++ b/tools/dtoc/fdt.py @@ -501,6 +501,24 @@ class Node: val = bytes(val, 'utf-8') return self.AddData(prop_name, val + b'\0') + def AddStringList(self, prop_name, val): + """Add a new string-list property to a node + + The device tree is marked dirty so that the value will be written to + the blob on the next sync. + + Args: + prop_name: Name of property to add + val (list of str): List of strings to add + + Returns: + Prop added + """ + out = b'' + for string in val: + out += bytes(string, 'utf-8') + b'\0' + return self.AddData(prop_name, out) + def AddInt(self, prop_name, val): """Add a new integer property to a node diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py index c789822afae..5a5333b1342 100755 --- a/tools/dtoc/test_fdt.py +++ b/tools/dtoc/test_fdt.py @@ -531,6 +531,14 @@ class TestProp(unittest.TestCase): self.node.AddData('data', tools.get_bytes(65, 20000)) self.dtb.Sync(auto_resize=True) + def test_string_list(self): + """Test adding string-list property to a node""" + val = ['123', '456'] + self.node.AddStringList('stringlist', val) + self.dtb.Sync(auto_resize=True) + data = self.fdt.getprop(self.node.Offset(), 'stringlist') + self.assertEqual(b'123\x00456\0', data) + def testFromData(self): dtb2 = fdt.Fdt.FromData(self.dtb.GetContents()) self.assertEqual(dtb2.GetContents(), self.dtb.GetContents()) -- cgit v1.3.1 From a30c39f2f77f4fb57a22a50c6d6b477d5d2f4342 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 8 Feb 2022 11:49:51 -0700 Subject: dtoc: Support deleting a node Add a function to delete a node. This is synced to the tree when requested. Signed-off-by: Simon Glass --- tools/dtoc/fdt.py | 17 +++++++++++++++++ tools/dtoc/test_fdt.py | 9 +++++++++ 2 files changed, 26 insertions(+) diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py index 026f7a273f9..f69f89cd783 100644 --- a/tools/dtoc/fdt.py +++ b/tools/dtoc/fdt.py @@ -548,6 +548,23 @@ class Node: self.subnodes.append(subnode) return subnode + def Delete(self): + """Delete a node + + The node is deleted and the offset cache is invalidated. + + Args: + node (Node): Node to delete + + Raises: + ValueError if the node does not exist + """ + CheckErr(self._fdt._fdt_obj.del_node(self.Offset()), + "Node '%s': delete" % self.path) + parent = self.parent + self._fdt.Invalidate() + parent.subnodes.remove(self) + def Sync(self, auto_resize=False): """Sync node changes back to the device tree diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py index 5a5333b1342..ee603cc152d 100755 --- a/tools/dtoc/test_fdt.py +++ b/tools/dtoc/test_fdt.py @@ -539,6 +539,15 @@ class TestProp(unittest.TestCase): data = self.fdt.getprop(self.node.Offset(), 'stringlist') self.assertEqual(b'123\x00456\0', data) + def test_delete_node(self): + """Test deleting a node""" + old_offset = self.fdt.path_offset('/spl-test') + self.assertGreater(old_offset, 0) + self.node.Delete() + self.dtb.Sync() + new_offset = self.fdt.path_offset('/spl-test', libfdt.QUIET_NOTFOUND) + self.assertEqual(-libfdt.NOTFOUND, new_offset) + def testFromData(self): dtb2 = fdt.Fdt.FromData(self.dtb.GetContents()) self.assertEqual(dtb2.GetContents(), self.dtb.GetContents()) -- cgit v1.3.1 From dd857ee7614fea27de3eb6dbd87749a9640c69b1 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 8 Feb 2022 11:49:52 -0700 Subject: dtoc: Allow deleting nodes and adding them in the same sync This does not work at present, since the current algorithm assumes that either there are no nodes or all nodes have an offset. If a node is new, but an old node is still in the tree, then syncing fails due to this assumption. Fix it and add a test. Signed-off-by: Simon Glass --- tools/dtoc/fdt.py | 2 ++ tools/dtoc/test_fdt.py | 11 +++++++++++ 2 files changed, 13 insertions(+) diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py index f69f89cd783..c16909a8769 100644 --- a/tools/dtoc/fdt.py +++ b/tools/dtoc/fdt.py @@ -356,6 +356,8 @@ class Node: offset = fdt_obj.first_subnode(self._offset, QUIET_NOTFOUND) for subnode in self.subnodes: + if subnode._offset is None: + continue if subnode.name != fdt_obj.get_name(offset): raise ValueError('Internal error, node name mismatch %s != %s' % (subnode.name, fdt_obj.get_name(offset))) diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py index ee603cc152d..5455759acf0 100755 --- a/tools/dtoc/test_fdt.py +++ b/tools/dtoc/test_fdt.py @@ -272,6 +272,17 @@ class TestNode(unittest.TestCase): self.dtb.Sync(auto_resize=True) + def testAddOneNode(self): + """Testing deleting and adding a subnode before syncing""" + subnode = self.node.AddSubnode('subnode') + self.node.AddSubnode('subnode2') + self.dtb.Sync(auto_resize=True) + + # Delete a node and add a new one + subnode.Delete() + self.node.AddSubnode('subnode3') + self.dtb.Sync() + def testRefreshNameMismatch(self): """Test name mismatch when syncing nodes and properties""" prop = self.node.AddInt('integer-a', 12) -- cgit v1.3.1 From 7e4b66aa8743aca134883d0ebc42535ce43ecf25 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 8 Feb 2022 11:49:53 -0700 Subject: dtoc: Support reading a list of arguments It is helpful to support a string or stringlist containing a list of space-separated arguments, for example: args = "-n fred", "-a", "123"; This resolves to the list: -n fred -a 123 which can be passed to a program as arguments. Add a helper to do the required processing. Signed-off-by: Simon Glass --- tools/dtoc/fdt_util.py | 12 ++++++++++++ tools/dtoc/test/dtoc_test_simple.dts | 1 + tools/dtoc/test_fdt.py | 15 +++++++++++++++ 3 files changed, 28 insertions(+) diff --git a/tools/dtoc/fdt_util.py b/tools/dtoc/fdt_util.py index d59ea2fe62a..c82e7747aa3 100644 --- a/tools/dtoc/fdt_util.py +++ b/tools/dtoc/fdt_util.py @@ -184,6 +184,18 @@ def GetStringList(node, propname, default=None): return [strval] return value +def GetArgs(node, propname): + prop = node.props.get(propname) + if not prop: + raise ValueError(f"Node '{node.path}': Expected property '{propname}'") + if prop.bytes: + value = GetStringList(node, propname) + else: + value = [] + lists = [v.split() for v in value] + args = [x for l in lists for x in l] + return args + def GetBool(node, propname, default=False): """Get an boolean from a property diff --git a/tools/dtoc/test/dtoc_test_simple.dts b/tools/dtoc/test/dtoc_test_simple.dts index 4c2c70af222..2d321fb0345 100644 --- a/tools/dtoc/test/dtoc_test_simple.dts +++ b/tools/dtoc/test/dtoc_test_simple.dts @@ -62,5 +62,6 @@ orig-node { orig = <1 23 4>; + args = "-n first", "second", "-p", "123,456", "-x"; }; }; diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py index 5455759acf0..576d65b97e8 100755 --- a/tools/dtoc/test_fdt.py +++ b/tools/dtoc/test_fdt.py @@ -652,6 +652,21 @@ class TestFdtUtil(unittest.TestCase): self.assertEqual(['test'], fdt_util.GetStringList(self.node, 'missing', ['test'])) + def testGetArgs(self): + node = self.dtb.GetNode('/orig-node') + self.assertEqual(['message'], fdt_util.GetArgs(self.node, 'stringval')) + self.assertEqual( + ['multi-word', 'message'], + fdt_util.GetArgs(self.node, 'stringarray')) + self.assertEqual([], fdt_util.GetArgs(self.node, 'boolval')) + self.assertEqual(['-n', 'first', 'second', '-p', '123,456', '-x'], + fdt_util.GetArgs(node, 'args')) + with self.assertRaises(ValueError) as exc: + fdt_util.GetArgs(self.node, 'missing') + self.assertIn( + "Node '/spl-test': Expected property 'missing'", + str(exc.exception)) + def testGetBool(self): self.assertEqual(True, fdt_util.GetBool(self.node, 'boolval')) self.assertEqual(False, fdt_util.GetBool(self.node, 'missing')) -- cgit v1.3.1 From 206985ecb7a9aa32c4e9e2b757e0261b7922ddb8 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 8 Feb 2022 11:49:54 -0700 Subject: binman: Update docs to indicate mkimage is supported Now that there is a mkimage entry-type, update the docs to remove the future reference. Signed-off-by: Simon Glass --- tools/binman/binman.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/binman/binman.rst b/tools/binman/binman.rst index ab5a5e06b15..9e39e678a39 100644 --- a/tools/binman/binman.rst +++ b/tools/binman/binman.rst @@ -163,8 +163,8 @@ Consider sunxi. It has the following steps: Binman is intended to replace the last step. The U-Boot build system builds u-boot.bin and sunxi-spl.bin. Binman can then take over creation of -sunxi-spl.bin (by calling mksunxiboot, or hopefully one day mkimage). In any -case, it would then create the image from the component parts. +sunxi-spl.bin by calling mksunxiboot or mkimage. In any case, it would then +create the image from the component parts. This simplifies the U-Boot Makefile somewhat, since various pieces of logic can be replaced by a call to binman. -- cgit v1.3.1 From 4d38dd77f901a48736c540a9c7499816fa9b4d90 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 8 Feb 2022 11:49:55 -0700 Subject: elf: Add a way to read segment information from an ELF file Add a function which reads the segments and the entry address. Also fix a comment nit in the tests while we are here. Signed-off-by: Simon Glass --- tools/binman/elf.py | 37 +++++++++++++++++++++++++++++++++++++ tools/binman/elf_test.py | 31 +++++++++++++++++++++++++++++-- 2 files changed, 66 insertions(+), 2 deletions(-) diff --git a/tools/binman/elf.py b/tools/binman/elf.py index bc4966e8a84..5e7d6ae7b97 100644 --- a/tools/binman/elf.py +++ b/tools/binman/elf.py @@ -20,6 +20,7 @@ from patman import tout ELF_TOOLS = True try: from elftools.elf.elffile import ELFFile + from elftools.elf.elffile import ELFError from elftools.elf.sections import SymbolTableSection except: # pragma: no cover ELF_TOOLS = False @@ -369,3 +370,39 @@ def UpdateFile(infile, outfile, start_sym, end_sym, insert): newdata += data[syms[end_sym].offset:] tools.write_file(outfile, newdata) tout.info('Written to offset %#x' % syms[start_sym].offset) + +def read_segments(data): + """Read segments from an ELF file + + Args: + data (bytes): Contents of file + + Returns: + tuple: + list of segments, each: + int: Segment number (0 = first) + int: Start address of segment in memory + bytes: Contents of segment + int: entry address for image + + Raises: + ValueError: elftools is not available + """ + if not ELF_TOOLS: + raise ValueError('Python elftools package is not available') + with io.BytesIO(data) as inf: + try: + elf = ELFFile(inf) + except ELFError as err: + raise ValueError(err) + entry = elf.header['e_entry'] + segments = [] + for i in range(elf.num_segments()): + segment = elf.get_segment(i) + if segment['p_type'] != 'PT_LOAD' or not segment['p_memsz']: + skipped = 1 # To make code-coverage see this line + continue + start = segment['p_offset'] + rend = start + segment['p_filesz'] + segments.append((i, segment['p_paddr'], data[start:rend])) + return segments, entry diff --git a/tools/binman/elf_test.py b/tools/binman/elf_test.py index 47ebfbac4a6..f92352d54f6 100644 --- a/tools/binman/elf_test.py +++ b/tools/binman/elf_test.py @@ -56,8 +56,8 @@ class FakeSection: def BuildElfTestFiles(target_dir): """Build ELF files used for testing in binman - This compiles and links the test files into the specified directory. It the - Makefile and source files in the binman test/ directory. + This compiles and links the test files into the specified directory. It uses + the Makefile and source files in the binman test/ directory. Args: target_dir: Directory to put the files into @@ -258,6 +258,33 @@ class TestElf(unittest.TestCase): offset = elf.GetSymbolFileOffset(fname, ['missing_sym']) self.assertEqual({}, offset) + def test_read_segments(self): + """Test for read_segments()""" + if not elf.ELF_TOOLS: + self.skipTest('Python elftools not available') + fname = self.ElfTestFile('embed_data') + segments, entry = elf.read_segments(tools.ReadFile(fname)) + + def test_read_segments_fail(self): + """Test for read_segments() without elftools""" + try: + old_val = elf.ELF_TOOLS + elf.ELF_TOOLS = False + fname = self.ElfTestFile('embed_data') + with self.assertRaises(ValueError) as e: + elf.read_segments(tools.ReadFile(fname)) + self.assertIn('Python elftools package is not available', + str(e.exception)) + finally: + elf.ELF_TOOLS = old_val + + def test_read_segments_bad_data(self): + """Test for read_segments() with an invalid ELF file""" + fname = self.ElfTestFile('embed_data') + with self.assertRaises(ValueError) as e: + elf.read_segments(tools.GetBytes(100, 100)) + self.assertIn('Magic number does not match', str(e.exception)) + if __name__ == '__main__': unittest.main() -- cgit v1.3.1 From 47f420ae082fe3ec11afda4aab1b96e3e38752ef Mon Sep 17 00:00:00 2001 From: Roger Quadros Date: Sat, 19 Feb 2022 20:50:04 +0200 Subject: binman: Add support for TEE BL32 Add an entry for OP-TEE Trusted OS 'BL32' payload. This is required by platforms using Cortex-A cores with TrustZone technology. Signed-off-by: Roger Quadros Reviewed-by: Simon Glass Add missing-blob-help, renumber the test file, update entry-docs: Signed-off-by: Simon Glass --- Makefile | 1 + tools/binman/entries.rst | 13 +++++++++++++ tools/binman/etype/tee_os.py | 22 ++++++++++++++++++++++ tools/binman/ftest.py | 8 ++++++++ tools/binman/missing-blob-help | 4 ++++ tools/binman/test/222_tee_os.dts | 14 ++++++++++++++ 6 files changed, 62 insertions(+) create mode 100644 tools/binman/etype/tee_os.py create mode 100644 tools/binman/test/222_tee_os.dts diff --git a/Makefile b/Makefile index 4b152249ca5..697cc51d67f 100644 --- a/Makefile +++ b/Makefile @@ -1327,6 +1327,7 @@ cmd_binman = $(srctree)/tools/binman/binman $(if $(BINMAN_DEBUG),-D) \ -I arch/$(ARCH)/dts -a of-list=$(CONFIG_OF_LIST) \ $(foreach f,$(BINMAN_INDIRS),-I $(f)) \ -a atf-bl31-path=${BL31} \ + -a tee-os-path=${TEE} \ -a opensbi-path=${OPENSBI} \ -a default-dt=$(default_dt) \ -a scp-path=$(SCP) \ diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst index c47f7df0980..88230a69d4e 100644 --- a/tools/binman/entries.rst +++ b/tools/binman/entries.rst @@ -1103,6 +1103,19 @@ available. This is set by the `SetAllowMissing()` method, if +Entry: tee-os: Entry containing an OP-TEE Trusted OS (TEE) blob +--------------------------------------------------------------- + +Properties / Entry arguments: + - tee-os-path: Filename of file to read into entry. This is typically + called tee-pager.bin + +This entry holds the run-time firmware, typically started by U-Boot SPL. +See the U-Boot README for your architecture or board for how to use it. See +https://github.com/OP-TEE/optee_os for more information about OP-TEE. + + + Entry: text: An entry which contains text ----------------------------------------- diff --git a/tools/binman/etype/tee_os.py b/tools/binman/etype/tee_os.py new file mode 100644 index 00000000000..6ce4b672de4 --- /dev/null +++ b/tools/binman/etype/tee_os.py @@ -0,0 +1,22 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright (C) 2022 Texas Instruments Incorporated - https://www.ti.com/ +# +# Entry-type module for OP-TEE Trusted OS firmware blob +# + +from binman.etype.blob_named_by_arg import Entry_blob_named_by_arg + +class Entry_tee_os(Entry_blob_named_by_arg): + """Entry containing an OP-TEE Trusted OS (TEE) blob + + Properties / Entry arguments: + - tee-os-path: Filename of file to read into entry. This is typically + called tee-pager.bin + + This entry holds the run-time firmware, typically started by U-Boot SPL. + See the U-Boot README for your architecture or board for how to use it. See + https://github.com/OP-TEE/optee_os for more information about OP-TEE. + """ + def __init__(self, section, etype, node): + super().__init__(section, etype, node, 'tee-os') + self.external = True diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 4616a29deb6..6a77f1da1ee 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -84,6 +84,7 @@ FSP_M_DATA = b'fsp_m' FSP_S_DATA = b'fsp_s' FSP_T_DATA = b'fsp_t' ATF_BL31_DATA = b'bl31' +TEE_OS_DATA = b'this is some tee OS data' ATF_BL2U_DATA = b'bl2u' OPENSBI_DATA = b'opensbi' SCP_DATA = b'scp' @@ -188,6 +189,7 @@ class TestFunctional(unittest.TestCase): TestFunctional._MakeInputFile('compress', COMPRESS_DATA) TestFunctional._MakeInputFile('compress_big', COMPRESS_DATA_BIG) TestFunctional._MakeInputFile('bl31.bin', ATF_BL31_DATA) + TestFunctional._MakeInputFile('tee-pager.bin', TEE_OS_DATA) TestFunctional._MakeInputFile('bl2u.bin', ATF_BL2U_DATA) TestFunctional._MakeInputFile('fw_dynamic.bin', OPENSBI_DATA) TestFunctional._MakeInputFile('scp.bin', SCP_DATA) @@ -5296,5 +5298,11 @@ fdt fdtmap Extract the devicetree blob from the fdtmap fnode = mkimage_dtb.GetNode('/images/fdt-1/hash') self.assertIn('value', fnode.props) + def testPackTeeOs(self): + """Test that an image with an TEE binary can be created""" + data = self._DoReadFile('222_tee_os.dts') + self.assertEqual(TEE_OS_DATA, data[:len(TEE_OS_DATA)]) + + if __name__ == "__main__": unittest.main() diff --git a/tools/binman/missing-blob-help b/tools/binman/missing-blob-help index 551ca87f6cb..c61ca02a35e 100644 --- a/tools/binman/missing-blob-help +++ b/tools/binman/missing-blob-help @@ -33,3 +33,7 @@ k3-rti-wdt-firmware: If CONFIG_WDT_K3_RTI_LOAD_FW is enabled, a firmware image is needed for the R5F core(s) to trigger the system reset. One possible source is https://github.com/siemens/k3-rti-wdt. + +tee-os: +See the documentation for your board. You may need to build Open Portable +Trusted Execution Environment (OP-TEE) with TEE=/path/to/tee.bin diff --git a/tools/binman/test/222_tee_os.dts b/tools/binman/test/222_tee_os.dts new file mode 100644 index 00000000000..68854972942 --- /dev/null +++ b/tools/binman/test/222_tee_os.dts @@ -0,0 +1,14 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + tee-os { + filename = "tee-pager.bin"; + }; + }; +}; -- cgit v1.3.1 From 523cde0637f264380eeaaf0cfa82fdbbd99a261f Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 8 Feb 2022 11:49:57 -0700 Subject: binman: Add to the TODO Add some ideas that have come to mind recently. Signed-off-by: Simon Glass --- tools/binman/binman.rst | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tools/binman/binman.rst b/tools/binman/binman.rst index 9e39e678a39..d6b95de1f5d 100644 --- a/tools/binman/binman.rst +++ b/tools/binman/binman.rst @@ -1402,6 +1402,14 @@ Some ideas: - Detect invalid properties in nodes - Sort the fdtmap by offset - Output temporary files to a different directory +- Rationalise the fdt, fdt_util and pylibfdt modules which currently have some + overlapping and confusing functionality +- Update the fdt library to use a better format for Prop.value (the current one + is useful for dtoc but not much else) +- Figure out how to make Fdt support changing the node order, so that + Node.AddSubnode() can support adding a node before another, existing node. + Perhaps it should completely regenerate the flat tree? + -- Simon Glass -- cgit v1.3.1 From 5c044ff52362f379b5a9296e724df9546ae98b34 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 8 Feb 2022 11:49:58 -0700 Subject: binman: Support a list of strings with the mkimage etype At present the 'args' property of the mkimage entry type is a string. This makes it difficult to include CONFIG options in that property. In particular, this does not work: args = "-n CONFIG_SYS_SOC -E" since the preprocessor does not operate within strings, nor does this: args = "-n" CONFIG_SYS_SOC" "-E" since the device tree compiler does not understand string concatenation. With this new feature, we can do: args = "-n", CONFIG_SYS_SOC, "-E"; Signed-off-by: Simon Glass --- tools/binman/elf_test.py | 6 +++--- tools/binman/entries.rst | 11 +++++++++++ tools/binman/etype/mkimage.py | 13 ++++++++++++- 3 files changed, 26 insertions(+), 4 deletions(-) diff --git a/tools/binman/elf_test.py b/tools/binman/elf_test.py index f92352d54f6..a67915bda63 100644 --- a/tools/binman/elf_test.py +++ b/tools/binman/elf_test.py @@ -263,7 +263,7 @@ class TestElf(unittest.TestCase): if not elf.ELF_TOOLS: self.skipTest('Python elftools not available') fname = self.ElfTestFile('embed_data') - segments, entry = elf.read_segments(tools.ReadFile(fname)) + segments, entry = elf.read_segments(tools.read_file(fname)) def test_read_segments_fail(self): """Test for read_segments() without elftools""" @@ -272,7 +272,7 @@ class TestElf(unittest.TestCase): elf.ELF_TOOLS = False fname = self.ElfTestFile('embed_data') with self.assertRaises(ValueError) as e: - elf.read_segments(tools.ReadFile(fname)) + elf.read_segments(tools.read_file(fname)) self.assertIn('Python elftools package is not available', str(e.exception)) finally: @@ -282,7 +282,7 @@ class TestElf(unittest.TestCase): """Test for read_segments() with an invalid ELF file""" fname = self.ElfTestFile('embed_data') with self.assertRaises(ValueError) as e: - elf.read_segments(tools.GetBytes(100, 100)) + elf.read_segments(tools.get_bytes(100, 100)) self.assertIn('Magic number does not match', str(e.exception)) diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst index 88230a69d4e..c4aa5fffe8e 100644 --- a/tools/binman/entries.rst +++ b/tools/binman/entries.rst @@ -931,6 +931,17 @@ This calls mkimage to create an imximage with u-boot-spl.bin as the input file. The output from mkimage then becomes part of the image produced by binman. +To use CONFIG options in the arguments, use a string list instead, as in +this example which also produces four arguments:: + + mkimage { + args = "-n", CONFIG_SYS_SOC, "-T imximage"; + + u-boot-spl { + }; + }; + + Entry: opensbi: RISC-V OpenSBI fw_dynamic blob diff --git a/tools/binman/etype/mkimage.py b/tools/binman/etype/mkimage.py index baa16f36f34..ba4fb25de88 100644 --- a/tools/binman/etype/mkimage.py +++ b/tools/binman/etype/mkimage.py @@ -31,10 +31,21 @@ class Entry_mkimage(Entry): This calls mkimage to create an imximage with u-boot-spl.bin as the input file. The output from mkimage then becomes part of the image produced by binman. + + To use CONFIG options in the arguments, use a string list instead, as in + this example which also produces four arguments:: + + mkimage { + args = "-n", CONFIG_SYS_SOC, "-T imximage"; + + u-boot-spl { + }; + }; + """ def __init__(self, section, etype, node): super().__init__(section, etype, node) - self._args = fdt_util.GetString(self._node, 'args').split(' ') + self._args = fdt_util.GetArgs(self._node, 'args') self._mkimage_entries = OrderedDict() self.align_default = None self.ReadEntries() -- cgit v1.3.1 From 56385c585fa5c51aacfd7ffa8021ea705c715b6f Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 8 Feb 2022 11:49:59 -0700 Subject: binman: Add a ELF test file with disjoint text sections Add a file that has two text sections at different addresses, so we can test this behaviour in binman, once added. Signed-off-by: Simon Glass --- tools/binman/test/Makefile | 6 +++++- tools/binman/test/elf_sections.c | 20 ++++++++++++++++++++ tools/binman/test/elf_sections.lds | 31 +++++++++++++++++++++++++++++++ 3 files changed, 56 insertions(+), 1 deletion(-) create mode 100644 tools/binman/test/elf_sections.c create mode 100644 tools/binman/test/elf_sections.lds diff --git a/tools/binman/test/Makefile b/tools/binman/test/Makefile index 387ba163353..57057e2d588 100644 --- a/tools/binman/test/Makefile +++ b/tools/binman/test/Makefile @@ -29,11 +29,12 @@ LDS_BINMAN := -T $(SRC)u_boot_binman_syms.lds LDS_BINMAN_BAD := -T $(SRC)u_boot_binman_syms_bad.lds LDS_BINMAN_X86 := -T $(SRC)u_boot_binman_syms_x86.lds LDS_BINMAN_EMBED := -T $(SRC)u_boot_binman_embed.lds +LDS_EFL_SECTIONS := -T $(SRC)elf_sections.lds TARGETS = u_boot_ucode_ptr u_boot_no_ucode_ptr bss_data \ u_boot_binman_syms u_boot_binman_syms.bin u_boot_binman_syms_bad \ u_boot_binman_syms_size u_boot_binman_syms_x86 embed_data \ - u_boot_binman_embed u_boot_binman_embed_sm + u_boot_binman_embed u_boot_binman_embed_sm elf_sections all: $(TARGETS) @@ -70,6 +71,9 @@ u_boot_binman_embed: u_boot_binman_embed.c u_boot_binman_embed_sm: CFLAGS += $(LDS_BINMAN_EMBED) u_boot_binman_embed_sm: u_boot_binman_embed_sm.c +elf_sections: CFLAGS += $(LDS_EFL_SECTIONS) +elf_sections: elf_sections.c + clean: rm -f $(TARGETS) diff --git a/tools/binman/test/elf_sections.c b/tools/binman/test/elf_sections.c new file mode 100644 index 00000000000..9bcce9af021 --- /dev/null +++ b/tools/binman/test/elf_sections.c @@ -0,0 +1,20 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright 2022 Google LLC + * + * Program containing two text sections + */ + +int __attribute__((section(".sram_data"))) data[29]; + +int __attribute__((section(".sram_code"))) calculate(int x) +{ + data[0] = x; + + return x * x; +} + +int main(void) +{ + return calculate(123); +} diff --git a/tools/binman/test/elf_sections.lds b/tools/binman/test/elf_sections.lds new file mode 100644 index 00000000000..7b6e932592f --- /dev/null +++ b/tools/binman/test/elf_sections.lds @@ -0,0 +1,31 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * Copyright (c) 2016 Google, Inc + */ + +OUTPUT_FORMAT("elf32-i386", "elf32-i386", "elf32-i386") +OUTPUT_ARCH(i386) +ENTRY(_start) + +SECTIONS +{ + . = 0x00000010; + _start = .; + + . = ALIGN(4); + .text : + { + *(.text*) + } + + . = 0x00001000; + .sram : + { + *(.sram*) + } + + /DISCARD/ : { + *(.comment) + *(.dyn*) + } +} -- cgit v1.3.1 From 81b71c32005475395b5b4d0945de4e6a9136d191 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 8 Feb 2022 11:50:00 -0700 Subject: binman: Move entry-data collection into a Entry method Collecting the data from a list of entries and putting it in a file is a useful operation that will be needed by other entry types. Put this into a method in the Entry class. Add some documentation about how to collect data for an entry type. Signed-off-by: Simon Glass --- tools/binman/binman.rst | 86 +++++++++++++++++++++++++++++++++++++++++++ tools/binman/entry.py | 28 ++++++++++++++ tools/binman/etype/mkimage.py | 14 ++----- 3 files changed, 118 insertions(+), 10 deletions(-) diff --git a/tools/binman/binman.rst b/tools/binman/binman.rst index d6b95de1f5d..771645380ed 100644 --- a/tools/binman/binman.rst +++ b/tools/binman/binman.rst @@ -1358,6 +1358,92 @@ development, since dealing with exceptions and problems in threads is more difficult. This avoids any use of ThreadPoolExecutor. +Collecting data for an entry type +--------------------------------- + +Some entry types deal with data obtained from others. For example, +`Entry_mkimage` calls the `mkimage` tool with data from its subnodes:: + + mkimage { + args = "-n test -T script"; + + u-boot-spl { + }; + + u-boot { + }; + }; + +This shows mkimage being passed a file consisting of SPL and U-Boot proper. It +is create by calling `Entry.collect_contents_to_file()`. Note that in this case, +the data is passed to mkimage for processing but does not appear separately in +the image. It may not appear at all, depending on what mkimage does. The +contents of the `mkimage` entry are entirely dependent on the processing done +by the entry, with the provided subnodes (`u-boot-spl` and `u-boot`) simply +providing the input data for that processing. + +Note that `Entry.collect_contents_to_file()` simply concatenates the data from +the different entries together, with no control over alignment, etc. Another +approach is to subclass `Entry_section` so that those features become available, +such as `size` and `pad-byte`. Then the contents of the entry can be obtained by +calling `BuildSectionData()`. + +There are other ways to obtain data also, depending on the situation. If the +entry type is simply signing data which exists elsewhere in the image, then +you can use `Entry_collection` as a base class. It lets you use a property +called `content` which lists the entries containing data to be processed. This +is used by `Entry_vblock`, for example:: + + u_boot: u-boot { + }; + vblock { + content = <&u_boot &dtb>; + keyblock = "firmware.keyblock"; + signprivate = "firmware_data_key.vbprivk"; + version = <1>; + kernelkey = "kernel_subkey.vbpubk"; + preamble-flags = <1>; + }; + + dtb: u-boot-dtb { + }; + +which shows an image containing `u-boot` and `u-boot-dtb`, with the `vblock` +image collecting their contents to produce input for its signing process, +without affecting those entries, which still appear in the final image +untouched. + +Another example is where an entry type needs several independent pieces of input +to function. For example, `Entry_fip` allows a number of different binary blobs +to be placed in their own individual places in a custom data structure in the +output image. To make that work you can add subnodes for each of them and call +`Entry.Create()` on each subnode, as `Entry_fip` does. Then the data for each +blob can come from any suitable place, such as an `Entry_u_boot` or an +`Entry_blob` or anything else:: + + atf-fip { + fip-hdr-flags = /bits/ 64 <0x123>; + soc-fw { + fip-flags = /bits/ 64 <0x123456789abcdef>; + filename = "bl31.bin"; + }; + + u-boot { + fip-uuid = [fc 65 13 92 4a 5b 11 ec + 94 35 ff 2d 1c fc 79 9c]; + }; + }; + +The `soc-fw` node is a `blob-ext` (i.e. it reads in a named binary file) whereas +`u-boot` is a normal entry type. This works because `Entry_fip` selects the +`blob-ext` entry type if the node name (here `soc-fw`) is recognised as being +a known blob type. + +When adding new entry types you are encouraged to use subnodes to provide the +data for processing, unless the `content` approach is more suitable. Ad-hoc +properties and other methods of obtaining data are discouraged, since it adds to +confusion for users. + History / Credits ----------------- diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 631215dfc88..bf68a85b245 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -1123,3 +1123,31 @@ features to produce new behaviours. update_hash: True if hash should be updated, False if not """ self.update_hash = update_hash + + def collect_contents_to_file(self, entries, prefix): + """Put the contents of a list of entries into a file + + Args: + entries (list of Entry): Entries to collect + prefix (str): Filename prefix of file to write to + + If any entry does not have contents yet, this function returns False + for the data. + + Returns: + Tuple: + bytes: Concatenated data from all the entries (or False) + str: Filename of file written (or False if no data) + str: Unique portion of filename (or False if no data) + """ + data = b'' + for entry in entries: + # First get the input data and put it in a file. If not available, + # try later. + if not entry.ObtainContents(): + return False, False, False + data += entry.GetData() + uniq = self.GetUniqueName() + fname = tools.get_output_filename(f'{prefix}.{uniq}') + tools.write_file(fname, data) + return data, fname, uniq diff --git a/tools/binman/etype/mkimage.py b/tools/binman/etype/mkimage.py index ba4fb25de88..8e74ebb9c06 100644 --- a/tools/binman/etype/mkimage.py +++ b/tools/binman/etype/mkimage.py @@ -51,16 +51,10 @@ class Entry_mkimage(Entry): self.ReadEntries() def ObtainContents(self): - data = b'' - for entry in self._mkimage_entries.values(): - # First get the input data and put it in a file. If not available, - # try later. - if not entry.ObtainContents(): - return False - data += entry.GetData() - uniq = self.GetUniqueName() - input_fname = tools.get_output_filename('mkimage.%s' % uniq) - tools.write_file(input_fname, data) + data, input_fname, uniq = self.collect_contents_to_file( + self._mkimage_entries.values(), 'mkimage') + if data is False: + return False output_fname = tools.get_output_filename('mkimage-out.%s' % uniq) if self.mkimage.run_cmd('-d', input_fname, *self._args, output_fname) is not None: -- cgit v1.3.1 From dbe17c0008cfdbcde4ffa3a9c411e25a01553847 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 8 Feb 2022 11:50:01 -0700 Subject: binman: fit: Refactor to reduce function size Split subnode and property processing into separate functions to make the _AddNode() function a little smaller. Tweak a few comments. This does not change any functionality. Signed-off-by: Simon Glass --- tools/binman/etype/fit.py | 114 +++++++++++++++++++++++++++++----------------- 1 file changed, 71 insertions(+), 43 deletions(-) diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py index 1e957023f35..2657e6c9d2a 100644 --- a/tools/binman/etype/fit.py +++ b/tools/binman/etype/fit.py @@ -142,12 +142,80 @@ class Entry_fit(Entry_section): super().ReadNode() def ReadEntries(self): + def _process_prop(pname, prop): + """Process special properties + + Handles properties with generated values. At present the only + supported property is 'default', i.e. the default device tree in + the configurations node. + + Args: + pname (str): Name of property + prop (Prop): Property to process + """ + if pname == 'default': + val = prop.value + # Handle the 'default' property + if val.startswith('@'): + if not self._fdts: + return + if not self._fit_default_dt: + self.Raise("Generated 'default' node requires default-dt entry argument") + if self._fit_default_dt not in self._fdts: + self.Raise("default-dt entry argument '%s' not found in fdt list: %s" % + (self._fit_default_dt, + ', '.join(self._fdts))) + seq = self._fdts.index(self._fit_default_dt) + val = val[1:].replace('DEFAULT-SEQ', str(seq + 1)) + fsw.property_string(pname, val) + return + fsw.property(pname, prop.bytes) + + def _generate_node(subnode, depth, in_images): + """Generate nodes from a template + + This creates one node for each member of self._fdts using the + provided template. If a property value contains 'NAME' it is + replaced with the filename of the FDT. If a property value contains + SEQ it is replaced with the node sequence number, where 1 is the + first. + + Args: + subnode (None): Generator node to process + depth: Current node depth (0 is the base 'fit' node) + in_images: True if this is inside the 'images' node, so that + 'data' properties should be generated + """ + if self._fdts: + # Generate nodes for each FDT + for seq, fdt_fname in enumerate(self._fdts): + node_name = subnode.name[1:].replace('SEQ', str(seq + 1)) + fname = tools.get_input_filename(fdt_fname + '.dtb') + with fsw.add_node(node_name): + for pname, prop in subnode.props.items(): + val = prop.bytes.replace( + b'NAME', tools.to_bytes(fdt_fname)) + val = val.replace( + b'SEQ', tools.to_bytes(str(seq + 1))) + fsw.property(pname, val) + + # Add data for 'images' nodes (but not 'config') + if depth == 1 and in_images: + fsw.property('data', tools.read_file(fname)) + else: + if self._fdts is None: + if self._fit_list_prop: + self.Raise("Generator node requires '%s' entry argument" % + self._fit_list_prop.value) + else: + self.Raise("Generator node requires 'fit,fdt-list' property") + def _AddNode(base_node, depth, node): """Add a node to the FIT Args: base_node: Base Node of the FIT (with 'description' property) - depth: Current node depth (0 is the base node) + depth: Current node depth (0 is the base 'fit' node) node: Current node to process There are two cases to deal with: @@ -157,23 +225,7 @@ class Entry_fit(Entry_section): """ for pname, prop in node.props.items(): if not pname.startswith('fit,'): - if pname == 'default': - val = prop.value - # Handle the 'default' property - if val.startswith('@'): - if not self._fdts: - continue - if not self._fit_default_dt: - self.Raise("Generated 'default' node requires default-dt entry argument") - if self._fit_default_dt not in self._fdts: - self.Raise("default-dt entry argument '%s' not found in fdt list: %s" % - (self._fit_default_dt, - ', '.join(self._fdts))) - seq = self._fdts.index(self._fit_default_dt) - val = val[1:].replace('DEFAULT-SEQ', str(seq + 1)) - fsw.property_string(pname, val) - continue - fsw.property(pname, prop.bytes) + _process_prop(pname, prop) rel_path = node.path[len(base_node.path):] in_images = rel_path.startswith('/images') @@ -198,31 +250,7 @@ class Entry_fit(Entry_section): # fsw.add_node() or _AddNode() for it. pass elif self.GetImage().generate and subnode.name.startswith('@'): - if self._fdts: - # Generate notes for each FDT - for seq, fdt_fname in enumerate(self._fdts): - node_name = subnode.name[1:].replace('SEQ', - str(seq + 1)) - fname = tools.get_input_filename(fdt_fname + '.dtb') - with fsw.add_node(node_name): - for pname, prop in subnode.props.items(): - val = prop.bytes.replace( - b'NAME', tools.to_bytes(fdt_fname)) - val = val.replace( - b'SEQ', tools.to_bytes(str(seq + 1))) - fsw.property(pname, val) - - # Add data for 'fdt' nodes (but not 'config') - if depth == 1 and in_images: - fsw.property('data', - tools.read_file(fname)) - else: - if self._fdts is None: - if self._fit_list_prop: - self.Raise("Generator node requires '%s' entry argument" % - self._fit_list_prop.value) - else: - self.Raise("Generator node requires 'fit,fdt-list' property") + _generate_node(subnode, depth, in_images) else: with fsw.add_node(subnode.name): _AddNode(base_node, depth + 1, subnode) -- cgit v1.3.1 From 98e0de3fb788cfd9b895840982a15dfcbe460920 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 8 Feb 2022 11:50:02 -0700 Subject: binman: Tidy up the docs a little with fit Add a few quotes and clarify the data property. Signed-off-by: Simon Glass --- tools/binman/entries.rst | 15 ++++++++------- tools/binman/etype/fit.py | 15 ++++++++------- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst index c4aa5fffe8e..053e1d78388 100644 --- a/tools/binman/entries.rst +++ b/tools/binman/entries.rst @@ -554,10 +554,10 @@ For example, this creates an image containing a FIT with U-Boot SPL:: }; U-Boot supports creating fdt and config nodes automatically. To do this, -pass an of-list property (e.g. -a of-list=file1 file2). This tells binman -that you want to generates nodes for two files: file1.dtb and file2.dtb -The fit,fdt-list property (see above) indicates that of-list should be used. -If the property is missing you will get an error. +pass an `of-list` property (e.g. `-a of-list=file1 file2`). This tells +binman that you want to generates nodes for two files: `file1.dtb` and +`file2.dtb`. The `fit,fdt-list` property (see above) indicates that +`of-list` should be used. If the property is missing you will get an error. Then add a 'generator node', a node with a name starting with '@':: @@ -569,10 +569,11 @@ Then add a 'generator node', a node with a name starting with '@':: }; }; -This tells binman to create nodes fdt-1 and fdt-2 for each of your two +This tells binman to create nodes `fdt-1` and `fdt-2` for each of your two files. All the properties you specify will be included in the node. This node acts like a template to generate the nodes. The generator node itself does not appear in the output - it is replaced with what binman generates. +A 'data' property is created with the contents of the FDT file. You can create config nodes in a similar way:: @@ -586,8 +587,8 @@ You can create config nodes in a similar way:: }; }; -This tells binman to create nodes config-1 and config-2, i.e. a config for -each of your two files. +This tells binman to create nodes `config-1` and `config-2`, i.e. a config +for each of your two files. Available substitutions for '@' nodes are: diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py index 2657e6c9d2a..d3e61c4ad22 100644 --- a/tools/binman/etype/fit.py +++ b/tools/binman/etype/fit.py @@ -48,10 +48,10 @@ class Entry_fit(Entry_section): }; U-Boot supports creating fdt and config nodes automatically. To do this, - pass an of-list property (e.g. -a of-list=file1 file2). This tells binman - that you want to generates nodes for two files: file1.dtb and file2.dtb - The fit,fdt-list property (see above) indicates that of-list should be used. - If the property is missing you will get an error. + pass an `of-list` property (e.g. `-a of-list=file1 file2`). This tells + binman that you want to generates nodes for two files: `file1.dtb` and + `file2.dtb`. The `fit,fdt-list` property (see above) indicates that + `of-list` should be used. If the property is missing you will get an error. Then add a 'generator node', a node with a name starting with '@':: @@ -63,10 +63,11 @@ class Entry_fit(Entry_section): }; }; - This tells binman to create nodes fdt-1 and fdt-2 for each of your two + This tells binman to create nodes `fdt-1` and `fdt-2` for each of your two files. All the properties you specify will be included in the node. This node acts like a template to generate the nodes. The generator node itself does not appear in the output - it is replaced with what binman generates. + A 'data' property is created with the contents of the FDT file. You can create config nodes in a similar way:: @@ -80,8 +81,8 @@ class Entry_fit(Entry_section): }; }; - This tells binman to create nodes config-1 and config-2, i.e. a config for - each of your two files. + This tells binman to create nodes `config-1` and `config-2`, i.e. a config + for each of your two files. Available substitutions for '@' nodes are: -- cgit v1.3.1 From 6a0b5f8b9caa0c8e3a39ed8c979cfdbbf66b1841 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 8 Feb 2022 11:50:03 -0700 Subject: binman: Allow different operations in FIT generator nodes At present we only support expanding out FDT nodes. Make the operation into an @operation property, so that others can be supported. Re-arrange and tidy up the documentation so that it has separate headings for each topic. Signed-off-by: Simon Glass --- tools/binman/entries.rst | 85 ++++++++++++++------ tools/binman/etype/fit.py | 137 ++++++++++++++++++++++++++------- tools/binman/ftest.py | 18 +++++ tools/binman/test/223_fit_fdt_oper.dts | 56 ++++++++++++++ tools/binman/test/224_fit_bad_oper.dts | 27 +++++++ 5 files changed, 274 insertions(+), 49 deletions(-) create mode 100644 tools/binman/test/223_fit_fdt_oper.dts create mode 100644 tools/binman/test/224_fit_bad_oper.dts diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst index 053e1d78388..484cde5c800 100644 --- a/tools/binman/entries.rst +++ b/tools/binman/entries.rst @@ -553,6 +553,68 @@ For example, this creates an image containing a FIT with U-Boot SPL:: }; }; +More complex setups can be created, with generated nodes, as described +below. + +Properties (in the 'fit' node itself) +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Special properties have a `fit,` prefix, indicating that they should be +processed but not included in the final FIT. + +The top-level 'fit' node supports the following special properties: + + fit,external-offset + Indicates that the contents of the FIT are external and provides the + external offset. This is passed to mkimage via the -E and -p flags. + + fit,fdt-list + Indicates the entry argument which provides the list of device tree + files for the gen-fdt-nodes operation (as below). This is often + `of-list` meaning that `-a of-list="dtb1 dtb2..."` should be passed + to binman. + +Substitutions +~~~~~~~~~~~~~ + +Node names and property values support a basic string-substitution feature. +Available substitutions for '@' nodes (and property values) are: + +SEQ: + Sequence number of the generated fdt (1, 2, ...) +NAME + Name of the dtb as provided (i.e. without adding '.dtb') + +The `default` property, if present, will be automatically set to the name +if of configuration whose devicetree matches the `default-dt` entry +argument, e.g. with `-a default-dt=sun50i-a64-pine64-lts`. + +Available substitutions for property values in these nodes are: + +DEFAULT-SEQ: + Sequence number of the default fdt, as provided by the 'default-dt' + entry argument + +Available operations +~~~~~~~~~~~~~~~~~~~~ + +You can add an operation to an '@' node to indicate which operation is +required:: + + @fdt-SEQ { + fit,operation = "gen-fdt-nodes"; + ... + }; + +Available operations are: + +gen-fdt-nodes + Generate FDT nodes as above. This is the default if there is no + `fit,operation` property. + +Generating nodes from an FDT list (gen-fdt-nodes) +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + U-Boot supports creating fdt and config nodes automatically. To do this, pass an `of-list` property (e.g. `-a of-list=file1 file2`). This tells binman that you want to generates nodes for two files: `file1.dtb` and @@ -590,32 +652,9 @@ You can create config nodes in a similar way:: This tells binman to create nodes `config-1` and `config-2`, i.e. a config for each of your two files. -Available substitutions for '@' nodes are: - -SEQ: - Sequence number of the generated fdt (1, 2, ...) -NAME - Name of the dtb as provided (i.e. without adding '.dtb') - Note that if no devicetree files are provided (with '-a of-list' as above) then no nodes will be generated. -The 'default' property, if present, will be automatically set to the name -if of configuration whose devicetree matches the 'default-dt' entry -argument, e.g. with '-a default-dt=sun50i-a64-pine64-lts'. - -Available substitutions for '@' property values are - -DEFAULT-SEQ: - Sequence number of the default fdt,as provided by the 'default-dt' entry - argument - -Properties (in the 'fit' node itself): - fit,external-offset: Indicates that the contents of the FIT are external - and provides the external offset. This is passsed to mkimage via - the -E and -p flags. - - Entry: fmap: An entry which contains an Fmap section diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py index d3e61c4ad22..2d4c5f65451 100644 --- a/tools/binman/etype/fit.py +++ b/tools/binman/etype/fit.py @@ -14,7 +14,14 @@ from dtoc import fdt_util from dtoc.fdt import Fdt from patman import tools +# Supported operations, with the fit,operation property +OP_GEN_FDT_NODES = range(1) +OPERATIONS = { + 'gen-fdt-nodes': OP_GEN_FDT_NODES, + } + class Entry_fit(Entry_section): + """Flat Image Tree (FIT) This calls mkimage to create a FIT (U-Boot Flat Image Tree) based on the @@ -47,6 +54,68 @@ class Entry_fit(Entry_section): }; }; + More complex setups can be created, with generated nodes, as described + below. + + Properties (in the 'fit' node itself) + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + + Special properties have a `fit,` prefix, indicating that they should be + processed but not included in the final FIT. + + The top-level 'fit' node supports the following special properties: + + fit,external-offset + Indicates that the contents of the FIT are external and provides the + external offset. This is passed to mkimage via the -E and -p flags. + + fit,fdt-list + Indicates the entry argument which provides the list of device tree + files for the gen-fdt-nodes operation (as below). This is often + `of-list` meaning that `-a of-list="dtb1 dtb2..."` should be passed + to binman. + + Substitutions + ~~~~~~~~~~~~~ + + Node names and property values support a basic string-substitution feature. + Available substitutions for '@' nodes (and property values) are: + + SEQ: + Sequence number of the generated fdt (1, 2, ...) + NAME + Name of the dtb as provided (i.e. without adding '.dtb') + + The `default` property, if present, will be automatically set to the name + if of configuration whose devicetree matches the `default-dt` entry + argument, e.g. with `-a default-dt=sun50i-a64-pine64-lts`. + + Available substitutions for property values in these nodes are: + + DEFAULT-SEQ: + Sequence number of the default fdt, as provided by the 'default-dt' + entry argument + + Available operations + ~~~~~~~~~~~~~~~~~~~~ + + You can add an operation to an '@' node to indicate which operation is + required:: + + @fdt-SEQ { + fit,operation = "gen-fdt-nodes"; + ... + }; + + Available operations are: + + gen-fdt-nodes + Generate FDT nodes as above. This is the default if there is no + `fit,operation` property. + + Generating nodes from an FDT list (gen-fdt-nodes) + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + U-Boot supports creating fdt and config nodes automatically. To do this, pass an `of-list` property (e.g. `-a of-list=file1 file2`). This tells binman that you want to generates nodes for two files: `file1.dtb` and @@ -84,31 +153,8 @@ class Entry_fit(Entry_section): This tells binman to create nodes `config-1` and `config-2`, i.e. a config for each of your two files. - Available substitutions for '@' nodes are: - - SEQ: - Sequence number of the generated fdt (1, 2, ...) - NAME - Name of the dtb as provided (i.e. without adding '.dtb') - Note that if no devicetree files are provided (with '-a of-list' as above) then no nodes will be generated. - - The 'default' property, if present, will be automatically set to the name - if of configuration whose devicetree matches the 'default-dt' entry - argument, e.g. with '-a default-dt=sun50i-a64-pine64-lts'. - - Available substitutions for '@' property values are - - DEFAULT-SEQ: - Sequence number of the default fdt,as provided by the 'default-dt' entry - argument - - Properties (in the 'fit' node itself): - fit,external-offset: Indicates that the contents of the FIT are external - and provides the external offset. This is passsed to mkimage via - the -E and -p flags. - """ def __init__(self, section, etype, node): """ @@ -142,6 +188,26 @@ class Entry_fit(Entry_section): self.ReadEntries() super().ReadNode() + def _get_operation(self, subnode): + """Get the operation referenced by a subnode + + Args: + subnode (Node): Subnode (of the FIT) to check + + Returns: + int: Operation to perform + + Raises: + ValueError: Invalid operation name + """ + oper_name = subnode.props.get('fit,operation') + if not oper_name: + return OP_GEN_FDT_NODES + oper = OPERATIONS.get(oper_name.value) + if not oper: + self.Raise(f"Unknown operation '{oper_name.value}'") + return oper + def ReadEntries(self): def _process_prop(pname, prop): """Process special properties @@ -172,8 +238,8 @@ class Entry_fit(Entry_section): return fsw.property(pname, prop.bytes) - def _generate_node(subnode, depth, in_images): - """Generate nodes from a template + def _scan_gen_fdt_nodes(subnode, depth, in_images): + """Generate FDT nodes This creates one node for each member of self._fdts using the provided template. If a property value contains 'NAME' it is @@ -211,6 +277,25 @@ class Entry_fit(Entry_section): else: self.Raise("Generator node requires 'fit,fdt-list' property") + def _scan_node(subnode, depth, in_images): + """Generate nodes from a template + + This creates one node for each member of self._fdts using the + provided template. If a property value contains 'NAME' it is + replaced with the filename of the FDT. If a property value contains + SEQ it is replaced with the node sequence number, where 1 is the + first. + + Args: + subnode (None): Generator node to process + depth: Current node depth (0 is the base 'fit' node) + in_images: True if this is inside the 'images' node, so that + 'data' properties should be generated + """ + oper = self._get_operation(subnode) + if oper == OP_GEN_FDT_NODES: + _scan_gen_fdt_nodes(subnode, depth, in_images) + def _AddNode(base_node, depth, node): """Add a node to the FIT @@ -251,7 +336,7 @@ class Entry_fit(Entry_section): # fsw.add_node() or _AddNode() for it. pass elif self.GetImage().generate and subnode.name.startswith('@'): - _generate_node(subnode, depth, in_images) + _scan_node(subnode, depth, in_images) else: with fsw.add_node(subnode.name): _AddNode(base_node, depth + 1, subnode) diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 6a77f1da1ee..8f00db69455 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -5303,6 +5303,24 @@ fdt fdtmap Extract the devicetree blob from the fdtmap data = self._DoReadFile('222_tee_os.dts') self.assertEqual(TEE_OS_DATA, data[:len(TEE_OS_DATA)]) + def testFitFdtOper(self): + """Check handling of a specified FIT operation""" + entry_args = { + 'of-list': 'test-fdt1 test-fdt2', + 'default-dt': 'test-fdt2', + } + self._DoReadFileDtb( + '223_fit_fdt_oper.dts', + entry_args=entry_args, + extra_indirs=[os.path.join(self._indir, TEST_FDT_SUBDIR)])[0] + + def testFitFdtBadOper(self): + """Check handling of an FDT map when the section cannot be found""" + with self.assertRaises(ValueError) as exc: + self._DoReadFileDtb('224_fit_bad_oper.dts') + self.assertIn("Node '/binman/fit': Unknown operation 'unknown'", + str(exc.exception)) + if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/223_fit_fdt_oper.dts b/tools/binman/test/223_fit_fdt_oper.dts new file mode 100644 index 00000000000..e630165acf4 --- /dev/null +++ b/tools/binman/test/223_fit_fdt_oper.dts @@ -0,0 +1,56 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + u-boot { + }; + fit { + description = "test-desc"; + #address-cells = <1>; + fit,fdt-list = "of-list"; + + images { + kernel { + description = "Vanilla Linux kernel"; + type = "kernel"; + arch = "ppc"; + os = "linux"; + compression = "gzip"; + load = <00000000>; + entry = <00000000>; + hash-1 { + algo = "crc32"; + }; + hash-2 { + algo = "sha1"; + }; + u-boot { + }; + }; + @fdt-SEQ { + fit,operation = "gen-fdt-nodes"; + description = "fdt-NAME.dtb"; + type = "flat_dt"; + compression = "none"; + }; + }; + + configurations { + default = "@config-DEFAULT-SEQ"; + @config-SEQ { + description = "conf-NAME.dtb"; + firmware = "uboot"; + loadables = "atf"; + fdt = "fdt-SEQ"; + }; + }; + }; + u-boot-nodtb { + }; + }; +}; diff --git a/tools/binman/test/224_fit_bad_oper.dts b/tools/binman/test/224_fit_bad_oper.dts new file mode 100644 index 00000000000..cee801e2eab --- /dev/null +++ b/tools/binman/test/224_fit_bad_oper.dts @@ -0,0 +1,27 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + fit { + description = "test-desc"; + #address-cells = <1>; + fit,fdt-list = "of-list"; + + images { + @fdt-SEQ { + fit,operation = "unknown"; + description = "fdt-NAME.dtb"; + type = "flat_dt"; + compression = "none"; + }; + }; + }; + fdtmap { + }; + }; +}; -- cgit v1.3.1 From 606a14ba2f0e01cf7aefe4924d17b5dfda27d1de Mon Sep 17 00:00:00 2001 From: Angus Ainslie Date: Thu, 3 Feb 2022 10:08:38 -0800 Subject: phy: phy-uclass: check the parents for phys The port/hub leaf nodes don't contain the phy definitions in some dts files so check the parents. Signed-off-by: Angus Ainslie Reviewed-by: Simon Glass --- drivers/phy/phy-uclass.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/drivers/phy/phy-uclass.c b/drivers/phy/phy-uclass.c index 49e2ec25c28..8b84da3ce0d 100644 --- a/drivers/phy/phy-uclass.c +++ b/drivers/phy/phy-uclass.c @@ -354,23 +354,31 @@ int generic_phy_configure(struct phy *phy, void *params) int generic_phy_get_bulk(struct udevice *dev, struct phy_bulk *bulk) { int i, ret, count; + struct udevice *phydev = dev; bulk->count = 0; /* Return if no phy declared */ - if (!dev_read_prop(dev, "phys", NULL)) - return 0; + if (!dev_read_prop(dev, "phys", NULL)) { + phydev = dev->parent; + if (!dev_read_prop(phydev, "phys", NULL)) { + pr_err("%s : no phys property\n", __func__); + return 0; + } + } - count = dev_count_phandle_with_args(dev, "phys", "#phy-cells", 0); - if (count < 1) + count = dev_count_phandle_with_args(phydev, "phys", "#phy-cells", 0); + if (count < 1) { + pr_err("%s : no phys found %d\n", __func__, count); return count; + } - bulk->phys = devm_kcalloc(dev, count, sizeof(struct phy), GFP_KERNEL); + bulk->phys = devm_kcalloc(phydev, count, sizeof(struct phy), GFP_KERNEL); if (!bulk->phys) return -ENOMEM; for (i = 0; i < count; i++) { - ret = generic_phy_get_by_index(dev, i, &bulk->phys[i]); + ret = generic_phy_get_by_index(phydev, i, &bulk->phys[i]); if (ret) { pr_err("Failed to get PHY%d for %s\n", i, dev->name); return ret; -- cgit v1.3.1 From 70f42e720c90faa2fa27836288559e0d647862b7 Mon Sep 17 00:00:00 2001 From: Philippe Reynes Date: Wed, 9 Feb 2022 18:01:24 +0100 Subject: scripts: dtc: libfdt: fdt_ro.c: always define fdt_check_full On some configs (like stm32mp15_dhcom_basic_defconfig), if configs SPL_LOAD_FIT_FULL and SPL_FIT_FULL_CHECK are enabled. Then the compilatio fails with the following error: arm-linux-gnueabi-ld.bfd: boot/image-fit.o: in function `fit_check_format': /uboot/u-boot-stm/boot/image-fit.c:1641: undefined reference to `fdt_check_full' scripts/Makefile.spl:509: recipe for target 'spl/u-boot-spl' failed This issue happens because the function fdt_check_full is only defined if "!defined(FDT_ASSUME_MASK) || FDT_ASSUME_MASK != 0xff". But this function may be called even if this condition are not verified. To avoid this issue, the function fdt_check_full is always defined. Signed-off-by: Philippe Reynes Reviewed-by: Simon Glass --- scripts/dtc/libfdt/fdt_ro.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/scripts/dtc/libfdt/fdt_ro.c b/scripts/dtc/libfdt/fdt_ro.c index efe7efe9211..63eaf57f43a 100644 --- a/scripts/dtc/libfdt/fdt_ro.c +++ b/scripts/dtc/libfdt/fdt_ro.c @@ -937,4 +937,10 @@ int fdt_check_full(const void *fdt, size_t bufsize) } } } -#endif +#else +int fdt_check_full(const void __always_unused *fdt, + size_t __always_unused bufsize) +{ + return 0; +} +#endif /* #if !defined(FDT_ASSUME_MASK) || FDT_ASSUME_MASK != 0xff */ -- cgit v1.3.1