Re: [Qemu-devel] [PATCH 010/126] target-s390: Reorg exception handling

[Alexander Graf <agraf@suse.de>]

On 09/09/2012 11:04 PM, Richard Henderson wrote:
> Make the user path more like the system path.  Prepare for more kinds
> of runtime exceptions.  Compute ILC from S->NEXT_PC, rather than
> passing it around.
>
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>   linux-user/main.c          | 133 +++++++++++++++++----------
>   target-s390x/cpu.h         |  11 ---
>   target-s390x/misc_helper.c |  10 ++-
>   target-s390x/translate.c   | 220 +++++++++++++++++----------------------------
>   4 files changed, 174 insertions(+), 200 deletions(-)
>
> diff --git a/linux-user/main.c b/linux-user/main.c
> index 1a1c661..8f6f39b 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -2935,71 +2935,108 @@ void cpu_loop(CPUAlphaState *env)
>   #ifdef TARGET_S390X
>   void cpu_loop(CPUS390XState *env)
>   {
> -    int trapnr;
> +    int trapnr, n, sig;
>       target_siginfo_t info;
> +    target_ulong addr;
>   
>       while (1) {
> -        trapnr = cpu_s390x_exec (env);
> -
> +        trapnr = cpu_s390x_exec(env);
>           switch (trapnr) {
>           case EXCP_INTERRUPT:
> -            /* just indicate that signals should be handled asap */
> +            /* Just indicate that signals should be handled asap.  */
>               break;
> -        case EXCP_DEBUG:
> -            {
> -                int sig;
>   
> -                sig = gdb_handlesig (env, TARGET_SIGTRAP);
> -                if (sig) {
> -                    info.si_signo = sig;
> -                    info.si_errno = 0;
> -                    info.si_code = TARGET_TRAP_BRKPT;
> -                    queue_signal(env, info.si_signo, &info);
> -                }
> +        case EXCP_SVC:
> +            n = env->int_svc_code;
> +            if (!n) {
> +                /* syscalls > 255 */
> +                n = env->regs[1];
>               }
> +            env->psw.addr += env->int_svc_ilc;
> +            env->regs[2] = do_syscall(env, n, env->regs[2], env->regs[3],
> +                                      env->regs[4], env->regs[5],
> +                                      env->regs[6], env->regs[7], 0, 0);
>               break;
> -        case EXCP_SVC:
> -            {
> -                int n = env->int_svc_code;
> -                if (!n) {
> -                    /* syscalls > 255 */
> -                    n = env->regs[1];
> -                }
> -                env->psw.addr += env->int_svc_ilc;
> -                env->regs[2] = do_syscall(env, n,
> -                           env->regs[2],
> -                           env->regs[3],
> -                           env->regs[4],
> -                           env->regs[5],
> -                           env->regs[6],
> -                           env->regs[7],
> -                           0, 0);
> +
> +        case EXCP_DEBUG:
> +            sig = gdb_handlesig(env, TARGET_SIGTRAP);
> +            if (sig) {
> +                n = TARGET_TRAP_BRKPT;
> +                goto do_signal_pc;
>               }
>               break;
> -        case EXCP_ADDR:
> -            {
> -                info.si_signo = SIGSEGV;
> -                info.si_errno = 0;
> +        case EXCP_PGM:
> +            n = env->int_pgm_code;
> +            switch (n) {
> +            case PGM_OPERATION:
> +            case PGM_PRIVILEGED:
> +                sig = SIGILL;
> +                n = TARGET_ILL_ILLOPC;
> +                goto do_signal_pc;
> +            case PGM_PROTECTION:
> +            case PGM_ADDRESSING:
> +                sig = SIGSEGV;
>                   /* XXX: check env->error_code */
> -                info.si_code = TARGET_SEGV_MAPERR;
> -                info._sifields._sigfault._addr = env->__excp_addr;
> -                queue_signal(env, info.si_signo, &info);
> +                n = TARGET_SEGV_MAPERR;
> +                addr = env->__excp_addr;
> +                goto do_signal;
> +            case PGM_EXECUTE:
> +            case PGM_SPECIFICATION:
> +            case PGM_DATA:
> +            case PGM_SPECIAL_OP:
> +            case PGM_OPERAND:
> +                sig = SIGILL;
> +                n = TARGET_ILL_ILLOPN;
> +                goto do_signal_pc;
> +            case PGM_FIXPT_OVERFLOW:
> +                sig = SIGFPE;
> +                n = TARGET_FPE_INTOVF;
> +                goto do_signal_pc;
> +            case PGM_FIXPT_DIVIDE:
> +                sig = SIGFPE;
> +                n = TARGET_FPE_INTDIV;
> +                goto do_signal_pc;
> +            case PGM_HFP_EXP_OVERFLOW:
> +                sig = SIGFPE;
> +                n = TARGET_FPE_FLTOVF;
> +                goto do_signal_pc;
> +            case PGM_HFP_EXP_UNDERFLOW:
> +                sig = SIGFPE;
> +                n = TARGET_FPE_FLTUND;
> +                goto do_signal_pc;
> +            case PGM_HFP_SIGNIFICANCE:
> +                sig = SIGFPE;
> +                n = TARGET_FPE_FLTRES;
> +                goto do_signal_pc;
> +            case PGM_HFP_DIVIDE:
> +                sig = SIGFPE;
> +                n = TARGET_FPE_FLTDIV;
> +                goto do_signal_pc;
> +            case PGM_HFP_SQRT:
> +                sig = SIGFPE;
> +                n = TARGET_FPE_FLTINV;
> +                goto do_signal_pc;
> +            default:
> +                fprintf(stderr, "Unhandled program exception: %#x\n", n);
> +                cpu_dump_state(env, stderr, fprintf, 0);
> +                exit(1);
>               }
>               break;
> -        case EXCP_SPEC:
> -            {
> -                fprintf(stderr,"specification exception insn 0x%08x%04x\n", ldl(env->psw.addr), lduw(env->psw.addr + 4));
> -                info.si_signo = SIGILL;
> -                info.si_errno = 0;
> -                info.si_code = TARGET_ILL_ILLOPC;
> -                info._sifields._sigfault._addr = env->__excp_addr;
> -                queue_signal(env, info.si_signo, &info);
> -            }
> +
> +        do_signal_pc:
> +            addr = env->psw.addr;
> +        do_signal:
> +            info.si_signo = sig;
> +            info.si_errno = 0;
> +            info.si_code = n;
> +            info._sifields._sigfault._addr = addr;
> +            queue_signal(env, info.si_signo, &info);
>               break;
> +
>           default:
> -            printf ("Unhandled trap: 0x%x\n", trapnr);
> +            fprintf(stderr, "Unhandled trap: 0x%x\n", trapnr);
>               cpu_dump_state(env, stderr, fprintf, 0);
> -            exit (1);
> +            exit(1);
>           }
>           process_pending_signals (env);
>       }
> diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
> index 471fb91..17829b5 100644
> --- a/target-s390x/cpu.h
> +++ b/target-s390x/cpu.h
> @@ -352,21 +352,10 @@ static inline void cpu_set_tls(CPUS390XState *env, target_ulong newtls)
>   
>   #include "exec-all.h"
>   
> -#ifdef CONFIG_USER_ONLY
> -
> -#define EXCP_OPEX 1 /* operation exception (sigill) */
> -#define EXCP_SVC 2 /* supervisor call (syscall) */
> -#define EXCP_ADDR 5 /* addressing exception */
> -#define EXCP_SPEC 6 /* specification exception */
> -
> -#else
> -
>   #define EXCP_EXT 1 /* external interrupt */
>   #define EXCP_SVC 2 /* supervisor call (syscall) */
>   #define EXCP_PGM 3 /* program interruption */
>   
> -#endif /* CONFIG_USER_ONLY */
> -
>   #define INTERRUPT_EXT        (1 << 0)
>   #define INTERRUPT_TOD        (1 << 1)
>   #define INTERRUPT_CPUTIMER   (1 << 2)
> diff --git a/target-s390x/misc_helper.c b/target-s390x/misc_helper.c
> index f405b97..1502f15 100644
> --- a/target-s390x/misc_helper.c
> +++ b/target-s390x/misc_helper.c
> @@ -86,10 +86,12 @@ void do_interrupt(CPUS390XState *env)
>   int cpu_s390x_handle_mmu_fault(CPUS390XState *env, target_ulong address,
>                                  int rw, int mmu_idx)
>   {
> -    /* fprintf(stderr, "%s: address 0x%lx rw %d mmu_idx %d\n",
> -       __func__, address, rw, mmu_idx); */
> -    env->exception_index = EXCP_ADDR;
> -    /* FIXME: find out how this works on a real machine */
> +    env->exception_index = EXCP_PGM;
> +    env->int_pgm_code = PGM_PROTECTION;
> +    /* A real machine puts the address in LowCore, which the kernel
> +       helpfully interprets for us.  Since we've nothing similar
> +       within the userland address space, invent something for use
> +       within cpu_loop.  */
>       env->__excp_addr = address;
>       return 1;
>   }
> diff --git a/target-s390x/translate.c b/target-s390x/translate.c
> index 996d786..70e5d87 100644
> --- a/target-s390x/translate.c
> +++ b/target-s390x/translate.c
> @@ -18,7 +18,6 @@
>    * License along with this library; if not, see <http://www.gnu.org/licenses/>.
>    */
>   
> -/* #define DEBUG_ILLEGAL_INSTRUCTIONS */
>   /* #define DEBUG_INLINE_BRANCHES */
>   #define S390X_DEBUG_DISAS
>   /* #define S390X_DEBUG_DISAS_VERBOSE */
> @@ -336,103 +335,52 @@ static inline int get_mem_index(DisasContext *s)
>       }
>   }
>   
> -static inline void gen_debug(DisasContext *s)
> +static void gen_exception(int excp)
>   {
> -    TCGv_i32 tmp = tcg_const_i32(EXCP_DEBUG);
> -    update_psw_addr(s);
> -    gen_op_calc_cc(s);
> -    gen_helper_exception(cpu_env, tmp);
> -    tcg_temp_free_i32(tmp);
> -    s->is_jmp = DISAS_EXCP;
> -}
> -
> -#ifdef CONFIG_USER_ONLY
> -
> -static void gen_illegal_opcode(DisasContext *s, int ilc)
> -{
> -    TCGv_i32 tmp = tcg_const_i32(EXCP_SPEC);
> -    update_psw_addr(s);
> -    gen_op_calc_cc(s);
> +    TCGv_i32 tmp = tcg_const_i32(excp);
>       gen_helper_exception(cpu_env, tmp);
>       tcg_temp_free_i32(tmp);
> -    s->is_jmp = DISAS_EXCP;
>   }
>   
> -#else /* CONFIG_USER_ONLY */
> -
> -static void debug_print_inst(DisasContext *s, int ilc)
> -{
> -#ifdef DEBUG_ILLEGAL_INSTRUCTIONS
> -    uint64_t inst = 0;
> -
> -    switch (ilc & 3) {
> -    case 1:
> -        inst = ld_code2(s->pc);
> -        break;
> -    case 2:
> -        inst = ld_code4(s->pc);
> -        break;
> -    case 3:
> -        inst = ld_code6(s->pc);
> -        break;
> -    }
> -
> -    fprintf(stderr, "Illegal instruction [%d at %016" PRIx64 "]: 0x%016"
> -            PRIx64 "\n", ilc, s->pc, inst);
> -#endif
> -}
> -
> -static void gen_program_exception(DisasContext *s, int ilc, int code)
> +static void gen_program_exception(DisasContext *s, int code)
>   {
>       TCGv_i32 tmp;
>   
> -    debug_print_inst(s, ilc);
> -
> -    /* remember what pgm exeption this was */
> +    /* Remember what pgm exeption this was.  */
>       tmp = tcg_const_i32(code);
>       tcg_gen_st_i32(tmp, cpu_env, offsetof(CPUS390XState, int_pgm_code));
>       tcg_temp_free_i32(tmp);
>   
> -    tmp = tcg_const_i32(ilc);
> +    tmp = tcg_const_i32(s->next_pc - s->pc);

Mind to explain this one?


>       tcg_gen_st_i32(tmp, cpu_env, offsetof(CPUS390XState, int_pgm_ilc));
>       tcg_temp_free_i32(tmp);


Alex