From affa7a1c7e1188fbb8e9f418f13ced0cba57a1ba Mon Sep 17 00:00:00 2001 From: Peter Bigot Date: Fri, 22 Jan 2021 16:58:26 -0600 Subject: [PATCH] Revert "device: add post-process of elf file to manage device handles" This reverts commit 40d3653758cd81c3eafe752485ae480899a071d3. Signed-off-by: Peter Bigot --- CMakeLists.txt | 13 -- CODEOWNERS | 1 - doc/guides/build/index.rst | 22 +-- include/device.h | 9 +- include/linker/common-rom.ld | 11 -- kernel/include/kernel_offsets.h | 4 - scripts/gen_handles.py | 312 -------------------------------- 7 files changed, 5 insertions(+), 367 deletions(-) delete mode 100755 scripts/gen_handles.py diff --git a/CMakeLists.txt b/CMakeLists.txt index 163b71753fe..bc95d53ee04 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -748,19 +748,6 @@ if(CONFIG_GEN_ISR_TABLES) set_property(GLOBAL APPEND PROPERTY GENERATED_KERNEL_SOURCE_FILES isr_tables.c) endif() -# dev_handles.c is generated from ${ZEPHYR_PREBUILT_EXECUTABLE} by -# gen_handles.py -add_custom_command( - OUTPUT dev_handles.c - COMMAND - ${PYTHON_EXECUTABLE} - ${ZEPHYR_BASE}/scripts/gen_handles.py - --output-source dev_handles.c - --kernel $ - DEPENDS ${ZEPHYR_PREBUILT_EXECUTABLE} - ) -set_property(GLOBAL APPEND PROPERTY GENERATED_KERNEL_SOURCE_FILES dev_handles.c) - if(CONFIG_CODE_DATA_RELOCATION) # @Intent: Linker script to relocate .text, data and .bss sections toolchain_ld_relocation() diff --git a/CODEOWNERS b/CODEOWNERS index 9dee1807daf..f7e509aea91 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -504,7 +504,6 @@ /scripts/pylib/twister/expr_parser.py @nashif /scripts/schemas/twister/ @nashif /scripts/gen_app_partitions.py @andrewboie -/scripts/gen_handles.py @pabigot /scripts/get_maintainer.py @nashif /scripts/dts/ @mbolivar-nordic @galak /scripts/release/ @nashif diff --git a/doc/guides/build/index.rst b/doc/guides/build/index.rst index 8d1b54a3469..40ecf3b106d 100644 --- a/doc/guides/build/index.rst +++ b/doc/guides/build/index.rst @@ -151,16 +151,9 @@ is skipped. Final binary ============ -The binary from the prevoius tage is incomplete, with empty and/or -placeholder sections that must be filled in by, essentially, reflection. - -Device dependencies - The *gen_handles.py* script scans the first-pass binary to determine - relationships between devices that were recorded from devicetree data, - and replaces the encoded relationships with values that are optimized to - locate the devices actually present in the application. - -When :ref:`usermode_api` is enabled: +In some configurations, the binary from the previous stage is +incomplete, with empty and/or placeholder sections that must be filled +in by, essentially, reflection. When :ref:`usermode_api` is enabled: Kernel object hashing The *gen_kobject_list.py* scans the *ELF DWARF* @@ -209,15 +202,6 @@ The following is a detailed description of the scripts used during the build pro :start-after: """ :end-before: """ -.. _gen_handles.py: - -:zephyr_file:`scripts/gen_handles.py` -========================================== - -.. include:: ../../../scripts/gen_handles.py - :start-after: """ - :end-before: """ - .. _gen_kobject_list.py: :zephyr_file:`scripts/gen_kobject_list.py` diff --git a/include/device.h b/include/device.h index 92574a38243..39e6db17eac 100644 --- a/include/device.h +++ b/include/device.h @@ -817,16 +817,11 @@ static inline int device_pm_put_sync(const struct device *dev) { return -ENOTSUP Z_DEVICE_DEFINE_HANDLES(node_id, dev_name, __VA_ARGS__) #define Z_DEVICE_DEFINE_HANDLES(node_id, dev_name, ...) \ - extern const device_handle_t \ - Z_DEVICE_HANDLE_NAME(node_id, dev_name)[]; \ - const device_handle_t \ - __attribute__((__weak__, \ - __section__(".__device_handles_pass1"))) \ - Z_DEVICE_HANDLE_NAME(node_id, dev_name)[] = { \ + static const device_handle_t Z_DEVICE_HANDLE_NAME(node_id,dev_name)[] = { \ COND_CODE_1(DT_NODE_EXISTS(node_id), ( \ DT_DEP_ORD(node_id), \ DT_REQUIRES_DEP_ORDS(node_id) \ - ), ( \ + ),( \ DEVICE_HANDLE_NULL, \ )) \ DEVICE_HANDLE_SEP, \ diff --git a/include/linker/common-rom.ld b/include/linker/common-rom.ld index 5767d8c6c8f..750e0dc381b 100644 --- a/include/linker/common-rom.ld +++ b/include/linker/common-rom.ld @@ -37,17 +37,6 @@ } ASSERT(SIZEOF(initlevel_error) == 0, "Undefined initialization levels used.") - SECTION_DATA_PROLOGUE(device_handles,,) - { - __device_handles_start = .; -#ifdef LINKER_PASS2 - KEEP(*(SORT(.__device_handles_pass2))); -#else /* LINKER_PASS2 */ - KEEP(*(SORT(.__device_handles_pass1))); -#endif /* LINKER_PASS2 */ - __device_handles_end = .; - } GROUP_LINK_IN(ROMABLE_REGION) - #ifdef CONFIG_CPLUSPLUS SECTION_PROLOGUE(_CTOR_SECTION_NAME,,) { diff --git a/kernel/include/kernel_offsets.h b/kernel/include/kernel_offsets.h index 884aeb271b7..466aa20ca19 100644 --- a/kernel/include/kernel_offsets.h +++ b/kernel/include/kernel_offsets.h @@ -86,9 +86,5 @@ GEN_ABSOLUTE_SYM(K_THREAD_SIZEOF, sizeof(struct k_thread)); /* size of the device structure. Used by linker scripts */ GEN_ABSOLUTE_SYM(_DEVICE_STRUCT_SIZEOF, sizeof(const struct device)); -/* member offsets in the device structure. Used in image post-processing */ -GEN_ABSOLUTE_SYM(_DEVICE_STRUCT_HANDLES_OFFSET, - offsetof(struct device, handles)); - /* LCOV_EXCL_STOP */ #endif /* ZEPHYR_KERNEL_INCLUDE_KERNEL_OFFSETS_H_ */ diff --git a/scripts/gen_handles.py b/scripts/gen_handles.py deleted file mode 100755 index a77354216ad..00000000000 --- a/scripts/gen_handles.py +++ /dev/null @@ -1,312 +0,0 @@ -#!/usr/bin/env python3 -# -# Copyright (c) 2017 Intel Corporation -# Copyright (c) 2020 Nordic Semiconductor NA -# -# SPDX-License-Identifier: Apache-2.0 -"""Translate generic handles into ones optimized for the application. - -Immutable device data includes information about dependencies, -e.g. that a particular sensor is controlled through a specific I2C bus -and that it signals event on a pin on a specific GPIO controller. -This information is encoded in the first-pass binary using identifiers -derived from the devicetree. This script extracts those identifiers -and replaces them with ones optimized for use with the devices -actually present. - -For example the sensor might have a first-pass handle defined by its -devicetree ordinal 52, with the I2C driver having ordinal 24 and the -GPIO controller ordinal 14. The runtime ordinal is the index of the -corresponding device in the static devicetree array, which might be 6, -5, and 3, respectively. - -The output is a C source file that provides alternative definitions -for the array contents referenced from the immutable device objects. -In the final link these definitions supersede the ones in the -driver-specific object file. -""" - -import sys -import argparse -import os -import struct -import pickle -from distutils.version import LooseVersion - -import elftools -from elftools.elf.elffile import ELFFile -from elftools.elf.sections import SymbolTableSection -import elftools.elf.enums - -if LooseVersion(elftools.__version__) < LooseVersion('0.24'): - sys.exit("pyelftools is out of date, need version 0.24 or later") - -ZEPHYR_BASE = os.getenv("ZEPHYR_BASE") -sys.path.insert(0, os.path.join(ZEPHYR_BASE, "scripts/dts")) - -scr = os.path.basename(sys.argv[0]) - -def debug(text): - if not args.verbose: - return - sys.stdout.write(scr + ": " + text + "\n") - -def parse_args(): - global args - - parser = argparse.ArgumentParser( - description=__doc__, - formatter_class=argparse.RawDescriptionHelpFormatter) - - parser.add_argument("-k", "--kernel", required=True, - help="Input zephyr ELF binary") - parser.add_argument("-o", "--output-source", required=True, - help="Output source file") - - parser.add_argument("-v", "--verbose", action="store_true", - help="Print extra debugging information") - args = parser.parse_args() - if "VERBOSE" in os.environ: - args.verbose = 1 - - -def symbol_data(elf, sym): - addr = sym.entry.st_value - len = sym.entry.st_size - for section in elf.iter_sections(): - start = section['sh_addr'] - end = start + section['sh_size'] - - if (start <= addr) and (addr + len) <= end: - offset = addr - section['sh_addr'] - return bytes(section.data()[offset:offset + len]) - -def symbol_handle_data(elf, sym): - data = symbol_data(elf, sym) - if data: - format = "<" if elf.little_endian else ">" - format += "%uh" % (len(data) / 2) - return struct.unpack(format, data) - -# These match the corresponding constants in -DEVICE_HANDLE_SEP = -32768 -DEVICE_HANDLE_ENDS = 32767 -def handle_name(hdl): - if hdl == DEVICE_HANDLE_SEP: - return "DEVICE_HANDLE_SEP" - if hdl == DEVICE_HANDLE_ENDS: - return "DEVICE_HANDLE_ENDS" - if hdl == 0: - return "DEVICE_HANDLE_NULL" - return str(int(hdl)) - -class Device: - """ - Represents information about a device object and its references to other objects. - """ - def __init__(self, elf, ld_constants, sym, addr): - self.elf = elf - self.ld_constants = ld_constants - self.sym = sym - self.addr = addr - # Point to the handles instance associated with the device; - # assigned by correlating the device struct handles pointer - # value with the addr of a Handles instance. - self.__handles = None - - @property - def obj_handles(self): - """ - Returns the value from the device struct handles field, pointing to the - array of handles for devices this device depends on. - """ - if self.__handles is None: - data = symbol_data(self.elf, self.sym) - format = "<" if self.elf.little_endian else ">" - if self.elf.elfclass == 32: - format += "I" - size = 4 - else: - format += "Q" - size = 8 - offset = self.ld_constants["DEVICE_STRUCT_HANDLES_OFFSET"] - self.__handles = struct.unpack(format, data[offset:offset + size])[0] - return self.__handles - -class Handles: - def __init__(self, sym, addr, handles, node): - self.sym = sym - self.addr = addr - self.handles = handles - self.node = node - self.dep_ord = None - self.dev_deps = None - self.ext_deps = None - -def main(): - parse_args() - - assert args.kernel, "--kernel ELF required to extract data" - elf = ELFFile(open(args.kernel, "rb")) - - edtser = os.path.join(os.path.split(args.kernel)[0], "edt.pickle") - with open(edtser, 'rb') as f: - edt = pickle.load(f) - - devices = [] - handles = [] - # Leading _ are stripped from the stored constant key - want_constants = set(["__device_start", - "_DEVICE_STRUCT_SIZEOF", - "_DEVICE_STRUCT_HANDLES_OFFSET"]) - ld_constants = dict() - - for section in elf.iter_sections(): - if isinstance(section, SymbolTableSection): - for sym in section.iter_symbols(): - if sym.name in want_constants: - ld_constants[sym.name.lstrip("_")] = sym.entry.st_value - continue - if sym.entry.st_info.type != 'STT_OBJECT': - continue - if sym.name.startswith("__device"): - addr = sym.entry.st_value - if sym.name.startswith("__device_"): - devices.append(Device(elf, ld_constants, sym, addr)) - debug("device %s" % (sym.name,)) - elif sym.name.startswith("__devicehdl_"): - hdls = symbol_handle_data(elf, sym) - - # The first element of the hdls array is the dependency - # ordinal of the device, which identifies the devicetree - # node. - node = edt.dep_ord2node[hdls[0]] if (hdls and hdls[0] != 0) else None - handles.append(Handles(sym, addr, hdls, node)) - debug("handles %s %d %s" % (sym.name, hdls[0] if hdls else -1, node)) - - assert len(want_constants) == len(ld_constants), "linker map data incomplete" - - devices = sorted(devices, key = lambda k: k.sym.entry.st_value) - - device_start_addr = ld_constants["device_start"] - device_size = 0 - - assert len(devices) == len(handles), 'mismatch devices and handles' - - used_nodes = set() - for handle in handles: - for device in devices: - if handle.addr == device.obj_handles: - handle.device = device - break - device = handle.device - assert device, 'no device for %s' % (handle.sym.name,) - - device.handle = handle - - if device_size == 0: - device_size = device.sym.entry.st_size - - # Where in the sequence of devices this particular node occurs - device.dev_ordinal = (device.sym.entry.st_value - device_start_addr) / device_size - debug("%s dev ordinal %d" % (device.sym.name, device.dev_ordinal)) - - n = handle.node - if n is not None: - debug("%s dev ordinal %d\n\t%s" % (n.path, device.dev_ordinal, ' ; '.join(str(_) for _ in handle.handles))) - used_nodes.add(n) - n.__device = device - else: - debug("orphan %d" % (device.dev_ordinal,)) - hv = handle.handles - hvi = 1 - handle.dev_deps = [] - handle.ext_deps = [] - deps = handle.dev_deps - while True: - h = hv[hvi] - if h == DEVICE_HANDLE_ENDS: - break - if h == DEVICE_HANDLE_SEP: - deps = handle.ext_deps - else: - deps.append(h) - n = edt - hvi += 1 - - # Compute the dependency graph induced from the full graph restricted to the - # the nodes that exist in the application. Note that the edges in the - # induced graph correspond to paths in the full graph. - root = edt.dep_ord2node[0] - assert root not in used_nodes - - for sn in used_nodes: - # Where we're storing the final set of nodes: these are all used - sn.__depends = set() - - deps = set(sn.depends_on) - debug("\nNode: %s\nOrig deps:\n\t%s" % (sn.path, "\n\t".join([dn.path for dn in deps]))) - while len(deps) > 0: - dn = deps.pop() - if dn in used_nodes: - # this is used - sn.__depends.add(dn) - elif dn != root: - # forward the dependency up one level - for ddn in dn.depends_on: - deps.add(ddn) - debug("final deps:\n\t%s\n" % ("\n\t".join([ _dn.path for _dn in sn.__depends]))) - - with open(args.output_source, "w") as fp: - fp.write('#include \n') - - for dev in devices: - hs = dev.handle - assert hs, "no hs for %s" % (dev.sym.name,) - dep_paths = [] - ext_paths = [] - hdls = [] - - sn = hs.node - if sn: - hdls.extend(dn.__device.dev_ordinal for dn in sn.__depends) - for dn in sn.depends_on: - if dn in sn.__depends: - dep_paths.append(dn.path) - else: - dep_paths.append('(%s)' % dn.path) - if len(hs.ext_deps) > 0: - # TODO: map these to something smaller? - ext_paths.extend(map(str, hs.ext_deps)) - hdls.append(DEVICE_HANDLE_SEP) - hdls.extend(hs.ext_deps) - - # When CONFIG_USERSPACE is enabled the pre-built elf is - # also used to get hashes that identify kernel objects by - # address. We can't allow the size of any object in the - # final elf to change. - while len(hdls) < len(hs.handles): - hdls.append(DEVICE_HANDLE_ENDS) - assert len(hdls) == len(hs.handles), "%s handle overflow" % (dev.sym.name,) - - lines = [ - '', - '/* %d : %s:' % (dev.dev_ordinal, (sn and sn.path) or "sysinit"), - ] - - if len(dep_paths) > 0: - lines.append(' * - %s' % ('\n * - '.join(dep_paths))) - if len(ext_paths) > 0: - lines.append(' * + %s' % ('\n * + '.join(ext_paths))) - - lines.extend([ - ' */', - 'const device_handle_t __attribute__((__section__(".__device_handles_pass2")))', - '%s[] = { %s };' % (hs.sym.name, ', '.join([handle_name(_h) for _h in hdls])), - '', - ]) - - fp.write('\n'.join(lines)) - -if __name__ == "__main__": - main()