[Skiboot] [PATCH] Ensure reserved memory ranges are exposed correctly to host (fix corrupted SLW image)
Jeremy Kerr
jk at ozlabs.org
Tue Sep 22 16:26:50 AEST 2015
Hi Stewart,
> Received: from d03dlp03.boulder.ibm.com (9.17.202.179)
> by e33.co.us.ibm.com (192.168.1.133) with IBM ESMTP SMTP Gateway:
> Authorized Use Only! Violators will be prosecuted;
eep!
> 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)".
Kinda - the _HEAP region is just the stuff that we could possibly allocate
from.
> 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)
Sounds good to me.
> 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)
I assume this is from an unrelated change:
Otherwise:
Acked-by: Jeremy Kerr <jk at ozlabs.org>
Cheers,
Jeremy
More information about the Skiboot
mailing list