On Fri, 20 Aug 2021 11:39:58 +0000
Janosch Frank <frankja@xxxxxxxxxxxxx> wrote:
> Right now we only get told the kind of program exception as well as
> the PSW at the point where it happened.
>
> For addressing exceptions the PSW is not always enough so let's print
> the TEID which contains the failing address and flags that tell us
> more about the kind of address exception.
>
> Signed-off-by: Janosch Frank <frankja@xxxxxxxxxxxxx>
Reviewed-by: Claudio Imbrenda <imbrenda@xxxxxxxxxxxxx>
(but see a small nit below)
> ---
> lib/s390x/asm/arch_def.h | 5 +++
> lib/s390x/fault.c | 76 ++++++++++++++++++++++++++++++++++++++++
> lib/s390x/fault.h | 44 +++++++++++++++++++++++
> lib/s390x/interrupt.c | 29 +++++++++++++--
> s390x/Makefile | 1 +
> 5 files changed, 153 insertions(+), 2 deletions(-)
> create mode 100644 lib/s390x/fault.c
> create mode 100644 lib/s390x/fault.h
>
> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
> index 302ef1ff..ab5a9043 100644
> --- a/lib/s390x/asm/arch_def.h
> +++ b/lib/s390x/asm/arch_def.h
> @@ -41,6 +41,11 @@ struct psw {
> uint64_t addr;
> };
>
> +#define AS_PRIM 0
> +#define AS_ACCR 1
> +#define AS_SECN 2
> +#define AS_HOME 3
> +
> #define PSW_MASK_EXT 0x0100000000000000UL
> #define PSW_MASK_IO 0x0200000000000000UL
> #define PSW_MASK_DAT 0x0400000000000000UL
> diff --git a/lib/s390x/fault.c b/lib/s390x/fault.c
> new file mode 100644
> index 00000000..fd1490e3
> --- /dev/null
> +++ b/lib/s390x/fault.c
> @@ -0,0 +1,76 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Library to decode addressing related exceptions
> + *
> + * Copyright 2021 IBM Corp.
> + *
> + * Authors:
> + * Janosch Frank <frankja@xxxxxxxxxxxxx>
> + */
> +#include <libcflat.h>
> +#include <bitops.h>
> +#include <asm/arch_def.h>
> +#include <asm/page.h>
> +#include <fault.h>
> +
> +static struct lowcore *lc = (struct lowcore *)0x0;
> +
> +/* Decodes the protection exceptions we'll most likely see */
> +static void decode_pgm_prot(uint64_t teid)
I think this is also a "print_decode_pgm_prot", but it's static
so I guess we don't really care too much
> +{
> + if (prot_is_lap(teid)) {
> + printf("Type: LAP\n");
> + return;
> + }
> +
> + if (prot_is_iep(teid)) {
> + printf("Type: IEP\n");
> + return;
> + }
> +
> + if (prot_is_datp(teid)) {
> + printf("Type: DAT\n");
> + return;
> + }
> +}
> +
> +void print_decode_teid(uint64_t teid)
> +{
> + int asce_id = teid & 3;
> + bool dat = lc->pgm_old_psw.mask & PSW_MASK_DAT;
> +
> + printf("Memory exception information:\n");
> + printf("DAT: %s\n", dat ? "on" : "off");
> +
> + printf("AS: ");
> + switch (asce_id) {
> + case AS_PRIM:
> + printf("Primary\n");
> + break;
> + case AS_ACCR:
> + printf("Access Register\n");
> + break;
> + case AS_SECN:
> + printf("Secondary\n");
> + break;
> + case AS_HOME:
> + printf("Home\n");
> + break;
> + }
> +
> + if (lc->pgm_int_code == PGM_INT_CODE_PROTECTION)
> + decode_pgm_prot(teid);
> +
> + /*
> + * If teid bit 61 is off for these two exception the reported
> + * address is unpredictable.
> + */
> + if ((lc->pgm_int_code == PGM_INT_CODE_SECURE_STOR_ACCESS ||
> + lc->pgm_int_code == PGM_INT_CODE_SECURE_STOR_VIOLATION) &&
> + !test_bit_inv(61, &teid)) {
> + printf("Address: %lx, unpredictable\n ", teid & PAGE_MASK);
> + return;
> + }
> + printf("TEID: %lx\n", teid);
> + printf("Address: %lx\n\n", teid & PAGE_MASK);
> +}
> diff --git a/lib/s390x/fault.h b/lib/s390x/fault.h
> new file mode 100644
> index 00000000..726da2f0
> --- /dev/null
> +++ b/lib/s390x/fault.h
> @@ -0,0 +1,44 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Headers for fault.c
> + *
> + * Copyright 2021 IBM Corp.
> + *
> + * Authors:
> + * Janosch Frank <frankja@xxxxxxxxxxxxx>
> + */
> +#ifndef _S390X_FAULT_H_
> +#define _S390X_FAULT_H_
> +
> +#include <bitops.h>
> +
> +/* Instruction execution prevention, i.e. no-execute, 101 */
> +static inline bool prot_is_iep(uint64_t teid)
> +{
> + if (test_bit_inv(56, &teid) && !test_bit_inv(60, &teid) && test_bit_inv(61, &teid))
> + return true;
> +
> + return false;
> +}
> +
> +/* Standard DAT exception, 001 */
> +static inline bool prot_is_datp(uint64_t teid)
> +{
> + if (!test_bit_inv(56, &teid) && !test_bit_inv(60, &teid) && test_bit_inv(61, &teid))
> + return true;
> +
> + return false;
> +}
> +
> +/* Low-address protection exception, 100 */
> +static inline bool prot_is_lap(uint64_t teid)
> +{
> + if (test_bit_inv(56, &teid) && !test_bit_inv(60, &teid) && !test_bit_inv(61, &teid))
> + return true;
> +
> + return false;
> +}
> +
> +void print_decode_teid(uint64_t teid);
> +
> +#endif /* _S390X_FAULT_H_ */
> diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
> index 01ded49d..721e6758 100644
> --- a/lib/s390x/interrupt.c
> +++ b/lib/s390x/interrupt.c
> @@ -12,6 +12,8 @@
> #include <sclp.h>
> #include <interrupt.h>
> #include <sie.h>
> +#include <fault.h>
> +#include <asm/page.h>
>
> static bool pgm_int_expected;
> static bool ext_int_expected;
> @@ -76,8 +78,7 @@ static void fixup_pgm_int(struct stack_frame_int *stack)
> break;
> case PGM_INT_CODE_PROTECTION:
> /* Handling for iep.c test case. */
> - if (lc->trans_exc_id & 0x80UL && lc->trans_exc_id & 0x04UL &&
> - !(lc->trans_exc_id & 0x08UL))
> + if (prot_is_iep(lc->trans_exc_id))
> /*
> * We branched to the instruction that caused
> * the exception so we can use the return
> @@ -126,6 +127,26 @@ static void fixup_pgm_int(struct stack_frame_int *stack)
> /* suppressed/terminated/completed point already at the next address */
> }
>
> +static void print_storage_exception_information(void)
> +{
> + switch (lc->pgm_int_code) {
> + case PGM_INT_CODE_PROTECTION:
> + case PGM_INT_CODE_PAGE_TRANSLATION:
> + case PGM_INT_CODE_SEGMENT_TRANSLATION:
> + case PGM_INT_CODE_ASCE_TYPE:
> + case PGM_INT_CODE_REGION_FIRST_TRANS:
> + case PGM_INT_CODE_REGION_SECOND_TRANS:
> + case PGM_INT_CODE_REGION_THIRD_TRANS:
> + case PGM_INT_CODE_SECURE_STOR_ACCESS:
> + case PGM_INT_CODE_NON_SECURE_STOR_ACCESS:
> + case PGM_INT_CODE_SECURE_STOR_VIOLATION:
> + print_decode_teid(lc->trans_exc_id);
> + break;
> + default:
> + return;
> + }
> +}
> +
> static void print_int_regs(struct stack_frame_int *stack)
> {
> printf("\n");
> @@ -155,6 +176,10 @@ static void print_pgm_info(struct stack_frame_int *stack)
> lc->pgm_int_code, stap(), lc->pgm_old_psw.addr, lc->pgm_int_id);
> print_int_regs(stack);
> dump_stack();
> +
> + /* Dump stack doesn't end with a \n so we add it here instead */
> + printf("\n");
> + print_storage_exception_information();
> report_summary();
> abort();
> }
> diff --git a/s390x/Makefile b/s390x/Makefile
> index ef8041a6..5d1a33a0 100644
> --- a/s390x/Makefile
> +++ b/s390x/Makefile
> @@ -72,6 +72,7 @@ cflatobjs += lib/s390x/css_lib.o
> cflatobjs += lib/s390x/malloc_io.o
> cflatobjs += lib/s390x/uv.o
> cflatobjs += lib/s390x/sie.o
> +cflatobjs += lib/s390x/fault.o
>
> OBJDIRS += lib/s390x
>
