Re: [kvm-unit-tests PATCH v5 13/18] powerpc/ppc64: adapt arm's setup

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

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

On Wed, Feb 17, 2016 at 07:40:54PM +0100, Andrew Jones wrote:
> Copy arm's setup code (also DT based) over to powerpc, adapting
> it a bit. Also bring over arm's setup selftest, giving powerpc
> its first test.
> 
> The largest change from arm's setup.c is that instead of using a
> hardcoded SMP_CACHE_BYTES, cpu_set() is extended to extract the
> line size from the cpu DT nodes. That change also requires that
> we call cpu_init() before mem_init() in setup().
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Tested-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  lib/powerpc/asm/setup.h |  30 +++++++++++
>  lib/powerpc/setup.c     | 141 ++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/ppc64/asm/setup.h   |   1 +
>  powerpc/Makefile.common |   1 +
>  powerpc/cstart64.S      |  16 +++++-
>  powerpc/selftest.c      |  63 ++++++++++++++++++++--
>  6 files changed, 248 insertions(+), 4 deletions(-)
>  create mode 100644 lib/powerpc/asm/setup.h
>  create mode 100644 lib/powerpc/setup.c
>  create mode 100644 lib/ppc64/asm/setup.h
> 
> diff --git a/lib/powerpc/asm/setup.h b/lib/powerpc/asm/setup.h
> new file mode 100644
> index 0000000000000..8288ff37ff1b9
> --- /dev/null
> +++ b/lib/powerpc/asm/setup.h
> @@ -0,0 +1,30 @@
> +#ifndef _ASMPOWERPC_SETUP_H_
> +#define _ASMPOWERPC_SETUP_H_
> +/*
> + * Copyright (C) 2016, Red Hat Inc, Andrew Jones <drjones@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.
> + */
> +#include <libcflat.h>
> +#include <alloc.h>	/* phys_addr_t */
> +
> +#define NR_CPUS			8	/* arbitrarily set for now */

We probably want to set this larger sooner rather than later.  The
test machines the virt dev team uses have 160 logical CPUs, and
they're not particularly large as Power systems go.

> +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 __physical_start, __physical_end;
> +extern unsigned __smp_cache_bytes;
> +
> +#define PHYSICAL_START		(__physical_start)
> +#define PHYSICAL_END		(__physical_end)
> +#define SMP_CACHE_BYTES		(__smp_cache_bytes)
> +
> +#endif /* _ASMPOWERPC_SETUP_H_ */
> diff --git a/lib/powerpc/setup.c b/lib/powerpc/setup.c
> new file mode 100644
> index 0000000000000..70f50deb9bacf
> --- /dev/null
> +++ b/lib/powerpc/setup.c
> @@ -0,0 +1,141 @@
> +/*
> + * Initialize machine setup information and I/O.
> + *
> + * After running setup() unit tests may query how many cpus they have
> + * (nr_cpus), how much memory they have (PHYSICAL_END - PHYSICAL_START),
> + * may use dynamic memory allocation (malloc, etc.), printf, and exit.
> + * Finally, argc and argv are also ready to be passed to main().
> + *
> + * Copyright (C) 2016, Red Hat Inc, Andrew Jones <drjones@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.
> + */
> +#include <libcflat.h>
> +#include <libfdt/libfdt.h>
> +#include <devicetree.h>
> +#include <alloc.h>
> +#include <asm/setup.h>
> +#include <asm/page.h>
> +
> +extern unsigned long stacktop;
> +extern void io_init(void);
> +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 __physical_start, __physical_end;
> +unsigned __smp_cache_bytes;
> +
> +static void cpu_set(int fdtnode, u32 regval, void *info)
> +{
> +	const struct fdt_property *prop;
> +	unsigned *max_linesz = (unsigned *)info, sz;
> +	int cpu = nr_cpus++;
> +	u32 *data;
> +
> +	if (cpu >= NR_CPUS) {
> +		printf("Number cpus exceeds maximum supported (%d).\n",
> +			NR_CPUS);
> +		assert(0);
> +	}
> +	cpus[cpu] = regval;
> +
> +	prop = fdt_get_property(dt_fdt(), fdtnode, "i-cache-line-size", NULL);
> +	assert(prop != NULL);
> +	data = (u32 *)prop->data;
> +	sz = fdt32_to_cpu(*data);
> +
> +	if (*max_linesz < sz)
> +		*max_linesz = sz;
> +
> +	prop = fdt_get_property(dt_fdt(), fdtnode, "d-cache-line-size", NULL);
> +	assert(prop != NULL);
> +	data = (u32 *)prop->data;
> +	sz = fdt32_to_cpu(*data);
> +
> +	if (*max_linesz < sz)
> +		*max_linesz = sz;

Hmm.. what is __smp_cache_bytes used for, exactly?

If you're just trying to get cacheline alignment, then this is
correct.

However, if you're using this to control loops which step over each
cacheline to invalidate / flush / whatever, then you'd need the
minimum cacheline size instead.

Of course, they'll all be the same in practice on any current machine.

> +}
> +
> +static void cpu_init(void)
> +{
> +	unsigned max_linesz = 0;
> +	int ret;
> +
> +	nr_cpus = 0;
> +	ret = dt_for_each_cpu_node(cpu_set, &max_linesz);
> +	assert(ret == 0);
> +	__smp_cache_bytes = max_linesz;
> +}
> +
> +static void mem_init(phys_addr_t 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));
> +
> +	__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(SMP_CACHE_BYTES);
> +}
> +
> +void setup(const void *fdt)
> +{
> +	const char *bootargs;
> +	u32 fdt_size;
> +	int ret;
> +
> +	/*
> +	 * Move the fdt to just above the stack. The free memory
> +	 * then starts just after the fdt.
> +	 */
> +	fdt_size = fdt_totalsize(fdt);
> +	ret = fdt_move(fdt, &stacktop, fdt_size);
> +	assert(ret == 0);
> +	ret = dt_init(&stacktop);
> +	assert(ret == 0);
> +
> +	cpu_init();
> +	mem_init(PAGE_ALIGN((unsigned long)&stacktop + fdt_size));
> +	io_init();
> +
> +	ret = dt_get_bootargs(&bootargs);
> +	assert(ret == 0);
> +	setup_args(bootargs);
> +}
> diff --git a/lib/ppc64/asm/setup.h b/lib/ppc64/asm/setup.h
> new file mode 100644
> index 0000000000000..20192985928a4
> --- /dev/null
> +++ b/lib/ppc64/asm/setup.h
> @@ -0,0 +1 @@
> +#include "../../powerpc/asm/setup.h"
> diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
> index b21e3933d0643..539bd33d1c309 100644
> --- a/powerpc/Makefile.common
> +++ b/powerpc/Makefile.common
> @@ -26,6 +26,7 @@ cflatobjs += lib/alloc.o
>  cflatobjs += lib/devicetree.o
>  cflatobjs += lib/powerpc/io.o
>  cflatobjs += lib/powerpc/hcall.o
> +cflatobjs += lib/powerpc/setup.o
>  
>  libgcc := $(shell $(CC) $(machine) --print-libgcc-file-name)
>  
> diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S
> index 1884d79871ba5..526452835754f 100644
> --- a/powerpc/cstart64.S
> +++ b/powerpc/cstart64.S
> @@ -20,11 +20,17 @@
>  
>  .section .init
>  
> +/*
> + * start is the entry point. r3 points to the DTB
> + */
>  .globl start
>  start:
>  	LOAD_REG_IMMEDIATE(r1, stackptr)
>  	LOAD_REG_IMMEDIATE(r2, tocptr)
>  
> +	/* save DTB pointer */
> +	std	r3, 56(r1)
> +
>  	/* patch sc1 if needed */
>  	bl	hcall_have_broken_sc1
>  	cmpwi	r3, 0
> @@ -33,7 +39,15 @@ start:
>  	LOAD_REG_IMMEDIATE(r4, SC1_REPLACEMENT)
>  	stw	r4, 0(r3)
>  
> -1:	bl	main
> +	/* complete setup */
> +1:	ld	r3, 56(r1)
> +	bl	setup
> +
> +	/* run the test */
> +	LOAD_REG_IMMEDIATE(r5, __argc)
> +	LOAD_REG_IMMEDIATE(r4, __argv)
> +	lwz	r3, 0(r5)
> +	bl	main
>  	bl	exit
>  	b	halt
>  
> diff --git a/powerpc/selftest.c b/powerpc/selftest.c
> index 2f2a5215dd55c..84867e482d2a2 100644
> --- a/powerpc/selftest.c
> +++ b/powerpc/selftest.c
> @@ -1,7 +1,64 @@
> +/*
> + * Test the framework itself. These tests confirm that setup works.
> + *
> + * Copyright (C) 2016, Red Hat Inc, Andrew Jones <drjones@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.
> + */
>  #include <libcflat.h>
> +#include <util.h>
> +#include <asm/setup.h>
>  
> -int main(void)
> +static void check_setup(int argc, char **argv)
>  {
> -	printf("hello world\n");
> -	return 0;
> +	int nr_tests = 0, len, i;
> +	long val;
> +
> +	for (i = 0; i < argc; ++i) {
> +
> +		len = parse_keyval(argv[i], &val);
> +		if (len == -1)
> +			continue;
> +
> +		argv[i][len] = '\0';
> +		report_prefix_push(argv[i]);
> +
> +		if (strcmp(argv[i], "mem") == 0) {
> +
> +			phys_addr_t memsize = PHYSICAL_END - PHYSICAL_START;
> +			phys_addr_t expected = ((phys_addr_t)val)*1024*1024;
> +
> +			report("size = %d MB", memsize == expected,
> +							memsize/1024/1024);
> +			++nr_tests;
> +
> +		} else if (strcmp(argv[i], "smp") == 0) {
> +
> +			report("nr_cpus = %d", nr_cpus == (int)val, nr_cpus);
> +			++nr_tests;
> +		}
> +
> +		report_prefix_pop();
> +	}
> +
> +	if (nr_tests < 2)
> +		report_abort("missing input");
> +}
> +
> +int main(int argc, char **argv)
> +{
> +	report_prefix_push("selftest");
> +
> +	if (!argc)
> +		report_abort("no test specified");
> +
> +	report_prefix_push(argv[0]);
> +
> +	if (strcmp(argv[0], "setup") == 0) {
> +
> +		check_setup(argc-1, &argv[1]);
> +
> +	}
> +
> +	return report_summary();
>  }

-- 
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 --]