s390x: program interrupt handler
v2 cover s390x: program interrupt handler
Add a program interrupt handler and two simple program interrupt tests.
The check for the ADRESSING exception will fail on
- TCG without "target/s390x: addressing exceptions are suppressing"
- Old KVM versions without 492d8642eaef ("KVM: s390: Forward PSW to next instruction for addressing exceptions")
v1 -> v2: (s390x: generate asm offsets for the lowcore)
- Added missing field (mcck extended save area)
- Dropped alignment attribute for naturally aligned struct
v1 -> v2: (pgm interrupt handler and a way to test them)
- Renamed everything from irq->int/interrupt to keep it consistent
- Use global lowcore variable struct lowcore *lc;
- Safe+restore fpr and fpc in interrupt handler
- Setup pgm int handler using mvc instruction
- Renamed received_pgm_irq to check_pgm_int_code() and moved "report" into "check_pgm_int_code()"
- Minor style + comment fixes
David Hildenbrand (2):
s390x: generate asm offsets for the lowcore
s390x: pgm interrupt handler and a way to test them
lib/s390x/asm-offsets.c | 74 ++++++++++++++++++++++ lib/s390x/asm/arch_def.h | 146 ++++++++++++++++++++++++++++++++++++++++++++ lib/s390x/asm/asm-offsets.h | 10 +++ lib/s390x/asm/interrupt.h | 18 ++++++ lib/s390x/interrupt.c | 82 +++++++++++++++++++++++++ s390x/Makefile | 10 ++- s390x/cstart64.S | 55 +++++++++++++++++ s390x/selftest.c | 13 ++++ 8 files changed, 407 insertions(+), 1 deletion(-) create mode 100644 lib/s390x/asm-offsets.c create mode 100644 lib/s390x/asm/arch_def.h create mode 100644 lib/s390x/asm/asm-offsets.h create mode 100644 lib/s390x/asm/interrupt.h create mode 100644 lib/s390x/interrupt.c
Signature · 1 line
2.9.3
PATCH v2 1/2 s390x: generate asm offsets for the lowcore
Add the s390x psw and lowcore structs and generate asm offsets for lowcore fields. Add all lowcore fields defined in the current PoP (z13).
--- lib/s390x/asm-offsets.c | 71 ++++++++++++++++++++++++++++++++++++ lib/s390x/asm/arch_def.h | 87 +++++++++++++++++++++++++++++++++++++++++++++ lib/s390x/asm/asm-offsets.h | 10 ++++++ s390x/Makefile | 8 ++++- 4 files changed, 175 insertions(+), 1 deletion(-) create mode 100644 lib/s390x/asm-offsets.c create mode 100644 lib/s390x/asm/arch_def.h create mode 100644 lib/s390x/asm/asm-offsets.h
lib/s390x/asm-offsets.c new file +71 -0
@@ -0,0 +1,71 @@ +/* +* Copyright (c) 2017 Red Hat Inc +* +* Authors: +* David Hildenbrand <[email protected]> +* +* This code is free software; you can redistribute it and/or modify it +* under the terms of the GNU Library General Public License version 2. +*/ +#include <libcflat.h> +#include <kbuild.h> +#include <asm/arch_def.h> + +int main (void ) +{ +OFFSET (GEN_LC_EXT_INT_PARAM , lowcore, ext_int_param); +OFFSET (GEN_LC_CPU_ADDR , lowcore, cpu_addr); +OFFSET (GEN_LC_EXT_INT_CODE , lowcore, ext_int_code); +OFFSET (GEN_LC_SVC_INT_ID , lowcore, svc_int_id); +OFFSET (GEN_LC_SVC_INT_CODE , lowcore, svc_int_code); +OFFSET (GEN_LC_PGM_INT_ID , lowcore, pgm_int_id); +OFFSET (GEN_LC_PGM_INT_CODE , lowcore, pgm_int_code); +OFFSET (GEN_LC_DXC_VXC , lowcore, dxc_vxc); +OFFSET (GEN_LC_MON_CLASS_NB , lowcore, mon_class_nb); +OFFSET (GEN_LC_PER_CODE , lowcore, per_code); +OFFSET (GEN_LC_PER_ATMID , lowcore, per_atmid); +OFFSET (GEN_LC_PER_ADDR , lowcore, per_addr); +OFFSET (GEN_LC_EXC_ACC_ID , lowcore, exc_acc_id); +OFFSET (GEN_LC_PER_ACC_ID , lowcore, per_acc_id); +OFFSET (GEN_LC_OP_ACC_ID , lowcore, op_acc_id); +OFFSET (GEN_LC_ARCH_MODE_ID , lowcore, arch_mode_id); +OFFSET (GEN_LC_TRANS_EXC_ID , lowcore, trans_exc_id); +OFFSET (GEN_LC_MON_CODE , lowcore, mon_code); +OFFSET (GEN_LC_SUBSYS_ID_WORD , lowcore, subsys_id_word); +OFFSET (GEN_LC_IO_INT_PARAM , lowcore, io_int_param); +OFFSET (GEN_LC_IO_INT_WORD , lowcore, io_int_word); +OFFSET (GEN_LC_STFL , lowcore, stfl); +OFFSET (GEN_LC_MCCK_INT_CODE , lowcore, mcck_int_code); +OFFSET (GEN_LC_EXT_DAMAGE_CODE , lowcore, ext_damage_code); +OFFSET (GEN_LC_FAILING_STORAGE_ADDR , lowcore, failing_storage_addr); +OFFSET (GEN_LC_EMON_CA_ORIGIN , lowcore, emon_ca_origin); +OFFSET (GEN_LC_EMON_CA_SIZE , lowcore, emon_ca_size); +OFFSET (GEN_LC_EMON_EXC_COUNT , lowcore, emon_exc_count); +OFFSET (GEN_LC_BREAKING_EVENT_ADDR , lowcore, breaking_event_addr); +OFFSET (GEN_LC_RESTART_OLD_PSW , lowcore, restart_old_psw); +OFFSET (GEN_LC_EXT_OLD_PSW , lowcore, ext_old_psw); +OFFSET (GEN_LC_SVC_OLD_PSW , lowcore, svc_old_psw); +OFFSET (GEN_LC_PGM_OLD_PSW , lowcore, pgm_old_psw); +OFFSET (GEN_LC_MCCK_OLD_PSW , lowcore, mcck_old_psw); +OFFSET (GEN_LC_IO_OLD_PSW , lowcore, io_old_psw); +OFFSET (GEN_LC_RESTART_NEW_PSW , lowcore, restart_new_psw); +OFFSET (GEN_LC_EXT_NEW_PSW , lowcore, ext_new_psw); +OFFSET (GEN_LC_SVC_NEW_PSW , lowcore, svc_new_psw); +OFFSET (GEN_LC_PGM_NEW_PSW , lowcore, pgm_new_psw); +OFFSET (GEN_LC_MCCK_NEW_PSW , lowcore, mcck_new_psw); +OFFSET (GEN_LC_IO_NEW_PSW , lowcore, io_new_psw); +OFFSET (GEN_LC_MCCK_EXT_SA_ADDR , lowcore, mcck_ext_sa_addr); +OFFSET (GEN_LC_FPRS_SA , lowcore, fprs_sa); +OFFSET (GEN_LC_GRS_SA , lowcore, grs_sa); +OFFSET (GEN_LC_PSW_SA , lowcore, psw_sa); +OFFSET (GEN_LC_PREFIX_SA , lowcore, prefix_sa); +OFFSET (GEN_LC_FPC_SA , lowcore, fpc_sa); +OFFSET (GEN_LC_TOD_PR_SA , lowcore, tod_pr_sa); +OFFSET (GEN_LC_CPUTM_SA , lowcore, cputm_sa); +OFFSET (GEN_LC_CC_SA , lowcore, cc_sa); +OFFSET (GEN_LC_ARS_SA , lowcore, ars_sa); +OFFSET (GEN_LC_CRS_SA , lowcore, crs_sa); +OFFSET (GEN_LC_PGM_INT_TDB , lowcore, pgm_int_tdb); + +return 0 ; +}
lib/s390x/asm/arch_def.h new file +87 -0
87 lines not on this page · raw message
lib/s390x/asm/asm-offsets.h new file +10 -0
@@ -0,0 +1,10 @@ +/* +* Copyright (c) 2017 Red Hat Inc +* +* Authors: +* David Hildenbrand <[email protected]> +* +* This code is free software; you can redistribute it and/or modify it +* under the terms of the GNU Library General Public License version 2. +*/ +#include <generated/asm-offsets.h>
s390x/Makefile +7 -1
@@ -15,6 +15,9 @@ LDFLAGS += -nostdlib # We want to keep intermediate files .PRECIOUS: %.o +asm-offsets = lib/$(ARCH)/asm-offsets.h +include $(SRCDIR)/scripts/asm-offsets.mak + cflatobjs += lib/util.o cflatobjs += lib/alloc.o cflatobjs += lib/s390x/io.o
@@ -33,5 +36,8 @@ FLATLIBS = $(libcflat) $(filter %.o, $^) $(FLATLIBS) $(@:.elf=.aux.o) $(RM) $(@:.elf=.aux.o) -arch_clean: +arch_clean: asm_offsets_clean $(RM) $(TEST_DIR)/*.{o,elf} $(TEST_DIR)/.*.d lib/s390x/.*.d + +generated-files = $(asm-offsets) +$(test_cases:.elf=.o) $(cstart.o) $(cflatobjs): $(generated-files)
Signature · 1 line
2.9.3
PATCH v2 2/2 s390x: pgm interrupt handler and a way to test them
In reply to David HildenbrandThe program interrupt handler will detect unexpected program interrupts and allow to expect + verify program interrupts for testing purposes.
We need "-fno-delete-null-pointer-checks", otherwise trying to access the lowcore at address 0 makes GCC generate very weird code.
Add two tests to test for simple operation and addressing exceptions.
--- lib/s390x/asm-offsets.c | 3 ++ lib/s390x/asm/arch_def.h | 61 ++++++++++++++++++++++++++++++++++- lib/s390x/asm/interrupt.h | 18 +++++++++++ lib/s390x/interrupt.c | 82 +++++++++++++++++++++++++++++++++++++++++++++++ s390x/Makefile | 2 ++ s390x/cstart64.S | 55 +++++++++++++++++++++++++++++++ s390x/selftest.c | 13 ++++++++ 7 files changed, 233 insertions(+), 1 deletion(-) create mode 100644 lib/s390x/asm/interrupt.h create mode 100644 lib/s390x/interrupt.c
lib/s390x/asm-offsets.c +3 -0
@@ -54,6 +54,9 @@ int main(void)OFFSET (GEN_LC_PGM_NEW_PSW , lowcore, pgm_new_psw);OFFSET (GEN_LC_MCCK_NEW_PSW , lowcore, mcck_new_psw);OFFSET (GEN_LC_IO_NEW_PSW , lowcore, io_new_psw); +OFFSET (GEN_LC_SW_INT_GRS , lowcore, sw_int_grs); +OFFSET (GEN_LC_SW_INT_FPRS , lowcore, sw_int_fprs); +OFFSET (GEN_LC_SW_INT_FPC , lowcore, sw_int_fpc);OFFSET (GEN_LC_MCCK_EXT_SA_ADDR , lowcore, mcck_ext_sa_addr);OFFSET (GEN_LC_FPRS_SA , lowcore, fprs_sa);OFFSET (GEN_LC_GRS_SA , lowcore, grs_sa);
lib/s390x/asm/arch_def.h +60 -1
71 lines not on this page · raw message
lib/s390x/asm/interrupt.h new file +18 -0
@@ -0,0 +1,18 @@ +/* +* Copyright (c) 2017 Red Hat Inc +* +* Authors: +* David Hildenbrand <[email protected]> +* +* This code is free software; you can redistribute it and/or modify it +* under the terms of the GNU Library General Public License version 2. +*/ +#ifndef _ASMS390X_IRQ_H_ +#define _ASMS390X_IRQ_H_ +#include <asm/arch_def.h> + +void handle_pgm_int (void ); +void expect_pgm_int (void ); +void check_pgm_int_code (uint16_t code); + +#endif
lib/s390x/interrupt.c new file +82 -0
82 lines not on this page · raw message
s390x/Makefile +2 -0
@@ -10,6 +10,7 @@ CFLAGS += -Wextra CFLAGS += -I $(SRCDIR)/lib CFLAGS += -O2 CFLAGS += -march=z900 +CFLAGS += -fno-delete-null-pointer-checks LDFLAGS += -nostdlib # We want to keep intermediate files
@@ -23,6 +24,7 @@ cflatobjs += lib/alloc.o cflatobjs += lib/s390x/io.o cflatobjs += lib/s390x/stack.o cflatobjs += lib/s390x/sclp-ascii.o +cflatobjs += lib/s390x/interrupt.o OBJDIRS += lib/s390x
s390x/cstart64.S +55 -0
76 lines not on this page · raw message
s390x/selftest.c +13 -0
@@ -10,6 +10,7 @@*/ #include <libcflat.h> #include <util.h> +#include <asm/interrupt.h> static void test_fp (void ) {
@@ -25,6 +26,17 @@ static void test_fp(void)report ("3.0/2.0 == 1.5" , c ==1.5 ); } +static void test_pgm_int (void ) +{ +expect_pgm_int (); + asmvolatile (" .insn e,0x0001" );/* used for SW breakpoints in QEMU */ +check_pgm_int_code (PGM_INT_CODE_OPERATION ); + +expect_pgm_int (); + *((unsigned int *)-1 ) =1 ; +check_pgm_int_code (PGM_INT_CODE_ADDRESSING ); +} +int main (int argc,char **argv) {report_prefix_push ("selftest" );
@@ -36,6 +48,7 @@ int main(int argc, char**argv)report ("argv[2] == 123" , !strcmp (argv[2 ],"123" ));test_fp (); +test_pgm_int ();return report_summary (); }
Signature · 1 line
2.9.3
Paolo Bonzini
maintainerRe: [kvm-unit-tests PATCH v2 2/2] s390x: pgm interrupt handler and a way to test them
s390x/selftest.c static void test_fp(void) · line 37
Quoted 3 lines · David Hildenbrand
s390x/selftest.c +3
-0+expect_pgm_int (); + *((unsigned int *)-1 ) =1 ; +check_pgm_int_code (PGM_INT_CODE_ADDRESSING );
QEMU gets
ABORT: selftest: Unexpected program interrupt: 5 at 0x12b4c, ilen 4
It seems that QEMU doesn't point the PSW at the next instruction:
IN: main 0x0000000000012b44: lghi %r1,-1 0x0000000000012b48: lhi %r3,1 0x0000000000012b4c: st %r3,0(%r1) 0x0000000000012b50: lghi %r2,5 0x0000000000012b54: brasl %r14,0x128e8 0x0000000000012b4c: st %r3,0(%r1) 0x0000000000012b50: lghi %r2,5 0x0000000000012b54: brasl %r14,0x128e8 # this is expect_pgm_int Trace 0x7f5ff6c7f520 [0: 0000000000012b44] main Trace 0x7f5ff6c7bc20 [0: 000000000001004e] Trace 0x7f5ff6c7c960 [0: 0000000000012918] handle_pgm_int Trace 0x7f5ff6c7d280 [0: 00000000000100a4]
and now it gets another program interrupt:
IN: main 0x0000000000012b4c: st %r3,0(%r1) 0x0000000000012b50: lghi %r2,5 0x0000000000012b54: brasl %r14,0x128e8 Trace 0x7f5ff6c7f600 [0: 0000000000012b4c] main Trace 0x7f5ff6c7bc20 [0: 000000000001004e] Trace 0x7f5ff6c7c960 [0: 0000000000012918] handle_pgm_int
Thanks,
Paolo
Re: [kvm-unit-tests PATCH v2 2/2] s390x: pgm interrupt handler and a way to test them
Quoted 9 lines · Paolo Bonzini
s390x/selftest.c static void test_fp(void) · line 37
Quoted 3 lines · David Hildenbrand
s390x/selftest.c +3
-0+expect_pgm_int (); + *((unsigned int *)-1 ) =1 ; +check_pgm_int_code (PGM_INT_CODE_ADDRESSING );QEMU gets
Quoted 3 lines
ABORT: selftest: Unexpected program interrupt: 5 at
0x12b4c, ilen 4It seems that QEMU doesn't point the PSW at the next instruction
David mentioned it in the cover letter ... you likely need this patch here first:
http://patchwork.ozlabs.org/patch/768173/
Thomas
Paolo Bonzini
maintainerRe: [kvm-unit-tests PATCH v2 2/2] s390x: pgm interrupt handler and a way to test them
Quoted 12 lines · Thomas Huth
Quoted 11 lines · Paolo Bonzini
s390x/selftest.c static void test_fp(void) · line 37
Quoted 3 lines · David Hildenbrand
s390x/selftest.c +3
-0+expect_pgm_int (); + *((unsigned int *)-1 ) =1 ; +check_pgm_int_code (PGM_INT_CODE_ADDRESSING );QEMU gets
ABORT: selftest: Unexpected program interrupt: 5 at
0x12b4c, ilen 4
Quoted 9 lines
Quoted 1 line
It seems that QEMU doesn't point the PSW at the next instruction
David mentioned it in the cover letter ... you likely need this patch here first:
http://patchwork.ozlabs.org/patch/768173/
Thomas
That helps indeed. :)
Paolo
Re: [kvm-unit-tests PATCH v2 2/2] s390x: pgm interrupt handler and a way to test them
In reply to Paolo BonziniQuoted 28 lines · Paolo Bonzini
s390x/selftest.c static void test_fp(void) · line 37
Quoted 3 lines · David Hildenbrand
s390x/selftest.c +3
-0+expect_pgm_int (); + *((unsigned int *)-1 ) =1 ; +check_pgm_int_code (PGM_INT_CODE_ADDRESSING );QEMU gets
ABORT: selftest: Unexpected program interrupt: 5 at
0x12b4c, ilen 4It seems that QEMU doesn't point the PSW at the next instruction:
IN: main 0x0000000000012b44: lghi %r1,-1 0x0000000000012b48: lhi %r3,1 0x0000000000012b4c: st %r3,0(%r1) 0x0000000000012b50: lghi %r2,5 0x0000000000012b54: brasl %r14,0x128e8 0x0000000000012b4c: st %r3,0(%r1) 0x0000000000012b50: lghi %r2,5 0x0000000000012b54: brasl %r14,0x128e8 # this is expect_pgm_int Trace 0x7f5ff6c7f520 [0: 0000000000012b44] main Trace 0x7f5ff6c7bc20 [0: 000000000001004e] Trace 0x7f5ff6c7c960 [0: 0000000000012918] handle_pgm_int Trace 0x7f5ff6c7d280 [0: 00000000000100a4]
Quoted 1 line
and now it gets another program interrupt:
Exactly, the emulator doesn't properly forward the PSW.
Quoted 10 lines
IN: main 0x0000000000012b4c: st %r3,0(%r1) 0x0000000000012b50: lghi %r2,5 0x0000000000012b54: brasl %r14,0x128e8 Trace 0x7f5ff6c7f600 [0: 0000000000012b4c] main Trace 0x7f5ff6c7bc20 [0: 000000000001004e] Trace 0x7f5ff6c7c960 [0: 0000000000012918] handle_pgm_int
Quoted 4 lines
Thanks,
Paolo
Yes, known problem as described in the cover letter. Happens with current TCG and old KVM. Patch for TCG is on the list.
Thanks for testing!
Signature · 3 lines
Thanks, David
Re: [kvm-unit-tests PATCH v2 2/2] s390x: pgm interrupt handler and a way to test them
In reply to David HildenbrandQuoted 10 lines · David Hildenbrand
The program interrupt handler will detect unexpected program interrupts and allow to expect + verify program interrupts for testing purposes.
We need "-fno-delete-null-pointer-checks", otherwise trying to access the lowcore at address 0 makes GCC generate very weird code.
Add two tests to test for simple operation and addressing exceptions.
Signed-off-by David Hildenbrand <[email protected]>---
[...]
Quoted 13 lines
lib/s390x/interrupt.c new file +7
-0@@ -0,0 +1,82 @@ +/* +* s390x interrupt handling +* +* Copyright (c) 2017 Red Hat Inc +* +* Authors: +* David Hildenbrand <[email protected]>
lib/s390x/interrupt.c line 17
Quoted 10 lines
lib/s390x/interrupt.c new file +10
-0+* +* This code is free software; you can redistribute it and/or modify it +* under the terms of the GNU Library General Public License version 2. +*/ +#include <libcflat.h> +#include <asm/interrupt.h> +#include <asm/barrier.h> + +static bool pgm_int_expected; +static struct lowcore *lc;
Cosmetic suggestion:
Maybe make it "static struct lowcore const *lc = 0" to be more explicit?
Apart from that, patch looks fine to me now, so with or without that change:
Re: [kvm-unit-tests PATCH v2 2/2] s390x: pgm interrupt handler and a way to test them
Quoted 30 lines · Thomas Huth
Quoted 10 lines · David Hildenbrand
The program interrupt handler will detect unexpected program interrupts and allow to expect + verify program interrupts for testing purposes.
We need "-fno-delete-null-pointer-checks", otherwise trying to access the lowcore at address 0 makes GCC generate very weird code.
Add two tests to test for simple operation and addressing exceptions.
Signed-off-by David Hildenbrand <[email protected]>---
[...]
Quoted 18 lines
lib/s390x/interrupt.c new file +12
-0@@ -0,0 +1,82 @@ +/* +* s390x interrupt handling +* +* Copyright (c) 2017 Red Hat Inc +* +* Authors: +* David Hildenbrand <[email protected]> +* +* This code is free software; you can redistribute it and/or modify it +* under the terms of the GNU Library General Public License version 2. +*/ +#include <libcflat.h>
Quoted 8 lines
lib/s390x/interrupt.c line 17
Quoted 5 lines
lib/s390x/interrupt.c new file +5
-0+#include <asm/interrupt.h> +#include <asm/barrier.h> + +static bool pgm_int_expected; +static struct lowcore *lc;Cosmetic suggestion:
Maybe make it "static struct lowcore const *lc = 0" to be more explicit?
You most probably mean
static struct lowcore *const lc = 0;
(otherwise it won't compile)
makes sense. If I have to resend, I'll add it.
Quoted 5 lines
Apart from that, patch looks fine to me now, so with or without that change:
Reviewed-by Thomas Huth <[email protected]>
Thanks!
Signature · 3 lines
Thanks, David