From mboxrd@z Thu Jan  1 00:00:00 1970
From: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
Subject: Re: [kvm-unit-tests PATCH V2 2/4] lib/powerpc: Add generic
 decrementer exception handler
Date: Fri, 12 Aug 2016 16:17:54 +1000
Message-ID: <1470982674.4695.11.camel@gmail.com>
References: <1470794377-14427-1-git-send-email-sjitindarsingh@gmail.com>
	 <1470794377-14427-2-git-send-email-sjitindarsingh@gmail.com>
	 <3b34c69f-3d13-8e77-5e9c-896d9c55179f@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8bit
Cc: pbonzini@redhat.com, rkrcmar@redhat.com, kvm-ppc@vger.kernel.org,
	lvivier@redhat.com, drjones@redhat.com
To: Thomas Huth <thuth@redhat.com>, kvm@vger.kernel.org
Return-path: <kvm-owner@vger.kernel.org>
Received: from mail-pf0-f196.google.com ([209.85.192.196]:33677 "EHLO
	mail-pf0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1751539AbcHLGSI (ORCPT <rfc822;kvm@vger.kernel.org>);
	Fri, 12 Aug 2016 02:18:08 -0400
In-Reply-To: <3b34c69f-3d13-8e77-5e9c-896d9c55179f@redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Wed, 2016-08-10 at 12:38 +0200, Thomas Huth wrote:
> On 10.08.2016 03:59, Suraj Jitindar Singh wrote:
> > 
> > Add the lib/powerpc/handlers.c file and associated header files as
> > a place
> > to implement generic exception handler functions for inclusion in
> > tests.
> > 
> > Add a generic exception handler for a decrementer (0x900) interrupt
> > which
> > will reset the decrementer to its maximum value.
> > 
> > Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> > ---
> >  lib/powerpc/asm/handlers.h |  8 ++++++++
> >  lib/powerpc/handlers.c     | 25 +++++++++++++++++++++++++
> >  lib/ppc64/asm/handlers.h   |  1 +
> >  powerpc/Makefile.common    |  1 +
> >  4 files changed, 35 insertions(+)
> >  create mode 100644 lib/powerpc/asm/handlers.h
> >  create mode 100644 lib/powerpc/handlers.c
> >  create mode 100644 lib/ppc64/asm/handlers.h
> > 
> > diff --git a/lib/powerpc/asm/handlers.h
> > b/lib/powerpc/asm/handlers.h
> > new file mode 100644
> > index 0000000..64ba727
> > --- /dev/null
> > +++ b/lib/powerpc/asm/handlers.h
> > @@ -0,0 +1,8 @@
> > +#ifndef _ASMPOWERPC_HANDLERS_H_
> > +#define _ASMPOWERPC_HANDLERS_H_
> > +
> > +#include <asm/ptrace.h>
> > +
> > +void dec_except_handler(struct pt_regs *regs, void *data);
> > +
> > +#endif /* _ASMPOWERPC_HANDLERS_H_ */
> > diff --git a/lib/powerpc/handlers.c b/lib/powerpc/handlers.c
> > new file mode 100644
> > index 0000000..6a621f1
> > --- /dev/null
> > +++ b/lib/powerpc/handlers.c
> > @@ -0,0 +1,25 @@
> > +/*
> > + * Generic exception handlers for registration and use in tests
> > + *
> > + * Copyright 2016 Suraj Jitindar Singh, IBM.
> > + *
> > + * This work is licensed under the terms of the GNU LGPL, version
> > 2.
> > + */
> > +
> > +#include <libcflat.h>
> > +#include <asm/handlers.h>
> > +#include <asm/ptrace.h>
> > +
> > +/*
> > + * Generic handler for decrementer exceptions (0x900)
> > + * Just reset the decrementer back to its maximum value
> > (0x7FFFFFFF)
> > + */
> > +void dec_except_handler(struct pt_regs *regs __unused, void *data
> > __unused)
> > +{
> > +	uint32_t dec = 0x7FFFFFFF;
> > +
> > +	asm volatile ( "mtdec %0"	:
> > +					: "r" (dec)
> > +					:
> > +		     );
> I think most "asm volatile" statements are written in a more
> condensed
> way in kvm-unit-tests, so it might be more consistent to do that
> here,
> too instead:
> 
> 	asm volatile("mtdec %0"	: : "r" (dec));
> 
Will make whitespace changes to make this look more like existing code.
> > 
> > +}
> > diff --git a/lib/ppc64/asm/handlers.h b/lib/ppc64/asm/handlers.h
> > new file mode 100644
> > index 0000000..92e6fb2
> > --- /dev/null
> > +++ b/lib/ppc64/asm/handlers.h
> > @@ -0,0 +1 @@
> > +#include "../../powerpc/asm/handlers.h"
> > diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
> > index 3f8887d..404194b 100644
> > --- a/powerpc/Makefile.common
> > +++ b/powerpc/Makefile.common
> > @@ -36,6 +36,7 @@ cflatobjs += lib/powerpc/hcall.o
> >  cflatobjs += lib/powerpc/setup.o
> >  cflatobjs += lib/powerpc/rtas.o
> >  cflatobjs += lib/powerpc/processor.o
> > +cflatobjs += lib/powerpc/handlers.o
> >  
> >  FLATLIBS = $(libcflat) $(LIBFDT_archive)
> >  %.elf: CFLAGS += $(arch_CFLAGS)
> Apart from the above cosmetic nit, the patch looks fine to me.
> 
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> 
Thanks for looking at this