From mboxrd@z Thu Jan  1 00:00:00 1970
From: Thomas Huth <thuth@redhat.com>
Subject: Re: [kvm-unit-tests PATCH v2 09/14] powerpc/ppc64: adapt arm's setup
Date: Fri, 12 Feb 2016 12:50:12 +0100
Message-ID: <56BDC6F4.4050601@redhat.com>
References: <1454957594-30601-1-git-send-email-drjones@redhat.com>
 <1454957594-30601-10-git-send-email-drjones@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
Cc: dgibson@redhat.com, david@gibson.dropbear.id.au, agraf@suse.de,
	lvivier@redhat.com, pbonzini@redhat.com
To: Andrew Jones <drjones@redhat.com>, kvm@vger.kernel.org,
	kvm-ppc@vger.kernel.org
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:46651 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1751746AbcBLLuR (ORCPT <rfc822;kvm@vger.kernel.org>);
	Fri, 12 Feb 2016 06:50:17 -0500
In-Reply-To: <1454957594-30601-10-git-send-email-drjones@redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

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