Re: [kvm-unit-tests PATCH v2 2/6] lib/alloc: improve init

[Laurent Vivier <lvivier@redhat.com>]

On 02/11/2016 21:52, Andrew Jones wrote:
> Add some asserts to make sure phys_alloc_init is called only once
> and before any other alloc calls. Also require that an alignment
> is set up just once and *before* init, or never.
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>

Reviewed-by: Laurent Vivier <lvivier@redhat.com>

> ---
>  lib/alloc.c         | 26 ++++++++++++++++++--------
>  lib/alloc.h         |  7 +++++--
>  lib/arm/setup.c     |  2 +-
>  lib/powerpc/setup.c |  2 +-
>  4 files changed, 25 insertions(+), 12 deletions(-)
> 
> diff --git a/lib/alloc.c b/lib/alloc.c
> index e1d7b8a380ff..5b3ef5d00f8a 100644
> --- a/lib/alloc.c
> +++ b/lib/alloc.c
> @@ -43,18 +43,28 @@ void phys_alloc_show(void)
>  void phys_alloc_init(phys_addr_t base_addr, phys_addr_t size)
>  {
>  	spin_lock(&lock);
> +
> +	if (!align_min)
> +		align_min = DEFAULT_MINIMUM_ALIGNMENT;
> +
> +	assert(!top);
> +	assert(!(base_addr & (align_min - 1)));
> +	assert(size);
> +
>  	base = base_addr;
>  	top = base + size;
> -	align_min = DEFAULT_MINIMUM_ALIGNMENT;
> -	nr_regions = 0;
> +
>  	spin_unlock(&lock);
>  }
>  
>  void phys_alloc_set_minimum_alignment(phys_addr_t align)
>  {
> -	assert(align && !(align & (align - 1)));
>  	spin_lock(&lock);
> +
> +	assert(!align_min);
> +	assert(align && !(align & (align - 1)));
>  	align_min = align;
> +
>  	spin_unlock(&lock);
>  }
>  
> @@ -63,14 +73,14 @@ static phys_addr_t phys_alloc_aligned_safe(phys_addr_t size,
>  {
>  	static bool warned = false;
>  	phys_addr_t addr, size_orig = size;
> -	u64 top_safe;
> +	u64 top_safe = top;
>  
> -	spin_lock(&lock);
> +	if (safe && sizeof(long) == 4)
> +		top_safe = MIN(top, 1ULL << 32);
>  
> -	top_safe = top;
> +	assert(top_safe && base < top_safe);
>  
> -	if (safe && sizeof(long) == 4)
> -		top_safe = MIN(top_safe, 1ULL << 32);
> +	spin_lock(&lock);
>  
>  	align = MAX(align, align_min);
>  
> diff --git a/lib/alloc.h b/lib/alloc.h
> index c12bd15f7afc..bd3c4e8ff3f6 100644
> --- a/lib/alloc.h
> +++ b/lib/alloc.h
> @@ -73,13 +73,16 @@ static inline void *memalign(size_t alignment, size_t size)
>  
>  /*
>   * phys_alloc_init creates the initial free memory region of size @size
> - * at @base. The minimum alignment is set to DEFAULT_MINIMUM_ALIGNMENT.
> + * at @base. If a minimum alignment has not been set then
> + * DEFAULT_MINIMUM_ALIGNMENT is used. phys_alloc_init may only be called
> + * once.
>   */
>  extern void phys_alloc_init(phys_addr_t base, phys_addr_t size);
>  
>  /*
>   * phys_alloc_set_minimum_alignment sets the minimum alignment to
> - * @align.
> + * @align. phys_alloc_set_minimum_alignment must be called before
> + * phys_alloc_init and only once.
>   */
>  extern void phys_alloc_set_minimum_alignment(phys_addr_t align);
>  
> diff --git a/lib/arm/setup.c b/lib/arm/setup.c
> index 7e7b39f11dde..d02d1a731d20 100644
> --- a/lib/arm/setup.c
> +++ b/lib/arm/setup.c
> @@ -94,8 +94,8 @@ static void mem_init(phys_addr_t freemem_start)
>  	__phys_offset = mem.start;	/* PHYS_OFFSET */
>  	__phys_end = mem.end;		/* PHYS_END */
>  
> -	phys_alloc_init(freemem_start, primary.end - freemem_start);
>  	phys_alloc_set_minimum_alignment(SMP_CACHE_BYTES);
> +	phys_alloc_init(freemem_start, primary.end - freemem_start);
>  
>  	mmu_enable_idmap();
>  }
> diff --git a/lib/powerpc/setup.c b/lib/powerpc/setup.c
> index 9153f7bebc6a..b390dadaebbe 100644
> --- a/lib/powerpc/setup.c
> +++ b/lib/powerpc/setup.c
> @@ -146,9 +146,9 @@ static void mem_init(phys_addr_t freemem_start)
>  	__physical_start = mem.start;	/* PHYSICAL_START */
>  	__physical_end = mem.end;	/* PHYSICAL_END */
>  
> -	phys_alloc_init(freemem_start, primary.end - freemem_start);
>  	phys_alloc_set_minimum_alignment(__icache_bytes > __dcache_bytes
>  					 ? __icache_bytes : __dcache_bytes);
> +	phys_alloc_init(freemem_start, primary.end - freemem_start);
>  }
>  
>  void setup(const void *fdt)
>