diff mbox

[v17,5/9] target-avr: adding AVR interrupt handling

Message ID 1471522070-77598-6-git-send-email-mrolnik@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Michael Rolnik Aug. 18, 2016, 12:07 p.m. UTC 
Signed-off-by: Michael Rolnik <mrolnik@gmail.com>
---
 target-avr/helper.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

Comments

Peter Maydell Aug. 18, 2016, 6:32 p.m. UTC | #1 
On 18 August 2016 at 13:07, Michael Rolnik <mrolnik@gmail.com> wrote:
> Signed-off-by: Michael Rolnik <mrolnik@gmail.com>
> ---
>  target-avr/helper.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
>
> diff --git a/target-avr/helper.c b/target-avr/helper.c
> index b48222d..8511fb7 100644
> --- a/target-avr/helper.c
> +++ b/target-avr/helper.c
> @@ -32,11 +32,66 @@
>  bool avr_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
>  {
>      bool ret = false;
> +    CPUClass *cc = CPU_GET_CLASS(cs);
> +    AVRCPU *cpu = AVR_CPU(cs);
> +    CPUAVRState *env = &cpu->env;
> +
> +    if (interrupt_request & CPU_INTERRUPT_RESET) {
> +        if (cpu_interrupts_enabled(env)) {
> +            cs->exception_index = EXCP_RESET;
> +            cc->do_interrupt(cs);
> +
> +            cs->interrupt_request &= ~CPU_INTERRUPT_RESET;
> +
> +            ret = true;
> +        }
> +    }

Are you sure that you need to handle CPU_INTERRUPT_RESET here?
It looks to me like the code in cpu-exec.c should deal with it
for you.

> +    if (interrupt_request & CPU_INTERRUPT_HARD) {
> +        if (cpu_interrupts_enabled(env) && env->intsrc != 0) {
> +            int index = ctz32(env->intsrc);
> +            cs->exception_index = EXCP_INT(index);
> +            cc->do_interrupt(cs);
> +
> +            env->intsrc &= env->intsrc - 1; /* clear the interrupt */

I think clearing the env->intsrc bit should go in avr_cpu_do_interrupt().

> +            cs->interrupt_request &= ~CPU_INTERRUPT_HARD;

I'm not sure what the interrupt model for this CPU is,
but other CPU models don't do this, so maybe you don't
want to either. (The usual model is that CPU_INTERRUPT_HARD
corresponds to an interrupt input signal to the CPU;
for instance on ARM it's the IRQ line. When the signal
goes high we call cpu_interrupt(cs, CPU_INTERRUPT_HARD)
which sets the bit, and when it goes low we call
cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD) which clears
the bit.)

> +
> +            ret = true;
> +        }
> +    }
>      return ret;
>  }
>
>  void avr_cpu_do_interrupt(CPUState *cs)
>  {
> +    AVRCPU *cpu = AVR_CPU(cs);
> +    CPUAVRState *env = &cpu->env;
> +
> +    uint32_t ret = env->pc_w;
> +    int vector = 0;
> +    int size = avr_feature(env, AVR_FEATURE_JMP_CALL) ? 2 : 1;
> +    int base = 0; /* TODO: where to get it */
> +
> +    if (cs->exception_index == EXCP_RESET) {
> +        vector = 0;
> +    } else if (env->intsrc != 0) {
> +        vector = ctz32(env->intsrc) + 1;
> +    }

Should env->intsrc==0 really be treated like reset?
(If it's a can't-happen case then asserting would probably be good.)

> +
> +    if (avr_feature(env, AVR_FEATURE_3_BYTE_PC)) {
> +        cpu_stb_data(env, env->sp--, (ret & 0x0000ff));
> +        cpu_stb_data(env, env->sp--, (ret & 0x00ff00) >>  8);
> +        cpu_stb_data(env, env->sp--, (ret & 0xff0000) >> 16);
> +    } else if (avr_feature(env, AVR_FEATURE_2_BYTE_PC)) {
> +        cpu_stb_data(env, env->sp--, (ret & 0x0000ff));
> +        cpu_stb_data(env, env->sp--, (ret & 0x00ff00) >>  8);
> +    } else {
> +        cpu_stb_data(env, env->sp--, (ret & 0x0000ff));
> +    }
> +
> +    env->pc_w = base + vector * size;
> +    env->sregI = 0; /* clear Global Interrupt Flag */
> +
> +    cs->exception_index = -1;
>  }
>
>  int avr_cpu_memory_rw_debug(CPUState *cs, vaddr addr, uint8_t *buf,
> --
> 2.4.9 (Apple Git-60)

thanks
-- PMM
diff mbox

Patch

diff --git a/target-avr/helper.c b/target-avr/helper.c
index b48222d..8511fb7 100644
--- a/target-avr/helper.c
+++ b/target-avr/helper.c
@@ -32,11 +32,66 @@ 
 bool avr_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
 {
     bool ret = false;
+    CPUClass *cc = CPU_GET_CLASS(cs);
+    AVRCPU *cpu = AVR_CPU(cs);
+    CPUAVRState *env = &cpu->env;
+
+    if (interrupt_request & CPU_INTERRUPT_RESET) {
+        if (cpu_interrupts_enabled(env)) {
+            cs->exception_index = EXCP_RESET;
+            cc->do_interrupt(cs);
+
+            cs->interrupt_request &= ~CPU_INTERRUPT_RESET;
+
+            ret = true;
+        }
+    }
+    if (interrupt_request & CPU_INTERRUPT_HARD) {
+        if (cpu_interrupts_enabled(env) && env->intsrc != 0) {
+            int index = ctz32(env->intsrc);
+            cs->exception_index = EXCP_INT(index);
+            cc->do_interrupt(cs);
+
+            env->intsrc &= env->intsrc - 1; /* clear the interrupt */
+            cs->interrupt_request &= ~CPU_INTERRUPT_HARD;
+
+            ret = true;
+        }
+    }
     return ret;
 }
 
 void avr_cpu_do_interrupt(CPUState *cs)
 {
+    AVRCPU *cpu = AVR_CPU(cs);
+    CPUAVRState *env = &cpu->env;
+
+    uint32_t ret = env->pc_w;
+    int vector = 0;
+    int size = avr_feature(env, AVR_FEATURE_JMP_CALL) ? 2 : 1;
+    int base = 0; /* TODO: where to get it */
+
+    if (cs->exception_index == EXCP_RESET) {
+        vector = 0;
+    } else if (env->intsrc != 0) {
+        vector = ctz32(env->intsrc) + 1;
+    }
+
+    if (avr_feature(env, AVR_FEATURE_3_BYTE_PC)) {
+        cpu_stb_data(env, env->sp--, (ret & 0x0000ff));
+        cpu_stb_data(env, env->sp--, (ret & 0x00ff00) >>  8);
+        cpu_stb_data(env, env->sp--, (ret & 0xff0000) >> 16);
+    } else if (avr_feature(env, AVR_FEATURE_2_BYTE_PC)) {
+        cpu_stb_data(env, env->sp--, (ret & 0x0000ff));
+        cpu_stb_data(env, env->sp--, (ret & 0x00ff00) >>  8);
+    } else {
+        cpu_stb_data(env, env->sp--, (ret & 0x0000ff));
+    }
+
+    env->pc_w = base + vector * size;
+    env->sregI = 0; /* clear Global Interrupt Flag */
+
+    cs->exception_index = -1;
 }
 
 int avr_cpu_memory_rw_debug(CPUState *cs, vaddr addr, uint8_t *buf,