Re: [kvm-unit-tests PATCH V2 2/4] lib/powerpc: Add generic decrementer exception handler

[Thomas Huth <thuth@redhat.com>]

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));

> +}
> 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>