From mboxrd@z Thu Jan  1 00:00:00 1970
From: Thomas Huth <thuth@redhat.com>
Subject: Re: [kvm-unit-tests PATCH v3 16/17] powerpc/ppc64: add RTAS support
Date: Mon, 15 Feb 2016 10:59:31 +0100
Message-ID: <56C1A183.4000107@redhat.com>
References: <1455380910-30604-1-git-send-email-drjones@redhat.com>
 <1455380910-30604-17-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]:33844 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1752361AbcBOJ7h (ORCPT <rfc822;kvm@vger.kernel.org>);
	Mon, 15 Feb 2016 04:59:37 -0500
In-Reply-To: <1455380910-30604-17-git-send-email-drjones@redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On 13.02.2016 17:28, Andrew Jones wrote:
> Add enough RTAS support to start adding RTAS commands. Just add
> power-off for now.
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  lib/powerpc/asm/rtas.h  |  26 +++++++++
>  lib/powerpc/io.c        |   2 +
>  lib/powerpc/rtas.c      | 139 ++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/ppc64/asm/rtas.h    |   1 +
>  powerpc/Makefile.common |   1 +
>  5 files changed, 169 insertions(+)
>  create mode 100644 lib/powerpc/asm/rtas.h
>  create mode 100644 lib/powerpc/rtas.c
>  create mode 100644 lib/ppc64/asm/rtas.h
...
> diff --git a/lib/powerpc/rtas.c b/lib/powerpc/rtas.c
> new file mode 100644
> index 0000000000000..b6ae7391181f9
> --- /dev/null
> +++ b/lib/powerpc/rtas.c
> @@ -0,0 +1,139 @@
> +/*
> + * powerpc RTAS
> + *
> + * 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 <asm/spinlock.h>
> +#include <asm/hcall.h>
> +#include <asm/io.h>
> +#include <asm/rtas.h>
> +
> +#if defined(__powerpc64__)
> +struct func_ptr {
> +	unsigned long ptr;
> +	unsigned long toc;
> +};
> +static struct func_ptr rtas_entry_funcptr;
> +#endif
> +typedef void (*rtas_entry_t)(unsigned long);
> +static rtas_entry_t enter_rtas;
> +static struct rtas_args rtas_args;
> +static struct spinlock lock;

I'd maybe name the lock variable "rtas_lock" instead (in case we ever
use more locks in this file, it's then more obvious what this lock is
about) ... but apart from that, and the bug in the sc1 loop that you
already mentioned, the patch looks fine to me.

 Thomas