[Skiboot] [PATCH] Ensure reserved memory ranges are exposed correctly to host (fix corrupted SLW image)

[Jeremy Kerr]

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