core: Prevent adding new regions after mem_region_add_dt_reserved

If we reserve any memory after mem_region_add_dt_reserved, that
reservation won't appear in the device tree. Ensure that we can't
add new regions after this point.

Also, add a testcase for the finalise, including some basic
reserved-ranges property checks.

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>

3 files changed, 199 insertions, 1 deletions

diff --git a/core/mem_region.c b/core/mem_region.c
index afffe99..a5a11f5 100644
--- a/core/mem_region.c
+++ b/core/mem_region.c
@@ -47,6 +47,8 @@ struct lock mem_region_lock = LOCK_UNLOCKED;
 
 static struct list_head regions = LIST_HEAD_INIT(regions);
 
+static bool mem_regions_finalised = false;
+
 unsigned long top_of_ram = SKIBOOT_BASE + SKIBOOT_SIZE;
 
 static struct mem_region skiboot_os_reserve = {
@@ -637,6 +639,12 @@ static bool add_region(struct mem_region *region)
 {
 	struct mem_region *r;
 
+	if (mem_regions_finalised) {
+		prerror("MEM: add_region(%s@0x%llx) called after finalise!\n",
+				region->name, region->start);
+		return false;
+	}
+
 	/* First split any regions which intersect. */
 	list_for_each(&regions, r, list)
 		if (!maybe_split(r, region->start) ||
@@ -876,6 +884,7 @@ void mem_region_release_unused(void)
 	struct mem_region *r;
 
 	lock(&mem_region_lock);
+	assert(!mem_regions_finalised);
 
 	printf("Releasing unused memory:\n");
 	list_for_each(&regions, r, list) {
@@ -929,7 +938,11 @@ void mem_region_add_dt_reserved(void)
 	names_len = 0;
 	ranges_len = 0;
 
+	/* Finalise the region list, so we know that the regions list won't be
+	 * altered after this point. The regions' free lists may change after
+	 * we drop the lock, but we don't access those. */
 	lock(&mem_region_lock);
+	mem_regions_finalised = true;
 
 	/* First pass: calculate length of property data */
 	list_for_each(&regions, region, list) {
diff --git a/core/test/Makefile.check b/core/test/Makefile.check
index 457b61c..3044ecc 100644
--- a/core/test/Makefile.check
+++ b/core/test/Makefile.check
@@ -1,5 +1,5 @@
 # -*-Makefile-*-
-CORE_TEST := core/test/run-device core/test/run-mem_region core/test/run-malloc core/test/run-malloc-speed core/test/run-mem_region_init core/test/run-mem_region_release_unused core/test/run-mem_region_release_unused_noalloc core/test/run-trace core/test/run-msg core/test/run-pel core/test/run-pool core/test/run-timer
+CORE_TEST := core/test/run-device core/test/run-mem_region core/test/run-malloc core/test/run-malloc-speed core/test/run-mem_region_init core/test/run-mem_region_release_unused core/test/run-mem_region_release_unused_noalloc core/test/run-mem_region_reservations core/test/run-trace core/test/run-msg core/test/run-pel core/test/run-pool core/test/run-timer
 
 CORE_TEST_NOSTUB := core/test/run-console-log
 CORE_TEST_NOSTUB += core/test/run-console-log-buf-overrun
diff --git a/core/test/run-mem_region_reservations.c b/core/test/run-mem_region_reservations.c
new file mode 100644
index 0000000..3288432
--- /dev/null
+++ b/core/test/run-mem_region_reservations.c
@@ -0,0 +1,185 @@
+/* Copyright 2013-2015 IBM Corp.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *	http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+ * implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <config.h>
+
+#define BITS_PER_LONG (sizeof(long) * 8)
+/* Don't include this, it's PPC-specific */
+#define __CPU_H
+static unsigned int cpu_max_pir = 1;
+struct cpu_thread {
+	unsigned int			chip_id;
+};
+
+#include <stdlib.h>
+
+static void *real_malloc(size_t size)
+{
+	return malloc(size);
+}
+
+static void real_free(void *p)
+{
+	return free(p);
+}
+
+#undef malloc
+#undef free
+#undef realloc
+
+#include <skiboot.h>
+#include <mem_region-malloc.h>
+
+/* We need mem_region to accept __location__ */
+#define is_rodata(p) true
+#include "../mem_region.c"
+#include "../malloc.c"
+
+/* But we need device tree to make copies of names. */
+#undef is_rodata
+#define is_rodata(p) false
+#include "../../libc/string/strdup.c"
+
+#include "../device.c"
+#include <assert.h>
+#include <stdio.h>
+
+void lock(struct lock *l)
+{
+	assert(!l->lock_val);
+	l->lock_val++;
+}
+
+void unlock(struct lock *l)
+{
+	assert(l->lock_val);
+	l->lock_val--;
+}
+
+bool lock_held_by_me(struct lock *l)
+{
+	return l->lock_val;
+}
+
+#define TEST_HEAP_ORDER 12
+#define TEST_HEAP_SIZE (1ULL << TEST_HEAP_ORDER)
+
+static void add_mem_node(uint64_t start, uint64_t len)
+{
+	struct dt_node *mem;
+	u64 reg[2];
+	char *name;
+
+	name = (char*)malloc(sizeof("memory@") + STR_MAX_CHARS(reg[0]));
+	assert(name);
+
+	/* reg contains start and length */
+	reg[0] = cpu_to_be64(start);
+	reg[1] = cpu_to_be64(len);
+
+	sprintf(name, "memory@%llx", (long long)start);
+
+	mem = dt_new(dt_root, name);
+	dt_add_property_string(mem, "device_type", "memory");
+	dt_add_property(mem, "reg", reg, sizeof(reg));
+	free(name);
+}
+
+void add_chip_dev_associativity(struct dt_node *dev __attribute__((unused)))
+{
+}
+
+static struct {
+	const char	*name;
+	uint64_t	addr;
+	bool		found;
+} test_regions[] = {
+	{ "test.1", 0x1000, false },
+	{ "test.2", 0x2000, false },
+	{ "test.3", 0x4000, false },
+};
+
+int main(void)
+{
+	const struct dt_property *names, *ranges;
+	struct mem_region *r;
+	unsigned int i, l, c;
+	uint64_t *rangep;
+	const char *name;
+	void *buf;
+
+	/* Use malloc for the heap, so valgrind can find issues. */
+	skiboot_heap.start = (long)real_malloc(TEST_HEAP_SIZE);
+	skiboot_heap.len = TEST_HEAP_SIZE;
+	skiboot_os_reserve.len = skiboot_heap.start;
+
+	dt_root = dt_new_root("");
+	dt_add_property_cells(dt_root, "#address-cells", 2);
+	dt_add_property_cells(dt_root, "#size-cells", 2);
+
+	buf = real_malloc(1024*1024);
+	add_mem_node((unsigned long)buf, 1024*1024);
+
+	/* Now convert. */
+	mem_region_init();
+
+	/* create our reservations */
+	for (i = 0; i < ARRAY_SIZE(test_regions); i++)
+		mem_reserve(test_regions[i].name, test_regions[i].addr, 0x1000);
+
+	/* release unused */
+	mem_region_release_unused();
+
+	/* and create reservations */
+	mem_region_add_dt_reserved();
+
+	/* ensure we can't create further reservations */
+	r = new_region("test.4", 0x5000, 0x1000, NULL, REGION_RESERVED);
+	assert(!add_region(r));
+
+	/* check dt properties */
+	names = dt_find_property(dt_root, "reserved-names");
+	ranges = dt_find_property(dt_root, "reserved-ranges");
+
+	assert(names && ranges);
+
+	/* walk through names & ranges properies, ensuring that the test
+	 * regions are all present */
+	for (name = names->prop, rangep = (uint64_t *)ranges->prop, c = 0;
+			name < names->prop + names->len;
+			name += l, rangep += 2) {
+		uint64_t addr;
+
+		addr = dt_get_number(rangep, 2);
+		l = strlen(name) + 1;
+
+		for (i = 0; i < ARRAY_SIZE(test_regions); i++) {
+			if (strcmp(test_regions[i].name, name))
+				continue;
+			assert(test_regions[i].addr == addr);
+			assert(!test_regions[i].found);
+			test_regions[i].found = true;
+			c++;
+		}
+	}
+
+	assert(c == ARRAY_SIZE(test_regions));
+
+	dt_free(dt_root);
+	real_free(buf);
+	real_free((void *)(long)skiboot_heap.start);
+	return 0;
+}