From mboxrd@z Thu Jan  1 00:00:00 1970
From: Thomas Huth <thuth@redhat.com>
Date: Fri, 18 Mar 2016 10:12:19 +0000
Subject: Re: [kvm-unit-tests PATCH 2/5] powerpc: add test to check invalid instruction trap
Message-Id: <56EBD483.3040404@redhat.com>
List-Id: <kvm-ppc.vger.kernel.org>
References: <1458141183-27207-1-git-send-email-lvivier@redhat.com>
 <1458141183-27207-3-git-send-email-lvivier@redhat.com>
 <56EBCF85.3030707@redhat.com> <56EBD381.3040109@redhat.com>
In-Reply-To: <56EBD381.3040109@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: Laurent Vivier <lvivier@redhat.com>, kvm@vger.kernel.org, kvm-ppc@vger.kernel.org
Cc: drjones@redhat.com, dgibson@redhat.com, pbonzini@redhat.com

On 18.03.2016 11:08, Laurent Vivier wrote:
> 
> 
> On 18/03/2016 10:51, Thomas Huth wrote:
>> On 16.03.2016 16:13, Laurent Vivier wrote:
>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>>> ---
>>>  powerpc/Makefile.common |  5 ++++-
>>>  powerpc/emulator.c      | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>>>  powerpc/unittests.cfg   |  3 +++
>>>  3 files changed, 53 insertions(+), 1 deletion(-)
>>>  create mode 100644 powerpc/emulator.c
>>>
>>> diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
>>> index ab2caf6..257e3fb 100644
>>> --- a/powerpc/Makefile.common
>>> +++ b/powerpc/Makefile.common
>>> @@ -7,7 +7,8 @@
>>>  tests-common = \
>>>  	$(TEST_DIR)/selftest.elf \
>>>  	$(TEST_DIR)/spapr_hcall.elf \
>>> -	$(TEST_DIR)/rtas.elf
>>> +	$(TEST_DIR)/rtas.elf \
>>> +	$(TEST_DIR)/emulator.elf
>>>  
>>>  all: $(TEST_DIR)/boot_rom.bin test_cases
>>>  
>>> @@ -70,3 +71,5 @@ $(TEST_DIR)/selftest.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/selftest.o
>>>  $(TEST_DIR)/spapr_hcall.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/spapr_hcall.o
>>>  
>>>  $(TEST_DIR)/rtas.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/rtas.o
>>> +
>>> +$(TEST_DIR)/emulator.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/emulator.o
>>> diff --git a/powerpc/emulator.c b/powerpc/emulator.c
>>> new file mode 100644
>>> index 0000000..1215c4f
>>> --- /dev/null
>>> +++ b/powerpc/emulator.c
>>> @@ -0,0 +1,46 @@
>>> +/*
>>> + * Test some powerpc instructions
>>> + */
>>> +
>>> +#include <libcflat.h>
>>> +#include <asm/processor.h>
>>> +
>>> +static int volatile is_invalid;
>>> +
>>> +static void program_check_handler(struct pt_regs *regs, void *opaque)
>>> +{
>>> +	int *data = opaque;
>>> +
>>> +	printf("Detected invalid instruction 0x%016lx: %08x\n",
>>> +	       regs->nip, *(uint32_t*)regs->nip);
>>
>> Should the test always print this, or is this rather confusing for the
>> users? Then it should maybe be commented out by default, I think.
> 
> In fact, it was really helpful to debug (for instance with kvm-pr), it's
> why I let it. But as I agree with you, I'll remove it.

You could also add a CLI option to this unit test, so that it e.g. only
printed when the test is started with a "-v" parameter or so?

 Thomas