[Skiboot] [PATCH] Ensure reserved memory ranges are exposed correctly to host (fix corrupted SLW image)
Stewart Smith
stewart at linux.vnet.ibm.com
Tue Sep 22 16:13:45 AEST 2015
Memory regions in skiboot have an interesting life cycle. First, we get
a bunch from the initial device tree or hdat specifying some existing
reserved ranges (as well as adding some of our own if they're missing)
but we also get ranges for the entirety of RAM.
The idea is that we can do node local allocations for per node resources
(which we do) and then, just prior to booting linux, we copy the reserved
memory regions to expose to linux along with a set of reserver regions
to cover the node local allocations.
The problem was that mem_range_is_reserved() was wanting subtle different
semantics for memory region type than region_is_reserved() provided.
That is, we were overriding the meaning of REGION_SKIBOOT_HEAP to mean both
"this is reserved by skiboot" *and* "this is a memory region that covers
all of memory and will be shrunk to cover just the memory we have allocated
for it just before we boot the payload (linux)".
So what would happen is we would ask "hey, is the memory holding the SLW
image reserved?" and we'd get the answer of "yes" but referring to the memory
region that covers the entirety of memory in a NUMA node, *not* meaning
our intent of "this will be reserved when we start linux".
To fix this, introduce a new memory region type REGION_MEMORY. This has
the semantics of a memory region that covers a block of memory that we can
allocate from (using local_alloc) and that the part that was allocated
will be passed to linux as reserved, but that the entire range will not
be reserved.
So our new semantics are:
- region_is_reservable() is true if the region *MAY* be reserved
(i.e. is the regions that cover the whole of memory OR is explicitly reserved)
- region_is_reserved() is true if the region *WILL* be reserved
(i.e. is explicitly reserved)
This way we check that the SLW image is explicitly reserved and if it isn't,
we reserve it.
Fixes: 58033e44
Signed-off-by: Stewart Smith <stewart at linux.vnet.ibm.com>
---
core/mem_region.c | 36 ++++++++++++++++++++-----------
core/test/run-mem_region_init.c | 2 +-
core/test/run-mem_region_release_unused.c | 2 +-
include/mem_region.h | 3 +++
libpore/p8_delta_scan_rw.h | 4 +++-
5 files changed, 32 insertions(+), 15 deletions(-)
diff --git a/core/mem_region.c b/core/mem_region.c
index 17fe268..f8d9a11 100644
--- a/core/mem_region.c
+++ b/core/mem_region.c
@@ -133,7 +133,8 @@ static struct alloc_hdr *next_hdr(const struct mem_region *region,
static void init_allocatable_region(struct mem_region *region)
{
struct free_hdr *f = region_start(region);
- assert(region->type == REGION_SKIBOOT_HEAP);
+ assert(region->type == REGION_SKIBOOT_HEAP ||
+ region->type == REGION_MEMORY);
f->hdr.num_longs = region->len / sizeof(long);
f->hdr.free = true;
f->hdr.prev_free = false;
@@ -256,11 +257,16 @@ static void bad_header(const struct mem_region *region,
abort();
}
-static bool region_is_reserved(struct mem_region *region)
+static bool region_is_reservable(struct mem_region *region)
{
return region->type != REGION_OS;
}
+static bool region_is_reserved(struct mem_region *region)
+{
+ return region->type != REGION_OS && region->type != REGION_MEMORY;
+}
+
void mem_dump_allocs(void)
{
struct mem_region *region;
@@ -269,7 +275,8 @@ void mem_dump_allocs(void)
/* Second pass: populate property data */
printf("Memory regions:\n");
list_for_each(®ions, region, list) {
- if (region->type != REGION_SKIBOOT_HEAP)
+ if (!(region->type == REGION_SKIBOOT_HEAP ||
+ region->type == REGION_MEMORY))
continue;
printf(" 0x%012llx..%012llx : %s\n",
(long long)region->start,
@@ -299,7 +306,8 @@ int64_t mem_dump_free(void)
printf("Free space in HEAP memory regions:\n");
list_for_each(®ions, region, list) {
- if (region->type != REGION_SKIBOOT_HEAP)
+ if (!(region->type == REGION_SKIBOOT_HEAP ||
+ region->type == REGION_MEMORY))
continue;
region_free = 0;
@@ -335,7 +343,8 @@ static void *__mem_alloc(struct mem_region *region, size_t size, size_t align,
assert(is_rodata(location));
/* Unallocatable region? */
- if (region->type != REGION_SKIBOOT_HEAP)
+ if (!(region->type == REGION_SKIBOOT_HEAP ||
+ region->type == REGION_MEMORY))
return NULL;
/* First allocation? */
@@ -535,8 +544,9 @@ bool mem_check(const struct mem_region *region)
}
/* Not ours to play with, or empty? Don't do anything. */
- if (region->type != REGION_SKIBOOT_HEAP ||
- region->free_list.n.next == NULL)
+ if (!(region->type == REGION_MEMORY ||
+ region->type == REGION_SKIBOOT_HEAP) ||
+ region->free_list.n.next == NULL)
return true;
/* Walk linearly. */
@@ -738,7 +748,8 @@ restart:
const struct dt_property *prop;
const __be32 *ids;
- if (region->type != REGION_SKIBOOT_HEAP)
+ if (!(region->type == REGION_SKIBOOT_HEAP ||
+ region->type == REGION_MEMORY))
continue;
/* Don't allocate from normal heap. */
@@ -950,7 +961,7 @@ void mem_region_init(void)
strcat(rname, i->name);
start = dt_get_address(i, 0, &len);
lock(&mem_region_lock);
- region = new_region(rname, start, len, i, REGION_SKIBOOT_HEAP);
+ region = new_region(rname, start, len, i, REGION_MEMORY);
if (!region) {
prerror("MEM: Could not add mem region %s!\n", i->name);
abort();
@@ -1023,7 +1034,8 @@ void mem_region_release_unused(void)
uint64_t used_len;
/* If it's not allocatable, ignore it. */
- if (r->type != REGION_SKIBOOT_HEAP)
+ if (!(r->type == REGION_SKIBOOT_HEAP ||
+ r->type == REGION_MEMORY))
continue;
used_len = allocated_length(r);
@@ -1123,7 +1135,7 @@ void mem_region_add_dt_reserved(void)
/* First pass: calculate length of property data */
list_for_each(®ions, region, list) {
- if (!region_is_reserved(region))
+ if (!region_is_reservable(region))
continue;
names_len += strlen(region->name) + 1;
ranges_len += 2 * sizeof(uint64_t);
@@ -1135,7 +1147,7 @@ void mem_region_add_dt_reserved(void)
printf("Reserved regions:\n");
/* Second pass: populate property data */
list_for_each(®ions, region, list) {
- if (!region_is_reserved(region))
+ if (!region_is_reservable(region))
continue;
len = strlen(region->name) + 1;
memcpy(name, region->name, len);
diff --git a/core/test/run-mem_region_init.c b/core/test/run-mem_region_init.c
index 7ab23d2..d780ae5 100644
--- a/core/test/run-mem_region_init.c
+++ b/core/test/run-mem_region_init.c
@@ -164,7 +164,7 @@ int main(void)
r == &skiboot_os_reserve)
builtins++;
else
- assert(r->type == REGION_SKIBOOT_HEAP);
+ assert(r->type == REGION_MEMORY);
assert(mem_check(r));
}
assert(builtins == 5);
diff --git a/core/test/run-mem_region_release_unused.c b/core/test/run-mem_region_release_unused.c
index 980f4c4..fca5278 100644
--- a/core/test/run-mem_region_release_unused.c
+++ b/core/test/run-mem_region_release_unused.c
@@ -159,7 +159,7 @@ int main(void)
if (r == &skiboot_cpu_stacks)
continue;
if (r == other) {
- assert(r->type == REGION_SKIBOOT_HEAP);
+ assert(r->type == REGION_MEMORY);
assert(r->len < 1024 * 1024);
} else {
assert(r->type == REGION_OS);
diff --git a/include/mem_region.h b/include/mem_region.h
index 913dbb6..c34858e 100644
--- a/include/mem_region.h
+++ b/include/mem_region.h
@@ -25,6 +25,9 @@ enum mem_region_type {
/* ranges allocatable by mem_alloc: this will be most of memory */
REGION_SKIBOOT_HEAP,
+ /* ranges allocatable by mem_alloc but shrunk (e.g. whole memory) */
+ REGION_MEMORY,
+
/* ranges used explicitly for skiboot, but not allocatable. eg .text */
REGION_SKIBOOT_FIRMWARE,
diff --git a/libpore/p8_delta_scan_rw.h b/libpore/p8_delta_scan_rw.h
index 5637e7c..54002e4 100644
--- a/libpore/p8_delta_scan_rw.h
+++ b/libpore/p8_delta_scan_rw.h
@@ -156,10 +156,12 @@
#define MY_DBG(_fmt_, _args_...) printf(_fmt_, ##_args_)
#else // End of SLW_COMMAND_LINE
#ifdef __SKIBOOT__
+#define pr_fmt(fmt) "LIBPORE: " fmt
#include <skiboot.h>
-//#define MY_INF(_fmt_, _args_...) printf(_fmt_, ##_args_)
+//#define MY_INF(_fmt_, _args_...) prlog(PR_TRACE, _fmt_, ##_args_)
#define MY_INF(_fmt_, _args_...) do { } while(0)
#define MY_ERR(_fmt_, _args_...) prerror(_fmt_, ##_args_)
+//#define MY_DBG(_fmt_, _args_...) prlog(PR_INSANE, _fmt_, ##_args_)
#define MY_DBG(_fmt_, _args_...) do { } while(0)
#else
#define MY_INF(_fmt_, _args_...) do { } while(0)
--
2.1.4
More information about the Skiboot
mailing list