Re: [kvm-unit-tests PATCH v2 09/14] powerpc/ppc64: adapt arm's setup

[Thomas Huth <thuth@redhat.com>]

On 08.02.2016 19:53, 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.
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  lib/powerpc/asm/setup.h | 27 +++++++++++++++
>  lib/powerpc/setup.c     | 91 +++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/ppc64/asm/setup.h   |  1 +
>  powerpc/cstart64.S      |  9 +++++
>  powerpc/selftest.c      | 64 ++++++++++++++++++++++++++++++++--
>  5 files changed, 189 insertions(+), 3 deletions(-)
>  create mode 100644 lib/powerpc/asm/setup.h
>  create mode 100644 lib/ppc64/asm/setup.h
...
> diff --git a/powerpc/selftest.c b/powerpc/selftest.c
> index 2f2a5215dd55c..3dd7ce9945307 100644
> --- a/powerpc/selftest.c
> +++ b/powerpc/selftest.c
> @@ -1,7 +1,65 @@
> +/*
> + * 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 <alloc.h>		/* phys_addr_t */
> +#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;

Could you maybe add a check for nr_cpus <= NR_CPUS somewhere and print a
friendly warning message if it the amount of CPUs exceeds NR_CPUS?
Otherwise you only get a very cryptic assert() message if you
accidentially specify too many CPUs ... (it just happened to me, and it
took a while till I understood what was wrong).

> +		}
> +
> +		report_prefix_pop();
> +	}
> +
> +	if (nr_tests < 2)
> +		report_abort("missing input");
> +}
> +
> +int main(int argc, char **argv)
> +{
> +	report_prefix_push("selftest");
> +
> +	if (argc < 1)
> +		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();
>  }
> 

 Thomas