diff options
author | Tom Rini <trini@konsulko.com> | 2018-07-10 10:29:14 -0400 |
---|---|---|
committer | Tom Rini <trini@konsulko.com> | 2018-07-10 10:29:14 -0400 |
commit | e3396ffd720877976141fa0b76a0b8ee9643d7d1 (patch) | |
tree | 7baee90ec8c5759a420f37a796b91fc77f69e7c7 /tools/binman | |
parent | 495c70f9dfad1a5428ec84b52e8667ea4760ecd6 (diff) | |
parent | 16b8d6b76992690c65c58dc8b0591496cc5e46ef (diff) |
Merge git://git.denx.de/u-boot-dm
Diffstat (limited to 'tools/binman')
23 files changed, 534 insertions, 131 deletions
diff --git a/tools/binman/README b/tools/binman/README index 22f21bc5b4..207928aa95 100644 --- a/tools/binman/README +++ b/tools/binman/README @@ -462,7 +462,22 @@ Order of image creation Image creation proceeds in the following order, for each entry in the image. -1. GetEntryContents() - the contents of each entry are obtained, normally by +1. AddMissingProperties() - binman can add calculated values to the device +tree as part of its processing, for example the position and size of each +entry. This method adds any properties associated with this, expanding the +device tree as needed. These properties can have placeholder values which are +set later by SetCalculatedProperties(). By that stage the size of sections +cannot be changed (since it would cause the images to need to be repacked), +but the correct values can be inserted. + +2. ProcessFdt() - process the device tree information as required by the +particular entry. This may involve adding or deleting properties. If the +processing is complete, this method should return True. If the processing +cannot complete because it needs the ProcessFdt() method of another entry to +run first, this method should return False, in which case it will be called +again later. + +3. GetEntryContents() - the contents of each entry are obtained, normally by reading from a file. This calls the Entry.ObtainContents() to read the contents. The default version of Entry.ObtainContents() calls Entry.GetDefaultFilename() and then reads that file. So a common mechanism @@ -471,33 +486,38 @@ functions must return True when they have read the contents. Binman will retry calling the functions a few times if False is returned, allowing dependencies between the contents of different entries. -2. GetEntryPositions() - calls Entry.GetPositions() for each entry. This can +4. GetEntryPositions() - calls Entry.GetPositions() for each entry. This can return a dict containing entries that need updating. The key should be the entry name and the value is a tuple (pos, size). This allows an entry to provide the position and size for other entries. The default implementation of GetEntryPositions() returns {}. -3. PackEntries() - calls Entry.Pack() which figures out the position and +5. PackEntries() - calls Entry.Pack() which figures out the position and size of an entry. The 'current' image position is passed in, and the function returns the position immediately after the entry being packed. The default implementation of Pack() is usually sufficient. -4. CheckSize() - checks that the contents of all the entries fits within +6. CheckSize() - checks that the contents of all the entries fits within the image size. If the image does not have a defined size, the size is set large enough to hold all the entries. -5. CheckEntries() - checks that the entries do not overlap, nor extend +7. CheckEntries() - checks that the entries do not overlap, nor extend outside the image. -6. ProcessEntryContents() - this calls Entry.ProcessContents() on each entry. +8. SetCalculatedProperties() - update any calculated properties in the device +tree. This sets the correct 'pos' and 'size' vaues, for example. + +9. ProcessEntryContents() - this calls Entry.ProcessContents() on each entry. The default implementatoin does nothing. This can be overriden to adjust the contents of an entry in some way. For example, it would be possible to create an entry containing a hash of the contents of some other entries. At this stage the position and size of entries should not be adjusted. -6. WriteEntryInfo() +10. WriteSymbols() - write the value of symbols into the U-Boot SPL binary. +See 'Access to binman entry positions at run time' below for a description of +what happens in this stage. -7. BuildImage() - builds the image and writes it to a file. This is the final +11. BuildImage() - builds the image and writes it to a file. This is the final step. @@ -583,8 +603,7 @@ implementations target 100% test coverage. Run 'binman -T' to check this. To enable Python test coverage on Debian-type distributions (e.g. Ubuntu): - $ sudo apt-get install python-pip python-pytest - $ sudo pip install coverage + $ sudo apt-get install python-coverage python-pytest Advanced Features / Technical docs @@ -650,13 +669,11 @@ To do ----- Some ideas: -- Fill out the device tree to include the final position and size of each - entry (since the input file may not always specify these). See also - 'Access to binman entry positions at run time' above - Use of-platdata to make the information available to code that is unable to use device tree (such as a very small SPL image) - Allow easy building of images by specifying just the board name -- Produce a full Python binding for libfdt (for upstream) +- Produce a full Python binding for libfdt (for upstream). This is nearing + completion but some work remains - Add an option to decode an image into the constituent binaries - Support building an image for a board (-b) more completely, with a configurable build directory diff --git a/tools/binman/binman.py b/tools/binman/binman.py index 31b045337d..52e02ed91b 100755 --- a/tools/binman/binman.py +++ b/tools/binman/binman.py @@ -26,6 +26,7 @@ sys.path.insert(0, 'scripts/dtc/pylibfdt') import cmdline import command import control +import test_util def RunTests(debug, args): """Run the functional tests and any embedded doctests @@ -54,14 +55,12 @@ def RunTests(debug, args): # Run the entry tests first ,since these need to be the first to import the # 'entry' module. - suite = unittest.TestLoader().loadTestsFromTestCase(entry_test.TestEntry) - suite.run(result) test_name = args and args[0] or None - for module in (ftest.TestFunctional, fdt_test.TestFdt, elf_test.TestElf, - image_test.TestImage): + for module in (entry_test.TestEntry, ftest.TestFunctional, fdt_test.TestFdt, + elf_test.TestElf, image_test.TestImage): if test_name: try: - suite = unittest.TestLoader().loadTestsFromName(args[0], module) + suite = unittest.TestLoader().loadTestsFromName(test_name, module) except AttributeError: continue else: @@ -80,33 +79,12 @@ def RunTests(debug, args): def RunTestCoverage(): """Run the tests and check that we get 100% coverage""" - # This uses the build output from sandbox_spl to get _libfdt.so - cmd = ('PYTHONPATH=$PYTHONPATH:%s/sandbox_spl/tools coverage run ' - '--include "tools/binman/*.py" --omit "*test*,*binman.py" ' - 'tools/binman/binman.py -t' % options.build_dir) - os.system(cmd) - stdout = command.Output('coverage', 'report') - lines = stdout.splitlines() - - test_set= set([os.path.basename(line.split()[0]) - for line in lines if '/etype/' in line]) glob_list = glob.glob(os.path.join(our_path, 'etype/*.py')) - all_set = set([os.path.basename(item) for item in glob_list]) - missing_list = all_set - missing_list.difference_update(test_set) - missing_list.remove('_testing.py') - coverage = lines[-1].split(' ')[-1] - ok = True - if missing_list: - print 'Missing tests for %s' % (', '.join(missing_list)) - ok = False - if coverage != '100%': - print stdout - print "Type 'coverage html' to get a report in htmlcov/index.html" - print 'Coverage error: %s, but should be 100%%' % coverage - ok = False - if not ok: - raise ValueError('Test coverage failure') + all_set = set([os.path.splitext(os.path.basename(item))[0] + for item in glob_list if '_testing' not in item]) + test_util.RunTestCoverage('tools/binman/binman.py', None, + ['*test*', '*binman.py', 'tools/patman/*', 'tools/dtoc/*'], + options.build_dir, all_set) def RunBinman(options, args): """Main entry point to binman once arguments are parsed diff --git a/tools/binman/bsection.py b/tools/binman/bsection.py index 3f30f6e4fe..de439ef625 100644 --- a/tools/binman/bsection.py +++ b/tools/binman/bsection.py @@ -90,6 +90,29 @@ class Section(object): entry.SetPrefix(self._name_prefix) self._entries[node.name] = entry + def AddMissingProperties(self): + for entry in self._entries.values(): + entry.AddMissingProperties() + + def SetCalculatedProperties(self): + for entry in self._entries.values(): + entry.SetCalculatedProperties() + + def ProcessFdt(self, fdt): + todo = self._entries.values() + for passnum in range(3): + next_todo = [] + for entry in todo: + if not entry.ProcessFdt(fdt): + next_todo.append(entry) + todo = next_todo + if not todo: + break + if todo: + self._Raise('Internal error: Could not complete processing of Fdt: ' + 'remaining %s' % todo) + return True + def CheckSize(self): """Check that the section contents does not exceed its size, etc.""" contents_size = 0 @@ -162,6 +185,10 @@ class Section(object): todo = next_todo if not todo: break + if todo: + self._Raise('Internal error: Could not complete processing of ' + 'contents: remaining %s' % todo) + return True def _SetEntryPosSize(self, name, pos, size): """Set the position and size of an entry diff --git a/tools/binman/cmdline.py b/tools/binman/cmdline.py index bf63919eb7..ae2d1670f9 100644 --- a/tools/binman/cmdline.py +++ b/tools/binman/cmdline.py @@ -42,6 +42,8 @@ def ParseArgs(argv): default=False, help='run tests') parser.add_option('-T', '--test-coverage', action='store_true', default=False, help='run tests and check for 100% coverage') + parser.add_option('-u', '--update-fdt', action='store_true', + default=False, help='Update the binman node with position/size info') parser.add_option('-v', '--verbosity', default=1, type='int', help='Control verbosity: 0=silent, 1=progress, 3=full, ' '4=debug') diff --git a/tools/binman/control.py b/tools/binman/control.py index 9243472906..a40b300fda 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -21,6 +21,11 @@ import tout # Make this global so that it can be referenced from tests images = OrderedDict() +# Records the device-tree files known to binman, keyed by filename (e.g. +# 'u-boot-spl.dtb') +fdt_files = {} + + def _ReadImageDesc(binman_node): """Read the image descriptions from the /binman node @@ -53,6 +58,24 @@ def _FindBinmanNode(dtb): return node return None +def GetFdt(fname): + """Get the Fdt object for a particular device-tree filename + + Binman keeps track of at least one device-tree file called u-boot.dtb but + can also have others (e.g. for SPL). This function looks up the given + filename and returns the associated Fdt object. + + Args: + fname: Filename to look up (e.g. 'u-boot.dtb'). + + Returns: + Fdt object associated with the filename + """ + return fdt_files[fname] + +def GetFdtPath(fname): + return fdt_files[fname]._fname + def Binman(options, args): """The main control code for binman @@ -93,12 +116,41 @@ def Binman(options, args): try: tools.SetInputDirs(options.indir) tools.PrepareOutputDir(options.outdir, options.preserve) - dtb = fdt.FdtScan(dtb_fname) + + # Get the device tree ready by compiling it and copying the compiled + # output into a file in our output directly. Then scan it for use + # in binman. + dtb_fname = fdt_util.EnsureCompiled(dtb_fname) + fname = tools.GetOutputFilename('u-boot-out.dtb') + with open(dtb_fname) as infd: + with open(fname, 'wb') as outfd: + outfd.write(infd.read()) + dtb = fdt.FdtScan(fname) + + # Note the file so that GetFdt() can find it + fdt_files['u-boot.dtb'] = dtb node = _FindBinmanNode(dtb) if not node: raise ValueError("Device tree '%s' does not have a 'binman' " "node" % dtb_fname) + images = _ReadImageDesc(node) + + # Prepare the device tree by making sure that any missing + # properties are added (e.g. 'pos' and 'size'). The values of these + # may not be correct yet, but we add placeholders so that the + # size of the device tree is correct. Later, in + # SetCalculatedProperties() we will insert the correct values + # without changing the device-tree size, thus ensuring that our + # entry positions remain the same. + for image in images.values(): + if options.update_fdt: + image.AddMissingProperties() + image.ProcessFdt(dtb) + + dtb.Pack() + dtb.Flush() + for image in images.values(): # Perform all steps for this image, including checking and # writing it. This means that errors found with a later @@ -109,11 +161,15 @@ def Binman(options, args): image.PackEntries() image.CheckSize() image.CheckEntries() + if options.update_fdt: + image.SetCalculatedProperties() image.ProcessEntryContents() image.WriteSymbols() image.BuildImage() if options.map: image.WriteMap() + with open(fname, 'wb') as outfd: + outfd.write(dtb.GetContents()) finally: tools.FinaliseOutputDir() finally: diff --git a/tools/binman/elf_test.py b/tools/binman/elf_test.py index fb6e451cf0..9c8f1feca8 100644 --- a/tools/binman/elf_test.py +++ b/tools/binman/elf_test.py @@ -4,33 +4,15 @@ # # Test for the elf module -from contextlib import contextmanager import os import sys import unittest -try: - from StringIO import StringIO -except ImportError: - from io import StringIO - import elf +import test_util binman_dir = os.path.dirname(os.path.realpath(sys.argv[0])) -# Use this to suppress stdout/stderr output: -# with capture_sys_output() as (stdout, stderr) -# ...do something... -@contextmanager -def capture_sys_output(): - capture_out, capture_err = StringIO(), StringIO() - old_out, old_err = sys.stdout, sys.stderr - try: - sys.stdout, sys.stderr = capture_out, capture_err - yield capture_out, capture_err - finally: - sys.stdout, sys.stderr = old_out, old_err - class FakeEntry: def __init__(self, contents_size): @@ -110,7 +92,7 @@ class TestElf(unittest.TestCase): entry = FakeEntry(20) section = FakeSection() elf_fname = os.path.join(binman_dir, 'test', 'u_boot_binman_syms') - with capture_sys_output() as (stdout, stderr): + with test_util.capture_sys_output() as (stdout, stderr): syms = elf.LookupAndWriteSymbols(elf_fname, entry, section) elf.debug = False self.assertTrue(len(stdout.getvalue()) > 0) diff --git a/tools/binman/entry.py b/tools/binman/entry.py index e4d688c91f..6a173e663d 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -55,6 +55,7 @@ class Entry(object): self.name = node and (name_prefix + node.name) or 'none' self.pos = None self.size = None + self.data = '' self.contents_size = 0 self.align = None self.align_size = None @@ -129,6 +130,20 @@ class Entry(object): self.align_end = fdt_util.GetInt(self._node, 'align-end') self.pos_unset = fdt_util.GetBool(self._node, 'pos-unset') + def AddMissingProperties(self): + """Add new properties to the device tree as needed for this entry""" + for prop in ['pos', 'size']: + if not prop in self._node.props: + self._node.AddZeroProp(prop) + + def SetCalculatedProperties(self): + """Set the value of device-tree properties calculated by binman""" + self._node.SetInt('pos', self.pos) + self._node.SetInt('size', self.size) + + def ProcessFdt(self, fdt): + return True + def SetPrefix(self, prefix): """Set the name prefix for a node @@ -138,6 +153,33 @@ class Entry(object): if prefix: self.name = prefix + self.name + def SetContents(self, data): + """Set the contents of an entry + + This sets both the data and content_size properties + + Args: + data: Data to set to the contents (string) + """ + self.data = data + self.contents_size = len(self.data) + + def ProcessContentsUpdate(self, data): + """Update the contens of an entry, after the size is fixed + + This checks that the new data is the same size as the old. + + Args: + data: Data to set to the contents (string) + + Raises: + ValueError if the new data size is not the same as the old + """ + if len(data) != self.contents_size: + self.Raise('Cannot update entry size from %d to %d' % + (len(data), self.contents_size)) + self.SetContents(data) + def ObtainContents(self): """Figure out the contents of an entry. diff --git a/tools/binman/etype/_testing.py b/tools/binman/etype/_testing.py index c376dd5c9c..6a1af57798 100644 --- a/tools/binman/etype/_testing.py +++ b/tools/binman/etype/_testing.py @@ -9,17 +9,46 @@ from entry import Entry import fdt_util import tools + class Entry__testing(Entry): + """A fake entry used for testing + + Properties: + return_invalid_entry: Return an invalid entry from GetPositions() + """ def __init__(self, section, etype, node): Entry.__init__(self, section, etype, node) + self.return_invalid_entry = fdt_util.GetBool(self._node, + 'return-invalid-entry') + self.return_unknown_contents = fdt_util.GetBool(self._node, + 'return-unknown-contents') + self.bad_update_contents = fdt_util.GetBool(self._node, + 'bad-update-contents') + self.process_fdt_ready = False + self.never_complete_process_fdt = fdt_util.GetBool(self._node, + 'never-complete-process-fdt') def ObtainContents(self): + if self.return_unknown_contents: + return False self.data = 'a' self.contents_size = len(self.data) return True - def ReadContents(self): - return True - def GetPositions(self): - return {'invalid-entry': [1, 2]} + if self.return_invalid_entry : + return {'invalid-entry': [1, 2]} + return {} + + def ProcessContents(self): + if self.bad_update_contents: + # Request to update the conents with something larger, to cause a + # failure. + self.ProcessContentsUpdate('aa') + + def ProcessFdt(self, fdt): + """Force reprocessing the first time""" + ready = self.process_fdt_ready + if not self.never_complete_process_fdt: + self.process_fdt_ready = True + return ready diff --git a/tools/binman/etype/blob.py b/tools/binman/etype/blob.py index 16b1e5f64d..28e6651a93 100644 --- a/tools/binman/etype/blob.py +++ b/tools/binman/etype/blob.py @@ -28,8 +28,7 @@ class Entry_blob(Entry): # new Entry method which can read in chunks. Then we could copy # the data in chunks and avoid reading it all at once. For now # this seems like an unnecessary complication. - self.data = fd.read() - self.contents_size = len(self.data) + self.SetContents(fd.read()) return True def GetDefaultFilename(self): diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index 139fcad51a..787257d3ec 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -17,8 +17,15 @@ class Entry_section(Entry): Entry.__init__(self, image, etype, node) self._section = bsection.Section(node.name, node) + def ProcessFdt(self, fdt): + return self._section.ProcessFdt(fdt) + + def AddMissingProperties(self): + Entry.AddMissingProperties(self) + self._section.AddMissingProperties() + def ObtainContents(self): - self._section.GetEntryContents() + return self._section.GetEntryContents() def GetData(self): return self._section.GetData() @@ -42,6 +49,10 @@ class Entry_section(Entry): """Write symbol values into binary files for access at run time""" self._section.WriteSymbols() + def SetCalculatedProperties(self): + Entry.SetCalculatedProperties(self) + self._section.SetCalculatedProperties() + def ProcessContents(self): self._section.ProcessEntryContents() super(Entry_section, self).ProcessContents() diff --git a/tools/binman/etype/u_boot_dtb_with_ucode.py b/tools/binman/etype/u_boot_dtb_with_ucode.py index 1e530d6570..3808d6d278 100644 --- a/tools/binman/etype/u_boot_dtb_with_ucode.py +++ b/tools/binman/etype/u_boot_dtb_with_ucode.py @@ -5,6 +5,7 @@ # Entry-type module for U-Boot device tree with the microcode removed # +import control import fdt from entry import Entry from blob import Entry_blob @@ -22,13 +23,13 @@ class Entry_u_boot_dtb_with_ucode(Entry_blob): self.collate = False self.ucode_offset = None self.ucode_size = None + self.ucode = None + self.ready = False def GetDefaultFilename(self): return 'u-boot.dtb' - def ObtainContents(self): - Entry_blob.ObtainContents(self) - + def ProcessFdt(self, fdt): # If the section does not need microcode, there is nothing to do ucode_dest_entry = self.section.FindEntryType( 'u-boot-spl-with-ucode-ptr') @@ -38,36 +39,35 @@ class Entry_u_boot_dtb_with_ucode(Entry_blob): if not ucode_dest_entry or not ucode_dest_entry.target_pos: return True - # Create a new file to hold the copied device tree - dtb_name = 'u-boot-dtb-with-ucode.dtb' - fname = tools.GetOutputFilename(dtb_name) - with open(fname, 'wb') as fd: - fd.write(self.data) - # Remove the microcode - dtb = fdt.FdtScan(fname) - ucode = dtb.GetNode('/microcode') - if not ucode: + fname = self.GetDefaultFilename() + fdt = control.GetFdt(fname) + self.ucode = fdt.GetNode('/microcode') + if not self.ucode: raise self.Raise("No /microcode node found in '%s'" % fname) # There's no need to collate it (move all microcode into one place) # if we only have one chunk of microcode. - self.collate = len(ucode.subnodes) > 1 - for node in ucode.subnodes: + self.collate = len(self.ucode.subnodes) > 1 + for node in self.ucode.subnodes: data_prop = node.props.get('data') if data_prop: self.ucode_data += ''.join(data_prop.bytes) if self.collate: - prop = node.DeleteProp('data') - else: + node.DeleteProp('data') + return True + + def ObtainContents(self): + # Call the base class just in case it does something important. + Entry_blob.ObtainContents(self) + self._pathname = control.GetFdtPath(self._filename) + self.ReadContents() + if self.ucode: + for node in self.ucode.subnodes: + data_prop = node.props.get('data') + if data_prop and not self.collate: # Find the offset in the device tree of the ucode data self.ucode_offset = data_prop.GetOffset() + 12 self.ucode_size = len(data_prop.bytes) - if self.collate: - dtb.Pack() - dtb.Flush() - - # Make this file the contents of this entry - self._pathname = fname - self.ReadContents() + self.ready = True return True diff --git a/tools/binman/etype/u_boot_spl_bss_pad.py b/tools/binman/etype/u_boot_spl_bss_pad.py index 3d2dea2e0d..65f631d3c5 100644 --- a/tools/binman/etype/u_boot_spl_bss_pad.py +++ b/tools/binman/etype/u_boot_spl_bss_pad.py @@ -22,5 +22,5 @@ class Entry_u_boot_spl_bss_pad(Entry_blob): bss_size = elf.GetSymbolAddress(fname, '__bss_size') if not bss_size: self.Raise('Expected __bss_size symbol in spl/u-boot-spl') - self.data = chr(0) * bss_size - self.contents_size = bss_size + self.SetContents(chr(0) * bss_size) + return True diff --git a/tools/binman/etype/u_boot_ucode.py b/tools/binman/etype/u_boot_ucode.py index 3a0cff7c3a..ea0c85cc5c 100644 --- a/tools/binman/etype/u_boot_ucode.py +++ b/tools/binman/etype/u_boot_ucode.py @@ -64,9 +64,14 @@ class Entry_u_boot_ucode(Entry_blob): self.data = '' return True - # Get the microcode from the device tree entry + # Get the microcode from the device tree entry. If it is not available + # yet, return False so we will be called later. If the section simply + # doesn't exist, then we may as well return True, since we are going to + # get an error anyway. fdt_entry = self.section.FindEntryType('u-boot-dtb-with-ucode') - if not fdt_entry or not fdt_entry.ucode_data: + if not fdt_entry: + return True + if not fdt_entry.ready: return False if not fdt_entry.collate: diff --git a/tools/binman/etype/u_boot_with_ucode_ptr.py b/tools/binman/etype/u_boot_with_ucode_ptr.py index 41c2ded2fe..8b1e41152e 100644 --- a/tools/binman/etype/u_boot_with_ucode_ptr.py +++ b/tools/binman/etype/u_boot_with_ucode_ptr.py @@ -28,7 +28,7 @@ class Entry_u_boot_with_ucode_ptr(Entry_blob): def GetDefaultFilename(self): return 'u-boot-nodtb.bin' - def ObtainContents(self): + def ProcessFdt(self, fdt): # Figure out where to put the microcode pointer fname = tools.GetInputFilename(self.elf_fname) sym = elf.GetSymbolAddress(fname, '_dt_ucode_base_size') @@ -36,8 +36,7 @@ class Entry_u_boot_with_ucode_ptr(Entry_blob): self.target_pos = sym elif not fdt_util.GetBool(self._node, 'optional-ucode'): self.Raise('Cannot locate _dt_ucode_base_size symbol in u-boot') - - return Entry_blob.ObtainContents(self) + return True def ProcessContents(self): # If the image does not need microcode, there is nothing to do @@ -73,7 +72,7 @@ class Entry_u_boot_with_ucode_ptr(Entry_blob): pos, size = ucode_entry.pos, ucode_entry.size else: dtb_entry = self.section.FindEntryType('u-boot-dtb-with-ucode') - if not dtb_entry: + if not dtb_entry or not dtb_entry.ready: self.Raise('Cannot find microcode region u-boot-dtb-with-ucode') pos = dtb_entry.pos + dtb_entry.ucode_offset size = dtb_entry.ucode_size @@ -81,5 +80,5 @@ class Entry_u_boot_with_ucode_ptr(Entry_blob): # Write the microcode position and size into the entry pos_and_size = struct.pack('<2L', pos, size) self.target_pos -= self.pos - self.data = (self.data[:self.target_pos] + pos_and_size + - self.data[self.target_pos + 8:]) + self.ProcessContentsUpdate(self.data[:self.target_pos] + pos_and_size + + self.data[self.target_pos + 8:]) diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index eb8a0793cb..12164a85b4 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -146,19 +146,23 @@ class TestFunctional(unittest.TestCase): # options.verbosity = tout.DEBUG return control.Binman(options, args) - def _DoTestFile(self, fname, debug=False, map=False): + def _DoTestFile(self, fname, debug=False, map=False, update_dtb=False): """Run binman with a given test file Args: fname: Device-tree source filename to use (e.g. 05_simple.dts) debug: True to enable debugging output map: True to output map files for the images + update_dtb: Update the position and size of each entry in the device + tree before packing it into the image """ args = ['-p', '-I', self._indir, '-d', self.TestFile(fname)] if debug: args.append('-D') if map: args.append('-m') + if update_dtb: + args.append('-up') return self._DoBinman(*args) def _SetupDtb(self, fname, outfile='u-boot.dtb'): @@ -183,7 +187,8 @@ class TestFunctional(unittest.TestCase): TestFunctional._MakeInputFile(outfile, data) return data - def _DoReadFileDtb(self, fname, use_real_dtb=False, map=False): + def _DoReadFileDtb(self, fname, use_real_dtb=False, map=False, + update_dtb=False): """Run binman and return the resulting image This runs binman with a given test file and then reads the resulting @@ -199,6 +204,8 @@ class TestFunctional(unittest.TestCase): test contents (the U_BOOT_DTB_DATA string) can be used. But in some test we need the real contents. map: True to output map files for the images + update_dtb: Update the position and size of each entry in the device + tree before packing it into the image Returns: Tuple: @@ -212,21 +219,22 @@ class TestFunctional(unittest.TestCase): dtb_data = self._SetupDtb(fname) try: - retcode = self._DoTestFile(fname, map=map) + retcode = self._DoTestFile(fname, map=map, update_dtb=update_dtb) self.assertEqual(0, retcode) + out_dtb_fname = control.GetFdtPath('u-boot.dtb') # Find the (only) image, read it and return its contents image = control.images['image'] - fname = tools.GetOutputFilename('image.bin') - self.assertTrue(os.path.exists(fname)) + image_fname = tools.GetOutputFilename('image.bin') + self.assertTrue(os.path.exists(image_fname)) if map: map_fname = tools.GetOutputFilename('image.map') with open(map_fname) as fd: map_data = fd.read() else: map_data = None - with open(fname) as fd: - return fd.read(), dtb_data, map_data + with open(image_fname) as fd: + return fd.read(), dtb_data, map_data, out_dtb_fname finally: # Put the test file back if use_real_dtb: @@ -300,6 +308,26 @@ class TestFunctional(unittest.TestCase): """ return struct.unpack('>L', dtb[4:8])[0] + def _GetPropTree(self, dtb_data, node_names): + def AddNode(node, path): + if node.name != '/': + path += '/' + node.name + #print 'path', path + for subnode in node.subnodes: + for prop in subnode.props.values(): + if prop.name in node_names: + prop_path = path + '/' + subnode.name + ':' + prop.name + tree[prop_path[len('/binman/'):]] = fdt_util.fdt32_to_cpu( + prop.value) + #print ' ', prop.name + AddNode(subnode, path) + + tree = {} + dtb = fdt.Fdt(dtb_data) + dtb.Scan() + AddNode(dtb.GetRoot(), '') + return tree + def testRun(self): """Test a basic run with valid args""" result = self._RunBinman('-h') @@ -688,37 +716,56 @@ class TestFunctional(unittest.TestCase): data = self._DoReadFile('33_x86-start16.dts') self.assertEqual(X86_START16_DATA, data[:len(X86_START16_DATA)]) - def _RunMicrocodeTest(self, dts_fname, nodtb_data): + def _RunMicrocodeTest(self, dts_fname, nodtb_data, ucode_second=False): + """Handle running a test for insertion of microcode + + Args: + dts_fname: Name of test .dts file + nodtb_data: Data that we expect in the first section + ucode_second: True if the microsecond entry is second instead of + third + + Returns: + Tuple: + Contents of first region (U-Boot or SPL) + Position and size components of microcode pointer, as inserted + in the above (two 4-byte words) + """ data = self._DoReadFile(dts_fname, True) # Now check the device tree has no microcode - second = data[len(nodtb_data):] + if ucode_second: + ucode_content = data[len(nodtb_data):] + ucode_pos = len(nodtb_data) + dtb_with_ucode = ucode_content[16:] + fdt_len = self.GetFdtLen(dtb_with_ucode) + else: + dtb_with_ucode = data[len(nodtb_data):] + fdt_len = self.GetFdtLen(dtb_with_ucode) + ucode_content = dtb_with_ucode[fdt_len:] + ucode_pos = len(nodtb_data) + fdt_len fname = tools.GetOutputFilename('test.dtb') with open(fname, 'wb') as fd: - fd.write(second) + fd.write(dtb_with_ucode) dtb = fdt.FdtScan(fname) ucode = dtb.GetNode('/microcode') self.assertTrue(ucode) for node in ucode.subnodes: self.assertFalse(node.props.get('data')) - fdt_len = self.GetFdtLen(second) - third = second[fdt_len:] - # Check that the microcode appears immediately after the Fdt # This matches the concatenation of the data properties in # the /microcode/update@xxx nodes in 34_x86_ucode.dts. ucode_data = struct.pack('>4L', 0x12345678, 0x12345679, 0xabcd0000, 0x78235609) - self.assertEqual(ucode_data, third[:len(ucode_data)]) - ucode_pos = len(nodtb_data) + fdt_len + self.assertEqual(ucode_data, ucode_content[:len(ucode_data)]) # Check that the microcode pointer was inserted. It should match the # expected position and size pos_and_size = struct.pack('<2L', 0xfffffe00 + ucode_pos, len(ucode_data)) - first = data[:len(nodtb_data)] - return first, pos_and_size + u_boot = data[:len(nodtb_data)] + return u_boot, pos_and_size def testPackUbootMicrocode(self): """Test that x86 microcode can be handled correctly @@ -826,7 +873,7 @@ class TestFunctional(unittest.TestCase): """Test that we can cope with an image without microcode (e.g. qemu)""" with open(self.TestFile('u_boot_no_ucode_ptr')) as fd: TestFunctional._MakeInputFile('u-boot', fd.read()) - data, dtb, _ = self._DoReadFileDtb('44_x86_optional_ucode.dts', True) + data, dtb, _, _ = self._DoReadFileDtb('44_x86_optional_ucode.dts', True) # Now check the device tree has no microcode self.assertEqual(U_BOOT_NODTB_DATA, data[:len(U_BOOT_NODTB_DATA)]) @@ -881,23 +928,42 @@ class TestFunctional(unittest.TestCase): data = self._DoReadFile('48_x86-start16-spl.dts') self.assertEqual(X86_START16_SPL_DATA, data[:len(X86_START16_SPL_DATA)]) - def testPackUbootSplMicrocode(self): - """Test that x86 microcode can be handled correctly in SPL + def _PackUbootSplMicrocode(self, dts, ucode_second=False): + """Helper function for microcode tests We expect to see the following in the image, in order: u-boot-spl-nodtb.bin with a microcode pointer inserted at the correct place u-boot.dtb with the microcode removed the microcode + + Args: + dts: Device tree file to use for test + ucode_second: True if the microsecond entry is second instead of + third """ # ELF file with a '_dt_ucode_base_size' symbol with open(self.TestFile('u_boot_ucode_ptr')) as fd: TestFunctional._MakeInputFile('spl/u-boot-spl', fd.read()) - first, pos_and_size = self._RunMicrocodeTest('49_x86_ucode_spl.dts', - U_BOOT_SPL_NODTB_DATA) + first, pos_and_size = self._RunMicrocodeTest(dts, U_BOOT_SPL_NODTB_DATA, + ucode_second=ucode_second) self.assertEqual('splnodtb with microc' + pos_and_size + 'ter somewhere in here', first) + def testPackUbootSplMicrocode(self): + """Test that x86 microcode can be handled correctly in SPL""" + self._PackUbootSplMicrocode('49_x86_ucode_spl.dts') + + def testPackUbootSplMicrocodeReorder(self): + """Test that order doesn't matter for microcode entries + + This is the same as testPackUbootSplMicrocode but when we process the + u-boot-ucode entry we have not yet seen the u-boot-dtb-with-ucode + entry, so we reply on binman to try later. + """ + self._PackUbootSplMicrocode('58_x86_ucode_spl_needs_retry.dts', + ucode_second=True) + def testPackMrc(self): """Test that an image with an MRC binary can be created""" data = self._DoReadFile('50_intel_mrc.dts') @@ -942,7 +1008,7 @@ class TestFunctional(unittest.TestCase): def testMap(self): """Tests outputting a map of the images""" - _, _, map_data = self._DoReadFileDtb('55_sections.dts', map=True) + _, _, map_data, _ = self._DoReadFileDtb('55_sections.dts', map=True) self.assertEqual('''Position Size Name 00000000 00000010 section@0 00000000 00000004 u-boot @@ -952,7 +1018,7 @@ class TestFunctional(unittest.TestCase): def testNamePrefix(self): """Tests that name prefixes are used""" - _, _, map_data = self._DoReadFileDtb('56_name_prefix.dts', map=True) + _, _, map_data, _ = self._DoReadFileDtb('56_name_prefix.dts', map=True) self.assertEqual('''Position Size Name 00000000 00000010 section@0 00000000 00000004 ro-u-boot @@ -960,5 +1026,50 @@ class TestFunctional(unittest.TestCase): 00000000 00000004 rw-u-boot ''', map_data) + def testUnknownContents(self): + """Test that obtaining the contents works as expected""" + with self.assertRaises(ValueError) as e: + self._DoReadFile('57_unknown_contents.dts', True) + self.assertIn("Section '/binman': Internal error: Could not complete " + "processing of contents: remaining [<_testing.Entry__testing ", + str(e.exception)) + + def testBadChangeSize(self): + """Test that trying to change the size of an entry fails""" + with self.assertRaises(ValueError) as e: + self._DoReadFile('59_change_size.dts', True) + self.assertIn("Node '/binman/_testing': Cannot update entry size from " + '2 to 1', str(e.exception)) + + def testUpdateFdt(self): + """Test that we can update the device tree with pos/size info""" + _, _, _, out_dtb_fname = self._DoReadFileDtb('60_fdt_update.dts', + update_dtb=True) + props = self._GetPropTree(out_dtb_fname, ['pos', 'size']) + with open('/tmp/x.dtb', 'wb') as outf: + with open(out_dtb_fname) as inf: + outf.write(inf.read()) + self.assertEqual({ + '_testing:pos': 32, + '_testing:size': 1, + 'section@0/u-boot:pos': 0, + 'section@0/u-boot:size': len(U_BOOT_DATA), + 'section@0:pos': 0, + 'section@0:size': 16, + + 'section@1/u-boot:pos': 0, + 'section@1/u-boot:size': len(U_BOOT_DATA), + 'section@1:pos': 16, + 'section@1:size': 16, + 'size': 40 + }, props) + + def testUpdateFdtBad(self): + """Test that we detect when ProcessFdt never completes""" + with self.assertRaises(ValueError) as e: + self._DoReadFileDtb('61_fdt_update_bad.dts', update_dtb=True) + self.assertIn('Could not complete processing of Fdt: remaining ' + '[<_testing.Entry__testing', str(e.exception)) + if __name__ == "__main__": unittest.main() diff --git a/tools/binman/image.py b/tools/binman/image.py index 835b66c99f..94028f19a5 100644 --- a/tools/binman/image.py +++ b/tools/binman/image.py @@ -54,6 +54,20 @@ class Image: self._filename = filename self._section = bsection.Section('main-section', self._node) + def AddMissingProperties(self): + """Add properties that are not present in the device tree + + When binman has completed packing the entries the position and size of + each entry are known. But before this the device tree may not specify + these. Add any missing properties, with a dummy value, so that the + size of the entry is correct. That way we can insert the correct values + later. + """ + self._section.AddMissingProperties() + + def ProcessFdt(self, fdt): + return self._section.ProcessFdt(fdt) + def GetEntryContents(self): """Call ObtainContents() for the section """ @@ -79,6 +93,9 @@ class Image: """Check that entries do not overlap or extend outside the image""" self._section.CheckEntries() + def SetCalculatedProperties(self): + self._section.SetCalculatedProperties() + def ProcessEntryContents(self): """Call the ProcessContents() method for each entry diff --git a/tools/binman/image_test.py b/tools/binman/image_test.py index 45dd2378c8..3775e1afb0 100644 --- a/tools/binman/image_test.py +++ b/tools/binman/image_test.py @@ -7,7 +7,7 @@ import unittest from image import Image -from elf_test import capture_sys_output +from test_util import capture_sys_output class TestImage(unittest.TestCase): def testInvalidFormat(self): diff --git a/tools/binman/test/41_unknown_pos_size.dts b/tools/binman/test/41_unknown_pos_size.dts index a8e7d8aa22..94fe821c47 100644 --- a/tools/binman/test/41_unknown_pos_size.dts +++ b/tools/binman/test/41_unknown_pos_size.dts @@ -6,6 +6,7 @@ binman { _testing { + return-invalid-entry; }; }; }; diff --git a/tools/binman/test/57_unknown_contents.dts b/tools/binman/test/57_unknown_contents.dts new file mode 100644 index 0000000000..6ea98d7cab --- /dev/null +++ b/tools/binman/test/57_unknown_contents.dts @@ -0,0 +1,14 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + _testing { + return-unknown-contents; + }; + }; +}; diff --git a/tools/binman/test/58_x86_ucode_spl_needs_retry.dts b/tools/binman/test/58_x86_ucode_spl_needs_retry.dts new file mode 100644 index 0000000000..e2cb80cf6e --- /dev/null +++ b/tools/binman/test/58_x86_ucode_spl_needs_retry.dts @@ -0,0 +1,36 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + sort-by-pos; + end-at-4gb; + size = <0x200>; + u-boot-spl-with-ucode-ptr { + }; + + /* + * Microcode goes before the DTB which contains it, so binman + * will need to obtain the contents of the next section before + * obtaining the contents of this one. + */ + u-boot-ucode { + }; + + u-boot-dtb-with-ucode { + }; + }; + + microcode { + update@0 { + data = <0x12345678 0x12345679>; + }; + update@1 { + data = <0xabcd0000 0x78235609>; + }; + }; +}; diff --git a/tools/binman/test/59_change_size.dts b/tools/binman/test/59_change_size.dts new file mode 100644 index 0000000000..1a69026a64 --- /dev/null +++ b/tools/binman/test/59_change_size.dts @@ -0,0 +1,14 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + _testing { + bad-update-contents; + }; + }; +}; diff --git a/tools/binman/test/60_fdt_update.dts b/tools/binman/test/60_fdt_update.dts new file mode 100644 index 0000000000..f53c8a5053 --- /dev/null +++ b/tools/binman/test/60_fdt_update.dts @@ -0,0 +1,31 @@ +// SPDX-License-Identifier: GPL-2.0+ +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + pad-byte = <0x26>; + size = <0x28>; + section@0 { + read-only; + name-prefix = "ro-"; + size = <0x10>; + pad-byte = <0x21>; + + u-boot { + }; + }; + section@1 { + name-prefix = "rw-"; + size = <0x10>; + pad-byte = <0x61>; + + u-boot { + }; + }; + _testing { + }; + }; +}; diff --git a/tools/binman/test/61_fdt_update_bad.dts b/tools/binman/test/61_fdt_update_bad.dts new file mode 100644 index 0000000000..e5abf31699 --- /dev/null +++ b/tools/binman/test/61_fdt_update_bad.dts @@ -0,0 +1,32 @@ +// SPDX-License-Identifier: GPL-2.0+ +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + pad-byte = <0x26>; + size = <0x28>; + section@0 { + read-only; + name-prefix = "ro-"; + size = <0x10>; + pad-byte = <0x21>; + + u-boot { + }; + }; + section@1 { + name-prefix = "rw-"; + size = <0x10>; + pad-byte = <0x61>; + + u-boot { + }; + }; + _testing { + never-complete-process-fdt; + }; + }; +}; |