[U-Boot] [PATCH 00/34] binman: Various improvements and tidy-ups

This series includes a number of minor improvements to binman, including:
- Dropping some test Elf files and building them from source instead - Refactoring of x86 16-bit entries - Support for SPL symbols within sections - Handle the 'notes' sections and hidden symbols in recent binutils - Improved error reporting with a tool fails
Simon Glass (34): patman: Drop binary parameter patman: Update command.Run() to handle failure better binman: Use cls instead of self for class methods binman: Allow use of help and entry-docs without libfdt binman: Drop .note section from ELF binman: Handle hidden symbols in ELF files binman: Correct use of 'replace' in IFWI tests binman: Add support for an x86 'reset' section binman: x86: Separate out 16-bit reset and init code binman: Add support for Intel FIT binman: Fix IFWI output when using an Intel FIT image binman: Use tools.Run() to run objdump binman: Use the Makefile to build ELF test files binman: Use the Makefile for u_boot_ucode_ptr binman: Use the Makefile for u_boot_no_ucode_ptr binman: Use the Makefile for u_boot_binman_syms binman: Use the Makefile for u_boot_binman_syms_size binman: Use the Makefile for u_boot_binman_syms_bad binman: Clean up unnecessary code related to ELF test files binman: Allow symbols to be resolved inside sections binman: Use underscore in test filenames binman: Rename some two-digit test files binman: Avoid needing the section size in advance binman: Increase size of TPL and SPL test data binman: Allow support for writing a size symbol to binaries binman: Correct symbol calculation with non-zero image base binman: Add support for Intel FSP meminit binman: Fix entry comment for Intel descriptor binman: Update IFWI entry to read entries outside constructor binman: Update IFWI entry to support updates binman: Support writing symbols into entries within an IFWI binman: Write symbol info before image inclusion binman: Add logging for the number of pack passes binman: Drop comment-out code in testUpdateFdtOutput()
Makefile | 10 +- arch/x86/dts/u-boot.dtsi | 9 + scripts/Makefile.spl | 22 +- tools/binman/README.entries | 136 ++++++++-- tools/binman/control.py | 13 +- tools/binman/elf.py | 11 +- tools/binman/elf_test.py | 57 ++++- tools/binman/entry.py | 5 +- tools/binman/etype/blob.py | 1 - tools/binman/etype/blob_dtb.py | 6 +- tools/binman/etype/cbfs.py | 5 +- tools/binman/etype/fdtmap.py | 13 +- tools/binman/etype/files.py | 5 +- tools/binman/etype/intel_descriptor.py | 2 +- tools/binman/etype/intel_fit.py | 32 +++ tools/binman/etype/intel_fit_ptr.py | 41 +++ tools/binman/etype/intel_fsp_m.py | 27 ++ tools/binman/etype/intel_ifwi.py | 68 +++-- tools/binman/etype/section.py | 16 +- tools/binman/etype/u_boot_dtb_with_ucode.py | 5 +- tools/binman/etype/u_boot_spl.py | 2 +- tools/binman/etype/u_boot_tpl.py | 2 +- tools/binman/etype/x86_reset16.py | 29 +++ tools/binman/etype/x86_reset16_spl.py | 29 +++ tools/binman/etype/x86_reset16_tpl.py | 29 +++ tools/binman/etype/x86_start16.py | 15 +- tools/binman/etype/x86_start16_spl.py | 19 +- tools/binman/etype/x86_start16_tpl.py | 18 +- tools/binman/ftest.py | 241 ++++++++++++------ tools/binman/test/021_image_pad.dts | 2 +- tools/binman/test/024_sorted.dts | 2 +- tools/binman/test/028_pack_4gb_outside.dts | 2 +- .../test/{029_x86-rom.dts => 029_x86_rom.dts} | 2 +- ...no-desc.dts => 030_x86_rom_me_no_desc.dts} | 0 ...{031_x86-rom-me.dts => 031_x86_rom_me.dts} | 0 .../{032_intel-vga.dts => 032_intel_vga.dts} | 0 ...33_x86-start16.dts => 033_x86_start16.dts} | 0 .../{042_intel-fsp.dts => 042_intel_fsp.dts} | 0 .../{043_intel-cmc.dts => 043_intel_cmc.dts} | 0 .../{046_intel-vbt.dts => 046_intel_vbt.dts} | 0 ...tart16-spl.dts => 048_x86_start16_spl.dts} | 0 tools/binman/test/053_symbols.dts | 2 +- ...tart16-tpl.dts => 081_x86_start16_tpl.dts} | 0 ...=> 098_4gb_and_skip_at_start_together.dts} | 0 ..._x86-rom-ifwi.dts => 111_x86_rom_ifwi.dts} | 2 +- ...nodesc.dts => 112_x86_rom_ifwi_nodesc.dts} | 2 +- ...nodata.dts => 113_x86_rom_ifwi_nodata.dts} | 2 +- tools/binman/test/144_x86_reset16.dts | 13 + tools/binman/test/145_x86_reset16_spl.dts | 13 + tools/binman/test/146_x86_reset16_tpl.dts | 13 + tools/binman/test/147_intel_fit.dts | 20 ++ tools/binman/test/148_intel_fit_missing.dts | 17 ++ tools/binman/test/149_symbols_tpl.dts | 28 ++ ...> 150_powerpc_mpc85xx_bootpg_resetvec.dts} | 0 .../binman/test/151_x86_rom_ifwi_section.dts | 33 +++ tools/binman/test/152_intel_fsp_m.dts | 14 + tools/binman/test/Makefile | 14 +- tools/binman/test/bss_data | Bin 5020 -> 0 bytes tools/binman/test/u_boot_binman_syms | Bin 4924 -> 0 bytes tools/binman/test/u_boot_binman_syms.c | 1 + tools/binman/test/u_boot_binman_syms.lds | 3 +- tools/binman/test/u_boot_binman_syms_bad | Bin 4890 -> 0 bytes tools/binman/test/u_boot_binman_syms_size | Bin 4825 -> 0 bytes tools/binman/test/u_boot_no_ucode_ptr | Bin 4182 -> 0 bytes tools/binman/test/u_boot_ucode_ptr | Bin 4175 -> 0 bytes tools/binman/test/u_boot_ucode_ptr.lds | 3 +- tools/patman/cros_subprocess.py | 3 +- tools/patman/tools.py | 23 +- 68 files changed, 870 insertions(+), 212 deletions(-) create mode 100644 tools/binman/etype/intel_fit.py create mode 100644 tools/binman/etype/intel_fit_ptr.py create mode 100644 tools/binman/etype/intel_fsp_m.py create mode 100644 tools/binman/etype/x86_reset16.py create mode 100644 tools/binman/etype/x86_reset16_spl.py create mode 100644 tools/binman/etype/x86_reset16_tpl.py rename tools/binman/test/{029_x86-rom.dts => 029_x86_rom.dts} (87%) rename tools/binman/test/{030_x86-rom-me-no-desc.dts => 030_x86_rom_me_no_desc.dts} (100%) rename tools/binman/test/{031_x86-rom-me.dts => 031_x86_rom_me.dts} (100%) rename tools/binman/test/{032_intel-vga.dts => 032_intel_vga.dts} (100%) rename tools/binman/test/{033_x86-start16.dts => 033_x86_start16.dts} (100%) rename tools/binman/test/{042_intel-fsp.dts => 042_intel_fsp.dts} (100%) rename tools/binman/test/{043_intel-cmc.dts => 043_intel_cmc.dts} (100%) rename tools/binman/test/{046_intel-vbt.dts => 046_intel_vbt.dts} (100%) rename tools/binman/test/{048_x86-start16-spl.dts => 048_x86_start16_spl.dts} (100%) rename tools/binman/test/{081_x86-start16-tpl.dts => 081_x86_start16_tpl.dts} (100%) rename tools/binman/test/{80_4gb_and_skip_at_start_together.dts => 098_4gb_and_skip_at_start_together.dts} (100%) rename tools/binman/test/{111_x86-rom-ifwi.dts => 111_x86_rom_ifwi.dts} (95%) rename tools/binman/test/{112_x86-rom-ifwi-nodesc.dts => 112_x86_rom_ifwi_nodesc.dts} (95%) rename tools/binman/test/{113_x86-rom-ifwi-nodata.dts => 113_x86_rom_ifwi_nodata.dts} (95%) create mode 100644 tools/binman/test/144_x86_reset16.dts create mode 100644 tools/binman/test/145_x86_reset16_spl.dts create mode 100644 tools/binman/test/146_x86_reset16_tpl.dts create mode 100644 tools/binman/test/147_intel_fit.dts create mode 100644 tools/binman/test/148_intel_fit_missing.dts create mode 100644 tools/binman/test/149_symbols_tpl.dts rename tools/binman/test/{81_powerpc_mpc85xx_bootpg_resetvec.dts => 150_powerpc_mpc85xx_bootpg_resetvec.dts} (100%) create mode 100644 tools/binman/test/151_x86_rom_ifwi_section.dts create mode 100644 tools/binman/test/152_intel_fsp_m.dts delete mode 100755 tools/binman/test/bss_data delete mode 100755 tools/binman/test/u_boot_binman_syms delete mode 100755 tools/binman/test/u_boot_binman_syms_bad delete mode 100755 tools/binman/test/u_boot_binman_syms_size delete mode 100755 tools/binman/test/u_boot_no_ucode_ptr delete mode 100755 tools/binman/test/u_boot_ucode_ptr

Since cros_subprocess use bytestrings now, this feature not needed. Drop it.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/patman/cros_subprocess.py | 3 +-- tools/patman/tools.py | 15 +++++++-------- 2 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/tools/patman/cros_subprocess.py b/tools/patman/cros_subprocess.py index 06be64cc2c..0f0d60dfb7 100644 --- a/tools/patman/cros_subprocess.py +++ b/tools/patman/cros_subprocess.py @@ -54,7 +54,7 @@ class Popen(subprocess.Popen): """
def __init__(self, args, stdin=None, stdout=PIPE_PTY, stderr=PIPE_PTY, - shell=False, cwd=None, env=None, binary=False, **kwargs): + shell=False, cwd=None, env=None, **kwargs): """Cut-down constructor
Args: @@ -72,7 +72,6 @@ class Popen(subprocess.Popen): """ stdout_pty = None stderr_pty = None - self.binary = binary
if stdout == PIPE_PTY: stdout_pty = pty.openpty() diff --git a/tools/patman/tools.py b/tools/patman/tools.py index 0d4705db76..97441ca796 100644 --- a/tools/patman/tools.py +++ b/tools/patman/tools.py @@ -186,7 +186,7 @@ def PathHasFile(path_spec, fname): return True return False
-def Run(name, *args, **kwargs): +def Run(name, *args): """Run a tool with some arguments
This runs a 'tool', which is a program used by binman to process files and @@ -196,7 +196,6 @@ def Run(name, *args, **kwargs): Args: name: Command name to run args: Arguments to the tool - kwargs: Options to pass to command.run()
Returns: CommandResult object @@ -206,8 +205,8 @@ def Run(name, *args, **kwargs): if tool_search_paths: env = dict(os.environ) env['PATH'] = ':'.join(tool_search_paths) + ':' + env['PATH'] - return command.Run(name, *args, capture=True, - capture_stderr=True, env=env, **kwargs) + return command.Run(name, *args, capture=True, capture_stderr=True, + env=env) except: if env and not PathHasFile(env['PATH'], name): msg = "Please install tool '%s'" % name @@ -401,14 +400,14 @@ def Compress(indata, algo, with_header=True): fname = GetOutputFilename('%s.comp.tmp' % algo) WriteFile(fname, indata) if algo == 'lz4': - data = Run('lz4', '--no-frame-crc', '-c', fname, binary=True) + data = Run('lz4', '--no-frame-crc', '-c', fname) # cbfstool uses a very old version of lzma elif algo == 'lzma': outfname = GetOutputFilename('%s.comp.otmp' % algo) Run('lzma_alone', 'e', fname, outfname, '-lc1', '-lp0', '-pb0', '-d8') data = ReadFile(outfname) elif algo == 'gzip': - data = Run('gzip', '-c', fname, binary=True) + data = Run('gzip', '-c', fname) else: raise ValueError("Unknown algorithm '%s'" % algo) if with_header: @@ -441,13 +440,13 @@ def Decompress(indata, algo, with_header=True): with open(fname, 'wb') as fd: fd.write(indata) if algo == 'lz4': - data = Run('lz4', '-dc', fname, binary=True) + data = Run('lz4', '-dc', fname) elif algo == 'lzma': outfname = GetOutputFilename('%s.decomp.otmp' % algo) Run('lzma_alone', 'd', fname, outfname) data = ReadFile(outfname) elif algo == 'gzip': - data = Run('gzip', '-cd', fname, binary=True) + data = Run('gzip', '-cd', fname) else: raise ValueError("Unknown algorithm '%s'" % algo) return data

Since cros_subprocess use bytestrings now, this feature not needed. Drop it.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/patman/cros_subprocess.py | 3 +-- tools/patman/tools.py | 15 +++++++-------- 2 files changed, 8 insertions(+), 10 deletions(-)
Applied to u-boot-dm, thanks!

At present tools are not expected to fail. If they do an exception is raised but there is no detail about what went wrong. This makes it hard to debug if something does actually go wrong.
Fix this by outputting both stderr and stdout on failure.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/patman/tools.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/tools/patman/tools.py b/tools/patman/tools.py index 97441ca796..0952681579 100644 --- a/tools/patman/tools.py +++ b/tools/patman/tools.py @@ -205,8 +205,14 @@ def Run(name, *args): if tool_search_paths: env = dict(os.environ) env['PATH'] = ':'.join(tool_search_paths) + ':' + env['PATH'] - return command.Run(name, *args, capture=True, capture_stderr=True, - env=env) + all_args = (name,) + args + result = command.RunPipe([all_args], capture=True, capture_stderr=True, + env=env, raise_on_error=False) + if result.return_code: + raise Exception("Error %d running '%s': %s" % + (result.return_code,' '.join(all_args), + result.stderr)) + return result.stdout except: if env and not PathHasFile(env['PATH'], name): msg = "Please install tool '%s'" % name

At present tools are not expected to fail. If they do an exception is raised but there is no detail about what went wrong. This makes it hard to debug if something does actually go wrong.
Fix this by outputting both stderr and stdout on failure.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/patman/tools.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
Applied to u-boot-dm, thanks!

It is more common to use the name 'cls' for the class object of a class method, to distinguish it from normal methods, which use 'self' Update the binman tests accordingly.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/ftest.py | 54 +++++++++++++++++++++---------------------- 1 file changed, 27 insertions(+), 27 deletions(-)
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 0f3b70b3bb..acf361fa84 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -94,16 +94,16 @@ class TestFunctional(unittest.TestCase): the test/ diurectory. """ @classmethod - def setUpClass(self): + def setUpClass(cls): global entry import entry
# Handle the case where argv[0] is 'python' - self._binman_dir = os.path.dirname(os.path.realpath(sys.argv[0])) - self._binman_pathname = os.path.join(self._binman_dir, 'binman') + cls._binman_dir = os.path.dirname(os.path.realpath(sys.argv[0])) + cls._binman_pathname = os.path.join(cls._binman_dir, 'binman')
# Create a temporary directory for input files - self._indir = tempfile.mkdtemp(prefix='binmant.') + cls._indir = tempfile.mkdtemp(prefix='binmant.')
# Create some test files TestFunctional._MakeInputFile('u-boot.bin', U_BOOT_DATA) @@ -113,7 +113,7 @@ class TestFunctional(unittest.TestCase): TestFunctional._MakeInputFile('blobfile', BLOB_DATA) TestFunctional._MakeInputFile('me.bin', ME_DATA) TestFunctional._MakeInputFile('vga.bin', VGA_DATA) - self._ResetDtbs() + cls._ResetDtbs() TestFunctional._MakeInputFile('u-boot-x86-16bit.bin', X86_START16_DATA) TestFunctional._MakeInputFile('u-boot-br.bin', PPC_MPC85XX_BR_DATA) TestFunctional._MakeInputFile('spl/u-boot-x86-16bit-spl.bin', @@ -135,35 +135,35 @@ class TestFunctional(unittest.TestCase): TestFunctional._MakeInputFile('refcode.bin', REFCODE_DATA)
# ELF file with a '_dt_ucode_base_size' symbol - with open(self.TestFile('u_boot_ucode_ptr'), 'rb') as fd: + with open(cls.TestFile('u_boot_ucode_ptr'), 'rb') as fd: TestFunctional._MakeInputFile('u-boot', fd.read())
# Intel flash descriptor file - with open(self.TestFile('descriptor.bin'), 'rb') as fd: + with open(cls.TestFile('descriptor.bin'), 'rb') as fd: TestFunctional._MakeInputFile('descriptor.bin', fd.read())
- shutil.copytree(self.TestFile('files'), - os.path.join(self._indir, 'files')) + shutil.copytree(cls.TestFile('files'), + os.path.join(cls._indir, 'files'))
TestFunctional._MakeInputFile('compress', COMPRESS_DATA)
# Travis-CI may have an old lz4 - self.have_lz4 = True + cls.have_lz4 = True try: tools.Run('lz4', '--no-frame-crc', '-c', - os.path.join(self._indir, 'u-boot.bin')) + os.path.join(cls._indir, 'u-boot.bin')) except: - self.have_lz4 = False + cls.have_lz4 = False
@classmethod - def tearDownClass(self): + def tearDownClass(cls): """Remove the temporary input directory and its contents""" - if self.preserve_indir: - print('Preserving input dir: %s' % self._indir) + if cls.preserve_indir: + print('Preserving input dir: %s' % cls._indir) else: - if self._indir: - shutil.rmtree(self._indir) - self._indir = None + if cls._indir: + shutil.rmtree(cls._indir) + cls._indir = None
@classmethod def setup_test_args(cls, preserve_indir=False, preserve_outdirs=False, @@ -226,7 +226,7 @@ class TestFunctional(unittest.TestCase): return tmpdir, updated_fname
@classmethod - def _ResetDtbs(self): + def _ResetDtbs(cls): TestFunctional._MakeInputFile('u-boot.dtb', U_BOOT_DTB_DATA) TestFunctional._MakeInputFile('spl/u-boot-spl.dtb', U_BOOT_SPL_DTB_DATA) TestFunctional._MakeInputFile('tpl/u-boot-tpl.dtb', U_BOOT_TPL_DTB_DATA) @@ -432,7 +432,7 @@ class TestFunctional(unittest.TestCase): return self._DoReadFileDtb(fname, use_real_dtb)[0]
@classmethod - def _MakeInputFile(self, fname, contents): + def _MakeInputFile(cls, fname, contents): """Create a new test input file, creating directories as needed
Args: @@ -441,7 +441,7 @@ class TestFunctional(unittest.TestCase): Returns: Full pathname of file created """ - pathname = os.path.join(self._indir, fname) + pathname = os.path.join(cls._indir, fname) dirname = os.path.dirname(pathname) if dirname and not os.path.exists(dirname): os.makedirs(dirname) @@ -450,7 +450,7 @@ class TestFunctional(unittest.TestCase): return pathname
@classmethod - def _MakeInputDir(self, dirname): + def _MakeInputDir(cls, dirname): """Create a new test input directory, creating directories as needed
Args: @@ -459,24 +459,24 @@ class TestFunctional(unittest.TestCase): Returns: Full pathname of directory created """ - pathname = os.path.join(self._indir, dirname) + pathname = os.path.join(cls._indir, dirname) if not os.path.exists(pathname): os.makedirs(pathname) return pathname
@classmethod - def _SetupSplElf(self, src_fname='bss_data'): + def _SetupSplElf(cls, src_fname='bss_data'): """Set up an ELF file with a '_dt_ucode_base_size' symbol
Args: Filename of ELF file to use as SPL """ - with open(self.TestFile(src_fname), 'rb') as fd: + with open(cls.TestFile(src_fname), 'rb') as fd: TestFunctional._MakeInputFile('spl/u-boot-spl', fd.read())
@classmethod - def TestFile(self, fname): - return os.path.join(self._binman_dir, 'test', fname) + def TestFile(cls, fname): + return os.path.join(cls._binman_dir, 'test', fname)
def AssertInList(self, grep_list, target): """Assert that at least one of a list of things is in a target

It is more common to use the name 'cls' for the class object of a class method, to distinguish it from normal methods, which use 'self' Update the binman tests accordingly.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/ftest.py | 54 +++++++++++++++++++++---------------------- 1 file changed, 27 insertions(+), 27 deletions(-)
Applied to u-boot-dm, thanks!

At present if libfdt is not available binman can't do anything much. Improve the situation a little.
Ideally there should be a test to cover this, but I'm not quite sure how to fake this.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/control.py | 10 ++++++++-- tools/binman/entry.py | 5 ++++- tools/binman/etype/blob.py | 1 - tools/binman/etype/blob_dtb.py | 6 ++++-- tools/binman/etype/cbfs.py | 5 ++++- tools/binman/etype/fdtmap.py | 13 +++++++++---- tools/binman/etype/files.py | 5 ++++- tools/binman/etype/u_boot_dtb_with_ucode.py | 5 ++++- 8 files changed, 37 insertions(+), 13 deletions(-)
diff --git a/tools/binman/control.py b/tools/binman/control.py index 9e7587864c..43f5d49406 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -15,8 +15,6 @@ import tools import cbfs_util import command import elf -from image import Image -import state import tout
# List of images we plan to create @@ -459,6 +457,10 @@ def Binman(args): Args: args: Command line arguments Namespace object """ + # Put these here so that we can import this module without libfdt + global state + global Image + if args.full_help: pager = os.getenv('PAGER') if not pager: @@ -468,6 +470,10 @@ def Binman(args): command.Run(pager, fname) return 0
+ # Put these here so that we can import this module without libfdt + from image import Image + import state + if args.cmd == 'ls': try: tools.PrepareOutputDir(None) diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 6a2c6e0d92..04c26f9e50 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -21,7 +21,6 @@ import os import sys
import fdt_util -import state import tools from tools import ToHex, ToHexSize import tout @@ -71,6 +70,10 @@ class Entry(object): orig_size: Original size value read from node """ def __init__(self, section, etype, node, name_prefix=''): + # Put this here to allow entry-docs and help to work without libfdt + global state + import state + self.section = section self.etype = etype self._node = node diff --git a/tools/binman/etype/blob.py b/tools/binman/etype/blob.py index d15d0789e5..d34c7b51bf 100644 --- a/tools/binman/etype/blob.py +++ b/tools/binman/etype/blob.py @@ -7,7 +7,6 @@
from entry import Entry import fdt_util -import state import tools import tout
diff --git a/tools/binman/etype/blob_dtb.py b/tools/binman/etype/blob_dtb.py index 5b559967d7..b2afa064c1 100644 --- a/tools/binman/etype/blob_dtb.py +++ b/tools/binman/etype/blob_dtb.py @@ -5,8 +5,6 @@ # Entry-type module for U-Boot device tree files #
-import state - from entry import Entry from blob import Entry_blob
@@ -18,6 +16,10 @@ class Entry_blob_dtb(Entry_blob): 'state' module. """ def __init__(self, section, etype, node): + # Put this here to allow entry-docs and help to work without libfdt + global state + import state + Entry_blob.__init__(self, section, etype, node)
def ObtainContents(self): diff --git a/tools/binman/etype/cbfs.py b/tools/binman/etype/cbfs.py index 28a9c81a8a..35b78370b2 100644 --- a/tools/binman/etype/cbfs.py +++ b/tools/binman/etype/cbfs.py @@ -11,7 +11,6 @@ import cbfs_util from cbfs_util import CbfsWriter from entry import Entry import fdt_util -import state
class Entry_cbfs(Entry): """Entry containing a Coreboot Filesystem (CBFS) @@ -164,6 +163,10 @@ class Entry_cbfs(Entry): both of size 1MB. """ def __init__(self, section, etype, node): + # Put this here to allow entry-docs and help to work without libfdt + global state + import state + Entry.__init__(self, section, etype, node) self._cbfs_arg = fdt_util.GetString(node, 'cbfs-arch', 'x86') self._cbfs_entries = OrderedDict() diff --git a/tools/binman/etype/fdtmap.py b/tools/binman/etype/fdtmap.py index b1810b9ddb..5dc08b8289 100644 --- a/tools/binman/etype/fdtmap.py +++ b/tools/binman/etype/fdtmap.py @@ -8,11 +8,7 @@ This handles putting an FDT into the image with just the information about the image. """
-import libfdt - from entry import Entry -from fdt import Fdt -import state import tools import tout
@@ -80,6 +76,15 @@ class Entry_fdtmap(Entry): added as necessary. See the binman README. """ def __init__(self, section, etype, node): + # Put these here to allow entry-docs and help to work without libfdt + global libfdt + global state + global Fdt + + import libfdt + import state + from fdt import Fdt + Entry.__init__(self, section, etype, node)
def _GetFdtmap(self): diff --git a/tools/binman/etype/files.py b/tools/binman/etype/files.py index 0068b305f7..3473a2b1ef 100644 --- a/tools/binman/etype/files.py +++ b/tools/binman/etype/files.py @@ -11,7 +11,6 @@ import os
from section import Entry_section import fdt_util -import state import tools
@@ -29,6 +28,10 @@ class Entry_files(Entry_section): at run-time so you can obtain the file positions. """ def __init__(self, section, etype, node): + # Put this here to allow entry-docs and help to work without libfdt + global state + import state + Entry_section.__init__(self, section, etype, node) self._pattern = fdt_util.GetString(self._node, 'pattern') if not self._pattern: diff --git a/tools/binman/etype/u_boot_dtb_with_ucode.py b/tools/binman/etype/u_boot_dtb_with_ucode.py index cb6c3730d7..6efd24a9b3 100644 --- a/tools/binman/etype/u_boot_dtb_with_ucode.py +++ b/tools/binman/etype/u_boot_dtb_with_ucode.py @@ -7,7 +7,6 @@
from entry import Entry from blob_dtb import Entry_blob_dtb -import state import tools
class Entry_u_boot_dtb_with_ucode(Entry_blob_dtb): @@ -25,6 +24,10 @@ class Entry_u_boot_dtb_with_ucode(Entry_blob_dtb): it available to u_boot_ucode. """ def __init__(self, section, etype, node): + # Put this here to allow entry-docs and help to work without libfdt + global state + import state + Entry_blob_dtb.__init__(self, section, etype, node) self.ucode_data = b'' self.collate = False

At present if libfdt is not available binman can't do anything much. Improve the situation a little.
Ideally there should be a test to cover this, but I'm not quite sure how to fake this.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/control.py | 10 ++++++++-- tools/binman/entry.py | 5 ++++- tools/binman/etype/blob.py | 1 - tools/binman/etype/blob_dtb.py | 6 ++++-- tools/binman/etype/cbfs.py | 5 ++++- tools/binman/etype/fdtmap.py | 13 +++++++++---- tools/binman/etype/files.py | 5 ++++- tools/binman/etype/u_boot_dtb_with_ucode.py | 5 ++++- 8 files changed, 37 insertions(+), 13 deletions(-)
Applied to u-boot-dm, thanks!

Recent versions of binutils add a '.note.gnu.property' into the ELF file. This is not required and interferes with the expected output. Drop it.
Also fix testMakeElf() to use a different file for input and output.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/elf.py | 3 +++ tools/binman/elf_test.py | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/tools/binman/elf.py b/tools/binman/elf.py index af40024cea..c7ef74ce7d 100644 --- a/tools/binman/elf.py +++ b/tools/binman/elf.py @@ -221,6 +221,9 @@ SECTIONS .empty : { *(.empty) } :empty + /DISCARD/ : { + *(.note.gnu.property) + } .note : { *(.comment) } :note diff --git a/tools/binman/elf_test.py b/tools/binman/elf_test.py index 416e43baf0..cc6e9c5128 100644 --- a/tools/binman/elf_test.py +++ b/tools/binman/elf_test.py @@ -148,7 +148,7 @@ class TestElf(unittest.TestCase): expected_text = b'1234' expected_data = b'wxyz' elf_fname = os.path.join(outdir, 'elf') - bin_fname = os.path.join(outdir, 'elf') + bin_fname = os.path.join(outdir, 'bin')
# Make an Elf file and then convert it to a fkat binary file. This # should produce the original data.

Recent versions of binutils add a '.note.gnu.property' into the ELF file. This is not required and interferes with the expected output. Drop it.
Also fix testMakeElf() to use a different file for input and output.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/elf.py | 3 +++ tools/binman/elf_test.py | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-)
Applied to u-boot-dm, thanks!

Some versions of binutils generate hidden symbols which are currently not parsed by binman. Correct this.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/elf.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/binman/elf.py b/tools/binman/elf.py index c7ef74ce7d..66cfe796a2 100644 --- a/tools/binman/elf.py +++ b/tools/binman/elf.py @@ -72,7 +72,7 @@ def GetSymbols(fname, patterns): parts = rest[7:].split() section, size = parts[:2] if len(parts) > 2: - name = parts[2] + name = parts[2] if parts[2] != '.hidden' else parts[3] syms[name] = Symbol(section, int(value, 16), int(size,16), flags[1] == 'w')

Some versions of binutils generate hidden symbols which are currently not parsed by binman. Correct this.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/elf.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Applied to u-boot-dm, thanks!

At present the Intel IFWI entry uses 'replace' without the 'ifwi-' prefix. This is a fairly generic name which might conflict with the main Entry base class at some point, if more features are added. Add a prefix.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/README.entries | 6 ++++++ tools/binman/etype/intel_ifwi.py | 8 +++++++- tools/binman/test/111_x86-rom-ifwi.dts | 2 +- tools/binman/test/112_x86-rom-ifwi-nodesc.dts | 2 +- tools/binman/test/113_x86-rom-ifwi-nodata.dts | 2 +- 5 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/tools/binman/README.entries b/tools/binman/README.entries index 0f0e367d02..11c55fd8c8 100644 --- a/tools/binman/README.entries +++ b/tools/binman/README.entries @@ -432,6 +432,12 @@ The contents of the IFWI are specified by the subnodes of the IFWI node. Each subnode describes an entry which is placed into the IFWFI with a given sub-partition (and optional entry name).
+Properties for subnodes: + ifwi-subpart - sub-parition to put this entry into, e.g. "IBBP" + ifwi-entry - entry name t use, e.g. "IBBL" + ifwi-replace - if present, indicates that the item should be replaced + in the IFWI. Otherwise it is added. + See README.x86 for information about x86 binary blobs.
diff --git a/tools/binman/etype/intel_ifwi.py b/tools/binman/etype/intel_ifwi.py index 9cbdf3698a..f3745f7a8c 100644 --- a/tools/binman/etype/intel_ifwi.py +++ b/tools/binman/etype/intel_ifwi.py @@ -36,6 +36,12 @@ class Entry_intel_ifwi(Entry_blob): Each subnode describes an entry which is placed into the IFWFI with a given sub-partition (and optional entry name).
+ Properties for subnodes: + ifwi-subpart - sub-parition to put this entry into, e.g. "IBBP" + ifwi-entry - entry name t use, e.g. "IBBL" + ifwi-replace - if present, indicates that the item should be replaced + in the IFWI. Otherwise it is added. + See README.x86 for information about x86 binary blobs. """ def __init__(self, section, etype, node): @@ -95,7 +101,7 @@ class Entry_intel_ifwi(Entry_blob): for node in self._node.subnodes: entry = Entry.Create(self.section, node) entry.ReadNode() - entry._ifwi_replace = fdt_util.GetBool(node, 'replace') + entry._ifwi_replace = fdt_util.GetBool(node, 'ifwi-replace') entry._ifwi_subpart = fdt_util.GetString(node, 'ifwi-subpart') entry._ifwi_entry_name = fdt_util.GetString(node, 'ifwi-entry') self._ifwi_entries[entry._ifwi_subpart] = entry diff --git a/tools/binman/test/111_x86-rom-ifwi.dts b/tools/binman/test/111_x86-rom-ifwi.dts index 63b5972cc8..c0ba4f2ea4 100644 --- a/tools/binman/test/111_x86-rom-ifwi.dts +++ b/tools/binman/test/111_x86-rom-ifwi.dts @@ -20,7 +20,7 @@ convert-fit;
u-boot-tpl { - replace; + ifwi-replace; ifwi-subpart = "IBBP"; ifwi-entry = "IBBL"; }; diff --git a/tools/binman/test/112_x86-rom-ifwi-nodesc.dts b/tools/binman/test/112_x86-rom-ifwi-nodesc.dts index 21ec4654ff..0874440ab5 100644 --- a/tools/binman/test/112_x86-rom-ifwi-nodesc.dts +++ b/tools/binman/test/112_x86-rom-ifwi-nodesc.dts @@ -19,7 +19,7 @@ filename = "ifwi.bin";
u-boot-tpl { - replace; + ifwi-replace; ifwi-subpart = "IBBP"; ifwi-entry = "IBBL"; }; diff --git a/tools/binman/test/113_x86-rom-ifwi-nodata.dts b/tools/binman/test/113_x86-rom-ifwi-nodata.dts index 62486fd990..82a4bc8cdd 100644 --- a/tools/binman/test/113_x86-rom-ifwi-nodata.dts +++ b/tools/binman/test/113_x86-rom-ifwi-nodata.dts @@ -20,7 +20,7 @@
_testing { return-unknown-contents; - replace; + ifwi-replace; ifwi-subpart = "IBBP"; ifwi-entry = "IBBL"; };

At present the Intel IFWI entry uses 'replace' without the 'ifwi-' prefix. This is a fairly generic name which might conflict with the main Entry base class at some point, if more features are added. Add a prefix.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/README.entries | 6 ++++++ tools/binman/etype/intel_ifwi.py | 8 +++++++- tools/binman/test/111_x86-rom-ifwi.dts | 2 +- tools/binman/test/112_x86-rom-ifwi-nodesc.dts | 2 +- tools/binman/test/113_x86-rom-ifwi-nodata.dts | 2 +- 5 files changed, 16 insertions(+), 4 deletions(-)
Applied to u-boot-dm, thanks!

At present binman has a single entry type for the 16-bit code code needed to start up an x86 processor. This entry is intended to include both the reset vector itself as well as the code to move to 32-bit mode.
However this is not very flexible since in some cases other data needs to be included at the top of the SPI flash, in between these two pieces. For example Intel requires that a FIT (Firmware Image Table) pointer be placed 0x40 bytes before the end of the ROM.
To deal with this, add a new reset entry for just the reset vector. A subsequent change will adjust the existing 'start16' entry.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/README.entries | 48 +++++++++++++++++++++++ tools/binman/etype/x86_reset16.py | 29 ++++++++++++++ tools/binman/etype/x86_reset16_spl.py | 29 ++++++++++++++ tools/binman/etype/x86_reset16_tpl.py | 29 ++++++++++++++ tools/binman/ftest.py | 30 +++++++++++++- tools/binman/test/144_x86_reset16.dts | 13 ++++++ tools/binman/test/145_x86_reset16_spl.dts | 13 ++++++ tools/binman/test/146_x86_reset16_tpl.dts | 13 ++++++ 8 files changed, 203 insertions(+), 1 deletion(-) create mode 100644 tools/binman/etype/x86_reset16.py create mode 100644 tools/binman/etype/x86_reset16_spl.py create mode 100644 tools/binman/etype/x86_reset16_tpl.py create mode 100644 tools/binman/test/144_x86_reset16.dts create mode 100644 tools/binman/test/145_x86_reset16_spl.dts create mode 100644 tools/binman/test/146_x86_reset16_tpl.dts
diff --git a/tools/binman/README.entries b/tools/binman/README.entries index 11c55fd8c8..55e3fa0dcc 100644 --- a/tools/binman/README.entries +++ b/tools/binman/README.entries @@ -937,6 +937,54 @@ and kernel are genuine.
+Entry: x86-reset16: x86 16-bit reset code for U-Boot +---------------------------------------------------- + +Properties / Entry arguments: + - filename: Filename of u-boot-x86-reset16.bin (default + 'u-boot-x86-reset16.bin') + +x86 CPUs start up in 16-bit mode, even if they are 32-bit CPUs. This code +must be placed at a particular address. This entry holds that code. It is +typically placed at offset CONFIG_RESET_VEC_LOC. The code is responsible +for jumping to the x86-start16 code, which continues execution. + +For 64-bit U-Boot, the 'x86_reset16_spl' entry type is used instead. + + + +Entry: x86-reset16-spl: x86 16-bit reset code for U-Boot +-------------------------------------------------------- + +Properties / Entry arguments: + - filename: Filename of u-boot-x86-reset16.bin (default + 'u-boot-x86-reset16.bin') + +x86 CPUs start up in 16-bit mode, even if they are 32-bit CPUs. This code +must be placed at a particular address. This entry holds that code. It is +typically placed at offset CONFIG_RESET_VEC_LOC. The code is responsible +for jumping to the x86-start16 code, which continues execution. + +For 32-bit U-Boot, the 'x86_reset_spl' entry type is used instead. + + + +Entry: x86-reset16-tpl: x86 16-bit reset code for U-Boot +-------------------------------------------------------- + +Properties / Entry arguments: + - filename: Filename of u-boot-x86-reset16.bin (default + 'u-boot-x86-reset16.bin') + +x86 CPUs start up in 16-bit mode, even if they are 32-bit CPUs. This code +must be placed at a particular address. This entry holds that code. It is +typically placed at offset CONFIG_RESET_VEC_LOC. The code is responsible +for jumping to the x86-start16 code, which continues execution. + +For 32-bit U-Boot, the 'x86_reset_tpl' entry type is used instead. + + + Entry: x86-start16: x86 16-bit start-up code for U-Boot -------------------------------------------------------
diff --git a/tools/binman/etype/x86_reset16.py b/tools/binman/etype/x86_reset16.py new file mode 100644 index 0000000000..54eb814ea3 --- /dev/null +++ b/tools/binman/etype/x86_reset16.py @@ -0,0 +1,29 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright (c) 2016 Google, Inc +# Written by Simon Glass sjg@chromium.org +# +# Entry-type module for the 16-bit x86 reset code for U-Boot +# + +from entry import Entry +from blob import Entry_blob + +class Entry_x86_reset16(Entry_blob): + """x86 16-bit reset code for U-Boot + + Properties / Entry arguments: + - filename: Filename of u-boot-x86-reset16.bin (default + 'u-boot-x86-reset16.bin') + + x86 CPUs start up in 16-bit mode, even if they are 32-bit CPUs. This code + must be placed at a particular address. This entry holds that code. It is + typically placed at offset CONFIG_RESET_VEC_LOC. The code is responsible + for jumping to the x86-start16 code, which continues execution. + + For 64-bit U-Boot, the 'x86_reset16_spl' entry type is used instead. + """ + def __init__(self, section, etype, node): + Entry_blob.__init__(self, section, etype, node) + + def GetDefaultFilename(self): + return 'u-boot-x86-reset16.bin' diff --git a/tools/binman/etype/x86_reset16_spl.py b/tools/binman/etype/x86_reset16_spl.py new file mode 100644 index 0000000000..699a0c6bcb --- /dev/null +++ b/tools/binman/etype/x86_reset16_spl.py @@ -0,0 +1,29 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright (c) 2016 Google, Inc +# Written by Simon Glass sjg@chromium.org +# +# Entry-type module for the 16-bit x86 reset code for U-Boot +# + +from entry import Entry +from blob import Entry_blob + +class Entry_x86_reset16_spl(Entry_blob): + """x86 16-bit reset code for U-Boot + + Properties / Entry arguments: + - filename: Filename of u-boot-x86-reset16.bin (default + 'u-boot-x86-reset16.bin') + + x86 CPUs start up in 16-bit mode, even if they are 32-bit CPUs. This code + must be placed at a particular address. This entry holds that code. It is + typically placed at offset CONFIG_RESET_VEC_LOC. The code is responsible + for jumping to the x86-start16 code, which continues execution. + + For 32-bit U-Boot, the 'x86_reset_spl' entry type is used instead. + """ + def __init__(self, section, etype, node): + Entry_blob.__init__(self, section, etype, node) + + def GetDefaultFilename(self): + return 'spl/u-boot-x86-reset16-spl.bin' diff --git a/tools/binman/etype/x86_reset16_tpl.py b/tools/binman/etype/x86_reset16_tpl.py new file mode 100644 index 0000000000..4eedb8d601 --- /dev/null +++ b/tools/binman/etype/x86_reset16_tpl.py @@ -0,0 +1,29 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright (c) 2016 Google, Inc +# Written by Simon Glass sjg@chromium.org +# +# Entry-type module for the 16-bit x86 reset code for U-Boot +# + +from entry import Entry +from blob import Entry_blob + +class Entry_x86_reset16_tpl(Entry_blob): + """x86 16-bit reset code for U-Boot + + Properties / Entry arguments: + - filename: Filename of u-boot-x86-reset16.bin (default + 'u-boot-x86-reset16.bin') + + x86 CPUs start up in 16-bit mode, even if they are 32-bit CPUs. This code + must be placed at a particular address. This entry holds that code. It is + typically placed at offset CONFIG_RESET_VEC_LOC. The code is responsible + for jumping to the x86-start16 code, which continues execution. + + For 32-bit U-Boot, the 'x86_reset_tpl' entry type is used instead. + """ + def __init__(self, section, etype, node): + Entry_blob.__init__(self, section, etype, node) + + def GetDefaultFilename(self): + return 'tpl/u-boot-x86-reset16-tpl.bin' diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index acf361fa84..77445814a7 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -49,6 +49,9 @@ U_BOOT_TPL_DTB_DATA = b'tpldtb' X86_START16_DATA = b'start16' X86_START16_SPL_DATA = b'start16spl' X86_START16_TPL_DATA = b'start16tpl' +X86_RESET16_DATA = b'reset16' +X86_RESET16_SPL_DATA = b'reset16spl' +X86_RESET16_TPL_DATA = b'reset16tpl' PPC_MPC85XX_BR_DATA = b'ppcmpc85xxbr' U_BOOT_NODTB_DATA = b'nodtb with microcode pointer somewhere in here' U_BOOT_SPL_NODTB_DATA = b'splnodtb with microcode pointer somewhere in here' @@ -114,12 +117,22 @@ class TestFunctional(unittest.TestCase): TestFunctional._MakeInputFile('me.bin', ME_DATA) TestFunctional._MakeInputFile('vga.bin', VGA_DATA) cls._ResetDtbs() - TestFunctional._MakeInputFile('u-boot-x86-16bit.bin', X86_START16_DATA) + TestFunctional._MakeInputFile('u-boot-br.bin', PPC_MPC85XX_BR_DATA) + + TestFunctional._MakeInputFile('u-boot-x86-16bit.bin', X86_START16_DATA) TestFunctional._MakeInputFile('spl/u-boot-x86-16bit-spl.bin', X86_START16_SPL_DATA) TestFunctional._MakeInputFile('tpl/u-boot-x86-16bit-tpl.bin', X86_START16_TPL_DATA) + + TestFunctional._MakeInputFile('u-boot-x86-reset16.bin', + X86_RESET16_DATA) + TestFunctional._MakeInputFile('spl/u-boot-x86-reset16-spl.bin', + X86_RESET16_SPL_DATA) + TestFunctional._MakeInputFile('tpl/u-boot-x86-reset16-tpl.bin', + X86_RESET16_TPL_DATA) + TestFunctional._MakeInputFile('u-boot-nodtb.bin', U_BOOT_NODTB_DATA) TestFunctional._MakeInputFile('spl/u-boot-spl-nodtb.bin', U_BOOT_SPL_NODTB_DATA) @@ -3236,6 +3249,21 @@ class TestFunctional(unittest.TestCase): self.assertIn('Must specify exactly one entry path to write with -f', str(e.exception))
+ def testPackReset16(self): + """Test that an image with an x86 reset16 region can be created""" + data = self._DoReadFile('144_x86_reset16.dts') + self.assertEqual(X86_RESET16_DATA, data[:len(X86_RESET16_DATA)]) + + def testPackReset16Spl(self): + """Test that an image with an x86 reset16-spl region can be created""" + data = self._DoReadFile('145_x86_reset16_spl.dts') + self.assertEqual(X86_RESET16_SPL_DATA, data[:len(X86_RESET16_SPL_DATA)]) + + def testPackReset16Tpl(self): + """Test that an image with an x86 reset16-tpl region can be created""" + data = self._DoReadFile('146_x86_reset16_tpl.dts') + self.assertEqual(X86_RESET16_TPL_DATA, data[:len(X86_RESET16_TPL_DATA)]) +
if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/144_x86_reset16.dts b/tools/binman/test/144_x86_reset16.dts new file mode 100644 index 0000000000..ba90333b27 --- /dev/null +++ b/tools/binman/test/144_x86_reset16.dts @@ -0,0 +1,13 @@ +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + size = <16>; + + x86-reset16 { + }; + }; +}; diff --git a/tools/binman/test/145_x86_reset16_spl.dts b/tools/binman/test/145_x86_reset16_spl.dts new file mode 100644 index 0000000000..cc8d97a7e6 --- /dev/null +++ b/tools/binman/test/145_x86_reset16_spl.dts @@ -0,0 +1,13 @@ +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + size = <16>; + + x86-reset16-spl { + }; + }; +}; diff --git a/tools/binman/test/146_x86_reset16_tpl.dts b/tools/binman/test/146_x86_reset16_tpl.dts new file mode 100644 index 0000000000..041b16f3de --- /dev/null +++ b/tools/binman/test/146_x86_reset16_tpl.dts @@ -0,0 +1,13 @@ +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + size = <16>; + + x86-reset16-tpl { + }; + }; +};

At present binman has a single entry type for the 16-bit code code needed to start up an x86 processor. This entry is intended to include both the reset vector itself as well as the code to move to 32-bit mode.
However this is not very flexible since in some cases other data needs to be included at the top of the SPI flash, in between these two pieces. For example Intel requires that a FIT (Firmware Image Table) pointer be placed 0x40 bytes before the end of the ROM.
To deal with this, add a new reset entry for just the reset vector. A subsequent change will adjust the existing 'start16' entry.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/README.entries | 48 +++++++++++++++++++++++ tools/binman/etype/x86_reset16.py | 29 ++++++++++++++ tools/binman/etype/x86_reset16_spl.py | 29 ++++++++++++++ tools/binman/etype/x86_reset16_tpl.py | 29 ++++++++++++++ tools/binman/ftest.py | 30 +++++++++++++- tools/binman/test/144_x86_reset16.dts | 13 ++++++ tools/binman/test/145_x86_reset16_spl.dts | 13 ++++++ tools/binman/test/146_x86_reset16_tpl.dts | 13 ++++++ 8 files changed, 203 insertions(+), 1 deletion(-) create mode 100644 tools/binman/etype/x86_reset16.py create mode 100644 tools/binman/etype/x86_reset16_spl.py create mode 100644 tools/binman/etype/x86_reset16_tpl.py create mode 100644 tools/binman/test/144_x86_reset16.dts create mode 100644 tools/binman/test/145_x86_reset16_spl.dts create mode 100644 tools/binman/test/146_x86_reset16_tpl.dts
Applied to u-boot-dm, thanks!

At present these two sections of code are linked together into a single 2KB chunk in a single file. Some Intel SoCs like to have a FIT (Firmware Interface Table) in the ROM and the pointer for this needs to go at 0xffffffc0 which is in the middle of these two sections.
Make use of the new 'reset' entry and change the existing 16-bit entry to include just the 16-bit data.
Signed-off-by: Simon Glass sjg@chromium.org ---
Makefile | 10 ++++-- arch/x86/dts/u-boot.dtsi | 9 ++++++ scripts/Makefile.spl | 22 +++++++++---- tools/binman/README.entries | 46 +++++++++++++++------------ tools/binman/etype/x86_start16.py | 15 +++++---- tools/binman/etype/x86_start16_spl.py | 19 +++++------ tools/binman/etype/x86_start16_tpl.py | 18 ++++++----- tools/binman/ftest.py | 6 ++-- 8 files changed, 88 insertions(+), 57 deletions(-)
diff --git a/Makefile b/Makefile index 3b0864ae8e..df4a54e204 100644 --- a/Makefile +++ b/Makefile @@ -1402,14 +1402,18 @@ quiet_cmd_ldr = LD $@ cmd_ldr = $(LD) $(LDFLAGS_$(@F)) \ $(filter-out FORCE,$^) -o $@
-u-boot.rom: u-boot-x86-16bit.bin u-boot.bin \ +u-boot.rom: u-boot-x86-start16.bin u-boot-x86-reset16.bin u-boot.bin \ $(if $(CONFIG_SPL_X86_16BIT_INIT),spl/u-boot-spl.bin) \ $(if $(CONFIG_TPL_X86_16BIT_INIT),tpl/u-boot-tpl.bin) \ $(if $(CONFIG_HAVE_REFCODE),refcode.bin) FORCE $(call if_changed,binman)
-OBJCOPYFLAGS_u-boot-x86-16bit.bin := -O binary -j .start16 -j .resetvec -u-boot-x86-16bit.bin: u-boot FORCE +OBJCOPYFLAGS_u-boot-x86-start16.bin := -O binary -j .start16 +u-boot-x86-start16.bin: u-boot FORCE + $(call if_changed,objcopy) + +OBJCOPYFLAGS_u-boot-x86-reset16.bin := -O binary -j .resetvec +u-boot-x86-reset16.bin: u-boot FORCE $(call if_changed,objcopy) endif
diff --git a/arch/x86/dts/u-boot.dtsi b/arch/x86/dts/u-boot.dtsi index daeb168b65..0e87b88e10 100644 --- a/arch/x86/dts/u-boot.dtsi +++ b/arch/x86/dts/u-boot.dtsi @@ -120,14 +120,23 @@ x86-start16-tpl { offset = <CONFIG_SYS_X86_START16>; }; + x86-reset16-tpl { + offset = <CONFIG_RESET_VEC_LOC>; + }; #elif defined(CONFIG_SPL) x86-start16-spl { offset = <CONFIG_SYS_X86_START16>; }; + x86-reset16-spl { + offset = <CONFIG_RESET_VEC_LOC>; + }; #else x86-start16 { offset = <CONFIG_SYS_X86_START16>; }; + x86-reset16 { + offset = <CONFIG_RESET_VEC_LOC>; + }; #endif }; #endif diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl index 7af6b120b6..0f3d89b215 100644 --- a/scripts/Makefile.spl +++ b/scripts/Makefile.spl @@ -229,9 +229,11 @@ ALL-y += $(obj)/boot.bin endif
ifdef CONFIG_TPL_BUILD -ALL-$(CONFIG_TPL_X86_16BIT_INIT) += $(obj)/u-boot-x86-16bit-tpl.bin +ALL-$(CONFIG_TPL_X86_16BIT_INIT) += $(obj)/u-boot-x86-start16-tpl.bin \ + $(obj)/u-boot-x86-reset16-tpl.bin else -ALL-$(CONFIG_SPL_X86_16BIT_INIT) += $(obj)/u-boot-x86-16bit-spl.bin +ALL-$(CONFIG_SPL_X86_16BIT_INIT) += $(obj)/u-boot-x86-start16-spl.bin \ + $(obj)/u-boot-x86-reset16-spl.bin endif
ALL-$(CONFIG_ARCH_ZYNQ) += $(obj)/boot.bin @@ -337,12 +339,20 @@ OBJCOPYFLAGS_$(SPL_BIN)-nodtb.bin = $(SPL_OBJCFLAGS) -O binary \ $(obj)/$(SPL_BIN)-nodtb.bin: $(obj)/$(SPL_BIN) FORCE $(call if_changed,objcopy)
-OBJCOPYFLAGS_u-boot-x86-16bit-spl.bin := -O binary -j .start16 -j .resetvec -$(obj)/u-boot-x86-16bit-spl.bin: $(obj)/u-boot-spl FORCE +OBJCOPYFLAGS_u-boot-x86-start16-spl.bin := -O binary -j .start16 +$(obj)/u-boot-x86-start16-spl.bin: $(obj)/u-boot-spl FORCE $(call if_changed,objcopy)
-OBJCOPYFLAGS_u-boot-x86-16bit-tpl.bin := -O binary -j .start16 -j .resetvec -$(obj)/u-boot-x86-16bit-tpl.bin: $(obj)/u-boot-tpl FORCE +OBJCOPYFLAGS_u-boot-x86-start16-tpl.bin := -O binary -j .start16 +$(obj)/u-boot-x86-start16-tpl.bin: $(obj)/u-boot-tpl FORCE + $(call if_changed,objcopy) + +OBJCOPYFLAGS_u-boot-x86-reset16-spl.bin := -O binary -j .resetvec +$(obj)/u-boot-x86-reset16-spl.bin: $(obj)/u-boot-spl FORCE + $(call if_changed,objcopy) + +OBJCOPYFLAGS_u-boot-x86-reset16-tpl.bin := -O binary -j .resetvec +$(obj)/u-boot-x86-reset16-tpl.bin: $(obj)/u-boot-tpl FORCE $(call if_changed,objcopy)
LDFLAGS_$(SPL_BIN) += -T u-boot-spl.lds $(LDFLAGS_FINAL) diff --git a/tools/binman/README.entries b/tools/binman/README.entries index 55e3fa0dcc..d17b3cb078 100644 --- a/tools/binman/README.entries +++ b/tools/binman/README.entries @@ -989,14 +989,15 @@ Entry: x86-start16: x86 16-bit start-up code for U-Boot -------------------------------------------------------
Properties / Entry arguments: - - filename: Filename of u-boot-x86-16bit.bin (default - 'u-boot-x86-16bit.bin') + - filename: Filename of u-boot-x86-start16.bin (default + 'u-boot-x86-start16.bin')
x86 CPUs start up in 16-bit mode, even if they are 32-bit CPUs. This code -must be placed at a particular address. This entry holds that code. It is -typically placed at offset CONFIG_SYS_X86_START16. The code is responsible -for changing to 32-bit mode and jumping to U-Boot's entry point, which -requires 32-bit mode (for 32-bit U-Boot). +must be placed in the top 64KB of the ROM. The reset code jumps to it. This +entry holds that code. It is typically placed at offset +CONFIG_SYS_X86_START16. The code is responsible for changing to 32-bit mode +and jumping to U-Boot's entry point, which requires 32-bit mode (for 32-bit +U-Boot).
For 64-bit U-Boot, the 'x86_start16_spl' entry type is used instead.
@@ -1006,16 +1007,17 @@ Entry: x86-start16-spl: x86 16-bit start-up code for SPL --------------------------------------------------------
Properties / Entry arguments: - - filename: Filename of spl/u-boot-x86-16bit-spl.bin (default - 'spl/u-boot-x86-16bit-spl.bin') + - filename: Filename of spl/u-boot-x86-start16-spl.bin (default + 'spl/u-boot-x86-start16-spl.bin')
-x86 CPUs start up in 16-bit mode, even if they are 64-bit CPUs. This code -must be placed at a particular address. This entry holds that code. It is -typically placed at offset CONFIG_SYS_X86_START16. The code is responsible -for changing to 32-bit mode and starting SPL, which in turn changes to -64-bit mode and jumps to U-Boot (for 64-bit U-Boot). +x86 CPUs start up in 16-bit mode, even if they are 32-bit CPUs. This code +must be placed in the top 64KB of the ROM. The reset code jumps to it. This +entry holds that code. It is typically placed at offset +CONFIG_SYS_X86_START16. The code is responsible for changing to 32-bit mode +and jumping to U-Boot's entry point, which requires 32-bit mode (for 32-bit +U-Boot).
-For 32-bit U-Boot, the 'x86_start16' entry type is used instead. +For 32-bit U-Boot, the 'x86-start16' entry type is used instead.
@@ -1023,15 +1025,17 @@ Entry: x86-start16-tpl: x86 16-bit start-up code for TPL --------------------------------------------------------
Properties / Entry arguments: - - filename: Filename of tpl/u-boot-x86-16bit-tpl.bin (default - 'tpl/u-boot-x86-16bit-tpl.bin') + - filename: Filename of tpl/u-boot-x86-start16-tpl.bin (default + 'tpl/u-boot-x86-start16-tpl.bin')
-x86 CPUs start up in 16-bit mode, even if they are 64-bit CPUs. This code -must be placed at a particular address. This entry holds that code. It is -typically placed at offset CONFIG_SYS_X86_START16. The code is responsible -for changing to 32-bit mode and starting TPL, which in turn jumps to SPL. +x86 CPUs start up in 16-bit mode, even if they are 32-bit CPUs. This code +must be placed in the top 64KB of the ROM. The reset code jumps to it. This +entry holds that code. It is typically placed at offset +CONFIG_SYS_X86_START16. The code is responsible for changing to 32-bit mode +and jumping to U-Boot's entry point, which requires 32-bit mode (for 32-bit +U-Boot).
-If TPL is not being used, the 'x86_start16_spl or 'x86_start16' entry types +If TPL is not being used, the 'x86-start16-spl or 'x86-start16' entry types may be used instead.
diff --git a/tools/binman/etype/x86_start16.py b/tools/binman/etype/x86_start16.py index 7d32ecd321..6736b692d5 100644 --- a/tools/binman/etype/x86_start16.py +++ b/tools/binman/etype/x86_start16.py @@ -12,14 +12,15 @@ class Entry_x86_start16(Entry_blob): """x86 16-bit start-up code for U-Boot
Properties / Entry arguments: - - filename: Filename of u-boot-x86-16bit.bin (default - 'u-boot-x86-16bit.bin') + - filename: Filename of u-boot-x86-start16.bin (default + 'u-boot-x86-start16.bin')
x86 CPUs start up in 16-bit mode, even if they are 32-bit CPUs. This code - must be placed at a particular address. This entry holds that code. It is - typically placed at offset CONFIG_SYS_X86_START16. The code is responsible - for changing to 32-bit mode and jumping to U-Boot's entry point, which - requires 32-bit mode (for 32-bit U-Boot). + must be placed in the top 64KB of the ROM. The reset code jumps to it. This + entry holds that code. It is typically placed at offset + CONFIG_SYS_X86_START16. The code is responsible for changing to 32-bit mode + and jumping to U-Boot's entry point, which requires 32-bit mode (for 32-bit + U-Boot).
For 64-bit U-Boot, the 'x86_start16_spl' entry type is used instead. """ @@ -27,4 +28,4 @@ class Entry_x86_start16(Entry_blob): Entry_blob.__init__(self, section, etype, node)
def GetDefaultFilename(self): - return 'u-boot-x86-16bit.bin' + return 'u-boot-x86-start16.bin' diff --git a/tools/binman/etype/x86_start16_spl.py b/tools/binman/etype/x86_start16_spl.py index d85909e7ae..c8c70639de 100644 --- a/tools/binman/etype/x86_start16_spl.py +++ b/tools/binman/etype/x86_start16_spl.py @@ -12,19 +12,20 @@ class Entry_x86_start16_spl(Entry_blob): """x86 16-bit start-up code for SPL
Properties / Entry arguments: - - filename: Filename of spl/u-boot-x86-16bit-spl.bin (default - 'spl/u-boot-x86-16bit-spl.bin') + - filename: Filename of spl/u-boot-x86-start16-spl.bin (default + 'spl/u-boot-x86-start16-spl.bin')
- x86 CPUs start up in 16-bit mode, even if they are 64-bit CPUs. This code - must be placed at a particular address. This entry holds that code. It is - typically placed at offset CONFIG_SYS_X86_START16. The code is responsible - for changing to 32-bit mode and starting SPL, which in turn changes to - 64-bit mode and jumps to U-Boot (for 64-bit U-Boot). + x86 CPUs start up in 16-bit mode, even if they are 32-bit CPUs. This code + must be placed in the top 64KB of the ROM. The reset code jumps to it. This + entry holds that code. It is typically placed at offset + CONFIG_SYS_X86_START16. The code is responsible for changing to 32-bit mode + and jumping to U-Boot's entry point, which requires 32-bit mode (for 32-bit + U-Boot).
- For 32-bit U-Boot, the 'x86_start16' entry type is used instead. + For 32-bit U-Boot, the 'x86-start16' entry type is used instead. """ def __init__(self, section, etype, node): Entry_blob.__init__(self, section, etype, node)
def GetDefaultFilename(self): - return 'spl/u-boot-x86-16bit-spl.bin' + return 'spl/u-boot-x86-start16-spl.bin' diff --git a/tools/binman/etype/x86_start16_tpl.py b/tools/binman/etype/x86_start16_tpl.py index 46ce169ae0..5261a8adf0 100644 --- a/tools/binman/etype/x86_start16_tpl.py +++ b/tools/binman/etype/x86_start16_tpl.py @@ -12,19 +12,21 @@ class Entry_x86_start16_tpl(Entry_blob): """x86 16-bit start-up code for TPL
Properties / Entry arguments: - - filename: Filename of tpl/u-boot-x86-16bit-tpl.bin (default - 'tpl/u-boot-x86-16bit-tpl.bin') + - filename: Filename of tpl/u-boot-x86-start16-tpl.bin (default + 'tpl/u-boot-x86-start16-tpl.bin')
- x86 CPUs start up in 16-bit mode, even if they are 64-bit CPUs. This code - must be placed at a particular address. This entry holds that code. It is - typically placed at offset CONFIG_SYS_X86_START16. The code is responsible - for changing to 32-bit mode and starting TPL, which in turn jumps to SPL. + x86 CPUs start up in 16-bit mode, even if they are 32-bit CPUs. This code + must be placed in the top 64KB of the ROM. The reset code jumps to it. This + entry holds that code. It is typically placed at offset + CONFIG_SYS_X86_START16. The code is responsible for changing to 32-bit mode + and jumping to U-Boot's entry point, which requires 32-bit mode (for 32-bit + U-Boot).
- If TPL is not being used, the 'x86_start16_spl or 'x86_start16' entry types + If TPL is not being used, the 'x86-start16-spl or 'x86-start16' entry types may be used instead. """ def __init__(self, section, etype, node): Entry_blob.__init__(self, section, etype, node)
def GetDefaultFilename(self): - return 'tpl/u-boot-x86-16bit-tpl.bin' + return 'tpl/u-boot-x86-start16-tpl.bin' diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 77445814a7..04127faa6f 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -120,10 +120,10 @@ class TestFunctional(unittest.TestCase):
TestFunctional._MakeInputFile('u-boot-br.bin', PPC_MPC85XX_BR_DATA)
- TestFunctional._MakeInputFile('u-boot-x86-16bit.bin', X86_START16_DATA) - TestFunctional._MakeInputFile('spl/u-boot-x86-16bit-spl.bin', + TestFunctional._MakeInputFile('u-boot-x86-start16.bin', X86_START16_DATA) + TestFunctional._MakeInputFile('spl/u-boot-x86-start16-spl.bin', X86_START16_SPL_DATA) - TestFunctional._MakeInputFile('tpl/u-boot-x86-16bit-tpl.bin', + TestFunctional._MakeInputFile('tpl/u-boot-x86-start16-tpl.bin', X86_START16_TPL_DATA)
TestFunctional._MakeInputFile('u-boot-x86-reset16.bin',

At present these two sections of code are linked together into a single 2KB chunk in a single file. Some Intel SoCs like to have a FIT (Firmware Interface Table) in the ROM and the pointer for this needs to go at 0xffffffc0 which is in the middle of these two sections.
Make use of the new 'reset' entry and change the existing 16-bit entry to include just the 16-bit data.
Signed-off-by: Simon Glass sjg@chromium.org ---
Makefile | 10 ++++-- arch/x86/dts/u-boot.dtsi | 9 ++++++ scripts/Makefile.spl | 22 +++++++++---- tools/binman/README.entries | 46 +++++++++++++++------------ tools/binman/etype/x86_start16.py | 15 +++++---- tools/binman/etype/x86_start16_spl.py | 19 +++++------ tools/binman/etype/x86_start16_tpl.py | 18 ++++++----- tools/binman/ftest.py | 6 ++-- 8 files changed, 88 insertions(+), 57 deletions(-)
Applied to u-boot-dm, thanks!

A Firmware Image Table (FIT) is a data structure defined by Intel which contains information about various things needed by the SoC, such as microcode.
Add support for this entry as well as the pointer to it. The contents of FIT are fixed at present. Future work is needed to support adding microcode, etc.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/README.entries | 19 ++++++++++ tools/binman/etype/intel_fit.py | 32 ++++++++++++++++ tools/binman/etype/intel_fit_ptr.py | 41 +++++++++++++++++++++ tools/binman/ftest.py | 20 ++++++++++ tools/binman/test/147_intel_fit.dts | 20 ++++++++++ tools/binman/test/148_intel_fit_missing.dts | 17 +++++++++ 6 files changed, 149 insertions(+) create mode 100644 tools/binman/etype/intel_fit.py create mode 100644 tools/binman/etype/intel_fit_ptr.py create mode 100644 tools/binman/test/147_intel_fit.dts create mode 100644 tools/binman/test/148_intel_fit_missing.dts
diff --git a/tools/binman/README.entries b/tools/binman/README.entries index d17b3cb078..dba51e6daa 100644 --- a/tools/binman/README.entries +++ b/tools/binman/README.entries @@ -391,6 +391,25 @@ See README.x86 for information about x86 binary blobs.
+Entry: intel-fit: Intel Firmware Image Table (FIT) +-------------------------------------------------- + +This entry contains a dummy FIT as required by recent Intel CPUs. The FIT +contains information about the firmware and microcode available in the +image. + +At present binman only supports a basic FIT with no microcode. + + + +Entry: intel-fit-ptr: Intel Firmware Image Table (FIT) pointer +-------------------------------------------------------------- + +This entry contains a pointer to the FIT. It is required to be at address +0xffffffc0 in the image. + + + Entry: intel-fsp: Entry containing an Intel Firmware Support Package (FSP) file -------------------------------------------------------------------------------
diff --git a/tools/binman/etype/intel_fit.py b/tools/binman/etype/intel_fit.py new file mode 100644 index 0000000000..23606d27d0 --- /dev/null +++ b/tools/binman/etype/intel_fit.py @@ -0,0 +1,32 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright (c) 2016 Google, Inc +# Written by Simon Glass sjg@chromium.org +# +# Entry-type module for Intel Firmware Image Table +# + +import struct + +from blob import Entry_blob + +class Entry_intel_fit(Entry_blob): + """Intel Firmware Image Table (FIT) + + This entry contains a dummy FIT as required by recent Intel CPUs. The FIT + contains information about the firmware and microcode available in the + image. + + At present binman only supports a basic FIT with no microcode. + """ + def __init__(self, section, etype, node): + Entry_blob.__init__(self, section, etype, node) + + def ReadNode(self): + """Force 16-byte alignment as required by FIT pointer""" + Entry_blob.ReadNode(self) + self.align = 16 + + def ObtainContents(self): + data = struct.pack('<8sIHBB', '_FIT_ ', 1, 0x100, 0x80, 0x7d) + self.SetContents(data) + return True diff --git a/tools/binman/etype/intel_fit_ptr.py b/tools/binman/etype/intel_fit_ptr.py new file mode 100644 index 0000000000..148b206c3c --- /dev/null +++ b/tools/binman/etype/intel_fit_ptr.py @@ -0,0 +1,41 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright (c) 2016 Google, Inc +# Written by Simon Glass sjg@chromium.org +# +# Entry-type module for a pointer to an Intel Firmware Image Table +# + +import struct + +from blob import Entry_blob + +class Entry_intel_fit_ptr(Entry_blob): + """Intel Firmware Image Table (FIT) pointer + + This entry contains a pointer to the FIT. It is required to be at address + 0xffffffc0 in the image. + """ + def __init__(self, section, etype, node): + Entry_blob.__init__(self, section, etype, node) + if self.HasSibling('intel-fit') is False: + self.Raise("'intel-fit-ptr' section must have an 'intel-fit' sibling") + + def _GetContents(self): + fit_pos = self.GetSiblingImagePos('intel-fit') + return struct.pack('<II', fit_pos or 0, 0) + + def ObtainContents(self): + self.SetContents(self._GetContents()) + return True + + def ProcessContents(self): + """Write an updated version of the FIT pointer to this entry + + This is necessary since image_pos is not available when ObtainContents() + is called, since by then the entries have not been packed in the image. + """ + return self.ProcessContentsUpdate(self._GetContents()) + + def Pack(self, offset): + """Special pack method to set the offset to the right place""" + return Entry_blob.Pack(self, 0xffffffc0) diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 04127faa6f..080599fee3 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -3264,6 +3264,26 @@ class TestFunctional(unittest.TestCase): data = self._DoReadFile('146_x86_reset16_tpl.dts') self.assertEqual(X86_RESET16_TPL_DATA, data[:len(X86_RESET16_TPL_DATA)])
+ def testPackIntelFit(self): + """Test that an image with an Intel FIT and pointer can be created""" + data = self._DoReadFile('147_intel_fit.dts') + self.assertEqual(U_BOOT_DATA, data[:len(U_BOOT_DATA)]) + fit = data[16:32]; + self.assertEqual(b'_FIT_ \x01\x00\x00\x00\x00\x01\x80}' , fit) + ptr = struct.unpack('<i', data[0x40:0x44])[0] + + image = control.images['image'] + entries = image.GetEntries() + expected_ptr = entries['intel-fit'].image_pos - (1 << 32) + self.assertEqual(expected_ptr, ptr) + + def testPackIntelFitMissing(self): + """Test detection of a FIT pointer with not FIT region""" + with self.assertRaises(ValueError) as e: + self._DoReadFile('148_intel_fit_missing.dts') + self.assertIn("'intel-fit-ptr' section must have an 'intel-fit' sibling", + str(e.exception)) +
if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/147_intel_fit.dts b/tools/binman/test/147_intel_fit.dts new file mode 100644 index 0000000000..01ec40e5c7 --- /dev/null +++ b/tools/binman/test/147_intel_fit.dts @@ -0,0 +1,20 @@ +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + end-at-4gb; + size = <0x80>; + + u-boot { + }; + + intel-fit { + }; + + intel-fit-ptr { + }; + }; +}; diff --git a/tools/binman/test/148_intel_fit_missing.dts b/tools/binman/test/148_intel_fit_missing.dts new file mode 100644 index 0000000000..388c76b1ab --- /dev/null +++ b/tools/binman/test/148_intel_fit_missing.dts @@ -0,0 +1,17 @@ +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + end-at-4gb; + size = <0x80>; + + u-boot { + }; + + intel-fit-ptr { + }; + }; +};

A Firmware Image Table (FIT) is a data structure defined by Intel which contains information about various things needed by the SoC, such as microcode.
Add support for this entry as well as the pointer to it. The contents of FIT are fixed at present. Future work is needed to support adding microcode, etc.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/README.entries | 19 ++++++++++ tools/binman/etype/intel_fit.py | 32 ++++++++++++++++ tools/binman/etype/intel_fit_ptr.py | 41 +++++++++++++++++++++ tools/binman/ftest.py | 20 ++++++++++ tools/binman/test/147_intel_fit.dts | 20 ++++++++++ tools/binman/test/148_intel_fit_missing.dts | 17 +++++++++ 6 files changed, 149 insertions(+) create mode 100644 tools/binman/etype/intel_fit.py create mode 100644 tools/binman/etype/intel_fit_ptr.py create mode 100644 tools/binman/test/147_intel_fit.dts create mode 100644 tools/binman/test/148_intel_fit_missing.dts
Applied to u-boot-dm, thanks!

At present this entry does not work correctly when a FIT image is used as the input. It updates the FIT instead of the output image. The test passed because the FIT image happened to have the right data already.
Fix it.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/etype/intel_ifwi.py | 6 +++--- tools/binman/ftest.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/binman/etype/intel_ifwi.py b/tools/binman/etype/intel_ifwi.py index f3745f7a8c..e4da3e498a 100644 --- a/tools/binman/etype/intel_ifwi.py +++ b/tools/binman/etype/intel_ifwi.py @@ -75,10 +75,10 @@ class Entry_intel_ifwi(Entry_blob): self._pathname = outname else: # Provide a different code path here to ensure we have test coverage - inname = self._pathname + outname = self._pathname
# Delete OBBP if it is there, then add the required new items. - tools.RunIfwiTool(inname, tools.CMD_DELETE, subpart='OBBP') + tools.RunIfwiTool(outname, tools.CMD_DELETE, subpart='OBBP')
for entry in self._ifwi_entries.values(): # First get the input data and put it in a file @@ -89,7 +89,7 @@ class Entry_intel_ifwi(Entry_blob): input_fname = tools.GetOutputFilename('input.%s' % uniq) tools.WriteFile(input_fname, data)
- tools.RunIfwiTool(inname, + tools.RunIfwiTool(outname, tools.CMD_REPLACE if entry._ifwi_replace else tools.CMD_ADD, input_fname, entry._ifwi_subpart, entry._ifwi_entry_name)
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 080599fee3..bba07e7275 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -2044,7 +2044,7 @@ class TestFunctional(unittest.TestCase): subpart='IBBP', entry_name='IBBL')
tpl_data = tools.ReadFile(tpl_fname) - self.assertEqual(tpl_data[:len(U_BOOT_TPL_DATA)], U_BOOT_TPL_DATA) + self.assertEqual(U_BOOT_TPL_DATA, tpl_data[:len(U_BOOT_TPL_DATA)])
def testPackX86RomIfwi(self): """Test that an x86 ROM with Integrated Firmware Image can be created"""

At present this entry does not work correctly when a FIT image is used as the input. It updates the FIT instead of the output image. The test passed because the FIT image happened to have the right data already.
Fix it.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/etype/intel_ifwi.py | 6 +++--- tools/binman/ftest.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-)
Applied to u-boot-dm, thanks!

At present this command silently fails if something goes wrong. Use the tools.Run() function instead, since it reports errors.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/elf.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/binman/elf.py b/tools/binman/elf.py index 66cfe796a2..7bc7cf61b5 100644 --- a/tools/binman/elf.py +++ b/tools/binman/elf.py @@ -49,7 +49,7 @@ def GetSymbols(fname, patterns): key: Name of symbol value: Hex value of symbol """ - stdout = command.Output('objdump', '-t', fname, raise_on_error=False) + stdout = tools.Run('objdump', '-t', fname) lines = stdout.splitlines() if patterns: re_syms = re.compile('|'.join(patterns))

At present this command silently fails if something goes wrong. Use the tools.Run() function instead, since it reports errors.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/elf.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Applied to u-boot-dm, thanks!

At present the ELF test files are checked into the U-Boot tree. This is covenient since the files never change and can be used on non-x86 platforms. However it is not good practice to check in binaries and in this case it does not seem essential.
Update the binman test-file Makefile to support having source in a different directory. Adjust binman to run it to build bss_data, as a start. We can add other files as needed.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/elf_test.py | 23 +++++++++++++++++++++++ tools/binman/ftest.py | 24 ++++++++++++++++++------ tools/binman/test/Makefile | 3 ++- tools/binman/test/bss_data | Bin 5020 -> 0 bytes 4 files changed, 43 insertions(+), 7 deletions(-) delete mode 100755 tools/binman/test/bss_data
diff --git a/tools/binman/elf_test.py b/tools/binman/elf_test.py index cc6e9c5128..736b931fd5 100644 --- a/tools/binman/elf_test.py +++ b/tools/binman/elf_test.py @@ -50,6 +50,29 @@ class FakeSection: return self.sym_value
+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. + + Args: + target_dir: Directory to put the files into + """ + if not os.path.exists(target_dir): + os.mkdir(target_dir) + testdir = os.path.join(binman_dir, 'test') + + # If binman is involved from the main U-Boot Makefile the -r and -R + # flags are set in MAKEFLAGS. This prevents this Makefile from working + # correctly. So drop any make flags here. + if 'MAKEFLAGS' in os.environ: + del os.environ['MAKEFLAGS'] + tools.Run('make', '-C', target_dir, '-f', + os.path.join(testdir, 'Makefile'), 'SRC=%s/' % testdir, + 'bss_data', 'u_boot_ucode_ptr') + + class TestElf(unittest.TestCase): @classmethod def setUpClass(self): diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index bba07e7275..fad62bb04f 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -23,6 +23,7 @@ import cmdline import command import control import elf +import elf_test import fdt from etype import fdtmap from etype import image_header @@ -147,6 +148,9 @@ class TestFunctional(unittest.TestCase): TestFunctional._MakeInputFile('bmpblk.bin', BMPBLK_DATA) TestFunctional._MakeInputFile('refcode.bin', REFCODE_DATA)
+ cls._elf_testdir = os.path.join(cls._indir, 'elftest') + elf_test.BuildElfTestFiles(cls._elf_testdir) + # ELF file with a '_dt_ucode_base_size' symbol with open(cls.TestFile('u_boot_ucode_ptr'), 'rb') as fd: TestFunctional._MakeInputFile('u-boot', fd.read()) @@ -484,13 +488,21 @@ class TestFunctional(unittest.TestCase): Args: Filename of ELF file to use as SPL """ - with open(cls.TestFile(src_fname), 'rb') as fd: - TestFunctional._MakeInputFile('spl/u-boot-spl', fd.read()) + # TODO(sjg@chromium.org): Drop this when all Elf files use ElfTestFile() + if src_fname in ['bss_data']: + fname = cls.ElfTestFile(src_fname) + else: + fname = cls.TestFile(src_fname) + TestFunctional._MakeInputFile('spl/u-boot-spl', tools.ReadFile(fname))
@classmethod def TestFile(cls, fname): return os.path.join(cls._binman_dir, 'test', fname)
+ @classmethod + def ElfTestFile(cls, fname): + return os.path.join(cls._elf_testdir, fname) + def AssertInList(self, grep_list, target): """Assert that at least one of a list of things is in a target
@@ -1551,7 +1563,7 @@ class TestFunctional(unittest.TestCase): def testTpl(self): """Test that an image with TPL and ots device tree can be created""" # ELF file with a '__bss_size' symbol - with open(self.TestFile('bss_data'), 'rb') as fd: + with open(self.ElfTestFile('bss_data'), 'rb') as fd: TestFunctional._MakeInputFile('tpl/u-boot-tpl', fd.read()) data = self._DoReadFile('078_u_boot_tpl.dts') self.assertEqual(U_BOOT_TPL_DATA + U_BOOT_TPL_DTB_DATA, data) @@ -1861,16 +1873,16 @@ class TestFunctional(unittest.TestCase): def testElf(self): """Basic test of ELF entries""" self._SetupSplElf() - with open(self.TestFile('bss_data'), 'rb') as fd: + with open(self.ElfTestFile('bss_data'), 'rb') as fd: TestFunctional._MakeInputFile('tpl/u-boot-tpl', fd.read()) - with open(self.TestFile('bss_data'), 'rb') as fd: + with open(self.ElfTestFile('bss_data'), 'rb') as fd: TestFunctional._MakeInputFile('-boot', fd.read()) data = self._DoReadFile('096_elf.dts')
def testElfStrip(self): """Basic test of ELF entries""" self._SetupSplElf() - with open(self.TestFile('bss_data'), 'rb') as fd: + with open(self.ElfTestFile('bss_data'), 'rb') as fd: TestFunctional._MakeInputFile('-boot', fd.read()) data = self._DoReadFile('097_elf_strip.dts')
diff --git a/tools/binman/test/Makefile b/tools/binman/test/Makefile index e58fc80775..ce1c2f900c 100644 --- a/tools/binman/test/Makefile +++ b/tools/binman/test/Makefile @@ -7,6 +7,7 @@ # SPDX-License-Identifier: GPL-2.0+ #
+VPATH := $(SRC) CFLAGS := -march=i386 -m32 -nostdlib -I ../../../include
LDS_UCODE := -T u_boot_ucode_ptr.lds @@ -25,7 +26,7 @@ u_boot_no_ucode_ptr: u_boot_no_ucode_ptr.c u_boot_ucode_ptr: CFLAGS += $(LDS_UCODE) u_boot_ucode_ptr: u_boot_ucode_ptr.c
-bss_data: CFLAGS += bss_data.lds +bss_data: CFLAGS += $(SRC)bss_data.lds bss_data: bss_data.c
u_boot_binman_syms.bin: u_boot_binman_syms diff --git a/tools/binman/test/bss_data b/tools/binman/test/bss_data deleted file mode 100755 index afa28282aa157f6717c1ba244ecbfc6e1b081734..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001
literal 5020 zcmeHLyKWOf6uoP&!GJ+|Af+(H7C{(6hJ>s{1O*}i;vz!9k|7jmvEv0>!j5FGr4S(= z1rl`m016r?sner~AE2ip9R*N@0-1AWb`nc8lqu#)$M-R39(#6N?0tS?>89s-Vl5+C zVfN$CU=YG@j+l{90?A29j!9mR>*@<XFTe^W5IGjX=X`p3hjGA1SOyLOtTX>YU@kF< zm|&e)-boq-EK+#s=ZTZ35qA7G#*zMGT%X&LM}8Jqyj7L$-{T)<zJ0y(_Vh>Z{jc>) zoA)Lv)i*nzb0r)u1JV{C_dj{X>=n-E`M(bagY)oQhvscm#Cw|eiUr?)4Z<nZh%N9m z=h}(<tIYiI-10UUoZ-wV;1qBQI0c*nP64NYQ@|<U6mSYS1)Ks0M}ZQKvbeBtIVe@@ z{Z7&kLN%wtsf&G`%{-e4)pV$4&zic3>OE;EwK{y#HNI)1&RP<yN1eW^_gjw}Q>})m zBwkNM#m(qpx7LoMW}~~GiE7l6ny7lOCu()A-HtoS|Lal&^)SHCcil&Tp9HMgPjH0- zzvtN-*hQ~l7v6r;Bi!p{glWw6bl(A!hIw|q`5|6_-b4W29BS4qZwUqN%N~U8gQR^A zrZmf|AkG8i1!zb3;PIVU3)0{&JlC5}bMnrmF&)Q<Q9$nrPrCr#109(ka%l8?R%_)k z^iEJbiUPs&VX7PfhSyse7rBm_HM^e8hdtj5bJI~W`kUPBOr1?`cA%anPt}1QCfA)M zt&hodCyAl9EN;T^Iehs!uw(Sh3-I>2Mv+e-r`{#_QQVFIo;@!(Jhvxj;Qe&}5sc3w z=l$WG7=v<r=lkP1xr)3z#1~xah!<R~N)$2awKn3tszkk{)=lh?j@z|XN1|B&E26m5 FkiP*om$v`_

At present the ELF test files are checked into the U-Boot tree. This is covenient since the files never change and can be used on non-x86 platforms. However it is not good practice to check in binaries and in this case it does not seem essential.
Update the binman test-file Makefile to support having source in a different directory. Adjust binman to run it to build bss_data, as a start. We can add other files as needed.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/elf_test.py | 23 +++++++++++++++++++++++ tools/binman/ftest.py | 24 ++++++++++++++++++------ tools/binman/test/Makefile | 3 ++- tools/binman/test/bss_data | Bin 5020 -> 0 bytes 4 files changed, 43 insertions(+), 7 deletions(-) delete mode 100755 tools/binman/test/bss_data
Applied to u-boot-dm, thanks!

Remove this file from git and instead build it using the Makefile.
Update tools.GetInputFilename() to support reading files from an absolute path, so that we can read the Elf test files easily. Also make sure that the temp directory is report in ELF tests as this was commented out.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/elf_test.py | 19 +++++++++++++++---- tools/binman/ftest.py | 14 +++++++------- tools/binman/test/Makefile | 2 +- tools/binman/test/u_boot_ucode_ptr | Bin 4175 -> 0 bytes tools/binman/test/u_boot_ucode_ptr.lds | 3 ++- tools/patman/tools.py | 2 +- 6 files changed, 26 insertions(+), 14 deletions(-) delete mode 100755 tools/binman/test/u_boot_ucode_ptr
diff --git a/tools/binman/elf_test.py b/tools/binman/elf_test.py index 736b931fd5..403ca2b412 100644 --- a/tools/binman/elf_test.py +++ b/tools/binman/elf_test.py @@ -75,18 +75,29 @@ def BuildElfTestFiles(target_dir):
class TestElf(unittest.TestCase): @classmethod - def setUpClass(self): + def setUpClass(cls): + cls._indir = tempfile.mkdtemp(prefix='elf.') tools.SetInputDirs(['.']) + BuildElfTestFiles(cls._indir) + + @classmethod + def tearDownClass(cls): + if cls._indir: + shutil.rmtree(cls._indir) + + @classmethod + def ElfTestFile(cls, fname): + return os.path.join(cls._indir, fname)
def testAllSymbols(self): """Test that we can obtain a symbol from the ELF file""" - fname = os.path.join(binman_dir, 'test', 'u_boot_ucode_ptr') + fname = self.ElfTestFile('u_boot_ucode_ptr') syms = elf.GetSymbols(fname, []) self.assertIn('.ucode', syms)
def testRegexSymbols(self): """Test that we can obtain from the ELF file by regular expression""" - fname = os.path.join(binman_dir, 'test', 'u_boot_ucode_ptr') + fname = self.ElfTestFile('u_boot_ucode_ptr') syms = elf.GetSymbols(fname, ['ucode']) self.assertIn('.ucode', syms) syms = elf.GetSymbols(fname, ['missing']) @@ -201,7 +212,7 @@ class TestElf(unittest.TestCase): self.assertEqual(elf.ElfInfo(b'\0\0' + expected[2:], load, entry, len(expected)), elf.DecodeElf(data, load + 2)) - #shutil.rmtree(outdir) + shutil.rmtree(outdir)
if __name__ == '__main__': diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index fad62bb04f..e7ade0fddf 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -152,8 +152,8 @@ class TestFunctional(unittest.TestCase): elf_test.BuildElfTestFiles(cls._elf_testdir)
# ELF file with a '_dt_ucode_base_size' symbol - with open(cls.TestFile('u_boot_ucode_ptr'), 'rb') as fd: - TestFunctional._MakeInputFile('u-boot', fd.read()) + TestFunctional._MakeInputFile('u-boot', + tools.ReadFile(cls.ElfTestFile('u_boot_ucode_ptr')))
# Intel flash descriptor file with open(cls.TestFile('descriptor.bin'), 'rb') as fd: @@ -489,7 +489,7 @@ class TestFunctional(unittest.TestCase): Filename of ELF file to use as SPL """ # TODO(sjg@chromium.org): Drop this when all Elf files use ElfTestFile() - if src_fname in ['bss_data']: + if src_fname in ['bss_data', 'u_boot_ucode_ptr']: fname = cls.ElfTestFile(src_fname) else: fname = cls.TestFile(src_fname) @@ -1101,8 +1101,8 @@ class TestFunctional(unittest.TestCase):
finally: # Put the original file back - with open(self.TestFile('u_boot_ucode_ptr'), 'rb') as fd: - TestFunctional._MakeInputFile('u-boot', fd.read()) + TestFunctional._MakeInputFile('u-boot', + tools.ReadFile(self.ElfTestFile('u_boot_ucode_ptr')))
def testMicrocodeNotInImage(self): """Test that microcode must be placed within the image""" @@ -1818,8 +1818,8 @@ class TestFunctional(unittest.TestCase): u-boot-tpl.dtb with the microcode removed the microcode """ - with open(self.TestFile('u_boot_ucode_ptr'), 'rb') as fd: - TestFunctional._MakeInputFile('tpl/u-boot-tpl', fd.read()) + TestFunctional._MakeInputFile('tpl/u-boot-tpl', + tools.ReadFile(self.ElfTestFile('u_boot_ucode_ptr'))) first, pos_and_size = self._RunMicrocodeTest('093_x86_tpl_ucode.dts', U_BOOT_TPL_NODTB_DATA) self.assertEqual(b'tplnodtb with microc' + pos_and_size + diff --git a/tools/binman/test/Makefile b/tools/binman/test/Makefile index ce1c2f900c..fd660eac6e 100644 --- a/tools/binman/test/Makefile +++ b/tools/binman/test/Makefile @@ -10,7 +10,7 @@ VPATH := $(SRC) CFLAGS := -march=i386 -m32 -nostdlib -I ../../../include
-LDS_UCODE := -T u_boot_ucode_ptr.lds +LDS_UCODE := -T $(SRC)u_boot_ucode_ptr.lds LDS_BINMAN := -T u_boot_binman_syms.lds LDS_BINMAN_BAD := -T u_boot_binman_syms_bad.lds
diff --git a/tools/binman/test/u_boot_ucode_ptr b/tools/binman/test/u_boot_ucode_ptr deleted file mode 100755 index dbfb184cecfbcf55cf43ed4f4ac0ee90a7364d93..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001
literal 4175 zcmeHKze~eV5WfDv>Y%NqTOB%ds7Rl!MiB>>qFqFAD7b~B30kpv@e1VLh}je}Vs) zgM)i3QgG0CU(yE=7YE1p!Ew2t@9xWVH@o|LsZ@#-(v%@sqt7rjSl=(i5rZlmsZoxy zQ9SaF!jM>&I0rHVXMs3_>*wPh=u>4I0zc&NRXVJG0rgz2p&8H&Xa+O`ngPv#W<WEb z8PE)91~dczzyR*A5=(}qebAw|qP00Wx+^}sOy2XS&mWD8@+0oQG~%t+cBR&_15XAO zLu?77z7|AQ^SWt>h9TCMV?U7?UiPJBvzCKcpQta-m##SW0$~TeGpF8jNCaKqaY=Oj ze&6*ZKlNvnIWxzC`EXm}&a5V?u^%8<um|=meT89(@6%cSR#15x>_A>)8o(X9qLQXD z#1~o6OQFqqJIY{<8~_@#DLmzgZrQ+Xi@EVGZrnMRWWNeKSJ|ha`YAi9u{Z4aQjhnG z?c~ddtBklhOXCp#9(;g{)Q?Fq+c>PTU-d6wo4~YvUz*V$GtcEfbjfs-ZCgXv9QLkU KGKbO{NcskUZF7hK
diff --git a/tools/binman/test/u_boot_ucode_ptr.lds b/tools/binman/test/u_boot_ucode_ptr.lds index 0cf9b762b5..cf4d1b8bbd 100644 --- a/tools/binman/test/u_boot_ucode_ptr.lds +++ b/tools/binman/test/u_boot_ucode_ptr.lds @@ -9,9 +9,10 @@ ENTRY(_start)
SECTIONS { - . = 0xfffffdf0; + . = 0xfffffe14; _start = .; .ucode : { *(.ucode) } + .interp : { *(.interp*) } } diff --git a/tools/patman/tools.py b/tools/patman/tools.py index 0952681579..4a7fcdad21 100644 --- a/tools/patman/tools.py +++ b/tools/patman/tools.py @@ -125,7 +125,7 @@ def GetInputFilename(fname): Returns: The full path of the filename, within the input directory """ - if not indir: + if not indir or fname[:1] == '/': return fname for dirname in indir: pathname = os.path.join(dirname, fname)

Remove this file from git and instead build it using the Makefile.
Update tools.GetInputFilename() to support reading files from an absolute path, so that we can read the Elf test files easily. Also make sure that the temp directory is report in ELF tests as this was commented out.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/elf_test.py | 19 +++++++++++++++---- tools/binman/ftest.py | 14 +++++++------- tools/binman/test/Makefile | 2 +- tools/binman/test/u_boot_ucode_ptr | Bin 4175 -> 0 bytes tools/binman/test/u_boot_ucode_ptr.lds | 3 ++- tools/patman/tools.py | 2 +- 6 files changed, 26 insertions(+), 14 deletions(-) delete mode 100755 tools/binman/test/u_boot_ucode_ptr
Applied to u-boot-dm, thanks!

Remove this file from git and instead build it using the Makefile.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/elf_test.py | 2 +- tools/binman/ftest.py | 10 +++++----- tools/binman/test/u_boot_no_ucode_ptr | Bin 4182 -> 0 bytes 3 files changed, 6 insertions(+), 6 deletions(-) delete mode 100755 tools/binman/test/u_boot_no_ucode_ptr
diff --git a/tools/binman/elf_test.py b/tools/binman/elf_test.py index 403ca2b412..c7f51bb86a 100644 --- a/tools/binman/elf_test.py +++ b/tools/binman/elf_test.py @@ -70,7 +70,7 @@ def BuildElfTestFiles(target_dir): del os.environ['MAKEFLAGS'] tools.Run('make', '-C', target_dir, '-f', os.path.join(testdir, 'Makefile'), 'SRC=%s/' % testdir, - 'bss_data', 'u_boot_ucode_ptr') + 'bss_data', 'u_boot_ucode_ptr', 'u_boot_no_ucode_ptr')
class TestElf(unittest.TestCase): diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index e7ade0fddf..30a8b0b14c 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -489,7 +489,7 @@ class TestFunctional(unittest.TestCase): Filename of ELF file to use as SPL """ # TODO(sjg@chromium.org): Drop this when all Elf files use ElfTestFile() - if src_fname in ['bss_data', 'u_boot_ucode_ptr']: + if src_fname in ['bss_data', 'u_boot_ucode_ptr', 'u_boot_no_ucode_ptr']: fname = cls.ElfTestFile(src_fname) else: fname = cls.TestFile(src_fname) @@ -1091,8 +1091,8 @@ class TestFunctional(unittest.TestCase): """Test that a U-Boot binary without the microcode symbol is detected""" # ELF file without a '_dt_ucode_base_size' symbol try: - with open(self.TestFile('u_boot_no_ucode_ptr'), 'rb') as fd: - TestFunctional._MakeInputFile('u-boot', fd.read()) + TestFunctional._MakeInputFile('u-boot', + tools.ReadFile(self.ElfTestFile('u_boot_no_ucode_ptr')))
with self.assertRaises(ValueError) as e: self._RunPackUbootSingleMicrocode() @@ -1114,8 +1114,8 @@ class TestFunctional(unittest.TestCase):
def testWithoutMicrocode(self): """Test that we can cope with an image without microcode (e.g. qemu)""" - with open(self.TestFile('u_boot_no_ucode_ptr'), 'rb') as fd: - TestFunctional._MakeInputFile('u-boot', fd.read()) + TestFunctional._MakeInputFile('u-boot', + tools.ReadFile(self.ElfTestFile('u_boot_no_ucode_ptr'))) data, dtb, _, _ = self._DoReadFileDtb('044_x86_optional_ucode.dts', True)
# Now check the device tree has no microcode diff --git a/tools/binman/test/u_boot_no_ucode_ptr b/tools/binman/test/u_boot_no_ucode_ptr deleted file mode 100755 index f72462f0be41a934d468481bf627d6c1ec9a8e1c..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001
literal 4182 zcmeHKy-EW?5T5*Cv``}iEVQ^HMMSbDdWK+O6Euwq7AdTbT<(Ygb0^7OVG6bMF|2$8 zpFw;En;;05iM_7#<+2d5v9R2MkJ<U%oo}nTIXF5@Bod@0NhyLg`c%qheYX@xY_d2~ zpbVua@rie&6fxF02bhC1OPs;=i*XP1$+Hc>51hV9kJT?hJ(n9X3>XFs1BL;^fMLKe zU>GnA7zPXjhJk-z0Q*;tkz&+O8uWhlJbWr0zYHsC`1(-IehePl*#DA<*J^uKq2We> zj4WGJg<af^CRX{nY>SdDb~a)^k?3D_Wz%IXd$B&(ry!KRXa|vSqt1m_?06)iR_OU8 zT4A^A2a>P)v#fDuhJp8Cx5S>ApQ*-t5W&D4m^1gKRF3!4c|L2=dAsaDUTGS@9=oZN zrZL1<80e*?&UyRVV2vCIG~TA=ewpZ&4eYjfH}1ubyTF+3XR))wJ}tVRwr4fwh8=I} z@qDp8do$uXBd$)<SgrCAe1MC@kC)<YW3|P8-9L+IBF7Cw=>(xO`84NJ_C$;LPaVgT TQ=i-H`%b?z@X6`RW>3;L6WVxU

Remove this file from git and instead build it using the Makefile.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/elf_test.py | 2 +- tools/binman/ftest.py | 10 +++++----- tools/binman/test/u_boot_no_ucode_ptr | Bin 4182 -> 0 bytes 3 files changed, 6 insertions(+), 6 deletions(-) delete mode 100755 tools/binman/test/u_boot_no_ucode_ptr
Applied to u-boot-dm, thanks!

Remove this file from git and instead build it using the Makefile.
With this change a few things need to be adjusted:
1. The 'notes' section no-longer appears at the start of the ELF file (before the code), so update testSymbols to adjust the offsets.
2. The dynamic linker is disabled to avoid errors like:
"Not enough room for program headers, try linking with -N"
3. The interpreter note is moved to the end of the image, so that the binman symbols appear first.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/elf_test.py | 9 +++++---- tools/binman/ftest.py | 7 ++++--- tools/binman/test/Makefile | 5 +++-- tools/binman/test/u_boot_binman_syms | Bin 4924 -> 0 bytes tools/binman/test/u_boot_binman_syms.lds | 1 + 5 files changed, 13 insertions(+), 9 deletions(-) delete mode 100755 tools/binman/test/u_boot_binman_syms
diff --git a/tools/binman/elf_test.py b/tools/binman/elf_test.py index c7f51bb86a..ff036cb655 100644 --- a/tools/binman/elf_test.py +++ b/tools/binman/elf_test.py @@ -70,7 +70,8 @@ def BuildElfTestFiles(target_dir): del os.environ['MAKEFLAGS'] tools.Run('make', '-C', target_dir, '-f', os.path.join(testdir, 'Makefile'), 'SRC=%s/' % testdir, - 'bss_data', 'u_boot_ucode_ptr', 'u_boot_no_ucode_ptr') + 'bss_data', 'u_boot_ucode_ptr', 'u_boot_no_ucode_ptr', + 'u_boot_binman_syms', 'u_boot_binman_syms.bin')
class TestElf(unittest.TestCase): @@ -118,7 +119,7 @@ class TestElf(unittest.TestCase): """Test a symbol which extends outside the entry area is detected""" entry = FakeEntry(10) section = FakeSection() - elf_fname = os.path.join(binman_dir, 'test', 'u_boot_binman_syms') + elf_fname = self.ElfTestFile('u_boot_binman_syms') with self.assertRaises(ValueError) as e: syms = elf.LookupAndWriteSymbols(elf_fname, entry, section) self.assertIn('entry_path has offset 4 (size 8) but the contents size ' @@ -158,7 +159,7 @@ class TestElf(unittest.TestCase): """ entry = FakeEntry(20) section = FakeSection(sym_value=None) - elf_fname = os.path.join(binman_dir, 'test', 'u_boot_binman_syms') + elf_fname = self.ElfTestFile('u_boot_binman_syms') syms = elf.LookupAndWriteSymbols(elf_fname, entry, section) self.assertEqual(tools.GetBytes(255, 16) + tools.GetBytes(ord('a'), 4), entry.data) @@ -169,7 +170,7 @@ class TestElf(unittest.TestCase): tout.Init(tout.DEBUG) entry = FakeEntry(20) section = FakeSection() - elf_fname = os.path.join(binman_dir, 'test', 'u_boot_binman_syms') + elf_fname = self.ElfTestFile('u_boot_binman_syms') with test_util.capture_sys_output() as (stdout, stderr): syms = elf.LookupAndWriteSymbols(elf_fname, entry, section) self.assertTrue(len(stdout.getvalue()) > 0) diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 30a8b0b14c..507c481881 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -489,7 +489,8 @@ class TestFunctional(unittest.TestCase): Filename of ELF file to use as SPL """ # TODO(sjg@chromium.org): Drop this when all Elf files use ElfTestFile() - if src_fname in ['bss_data', 'u_boot_ucode_ptr', 'u_boot_no_ucode_ptr']: + if src_fname in ['bss_data', 'u_boot_ucode_ptr', 'u_boot_no_ucode_ptr', + 'u_boot_binman_syms']: fname = cls.ElfTestFile(src_fname) else: fname = cls.TestFile(src_fname) @@ -1223,14 +1224,14 @@ class TestFunctional(unittest.TestCase):
def testSymbols(self): """Test binman can assign symbols embedded in U-Boot""" - elf_fname = self.TestFile('u_boot_binman_syms') + elf_fname = self.ElfTestFile('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_offset'].address, addr)
self._SetupSplElf('u_boot_binman_syms') data = self._DoReadFile('053_symbols.dts') - sym_values = struct.pack('<LQL', 0x24 + 0, 0x24 + 24, 0x24 + 20) + sym_values = struct.pack('<LQL', 0, 24, 20) expected = (sym_values + U_BOOT_SPL_DATA[16:] + tools.GetBytes(0xff, 1) + U_BOOT_DATA + sym_values + U_BOOT_SPL_DATA[16:]) diff --git a/tools/binman/test/Makefile b/tools/binman/test/Makefile index fd660eac6e..7af5459793 100644 --- a/tools/binman/test/Makefile +++ b/tools/binman/test/Makefile @@ -8,10 +8,11 @@ #
VPATH := $(SRC) -CFLAGS := -march=i386 -m32 -nostdlib -I ../../../include +CFLAGS := -march=i386 -m32 -nostdlib -I $(SRC)../../../include \ + -Wl,--no-dynamic-linker
LDS_UCODE := -T $(SRC)u_boot_ucode_ptr.lds -LDS_BINMAN := -T u_boot_binman_syms.lds +LDS_BINMAN := -T $(SRC)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 \ diff --git a/tools/binman/test/u_boot_binman_syms b/tools/binman/test/u_boot_binman_syms deleted file mode 100755 index 126a1a623092cbe25f7a24118d26d4a0e8d0e0fc..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001
literal 4924 zcmeHLziZn-6h22z-3|pOlrCKi)!;&M$UzabQ)p;IAp|;jJ|tU74Au{<6U9ri2C`;r zyJaf0e?uVuMfa|SOdZ?rvwV*2lu|O^gHP|?``*2iK55ZAeY5|zR;vkPE5Z>b@{x4c zE5;WsFm2Hg^@?wr9YU~<c)%3$^{J<$xl=Gty#Jy5aCeOR=i1)kZNe&G6|f3e1*`&A z0jq#jz$#!BunJfOtO8bn`zgTx@h5-->>YH4`}5c1&&ByC=kmIE-njg7@cOG<?!33V z`>NIMMrEqY*0%Jew;7a^sB_W@r02_y_o5<NTV5yd0`KK=kLB1*9o#A5h?RvBXKpS* zqAqTJKZS1t4}mq*2k_j_XJUK-Z>))_MHhhw82$s+oLkVEs6T@@=QIAe5MwG*swm5} ziUw&Crm7exh3p9vPRSx4ZmE2f<tjToEMol^{$F&Mjw^bQgh#Q;vqC6y0uEGh+Fwi* zIvn8;Rn_;he5|UJkf_&T1g}SxKQ^m0)3H2COBt1eVSjVb7xMO@6pDwjki9HP;#5<e z|FdT(YXGx1hhdJ<lO3o#jUN<eO#J4^yQrI=`<S{NaIE7yqxmS>u9lN4Ibsc9FJs@4 ncxM85*3jr@_lETl{jT6ScUlE_F7M+JFyC(j{k|b*%=G&MuziL^
diff --git a/tools/binman/test/u_boot_binman_syms.lds b/tools/binman/test/u_boot_binman_syms.lds index 29cf9d0e54..926df873cb 100644 --- a/tools/binman/test/u_boot_binman_syms.lds +++ b/tools/binman/test/u_boot_binman_syms.lds @@ -25,5 +25,6 @@ SECTIONS KEEP(*(SORT(.binman_sym*))); __binman_sym_end = .; } + .interp : { *(.interp*) }
}

Remove this file from git and instead build it using the Makefile.
With this change a few things need to be adjusted:
1. The 'notes' section no-longer appears at the start of the ELF file (before the code), so update testSymbols to adjust the offsets.
2. The dynamic linker is disabled to avoid errors like:
"Not enough room for program headers, try linking with -N"
3. The interpreter note is moved to the end of the image, so that the binman symbols appear first.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/elf_test.py | 9 +++++---- tools/binman/ftest.py | 7 ++++--- tools/binman/test/Makefile | 5 +++-- tools/binman/test/u_boot_binman_syms | Bin 4924 -> 0 bytes tools/binman/test/u_boot_binman_syms.lds | 1 + 5 files changed, 13 insertions(+), 9 deletions(-) delete mode 100755 tools/binman/test/u_boot_binman_syms
Applied to u-boot-dm, thanks!

Remove this file from git and instead build it using the Makefile.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/elf_test.py | 5 +++-- tools/binman/ftest.py | 2 +- tools/binman/test/u_boot_binman_syms_size | Bin 4825 -> 0 bytes 3 files changed, 4 insertions(+), 3 deletions(-) delete mode 100755 tools/binman/test/u_boot_binman_syms_size
diff --git a/tools/binman/elf_test.py b/tools/binman/elf_test.py index ff036cb655..a913970150 100644 --- a/tools/binman/elf_test.py +++ b/tools/binman/elf_test.py @@ -71,7 +71,8 @@ def BuildElfTestFiles(target_dir): tools.Run('make', '-C', target_dir, '-f', os.path.join(testdir, 'Makefile'), 'SRC=%s/' % testdir, 'bss_data', 'u_boot_ucode_ptr', 'u_boot_no_ucode_ptr', - 'u_boot_binman_syms', 'u_boot_binman_syms.bin') + 'u_boot_binman_syms', 'u_boot_binman_syms.bin', + 'u_boot_binman_syms_size')
class TestElf(unittest.TestCase): @@ -145,7 +146,7 @@ class TestElf(unittest.TestCase): """ entry = FakeEntry(10) section = FakeSection() - elf_fname = os.path.join(binman_dir, 'test', 'u_boot_binman_syms_size') + elf_fname =self.ElfTestFile('u_boot_binman_syms_size') with self.assertRaises(ValueError) as e: syms = elf.LookupAndWriteSymbols(elf_fname, entry, section) self.assertIn('has size 1: only 4 and 8 are supported', diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 507c481881..51eab6fbfa 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -490,7 +490,7 @@ class TestFunctional(unittest.TestCase): """ # TODO(sjg@chromium.org): Drop this when all Elf files use ElfTestFile() if src_fname in ['bss_data', 'u_boot_ucode_ptr', 'u_boot_no_ucode_ptr', - 'u_boot_binman_syms']: + 'u_boot_binman_syms', 'u_boot_binman_syms_size']: fname = cls.ElfTestFile(src_fname) else: fname = cls.TestFile(src_fname) diff --git a/tools/binman/test/u_boot_binman_syms_size b/tools/binman/test/u_boot_binman_syms_size deleted file mode 100755 index d691e897c0f1842cb82efbc67f57d9f62853b99c..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001
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

Remove this file from git and instead build it using the Makefile.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/elf_test.py | 5 +++-- tools/binman/ftest.py | 2 +- tools/binman/test/u_boot_binman_syms_size | Bin 4825 -> 0 bytes 3 files changed, 4 insertions(+), 3 deletions(-) delete mode 100755 tools/binman/test/u_boot_binman_syms_size
Applied to u-boot-dm, thanks!

Remove this file from git and instead build it using the Makefile.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/elf_test.py | 4 ++-- tools/binman/test/Makefile | 2 +- tools/binman/test/u_boot_binman_syms_bad | Bin 4890 -> 0 bytes 3 files changed, 3 insertions(+), 3 deletions(-) delete mode 100755 tools/binman/test/u_boot_binman_syms_bad
diff --git a/tools/binman/elf_test.py b/tools/binman/elf_test.py index a913970150..1ee5d9d57c 100644 --- a/tools/binman/elf_test.py +++ b/tools/binman/elf_test.py @@ -72,7 +72,7 @@ def BuildElfTestFiles(target_dir): os.path.join(testdir, 'Makefile'), 'SRC=%s/' % testdir, 'bss_data', 'u_boot_ucode_ptr', 'u_boot_no_ucode_ptr', 'u_boot_binman_syms', 'u_boot_binman_syms.bin', - 'u_boot_binman_syms_size') + 'u_boot_binman_syms_size', 'u_boot_binman_syms_bad')
class TestElf(unittest.TestCase): @@ -134,7 +134,7 @@ class TestElf(unittest.TestCase): """ entry = FakeEntry(10) section = FakeSection() - elf_fname = os.path.join(binman_dir, 'test', 'u_boot_binman_syms_bad') + elf_fname = self.ElfTestFile('u_boot_binman_syms_bad') self.assertEqual(elf.LookupAndWriteSymbols(elf_fname, entry, section), None)
diff --git a/tools/binman/test/Makefile b/tools/binman/test/Makefile index 7af5459793..593bbe9bd9 100644 --- a/tools/binman/test/Makefile +++ b/tools/binman/test/Makefile @@ -13,7 +13,7 @@ CFLAGS := -march=i386 -m32 -nostdlib -I $(SRC)../../../include \
LDS_UCODE := -T $(SRC)u_boot_ucode_ptr.lds LDS_BINMAN := -T $(SRC)u_boot_binman_syms.lds -LDS_BINMAN_BAD := -T u_boot_binman_syms_bad.lds +LDS_BINMAN_BAD := -T $(SRC)u_boot_binman_syms_bad.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 \ diff --git a/tools/binman/test/u_boot_binman_syms_bad b/tools/binman/test/u_boot_binman_syms_bad deleted file mode 100755 index 8da3d9d48f388a9be53e92590984411691f6721f..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001
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?

Remove this file from git and instead build it using the Makefile.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/elf_test.py | 4 ++-- tools/binman/test/Makefile | 2 +- tools/binman/test/u_boot_binman_syms_bad | Bin 4890 -> 0 bytes 3 files changed, 3 insertions(+), 3 deletions(-) delete mode 100755 tools/binman/test/u_boot_binman_syms_bad
Applied to u-boot-dm, thanks!

We use the Makefile for all ELF test files now, so drop all the code that checks whether to get the test file from the Makefile or from the git repo.
Also add a comment to the Makefile indicating that it is run from binman.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/elf_test.py | 5 +---- tools/binman/ftest.py | 9 ++------- tools/binman/test/Makefile | 2 +- 3 files changed, 4 insertions(+), 12 deletions(-)
diff --git a/tools/binman/elf_test.py b/tools/binman/elf_test.py index 1ee5d9d57c..f05545bcb1 100644 --- a/tools/binman/elf_test.py +++ b/tools/binman/elf_test.py @@ -69,10 +69,7 @@ def BuildElfTestFiles(target_dir): if 'MAKEFLAGS' in os.environ: del os.environ['MAKEFLAGS'] tools.Run('make', '-C', target_dir, '-f', - os.path.join(testdir, 'Makefile'), 'SRC=%s/' % testdir, - 'bss_data', 'u_boot_ucode_ptr', 'u_boot_no_ucode_ptr', - 'u_boot_binman_syms', 'u_boot_binman_syms.bin', - 'u_boot_binman_syms_size', 'u_boot_binman_syms_bad') + os.path.join(testdir, 'Makefile'), 'SRC=%s/' % testdir)
class TestElf(unittest.TestCase): diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 51eab6fbfa..1d774e28e5 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -488,13 +488,8 @@ class TestFunctional(unittest.TestCase): Args: Filename of ELF file to use as SPL """ - # TODO(sjg@chromium.org): Drop this when all Elf files use ElfTestFile() - if src_fname in ['bss_data', 'u_boot_ucode_ptr', 'u_boot_no_ucode_ptr', - 'u_boot_binman_syms', 'u_boot_binman_syms_size']: - fname = cls.ElfTestFile(src_fname) - else: - fname = cls.TestFile(src_fname) - TestFunctional._MakeInputFile('spl/u-boot-spl', tools.ReadFile(fname)) + TestFunctional._MakeInputFile('spl/u-boot-spl', + tools.ReadFile(cls.ElfTestFile(src_fname)))
@classmethod def TestFile(cls, fname): diff --git a/tools/binman/test/Makefile b/tools/binman/test/Makefile index 593bbe9bd9..bdbb009874 100644 --- a/tools/binman/test/Makefile +++ b/tools/binman/test/Makefile @@ -1,5 +1,5 @@ # -# Builds test programs +# Builds test programs. This is launched from elf_test.BuildElfTestFiles() # # Copyright (C) 2017 Google, Inc # Written by Simon Glass sjg@chromium.org

We use the Makefile for all ELF test files now, so drop all the code that checks whether to get the test file from the Makefile or from the git repo.
Also add a comment to the Makefile indicating that it is run from binman.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/elf_test.py | 5 +---- tools/binman/ftest.py | 9 ++------- tools/binman/test/Makefile | 2 +- 3 files changed, 4 insertions(+), 12 deletions(-)
Applied to u-boot-dm, thanks!

At present we only support symbols inside binaries which are at the top level of an image. This restrictions seems unreasonable since more complex images may want to group binaries within different sections.
Relax the restriction.
Also fix a typo in the comment for testTpl().
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/etype/u_boot_spl.py | 2 +- tools/binman/etype/u_boot_tpl.py | 2 +- tools/binman/ftest.py | 35 +++++++++++++++++++++++++-- tools/binman/test/149_symbols_tpl.dts | 28 +++++++++++++++++++++ 4 files changed, 63 insertions(+), 4 deletions(-) create mode 100644 tools/binman/test/149_symbols_tpl.dts
diff --git a/tools/binman/etype/u_boot_spl.py b/tools/binman/etype/u_boot_spl.py index ab78714c8d..7fedd00021 100644 --- a/tools/binman/etype/u_boot_spl.py +++ b/tools/binman/etype/u_boot_spl.py @@ -40,4 +40,4 @@ class Entry_u_boot_spl(Entry_blob): return 'spl/u-boot-spl.bin'
def WriteSymbols(self, section): - elf.LookupAndWriteSymbols(self.elf_fname, self, section) + elf.LookupAndWriteSymbols(self.elf_fname, self, section.GetImage()) diff --git a/tools/binman/etype/u_boot_tpl.py b/tools/binman/etype/u_boot_tpl.py index 4d4bb92596..1b69c4f4a7 100644 --- a/tools/binman/etype/u_boot_tpl.py +++ b/tools/binman/etype/u_boot_tpl.py @@ -40,4 +40,4 @@ class Entry_u_boot_tpl(Entry_blob): return 'tpl/u-boot-tpl.bin'
def WriteSymbols(self, section): - elf.LookupAndWriteSymbols(self.elf_fname, self, section) + elf.LookupAndWriteSymbols(self.elf_fname, self, section.GetImage()) diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 1d774e28e5..008e747270 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -40,7 +40,7 @@ import tout U_BOOT_DATA = b'1234' U_BOOT_IMG_DATA = b'img' U_BOOT_SPL_DATA = b'56780123456789abcde' -U_BOOT_TPL_DATA = b'tpl' +U_BOOT_TPL_DATA = b'tpl9876543210fedcb' BLOB_DATA = b'89' ME_DATA = b'0abcd' VGA_DATA = b'vga' @@ -491,6 +491,16 @@ class TestFunctional(unittest.TestCase): TestFunctional._MakeInputFile('spl/u-boot-spl', tools.ReadFile(cls.ElfTestFile(src_fname)))
+ @classmethod + def _SetupTplElf(cls, src_fname='bss_data'): + """Set up an ELF file with a '_dt_ucode_base_size' symbol + + Args: + Filename of ELF file to use as TPL + """ + TestFunctional._MakeInputFile('tpl/u-boot-tpl', + tools.ReadFile(cls.ElfTestFile(src_fname))) + @classmethod def TestFile(cls, fname): return os.path.join(cls._binman_dir, 'test', fname) @@ -1557,7 +1567,7 @@ class TestFunctional(unittest.TestCase): "'other'", str(e.exception))
def testTpl(self): - """Test that an image with TPL and ots device tree can be created""" + """Test that an image with TPL and its device tree can be created""" # ELF file with a '__bss_size' symbol with open(self.ElfTestFile('bss_data'), 'rb') as fd: TestFunctional._MakeInputFile('tpl/u-boot-tpl', fd.read()) @@ -3292,6 +3302,27 @@ class TestFunctional(unittest.TestCase): self.assertIn("'intel-fit-ptr' section must have an 'intel-fit' sibling", str(e.exception))
+ def testSymbolsTplSection(self): + """Test binman can assign symbols embedded in U-Boot TPL in a section""" + self._SetupSplElf('u_boot_binman_syms') + self._SetupTplElf('u_boot_binman_syms') + data = self._DoReadFile('149_symbols_tpl.dts') + sym_values = struct.pack('<LQL', 4, 0x18, 0x30) + upto1 = 4 + len(U_BOOT_SPL_DATA) + expected1 = tools.GetBytes(0xff, 4) + sym_values + U_BOOT_SPL_DATA[16:] + self.assertEqual(expected1, data[:upto1]) + + upto2 = upto1 + 1 + len(U_BOOT_SPL_DATA) + expected2 = tools.GetBytes(0xff, 1) + sym_values + U_BOOT_SPL_DATA[16:] + self.assertEqual(expected2, data[upto1:upto2]) + + upto3 = 0x30 + len(U_BOOT_DATA) + expected3 = tools.GetBytes(0xff, 5) + U_BOOT_DATA + self.assertEqual(expected3, data[upto2:upto3]) + + expected4 = sym_values + U_BOOT_TPL_DATA[16:] + self.assertEqual(expected4, data[upto3:]) +
if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/149_symbols_tpl.dts b/tools/binman/test/149_symbols_tpl.dts new file mode 100644 index 0000000000..087e10f292 --- /dev/null +++ b/tools/binman/test/149_symbols_tpl.dts @@ -0,0 +1,28 @@ +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + pad-byte = <0xff>; + u-boot-spl { + offset = <4>; + }; + + u-boot-spl2 { + offset = <0x18>; + type = "u-boot-spl"; + }; + + u-boot { + offset = <0x30>; + }; + + section { + u-boot-tpl { + type = "u-boot-tpl"; + }; + }; + }; +};

At present we only support symbols inside binaries which are at the top level of an image. This restrictions seems unreasonable since more complex images may want to group binaries within different sections.
Relax the restriction.
Also fix a typo in the comment for testTpl().
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/etype/u_boot_spl.py | 2 +- tools/binman/etype/u_boot_tpl.py | 2 +- tools/binman/ftest.py | 35 +++++++++++++++++++++++++-- tools/binman/test/149_symbols_tpl.dts | 28 +++++++++++++++++++++ 4 files changed, 63 insertions(+), 4 deletions(-) create mode 100644 tools/binman/test/149_symbols_tpl.dts
Applied to u-boot-dm, thanks!

At present a small number of test files use hyphens instead of underscores. Rename them for consistency.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/ftest.py | 28 +++++++++---------- .../test/{029_x86-rom.dts => 029_x86_rom.dts} | 0 ...no-desc.dts => 030_x86_rom_me_no_desc.dts} | 0 ...{031_x86-rom-me.dts => 031_x86_rom_me.dts} | 0 .../{032_intel-vga.dts => 032_intel_vga.dts} | 0 ...33_x86-start16.dts => 033_x86_start16.dts} | 0 .../{042_intel-fsp.dts => 042_intel_fsp.dts} | 0 .../{043_intel-cmc.dts => 043_intel_cmc.dts} | 0 .../{046_intel-vbt.dts => 046_intel_vbt.dts} | 0 ...tart16-spl.dts => 048_x86_start16_spl.dts} | 0 ...tart16-tpl.dts => 081_x86_start16_tpl.dts} | 0 ..._x86-rom-ifwi.dts => 111_x86_rom_ifwi.dts} | 0 ...nodesc.dts => 112_x86_rom_ifwi_nodesc.dts} | 0 ...nodata.dts => 113_x86_rom_ifwi_nodata.dts} | 0 14 files changed, 14 insertions(+), 14 deletions(-) rename tools/binman/test/{029_x86-rom.dts => 029_x86_rom.dts} (100%) rename tools/binman/test/{030_x86-rom-me-no-desc.dts => 030_x86_rom_me_no_desc.dts} (100%) rename tools/binman/test/{031_x86-rom-me.dts => 031_x86_rom_me.dts} (100%) rename tools/binman/test/{032_intel-vga.dts => 032_intel_vga.dts} (100%) rename tools/binman/test/{033_x86-start16.dts => 033_x86_start16.dts} (100%) rename tools/binman/test/{042_intel-fsp.dts => 042_intel_fsp.dts} (100%) rename tools/binman/test/{043_intel-cmc.dts => 043_intel_cmc.dts} (100%) rename tools/binman/test/{046_intel-vbt.dts => 046_intel_vbt.dts} (100%) rename tools/binman/test/{048_x86-start16-spl.dts => 048_x86_start16_spl.dts} (100%) rename tools/binman/test/{081_x86-start16-tpl.dts => 081_x86_start16_tpl.dts} (100%) rename tools/binman/test/{111_x86-rom-ifwi.dts => 111_x86_rom_ifwi.dts} (100%) rename tools/binman/test/{112_x86-rom-ifwi-nodesc.dts => 112_x86_rom_ifwi_nodesc.dts} (100%) rename tools/binman/test/{113_x86-rom-ifwi-nodata.dts => 113_x86_rom_ifwi_nodata.dts} (100%)
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 008e747270..d0ec2f1f5c 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -921,7 +921,7 @@ class TestFunctional(unittest.TestCase): def testPackX86Rom(self): """Test that a basic x86 ROM can be created""" self._SetupSplElf() - data = self._DoReadFile('029_x86-rom.dts') + data = self._DoReadFile('029_x86_rom.dts') self.assertEqual(U_BOOT_DATA + tools.GetBytes(0, 7) + U_BOOT_SPL_DATA + tools.GetBytes(0, 2), data)
@@ -929,21 +929,21 @@ class TestFunctional(unittest.TestCase): """Test that an invalid Intel descriptor entry is detected""" TestFunctional._MakeInputFile('descriptor.bin', b'') with self.assertRaises(ValueError) as e: - self._DoTestFile('031_x86-rom-me.dts') + self._DoTestFile('031_x86_rom_me.dts') self.assertIn("Node '/binman/intel-descriptor': Cannot find Intel Flash Descriptor (FD) signature", str(e.exception))
def testPackX86RomBadDesc(self): """Test that the Intel requires a descriptor entry""" with self.assertRaises(ValueError) as e: - self._DoTestFile('030_x86-rom-me-no-desc.dts') + self._DoTestFile('030_x86_rom_me_no_desc.dts') self.assertIn("Node '/binman/intel-me': No offset set with " "offset-unset: should another entry provide this correct " "offset?", str(e.exception))
def testPackX86RomMe(self): """Test that an x86 ROM with an ME region can be created""" - data = self._DoReadFile('031_x86-rom-me.dts') + data = self._DoReadFile('031_x86_rom_me.dts') expected_desc = tools.ReadFile(self.TestFile('descriptor.bin')) if data[:0x1000] != expected_desc: self.fail('Expected descriptor binary at start of image') @@ -951,12 +951,12 @@ class TestFunctional(unittest.TestCase):
def testPackVga(self): """Test that an image with a VGA binary can be created""" - data = self._DoReadFile('032_intel-vga.dts') + data = self._DoReadFile('032_intel_vga.dts') self.assertEqual(VGA_DATA, data[:len(VGA_DATA)])
def testPackStart16(self): """Test that an image with an x86 start16 region can be created""" - data = self._DoReadFile('033_x86-start16.dts') + data = self._DoReadFile('033_x86_start16.dts') self.assertEqual(X86_START16_DATA, data[:len(X86_START16_DATA)])
def testPackPowerpcMpc85xxBootpgResetvec(self): @@ -1144,17 +1144,17 @@ class TestFunctional(unittest.TestCase):
def testPackFsp(self): """Test that an image with a FSP binary can be created""" - data = self._DoReadFile('042_intel-fsp.dts') + data = self._DoReadFile('042_intel_fsp.dts') self.assertEqual(FSP_DATA, data[:len(FSP_DATA)])
def testPackCmc(self): """Test that an image with a CMC binary can be created""" - data = self._DoReadFile('043_intel-cmc.dts') + data = self._DoReadFile('043_intel_cmc.dts') self.assertEqual(CMC_DATA, data[:len(CMC_DATA)])
def testPackVbt(self): """Test that an image with a VBT binary can be created""" - data = self._DoReadFile('046_intel-vbt.dts') + data = self._DoReadFile('046_intel_vbt.dts') self.assertEqual(VBT_DATA, data[:len(VBT_DATA)])
def testSplBssPad(self): @@ -1175,7 +1175,7 @@ class TestFunctional(unittest.TestCase):
def testPackStart16Spl(self): """Test that an image with an x86 start16 SPL region can be created""" - data = self._DoReadFile('048_x86-start16-spl.dts') + data = self._DoReadFile('048_x86_start16_spl.dts') self.assertEqual(X86_START16_SPL_DATA, data[:len(X86_START16_SPL_DATA)])
def _PackUbootSplMicrocode(self, dts, ucode_second=False): @@ -1595,7 +1595,7 @@ class TestFunctional(unittest.TestCase):
def testPackStart16Tpl(self): """Test that an image with an x86 start16 TPL region can be created""" - data = self._DoReadFile('081_x86-start16-tpl.dts') + data = self._DoReadFile('081_x86_start16_tpl.dts') self.assertEqual(X86_START16_TPL_DATA, data[:len(X86_START16_TPL_DATA)])
def testSelectImage(self): @@ -2067,20 +2067,20 @@ class TestFunctional(unittest.TestCase): def testPackX86RomIfwi(self): """Test that an x86 ROM with Integrated Firmware Image can be created""" self._SetupIfwi('fitimage.bin') - data = self._DoReadFile('111_x86-rom-ifwi.dts') + data = self._DoReadFile('111_x86_rom_ifwi.dts') self._CheckIfwi(data)
def testPackX86RomIfwiNoDesc(self): """Test that an x86 ROM with IFWI can be created from an ifwi.bin file""" self._SetupIfwi('ifwi.bin') - data = self._DoReadFile('112_x86-rom-ifwi-nodesc.dts') + data = self._DoReadFile('112_x86_rom_ifwi_nodesc.dts') self._CheckIfwi(data)
def testPackX86RomIfwiNoData(self): """Test that an x86 ROM with IFWI handles missing data""" self._SetupIfwi('ifwi.bin') with self.assertRaises(ValueError) as e: - data = self._DoReadFile('113_x86-rom-ifwi-nodata.dts') + data = self._DoReadFile('113_x86_rom_ifwi_nodata.dts') self.assertIn('Could not complete processing of contents', str(e.exception))
diff --git a/tools/binman/test/029_x86-rom.dts b/tools/binman/test/029_x86_rom.dts similarity index 100% rename from tools/binman/test/029_x86-rom.dts rename to tools/binman/test/029_x86_rom.dts diff --git a/tools/binman/test/030_x86-rom-me-no-desc.dts b/tools/binman/test/030_x86_rom_me_no_desc.dts similarity index 100% rename from tools/binman/test/030_x86-rom-me-no-desc.dts rename to tools/binman/test/030_x86_rom_me_no_desc.dts diff --git a/tools/binman/test/031_x86-rom-me.dts b/tools/binman/test/031_x86_rom_me.dts similarity index 100% rename from tools/binman/test/031_x86-rom-me.dts rename to tools/binman/test/031_x86_rom_me.dts diff --git a/tools/binman/test/032_intel-vga.dts b/tools/binman/test/032_intel_vga.dts similarity index 100% rename from tools/binman/test/032_intel-vga.dts rename to tools/binman/test/032_intel_vga.dts diff --git a/tools/binman/test/033_x86-start16.dts b/tools/binman/test/033_x86_start16.dts similarity index 100% rename from tools/binman/test/033_x86-start16.dts rename to tools/binman/test/033_x86_start16.dts diff --git a/tools/binman/test/042_intel-fsp.dts b/tools/binman/test/042_intel_fsp.dts similarity index 100% rename from tools/binman/test/042_intel-fsp.dts rename to tools/binman/test/042_intel_fsp.dts diff --git a/tools/binman/test/043_intel-cmc.dts b/tools/binman/test/043_intel_cmc.dts similarity index 100% rename from tools/binman/test/043_intel-cmc.dts rename to tools/binman/test/043_intel_cmc.dts diff --git a/tools/binman/test/046_intel-vbt.dts b/tools/binman/test/046_intel_vbt.dts similarity index 100% rename from tools/binman/test/046_intel-vbt.dts rename to tools/binman/test/046_intel_vbt.dts diff --git a/tools/binman/test/048_x86-start16-spl.dts b/tools/binman/test/048_x86_start16_spl.dts similarity index 100% rename from tools/binman/test/048_x86-start16-spl.dts rename to tools/binman/test/048_x86_start16_spl.dts diff --git a/tools/binman/test/081_x86-start16-tpl.dts b/tools/binman/test/081_x86_start16_tpl.dts similarity index 100% rename from tools/binman/test/081_x86-start16-tpl.dts rename to tools/binman/test/081_x86_start16_tpl.dts diff --git a/tools/binman/test/111_x86-rom-ifwi.dts b/tools/binman/test/111_x86_rom_ifwi.dts similarity index 100% rename from tools/binman/test/111_x86-rom-ifwi.dts rename to tools/binman/test/111_x86_rom_ifwi.dts diff --git a/tools/binman/test/112_x86-rom-ifwi-nodesc.dts b/tools/binman/test/112_x86_rom_ifwi_nodesc.dts similarity index 100% rename from tools/binman/test/112_x86-rom-ifwi-nodesc.dts rename to tools/binman/test/112_x86_rom_ifwi_nodesc.dts diff --git a/tools/binman/test/113_x86-rom-ifwi-nodata.dts b/tools/binman/test/113_x86_rom_ifwi_nodata.dts similarity index 100% rename from tools/binman/test/113_x86-rom-ifwi-nodata.dts rename to tools/binman/test/113_x86_rom_ifwi_nodata.dts

At present a small number of test files use hyphens instead of underscores. Rename them for consistency.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/ftest.py | 28 +++++++++---------- .../test/{029_x86-rom.dts => 029_x86_rom.dts} | 0 ...no-desc.dts => 030_x86_rom_me_no_desc.dts} | 0 ...{031_x86-rom-me.dts => 031_x86_rom_me.dts} | 0 .../{032_intel-vga.dts => 032_intel_vga.dts} | 0 ...33_x86-start16.dts => 033_x86_start16.dts} | 0 .../{042_intel-fsp.dts => 042_intel_fsp.dts} | 0 .../{043_intel-cmc.dts => 043_intel_cmc.dts} | 0 .../{046_intel-vbt.dts => 046_intel_vbt.dts} | 0 ...tart16-spl.dts => 048_x86_start16_spl.dts} | 0 ...tart16-tpl.dts => 081_x86_start16_tpl.dts} | 0 ..._x86-rom-ifwi.dts => 111_x86_rom_ifwi.dts} | 0 ...nodesc.dts => 112_x86_rom_ifwi_nodesc.dts} | 0 ...nodata.dts => 113_x86_rom_ifwi_nodata.dts} | 0 14 files changed, 14 insertions(+), 14 deletions(-) rename tools/binman/test/{029_x86-rom.dts => 029_x86_rom.dts} (100%) rename tools/binman/test/{030_x86-rom-me-no-desc.dts => 030_x86_rom_me_no_desc.dts} (100%) rename tools/binman/test/{031_x86-rom-me.dts => 031_x86_rom_me.dts} (100%) rename tools/binman/test/{032_intel-vga.dts => 032_intel_vga.dts} (100%) rename tools/binman/test/{033_x86-start16.dts => 033_x86_start16.dts} (100%) rename tools/binman/test/{042_intel-fsp.dts => 042_intel_fsp.dts} (100%) rename tools/binman/test/{043_intel-cmc.dts => 043_intel_cmc.dts} (100%) rename tools/binman/test/{046_intel-vbt.dts => 046_intel_vbt.dts} (100%) rename tools/binman/test/{048_x86-start16-spl.dts => 048_x86_start16_spl.dts} (100%) rename tools/binman/test/{081_x86-start16-tpl.dts => 081_x86_start16_tpl.dts} (100%) rename tools/binman/test/{111_x86-rom-ifwi.dts => 111_x86_rom_ifwi.dts} (100%) rename tools/binman/test/{112_x86-rom-ifwi-nodesc.dts => 112_x86_rom_ifwi_nodesc.dts} (100%) rename tools/binman/test/{113_x86-rom-ifwi-nodata.dts => 113_x86_rom_ifwi_nodata.dts} (100%)
Applied to u-boot-dm, thanks!

Two of the test files somehow were not converted to three digits. Fix them, using the next available numbers.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/ftest.py | 4 ++-- ...rt_together.dts => 098_4gb_and_skip_at_start_together.dts} | 0 ...g_resetvec.dts => 150_powerpc_mpc85xx_bootpg_resetvec.dts} | 0 3 files changed, 2 insertions(+), 2 deletions(-) rename tools/binman/test/{80_4gb_and_skip_at_start_together.dts => 098_4gb_and_skip_at_start_together.dts} (100%) rename tools/binman/test/{81_powerpc_mpc85xx_bootpg_resetvec.dts => 150_powerpc_mpc85xx_bootpg_resetvec.dts} (100%)
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index d0ec2f1f5c..75e25f3e23 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -906,7 +906,7 @@ class TestFunctional(unittest.TestCase): """Test that the end-at-4gb and skip-at-size property can't be used together""" with self.assertRaises(ValueError) as e: - self._DoTestFile('80_4gb_and_skip_at_start_together.dts') + self._DoTestFile('098_4gb_and_skip_at_start_together.dts') self.assertIn("Image '/binman': Provide either 'end-at-4gb' or " "'skip-at-start'", str(e.exception))
@@ -962,7 +962,7 @@ class TestFunctional(unittest.TestCase): def testPackPowerpcMpc85xxBootpgResetvec(self): """Test that an image with powerpc-mpc85xx-bootpg-resetvec can be created""" - data = self._DoReadFile('81_powerpc_mpc85xx_bootpg_resetvec.dts') + data = self._DoReadFile('150_powerpc_mpc85xx_bootpg_resetvec.dts') self.assertEqual(PPC_MPC85XX_BR_DATA, data[:len(PPC_MPC85XX_BR_DATA)])
def _RunMicrocodeTest(self, dts_fname, nodtb_data, ucode_second=False): diff --git a/tools/binman/test/80_4gb_and_skip_at_start_together.dts b/tools/binman/test/098_4gb_and_skip_at_start_together.dts similarity index 100% rename from tools/binman/test/80_4gb_and_skip_at_start_together.dts rename to tools/binman/test/098_4gb_and_skip_at_start_together.dts diff --git a/tools/binman/test/81_powerpc_mpc85xx_bootpg_resetvec.dts b/tools/binman/test/150_powerpc_mpc85xx_bootpg_resetvec.dts similarity index 100% rename from tools/binman/test/81_powerpc_mpc85xx_bootpg_resetvec.dts rename to tools/binman/test/150_powerpc_mpc85xx_bootpg_resetvec.dts

Two of the test files somehow were not converted to three digits. Fix them, using the next available numbers.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/ftest.py | 4 ++-- ...rt_together.dts => 098_4gb_and_skip_at_start_together.dts} | 0 ...g_resetvec.dts => 150_powerpc_mpc85xx_bootpg_resetvec.dts} | 0 3 files changed, 2 insertions(+), 2 deletions(-) rename tools/binman/test/{80_4gb_and_skip_at_start_together.dts => 098_4gb_and_skip_at_start_together.dts} (100%) rename tools/binman/test/{81_powerpc_mpc85xx_bootpg_resetvec.dts => 150_powerpc_mpc85xx_bootpg_resetvec.dts} (100%)
Applied to u-boot-dm, thanks!

Entries which include a section and need to obtain its contents call GetData(), as with any other entry. But the current implementation of this method in entry_Section requires the size of the section to be known. If it is unknown, an error is produced, since size is None:
TypeError: can't multiply sequence by non-int of type 'NoneType'
There is no need to know the size in advance since the code can be adjusted to build up the section piece by piece, instead of patching each entry into an existing bytearray.
Update the code to handle this and add a test.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/etype/section.py | 14 +++++--- tools/binman/ftest.py | 6 ++++ .../binman/test/151_x86_rom_ifwi_section.dts | 33 +++++++++++++++++++ 3 files changed, 49 insertions(+), 4 deletions(-) create mode 100644 tools/binman/test/151_x86_rom_ifwi_section.dts
diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index 5d34fc546a..ff5d30f3fa 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -142,13 +142,19 @@ class Entry_section(Entry): return self.GetEntryContents()
def GetData(self): - section_data = tools.GetBytes(self._pad_byte, self.size) + section_data = b''
for entry in self._entries.values(): data = entry.GetData() - base = self.pad_before + entry.offset - self._skip_at_start - section_data = (section_data[:base] + data + - section_data[base + len(data):]) + base = self.pad_before + (entry.offset or 0) - self._skip_at_start + pad = base - len(section_data) + if pad > 0: + section_data += tools.GetBytes(self._pad_byte, pad) + section_data += data + if self.size: + pad = self.size - len(section_data) + if pad > 0: + section_data += tools.GetBytes(self._pad_byte, pad) self.Detail('GetData: %d entries, total size %#x' % (len(self._entries), len(section_data))) return section_data diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 75e25f3e23..1eb0d3b684 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -3323,6 +3323,12 @@ class TestFunctional(unittest.TestCase): expected4 = sym_values + U_BOOT_TPL_DATA[16:] self.assertEqual(expected4, data[upto3:])
+ def testPackX86RomIfwiSectiom(self): + """Test that a section can be placed in an IFWI region""" + self._SetupIfwi('fitimage.bin') + data = self._DoReadFile('151_x86_rom_ifwi_section.dts') + self._CheckIfwi(data) +
if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/151_x86_rom_ifwi_section.dts b/tools/binman/test/151_x86_rom_ifwi_section.dts new file mode 100644 index 0000000000..7e455c3a4b --- /dev/null +++ b/tools/binman/test/151_x86_rom_ifwi_section.dts @@ -0,0 +1,33 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + sort-by-offset; + end-at-4gb; + size = <0x800000>; + intel-descriptor { + filename = "descriptor.bin"; + }; + + intel-ifwi { + offset-unset; + filename = "fitimage.bin"; + convert-fit; + + section { + ifwi-replace; + ifwi-subpart = "IBBP"; + ifwi-entry = "IBBL"; + u-boot-tpl { + }; + u-boot-dtb { + }; + }; + }; + }; +};

Entries which include a section and need to obtain its contents call GetData(), as with any other entry. But the current implementation of this method in entry_Section requires the size of the section to be known. If it is unknown, an error is produced, since size is None:
TypeError: can't multiply sequence by non-int of type 'NoneType'
There is no need to know the size in advance since the code can be adjusted to build up the section piece by piece, instead of patching each entry into an existing bytearray.
Update the code to handle this and add a test.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/etype/section.py | 14 +++++--- tools/binman/ftest.py | 6 ++++ .../binman/test/151_x86_rom_ifwi_section.dts | 33 +++++++++++++++++++ 3 files changed, 49 insertions(+), 4 deletions(-) create mode 100644 tools/binman/test/151_x86_rom_ifwi_section.dts
Applied to u-boot-dm, thanks!

At present these are large enough to hold 20 bytes of symbol data. Add four more bytes so we can add another test.
Unfortunately at present this involves changing a few test files to make room. We could adjust the test files to not specify sizes for entries. Then we could make the tests check the actual sizes. But for now, leave it as it is, since the effort is minor.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/ftest.py | 14 +++++++------- tools/binman/test/021_image_pad.dts | 2 +- tools/binman/test/024_sorted.dts | 2 +- tools/binman/test/028_pack_4gb_outside.dts | 2 +- tools/binman/test/029_x86_rom.dts | 2 +- tools/binman/test/053_symbols.dts | 2 +- tools/binman/test/149_symbols_tpl.dts | 4 ++-- 7 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 1eb0d3b684..65d3ad59ea 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -39,8 +39,8 @@ import tout # Contents of test files, corresponding to different entry types U_BOOT_DATA = b'1234' U_BOOT_IMG_DATA = b'img' -U_BOOT_SPL_DATA = b'56780123456789abcde' -U_BOOT_TPL_DATA = b'tpl9876543210fedcb' +U_BOOT_SPL_DATA = b'56780123456789abcdefghi' +U_BOOT_TPL_DATA = b'tpl9876543210fedcbazyw' BLOB_DATA = b'89' ME_DATA = b'0abcd' VGA_DATA = b'vga' @@ -922,7 +922,7 @@ class TestFunctional(unittest.TestCase): """Test that a basic x86 ROM can be created""" self._SetupSplElf() data = self._DoReadFile('029_x86_rom.dts') - self.assertEqual(U_BOOT_DATA + tools.GetBytes(0, 7) + U_BOOT_SPL_DATA + + self.assertEqual(U_BOOT_DATA + tools.GetBytes(0, 3) + U_BOOT_SPL_DATA + tools.GetBytes(0, 2), data)
def testPackX86RomMeNoDesc(self): @@ -1236,7 +1236,7 @@ class TestFunctional(unittest.TestCase):
self._SetupSplElf('u_boot_binman_syms') data = self._DoReadFile('053_symbols.dts') - sym_values = struct.pack('<LQL', 0, 24, 20) + sym_values = struct.pack('<LQL', 0, 28, 24) expected = (sym_values + U_BOOT_SPL_DATA[16:] + tools.GetBytes(0xff, 1) + U_BOOT_DATA + sym_values + U_BOOT_SPL_DATA[16:]) @@ -3307,7 +3307,7 @@ class TestFunctional(unittest.TestCase): self._SetupSplElf('u_boot_binman_syms') self._SetupTplElf('u_boot_binman_syms') data = self._DoReadFile('149_symbols_tpl.dts') - sym_values = struct.pack('<LQL', 4, 0x18, 0x30) + sym_values = struct.pack('<LQL', 4, 0x1c, 0x34) upto1 = 4 + len(U_BOOT_SPL_DATA) expected1 = tools.GetBytes(0xff, 4) + sym_values + U_BOOT_SPL_DATA[16:] self.assertEqual(expected1, data[:upto1]) @@ -3316,8 +3316,8 @@ class TestFunctional(unittest.TestCase): expected2 = tools.GetBytes(0xff, 1) + sym_values + U_BOOT_SPL_DATA[16:] self.assertEqual(expected2, data[upto1:upto2])
- upto3 = 0x30 + len(U_BOOT_DATA) - expected3 = tools.GetBytes(0xff, 5) + U_BOOT_DATA + upto3 = 0x34 + len(U_BOOT_DATA) + expected3 = tools.GetBytes(0xff, 1) + U_BOOT_DATA self.assertEqual(expected3, data[upto2:upto3])
expected4 = sym_values + U_BOOT_TPL_DATA[16:] diff --git a/tools/binman/test/021_image_pad.dts b/tools/binman/test/021_image_pad.dts index c6516689d9..1ff8dab296 100644 --- a/tools/binman/test/021_image_pad.dts +++ b/tools/binman/test/021_image_pad.dts @@ -10,7 +10,7 @@ };
u-boot { - offset = <20>; + offset = <24>; }; }; }; diff --git a/tools/binman/test/024_sorted.dts b/tools/binman/test/024_sorted.dts index d35d39f077..b79d9adf68 100644 --- a/tools/binman/test/024_sorted.dts +++ b/tools/binman/test/024_sorted.dts @@ -7,7 +7,7 @@ binman { sort-by-offset; u-boot { - offset = <22>; + offset = <26>; };
u-boot-spl { diff --git a/tools/binman/test/028_pack_4gb_outside.dts b/tools/binman/test/028_pack_4gb_outside.dts index 2216abfb70..11a1f6059e 100644 --- a/tools/binman/test/028_pack_4gb_outside.dts +++ b/tools/binman/test/028_pack_4gb_outside.dts @@ -13,7 +13,7 @@ };
u-boot-spl { - offset = <0xffffffeb>; + offset = <0xffffffe7>; }; }; }; diff --git a/tools/binman/test/029_x86_rom.dts b/tools/binman/test/029_x86_rom.dts index d5c69f9d4a..88aa007bba 100644 --- a/tools/binman/test/029_x86_rom.dts +++ b/tools/binman/test/029_x86_rom.dts @@ -13,7 +13,7 @@ };
u-boot-spl { - offset = <0xffffffeb>; + offset = <0xffffffe7>; }; }; }; diff --git a/tools/binman/test/053_symbols.dts b/tools/binman/test/053_symbols.dts index 9f135676cb..8af575158f 100644 --- a/tools/binman/test/053_symbols.dts +++ b/tools/binman/test/053_symbols.dts @@ -10,7 +10,7 @@ };
u-boot { - offset = <20>; + offset = <24>; };
u-boot-spl2 { diff --git a/tools/binman/test/149_symbols_tpl.dts b/tools/binman/test/149_symbols_tpl.dts index 087e10f292..dfc84af5e7 100644 --- a/tools/binman/test/149_symbols_tpl.dts +++ b/tools/binman/test/149_symbols_tpl.dts @@ -11,12 +11,12 @@ };
u-boot-spl2 { - offset = <0x18>; + offset = <0x1c>; type = "u-boot-spl"; };
u-boot { - offset = <0x30>; + offset = <0x34>; };
section {

At present these are large enough to hold 20 bytes of symbol data. Add four more bytes so we can add another test.
Unfortunately at present this involves changing a few test files to make room. We could adjust the test files to not specify sizes for entries. Then we could make the tests check the actual sizes. But for now, leave it as it is, since the effort is minor.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/ftest.py | 14 +++++++------- tools/binman/test/021_image_pad.dts | 2 +- tools/binman/test/024_sorted.dts | 2 +- tools/binman/test/028_pack_4gb_outside.dts | 2 +- tools/binman/test/029_x86_rom.dts | 2 +- tools/binman/test/053_symbols.dts | 2 +- tools/binman/test/149_symbols_tpl.dts | 4 ++-- 7 files changed, 14 insertions(+), 14 deletions(-)
Applied to u-boot-dm, thanks!

It is useful to be able to access the size of an image in SPL, with something like:
binman_sym_declare(unsigned long, u_boot_any, size);
... ulong u_boot_size = binman_sym(ulong, u_boot_any, size);
Add support for this and update the tests.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/elf_test.py | 4 ++-- tools/binman/etype/section.py | 2 ++ tools/binman/ftest.py | 14 +++++++------- tools/binman/test/u_boot_binman_syms.c | 1 + 4 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/tools/binman/elf_test.py b/tools/binman/elf_test.py index f05545bcb1..c0c11cb340 100644 --- a/tools/binman/elf_test.py +++ b/tools/binman/elf_test.py @@ -155,11 +155,11 @@ class TestElf(unittest.TestCase): This should produce -1 values for all thress symbols, taking up the first 16 bytes of the image. """ - entry = FakeEntry(20) + entry = FakeEntry(24) section = FakeSection(sym_value=None) elf_fname = self.ElfTestFile('u_boot_binman_syms') syms = elf.LookupAndWriteSymbols(elf_fname, entry, section) - self.assertEqual(tools.GetBytes(255, 16) + tools.GetBytes(ord('a'), 4), + self.assertEqual(tools.GetBytes(255, 20) + tools.GetBytes(ord('a'), 4), entry.data)
def testDebug(self): diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index ff5d30f3fa..d39c6ad81e 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -344,6 +344,8 @@ class Entry_section(Entry): return entry.offset elif prop_name == 'image_pos': return entry.image_pos + if prop_name == 'size': + return entry.size else: raise ValueError("%s: No such property '%s'" % (msg, prop_name))
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 65d3ad59ea..a740c3e03d 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -1236,10 +1236,10 @@ class TestFunctional(unittest.TestCase):
self._SetupSplElf('u_boot_binman_syms') data = self._DoReadFile('053_symbols.dts') - sym_values = struct.pack('<LQL', 0, 28, 24) - expected = (sym_values + U_BOOT_SPL_DATA[16:] + + sym_values = struct.pack('<LQLL', 0, 28, 24, 4) + expected = (sym_values + U_BOOT_SPL_DATA[20:] + tools.GetBytes(0xff, 1) + U_BOOT_DATA + sym_values + - U_BOOT_SPL_DATA[16:]) + U_BOOT_SPL_DATA[20:]) self.assertEqual(expected, data)
def testPackUnitAddress(self): @@ -3307,20 +3307,20 @@ class TestFunctional(unittest.TestCase): self._SetupSplElf('u_boot_binman_syms') self._SetupTplElf('u_boot_binman_syms') data = self._DoReadFile('149_symbols_tpl.dts') - sym_values = struct.pack('<LQL', 4, 0x1c, 0x34) + sym_values = struct.pack('<LQLL', 4, 0x1c, 0x34, 4) upto1 = 4 + len(U_BOOT_SPL_DATA) - expected1 = tools.GetBytes(0xff, 4) + sym_values + U_BOOT_SPL_DATA[16:] + expected1 = tools.GetBytes(0xff, 4) + sym_values + U_BOOT_SPL_DATA[20:] self.assertEqual(expected1, data[:upto1])
upto2 = upto1 + 1 + len(U_BOOT_SPL_DATA) - expected2 = tools.GetBytes(0xff, 1) + sym_values + U_BOOT_SPL_DATA[16:] + expected2 = tools.GetBytes(0xff, 1) + sym_values + U_BOOT_SPL_DATA[20:] self.assertEqual(expected2, data[upto1:upto2])
upto3 = 0x34 + len(U_BOOT_DATA) expected3 = tools.GetBytes(0xff, 1) + U_BOOT_DATA self.assertEqual(expected3, data[upto2:upto3])
- expected4 = sym_values + U_BOOT_TPL_DATA[16:] + expected4 = sym_values + U_BOOT_TPL_DATA[20:] self.assertEqual(expected4, data[upto3:])
def testPackX86RomIfwiSectiom(self): diff --git a/tools/binman/test/u_boot_binman_syms.c b/tools/binman/test/u_boot_binman_syms.c index 4898f983e3..4520b319f1 100644 --- a/tools/binman/test/u_boot_binman_syms.c +++ b/tools/binman/test/u_boot_binman_syms.c @@ -11,3 +11,4 @@ binman_sym_declare(unsigned long, u_boot_spl, offset); binman_sym_declare(unsigned long long, u_boot_spl2, offset); binman_sym_declare(unsigned long, u_boot_any, image_pos); +binman_sym_declare(unsigned long, u_boot_any, size);

It is useful to be able to access the size of an image in SPL, with something like:
binman_sym_declare(unsigned long, u_boot_any, size);
... ulong u_boot_size = binman_sym(ulong, u_boot_any, size);
Add support for this and update the tests.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/elf_test.py | 4 ++-- tools/binman/etype/section.py | 2 ++ tools/binman/ftest.py | 14 +++++++------- tools/binman/test/u_boot_binman_syms.c | 1 + 4 files changed, 12 insertions(+), 9 deletions(-)
Applied to u-boot-dm, thanks!

At present binman adds the image base address to the symbol value before it writes it to the binary. This is not correct since the symbol value itself (e.g. image position) has no relationship to the image base.
Fix this and update the tests to cover this case.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/elf.py | 4 +--- tools/binman/test/u_boot_binman_syms.lds | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/tools/binman/elf.py b/tools/binman/elf.py index 7bc7cf61b5..0c1a5b44b6 100644 --- a/tools/binman/elf.py +++ b/tools/binman/elf.py @@ -135,9 +135,7 @@ def LookupAndWriteSymbols(elf_fname, entry, section):
# Look up the symbol in our entry tables. value = section.LookupSymbol(name, sym.weak, msg) - if value is not None: - value += base.address - else: + if value is None: value = -1 pack_string = pack_string.lower() value_bytes = struct.pack(pack_string, value) diff --git a/tools/binman/test/u_boot_binman_syms.lds b/tools/binman/test/u_boot_binman_syms.lds index 926df873cb..825fc3f649 100644 --- a/tools/binman/test/u_boot_binman_syms.lds +++ b/tools/binman/test/u_boot_binman_syms.lds @@ -9,7 +9,7 @@ ENTRY(_start)
SECTIONS { - . = 0x00000000; + . = 0x00000010; _start = .;
. = ALIGN(4);

At present binman adds the image base address to the symbol value before it writes it to the binary. This is not correct since the symbol value itself (e.g. image position) has no relationship to the image base.
Fix this and update the tests to cover this case.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/elf.py | 4 +--- tools/binman/test/u_boot_binman_syms.lds | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-)
Applied to u-boot-dm, thanks!

On 9/26/19 6:38 PM, sjg@google.com wrote:
At present binman adds the image base address to the symbol value before it writes it to the binary. This is not correct since the symbol value itself (e.g. image position) has no relationship to the image base.
Fix this and update the tests to cover this case.
Signed-off-by: Simon Glass sjg@chromium.org
tools/binman/elf.py | 4 +--- tools/binman/test/u_boot_binman_syms.lds | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-)
Applied to u-boot-dm, thanks!
This seems to have only just been pushed. This patch breaks boot on Jetson TK1; u-boot-dm.git master hangs in SPL or before the main U-Boot prints anything, whereas after reverting this patch solves the issue.
With this patch applied, all I get is:
U-Boot SPL 2019.10-00490-g4f035abcde98 (Oct 14 2019 - 09:48:30 -0600) Trying to boot from RAM

Hi Stephen,
On Mon, 14 Oct 2019 at 09:49, Stephen Warren swarren@wwwdotorg.org wrote:
On 9/26/19 6:38 PM, sjg@google.com wrote:
At present binman adds the image base address to the symbol value before it writes it to the binary. This is not correct since the symbol value itself (e.g. image position) has no relationship to the image base.
Fix this and update the tests to cover this case.
Signed-off-by: Simon Glass sjg@chromium.org
tools/binman/elf.py | 4 +--- tools/binman/test/u_boot_binman_syms.lds | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-)
Applied to u-boot-dm, thanks!
This seems to have only just been pushed. This patch breaks boot on Jetson TK1; u-boot-dm.git master hangs in SPL or before the main U-Boot prints anything, whereas after reverting this patch solves the issue.
With this patch applied, all I get is:
U-Boot SPL 2019.10-00490-g4f035abcde98 (Oct 14 2019 - 09:48:30 -0600) Trying to boot from RAM
Yes, just pushed as I had to wait for u-boot-dm/testing to pass, and it has some flaky runs and then I went on holiday.
This is unfortunate. It looks like we were missing test coverage. I'll see if I can look at it later in the week, but for now I think I might drop this patch.
Regards, Simon

On 10/15/19 8:07 AM, Simon Glass wrote:
Hi Stephen,
On Mon, 14 Oct 2019 at 09:49, Stephen Warren swarren@wwwdotorg.org wrote:
On 9/26/19 6:38 PM, sjg@google.com wrote:
At present binman adds the image base address to the symbol value before it writes it to the binary. This is not correct since the symbol value itself (e.g. image position) has no relationship to the image base.
Fix this and update the tests to cover this case.
Signed-off-by: Simon Glass sjg@chromium.org
tools/binman/elf.py | 4 +--- tools/binman/test/u_boot_binman_syms.lds | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-)
Applied to u-boot-dm, thanks!
This seems to have only just been pushed. This patch breaks boot on Jetson TK1; u-boot-dm.git master hangs in SPL or before the main U-Boot prints anything, whereas after reverting this patch solves the issue.
With this patch applied, all I get is:
U-Boot SPL 2019.10-00490-g4f035abcde98 (Oct 14 2019 - 09:48:30 -0600) Trying to boot from RAM
Yes, just pushed as I had to wait for u-boot-dm/testing to pass, and it has some flaky runs and then I went on holiday.
This is unfortunate. It looks like we were missing test coverage. I'll see if I can look at it later in the week, but for now I think I might drop this patch.
Thanks. The latest push to u-boot-dm/master solves/removes this issue.

On 10/15/19 10:09 AM, Stephen Warren wrote:
On 10/15/19 8:07 AM, Simon Glass wrote:
Hi Stephen,
On Mon, 14 Oct 2019 at 09:49, Stephen Warren swarren@wwwdotorg.org wrote:
On 9/26/19 6:38 PM, sjg@google.com wrote:
At present binman adds the image base address to the symbol value before it writes it to the binary. This is not correct since the symbol value itself (e.g. image position) has no relationship to the image base.
Fix this and update the tests to cover this case.
Signed-off-by: Simon Glass sjg@chromium.org
tools/binman/elf.py | 4 +--- tools/binman/test/u_boot_binman_syms.lds | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-)
Applied to u-boot-dm, thanks!
This seems to have only just been pushed. This patch breaks boot on Jetson TK1; u-boot-dm.git master hangs in SPL or before the main U-Boot prints anything, whereas after reverting this patch solves the issue.
With this patch applied, all I get is:
U-Boot SPL 2019.10-00490-g4f035abcde98 (Oct 14 2019 - 09:48:30 -0600) Trying to boot from RAM
Yes, just pushed as I had to wait for u-boot-dm/testing to pass, and it has some flaky runs and then I went on holiday.
This is unfortunate. It looks like we were missing test coverage. I'll see if I can look at it later in the week, but for now I think I might drop this patch.
Thanks. The latest push to u-boot-dm/master solves/removes this issue.
This patch has now shown in in u-boot/master and u-boot-video/master, so Jetson TK1 testing is broken there now. Reverting this patch fixes the issue (I only tested that in u-boot/master).

Hi Stephen,
On Mon, 4 Nov 2019 at 10:34, Stephen Warren swarren@wwwdotorg.org wrote:
On 10/15/19 10:09 AM, Stephen Warren wrote:
On 10/15/19 8:07 AM, Simon Glass wrote:
Hi Stephen,
On Mon, 14 Oct 2019 at 09:49, Stephen Warren swarren@wwwdotorg.org wrote:
On 9/26/19 6:38 PM, sjg@google.com wrote:
At present binman adds the image base address to the symbol value before it writes it to the binary. This is not correct since the symbol value itself (e.g. image position) has no relationship to the image base.
Fix this and update the tests to cover this case.
Signed-off-by: Simon Glass sjg@chromium.org
tools/binman/elf.py | 4 +--- tools/binman/test/u_boot_binman_syms.lds | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-)
Applied to u-boot-dm, thanks!
This seems to have only just been pushed. This patch breaks boot on Jetson TK1; u-boot-dm.git master hangs in SPL or before the main U-Boot prints anything, whereas after reverting this patch solves the issue.
With this patch applied, all I get is:
U-Boot SPL 2019.10-00490-g4f035abcde98 (Oct 14 2019 - 09:48:30 -0600) Trying to boot from RAM
Yes, just pushed as I had to wait for u-boot-dm/testing to pass, and it has some flaky runs and then I went on holiday.
This is unfortunate. It looks like we were missing test coverage. I'll see if I can look at it later in the week, but for now I think I might drop this patch.
Thanks. The latest push to u-boot-dm/master solves/removes this issue.
This patch has now shown in in u-boot/master and u-boot-video/master, so Jetson TK1 testing is broken there now. Reverting this patch fixes the issue (I only tested that in u-boot/master).
Arggh OK I will sort this out this week.
Regards, Simon

The Intel FSP supports initialising memory early during boot using a binary blob called 'fspm'. Add support for this.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/README.entries | 17 +++++++++++++++++ tools/binman/etype/intel_fsp_m.py | 27 +++++++++++++++++++++++++++ tools/binman/ftest.py | 8 ++++++++ tools/binman/test/152_intel_fsp_m.dts | 14 ++++++++++++++ 4 files changed, 66 insertions(+) create mode 100644 tools/binman/etype/intel_fsp_m.py create mode 100644 tools/binman/test/152_intel_fsp_m.dts
diff --git a/tools/binman/README.entries b/tools/binman/README.entries index dba51e6daa..bce2244596 100644 --- a/tools/binman/README.entries +++ b/tools/binman/README.entries @@ -427,6 +427,23 @@ See README.x86 for information about x86 binary blobs.
+Entry: intel-fsp-m: Entry containing Intel Firmware Support Package (FSP) memory init +------------------------------------------------------------------------------------- + +Properties / Entry arguments: + - filename: Filename of file to read into entry + +This file contains a binary blob which is used on some devices to set up +SDRAM. U-Boot executes this code in SPL so that it can make full use of +memory. Documentation is typically not available in sufficient detail to +allow U-Boot do this this itself.. + +An example filename is 'fsp_m.bin' + +See README.x86 for information about x86 binary blobs. + + + Entry: intel-ifwi: Entry containing an Intel Integrated Firmware Image (IFWI) file ----------------------------------------------------------------------------------
diff --git a/tools/binman/etype/intel_fsp_m.py b/tools/binman/etype/intel_fsp_m.py new file mode 100644 index 0000000000..2d6b2b6621 --- /dev/null +++ b/tools/binman/etype/intel_fsp_m.py @@ -0,0 +1,27 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright 2019 Google LLC +# Written by Simon Glass sjg@chromium.org +# +# Entry-type module for Intel Firmware Support Package binary blob (T section) +# + +from entry import Entry +from blob import Entry_blob + +class Entry_intel_fsp_m(Entry_blob): + """Entry containing Intel Firmware Support Package (FSP) memory init + + Properties / Entry arguments: + - filename: Filename of file to read into entry + + This file contains a binary blob which is used on some devices to set up + SDRAM. U-Boot executes this code in SPL so that it can make full use of + memory. Documentation is typically not available in sufficient detail to + allow U-Boot do this this itself.. + + An example filename is 'fsp_m.bin' + + See README.x86 for information about x86 binary blobs. + """ + def __init__(self, section, etype, node): + Entry_blob.__init__(self, section, etype, node) diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index a740c3e03d..b0b942dfdc 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -72,6 +72,7 @@ FILES_DATA = (b"sorry I'm late\nOh, don't bother apologising, I'm " + b"sorry you're alive\n") COMPRESS_DATA = b'compress xxxxxxxxxxxxxxxxxxxxxx data' REFCODE_DATA = b'refcode' +FSP_M_DATA = b'fsp_m'
# The expected size for the device tree in some tests EXTRACT_DTB_SIZE = 0x3c9 @@ -147,6 +148,7 @@ class TestFunctional(unittest.TestCase): TestFunctional._MakeInputDir('devkeys') TestFunctional._MakeInputFile('bmpblk.bin', BMPBLK_DATA) TestFunctional._MakeInputFile('refcode.bin', REFCODE_DATA) + TestFunctional._MakeInputFile('fsp_m.bin', FSP_M_DATA)
cls._elf_testdir = os.path.join(cls._indir, 'elftest') elf_test.BuildElfTestFiles(cls._elf_testdir) @@ -3329,6 +3331,12 @@ class TestFunctional(unittest.TestCase): data = self._DoReadFile('151_x86_rom_ifwi_section.dts') self._CheckIfwi(data)
+ def testPackFspM(self): + """Test that an image with a FSP memory-init binary can be created""" + data = self._DoReadFile('152_intel_fsp_m.dts') + self.assertEqual(FSP_M_DATA, data[:len(FSP_M_DATA)]) + +
if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/152_intel_fsp_m.dts b/tools/binman/test/152_intel_fsp_m.dts new file mode 100644 index 0000000000..b6010f31c2 --- /dev/null +++ b/tools/binman/test/152_intel_fsp_m.dts @@ -0,0 +1,14 @@ +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + size = <16>; + + intel-fsp-m { + filename = "fsp_m.bin"; + }; + }; +};

The Intel FSP supports initialising memory early during boot using a binary blob called 'fspm'. Add support for this.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/README.entries | 17 +++++++++++++++++ tools/binman/etype/intel_fsp_m.py | 27 +++++++++++++++++++++++++++ tools/binman/ftest.py | 8 ++++++++ tools/binman/test/152_intel_fsp_m.dts | 14 ++++++++++++++ 4 files changed, 66 insertions(+) create mode 100644 tools/binman/etype/intel_fsp_m.py create mode 100644 tools/binman/test/152_intel_fsp_m.dts
Applied to u-boot-dm, thanks!

This comment references another entry type. Fix it.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/etype/intel_descriptor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/binman/etype/intel_descriptor.py b/tools/binman/etype/intel_descriptor.py index fb5e889ebf..b6477931d6 100644 --- a/tools/binman/etype/intel_descriptor.py +++ b/tools/binman/etype/intel_descriptor.py @@ -2,7 +2,7 @@ # Copyright (c) 2016 Google, Inc # Written by Simon Glass sjg@chromium.org # -# Entry-type module for 'u-boot' +# Entry-type module for Intel flash descriptor #
import struct

This comment references another entry type. Fix it.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/etype/intel_descriptor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Applied to u-boot-dm, thanks!

At present this class reads its entries in the constructor. This is not how things should be done now. Update it.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/etype/intel_ifwi.py | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/tools/binman/etype/intel_ifwi.py b/tools/binman/etype/intel_ifwi.py index e4da3e498a..ef2b35706f 100644 --- a/tools/binman/etype/intel_ifwi.py +++ b/tools/binman/etype/intel_ifwi.py @@ -48,7 +48,10 @@ class Entry_intel_ifwi(Entry_blob): Entry_blob.__init__(self, section, etype, node) self._convert_fit = fdt_util.GetBool(self._node, 'convert-fit') self._ifwi_entries = OrderedDict() + + def ReadNode(self): self._ReadSubnodes() + Entry_blob.ReadNode(self)
def ObtainContents(self): """Get the contects for the IFWI

At present this class reads its entries in the constructor. This is not how things should be done now. Update it.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/etype/intel_ifwi.py | 3 +++ 1 file changed, 3 insertions(+)
Applied to u-boot-dm, thanks!

Add support for the ProcessContents() method in this entry so that it is possible to support entries which change after initial creation.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/etype/intel_ifwi.py | 46 +++++++++++++++++++------------- 1 file changed, 28 insertions(+), 18 deletions(-)
diff --git a/tools/binman/etype/intel_ifwi.py b/tools/binman/etype/intel_ifwi.py index ef2b35706f..17792defe9 100644 --- a/tools/binman/etype/intel_ifwi.py +++ b/tools/binman/etype/intel_ifwi.py @@ -53,22 +53,8 @@ class Entry_intel_ifwi(Entry_blob): self._ReadSubnodes() Entry_blob.ReadNode(self)
- def ObtainContents(self): - """Get the contects for the IFWI - - Unfortunately we cannot create anything from scratch here, as Intel has - tools which create precursor binaries with lots of data and settings, - and these are not incorporated into binman. - - The first step is to get a file in the IFWI format. This is either - supplied directly or is extracted from a fitimage using the 'create' - subcommand. - - After that we delete the OBBP sub-partition and add each of the files - that we want in the IFWI file, one for each sub-entry of the IWFI node. - """ - self._pathname = tools.GetInputFilename(self._filename) - + def _BuildIfwi(self): + """Build the contents of the IFWI and write it to the 'data' property""" # Create the IFWI file if needed if self._convert_fit: inname = self._pathname @@ -85,8 +71,6 @@ class Entry_intel_ifwi(Entry_blob):
for entry in self._ifwi_entries.values(): # First get the input data and put it in a file - if not entry.ObtainContents(): - return False data = entry.GetData() uniq = self.GetUniqueName() input_fname = tools.GetOutputFilename('input.%s' % uniq) @@ -99,6 +83,32 @@ class Entry_intel_ifwi(Entry_blob): self.ReadBlobContents() return True
+ def ObtainContents(self): + """Get the contects for the IFWI + + Unfortunately we cannot create anything from scratch here, as Intel has + tools which create precursor binaries with lots of data and settings, + and these are not incorporated into binman. + + The first step is to get a file in the IFWI format. This is either + supplied directly or is extracted from a fitimage using the 'create' + subcommand. + + After that we delete the OBBP sub-partition and add each of the files + that we want in the IFWI file, one for each sub-entry of the IWFI node. + """ + self._pathname = tools.GetInputFilename(self._filename) + for entry in self._ifwi_entries.values(): + if not entry.ObtainContents(): + return False + return self._BuildIfwi() + + def ProcessContents(self): + orig_data = self.data + self._BuildIfwi() + same = orig_data == self.data + return same + def _ReadSubnodes(self): """Read the subnodes to find out what should go in this IFWI""" for node in self._node.subnodes:

Add support for the ProcessContents() method in this entry so that it is possible to support entries which change after initial creation.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/etype/intel_ifwi.py | 46 +++++++++++++++++++------------- 1 file changed, 28 insertions(+), 18 deletions(-)
Applied to u-boot-dm, thanks!

The Intel IFWI (Integrated Firmware Image) is effectively a section with other entries inside it. Support writing symbol information into entries within it.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/etype/intel_ifwi.py | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/tools/binman/etype/intel_ifwi.py b/tools/binman/etype/intel_ifwi.py index 17792defe9..36aadc210c 100644 --- a/tools/binman/etype/intel_ifwi.py +++ b/tools/binman/etype/intel_ifwi.py @@ -118,3 +118,8 @@ class Entry_intel_ifwi(Entry_blob): entry._ifwi_subpart = fdt_util.GetString(node, 'ifwi-subpart') entry._ifwi_entry_name = fdt_util.GetString(node, 'ifwi-entry') self._ifwi_entries[entry._ifwi_subpart] = entry + + def WriteSymbols(self, section): + """Write symbol values into binary files for access at run time""" + for entry in self._ifwi_entries.values(): + entry.WriteSymbols(self)

The Intel IFWI (Integrated Firmware Image) is effectively a section with other entries inside it. Support writing symbol information into entries within it.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/etype/intel_ifwi.py | 5 +++++ 1 file changed, 5 insertions(+)
Applied to u-boot-dm, thanks!

At present the symbol information is written to binaries just before binman exits. This is fine for entries within sections since the section contents is calculated when it is needed, so the updated symbol values are included in the image that is written.
However some binaries are inside entries which have already generated their contents and do not notice that the entries have changed (e.g. Intel IFWI).
Move the symbol writing earlier to cope with this.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/control.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/binman/control.py b/tools/binman/control.py index 43f5d49406..07dffbd89c 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -434,6 +434,7 @@ def ProcessImage(image, update_fdt, write_map, get_contents=True, for dtb_item in state.GetAllFdts(): dtb_item.Sync() dtb_item.Flush() + image.WriteSymbols() sizes_ok = image.ProcessEntryContents() if sizes_ok: break @@ -442,7 +443,6 @@ def ProcessImage(image, update_fdt, write_map, get_contents=True, image.Raise('Entries changed size after packing (tried %s passes)' % passes)
- image.WriteSymbols() image.BuildImage() if write_map: image.WriteMap()

At present the symbol information is written to binaries just before binman exits. This is fine for entries within sections since the section contents is calculated when it is needed, so the updated symbol values are included in the image that is written.
However some binaries are inside entries which have already generated their contents and do not notice that the entries have changed (e.g. Intel IFWI).
Move the symbol writing earlier to cope with this.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/control.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Applied to u-boot-dm, thanks!

Sometimes binman takes multiple passes to complete packing an image. Add logging to indicate this.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/control.py | 1 + 1 file changed, 1 insertion(+)
diff --git a/tools/binman/control.py b/tools/binman/control.py index 07dffbd89c..d1ca798cfb 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -439,6 +439,7 @@ def ProcessImage(image, update_fdt, write_map, get_contents=True, if sizes_ok: break image.ResetForPack() + tout.Info('Pack completed after %d pass(es)' % (pack_pass + 1)) if not sizes_ok: image.Raise('Entries changed size after packing (tried %s passes)' % passes)

Sometimes binman takes multiple passes to complete packing an image. Add logging to indicate this.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/control.py | 1 + 1 file changed, 1 insertion(+)
Applied to u-boot-dm, thanks!

This code is not needed so drop it.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/ftest.py | 2 -- 1 file changed, 2 deletions(-)
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index b0b942dfdc..0a66369cbd 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -1669,8 +1669,6 @@ class TestFunctional(unittest.TestCase): # source file (e.g. test/075_fdt_update_all.dts) thus does not enter # binman as a file called u-boot.dtb. To fix this, copy the file # over to the expected place. - #tools.WriteFile(os.path.join(self._indir, 'u-boot.dtb'), - #tools.ReadFile(tools.GetOutputFilename('source.dtb'))) start = 0 for fname in ['u-boot.dtb.out', 'spl/u-boot-spl.dtb.out', 'tpl/u-boot-tpl.dtb.out']:

This code is not needed so drop it.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/ftest.py | 2 -- 1 file changed, 2 deletions(-)
Applied to u-boot-dm, thanks!
participants (4)
-
Simon Glass
-
Simon Glass
-
sjg@google.com
-
Stephen Warren