Merge changes from topic "romlib-fixes" into integration

* changes:
  fix(romlib): wrap indirectly included functions
  fix(arm): remove duplicate jumptable entry

docs/components/romlib-design.rst

@@ -71,6 +71,15 @@ image(s) is replaced with the wrapper function.
 The "library at ROM" contains a necessary init function that initialises the
 global variables defined by the functions inside "library at ROM".
 
+Wrapper functions are specified at the link stage of compilation and cannot
+interpose uppon functions within the same translation unit. For example, if
+function ``fn_a`` calls ``fn_b`` within translation unit ``functions.c`` and
+the romlib jump table includes an entry for ``fn_b``, ``fn_a`` will include
+a reference to ``fn_b``'s original program text instead of the wrapper. Thus
+the jumptable author must take care to include public entry points into
+translation units to avoid paying the program text cost twice, once in the
+original executable and once in romlib.
+
 Script
 ~~~~~~
 
@@ -86,7 +95,7 @@ files for the "library at ROM" to work. It implements multiple functions:
 
 3. ``romlib_generator.py genwrappers [args]`` - Generates a wrapper function for
    each entry in the index file except for the ones that contain the keyword
-   ``patch``. The generated wrapper file is called ``<fn_name>.s``.
+   ``patch``. The generated wrapper file is called ``wrappers.s``.
 
 4. ``romlib_generator.py pre [args]`` - Preprocesses the index file which means
    it resolves all the include commands in the file recursively. It can also

lib/romlib/Makefile

@@ -45,7 +45,7 @@ endif
 
 .PHONY: all clean distclean
 
-all: $(BUILD_DIR)/romlib.bin $(LIB_DIR)/libwrappers.a
+all: $(BUILD_DIR)/romlib.bin $(BUILD_DIR)/romlib.ldflags $(LIB_DIR)/libwrappers.a
 
 %.o: %.s | $$(@D)/
 	$(s)echo "  AS      $@"
@@ -89,6 +89,10 @@ $(BUILD_DIR)/jmptbl.s: $(BUILD_DIR)/jmptbl.i | $$(@D)/
 	$(s)echo "  TBL     $@"
 	$(q)$(ROMLIB_GEN) gentbl --output $@ --bti=$(ENABLE_BTI) $<
 
+$(BUILD_DIR)/romlib.ldflags: ../../$(PLAT_DIR)/jmptbl.i
+	$(s)echo "  LDFLAGS $@"
+	$(q)$(ROMLIB_GEN) link-flags $< > $@
+
 clean:
 	$(q)rm -f $(BUILD_DIR)/*
 

lib/romlib/romlib_generator.py

@@ -182,6 +182,22 @@ class TableGenerator(RomlibApplication):
                 template_name = "jmptbl_entry_" + item["type"] + bti + ".S"
                 output_file.write(self.build_template(template_name, item, True))
 
+class LinkArgs(RomlibApplication):
+    """ Generates the link arguments to wrap functions. """
+
+    def __init__(self, prog):
+        RomlibApplication.__init__(self, prog)
+        self.args.add_argument("file", help="Input file")
+
+    def main(self):
+        index_file_parser = IndexFileParser()
+        index_file_parser.parse(self.config.file)
+
+        fns = [item["function_name"] for item in index_file_parser.items
+               if not item["patch"] and item["type"] != "reserved"]
+
+        print(" ".join("-Wl,--wrap " + f for f in fns))
+
 class WrapperGenerator(RomlibApplication):
     """
     Generates a wrapper function for each entry in the index file except for the ones that contain
@@ -214,21 +230,19 @@ class WrapperGenerator(RomlibApplication):
             if item["type"] == "reserved" or item["patch"]:
                 continue
 
-            asm = self.config.b + "/" + item["function_name"] + ".s"
-            if self.config.list:
-                # Only listing files
-                files.append(asm)
-            else:
-                with open(asm, "w") as asm_file:
-                    # The jump instruction is 4 bytes but BTI requires and extra instruction so
-                    # this makes it 8 bytes per entry.
-                    function_offset = item_index * (8 if self.config.bti else 4)
+            if not self.config.list:
+                # The jump instruction is 4 bytes but BTI requires and extra instruction so
+                # this makes it 8 bytes per entry.
+                function_offset = item_index * (8 if self.config.bti else 4)
 
-                    item["function_offset"] = function_offset
-                    asm_file.write(self.build_template("wrapper" + bti + ".S", item))
+                item["function_offset"] = function_offset
+                files.append(self.build_template("wrapper" + bti + ".S", item))
 
         if self.config.list:
-            print(" ".join(files))
+            print(self.config.b + "/wrappers.s")
+        else:
+            with open(self.config.b + "/wrappers.s", "w") as asm_file:
+                asm_file.write("\n".join(files))
 
 class VariableGenerator(RomlibApplication):
     """ Generates the jump table global variable with the absolute address in ROM. """
@@ -258,7 +272,8 @@ class VariableGenerator(RomlibApplication):
 
 if __name__ == "__main__":
     APPS = {"genvar": VariableGenerator, "pre": IndexPreprocessor,
-            "gentbl": TableGenerator, "genwrappers": WrapperGenerator}
+            "gentbl": TableGenerator, "genwrappers": WrapperGenerator,
+            "link-flags": LinkArgs}
 
     if len(sys.argv) < 2 or sys.argv[1] not in APPS:
         print("usage: romlib_generator.py [%s] [args]" % "|".join(APPS.keys()), file=sys.stderr)

lib/romlib/templates/wrapper.S

@@ -3,8 +3,9 @@
  *
  * SPDX-License-Identifier: BSD-3-Clause
  */
-	.globl	${function_name}
-${function_name}:
+	.section .text.__wrap_${function_name}
+	.globl	__wrap_${function_name}
+__wrap_${function_name}:
 	ldr	x17, =jmptbl
 	mov	x16, #${function_offset}
 	ldr	x17, [x17]

lib/romlib/templates/wrapper_bti.S

@@ -3,8 +3,9 @@
  *
  * SPDX-License-Identifier: BSD-3-Clause
  */
-	.globl	${function_name}
-${function_name}:
+	.section .text.__wrap_${function_name}
+	.globl	__wrap_${function_name}
+__wrap_${function_name}:
 	bti	jc
 	ldr	x17, =jmptbl
 	mov	x16, #${function_offset}

make_helpers/build_macros.mk

@@ -465,6 +465,10 @@ define linker_script_path
         $(patsubst %.S,$(BUILD_DIR)/%,$(1))
 endef
 
+ifeq ($(USE_ROMLIB),1)
+WRAPPER_FLAGS := @${BUILD_PLAT}/romlib/romlib.ldflags
+endif
+
 # MAKE_BL macro defines the targets and options to build each BL image.
 # Arguments:
 #   $(1) = BL stage
@@ -514,11 +518,11 @@ ifeq ($($(ARCH)-ld-id),arm-link)
 		--map --list="$(MAPFILE)" --scatter=${PLAT_DIR}/scat/${1}.scat \
 		$(LDPATHS) $(LIBWRAPPER) $(LDLIBS) $(BL_LIBS) $(OBJS)
 else ifeq ($($(ARCH)-ld-id),gnu-gcc)
-	$$(q)$($(ARCH)-ld) -o $$@ $$(TF_LDFLAGS) $$(LDFLAGS) $(BL_LDFLAGS) -Wl,-Map=$(MAPFILE) \
+	$$(q)$($(ARCH)-ld) -o $$@ $$(TF_LDFLAGS) $$(LDFLAGS) $$(WRAPPER_FLAGS) $(BL_LDFLAGS) -Wl,-Map=$(MAPFILE) \
 		$(addprefix -Wl$(comma)--script$(comma),$(LINKER_SCRIPTS)) -Wl,--script,$(DEFAULT_LINKER_SCRIPT) \
 		$(OBJS) $(LDPATHS) $(LIBWRAPPER) $(LDLIBS) $(BL_LIBS)
 else
-	$$(q)$($(ARCH)-ld) -o $$@ $$(TF_LDFLAGS) $$(LDFLAGS) $(BL_LDFLAGS) -Map=$(MAPFILE) \
+	$$(q)$($(ARCH)-ld) -o $$@ $$(TF_LDFLAGS) $$(LDFLAGS) $$(WRAPPER_FLAGS) $(BL_LDFLAGS) -Map=$(MAPFILE) \
 		$(addprefix -T ,$(LINKER_SCRIPTS)) --script $(DEFAULT_LINKER_SCRIPT) \
 		$(OBJS) $(LDPATHS) $(LIBWRAPPER) $(LDLIBS) $(BL_LIBS)
 endif

plat/arm/board/fvp/jmptbl.i

@@ -36,7 +36,6 @@ fdt     fdt_get_alias_namelen
 fdt     fdt_get_name
 fdt     fdt_get_alias
 fdt     fdt_node_offset_by_phandle
-fdt     fdt_subnode_offset
 fdt     fdt_add_subnode
 mbedtls mbedtls_asn1_get_alg
 mbedtls mbedtls_asn1_get_alg_null