diff mbox

[1/5] target-tricore: Add trap handling

Message ID 1455206520-6465-2-git-send-email-kbastian@mail.uni-paderborn.de
State New
Headers show

Commit Message

Bastian Koppelmann Feb. 11, 2016, 4:01 p.m. UTC
Add the infrastructure needed to generate and handle traps.

Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
---
 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(-)

Comments

Richard Henderson Feb. 12, 2016, 2:39 a.m. UTC | #1
On 02/12/2016 03:01 AM, Bastian Koppelmann wrote:
> Add the infrastructure needed to generate and handle traps.
>
> Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
> ---
>   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~
Richard Henderson Feb. 12, 2016, 2:44 a.m. UTC | #2
On 02/12/2016 03:01 AM, Bastian Koppelmann wrote:
> +static inline void
> +generate_trap(DisasContext *ctx, int class, int tin)
> +{
> +    TCGv_i32 classtemp = tcg_const_i32(class);
> +    gen_save_pc(ctx->pc);
> +    /* upper context cannot be saved, if the context list is empty */
> +    if (class != TRAPC_SYSCALL) {
> +        gen_helper_svucx(cpu_env);
> +    }
> +    /* Tin is loaded into d[15] */
> +    tcg_gen_movi_tl(cpu_gpr_d[15], tin);
> +    /* The return address in A[11] is updated. */
> +    if (class == TRAPC_SYSCALL) {
> +        tcg_gen_movi_tl(cpu_gpr_a[11], ctx->next_pc);
> +    } else {
> +        tcg_gen_movi_tl(cpu_gpr_a[11], ctx->pc);
> +    }
> +    gen_helper_raise_exception_err(cpu_env, classtemp, 0);
> +    tcg_temp_free(classtemp);
> +}

Again, drop the inline.  Missing ctx->bstate = BS_EXCP.


r~
Bastian Koppelmann Feb. 12, 2016, 11:12 a.m. UTC | #3
On 02/12/2016 03:39 AM, Richard Henderson wrote:
> On 02/12/2016 03:01 AM, Bastian Koppelmann wrote:
>> +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.

What do you mean by "pre-interrupt state"? The register context is saved
by generate_trap() calls.

Cheers,
Bastian
Richard Henderson Feb. 12, 2016, 6:55 p.m. UTC | #4
On 02/12/2016 10:12 PM, Bastian Koppelmann wrote:
> On 02/12/2016 03:39 AM, Richard Henderson wrote:
>> On 02/12/2016 03:01 AM, Bastian Koppelmann wrote:
>>> +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.
>
> What do you mean by "pre-interrupt state"? The register context is saved
> by generate_trap() calls.

Because do_interrupt is normally for handling asynchronous interrupts.  Stuff 
that isn't related at all to the instruction stream.  Which therefore could not 
have saved anything at all.

It does get (ab)used on some targets for synchronous events, because the method 
by which kernel entry is made is the same and the author wanted to share code.

But if you have no external hardware devices, no timer interrupt or anything, 
let's not start by passing synchronous events through this function.  It sounds 
like the code here in do_interrupt more properly belongs at the end of 
generate_trap.

Err.. the generate_trap in op_helper I mean.  It's confusing that you have two 
(static) functions of the same name in different files.

I do wonder if these two functions ought to share more code.  For instance, 
op_helper.c places TIN into a_15, whereas translate.c places TIN into d_15.


r~
Bastian Koppelmann Feb. 12, 2016, 8:08 p.m. UTC | #5
On 02/12/2016 07:55 PM, Richard Henderson wrote:
> On 02/12/2016 10:12 PM, Bastian Koppelmann wrote:
>> On 02/12/2016 03:39 AM, Richard Henderson wrote:
>>
>> What do you mean by "pre-interrupt state"? The register context is saved
>> by generate_trap() calls.
> 
> Because do_interrupt is normally for handling asynchronous interrupts. 
> Stuff that isn't related at all to the instruction stream.  Which
> therefore could not have saved anything at all.
> 

Ah, I see. This makes a lot of sense now. I didn't know that
do_interrupt() is only for asynchronous interrupts.

> It does get (ab)used on some targets for synchronous events, because the
> method by which kernel entry is made is the same and the author wanted
> to share code.
> 
> But if you have no external hardware devices, no timer interrupt or
> anything, let's not start by passing synchronous events through this
> function.  It sounds like the code here in do_interrupt more properly
> belongs at the end of generate_trap.
> 
> Err.. the generate_trap in op_helper I mean.  It's confusing that you
> have two (static) functions of the same name in different files.

Fair enough. I guess it makes sense to always use the helper for
generate_trap, since a helper call is necessary anyways.

> 
> I do wonder if these two functions ought to share more code.  For
> instance, op_helper.c places TIN into a_15, whereas translate.c places
> TIN into d_15.

Woops... good catch :).

Thanks for the review.

Cheers,
Bastian
diff mbox

Patch

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;
+    }
+    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);
+
+    /* Call Depth Counter is enabled, PSW.CDE = 1. */
+    env->PSW |= MASK_PSW_CDE;
+
+    /* Write permission to global registers A[0], A[1], A[8], A[9] is
+       disabled: PSW.GW = 0. */
+    env->PSW &= ~MASK_PSW_GW;
+
+    /*The interrupt system is globally disabled: ICR.IE = 0. The ‘old’
+      ICR.IE and ICR.CCPN are saved */
+
+    /* PCXI.PIE = ICR.IE */
+    env->PCXI = ((env->PCXI & ~MASK_PCXI_PIE) +
+                           ((env->ICR & MASK_ICR_IE) << 15));
+    /* PCXI.PCPN = ICR.CCPN */
+    env->PCXI = (env->PCXI & 0xffffff) +
+                          ((env->ICR & MASK_ICR_CCPN) << 24);
+
+    if (cs->exception_index <= TRAPC_NMI) {
+        /* The trap vector table is accessed to fetch the first instruction of
+           the trap handler. */
+        env->PC = env->BTV | (cs->exception_index << 5);
+    } else if (cs->exception_index == TRAPC_IRQ) {
+        /* The interrupt vector table is accessed to fetch the first instruction
+           of the interrupt handler. */
+        env->PC = env->BIV | ((env->ICR & MASK_ICR_PIPN) >> 10);
+    }
+}
diff --git a/target-tricore/helper.h b/target-tricore/helper.h
index cc221f1..5ae1125 100644
--- a/target-tricore/helper.h
+++ b/target-tricore/helper.h
@@ -132,6 +132,7 @@  DEF_HELPER_2(lducx, void, env, i32)
 DEF_HELPER_2(stlcx, void, env, i32)
 DEF_HELPER_2(stucx, void, env, i32)
 DEF_HELPER_1(svlcx, void, env)
+DEF_HELPER_1(svucx, void, env)
 DEF_HELPER_1(rslcx, void, env)
 /* Address mode helper */
 DEF_HELPER_1(br_update, i32, i32)
@@ -139,3 +140,5 @@  DEF_HELPER_2(circ_update, i32, i32, i32)
 /* PSW cache helper */
 DEF_HELPER_2(psw_write, void, env, i32)
 DEF_HELPER_1(psw_read, i32, env)
+/* Exceptions */
+DEF_HELPER_3(raise_exception_err, noreturn, env, i32, int)
diff --git a/target-tricore/op_helper.c b/target-tricore/op_helper.c
index 3aa6326..fbe2be0 100644
--- a/target-tricore/op_helper.c
+++ b/target-tricore/op_helper.c
@@ -21,6 +21,41 @@ 
 #include "exec/cpu_ldst.h"
 #include <zlib.h> /* for crc32 */
 
+
+/* Exception helpers */
+void helper_raise_exception_err(CPUTriCoreState *env, uint32_t class,
+                                int error_code)
+{
+    CPUState *cs = CPU(tricore_env_get_cpu(env));
+    cs->exception_index = class;
+    env->error_code = error_code;
+    cpu_loop_exit(cs);
+}
+
+static inline void generate_trap(CPUTriCoreState *env, uint32_t class,
+                                 uint32_t tin)
+{
+    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);
+}
+
 /* Addressing mode helper */
 
 static uint16_t reverse16(uint16_t val)
@@ -2625,6 +2660,47 @@  void helper_svlcx(CPUTriCoreState *env)
     }
 }
 
+void helper_svucx(CPUTriCoreState *env)
+{
+    target_ulong tmp_FCX;
+    target_ulong ea;
+    target_ulong new_FCX;
+
+    if (env->FCX == 0) {
+        /* FCU trap */
+    }
+    /* tmp_FCX = FCX; */
+    tmp_FCX = env->FCX;
+    /* EA = {FCX.FCXS, 6'b0, FCX.FCXO, 6'b0}; */
+    ea = ((env->FCX & MASK_FCX_FCXS) << 12) +
+         ((env->FCX & MASK_FCX_FCXO) << 6);
+    /* new_FCX = M(EA, word); */
+    new_FCX = cpu_ldl_data(env, ea);
+    /* M(EA, 16 * word) = {PCXI, PSW, A[10], A[11], D[8], D[9], D[10], D[11],
+                           A[12], A[13], A[14], A[15], D[12], D[13], D[14],
+                           D[15]}; */
+    save_context_upper(env, ea);
+
+    /* PCXI.PCPN = ICR.CCPN; */
+    env->PCXI = (env->PCXI & 0xffffff) +
+                ((env->ICR & MASK_ICR_CCPN) << 24);
+    /* PCXI.PIE = ICR.IE; */
+    env->PCXI = ((env->PCXI & ~MASK_PCXI_PIE) +
+                ((env->ICR & MASK_ICR_IE) << 15));
+    /* PCXI.UL = 1; */
+    env->PCXI |= MASK_PCXI_UL;
+
+    /* PCXI[19: 0] = FCX[19: 0]; */
+    env->PCXI = (env->PCXI & 0xfff00000) + (env->FCX & 0xfffff);
+    /* FCX[19: 0] = new_FCX[19: 0]; */
+    env->FCX = (env->FCX & 0xfff00000) + (new_FCX & 0xfffff);
+
+    /* if (tmp_FCX == LCX) trap(FCD);*/
+    if (tmp_FCX == env->LCX) {
+        /* FCD trap */
+    }
+}
+
 void helper_rslcx(CPUTriCoreState *env)
 {
     target_ulong ea;
diff --git a/target-tricore/translate.c b/target-tricore/translate.c
index a70fdf7..721878d 100644
--- a/target-tricore/translate.c
+++ b/target-tricore/translate.c
@@ -3244,6 +3244,27 @@  static inline void gen_goto_tb(DisasContext *ctx, int n, target_ulong dest)
     }
 }
 
+static inline void
+generate_trap(DisasContext *ctx, int class, int tin)
+{
+    TCGv_i32 classtemp = tcg_const_i32(class);
+    gen_save_pc(ctx->pc);
+    /* upper context cannot be saved, if the context list is empty */
+    if (class != TRAPC_SYSCALL) {
+        gen_helper_svucx(cpu_env);
+    }
+    /* Tin is loaded into d[15] */
+    tcg_gen_movi_tl(cpu_gpr_d[15], tin);
+    /* The return address in A[11] is updated. */
+    if (class == TRAPC_SYSCALL) {
+        tcg_gen_movi_tl(cpu_gpr_a[11], ctx->next_pc);
+    } else {
+        tcg_gen_movi_tl(cpu_gpr_a[11], ctx->pc);
+    }
+    gen_helper_raise_exception_err(cpu_env, classtemp, 0);
+    tcg_temp_free(classtemp);
+}
+
 static inline void gen_branch_cond(DisasContext *ctx, TCGCond cond, TCGv r1,
                                    TCGv r2, int16_t address)
 {