Re: [Qemu-devel] [PATCH 1/5] target-tricore: Add trap handling

[Richard Henderson]

On 02/12/2016 03:01 AM, Bastian Koppelmann wrote:
> Add the infrastructure needed to generate and handle traps.
>
> Signed-off-by: Bastian Koppelmann <address@hidden>
> ---
>   target-tricore/cpu-qom.h   |  2 +-
>   target-tricore/cpu.c       |  2 +-
>   target-tricore/cpu.h       |  1 +
>   target-tricore/helper.c    | 52 +++++++++++++++++++++++++++++++
>   target-tricore/helper.h    |  3 ++
>   target-tricore/op_helper.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++
>   target-tricore/translate.c | 21 +++++++++++++
>   7 files changed, 155 insertions(+), 2 deletions(-)
>
> diff --git a/target-tricore/cpu-qom.h b/target-tricore/cpu-qom.h
> index 66c9664..0909c0c 100644
> --- a/target-tricore/cpu-qom.h
> +++ b/target-tricore/cpu-qom.h
> @@ -65,6 +65,6 @@ static inline TriCoreCPU *tricore_env_get_cpu(CPUTriCoreState 
> *env)
>   hwaddr tricore_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
>   void tricore_cpu_dump_state(CPUState *cpu, FILE *f,
>                               fprintf_function cpu_fprintf, int flags);
> -
> +void tricore_cpu_do_interrupt(CPUState *cs);
>
>   #endif /*QEMU_TRICORE_CPU_QOM_H */
> diff --git a/target-tricore/cpu.c b/target-tricore/cpu.c
> index f8b8518..01e39dc 100644
> --- a/target-tricore/cpu.c
> +++ b/target-tricore/cpu.c
> @@ -166,7 +166,7 @@ static void tricore_cpu_class_init(ObjectClass *c, void 
> *data)
>       cc->reset = tricore_cpu_reset;
>       cc->class_by_name = tricore_cpu_class_by_name;
>       cc->has_work = tricore_cpu_has_work;
> -
> +    cc->do_interrupt = tricore_cpu_do_interrupt;
>       cc->dump_state = tricore_cpu_dump_state;
>       cc->set_pc = tricore_cpu_set_pc;
>       cc->synchronize_from_tb = tricore_cpu_synchronize_from_tb;
> diff --git a/target-tricore/cpu.h b/target-tricore/cpu.h
> index 20a12f3..b4cc2ef 100644
> --- a/target-tricore/cpu.h
> +++ b/target-tricore/cpu.h
> @@ -271,6 +271,7 @@ enum {
>       TRAPC_ASSERT   = 5,
>       TRAPC_SYSCALL  = 6,
>       TRAPC_NMI      = 7,
> +    TRAPC_IRQ      = 8
>   };
>
>   /* Class 0 TIN */
> diff --git a/target-tricore/helper.c b/target-tricore/helper.c
> index a8fd418..d781f21 100644
> --- a/target-tricore/helper.c
> +++ b/target-tricore/helper.c
> @@ -133,3 +133,55 @@ void psw_write(CPUTriCoreState *env, uint32_t val)
>       env->PSW_USB_SAV = ((val & MASK_USB_SAV) << 4);
>       env->PSW = val;
>   }
> +
> +void tricore_cpu_do_interrupt(CPUState *cs)
> +{
> +    TriCoreCPU *cpu = TRICORE_CPU(cs);
> +    CPUTriCoreState *env = &cpu->env;
> +
> +    /* The stack pointer in A[10] is set to the Interrupt Stack Pointer (ISP)
> +       when the processor was not previously using the interrupt stack
> +       (in case of PSW.IS = 0). The stack pointer bit is set for using the
> +       interrupt stack: PSW.IS = 1. */
> +    if ((env->PSW & MASK_PSW_IS) == 0) {
> +        env->gpr_a[10] = env->ISP;
> +    }

You appear to have forgotten to save pre-interrupt state here.

> +    env->PSW |= MASK_PSW_IS;
> +    /* The I/O mode is set to Supervisor mode, which means all permissions
> +       are enabled: PSW.IO = 10 B .*/
> +    env->PSW |= (2 << 10);
> +
> +    /*The current Protection Register Set is set to 0: PSW.PRS = 00 B .*/
> +    env->PSW &= ~(MASK_PSW_PRS);
> +
> +    /* The Call Depth Counter (CDC) is cleared, and the call depth limit is
> +       set for 64: PSW.CDC = 0000000 B .*/
> +    env->PSW &= ~(MASK_PSW_CDC);

Unnecessary parenthesis.

> +    if ((class == TRAPC_CTX_MNG) && (tin == TIN3_FCU)) {

Likewise.

> +static inline void generate_trap(CPUTriCoreState *env, uint32_t class,
> +                                 uint32_t tin)

Drop the inline.  This is an exception path, and therefore unlikely.

> +{
> +    CPUState *cs = CPU(tricore_env_get_cpu(env));
> +
> +    if ((class == TRAPC_CTX_MNG) && (tin == TIN3_FCU)) {
> +        /* upper context cannot be saved, if the context list is empty */
> +    } else {
> +        helper_svucx(env);
> +    }
> +    /* in order to allow the trap handlers for async traps to recognize when
> +       when they have interrupted FCD trap handler FCDSF flag is set */
> +    if (class == TRAPC_CTX_MNG && tin == TIN3_FCD) {
> +        env->SYSCON |= MASK_SYSCON_FCD_SF;
> +        /* when we fault here, the return address is the start of the faulting
> +           trap handler */
> +        env->gpr_a[11] = env->BTV | cs->exception_index << 5;
> +    } else {
> +        env->gpr_a[11] = env->PC;
> +    }
> +    env->gpr_a[15] = tin;
> +    helper_raise_exception_err(env, class, 0);

You probably want this version of generate_trap to use a raise_exception helper 
that uses cpu_loop_exit_restore.  This avoids the extra gen_save_pc calls you 
add in patch 2.


r~