binman: tegra: Adjust symbol calculation depending on end-at-4gb

A recent change adjusted the symbol calculation to work on x86 but broke
it for Tegra. In fact this is because they have different needs.

On x86 devices the code is linked to a ROM address and the end-at-4gb
property is used for the image. In this case there is no need to add the
base address of the image, since the base address is already built into
the offset and image-pos properties.

On other devices we must add the base address since the offsets start at
zero.

In addition the base address is currently added to the 'offset' and 'size'
values. It should in fact only be added to 'image-pos', since 'offset' is
relative to its parent and 'size' is not actually an address. This code
should have been adjusted when support for 'image-pos' and 'size' was
added, but it was not.

To correct these problems:
- move the code that handles adding the base address to section.py, which
  can check the end-at-4gb property and which property
  (offset/size/image-pos) is being read
- add the base address only when needed (only for image-pos and not if the
  image uses end-at-4gb)
- add a note to the documentation
- add a separate test to cover x86 behaviour

Fixes: 15c981cc (binman: Correct symbol calculation with non-zero image base)

Signed-off-by: Simon Glass <sjg@chromium.org>
Tested-by: Stephen Warren <swarren@nvidia.com>

4 files changed, 67 insertions, 1 deletions

diff --git a/tools/binman/test/155_symbols_tpl_x86.dts b/tools/binman/test/155_symbols_tpl_x86.dts
new file mode 100644
index 0000000000..72ca447447
--- /dev/null
+++ b/tools/binman/test/155_symbols_tpl_x86.dts
@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		pad-byte = <0xff>;
+		end-at-4gb;
+		size = <0x100>;
+		u-boot-spl {
+			offset = <0xffffff04>;
+		};
+
+		u-boot-spl2 {
+			offset = <0xffffff1c>;
+			type = "u-boot-spl";
+		};
+
+		u-boot {
+			offset = <0xffffff34>;
+		};
+
+		section {
+			u-boot-tpl {
+				type = "u-boot-tpl";
+			};
+		};
+	};
+};
diff --git a/tools/binman/test/Makefile b/tools/binman/test/Makefile
index bdbb009874..e4fd97bb2e 100644
--- a/tools/binman/test/Makefile
+++ b/tools/binman/test/Makefile
@@ -14,10 +14,11 @@ 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 $(SRC)u_boot_binman_syms_bad.lds
+LDS_BINMAN_X86 := -T $(SRC)u_boot_binman_syms_x86.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 \
-	u_boot_binman_syms_size
+	u_boot_binman_syms_size u_boot_binman_syms_x86
 
 all: $(TARGETS)
 
@@ -36,6 +37,9 @@ u_boot_binman_syms.bin: u_boot_binman_syms
 u_boot_binman_syms: CFLAGS += $(LDS_BINMAN)
 u_boot_binman_syms: u_boot_binman_syms.c
 
+u_boot_binman_syms_x86: CFLAGS += $(LDS_BINMAN_X86)
+u_boot_binman_syms_x86: u_boot_binman_syms_x86.c
+
 u_boot_binman_syms_bad: CFLAGS += $(LDS_BINMAN_BAD)
 u_boot_binman_syms_bad: u_boot_binman_syms_bad.c
 
diff --git a/tools/binman/test/u_boot_binman_syms_x86.c b/tools/binman/test/u_boot_binman_syms_x86.c
new file mode 120000
index 0000000000..939b2e965f
--- /dev/null
+++ b/tools/binman/test/u_boot_binman_syms_x86.c
@@ -0,0 +1 @@
+u_boot_binman_syms.c
\ No newline at end of file
diff --git a/tools/binman/test/u_boot_binman_syms_x86.lds b/tools/binman/test/u_boot_binman_syms_x86.lds
new file mode 100644
index 0000000000..9daf86f833
--- /dev/null
+++ b/tools/binman/test/u_boot_binman_syms_x86.lds
@@ -0,0 +1,30 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (c) 2016 Google, Inc
+ */
+
+OUTPUT_FORMAT("elf32-i386", "elf32-i386", "elf32-i386")
+OUTPUT_ARCH(i386)
+ENTRY(_start)
+
+SECTIONS
+{
+	. = 0xffffff00;
+	_start = .;
+
+	. = ALIGN(4);
+	.text :
+	{
+		__image_copy_start = .;
+		*(.text*)
+	}
+
+	. = ALIGN(4);
+	.binman_sym_table : {
+		__binman_sym_start = .;
+		KEEP(*(SORT(.binman_sym*)));
+		__binman_sym_end = .;
+	}
+	.interp : { *(.interp*) }
+
+}