Subject: Re: [PATCH part1 v2 3/9] ps3: Get lv1 high memory region from the repository

From: Geoff Levand <[email-masked]>
Date: Tue, 23 Aug 2011 13:53:40 -0700

On 08/11/2011 12:31 PM, Andre Heider wrote:
> This lets the bootloader preallocate the high lv1 region and pass its
> location to the kernel through the repository. Thus, it can be used to
> hold the initrd. If the region info doesn't exist, the kernel retains
> the old behavior and attempts to allocate the region itself.
> 
> Based on the patch
> "[PS3] Get lv1 high memory region from devtree"
> from Hector Martin <[email-masked]>
> 
> Signed-off-by: Andre Heider <[email-masked]>
> ---
>  arch/powerpc/platforms/ps3/mm.c |   46 ++++++++++++++++++++++++++++++++++++--
>  1 files changed, 43 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/ps3/mm.c b/arch/powerpc/platforms/ps3/mm.c
> index c204588..983b719 100644
> --- a/arch/powerpc/platforms/ps3/mm.c
> +++ b/arch/powerpc/platforms/ps3/mm.c
> @@ -78,12 +78,14 @@ enum {
>   * @base: base address
>   * @size: size in bytes
>   * @offset: difference between base and rm.size
> + * @destroy: flag if region should be destroyed upon shutdown
>   */
>  
>  struct mem_region {
>  	u64 base;
>  	u64 size;
>  	unsigned long offset;
> +	int destroy;
>  };
>  
>  /**
> @@ -261,6 +263,7 @@ static int ps3_mm_region_create(struct mem_region *r, unsigned long size)
>  		goto zero_region;
>  	}
>  
> +	r->destroy = 1;
>  	r->offset = r->base - map.rm.size;
>  	return result;
>  
> @@ -279,6 +282,12 @@ static void ps3_mm_region_destroy(struct mem_region *r)
>  	int result;
>  
>  	DBG("%s:%d: r->base = %llxh\n", __func__, __LINE__, r->base);
> +
> +	if (!r->destroy) {
> +		DBG("%s:%d: not destroying region\n", __func__, __LINE__);
> +		return;
> +	}
> +
>  	if (r->base) {
>  		result = lv1_release_memory(r->base);
>  		BUG_ON(result);
> @@ -287,6 +296,29 @@ static void ps3_mm_region_destroy(struct mem_region *r)
>  	}
>  }
>  
> +static int ps3_mm_get_repository_highmem(struct mem_region *r)
> +{
> +	int result = ps3_repository_read_highmem_info(&r->base, &r->size);
> +
> +	if (result)
> +		goto zero_region;
> +
> +	if (!r->base || !r->size) {
> +		result = -1;
> +		goto zero_region;
> +	}
> +
> +	r->offset = r->base - map.rm.size;
> +	DBG("%s:%d got high region from repository: %llxh %llxh\n",
> +	    __func__, __LINE__, r->base, r->size);
> +	return 0;
> +
> +zero_region:
> +	DBG("%s:%d no high region in repository...\n", __func__, __LINE__);

Three dots implies something more is on its way.  I
don't think we need them.

> +	r->size = r->base = r->offset = 0;
> +	return result;
> +}
> +
>  /**
>   * ps3_mm_add_memory - hot add memory
>   */
> @@ -303,6 +335,12 @@ static int __init ps3_mm_add_memory(void)
>  
>  	BUG_ON(!mem_init_done);
>  
> +	if (!map.r1.size) {
> +		DBG("%s:%d: no region 1, not adding memory\n",
> +		    __func__, __LINE__);
> +		return 0;
> +	}
> +
>  	start_addr = map.rm.size;
>  	start_pfn = start_addr >> PAGE_SHIFT;
>  	nr_pages = (map.r1.size + PAGE_SIZE - 1) >> PAGE_SHIFT;
> @@ -1217,9 +1255,11 @@ void __init ps3_mm_init(void)
>  	BUG_ON(map.rm.base);
>  	BUG_ON(!map.rm.size);
>  
> -
> -	/* arrange to do this in ps3_mm_add_memory */
> -	ps3_mm_region_create(&map.r1, map.total - map.rm.size);
> +	/* check if we got the highmem region from an earlier boot step */
> +	if (ps3_mm_get_repository_highmem(&map.r1)) {
> +		/* arrange to do this in ps3_mm_add_memory */
> +		ps3_mm_region_create(&map.r1, map.total - map.rm.size);
> +	}
>  
>  	/* correct map.total for the real total amount of memory we use */
>  	map.total = map.rm.size + map.r1.size;