Re: [kvm-unit-tests PATCH v4 05/17] arm/arm64: setup improvements

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 4559 bytes --]

On Mon, Feb 15, 2016 at 02:49:14PM +0100, Andrew Jones wrote:
> Handle multiple memory regions being described in the DT. Also
> add a more informative error message for when a user does -smp N,
> N > NR_CPUS.
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

For whatever that's worth, given that I don't know ARM or
kvm-unit-tests at all well.


> 
> ---
> powerpc will adapt arm's setup to be used as it's own, and it
> needs these changes. There's no reason to only do them for power,
> so we do them in arm first, getting them ready to be adapted.
> ---
>  lib/arm/asm/setup.h |  9 ++++++++
>  lib/arm/setup.c     | 66 ++++++++++++++++++++++++++++++++++++++---------------
>  2 files changed, 56 insertions(+), 19 deletions(-)
> 
> diff --git a/lib/arm/asm/setup.h b/lib/arm/asm/setup.h
> index 02b668672fca4..cb8fdbd38dd5d 100644
> --- a/lib/arm/asm/setup.h
> +++ b/lib/arm/asm/setup.h
> @@ -6,6 +6,7 @@
>   * This work is licensed under the terms of the GNU LGPL, version 2.
>   */
>  #include <libcflat.h>
> +#include <alloc.h>	/* phys_addr_t */
>  #include <asm/page.h>
>  #include <asm/pgtable-hwdef.h>
>  
> @@ -13,6 +14,14 @@
>  extern u32 cpus[NR_CPUS];
>  extern int nr_cpus;
>  
> +#define NR_MEM_REGIONS		8
> +#define MR_F_PRIMARY		(1U << 0)
> +struct mem_region {
> +	phys_addr_t start;
> +	phys_addr_t end;
> +	unsigned int flags;
> +};
> +extern struct mem_region mem_regions[NR_MEM_REGIONS];
>  extern phys_addr_t __phys_offset, __phys_end;
>  
>  #define PHYS_OFFSET		(__phys_offset)
> diff --git a/lib/arm/setup.c b/lib/arm/setup.c
> index da6edc1f9d8ff..8c6172ff94106 100644
> --- a/lib/arm/setup.c
> +++ b/lib/arm/setup.c
> @@ -27,12 +27,18 @@ extern void setup_args(const char *args);
>  u32 cpus[NR_CPUS] = { [0 ... NR_CPUS-1] = (~0U) };
>  int nr_cpus;
>  
> +struct mem_region mem_regions[NR_MEM_REGIONS];
>  phys_addr_t __phys_offset, __phys_end;
>  
>  static void cpu_set(int fdtnode __unused, u32 regval, void *info __unused)
>  {
>  	int cpu = nr_cpus++;
> -	assert(cpu < NR_CPUS);
> +
> +	if (cpu >= NR_CPUS) {
> +		printf("Number cpus exceeds maximum supported (%d).\n",
> +			NR_CPUS);
> +		assert(0);
> +	}
>  	cpus[cpu] = regval;
>  	set_cpu_present(cpu, true);
>  }
> @@ -49,24 +55,46 @@ static void cpu_init(void)
>  
>  static void mem_init(phys_addr_t freemem_start)
>  {
> -	/* we only expect one membank to be defined in the DT */
> -	struct dt_pbus_reg regs[1];
> -	phys_addr_t mem_start, mem_end;
> -	int ret;
> -
> -	ret = dt_get_memory_params(regs, 1);
> -	assert(ret != 0);
> -
> -	mem_start = regs[0].addr;
> -	mem_end = mem_start + regs[0].size;
> -
> -	assert(!(mem_start & ~PHYS_MASK) && !((mem_end-1) & ~PHYS_MASK));
> -	assert(freemem_start >= mem_start && freemem_start < mem_end);
> -
> -	__phys_offset = mem_start;	/* PHYS_OFFSET */
> -	__phys_end = mem_end;		/* PHYS_END */
> -
> -	phys_alloc_init(freemem_start, mem_end - freemem_start);
> +	struct dt_pbus_reg regs[NR_MEM_REGIONS];
> +	struct mem_region primary, mem = {
> +		.start = (phys_addr_t)-1,
> +	};
> +	int nr_regs, i;
> +
> +	nr_regs = dt_get_memory_params(regs, NR_MEM_REGIONS);
> +	assert(nr_regs > 0);
> +
> +	primary.end = 0;
> +
> +	for (i = 0; i < nr_regs; ++i) {
> +		mem_regions[i].start = regs[i].addr;
> +		mem_regions[i].end = regs[i].addr + regs[i].size;
> +
> +		/*
> +		 * pick the region we're in for our primary region
> +		 */
> +		if (freemem_start >= mem_regions[i].start
> +				&& freemem_start < mem_regions[i].end) {
> +			mem_regions[i].flags |= MR_F_PRIMARY;
> +			primary = mem_regions[i];
> +		}
> +
> +		/*
> +		 * set the lowest and highest addresses found,
> +		 * ignoring potential gaps
> +		 */
> +		if (mem_regions[i].start < mem.start)
> +			mem.start = mem_regions[i].start;
> +		if (mem_regions[i].end > mem.end)
> +			mem.end = mem_regions[i].end;
> +	}
> +	assert(primary.end != 0);
> +	assert(!(mem.start & ~PHYS_MASK) && !((mem.end - 1) & ~PHYS_MASK));
> +
> +	__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);
>  
>  	mmu_enable_idmap();

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]