selfboot: create selfboot_check function, remove check param

The selfboot function was changed at some point to take a parameter
which meant "check the allocated descriptors to see if they target
regions of real memory."

The region check had to be buried deep in the last step of loading since
that is where those descriptors were created and used.

An issue with the use of the parameter was that it was not possible
for compilers to easily divine whether the check code was used,
and it was hence possible for the code, and its dependencies, to be
compiled in even if never used (which caused problems for the
rampayload code).

Now that bounce buffers are gone, we can hoist the check code
to the outermost level. Further, by creating a selfload_check
and selfload function, we can make it easy for compilers
to discard unused code: if selfload_check is never called, all
the code it uses can be discarded too.

Change-Id: Id5b3f450fd18480d54ffb6e395429fba71edcd77
Signed-off-by: Ronald G. Minnich <rminnich@gmail.com>
Reviewed-on: https://review.coreboot.org/29259
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Aaron Durbin <adurbin@chromium.org>
Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net>

4 files changed, 56 insertions, 16 deletions

diff --git a/src/arch/arm64/arm_tf.c b/src/arch/arm64/arm_tf.c
index fcf66cac61..384b1b724e 100644
--- a/src/arch/arm64/arm_tf.c
+++ b/src/arch/arm64/arm_tf.c
@@ -50,7 +50,7 @@ void arm_tf_run_bl31(u64 payload_entry, u64 payload_arg0, u64 payload_spsr)
 	if (prog_locate(&bl31))
 		die("BL31 not found");
 
-	if (!selfload(&bl31, false))
+	if (!selfload(&bl31))
 		die("BL31 load failed");
 	bl31_entry = prog_entry(&bl31);
 
diff --git a/src/include/program_loading.h b/src/include/program_loading.h
index aa21cf57ab..e185b9271b 100644
--- a/src/include/program_loading.h
+++ b/src/include/program_loading.h
@@ -197,12 +197,15 @@ void payload_run(void);
 void mirror_payload(struct prog *payload);
 
 /*
- * Set check_regions to true to check that the payload targets usable memory.
- * With this flag set, if it does not, the load will fail and this function
- * will return false. On successful payload loading this functions return true.
+ * selfload() and selfload_check() load payloads into memory.
+ * selfload() does not check the payload to see if it targets memory.
+ * Call selfload_check() to check that the payload targets usable memory.
+ * If it does not, the load will fail and this function
+ * will return false. On successful payload loading these functions return true.
  *
  * Defined in src/lib/selfboot.c
  */
-bool selfload(struct prog *payload, bool check_regions);
+bool selfload_check(struct prog *payload);
+bool selfload(struct prog *payload);
 
 #endif /* PROGRAM_LOADING_H */
diff --git a/src/lib/prog_loaders.c b/src/lib/prog_loaders.c
index 883dbdc7ec..5004a7fd5e 100644
--- a/src/lib/prog_loaders.c
+++ b/src/lib/prog_loaders.c
@@ -185,7 +185,7 @@ void payload_load(void)
 
 	switch (prog_cbfs_type(payload)) {
 	case CBFS_TYPE_SELF: /* Simple ELF */
-		selfload(payload, true);
+		selfload_check(payload);
 		break;
 	case CBFS_TYPE_FIT: /* Flattened image tree */
 		if (IS_ENABLED(CONFIG_PAYLOAD_FIT_SUPPORT)) {
diff --git a/src/lib/selfboot.c b/src/lib/selfboot.c
index c9c8beefd8..efd2d3d5a9 100644
--- a/src/lib/selfboot.c
+++ b/src/lib/selfboot.c
@@ -30,6 +30,9 @@
 #include <timestamp.h>
 #include <cbmem.h>
 
+/* The type syntax for C is essentially unparsable. -- Rob Pike */
+typedef int (*checker_t)(struct cbfs_payload_segment *cbfssegs);
+
 /* Decode a serialized cbfs payload segment
  * from memory into native endianness.
  */
@@ -138,10 +141,26 @@ static int last_loadable_segment(struct cbfs_payload_segment *seg)
 	return read_be32(&(seg + 1)->type) == PAYLOAD_SEGMENT_ENTRY;
 }
 
-static int load_payload_segments(
-	struct cbfs_payload_segment *cbfssegs,
-	int check_regions,
-	uintptr_t *entry)
+static int check_payload_segments(struct cbfs_payload_segment *cbfssegs)
+{
+	uint8_t *dest;
+	size_t memsz;
+	struct cbfs_payload_segment *first_segment, *seg, segment;
+
+	for (first_segment = seg = cbfssegs;; ++seg) {
+		printk(BIOS_DEBUG, "Checking segment from ROM address 0x%p\n", seg);
+		cbfs_decode_payload_segment(&segment, seg);
+		dest = (uint8_t *)(uintptr_t)segment.load_addr;
+		memsz = segment.mem_len;
+		if (segment.type == PAYLOAD_SEGMENT_ENTRY)
+			break;
+		if (!segment_targets_usable_ram(dest, memsz))
+			return -1;
+	}
+	return 0;
+}
+
+static int load_payload_segments(struct cbfs_payload_segment *cbfssegs, uintptr_t *entry)
 {
 	uint8_t *dest, *src;
 	size_t filesz, memsz;
@@ -202,8 +221,6 @@ static int load_payload_segments(
 			printk(BIOS_EMERG, "Bad segment type %x\n", segment.type);
 			return -1;
 		}
-		if (check_regions && !segment_targets_usable_ram(dest, memsz))
-			return -1;
 		/* Note that the 'seg + 1' is safe as we only call this
 		 * function on "not the last" * items, since entry
 		 * is always last. */
@@ -221,19 +238,29 @@ __weak int payload_arch_usable_ram_quirk(uint64_t start, uint64_t size)
 	return 0;
 }
 
-bool selfload(struct prog *payload, bool check_regions)
+static void *selfprepare(struct prog *payload)
+{
+	void *data;
+	data = rdev_mmap_full(prog_rdev(payload));
+	return data;
+}
+
+static bool _selfload(struct prog *payload, checker_t f)
 {
 	uintptr_t entry = 0;
 	struct cbfs_payload_segment *cbfssegs;
 	void *data;
 
-	data = rdev_mmap_full(prog_rdev(payload));
-
+	data = selfprepare(payload);
 	if (data == NULL)
 		return false;
 
 	cbfssegs = &((struct cbfs_payload *)data)->segments;
-	if (load_payload_segments(cbfssegs, check_regions, &entry))
+
+	if (f && f(cbfssegs))
+		goto out;
+
+	if (load_payload_segments(cbfssegs, &entry))
 		goto out;
 
 	printk(BIOS_SPEW, "Loaded segments\n");
@@ -248,3 +275,13 @@ out:
 	rdev_munmap(prog_rdev(payload), data);
 	return false;
 }
+
+bool selfload_check(struct prog *payload)
+{
+	return _selfload(payload, check_payload_segments);
+}
+
+bool selfload(struct prog *payload)
+{
+	return _selfload(payload, NULL);
+}