diff mbox series

[7/7] target/nios2: Rewrite interrupt handling

Message ID 20220227182125.21809-8-richard.henderson@linaro.org
State Superseded
Headers show
Series target/nios2: Rewrite interrupt handling | expand

Commit Message

Richard Henderson Feb. 27, 2022, 6:21 p.m. UTC 
Drop irq_pending boolean.
Drop helper_check_interrupts.
Move checks for irq disabled into nios2_cpu_exec_interrupt.
End the TB on writes to ienable, just like to status.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/nios2/cpu.h       |  1 -
 target/nios2/helper.h    |  1 -
 target/nios2/cpu.c       | 10 ++++------
 target/nios2/op_helper.c | 19 -------------------
 target/nios2/translate.c | 14 +++++---------
 5 files changed, 9 insertions(+), 36 deletions(-)

Comments

Philippe Mathieu-Daudé Feb. 27, 2022, 10:05 p.m. UTC | #1 
On 27/2/22 19:21, Richard Henderson wrote:
> Drop irq_pending boolean.
> Drop helper_check_interrupts.
> Move checks for irq disabled into nios2_cpu_exec_interrupt.
> End the TB on writes to ienable, just like to status.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/nios2/cpu.h       |  1 -
>   target/nios2/helper.h    |  1 -
>   target/nios2/cpu.c       | 10 ++++------
>   target/nios2/op_helper.c | 19 -------------------
>   target/nios2/translate.c | 14 +++++---------
>   5 files changed, 9 insertions(+), 36 deletions(-)

LGTM but better have another review...

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Peter Maydell March 2, 2022, 1:06 p.m. UTC | #2 
On Sun, 27 Feb 2022 at 18:25, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Drop irq_pending boolean.

It wasn't a boolean, it was a uint32_t.

> Drop helper_check_interrupts.
> Move checks for irq disabled into nios2_cpu_exec_interrupt.
> End the TB on writes to ienable, just like to status.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

The code changes look good to me, but I think it would be
useful to have the commit message include a brief sketch
of how the new interrupt-handling design works and mention
that this means nios2 now works like the other targets.
At the moment the commit message is all "what" (which you can
get by looking at the patch) and no "why".

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM
diff mbox series

Patch

diff --git a/target/nios2/cpu.h b/target/nios2/cpu.h
index d2ba0c5bbd..a00e4229ce 100644
--- a/target/nios2/cpu.h
+++ b/target/nios2/cpu.h
@@ -160,7 +160,6 @@  struct CPUNios2State {
 
 #if !defined(CONFIG_USER_ONLY)
     Nios2MMU mmu;
-    uint32_t irq_pending;
 #endif
     int error_code;
 };
diff --git a/target/nios2/helper.h b/target/nios2/helper.h
index 21ef7f0791..a44ecfdf7a 100644
--- a/target/nios2/helper.h
+++ b/target/nios2/helper.h
@@ -24,5 +24,4 @@  DEF_HELPER_FLAGS_2(raise_exception, TCG_CALL_NO_WG, noreturn, env, i32)
 DEF_HELPER_2(mmu_write_tlbacc, void, env, i32)
 DEF_HELPER_2(mmu_write_tlbmisc, void, env, i32)
 DEF_HELPER_2(mmu_write_pteaddr, void, env, i32)
-DEF_HELPER_1(check_interrupts, void, env)
 #endif
diff --git a/target/nios2/cpu.c b/target/nios2/cpu.c
index 4cade61e93..6975ae4bdb 100644
--- a/target/nios2/cpu.c
+++ b/target/nios2/cpu.c
@@ -73,12 +73,9 @@  static void nios2_cpu_set_irq(void *opaque, int irq, int level)
 
     env->regs[CR_IPENDING] = deposit32(env->regs[CR_IPENDING], irq, 1, !!level);
 
-    env->irq_pending = env->regs[CR_IPENDING] & env->regs[CR_IENABLE];
-
-    if (env->irq_pending && (env->regs[CR_STATUS] & CR_STATUS_PIE)) {
-        env->irq_pending = 0;
+    if (env->regs[CR_IPENDING]) {
         cpu_interrupt(cs, CPU_INTERRUPT_HARD);
-    } else if (!env->irq_pending) {
+    } else {
         cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
     }
 }
@@ -134,7 +131,8 @@  static bool nios2_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
     CPUNios2State *env = &cpu->env;
 
     if ((interrupt_request & CPU_INTERRUPT_HARD) &&
-        (env->regs[CR_STATUS] & CR_STATUS_PIE)) {
+        (env->regs[CR_STATUS] & CR_STATUS_PIE) &&
+        (env->regs[CR_IPENDING] & env->regs[CR_IENABLE])) {
         cs->exception_index = EXCP_IRQ;
         nios2_cpu_do_interrupt(cs);
         return true;
diff --git a/target/nios2/op_helper.c b/target/nios2/op_helper.c
index d729379e4d..caa885f7b4 100644
--- a/target/nios2/op_helper.c
+++ b/target/nios2/op_helper.c
@@ -21,28 +21,9 @@ 
 #include "qemu/osdep.h"
 #include "cpu.h"
 #include "exec/helper-proto.h"
-#include "exec/cpu_ldst.h"
 #include "exec/exec-all.h"
 #include "qemu/main-loop.h"
 
-#if !defined(CONFIG_USER_ONLY)
-static void nios2_check_interrupts(CPUNios2State *env)
-{
-    if (env->irq_pending &&
-        (env->regs[CR_STATUS] & CR_STATUS_PIE)) {
-        env->irq_pending = 0;
-        cpu_interrupt(env_cpu(env), CPU_INTERRUPT_HARD);
-    }
-}
-
-void helper_check_interrupts(CPUNios2State *env)
-{
-    qemu_mutex_lock_iothread();
-    nios2_check_interrupts(env);
-    qemu_mutex_unlock_iothread();
-}
-#endif /* !CONFIG_USER_ONLY */
-
 void helper_raise_exception(CPUNios2State *env, uint32_t index)
 {
     CPUState *cs = env_cpu(env);
diff --git a/target/nios2/translate.c b/target/nios2/translate.c
index b17ce25a36..ce3aacf59d 100644
--- a/target/nios2/translate.c
+++ b/target/nios2/translate.c
@@ -489,19 +489,15 @@  static void wrctl(DisasContext *dc, uint32_t code, uint32_t flags)
     case CR_IPENDING:
         /* ipending is read only, writes ignored. */
         break;
+    case CR_STATUS:
+    case CR_IENABLE:
+        /* If interrupts were enabled using WRCTL, trigger them. */
+        dc->base.is_jmp = DISAS_UPDATE;
+        /* fall through */
     default:
         tcg_gen_mov_tl(cpu_R[instr.imm5 + CR_BASE], v);
         break;
     }
-
-    /* If interrupts were enabled using WRCTL, trigger them. */
-    if ((instr.imm5 + CR_BASE) == CR_STATUS) {
-        if (tb_cflags(dc->base.tb) & CF_USE_ICOUNT) {
-            gen_io_start();
-        }
-        gen_helper_check_interrupts(cpu_env);
-        dc->base.is_jmp = DISAS_UPDATE;
-    }
 #endif
 }