From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Vivier Date: Fri, 18 Mar 2016 10:08:01 +0000 Subject: Re: [kvm-unit-tests PATCH 2/5] powerpc: add test to check invalid instruction trap Message-Id: <56EBD381.3040109@redhat.com> List-Id: References: <1458141183-27207-1-git-send-email-lvivier@redhat.com> <1458141183-27207-3-git-send-email-lvivier@redhat.com> <56EBCF85.3030707@redhat.com> In-Reply-To: <56EBCF85.3030707@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Thomas Huth , kvm@vger.kernel.org, kvm-ppc@vger.kernel.org Cc: drjones@redhat.com, dgibson@redhat.com, pbonzini@redhat.com On 18/03/2016 10:51, Thomas Huth wrote: > On 16.03.2016 16:13, Laurent Vivier wrote: >> Signed-off-by: Laurent Vivier >> --- >> 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 >> +#include >> + >> +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. > >> + *data = 1; >> + >> + regs->nip += 4; >> +} >> + >> +static void test_illegal(void) >> +{ >> + report_prefix_push("invalid"); >> + >> + is_invalid = 0; >> + >> + asm volatile (".long 0"); >> + >> + report("exception", is_invalid); >> + >> + report_prefix_pop(); >> +} >> + >> +int main(void) >> +{ >> + handle_exception(0x700, program_check_handler, (void *)&is_invalid); >> + >> + report_prefix_push("emulator"); >> + >> + test_illegal(); >> + >> + report_prefix_pop(); >> + >> + return report_summary(); >> +} >> diff --git a/powerpc/unittests.cfg b/powerpc/unittests.cfg >> index 02b21c7..ed4fdbe 100644 >> --- a/powerpc/unittests.cfg >> +++ b/powerpc/unittests.cfg >> @@ -47,3 +47,6 @@ file = rtas.elf >> extra_params = -append "set-time-of-day" >> timeout = 5 >> groups = rtas >> + >> +[emulator] >> +file = emulator.elf > > Reviewed-by: Thomas Huth > > BTW: I think you could optionally also check the contents of SRR1 for > sane values in test_illegal(), in case you want to improve the test a > little bit. OK, Thanks, Laurent From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Vivier Subject: Re: [kvm-unit-tests PATCH 2/5] powerpc: add test to check invalid instruction trap Date: Fri, 18 Mar 2016 11:08:01 +0100 Message-ID: <56EBD381.3040109@redhat.com> References: <1458141183-27207-1-git-send-email-lvivier@redhat.com> <1458141183-27207-3-git-send-email-lvivier@redhat.com> <56EBCF85.3030707@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: drjones@redhat.com, dgibson@redhat.com, pbonzini@redhat.com To: Thomas Huth , kvm@vger.kernel.org, kvm-ppc@vger.kernel.org Return-path: Received: from mx1.redhat.com ([209.132.183.28]:42369 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754917AbcCRKIF (ORCPT ); Fri, 18 Mar 2016 06:08:05 -0400 In-Reply-To: <56EBCF85.3030707@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 18/03/2016 10:51, Thomas Huth wrote: > On 16.03.2016 16:13, Laurent Vivier wrote: >> Signed-off-by: Laurent Vivier >> --- >> 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 >> +#include >> + >> +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. > >> + *data = 1; >> + >> + regs->nip += 4; >> +} >> + >> +static void test_illegal(void) >> +{ >> + report_prefix_push("invalid"); >> + >> + is_invalid = 0; >> + >> + asm volatile (".long 0"); >> + >> + report("exception", is_invalid); >> + >> + report_prefix_pop(); >> +} >> + >> +int main(void) >> +{ >> + handle_exception(0x700, program_check_handler, (void *)&is_invalid); >> + >> + report_prefix_push("emulator"); >> + >> + test_illegal(); >> + >> + report_prefix_pop(); >> + >> + return report_summary(); >> +} >> diff --git a/powerpc/unittests.cfg b/powerpc/unittests.cfg >> index 02b21c7..ed4fdbe 100644 >> --- a/powerpc/unittests.cfg >> +++ b/powerpc/unittests.cfg >> @@ -47,3 +47,6 @@ file = rtas.elf >> extra_params = -append "set-time-of-day" >> timeout = 5 >> groups = rtas >> + >> +[emulator] >> +file = emulator.elf > > Reviewed-by: Thomas Huth > > BTW: I think you could optionally also check the contents of SRR1 for > sane values in test_illegal(), in case you want to improve the test a > little bit. OK, Thanks, Laurent