In some cases we need to read symbols from U-Boot. At present we have a a few cases which does this via 'nm' and 'grep'.

It is better to use objdump since that tells us the size of the symbols and also whether it is weak or not.

Add a new module which reads ELF information from files. Update existing uses of 'nm' to use this module.

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

tools/binman/binman.py | 3 +- tools/binman/elf.py | 77 +++++++++++++++++++++++++++++ tools/binman/elf_test.py | 32 ++++++++++++ tools/binman/etype/u_boot_spl_bss_pad.py | 7 +-- tools/binman/etype/u_boot_with_ucode_ptr.py | 9 ++-- tools/binman/ftest.py | 7 +++ tools/binman/test/bss_data.c | 2 +- tools/binman/test/u_boot_ucode_ptr.c | 2 +- 8 files changed, 128 insertions(+), 11 deletions(-) create mode 100644 tools/binman/elf.py create mode 100644 tools/binman/elf_test.py

diff --git a/tools/binman/binman.py b/tools/binman/binman.py index 3ccf25f1f88..81a613ddc40 100755 --- a/tools/binman/binman.py +++ b/tools/binman/binman.py @@ -33,6 +33,7 @@ import control

def RunTests(): """Run the functional tests and any embedded doctests""" + import elf_test import entry_test import fdt_test import ftest @@ -50,7 +51,7 @@ def RunTests(): # 'entry' module. suite = unittest.TestLoader().loadTestsFromTestCase(entry_test.TestEntry) suite.run(result) - for module in (ftest.TestFunctional, fdt_test.TestFdt): + for module in (ftest.TestFunctional, fdt_test.TestFdt, elf_test.TestElf): suite = unittest.TestLoader().loadTestsFromTestCase(module) suite.run(result)

diff --git a/tools/binman/elf.py b/tools/binman/elf.py new file mode 100644 index 00000000000..97208b17950 --- /dev/null +++ b/tools/binman/elf.py @@ -0,0 +1,77 @@ +# Copyright (c) 2016 Google, Inc +# Written by Simon Glass sjg@chromium.org +# +# SPDX-License-Identifier: GPL-2.0+ +# +# Handle various things related to ELF images +# + +from collections import namedtuple, OrderedDict +import command +import os +import re +import struct + +import tools + +Symbol = namedtuple('Symbol', ['section', 'address', 'size', 'weak']) + +# Used for tests which don't have an ELF file to read +ignore_missing_files = False + + +def GetSymbols(fname, patterns): + """Get the symbols from an ELF file + + Args: + fname: Filename of the ELF file to read + patterns: List of regex patterns to search for, each a string + + Returns: + None, if the file does not exist, or Dict: + key: Name of symbol + value: Hex value of symbol + """ + stdout = command.Output('objdump', '-t', fname, raise_on_error=False) + lines = stdout.splitlines() + if patterns: + re_syms = re.compile('|'.join(patterns)) + else: + re_syms = None + syms = {} + syms_started = False + for line in lines: + if not line or not syms_started: + if 'SYMBOL TABLE' in line: + syms_started = True + line = None # Otherwise code coverage complains about 'continue' + continue + if re_syms and not re_syms.search(line): + continue + + space_pos = line.find(' ') + value, rest = line[:space_pos], line[space_pos + 1:] + flags = rest[:7] + parts = rest[7:].split() + section, size = parts[:2] + if len(parts) > 2: + name = parts[2] + syms[name] = Symbol(section, int(value, 16), int(size,16), + flags[1] == 'w') + return syms + +def GetSymbolAddress(fname, sym_name): + """Get a value of a symbol from an ELF file + + Args: + fname: Filename of the ELF file to read + patterns: List of regex patterns to search for, each a string + + Returns: + Symbol value (as an integer) or None if not found + """ + syms = GetSymbols(fname, [sym_name]) + sym = syms.get(sym_name) + if not sym: + return None + return sym.address diff --git a/tools/binman/elf_test.py b/tools/binman/elf_test.py new file mode 100644 index 00000000000..dca7bc59f99 --- /dev/null +++ b/tools/binman/elf_test.py @@ -0,0 +1,32 @@ +# +# Copyright (c) 2017 Google, Inc +# Written by Simon Glass sjg@chromium.org +# +# SPDX-License-Identifier: GPL-2.0+ +# +# Test for the elf module + +import os +import sys +import unittest + +import elf + +binman_dir = os.path.dirname(os.path.realpath(sys.argv[0])) +fname = os.path.join(binman_dir, 'test', 'u_boot_ucode_ptr') + +class TestElf(unittest.TestCase): + def testAllSymbols(self): + syms = elf.GetSymbols(fname, []) + self.assertIn('.ucode', syms) + + def testRegexSymbols(self): + syms = elf.GetSymbols(fname, ['ucode']) + self.assertIn('.ucode', syms) + syms = elf.GetSymbols(fname, ['missing']) + self.assertNotIn('.ucode', syms) + syms = elf.GetSymbols(fname, ['missing', 'ucode']) + self.assertIn('.ucode', syms) + +if __name__ == '__main__': + unittest.main() diff --git a/tools/binman/etype/u_boot_spl_bss_pad.py b/tools/binman/etype/u_boot_spl_bss_pad.py index c005f28191f..c37f61db235 100644 --- a/tools/binman/etype/u_boot_spl_bss_pad.py +++ b/tools/binman/etype/u_boot_spl_bss_pad.py @@ -9,6 +9,7 @@ #

import command +import elf from entry import Entry from blob import Entry_blob import tools @@ -19,8 +20,8 @@ class Entry_u_boot_spl_bss_pad(Entry_blob):

def ObtainContents(self): fname = tools.GetInputFilename('spl/u-boot-spl') - args = [['nm', fname], ['grep', '__bss_size']] - out = command.RunPipe(args, capture=True).stdout.splitlines() - bss_size = int(out[0].split()[0], 16) + bss_size = elf.GetSymbolAddress(fname, '__bss_size') + if not bss_size: + self.Raise('Expected __bss_size symbol in spl/u-boot-spl') self.data = chr(0) * bss_size self.contents_size = bss_size diff --git a/tools/binman/etype/u_boot_with_ucode_ptr.py b/tools/binman/etype/u_boot_with_ucode_ptr.py index 6f01adb9701..99b437130db 100644 --- a/tools/binman/etype/u_boot_with_ucode_ptr.py +++ b/tools/binman/etype/u_boot_with_ucode_ptr.py @@ -9,6 +9,7 @@ import struct

import command +import elf from entry import Entry from blob import Entry_blob import fdt_util @@ -31,11 +32,9 @@ class Entry_u_boot_with_ucode_ptr(Entry_blob): def ObtainContents(self): # Figure out where to put the microcode pointer fname = tools.GetInputFilename(self.elf_fname) - args = [['nm', fname], ['grep', '-w', '_dt_ucode_base_size']] - out = (command.RunPipe(args, capture=True, raise_on_error=False). - stdout.splitlines()) - if len(out) == 1: - self.target_pos = int(out[0].split()[0], 16) + sym = elf.GetSymbolAddress(fname, '_dt_ucode_base_size') + if sym: + self.target_pos = sym elif not fdt_util.GetBool(self._node, 'optional-ucode'): self.Raise('Cannot locate _dt_ucode_base_size symbol in u-boot')

diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 9083143894f..c8155788afc 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -835,6 +835,13 @@ class TestFunctional(unittest.TestCase): data = self._DoReadFile('47_spl_bss_pad.dts') self.assertEqual(U_BOOT_SPL_DATA + (chr(0) * 10) + U_BOOT_DATA, data)

+ with open(self.TestFile('u_boot_ucode_ptr')) as fd: + TestFunctional._MakeInputFile('spl/u-boot-spl', fd.read()) + with self.assertRaises(ValueError) as e: + data = self._DoReadFile('47_spl_bss_pad.dts') + self.assertIn('Expected __bss_size symbol in spl/u-boot-spl', + str(e.exception)) + def testPackStart16Spl(self): """Test that an image with an x86 start16 region can be created""" data = self._DoReadFile('48_x86-start16-spl.dts') diff --git a/tools/binman/test/bss_data.c b/tools/binman/test/bss_data.c index f865a9d9f67..e0305c382c3 100644 --- a/tools/binman/test/bss_data.c +++ b/tools/binman/test/bss_data.c @@ -4,7 +4,7 @@ * SPDX-License-Identifier: GPL-2.0+ * * Simple program to create a _dt_ucode_base_size symbol which can be read - * by 'nm'. This is used by binman tests. + * by binutils. This is used by binman tests. */

int bss_data[10]; diff --git a/tools/binman/test/u_boot_ucode_ptr.c b/tools/binman/test/u_boot_ucode_ptr.c index 24f349fb9e4..734d54f0d41 100644 --- a/tools/binman/test/u_boot_ucode_ptr.c +++ b/tools/binman/test/u_boot_ucode_ptr.c @@ -4,7 +4,7 @@ * SPDX-License-Identifier: GPL-2.0+ * * Simple program to create a _dt_ucode_base_size symbol which can be read - * by 'nm'. This is used by binman tests. + * by binutils. This is used by binman tests. */

static unsigned long _dt_ucode_base_size[2]

This file contains the SPL device tree. Add support for including this by itself in images.

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

tools/binman/etype/u_boot_spl_dtb.py | 17 +++++++++++++++++ tools/binman/ftest.py | 7 +++++++ tools/binman/test/51_u_boot_spl_dtb.dts | 13 +++++++++++++ 3 files changed, 37 insertions(+) create mode 100644 tools/binman/etype/u_boot_spl_dtb.py create mode 100644 tools/binman/test/51_u_boot_spl_dtb.dts

diff --git a/tools/binman/etype/u_boot_spl_dtb.py b/tools/binman/etype/u_boot_spl_dtb.py new file mode 100644 index 00000000000..6c5ce1e996b --- /dev/null +++ b/tools/binman/etype/u_boot_spl_dtb.py @@ -0,0 +1,17 @@ +# Copyright (c) 2016 Google, Inc +# Written by Simon Glass sjg@chromium.org +# +# SPDX-License-Identifier: GPL-2.0+ +# +# Entry-type module for U-Boot device tree +# + +from entry import Entry +from blob import Entry_blob + +class Entry_u_boot_spl_dtb(Entry_blob): + def __init__(self, image, etype, node): + Entry_blob.__init__(self, image, etype, node) + + def GetDefaultFilename(self): + return 'spl/u-boot-spl.dtb' diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index c8155788afc..32bc7950b14 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -33,6 +33,7 @@ BLOB_DATA = '89' ME_DATA = '0abcd' VGA_DATA = 'vga' U_BOOT_DTB_DATA = 'udtb' +U_BOOT_SPL_DTB_DATA = 'spldtb' X86_START16_DATA = 'start16' X86_START16_SPL_DATA = 'start16spl' U_BOOT_NODTB_DATA = 'nodtb with microcode pointer somewhere in here' @@ -76,6 +77,7 @@ class TestFunctional(unittest.TestCase): TestFunctional._MakeInputFile('me.bin', ME_DATA) TestFunctional._MakeInputFile('vga.bin', VGA_DATA) TestFunctional._MakeInputFile('u-boot.dtb', U_BOOT_DTB_DATA) + TestFunctional._MakeInputFile('spl/u-boot-spl.dtb', U_BOOT_SPL_DTB_DATA) TestFunctional._MakeInputFile('u-boot-x86-16bit.bin', X86_START16_DATA) TestFunctional._MakeInputFile('spl/u-boot-x86-16bit-spl.bin', X86_START16_SPL_DATA) @@ -869,6 +871,11 @@ class TestFunctional(unittest.TestCase): data = self._DoReadFile('50_intel_mrc.dts') self.assertEqual(MRC_DATA, data[:len(MRC_DATA)])

+ def testSplDtb(self): + """Test that an image with spl/u-boot-spl.dtb can be created""" + data = self._DoReadFile('51_u_boot_spl_dtb.dts') + self.assertEqual(U_BOOT_SPL_DTB_DATA, data[:len(U_BOOT_SPL_DTB_DATA)]) +

if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/51_u_boot_spl_dtb.dts b/tools/binman/test/51_u_boot_spl_dtb.dts new file mode 100644 index 00000000000..3912f86b4cd --- /dev/null +++ b/tools/binman/test/51_u_boot_spl_dtb.dts @@ -0,0 +1,13 @@ +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + size = <16>; + + u-boot-spl-dtb { + }; + }; +};

This file contains SPL image without a device tree. Add support for including this in images.

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

tools/binman/etype/u_boot_spl_nodtb.py | 17 +++++++++++++++++ tools/binman/ftest.py | 5 +++++ tools/binman/test/52_u_boot_spl_nodtb.dts | 11 +++++++++++ 3 files changed, 33 insertions(+) create mode 100644 tools/binman/etype/u_boot_spl_nodtb.py create mode 100644 tools/binman/test/52_u_boot_spl_nodtb.dts

diff --git a/tools/binman/etype/u_boot_spl_nodtb.py b/tools/binman/etype/u_boot_spl_nodtb.py new file mode 100644 index 00000000000..880e0c78fbc --- /dev/null +++ b/tools/binman/etype/u_boot_spl_nodtb.py @@ -0,0 +1,17 @@ +# Copyright (c) 2016 Google, Inc +# Written by Simon Glass sjg@chromium.org +# +# SPDX-License-Identifier: GPL-2.0+ +# +# Entry-type module for 'u-boot-nodtb.bin' +# + +from entry import Entry +from blob import Entry_blob + +class Entry_u_boot_spl_nodtb(Entry_blob): + def __init__(self, image, etype, node): + Entry_blob.__init__(self, image, etype, node) + + def GetDefaultFilename(self): + return 'spl/u-boot-spl-nodtb.bin' diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 32bc7950b14..ed0697af006 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -876,6 +876,11 @@ class TestFunctional(unittest.TestCase): data = self._DoReadFile('51_u_boot_spl_dtb.dts') self.assertEqual(U_BOOT_SPL_DTB_DATA, data[:len(U_BOOT_SPL_DTB_DATA)])

+ def testSplNoDtb(self): + """Test that an image with spl/u-boot-spl-nodtb.bin can be created""" + data = self._DoReadFile('52_u_boot_spl_nodtb.dts') + self.assertEqual(U_BOOT_SPL_NODTB_DATA, data[:len(U_BOOT_SPL_NODTB_DATA)]) +

if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/52_u_boot_spl_nodtb.dts b/tools/binman/test/52_u_boot_spl_nodtb.dts new file mode 100644 index 00000000000..7f4e27780fe --- /dev/null +++ b/tools/binman/test/52_u_boot_spl_nodtb.dts @@ -0,0 +1,11 @@ +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + u-boot-spl-nodtb { + }; + }; +};

This feature is now supported. Drop the incorrect comment.

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

tools/binman/ftest.py | 1 - 1 file changed, 1 deletion(-)

diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index ed0697af006..590299da8bc 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -304,7 +304,6 @@ class TestFunctional(unittest.TestCase): self.assertEqual(0, len(result.stderr)) self.assertEqual(0, result.return_code)

- # Not yet available. def testBoard(self): """Test that we can run it with a specific board""" self._SetupDtb('05_simple.dts', 'sandbox/u-boot.dtb')

For testing we need to build some ELF files containing binman symbols. Add these to the Makefile and check in the binaries:

u_boot_binman_syms - normal, valid ELF file u_boot_binman_syms_bad - missing the __image_copy_start symbol u_boot_binman_syms_size - has a binman symbol with an invalid size

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

tools/binman/test/Makefile | 18 +++++++++++++++- tools/binman/test/u_boot_binman_syms | Bin 0 -> 4921 bytes tools/binman/test/u_boot_binman_syms.c | 14 +++++++++++++ tools/binman/test/u_boot_binman_syms.lds | 30 +++++++++++++++++++++++++++ tools/binman/test/u_boot_binman_syms_bad | Bin 0 -> 4890 bytes tools/binman/test/u_boot_binman_syms_bad.c | 1 + tools/binman/test/u_boot_binman_syms_bad.lds | 29 ++++++++++++++++++++++++++ tools/binman/test/u_boot_binman_syms_size | Bin 0 -> 4825 bytes tools/binman/test/u_boot_binman_syms_size.c | 12 +++++++++++ 9 files changed, 103 insertions(+), 1 deletion(-) create mode 100755 tools/binman/test/u_boot_binman_syms create mode 100644 tools/binman/test/u_boot_binman_syms.c create mode 100644 tools/binman/test/u_boot_binman_syms.lds create mode 100755 tools/binman/test/u_boot_binman_syms_bad create mode 120000 tools/binman/test/u_boot_binman_syms_bad.c create mode 100644 tools/binman/test/u_boot_binman_syms_bad.lds create mode 100755 tools/binman/test/u_boot_binman_syms_size create mode 100644 tools/binman/test/u_boot_binman_syms_size.c

diff --git a/tools/binman/test/Makefile b/tools/binman/test/Makefile index 217d13c666f..e58fc807757 100644 --- a/tools/binman/test/Makefile +++ b/tools/binman/test/Makefile @@ -10,8 +10,12 @@ CFLAGS := -march=i386 -m32 -nostdlib -I ../../../include

LDS_UCODE := -T u_boot_ucode_ptr.lds +LDS_BINMAN := -T u_boot_binman_syms.lds +LDS_BINMAN_BAD := -T u_boot_binman_syms_bad.lds

-TARGETS = u_boot_ucode_ptr u_boot_no_ucode_ptr bss_data +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

all: $(TARGETS)

@@ -24,6 +28,18 @@ u_boot_ucode_ptr: u_boot_ucode_ptr.c bss_data: CFLAGS += bss_data.lds bss_data: bss_data.c

+u_boot_binman_syms.bin: u_boot_binman_syms + objcopy -O binary $< -R .note.gnu.build-id $@ + +u_boot_binman_syms: CFLAGS += $(LDS_BINMAN) +u_boot_binman_syms: u_boot_binman_syms.c + +u_boot_binman_syms_bad: CFLAGS += $(LDS_BINMAN_BAD) +u_boot_binman_syms_bad: u_boot_binman_syms_bad.c + +u_boot_binman_syms_size: CFLAGS += $(LDS_BINMAN) +u_boot_binman_syms_size: u_boot_binman_syms_size.c + clean: rm -f $(TARGETS)

diff --git a/tools/binman/test/u_boot_binman_syms b/tools/binman/test/u_boot_binman_syms new file mode 100755 index 0000000000000000000000000000000000000000..2e02dc0ca9e2651bc12ac6d61d95dfa5ccee7a68 GIT binary patch literal 4921 zcmeHLJ!=$E6up~WjCLCl8=FP8Sdlz7>lA8b2$~2I5R9ddncX2S?0m5ECa{euEN#TX z-bUgtNGn*`S@{qA0oE3J&dyvj`LMBm7tY*|bKjl!4%5s#eE8&1tJM;<6={oR0g6Z6 ziV36#W+1E5srb51mVLw}Ca8C6Pe<$5V4ZmS!%g7M8P_+)p5uMNE8rFI3U~#)0$u^H zfLFjP;1%!+cm=!yUV;BpfdAvyfHttbyC*l3%`dz8*Q1?-H{tP%@4r60d-n0{<n67k z`@O+lRHm-%1?H{^)<@+s+jzARm|<Y<+^Xs#rdT8{jNsk8@-H<?GZ&lvv@}sUolVxK z6EV>=O~TY-jYab;7BkM0Bu?GgE&MaT0cI6g(GFZF^fiX(d1(gVJqOQCzK_1<-GlZ$ z0LQ)|Knu`xy9$?2a>N?IS!x!2k2Qg34~_os-VlGr!f)`r`ylvUER`!jbKcvlBA-xG z(|MX-<n<<p=cBbxy_vaoRD4H%{;r~v{5nFe=f2jkepFG*XBVfrhiCv?2FO2$9|GDx zN6fn+S{M9lc-;^4_os54s<cs-IeYRY6lN@%)?|^-Y(CF&n`edC!i95pLCdJoQ;)0H ehv}lh>2Q8^aXJy(EY!p8Gzs@(JI?Y&{k}g(dWGTu

literal 0 HcmV?d00001

diff --git a/tools/binman/test/u_boot_binman_syms.c b/tools/binman/test/u_boot_binman_syms.c new file mode 100644 index 00000000000..a9754769441 --- /dev/null +++ b/tools/binman/test/u_boot_binman_syms.c @@ -0,0 +1,14 @@ +/* + * Copyright (c) 2017 Google, Inc + * + * SPDX-License-Identifier: GPL-2.0+ + * + * Simple program to create some binman symbols. This is used by binman tests. + */ + +#define CONFIG_BINMAN +#include <binman_sym.h> + +binman_sym_declare(unsigned long, u_boot_spl, pos); +binman_sym_declare(unsigned long long, u_boot_spl2, pos); +binman_sym_declare(unsigned long, u_boot_any, pos); diff --git a/tools/binman/test/u_boot_binman_syms.lds b/tools/binman/test/u_boot_binman_syms.lds new file mode 100644 index 00000000000..d3130cdeb31 --- /dev/null +++ b/tools/binman/test/u_boot_binman_syms.lds @@ -0,0 +1,30 @@ +/* + * Copyright (c) 2016 Google, Inc + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +OUTPUT_FORMAT("elf32-i386", "elf32-i386", "elf32-i386") +OUTPUT_ARCH(i386) +ENTRY(_start) + +SECTIONS +{ + . = 0x00000000; + _start = .; + + . = ALIGN(4); + .text : + { + __image_copy_start = .; + *(.text*) + } + + . = ALIGN(4); + .binman_sym_table : { + __binman_sym_start = .; + KEEP(*(SORT(.binman_sym*))); + __binman_sym_end = .; + } + +} diff --git a/tools/binman/test/u_boot_binman_syms_bad b/tools/binman/test/u_boot_binman_syms_bad new file mode 100755 index 0000000000000000000000000000000000000000..8da3d9d48f388a9be53e92590984411691f6721f GIT binary patch literal 4890 zcmeHLJ!=$E6up~WjOjLFrOg&wtVkY797F^`2yqpRVv#}#JZ3VBF6_=MJ8y(ENwASv zTZuow-an8(k|ITFk^BHXXW!gp@?m5BF5I~v=e#@bo!MsJ-ulaDjYdP%=A<cFMQHwL zdnA$$ke<v-%i~p_D0%c7EYNRNk%88&oPGM66PF@fF|MzTy@H2EP#`D}6bK3g1%d)W zfuKN8ASe(N2nqxRf&%}i0RNBQhBx8;SDW(n=%|12{Pgtu@y6D{mtSwUKE3{Vd2)Yk z?Qy5KnUt9;JCS*0qNUYxR;}!=L}ocM5AONCh$(i)E=~~L)W;8+Ww{%e_gQI@ayUwt zhN+ljn2qDiVvPlPG!V0$kH>?|UEab!>$^ba;d9Esg+f<zM4p#s;JoMHxrukdtKU5+ z?<p|ymf^Grr29UdJ&Dm5;hd#r!F!<vA~`h1v%Mid#KJ>F-hI&dUhZQ7UZ1yrOXM4R zYC2Er>!RM|@O-r9g*UTShR0j-`;X7g>pMufp8HzF`iCBxJ=-|V6J$O3O*rv)h}25? zACdEJh}H)F8BzDcT1uPbxwGeAzOYH0nr+cmMJOgCJDKJaJIM>Ng^Q=|8p>*oQ;n?F U$JtH|)8YK34YE{hz2S%d1wo~NOaK4?

literal 0 HcmV?d00001

diff --git a/tools/binman/test/u_boot_binman_syms_bad.c b/tools/binman/test/u_boot_binman_syms_bad.c new file mode 120000 index 00000000000..939b2e965f8 --- /dev/null +++ b/tools/binman/test/u_boot_binman_syms_bad.c @@ -0,0 +1 @@ +u_boot_binman_syms.c \ No newline at end of file diff --git a/tools/binman/test/u_boot_binman_syms_bad.lds b/tools/binman/test/u_boot_binman_syms_bad.lds new file mode 100644 index 00000000000..0b474b53747 --- /dev/null +++ b/tools/binman/test/u_boot_binman_syms_bad.lds @@ -0,0 +1,29 @@ +/* + * Copyright (c) 2016 Google, Inc + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +OUTPUT_FORMAT("elf32-i386", "elf32-i386", "elf32-i386") +OUTPUT_ARCH(i386) +ENTRY(_start) + +SECTIONS +{ + . = 0x00000000; + _start = .; + + . = ALIGN(4); + .text : + { + *(.text*) + } + + . = ALIGN(4); + .binman_sym_table : { + __binman_sym_start = .; + KEEP(*(SORT(.binman_sym*))); + __binman_sym_end = .; + } + +} diff --git a/tools/binman/test/u_boot_binman_syms_size b/tools/binman/test/u_boot_binman_syms_size new file mode 100755 index 0000000000000000000000000000000000000000..d691e897c0f1842cb82efbc67f57d9f62853b99c GIT binary patch literal 4825 zcmeHLyKWOv5FI~+mChk_G%jq(2>Hq;D?t(oMY4z$2|-f0i*)7nuFOi_hrD+Sryze| zet;iH2NGo(I=XcH0Tm7Tf^f#WV{DlQ>O0c$+?hEuyZ6|Q=jzq#lTxWfVr8n3L=KW4 z>v_eY1}bf;Q8lj@d9Jn!Jm3KNYT?<jH4AW_asI=2a9hUxYh$<SHenU83RnfK0#*U5 zfK|XMU=^?mSOu&CRspNP|0%%##~%R|VDM&0-<k*N_Gt0-!_Cj{&TR2{@8Z`r4f_4V zMtkUHk;xj4dZC=ovuqjl-uE2Ub=3ZoJWr(3d1yv1yqV?xot&cB_-Ybm%FTkQ*9kmQ zZV-i|NTWxCJoQP9<1q9i^Dnn~3~~)9OB0p|O*zB!9oZDXyA8f8*PGzUt^?tUcZ@UV zy8v4Yyr=nb`N=iLF2J+I&cHjB0A6OZU%ea7U(omop7$LLem}I*4zNDYdq|WaCnobG z{+pNWjoiPo@noJajJ&%_clYaaIA%S$Kfhy{p05$=$i76(J0G?>=D&rP=W*<A#F&z4 zH;#?|^JS+x4K95(Cdn0yG@0sT7AHE1Q_`s!%?xSb@Q=igyv>Agf^anP^*B!EMcZ%C C$!_BS

literal 0 HcmV?d00001

diff --git a/tools/binman/test/u_boot_binman_syms_size.c b/tools/binman/test/u_boot_binman_syms_size.c new file mode 100644 index 00000000000..ee4c048b288 --- /dev/null +++ b/tools/binman/test/u_boot_binman_syms_size.c @@ -0,0 +1,12 @@ +/* + * Copyright (c) 2017 Google, Inc + * + * SPDX-License-Identifier: GPL-2.0+ + * + * Simple program to create some binman symbols. This is used by binman tests. + */ + +#define CONFIG_BINMAN +#include <binman_sym.h> + +binman_sym_declare(char, u_boot_spl, pos);

This is only 3 bytes long which is not enough to hold two symbol values, needed to test the binman symbols feature. Increase it to 15 bytes.

Using very small regions is useful since we can easily compare them in tests and errors are fairly easy to diagnose.

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

tools/binman/ftest.py | 12 ++++++------ tools/binman/test/21_image_pad.dts | 2 +- tools/binman/test/24_sorted.dts | 4 ++-- tools/binman/test/28_pack_4gb_outside.dts | 4 ++-- tools/binman/test/29_x86-rom.dts | 6 +++--- 5 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 590299da8bc..372b61fbb3b 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -28,7 +28,7 @@ import tout # Contents of test files, corresponding to different entry types U_BOOT_DATA = '1234' U_BOOT_IMG_DATA = 'img' -U_BOOT_SPL_DATA = '567' +U_BOOT_SPL_DATA = '56780123456789abcde' BLOB_DATA = '89' ME_DATA = '0abcd' VGA_DATA = 'vga' @@ -565,7 +565,7 @@ class TestFunctional(unittest.TestCase): def testImagePadByte(self): """Test that the image pad byte can be specified""" data = self._DoReadFile('21_image_pad.dts') - self.assertEqual(U_BOOT_SPL_DATA + (chr(0xff) * 9) + U_BOOT_DATA, data) + self.assertEqual(U_BOOT_SPL_DATA + (chr(0xff) * 1) + U_BOOT_DATA, data)

def testImageName(self): """Test that image files can be named""" @@ -587,7 +587,7 @@ class TestFunctional(unittest.TestCase): def testPackSorted(self): """Test that entries can be sorted""" data = self._DoReadFile('24_sorted.dts') - self.assertEqual(chr(0) * 5 + U_BOOT_SPL_DATA + chr(0) * 2 + + self.assertEqual(chr(0) * 1 + U_BOOT_SPL_DATA + chr(0) * 2 + U_BOOT_DATA, data)

def testPackZeroPosition(self): @@ -615,14 +615,14 @@ class TestFunctional(unittest.TestCase): with self.assertRaises(ValueError) as e: self._DoTestFile('28_pack_4gb_outside.dts') self.assertIn("Node '/binman/u-boot': Position 0x0 (0) is outside " - "the image starting at 0xfffffff0 (4294967280)", + "the image starting at 0xffffffe0 (4294967264)", str(e.exception))

def testPackX86Rom(self): """Test that a basic x86 ROM can be created""" data = self._DoReadFile('29_x86-rom.dts') - self.assertEqual(U_BOOT_DATA + chr(0) * 3 + U_BOOT_SPL_DATA + - chr(0) * 6, data) + self.assertEqual(U_BOOT_DATA + chr(0) * 7 + U_BOOT_SPL_DATA + + chr(0) * 2, data)

def testPackX86RomMeNoDesc(self): """Test that an invalid Intel descriptor entry is detected""" diff --git a/tools/binman/test/21_image_pad.dts b/tools/binman/test/21_image_pad.dts index daf8385f6d5..bf39dc1b6f7 100644 --- a/tools/binman/test/21_image_pad.dts +++ b/tools/binman/test/21_image_pad.dts @@ -10,7 +10,7 @@ };

u-boot { - pos = <12>; + pos = <20>; }; }; }; diff --git a/tools/binman/test/24_sorted.dts b/tools/binman/test/24_sorted.dts index 9f4151c932b..43a7831341c 100644 --- a/tools/binman/test/24_sorted.dts +++ b/tools/binman/test/24_sorted.dts @@ -7,11 +7,11 @@ binman { sort-by-pos; u-boot { - pos = <10>; + pos = <22>; };

u-boot-spl { - pos = <5>; + pos = <1>; }; }; }; diff --git a/tools/binman/test/28_pack_4gb_outside.dts b/tools/binman/test/28_pack_4gb_outside.dts index ff468c7d41d..18d6bb5b8af 100644 --- a/tools/binman/test/28_pack_4gb_outside.dts +++ b/tools/binman/test/28_pack_4gb_outside.dts @@ -7,13 +7,13 @@ binman { sort-by-pos; end-at-4gb; - size = <16>; + size = <32>; u-boot { pos = <0>; };

u-boot-spl { - pos = <0xfffffff7>; + pos = <0xffffffeb>; }; }; }; diff --git a/tools/binman/test/29_x86-rom.dts b/tools/binman/test/29_x86-rom.dts index 075ede36ab3..d49078e19e2 100644 --- a/tools/binman/test/29_x86-rom.dts +++ b/tools/binman/test/29_x86-rom.dts @@ -7,13 +7,13 @@ binman { sort-by-pos; end-at-4gb; - size = <16>; + size = <32>; u-boot { - pos = <0xfffffff0>; + pos = <0xffffffe0>; };

u-boot-spl { - pos = <0xfffffff7>; + pos = <0xffffffeb>; }; }; };

The elf module can provide some debugging information to assist with figuring out what is going wrong. This is also useful in tests. Update the -D option so that it is passed through to tests as well.

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

tools/binman/binman.py | 6 ++++-- tools/binman/control.py | 3 +++ tools/binman/elf.py | 3 +++ tools/binman/ftest.py | 17 +++++++++++++---- 4 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/tools/binman/binman.py b/tools/binman/binman.py index 81a613ddc40..aa513962663 100755 --- a/tools/binman/binman.py +++ b/tools/binman/binman.py @@ -31,7 +31,7 @@ import cmdline import command import control

-def RunTests(): +def RunTests(debug): """Run the functional tests and any embedded doctests""" import elf_test import entry_test @@ -46,6 +46,8 @@ def RunTests(): suite.run(result)

sys.argv = [sys.argv[0]] + if debug: + sys.argv.append('-D')

# Run the entry tests first ,since these need to be the first to import the # 'entry' module. @@ -111,7 +113,7 @@ def RunBinman(options, args): sys.tracebacklimit = 0

if options.test: - ret_code = RunTests() + ret_code = RunTests(options.debug)

elif options.test_coverage: RunTestCoverage() diff --git a/tools/binman/control.py b/tools/binman/control.py index e9d48df0301..e175e8d41bd 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -12,6 +12,7 @@ import sys import tools

import command +import elf import fdt import fdt_util from image import Image @@ -89,6 +90,8 @@ def Binman(options, args):

try: tout.Init(options.verbosity) + if options.debug: + elf.debug = True try: tools.SetInputDirs(options.indir) tools.PrepareOutputDir(options.outdir, options.preserve) diff --git a/tools/binman/elf.py b/tools/binman/elf.py index 97208b17950..0fb5a4a8ed6 100644 --- a/tools/binman/elf.py +++ b/tools/binman/elf.py @@ -14,6 +14,9 @@ import struct

import tools

+# This is enabled from control.py +debug = False + Symbol = namedtuple('Symbol', ['section', 'address', 'size', 'weak'])

# Used for tests which don't have an ELF file to read diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 372b61fbb3b..2bee6a168f3 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -136,7 +136,10 @@ class TestFunctional(unittest.TestCase): Returns: Return value (0 for success) """ - (options, args) = cmdline.ParseArgs(list(args)) + args = list(args) + if '-D' in sys.argv: + args = args + ['-D'] + (options, args) = cmdline.ParseArgs(args) options.pager = 'binman-invalid-pager' options.build_dir = self._indir

@@ -144,14 +147,16 @@ class TestFunctional(unittest.TestCase): # options.verbosity = tout.DEBUG return control.Binman(options, args)

- def _DoTestFile(self, fname): + def _DoTestFile(self, fname, debug=False): """Run binman with a given test file

Args: fname: Device tree source filename to use (e.g. 05_simple.dts) """ - return self._DoBinman('-p', '-I', self._indir, - '-d', self.TestFile(fname)) + args = ['-p', '-I', self._indir, '-d', self.TestFile(fname)] + if debug: + args.append('-D') + return self._DoBinman(*args)

def _SetupDtb(self, fname, outfile='u-boot.dtb'): """Set up a new test device-tree file @@ -363,6 +368,10 @@ class TestFunctional(unittest.TestCase): data = self._DoReadFile('05_simple.dts') self.assertEqual(U_BOOT_DATA, data)

+ def testSimpleDebug(self): + """Test a simple binman run with debugging enabled""" + data = self._DoTestFile('05_simple.dts', debug=True) + def testDual(self): """Test that we can handle creating two images

Binman construct images consisting of multiple binary files. These files sometimes need to know (at run timme) where their peers are located. For example, SPL may want to know where U-Boot is located in the image, so that it can jump to U-Boot correctly on boot.

In general the positions where the binaries end up after binman has finished packing them cannot be known at compile time. One reason for this is that binman does not know the size of the binaries until everything is compiled, linked and converted to binaries with objcopy.

To make this work, we add a feature to binman which checks each binary for symbol names starting with '_binman'. These are then decoded to figure out which entry and property they refer to. Then binman writes the value of this symbol into the appropriate binary. With this, the symbol will have the correct value at run time.

Macros are used to make this easier to use. As an example, this declares a symbol that will access the 'u-boot-spl' entry to find the 'pos' value (i.e. the position of SPL in the image):

binman_sym_declare(unsigned long, u_boot_spl, pos);

This converts to a symbol called '_binman_u_boot_spl_prop_pos' in any binary that includes it. Binman then updates the value in that binary, ensuring that it can be accessed at runtime with:

ulong u_boot_pos = binman_sym(ulong, u_boot_spl, pos);

This assigns the variable u_boot_pos to the position of SPL in the image.

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

include/binman_sym.h | 80 ++++++++++++++++++++++++++++++++++ tools/binman/binman.py | 4 +- tools/binman/control.py | 4 +- tools/binman/elf.py | 55 ++++++++++++++++++++++-- tools/binman/elf_test.py | 92 +++++++++++++++++++++++++++++++++++++++- tools/binman/etype/entry.py | 8 ++++ tools/binman/etype/u_boot_spl.py | 6 +++ tools/binman/ftest.py | 19 +++++++++ tools/binman/image.py | 79 ++++++++++++++++++++++++++++++++-- tools/binman/image_test.py | 46 ++++++++++++++++++++ tools/binman/test/53_symbols.dts | 20 +++++++++ 11 files changed, 403 insertions(+), 10 deletions(-) create mode 100644 include/binman_sym.h create mode 100644 tools/binman/image_test.py create mode 100644 tools/binman/test/53_symbols.dts

diff --git a/include/binman_sym.h b/include/binman_sym.h new file mode 100644 index 00000000000..3999b26d8d4 --- /dev/null +++ b/include/binman_sym.h @@ -0,0 +1,80 @@ +/* + * Symbol access for symbols set up by binman as part of the build. + * + * This allows C code to access the position of a particular part of the image + * assembled by binman. + * + * Copyright (c) 2017 Google, Inc + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#ifndef __BINMAN_SYM_H +#define __BINMAN_SYM_H + +#define BINMAN_SYM_MISSING (-1UL) + +#ifdef CONFIG_BINMAN + +/** + * binman_symname() - Internal fnuction to get a binman symbol name + * + * @entry_name: Name of the entry to look for (e.g. 'u_boot_spl') + * @_prop_name: Property value to get from that entry (e.g. 'pos') + * @returns name of the symbol for that entry and property + */ +#define binman_symname(_entry_name, _prop_name) \ + _binman_ ## _entry_name ## _prop_ ## _prop_name + +/** + * binman_sym_declare() - Declare a symbol that will be used at run-time + * + * @_type: Type f the symbol (e.g. unsigned long) + * @entry_name: Name of the entry to look for (e.g. 'u_boot_spl') + * @_prop_name: Property value to get from that entry (e.g. 'pos') + */ +#define binman_sym_declare(_type, _entry_name, _prop_name) \ + _type binman_symname(_entry_name, _prop_name) \ + __attribute__((aligned(4), unused, section(".binman_sym"))) + +/** + * binman_sym_declare_optional() - Declare an optional symbol + * + * If this symbol cannot be provided by binman, an error will not be generated. + * Instead the image will be assigned the value BINMAN_SYM_MISSING. + * + * @_type: Type f the symbol (e.g. unsigned long) + * @entry_name: Name of the entry to look for (e.g. 'u_boot_spl') + * @_prop_name: Property value to get from that entry (e.g. 'pos') + */ +#define binman_sym_declare_optional(_type, _entry_name, _prop_name) \ + _type binman_symname(_entry_name, _prop_name) \ + __attribute__((aligned(4), weak, unused, \ + section(".binman_sym"))) + +/** + * binman_sym() - Access a previously declared symbol + * + * This is used to get the value of a symbol. E.g.: + * + * ulong address = binman_sym(ulong, u_boot_spl, pos); + * + * @_type: Type f the symbol (e.g. unsigned long) + * @entry_name: Name of the entry to look for (e.g. 'u_boot_spl') + * @_prop_name: Property value to get from that entry (e.g. 'pos') + * @returns value of that property (filled in by binman) + */ +#define binman_sym(_type, _entry_name, _prop_name) \ + (*(_type *)&binman_symname(_entry_name, _prop_name)) + +#else /* !BINMAN */ + +#define binman_sym_declare(_type, _entry_name, _prop_name) + +#define binman_sym_declare_optional(_type, _entry_name, _prop_name) + +#define binman_sym(_type, _entry_name, _prop_name) BINMAN_SYM_MISSING + +#endif /* BINMAN */ + +#endif diff --git a/tools/binman/binman.py b/tools/binman/binman.py index aa513962663..1c8e8dbff65 100755 --- a/tools/binman/binman.py +++ b/tools/binman/binman.py @@ -37,6 +37,7 @@ def RunTests(debug): import entry_test import fdt_test import ftest + import image_test import test import doctest

@@ -53,7 +54,8 @@ def RunTests(debug): # 'entry' module. suite = unittest.TestLoader().loadTestsFromTestCase(entry_test.TestEntry) suite.run(result) - for module in (ftest.TestFunctional, fdt_test.TestFdt, elf_test.TestElf): + for module in (ftest.TestFunctional, fdt_test.TestFdt, elf_test.TestElf, + image_test.TestImage): suite = unittest.TestLoader().loadTestsFromTestCase(module) suite.run(result)

diff --git a/tools/binman/control.py b/tools/binman/control.py index e175e8d41bd..ffa2bbd80f6 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -90,8 +90,7 @@ def Binman(options, args):

try: tout.Init(options.verbosity) - if options.debug: - elf.debug = True + elf.debug = options.debug try: tools.SetInputDirs(options.indir) tools.PrepareOutputDir(options.outdir, options.preserve) @@ -112,6 +111,7 @@ def Binman(options, args): image.CheckSize() image.CheckEntries() image.ProcessEntryContents() + image.WriteSymbols() image.BuildImage() finally: tools.FinaliseOutputDir() diff --git a/tools/binman/elf.py b/tools/binman/elf.py index 0fb5a4a8ed6..80ff2253f03 100644 --- a/tools/binman/elf.py +++ b/tools/binman/elf.py @@ -19,9 +19,6 @@ debug = False

Symbol = namedtuple('Symbol', ['section', 'address', 'size', 'weak'])

-# Used for tests which don't have an ELF file to read -ignore_missing_files = False -

def GetSymbols(fname, patterns): """Get the symbols from an ELF file @@ -78,3 +75,55 @@ def GetSymbolAddress(fname, sym_name): if not sym: return None return sym.address + +def LookupAndWriteSymbols(elf_fname, entry, image): + """Replace all symbols in an entry with their correct values + + The entry contents is updated so that values for referenced symbols will be + visible at run time. This is done by finding out the symbols positions in + the entry (using the ELF file) and replacing them with values from binman's + data structures. + + Args: + elf_fname: Filename of ELF image containing the symbol information for + entry + entry: Entry to process + image: Image which can be used to lookup symbol values + """ + fname = tools.GetInputFilename(elf_fname) + syms = GetSymbols(fname, ['image', 'binman']) + if not syms: + return + base = syms.get('__image_copy_start') + if not base: + return + for name, sym in syms.iteritems(): + if name.startswith('_binman'): + msg = ("Image '%s': Symbol '%s'\n in entry '%s'" % + (image.GetPath(), name, entry.GetPath())) + offset = sym.address - base.address + if offset < 0 or offset + sym.size > entry.contents_size: + raise ValueError('%s has offset %x (size %x) but the contents ' + 'size is %x' % (entry.GetPath(), offset, + sym.size, entry.contents_size)) + if sym.size == 4: + pack_string = '<I' + elif sym.size == 8: + pack_string = '<Q' + else: + raise ValueError('%s has size %d: only 4 and 8 are supported' % + (msg, sym.size)) + + # Look up the symbol in our entry tables. + value = image.LookupSymbol(name, sym.weak, msg) + if value is not None: + value += base.address + else: + value = -1 + pack_string = pack_string.lower() + value_bytes = struct.pack(pack_string, value) + if debug: + print('%s:\n insert %s, offset %x, value %x, length %d' % + (msg, name, offset, value, len(value_bytes))) + entry.data = (entry.data[:offset] + value_bytes + + entry.data[offset + sym.size:]) diff --git a/tools/binman/elf_test.py b/tools/binman/elf_test.py index dca7bc59f99..e5fc28258dc 100644 --- a/tools/binman/elf_test.py +++ b/tools/binman/elf_test.py @@ -6,21 +6,60 @@ # # Test for the elf module

+from contextlib import contextmanager import os import sys import unittest

+try: + from StringIO import StringIO +except ImportError: + from io import StringIO + import elf

binman_dir = os.path.dirname(os.path.realpath(sys.argv[0])) -fname = os.path.join(binman_dir, 'test', 'u_boot_ucode_ptr') + +# Use this to suppress stdout/stderr output: +# with capture_sys_output() as (stdout, stderr) +# ...do something... +@contextmanager +def capture_sys_output(): + capture_out, capture_err = StringIO(), StringIO() + old_out, old_err = sys.stdout, sys.stderr + try: + sys.stdout, sys.stderr = capture_out, capture_err + yield capture_out, capture_err + finally: + sys.stdout, sys.stderr = old_out, old_err + + +class FakeEntry: + def __init__(self, contents_size): + self.contents_size = contents_size + self.data = 'a' * contents_size + + def GetPath(self): + return 'entry_path' + +class FakeImage: + def __init__(self, sym_value=1): + self.sym_value = sym_value + + def GetPath(self): + return 'image_path' + + def LookupSymbol(self, name, weak, msg): + return self.sym_value

class TestElf(unittest.TestCase): def testAllSymbols(self): + fname = os.path.join(binman_dir, 'test', 'u_boot_ucode_ptr') syms = elf.GetSymbols(fname, []) self.assertIn('.ucode', syms)

def testRegexSymbols(self): + fname = os.path.join(binman_dir, 'test', 'u_boot_ucode_ptr') syms = elf.GetSymbols(fname, ['ucode']) self.assertIn('.ucode', syms) syms = elf.GetSymbols(fname, ['missing']) @@ -28,5 +67,56 @@ class TestElf(unittest.TestCase): syms = elf.GetSymbols(fname, ['missing', 'ucode']) self.assertIn('.ucode', syms)

+ def testMissingFile(self): + entry = FakeEntry(10) + image = FakeImage() + with self.assertRaises(ValueError) as e: + syms = elf.LookupAndWriteSymbols('missing-file', entry, image) + self.assertIn("Filename 'missing-file' not found in input path", + str(e.exception)) + + def testOutsideFile(self): + entry = FakeEntry(10) + image = FakeImage() + elf_fname = os.path.join(binman_dir, 'test', 'u_boot_binman_syms') + with self.assertRaises(ValueError) as e: + syms = elf.LookupAndWriteSymbols(elf_fname, entry, image) + self.assertIn('entry_path has offset 4 (size 8) but the contents size ' + 'is a', str(e.exception)) + + def testMissingImageStart(self): + entry = FakeEntry(10) + image = FakeImage() + elf_fname = os.path.join(binman_dir, 'test', 'u_boot_binman_syms_bad') + self.assertEqual(elf.LookupAndWriteSymbols(elf_fname, entry, image), + None) + + def testBadSymbolSize(self): + entry = FakeEntry(10) + image = FakeImage() + elf_fname = os.path.join(binman_dir, 'test', 'u_boot_binman_syms_size') + with self.assertRaises(ValueError) as e: + syms = elf.LookupAndWriteSymbols(elf_fname, entry, image) + self.assertIn('has size 1: only 4 and 8 are supported', + str(e.exception)) + + def testNoValue(self): + entry = FakeEntry(20) + image = FakeImage(sym_value=None) + elf_fname = os.path.join(binman_dir, 'test', 'u_boot_binman_syms') + syms = elf.LookupAndWriteSymbols(elf_fname, entry, image) + self.assertEqual(chr(255) * 16 + 'a' * 4, entry.data) + + def testDebug(self): + elf.debug = True + entry = FakeEntry(20) + image = FakeImage() + elf_fname = os.path.join(binman_dir, 'test', 'u_boot_binman_syms') + with capture_sys_output() as (stdout, stderr): + syms = elf.LookupAndWriteSymbols(elf_fname, entry, image) + elf.debug = False + self.assertTrue(len(stdout.getvalue()) > 0) + + if __name__ == '__main__': unittest.main() diff --git a/tools/binman/etype/entry.py b/tools/binman/etype/entry.py index 67c57341ca2..5541887d475 100644 --- a/tools/binman/etype/entry.py +++ b/tools/binman/etype/entry.py @@ -198,3 +198,11 @@ class Entry(object):

def ProcessContents(self): pass + + def WriteSymbols(self, image): + """Write symbol values into binary files for access at run time + + Args: + image: Image containing the entry + """ + pass diff --git a/tools/binman/etype/u_boot_spl.py b/tools/binman/etype/u_boot_spl.py index 68b0148427d..3720b47fef6 100644 --- a/tools/binman/etype/u_boot_spl.py +++ b/tools/binman/etype/u_boot_spl.py @@ -6,12 +6,18 @@ # Entry-type module for spl/u-boot-spl.bin #

+import elf + from entry import Entry from blob import Entry_blob

class Entry_u_boot_spl(Entry_blob): def __init__(self, image, etype, node): Entry_blob.__init__(self, image, etype, node) + self.elf_fname = 'spl/u-boot-spl'

def GetDefaultFilename(self): return 'spl/u-boot-spl.bin' + + def WriteSymbols(self, image): + elf.LookupAndWriteSymbols(self.elf_fname, self, image) diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 2bee6a168f3..5812ab397cf 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -20,6 +20,7 @@ import binman import cmdline import command import control +import elf import fdt import fdt_util import tools @@ -573,6 +574,8 @@ class TestFunctional(unittest.TestCase):

def testImagePadByte(self): """Test that the image pad byte can be specified""" + with open(self.TestFile('bss_data')) as fd: + TestFunctional._MakeInputFile('spl/u-boot-spl', fd.read()) data = self._DoReadFile('21_image_pad.dts') self.assertEqual(U_BOOT_SPL_DATA + (chr(0xff) * 1) + U_BOOT_DATA, data)

@@ -889,6 +892,22 @@ class TestFunctional(unittest.TestCase): data = self._DoReadFile('52_u_boot_spl_nodtb.dts') self.assertEqual(U_BOOT_SPL_NODTB_DATA, data[:len(U_BOOT_SPL_NODTB_DATA)])

+ def testSymbols(self): + """Test binman can assign symbols embedded in U-Boot""" + elf_fname = self.TestFile('u_boot_binman_syms') + syms = elf.GetSymbols(elf_fname, ['binman', 'image']) + addr = elf.GetSymbolAddress(elf_fname, '__image_copy_start') + self.assertEqual(syms['_binman_u_boot_spl_prop_pos'].address, addr) + + with open(self.TestFile('u_boot_binman_syms')) as fd: + TestFunctional._MakeInputFile('spl/u-boot-spl', fd.read()) + data = self._DoReadFile('53_symbols.dts') + sym_values = struct.pack('<LQL', 0x24 + 0, 0x24 + 24, 0x24 + 20) + expected = (sym_values + U_BOOT_SPL_DATA[16:] + chr(0xff) + + U_BOOT_DATA + + sym_values + U_BOOT_SPL_DATA[16:]) + self.assertEqual(expected, data) +

if __name__ == "__main__": unittest.main() diff --git a/tools/binman/image.py b/tools/binman/image.py index 24c4f6f578a..741630f0912 100644 --- a/tools/binman/image.py +++ b/tools/binman/image.py @@ -6,8 +6,12 @@ # Class for an image, the output of binman #

+from __future__ import print_function + from collections import OrderedDict from operator import attrgetter +import re +import sys

import fdt_util import tools @@ -45,7 +49,7 @@ class Image: address. _entries: OrderedDict() of entries """ - def __init__(self, name, node): + def __init__(self, name, node, test=False): global entry global Entry import entry @@ -64,8 +68,9 @@ class Image: self._end_4gb = False self._entries = OrderedDict()

- self._ReadNode() - self._ReadEntries() + if not test: + self._ReadNode() + self._ReadEntries()

def _ReadNode(self): """Read properties from the image node""" @@ -119,6 +124,14 @@ class Image: """ raise ValueError("Image '%s': %s" % (self._node.path, msg))

+ def GetPath(self): + """Get the path of an image (in the FDT) + + Returns: + Full path of the node for this image + """ + return self._node.path + def _ReadEntries(self): for node in self._node.subnodes: self._entries[node.name] = Entry.Create(self, node) @@ -220,6 +233,11 @@ class Image: for entry in self._entries.values(): entry.ProcessContents()

+ def WriteSymbols(self): + """Write symbol values into binary files for access at run time""" + for entry in self._entries.values(): + entry.WriteSymbols(self) + def BuildImage(self): """Write the image to a file""" fname = tools.GetOutputFilename(self._filename) @@ -230,3 +248,58 @@ class Image: data = entry.GetData() fd.seek(self._pad_before + entry.pos - self._skip_at_start) fd.write(data) + + def LookupSymbol(self, sym_name, optional, msg): + """Look up a symbol in an ELF file + + Looks up a symbol in an ELF file. Only entry types which come from an + ELF image can be used by this function. + + At present the only entry property supported is pos. + + Args: + sym_name: Symbol name in the ELF file to look up in the format + _binman_<entry>_prop_<property> where <entry> is the name of + the entry and <property> is the property to find (e.g. + _binman_u_boot_prop_pos). As a special case, you can append + _any to <entry> to have it search for any matching entry. E.g. + _binman_u_boot_any_prop_pos will match entries called u-boot, + u-boot-img and u-boot-nodtb) + optional: True if the symbol is optional. If False this function + will raise if the symbol is not found + msg: Message to display if an error occurs + + Returns: + Value that should be assigned to that symbol, or None if it was + optional and not found + + Raises: + ValueError if the symbol is invalid or not found, or references a + property which is not supported + """ + m = re.match(r'^_binman_(\w+)_prop_(\w+)$', sym_name) + if not m: + raise ValueError("%s: Symbol '%s' has invalid format" % + (msg, sym_name)) + entry_name, prop_name = m.groups() + entry_name = entry_name.replace('_', '-') + entry = self._entries.get(entry_name) + if not entry: + if entry_name.endswith('-any'): + root = entry_name[:-4] + for name in self._entries: + if name.startswith(root): + rest = name[len(root):] + if rest in ['', '-img', '-nodtb']: + entry = self._entries[name] + if not entry: + err = ("%s: Entry '%s' not found in list (%s)" % + (msg, entry_name, ','.join(self._entries.keys()))) + if optional: + print('Warning: %s' % err, file=sys.stderr) + return None + raise ValueError(err) + if prop_name == 'pos': + return entry.pos + else: + raise ValueError("%s: No such property '%s'" % (msg, prop_name)) diff --git a/tools/binman/image_test.py b/tools/binman/image_test.py new file mode 100644 index 00000000000..1b50dda4dca --- /dev/null +++ b/tools/binman/image_test.py @@ -0,0 +1,46 @@ +# +# Copyright (c) 2017 Google, Inc +# Written by Simon Glass sjg@chromium.org +# +# SPDX-License-Identifier: GPL-2.0+ +# +# Test for the image module + +import unittest + +from image import Image +from elf_test import capture_sys_output + +class TestImage(unittest.TestCase): + def testInvalidFormat(self): + image = Image('name', 'node', test=True) + with self.assertRaises(ValueError) as e: + image.LookupSymbol('_binman_something_prop_', False, 'msg') + self.assertIn( + "msg: Symbol '_binman_something_prop_' has invalid format", + str(e.exception)) + + def testMissingSymbol(self): + image = Image('name', 'node', test=True) + image._entries = {} + with self.assertRaises(ValueError) as e: + image.LookupSymbol('_binman_type_prop_pname', False, 'msg') + self.assertIn("msg: Entry 'type' not found in list ()", + str(e.exception)) + + def testMissingSymbolOptional(self): + image = Image('name', 'node', test=True) + image._entries = {} + with capture_sys_output() as (stdout, stderr): + val = image.LookupSymbol('_binman_type_prop_pname', True, 'msg') + self.assertEqual(val, None) + self.assertEqual("Warning: msg: Entry 'type' not found in list ()\n", + stderr.getvalue()) + self.assertEqual('', stdout.getvalue()) + + def testBadProperty(self): + image = Image('name', 'node', test=True) + image._entries = {'u-boot': 1} + with self.assertRaises(ValueError) as e: + image.LookupSymbol('_binman_u_boot_prop_bad', False, 'msg') + self.assertIn("msg: No such property 'bad", str(e.exception)) diff --git a/tools/binman/test/53_symbols.dts b/tools/binman/test/53_symbols.dts new file mode 100644 index 00000000000..980b066eb21 --- /dev/null +++ b/tools/binman/test/53_symbols.dts @@ -0,0 +1,20 @@ +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + pad-byte = <0xff>; + u-boot-spl { + }; + + u-boot { + pos = <20>; + }; + + u-boot-spl2 { + type = "u-boot-spl"; + }; + }; +};

This area of the image contains symbols whose values are filled in by binman. If this feature is not used, the table is empty.

Add this to the ARM SPL link script.

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

arch/arm/config.mk | 6 ++++-- arch/arm/cpu/u-boot-spl.lds | 7 +++++++ 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/arch/arm/config.mk b/arch/arm/config.mk index 1a77779db4d..eb2ae532bf1 100644 --- a/arch/arm/config.mk +++ b/arch/arm/config.mk @@ -136,10 +136,12 @@ endif # limit ourselves to the sections we want in the .bin. ifdef CONFIG_ARM64 OBJCOPYFLAGS += -j .text -j .secure_text -j .secure_data -j .rodata -j .data \ - -j .u_boot_list -j .rela.dyn -j .got -j .got.plt + -j .u_boot_list -j .rela.dyn -j .got -j .got.plt \ + -j .binman_sym_table else OBJCOPYFLAGS += -j .text -j .secure_text -j .secure_data -j .rodata -j .hash \ - -j .data -j .got -j .got.plt -j .u_boot_list -j .rel.dyn + -j .data -j .got -j .got.plt -j .u_boot_list -j .rel.dyn \ + -j .binman_sym_table endif

# if a dtb section exists we always have to include it diff --git a/arch/arm/cpu/u-boot-spl.lds b/arch/arm/cpu/u-boot-spl.lds index 068163b73a6..65f7b68861e 100644 --- a/arch/arm/cpu/u-boot-spl.lds +++ b/arch/arm/cpu/u-boot-spl.lds @@ -36,6 +36,13 @@ SECTIONS KEEP(*(SORT(.u_boot_list*))); }

+ . = ALIGN(4); + .binman_sym_table : { + __binman_sym_start = .; + KEEP(*(SORT(.binman_sym*))); + __binman_sym_end = .; + } + . = ALIGN(4);

__image_copy_end = .;

Allow SPL to access binman symbols and use this to get the address of U-Boot. This falls back to CONFIG_SYS_TEXT_BASE if the binman symbol is not available.

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

common/spl/spl.c | 16 ++++++++++++++-- include/spl.h | 11 +++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/common/spl/spl.c b/common/spl/spl.c index aaddddd9958..9e4a1ad0edc 100644 --- a/common/spl/spl.c +++ b/common/spl/spl.c @@ -8,6 +8,7 @@ */

#include <common.h> +#include <binman_sym.h> #include <dm.h> #include <spl.h> #include <asm/u-boot.h> @@ -32,6 +33,9 @@ DECLARE_GLOBAL_DATA_PTR;

u32 *boot_params_ptr = NULL;

+/* See spl.h for information about this */ +binman_sym_declare(ulong, u_boot_any, pos); + /* Define board data structure */ static bd_t bdata __attribute__ ((section(".data")));

@@ -120,9 +124,17 @@ __weak void spl_board_prepare_for_boot(void)

void spl_set_header_raw_uboot(struct spl_image_info *spl_image) { + ulong u_boot_pos = binman_sym(ulong, u_boot_any, pos); + spl_image->size = CONFIG_SYS_MONITOR_LEN; - spl_image->entry_point = CONFIG_SYS_UBOOT_START; - spl_image->load_addr = CONFIG_SYS_TEXT_BASE; + if (u_boot_pos != BINMAN_SYM_MISSING) { + /* biman does not support separate entry addresses at present */ + spl_image->entry_point = u_boot_pos; + spl_image->load_addr = u_boot_pos; + } else { + spl_image->entry_point = CONFIG_SYS_UBOOT_START; + spl_image->load_addr = CONFIG_SYS_TEXT_BASE; + } spl_image->os = IH_OS_U_BOOT; spl_image->name = "U-Boot"; } diff --git a/include/spl.h b/include/spl.h index b14a29c57cc..36af553c167 100644 --- a/include/spl.h +++ b/include/spl.h @@ -7,6 +7,8 @@ #ifndef _SPL_H_ #define _SPL_H_

+#include <binman_sym.h> + /* Platform-specific defines */ #include <linux/compiler.h> #include <asm/spl.h> @@ -48,6 +50,15 @@ struct spl_load_info { void *buf); };

+/* + * We need to know the position of U-Boot in memory so we can jump to it. We + * allow any U-Boot binary to be used (u-boot.bin, u-boot-nodtb.bin, + * u-boot.img), hence the '_any'. These is no checking here that the correct + * image is found. For * example if u-boot.img is used we don't check that + * spl_parse_image_header() can parse a valid header. + */ +extern binman_sym_declare(ulong, u_boot_any, pos); + /** * spl_load_simple_fit() - Loads a fit image from a device. * @spl_image: Image description to set up

SPL supports reading U-Boot from a RAM location. At present this is hard-coded to the U-Boot text base address. Use binman to allow this to come from the image file, if binman is used.

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

common/spl/spl_ram.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/common/spl/spl_ram.c b/common/spl/spl_ram.c index b2645a19480..fa8c768773a 100644 --- a/common/spl/spl_ram.c +++ b/common/spl/spl_ram.c @@ -11,6 +11,8 @@ * SPDX-License-Identifier: GPL-2.0+ */ #include <common.h> +#include <binman_sym.h> +#include <mapmem.h> #include <spl.h> #include <libfdt.h>

@@ -48,15 +50,24 @@ static int spl_ram_load_image(struct spl_image_info *spl_image, load.read = spl_ram_load_read; spl_load_simple_fit(spl_image, &load, 0, header); } else { + ulong u_boot_pos = binman_sym(ulong, u_boot_any, pos); + debug("Legacy image\n"); /* * Get the header. It will point to an address defined by * handoff which will tell where the image located inside - * the flash. For now, it will temporary fixed to address - * pointed by U-Boot. + * the flash. */ - header = (struct image_header *) - (CONFIG_SYS_TEXT_BASE - sizeof(struct image_header)); + debug("u_boot_pos = %lx\n", u_boot_pos); + if (u_boot_pos == BINMAN_SYM_MISSING) { + /* + * No binman support or no information. For now, fix it + * to the address pointed to by U-Boot. + */ + u_boot_pos = CONFIG_SYS_TEXT_BASE - + sizeof(struct image_header); + } + header = (struct image_header *)map_sysmem(u_boot_pos, 0);

spl_parse_image_header(spl_image, header); }

Add this feature to the README.

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

tools/binman/README | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/tools/binman/README b/tools/binman/README index 4ef76c8f089..08c3e56bdef 100644 --- a/tools/binman/README +++ b/tools/binman/README @@ -439,6 +439,8 @@ contents of an entry in some way. For example, it would be possible to create an entry containing a hash of the contents of some other entries. At this stage the position and size of entries should not be adjusted.

+6. WriteEntryInfo() + 7. BuildImage() - builds the image and writes it to a file. This is the final step.

@@ -471,6 +473,33 @@ the 'warning' line in scripts/Makefile.lib to see what it has found: # u_boot_dtsi_options_debug = $(u_boot_dtsi_options_raw)

+Access to binman entry positions at run time +-------------------------------------------- + +Binman assembles images and determines where each entry is placed in the image. +This information may be useful to U-Boot at run time. For example, in SPL it +is useful to be able to find the location of U-Boot so that it can be executed +when SPL is finished. + +Binman allows you to declare symbols in the SPL image which are filled in +with their correct values during the build. For example: + + binman_sym_declare(ulong, u_boot_any, pos); + +declares a ulong value which will be assigned to the position of any U-Boot +image (u-boot.bin, u-boot.img, u-boot-nodtb.bin) that is present in the image. +You can access this value with something like: + + ulong u_boot_pos = binman_sym(ulong, u_boot_any, pos); + +Thus u_boot_pos will be set to the position of U-Boot in memory, assuming that +the whole image has been loaded, or is available in flash. You can then jump to +that address to start U-Boot. + +At present this feature is only supported in SPL. In principle it is possible +to fill in such symbols in U-Boot proper, as well. + + Code coverage -------------

@@ -543,7 +572,8 @@ To do

Some ideas: - Fill out the device tree to include the final position and size of each - entry (since the input file may not always specify these) + entry (since the input file may not always specify these). See also + 'Access to binman entry positions at run time' above - Use of-platdata to make the information available to code that is unable to use device tree (such as a very small SPL image) - Write an image map to a text file