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

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

[Richard Henderson]

This fixes the problems that I pointed out with respect to the
existing Internal Interrupt Controller, and a few cleanups on the way.
It passes check-avocado, which is the only nios2 test I know of,
so more testing would be appreciated.


r~


Richard Henderson (7):
  target/nios2: Remove mmu_read_debug
  target/nios2: Replace MMU_LOG with tracepoints
  target/nios2: Only build mmu.c for system mode
  target/nios2: Hoist R_ZERO check in rdctl
  target/nios2: Split mmu_write
  target/nios2: Special case ipending in rdctl and wrctl
  target/nios2: Rewrite interrupt handling

 meson.build               |   1 +
 target/nios2/cpu.h        |   1 -
 target/nios2/helper.h     |   6 +-
 target/nios2/mmu.h        |   1 -
 target/nios2/cpu.c        |  10 +-
 target/nios2/mmu.c        | 265 ++++++++++++++------------------------
 target/nios2/op_helper.c  |  29 -----
 target/nios2/translate.c  |  71 +++++-----
 target/nios2/meson.build  |   3 +-
 target/nios2/trace-events |  10 ++
 10 files changed, 150 insertions(+), 247 deletions(-)
 create mode 100644 target/nios2/trace-events

-- 
2.25.1

[PATCH 1/7] target/nios2: Remove mmu_read_debug

[Richard Henderson]

This functionality can be had via plugins, if desired.
In the meantime, it is unused code.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/nios2/helper.h    |  1 -
 target/nios2/mmu.h       |  1 -
 target/nios2/mmu.c       | 20 --------------------
 target/nios2/op_helper.c |  5 -----
 target/nios2/translate.c | 17 -----------------
 5 files changed, 44 deletions(-)

diff --git a/target/nios2/helper.h b/target/nios2/helper.h
index 6c8f0b5b35..6d8eec1814 100644
--- a/target/nios2/helper.h
+++ b/target/nios2/helper.h
@@ -21,7 +21,6 @@
 DEF_HELPER_FLAGS_2(raise_exception, TCG_CALL_NO_WG, noreturn, env, i32)
 
 #if !defined(CONFIG_USER_ONLY)
-DEF_HELPER_2(mmu_read_debug, void, env, i32)
 DEF_HELPER_3(mmu_write, void, env, i32, i32)
 DEF_HELPER_1(check_interrupts, void, env)
 #endif
diff --git a/target/nios2/mmu.h b/target/nios2/mmu.h
index 4f46fbb82e..b7785b46c0 100644
--- a/target/nios2/mmu.h
+++ b/target/nios2/mmu.h
@@ -44,7 +44,6 @@ void mmu_flip_um(CPUNios2State *env, unsigned int um);
 unsigned int mmu_translate(CPUNios2State *env,
                            Nios2MMULookup *lu,
                            target_ulong vaddr, int rw, int mmu_idx);
-void mmu_read_debug(CPUNios2State *env, uint32_t rn);
 void mmu_write(CPUNios2State *env, uint32_t rn, uint32_t v);
 void mmu_init(CPUNios2State *env);
 
diff --git a/target/nios2/mmu.c b/target/nios2/mmu.c
index 2545c06761..5616c39d54 100644
--- a/target/nios2/mmu.c
+++ b/target/nios2/mmu.c
@@ -35,26 +35,6 @@
 #define MMU_LOG(x)
 #endif
 
-void mmu_read_debug(CPUNios2State *env, uint32_t rn)
-{
-    switch (rn) {
-    case CR_TLBACC:
-        MMU_LOG(qemu_log("TLBACC READ %08X\n", env->regs[rn]));
-        break;
-
-    case CR_TLBMISC:
-        MMU_LOG(qemu_log("TLBMISC READ %08X\n", env->regs[rn]));
-        break;
-
-    case CR_PTEADDR:
-        MMU_LOG(qemu_log("PTEADDR READ %08X\n", env->regs[rn]));
-        break;
-
-    default:
-        break;
-    }
-}
-
 /* rw - 0 = read, 1 = write, 2 = fetch.  */
 unsigned int mmu_translate(CPUNios2State *env,
                            Nios2MMULookup *lu,
diff --git a/target/nios2/op_helper.c b/target/nios2/op_helper.c
index a59003855a..61fc4dc903 100644
--- a/target/nios2/op_helper.c
+++ b/target/nios2/op_helper.c
@@ -26,11 +26,6 @@
 #include "qemu/main-loop.h"
 
 #if !defined(CONFIG_USER_ONLY)
-void helper_mmu_read_debug(CPUNios2State *env, uint32_t rn)
-{
-    mmu_read_debug(env, rn);
-}
-
 void helper_mmu_write(CPUNios2State *env, uint32_t rn, uint32_t v)
 {
     mmu_write(env, rn, v);
diff --git a/target/nios2/translate.c b/target/nios2/translate.c
index f9abc2fdd2..194c8ebafd 100644
--- a/target/nios2/translate.c
+++ b/target/nios2/translate.c
@@ -448,23 +448,6 @@ static void rdctl(DisasContext *dc, uint32_t code, uint32_t flags)
     gen_check_supervisor(dc);
 
     switch (instr.imm5 + CR_BASE) {
-    case CR_PTEADDR:
-    case CR_TLBACC:
-    case CR_TLBMISC:
-    {
-#if !defined(CONFIG_USER_ONLY)
-        if (likely(instr.c != R_ZERO)) {
-            tcg_gen_mov_tl(cpu_R[instr.c], cpu_R[instr.imm5 + CR_BASE]);
-#ifdef DEBUG_MMU
-            TCGv_i32 tmp = tcg_const_i32(instr.imm5 + CR_BASE);
-            gen_helper_mmu_read_debug(cpu_R[instr.c], cpu_env, tmp);
-            tcg_temp_free_i32(tmp);
-#endif
-        }
-#endif
-        break;
-    }
-
     default:
         if (likely(instr.c != R_ZERO)) {
             tcg_gen_mov_tl(cpu_R[instr.c], cpu_R[instr.imm5 + CR_BASE]);
-- 
2.25.1

[PATCH 2/7] target/nios2: Replace MMU_LOG with tracepoints

[Richard Henderson]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 meson.build               |  1 +
 target/nios2/mmu.c        | 96 ++++++++++++---------------------------
 target/nios2/trace-events | 10 ++++
 3 files changed, 39 insertions(+), 68 deletions(-)
 create mode 100644 target/nios2/trace-events

diff --git a/meson.build b/meson.build
index 8df40bfac4..98f167d782 100644
--- a/meson.build
+++ b/meson.build
@@ -2692,6 +2692,7 @@ if have_system or have_user
     'target/i386',
     'target/i386/kvm',
     'target/mips/tcg',
+    'target/nios2',
     'target/ppc',
     'target/riscv',
     'target/s390x',
diff --git a/target/nios2/mmu.c b/target/nios2/mmu.c
index 5616c39d54..306370f675 100644
--- a/target/nios2/mmu.c
+++ b/target/nios2/mmu.c
@@ -23,18 +23,10 @@
 #include "cpu.h"
 #include "exec/exec-all.h"
 #include "mmu.h"
+#include "trace/trace-target_nios2.h"
 
 #if !defined(CONFIG_USER_ONLY)
 
-/* Define this to enable MMU debug messages */
-/* #define DEBUG_MMU */
-
-#ifdef DEBUG_MMU
-#define MMU_LOG(x) x
-#else
-#define MMU_LOG(x)
-#endif
-
 /* rw - 0 = read, 1 = write, 2 = fetch.  */
 unsigned int mmu_translate(CPUNios2State *env,
                            Nios2MMULookup *lu,
@@ -43,37 +35,26 @@ unsigned int mmu_translate(CPUNios2State *env,
     Nios2CPU *cpu = env_archcpu(env);
     int pid = (env->mmu.tlbmisc_wr & CR_TLBMISC_PID_MASK) >> 4;
     int vpn = vaddr >> 12;
+    int way, n_ways = cpu->tlb_num_ways;
 
-    MMU_LOG(qemu_log("mmu_translate vaddr %08X, pid %08X, vpn %08X\n",
-                     vaddr, pid, vpn));
-
-    int way;
-    for (way = 0; way < cpu->tlb_num_ways; way++) {
-
-        Nios2TLBEntry *entry =
-            &env->mmu.tlb[(way * cpu->tlb_num_ways) +
-                          (vpn & env->mmu.tlb_entry_mask)];
-
-        MMU_LOG(qemu_log("TLB[%d] TAG %08X, VPN %08X\n",
-                         (way * cpu->tlb_num_ways) +
-                         (vpn & env->mmu.tlb_entry_mask),
-                         entry->tag, (entry->tag >> 12)));
+    for (way = 0; way < n_ways; way++) {
+        uint32_t index = (way * n_ways) + (vpn & env->mmu.tlb_entry_mask);
+        Nios2TLBEntry *entry = &env->mmu.tlb[index];
 
         if (((entry->tag >> 12) != vpn) ||
             (((entry->tag & (1 << 11)) == 0) &&
             ((entry->tag & ((1 << cpu->pid_num_bits) - 1)) != pid))) {
+            trace_nios2_mmu_translate_miss(vaddr, pid, index, entry->tag);
             continue;
         }
+
         lu->vaddr = vaddr & TARGET_PAGE_MASK;
         lu->paddr = (entry->data & CR_TLBACC_PFN_MASK) << TARGET_PAGE_BITS;
         lu->prot = ((entry->data & CR_TLBACC_R) ? PAGE_READ : 0) |
                    ((entry->data & CR_TLBACC_W) ? PAGE_WRITE : 0) |
                    ((entry->data & CR_TLBACC_X) ? PAGE_EXEC : 0);
 
-        MMU_LOG(qemu_log("HIT TLB[%d] %08X %08X %08X\n",
-                         (way * cpu->tlb_num_ways) +
-                         (vpn & env->mmu.tlb_entry_mask),
-                         lu->vaddr, lu->paddr, lu->prot));
+        trace_nios2_mmu_translate_hit(vaddr, pid, index, lu->paddr, lu->prot);
         return 1;
     }
     return 0;
@@ -84,21 +65,18 @@ static void mmu_flush_pid(CPUNios2State *env, uint32_t pid)
     CPUState *cs = env_cpu(env);
     Nios2CPU *cpu = env_archcpu(env);
     int idx;
-    MMU_LOG(qemu_log("TLB Flush PID %d\n", pid));
 
     for (idx = 0; idx < cpu->tlb_num_entries; idx++) {
         Nios2TLBEntry *entry = &env->mmu.tlb[idx];
 
-        MMU_LOG(qemu_log("TLB[%d] => %08X %08X\n",
-                         idx, entry->tag, entry->data));
-
         if ((entry->tag & (1 << 10)) && (!(entry->tag & (1 << 11))) &&
             ((entry->tag & ((1 << cpu->pid_num_bits) - 1)) == pid)) {
             uint32_t vaddr = entry->tag & TARGET_PAGE_MASK;
 
-            MMU_LOG(qemu_log("TLB Flush Page %08X\n", vaddr));
-
+            trace_nios2_mmu_flush_pid_hit(pid, idx, vaddr);
             tlb_flush_page(cs, vaddr);
+        } else {
+            trace_nios2_mmu_flush_pid_miss(pid, idx, entry->tag);
         }
     }
 }
@@ -108,18 +86,15 @@ void mmu_write(CPUNios2State *env, uint32_t rn, uint32_t v)
     CPUState *cs = env_cpu(env);
     Nios2CPU *cpu = env_archcpu(env);
 
-    MMU_LOG(qemu_log("mmu_write %08X = %08X\n", rn, v));
-
     switch (rn) {
     case CR_TLBACC:
-        MMU_LOG(qemu_log("TLBACC: IG %02X, FLAGS %c%c%c%c%c, PFN %05X\n",
-                         v >> CR_TLBACC_IGN_SHIFT,
-                         (v & CR_TLBACC_C) ? 'C' : '.',
-                         (v & CR_TLBACC_R) ? 'R' : '.',
-                         (v & CR_TLBACC_W) ? 'W' : '.',
-                         (v & CR_TLBACC_X) ? 'X' : '.',
-                         (v & CR_TLBACC_G) ? 'G' : '.',
-                         v & CR_TLBACC_PFN_MASK));
+        trace_nios2_mmu_write_tlbacc(v >> CR_TLBACC_IGN_SHIFT,
+                                     (v & CR_TLBACC_C) ? 'C' : '.',
+                                     (v & CR_TLBACC_R) ? 'R' : '.',
+                                     (v & CR_TLBACC_W) ? 'W' : '.',
+                                     (v & CR_TLBACC_X) ? 'X' : '.',
+                                     (v & CR_TLBACC_G) ? 'G' : '.',
+                                     v & CR_TLBACC_PFN_MASK);
 
         /* if tlbmisc.WE == 1 then trigger a TLB write on writes to TLBACC */
         if (env->regs[CR_TLBMISC] & CR_TLBMISC_WR) {
@@ -138,16 +113,10 @@ void mmu_write(CPUNios2State *env, uint32_t rn, uint32_t v)
             if ((entry->tag != newTag) || (entry->data != newData)) {
                 if (entry->tag & (1 << 10)) {
                     /* Flush existing entry */
-                    MMU_LOG(qemu_log("TLB Flush Page (OLD) %08X\n",
-                                     entry->tag & TARGET_PAGE_MASK));
                     tlb_flush_page(cs, entry->tag & TARGET_PAGE_MASK);
                 }
                 entry->tag = newTag;
                 entry->data = newData;
-                MMU_LOG(qemu_log("TLB[%d] = %08X %08X\n",
-                                 (way * cpu->tlb_num_ways) +
-                                 (vpn & env->mmu.tlb_entry_mask),
-                                 entry->tag, entry->data));
             }
             /* Auto-increment tlbmisc.WAY */
             env->regs[CR_TLBMISC] =
@@ -161,15 +130,14 @@ void mmu_write(CPUNios2State *env, uint32_t rn, uint32_t v)
         break;
 
     case CR_TLBMISC:
-        MMU_LOG(qemu_log("TLBMISC: WAY %X, FLAGS %c%c%c%c%c%c, PID %04X\n",
-                         v >> CR_TLBMISC_WAY_SHIFT,
-                         (v & CR_TLBMISC_RD) ? 'R' : '.',
-                         (v & CR_TLBMISC_WR) ? 'W' : '.',
-                         (v & CR_TLBMISC_DBL) ? '2' : '.',
-                         (v & CR_TLBMISC_BAD) ? 'B' : '.',
-                         (v & CR_TLBMISC_PERM) ? 'P' : '.',
-                         (v & CR_TLBMISC_D) ? 'D' : '.',
-                         (v & CR_TLBMISC_PID_MASK) >> 4));
+        trace_nios2_mmu_write_tlbmisc(v >> CR_TLBMISC_WAY_SHIFT,
+                                      (v & CR_TLBMISC_RD) ? 'R' : '.',
+                                      (v & CR_TLBMISC_WR) ? 'W' : '.',
+                                      (v & CR_TLBMISC_DBL) ? '2' : '.',
+                                      (v & CR_TLBMISC_BAD) ? 'B' : '.',
+                                      (v & CR_TLBMISC_PERM) ? 'P' : '.',
+                                      (v & CR_TLBMISC_D) ? 'D' : '.',
+                                      (v & CR_TLBMISC_PID_MASK) >> 4);
 
         if ((v & CR_TLBMISC_PID_MASK) !=
             (env->mmu.tlbmisc_wr & CR_TLBMISC_PID_MASK)) {
@@ -193,11 +161,6 @@ void mmu_write(CPUNios2State *env, uint32_t rn, uint32_t v)
                  CR_TLBMISC_PID_SHIFT);
             env->regs[CR_PTEADDR] &= ~CR_PTEADDR_VPN_MASK;
             env->regs[CR_PTEADDR] |= (entry->tag >> 12) << CR_PTEADDR_VPN_SHIFT;
-            MMU_LOG(qemu_log("TLB READ way %d, vpn %05X, tag %08X, data %08X, "
-                             "tlbacc %08X, tlbmisc %08X, pteaddr %08X\n",
-                             way, vpn, entry->tag, entry->data,
-                             env->regs[CR_TLBACC], env->regs[CR_TLBMISC],
-                             env->regs[CR_PTEADDR]));
         } else {
             env->regs[CR_TLBMISC] = v;
         }
@@ -206,9 +169,8 @@ void mmu_write(CPUNios2State *env, uint32_t rn, uint32_t v)
         break;
 
     case CR_PTEADDR:
-        MMU_LOG(qemu_log("PTEADDR: PTBASE %03X, VPN %05X\n",
-                         v >> CR_PTEADDR_PTBASE_SHIFT,
-                         (v & CR_PTEADDR_VPN_MASK) >> CR_PTEADDR_VPN_SHIFT));
+        trace_nios2_mmu_write_pteaddr(v >> CR_PTEADDR_PTBASE_SHIFT,
+                                      (v & CR_PTEADDR_VPN_MASK) >> CR_PTEADDR_VPN_SHIFT);
 
         /* Writes to PTEADDR don't change the read-back VPN value */
         env->regs[CR_PTEADDR] = (v & ~CR_PTEADDR_VPN_MASK) |
@@ -226,8 +188,6 @@ void mmu_init(CPUNios2State *env)
     Nios2CPU *cpu = env_archcpu(env);
     Nios2MMU *mmu = &env->mmu;
 
-    MMU_LOG(qemu_log("mmu_init\n"));
-
     mmu->tlb_entry_mask = (cpu->tlb_num_entries / cpu->tlb_num_ways) - 1;
     mmu->tlb = g_new0(Nios2TLBEntry, cpu->tlb_num_entries);
 }
diff --git a/target/nios2/trace-events b/target/nios2/trace-events
new file mode 100644
index 0000000000..07f1f0a5e7
--- /dev/null
+++ b/target/nios2/trace-events
@@ -0,0 +1,10 @@
+# mmu.c
+nios2_mmu_translate_miss(uint32_t vaddr, uint32_t pid, uint32_t index, uint32_t tag) "mmu_translate: MISS vaddr=0x%08x pid=%u TLB[%u] tag=0x%08x"
+nios2_mmu_translate_hit(uint32_t vaddr, uint32_t pid, uint32_t index, uint32_t paddr, uint32_t prot) "mmu_translate: HIT vaddr=0x%08x pid=%u TLB[%u] paddr=0x%08x prot=0x%x"
+
+nios2_mmu_flush_pid_miss(uint32_t pid, uint32_t index, uint32_t vaddr) "mmu_flush: MISS pid=%u TLB[%u] tag=0x%08x"
+nios2_mmu_flush_pid_hit(uint32_t pid, uint32_t index, uint32_t vaddr) "mmu_flush: HIT pid=%u TLB[%u] vaddr=0x%08x"
+
+nios2_mmu_write_tlbacc(uint32_t ig, char c, char r, char w, char x, char g, uint32_t pfn) "mmu_write_tlbacc: ig=0x%02x flags=%c%c%c%c%c pfn=0x%08x"
+nios2_mmu_write_tlbmisc(uint32_t way, char r, char w, char t, char b, char p, char d, uint32_t pid) "mmu_write_tlbmisc: way=0x%x flags=%c%c%c%c%c%c pid=%u"
+nios2_mmu_write_pteaddr(uint32_t ptb, uint32_t vpn) "mmu_write_pteaddr: ptbase=0x%03x vpn=0x%05x"
-- 
2.25.1

[PATCH 3/7] target/nios2: Only build mmu.c for system mode

[Richard Henderson]

We can thus remove an ifdef covering the entire file.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/nios2/mmu.c       | 3 ---
 target/nios2/meson.build | 3 +--
 2 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/target/nios2/mmu.c b/target/nios2/mmu.c
index 306370f675..437fad09b7 100644
--- a/target/nios2/mmu.c
+++ b/target/nios2/mmu.c
@@ -25,7 +25,6 @@
 #include "mmu.h"
 #include "trace/trace-target_nios2.h"
 
-#if !defined(CONFIG_USER_ONLY)
 
 /* rw - 0 = read, 1 = write, 2 = fetch.  */
 unsigned int mmu_translate(CPUNios2State *env,
@@ -217,5 +216,3 @@ void dump_mmu(CPUNios2State *env)
                     (entry->data & CR_TLBACC_X) ? 'X' : '-');
     }
 }
-
-#endif /* !CONFIG_USER_ONLY */
diff --git a/target/nios2/meson.build b/target/nios2/meson.build
index e643917db1..62b384702d 100644
--- a/target/nios2/meson.build
+++ b/target/nios2/meson.build
@@ -2,14 +2,13 @@ nios2_ss = ss.source_set()
 nios2_ss.add(files(
   'cpu.c',
   'helper.c',
-  'mmu.c',
   'nios2-semi.c',
   'op_helper.c',
   'translate.c',
 ))
 
 nios2_softmmu_ss = ss.source_set()
-nios2_softmmu_ss.add(files('monitor.c'))
+nios2_softmmu_ss.add(files('monitor.c', 'mmu.c'))
 
 target_arch += {'nios2': nios2_ss}
 target_softmmu_arch += {'nios2': nios2_softmmu_ss}
-- 
2.25.1

[PATCH 4/7] target/nios2: Hoist R_ZERO check in rdctl

[Richard Henderson]

This will avoid having to replicate the check to additional cases.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/nios2/translate.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/target/nios2/translate.c b/target/nios2/translate.c
index 194c8ebafd..fa355308a9 100644
--- a/target/nios2/translate.c
+++ b/target/nios2/translate.c
@@ -447,11 +447,13 @@ static void rdctl(DisasContext *dc, uint32_t code, uint32_t flags)
 
     gen_check_supervisor(dc);
 
+    if (unlikely(instr.c == R_ZERO)) {
+        return;
+    }
+
     switch (instr.imm5 + CR_BASE) {
     default:
-        if (likely(instr.c != R_ZERO)) {
-            tcg_gen_mov_tl(cpu_R[instr.c], cpu_R[instr.imm5 + CR_BASE]);
-        }
+        tcg_gen_mov_tl(cpu_R[instr.c], cpu_R[instr.imm5 + CR_BASE]);
         break;
     }
 }
-- 
2.25.1

[PATCH 5/7] target/nios2: Split mmu_write

[Richard Henderson]

Create three separate functions for the three separate registers.
Avoid extra dispatch through op_helper.c.
Dispatch to the correct function in translation.
Clean up the ifdefs in wrctl.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/nios2/helper.h    |   4 +-
 target/nios2/mmu.c       | 180 +++++++++++++++++++--------------------
 target/nios2/op_helper.c |   5 --
 target/nios2/translate.c |  26 +++---
 4 files changed, 104 insertions(+), 111 deletions(-)

diff --git a/target/nios2/helper.h b/target/nios2/helper.h
index 6d8eec1814..21ef7f0791 100644
--- a/target/nios2/helper.h
+++ b/target/nios2/helper.h
@@ -21,6 +21,8 @@
 DEF_HELPER_FLAGS_2(raise_exception, TCG_CALL_NO_WG, noreturn, env, i32)
 
 #if !defined(CONFIG_USER_ONLY)
-DEF_HELPER_3(mmu_write, void, env, i32, 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/mmu.c b/target/nios2/mmu.c
index 437fad09b7..4daab2a7ab 100644
--- a/target/nios2/mmu.c
+++ b/target/nios2/mmu.c
@@ -23,6 +23,7 @@
 #include "cpu.h"
 #include "exec/exec-all.h"
 #include "mmu.h"
+#include "exec/helper-proto.h"
 #include "trace/trace-target_nios2.h"
 
 
@@ -80,106 +81,103 @@ static void mmu_flush_pid(CPUNios2State *env, uint32_t pid)
     }
 }
 
-void mmu_write(CPUNios2State *env, uint32_t rn, uint32_t v)
+void helper_mmu_write_tlbacc(CPUNios2State *env, uint32_t v)
 {
     CPUState *cs = env_cpu(env);
     Nios2CPU *cpu = env_archcpu(env);
 
-    switch (rn) {
-    case CR_TLBACC:
-        trace_nios2_mmu_write_tlbacc(v >> CR_TLBACC_IGN_SHIFT,
-                                     (v & CR_TLBACC_C) ? 'C' : '.',
-                                     (v & CR_TLBACC_R) ? 'R' : '.',
-                                     (v & CR_TLBACC_W) ? 'W' : '.',
-                                     (v & CR_TLBACC_X) ? 'X' : '.',
-                                     (v & CR_TLBACC_G) ? 'G' : '.',
-                                     v & CR_TLBACC_PFN_MASK);
+    trace_nios2_mmu_write_tlbacc(v >> CR_TLBACC_IGN_SHIFT,
+                                 (v & CR_TLBACC_C) ? 'C' : '.',
+                                 (v & CR_TLBACC_R) ? 'R' : '.',
+                                 (v & CR_TLBACC_W) ? 'W' : '.',
+                                 (v & CR_TLBACC_X) ? 'X' : '.',
+                                 (v & CR_TLBACC_G) ? 'G' : '.',
+                                 v & CR_TLBACC_PFN_MASK);
 
-        /* if tlbmisc.WE == 1 then trigger a TLB write on writes to TLBACC */
-        if (env->regs[CR_TLBMISC] & CR_TLBMISC_WR) {
-            int way = (env->regs[CR_TLBMISC] >> CR_TLBMISC_WAY_SHIFT);
-            int vpn = (env->mmu.pteaddr_wr & CR_PTEADDR_VPN_MASK) >> 2;
-            int pid = (env->mmu.tlbmisc_wr & CR_TLBMISC_PID_MASK) >> 4;
-            int g = (v & CR_TLBACC_G) ? 1 : 0;
-            int valid = ((vpn & CR_TLBACC_PFN_MASK) < 0xC0000) ? 1 : 0;
-            Nios2TLBEntry *entry =
-                &env->mmu.tlb[(way * cpu->tlb_num_ways) +
-                              (vpn & env->mmu.tlb_entry_mask)];
-            uint32_t newTag = (vpn << 12) | (g << 11) | (valid << 10) | pid;
-            uint32_t newData = v & (CR_TLBACC_C | CR_TLBACC_R | CR_TLBACC_W |
-                                    CR_TLBACC_X | CR_TLBACC_PFN_MASK);
+    /* if tlbmisc.WE == 1 then trigger a TLB write on writes to TLBACC */
+    if (env->regs[CR_TLBMISC] & CR_TLBMISC_WR) {
+        int way = (env->regs[CR_TLBMISC] >> CR_TLBMISC_WAY_SHIFT);
+        int vpn = (env->mmu.pteaddr_wr & CR_PTEADDR_VPN_MASK) >> 2;
+        int pid = (env->mmu.tlbmisc_wr & CR_TLBMISC_PID_MASK) >> 4;
+        int g = (v & CR_TLBACC_G) ? 1 : 0;
+        int valid = ((vpn & CR_TLBACC_PFN_MASK) < 0xC0000) ? 1 : 0;
+        Nios2TLBEntry *entry =
+            &env->mmu.tlb[(way * cpu->tlb_num_ways) +
+                          (vpn & env->mmu.tlb_entry_mask)];
+        uint32_t newTag = (vpn << 12) | (g << 11) | (valid << 10) | pid;
+        uint32_t newData = v & (CR_TLBACC_C | CR_TLBACC_R | CR_TLBACC_W |
+                                CR_TLBACC_X | CR_TLBACC_PFN_MASK);
 
-            if ((entry->tag != newTag) || (entry->data != newData)) {
-                if (entry->tag & (1 << 10)) {
-                    /* Flush existing entry */
-                    tlb_flush_page(cs, entry->tag & TARGET_PAGE_MASK);
-                }
-                entry->tag = newTag;
-                entry->data = newData;
+        if ((entry->tag != newTag) || (entry->data != newData)) {
+            if (entry->tag & (1 << 10)) {
+                /* Flush existing entry */
+                tlb_flush_page(cs, entry->tag & TARGET_PAGE_MASK);
             }
-            /* Auto-increment tlbmisc.WAY */
-            env->regs[CR_TLBMISC] =
-                (env->regs[CR_TLBMISC] & ~CR_TLBMISC_WAY_MASK) |
-                (((way + 1) & (cpu->tlb_num_ways - 1)) <<
-                 CR_TLBMISC_WAY_SHIFT);
+            entry->tag = newTag;
+            entry->data = newData;
         }
-
-        /* Writes to TLBACC don't change the read-back value */
-        env->mmu.tlbacc_wr = v;
-        break;
-
-    case CR_TLBMISC:
-        trace_nios2_mmu_write_tlbmisc(v >> CR_TLBMISC_WAY_SHIFT,
-                                      (v & CR_TLBMISC_RD) ? 'R' : '.',
-                                      (v & CR_TLBMISC_WR) ? 'W' : '.',
-                                      (v & CR_TLBMISC_DBL) ? '2' : '.',
-                                      (v & CR_TLBMISC_BAD) ? 'B' : '.',
-                                      (v & CR_TLBMISC_PERM) ? 'P' : '.',
-                                      (v & CR_TLBMISC_D) ? 'D' : '.',
-                                      (v & CR_TLBMISC_PID_MASK) >> 4);
-
-        if ((v & CR_TLBMISC_PID_MASK) !=
-            (env->mmu.tlbmisc_wr & CR_TLBMISC_PID_MASK)) {
-            mmu_flush_pid(env, (env->mmu.tlbmisc_wr & CR_TLBMISC_PID_MASK) >>
-                               CR_TLBMISC_PID_SHIFT);
-        }
-        /* if tlbmisc.RD == 1 then trigger a TLB read on writes to TLBMISC */
-        if (v & CR_TLBMISC_RD) {
-            int way = (v >> CR_TLBMISC_WAY_SHIFT);
-            int vpn = (env->mmu.pteaddr_wr & CR_PTEADDR_VPN_MASK) >> 2;
-            Nios2TLBEntry *entry =
-                &env->mmu.tlb[(way * cpu->tlb_num_ways) +
-                              (vpn & env->mmu.tlb_entry_mask)];
-
-            env->regs[CR_TLBACC] &= CR_TLBACC_IGN_MASK;
-            env->regs[CR_TLBACC] |= entry->data;
-            env->regs[CR_TLBACC] |= (entry->tag & (1 << 11)) ? CR_TLBACC_G : 0;
-            env->regs[CR_TLBMISC] =
-                (v & ~CR_TLBMISC_PID_MASK) |
-                ((entry->tag & ((1 << cpu->pid_num_bits) - 1)) <<
-                 CR_TLBMISC_PID_SHIFT);
-            env->regs[CR_PTEADDR] &= ~CR_PTEADDR_VPN_MASK;
-            env->regs[CR_PTEADDR] |= (entry->tag >> 12) << CR_PTEADDR_VPN_SHIFT;
-        } else {
-            env->regs[CR_TLBMISC] = v;
-        }
-
-        env->mmu.tlbmisc_wr = v;
-        break;
-
-    case CR_PTEADDR:
-        trace_nios2_mmu_write_pteaddr(v >> CR_PTEADDR_PTBASE_SHIFT,
-                                      (v & CR_PTEADDR_VPN_MASK) >> CR_PTEADDR_VPN_SHIFT);
-
-        /* Writes to PTEADDR don't change the read-back VPN value */
-        env->regs[CR_PTEADDR] = (v & ~CR_PTEADDR_VPN_MASK) |
-                                (env->regs[CR_PTEADDR] & CR_PTEADDR_VPN_MASK);
-        env->mmu.pteaddr_wr = v;
-        break;
-
-    default:
-        break;
+        /* Auto-increment tlbmisc.WAY */
+        env->regs[CR_TLBMISC] =
+            (env->regs[CR_TLBMISC] & ~CR_TLBMISC_WAY_MASK) |
+            (((way + 1) & (cpu->tlb_num_ways - 1)) <<
+             CR_TLBMISC_WAY_SHIFT);
     }
+
+    /* Writes to TLBACC don't change the read-back value */
+    env->mmu.tlbacc_wr = v;
+}
+
+void helper_mmu_write_tlbmisc(CPUNios2State *env, uint32_t v)
+{
+    Nios2CPU *cpu = env_archcpu(env);
+
+    trace_nios2_mmu_write_tlbmisc(v >> CR_TLBMISC_WAY_SHIFT,
+                                  (v & CR_TLBMISC_RD) ? 'R' : '.',
+                                  (v & CR_TLBMISC_WR) ? 'W' : '.',
+                                  (v & CR_TLBMISC_DBL) ? '2' : '.',
+                                  (v & CR_TLBMISC_BAD) ? 'B' : '.',
+                                  (v & CR_TLBMISC_PERM) ? 'P' : '.',
+                                  (v & CR_TLBMISC_D) ? 'D' : '.',
+                                  (v & CR_TLBMISC_PID_MASK) >> 4);
+
+    if ((v & CR_TLBMISC_PID_MASK) !=
+        (env->mmu.tlbmisc_wr & CR_TLBMISC_PID_MASK)) {
+        mmu_flush_pid(env, (env->mmu.tlbmisc_wr & CR_TLBMISC_PID_MASK) >>
+                           CR_TLBMISC_PID_SHIFT);
+    }
+    /* if tlbmisc.RD == 1 then trigger a TLB read on writes to TLBMISC */
+    if (v & CR_TLBMISC_RD) {
+        int way = (v >> CR_TLBMISC_WAY_SHIFT);
+        int vpn = (env->mmu.pteaddr_wr & CR_PTEADDR_VPN_MASK) >> 2;
+        Nios2TLBEntry *entry =
+            &env->mmu.tlb[(way * cpu->tlb_num_ways) +
+                          (vpn & env->mmu.tlb_entry_mask)];
+
+        env->regs[CR_TLBACC] &= CR_TLBACC_IGN_MASK;
+        env->regs[CR_TLBACC] |= entry->data;
+        env->regs[CR_TLBACC] |= (entry->tag & (1 << 11)) ? CR_TLBACC_G : 0;
+        env->regs[CR_TLBMISC] =
+            (v & ~CR_TLBMISC_PID_MASK) |
+            ((entry->tag & ((1 << cpu->pid_num_bits) - 1)) <<
+             CR_TLBMISC_PID_SHIFT);
+        env->regs[CR_PTEADDR] &= ~CR_PTEADDR_VPN_MASK;
+        env->regs[CR_PTEADDR] |= (entry->tag >> 12) << CR_PTEADDR_VPN_SHIFT;
+    } else {
+        env->regs[CR_TLBMISC] = v;
+    }
+
+    env->mmu.tlbmisc_wr = v;
+}
+
+void helper_mmu_write_pteaddr(CPUNios2State *env, uint32_t v)
+{
+    trace_nios2_mmu_write_pteaddr(v >> CR_PTEADDR_PTBASE_SHIFT,
+                                  (v & CR_PTEADDR_VPN_MASK) >> CR_PTEADDR_VPN_SHIFT);
+
+    /* Writes to PTEADDR don't change the read-back VPN value */
+    env->regs[CR_PTEADDR] = (v & ~CR_PTEADDR_VPN_MASK) |
+                            (env->regs[CR_PTEADDR] & CR_PTEADDR_VPN_MASK);
+    env->mmu.pteaddr_wr = v;
 }
 
 void mmu_init(CPUNios2State *env)
diff --git a/target/nios2/op_helper.c b/target/nios2/op_helper.c
index 61fc4dc903..d729379e4d 100644
--- a/target/nios2/op_helper.c
+++ b/target/nios2/op_helper.c
@@ -26,11 +26,6 @@
 #include "qemu/main-loop.h"
 
 #if !defined(CONFIG_USER_ONLY)
-void helper_mmu_write(CPUNios2State *env, uint32_t rn, uint32_t v)
-{
-    mmu_write(env, rn, v);
-}
-
 static void nios2_check_interrupts(CPUNios2State *env)
 {
     if (env->irq_pending &&
diff --git a/target/nios2/translate.c b/target/nios2/translate.c
index fa355308a9..52965ba17e 100644
--- a/target/nios2/translate.c
+++ b/target/nios2/translate.c
@@ -461,30 +461,28 @@ static void rdctl(DisasContext *dc, uint32_t code, uint32_t flags)
 /* ctlN <- rA */
 static void wrctl(DisasContext *dc, uint32_t code, uint32_t flags)
 {
-    R_TYPE(instr, code);
-
     gen_check_supervisor(dc);
 
+#ifndef CONFIG_USER_ONLY
+    R_TYPE(instr, code);
+    TCGv v = load_gpr(dc, instr.a);
+
     switch (instr.imm5 + CR_BASE) {
     case CR_PTEADDR:
-    case CR_TLBACC:
-    case CR_TLBMISC:
-    {
-#if !defined(CONFIG_USER_ONLY)
-        TCGv_i32 tmp = tcg_const_i32(instr.imm5 + CR_BASE);
-        gen_helper_mmu_write(cpu_env, tmp, load_gpr(dc, instr.a));
-        tcg_temp_free_i32(tmp);
-#endif
+        gen_helper_mmu_write_pteaddr(cpu_env, v);
+        break;
+    case CR_TLBACC:
+        gen_helper_mmu_write_tlbacc(cpu_env, v);
+        break;
+    case CR_TLBMISC:
+        gen_helper_mmu_write_tlbmisc(cpu_env, v);
         break;
-    }
-
     default:
-        tcg_gen_mov_tl(cpu_R[instr.imm5 + CR_BASE], load_gpr(dc, instr.a));
+        tcg_gen_mov_tl(cpu_R[instr.imm5 + CR_BASE], v);
         break;
     }
 
     /* If interrupts were enabled using WRCTL, trigger them. */
-#if !defined(CONFIG_USER_ONLY)
     if ((instr.imm5 + CR_BASE) == CR_STATUS) {
         if (tb_cflags(dc->base.tb) & CF_USE_ICOUNT) {
             gen_io_start();
-- 
2.25.1

[PATCH 6/7] target/nios2: Special case ipending in rdctl and wrctl

[Richard Henderson]

It was never correct to be able to write to ipending.
Until the rest of the irq code is tidied, the read of ipending
will generate an "unnecessary" mask.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/nios2/translate.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/target/nios2/translate.c b/target/nios2/translate.c
index 52965ba17e..b17ce25a36 100644
--- a/target/nios2/translate.c
+++ b/target/nios2/translate.c
@@ -452,6 +452,15 @@ static void rdctl(DisasContext *dc, uint32_t code, uint32_t flags)
     }
 
     switch (instr.imm5 + CR_BASE) {
+    case CR_IPENDING:
+        /*
+         * The value of the ipending register is synthetic.
+         * In hw, this is the AND of a set of hardware irq lines
+         * with the ienable register.  In qemu, we re-use the space
+         * of CR_IPENDING to store the set of irq lines.
+         */
+        tcg_gen_and_tl(cpu_R[instr.c], cpu_R[CR_IPENDING], cpu_R[CR_IENABLE]);
+        break;
     default:
         tcg_gen_mov_tl(cpu_R[instr.c], cpu_R[instr.imm5 + CR_BASE]);
         break;
@@ -477,6 +486,9 @@ static void wrctl(DisasContext *dc, uint32_t code, uint32_t flags)
     case CR_TLBMISC:
         gen_helper_mmu_write_tlbmisc(cpu_env, v);
         break;
+    case CR_IPENDING:
+        /* ipending is read only, writes ignored. */
+        break;
     default:
         tcg_gen_mov_tl(cpu_R[instr.imm5 + CR_BASE], v);
         break;
-- 
2.25.1

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

[Richard Henderson]

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(-)

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
 }
 
-- 
2.25.1

Re: [PATCH 2/7] target/nios2: Replace MMU_LOG with tracepoints

[Philippe Mathieu-Daudé]

On 27/2/22 19:21, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   meson.build               |  1 +
>   target/nios2/mmu.c        | 96 ++++++++++++---------------------------
>   target/nios2/trace-events | 10 ++++
>   3 files changed, 39 insertions(+), 68 deletions(-)
>   create mode 100644 target/nios2/trace-events

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Re: [PATCH 3/7] target/nios2: Only build mmu.c for system mode

[Philippe Mathieu-Daudé]

On 27/2/22 19:21, Richard Henderson wrote:
> We can thus remove an ifdef covering the entire file.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/nios2/mmu.c       | 3 ---
>   target/nios2/meson.build | 3 +--
>   2 files changed, 1 insertion(+), 5 deletions(-)

\o/

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Re: [PATCH 4/7] target/nios2: Hoist R_ZERO check in rdctl

[Philippe Mathieu-Daudé]

On 27/2/22 19:21, Richard Henderson wrote:
> This will avoid having to replicate the check to additional cases.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/nios2/translate.c | 8 +++++---
>   1 file changed, 5 insertions(+), 3 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

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

[Philippe Mathieu-Daudé]

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>

Re: [PATCH 6/7] target/nios2: Special case ipending in rdctl and wrctl

[Philippe Mathieu-Daudé]

On 27/2/22 19:21, Richard Henderson wrote:
> It was never correct to be able to write to ipending.
> Until the rest of the irq code is tidied, the read of ipending
> will generate an "unnecessary" mask.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/nios2/translate.c | 12 ++++++++++++
>   1 file changed, 12 insertions(+)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Re: [PATCH 1/7] target/nios2: Remove mmu_read_debug

[Peter Maydell]

On Sun, 27 Feb 2022 at 18:23, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> This functionality can be had via plugins, if desired.
> In the meantime, it is unused code.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/nios2/helper.h    |  1 -
>  target/nios2/mmu.h       |  1 -
>  target/nios2/mmu.c       | 20 --------------------
>  target/nios2/op_helper.c |  5 -----
>  target/nios2/translate.c | 17 -----------------
>  5 files changed, 44 deletions(-)

> --- a/target/nios2/translate.c
> +++ b/target/nios2/translate.c
> @@ -448,23 +448,6 @@ static void rdctl(DisasContext *dc, uint32_t code, uint32_t flags)
>      gen_check_supervisor(dc);
>
>      switch (instr.imm5 + CR_BASE) {
> -    case CR_PTEADDR:
> -    case CR_TLBACC:
> -    case CR_TLBMISC:
> -    {
> -#if !defined(CONFIG_USER_ONLY)
> -        if (likely(instr.c != R_ZERO)) {
> -            tcg_gen_mov_tl(cpu_R[instr.c], cpu_R[instr.imm5 + CR_BASE]);
> -#ifdef DEBUG_MMU
> -            TCGv_i32 tmp = tcg_const_i32(instr.imm5 + CR_BASE);
> -            gen_helper_mmu_read_debug(cpu_R[instr.c], cpu_env, tmp);
> -            tcg_temp_free_i32(tmp);
> -#endif
> -        }
> -#endif
> -        break;
> -    }
> -

For a moment I thought this was changing the behaviour of these
registers for linux-user, but gen_check_supervisor() means we
never execute the code generated here. (Side note: it would
be more sensible to stop generating code rather than generating
the "throw an exception" code and then the "write/read reg"
code as known-dead-code after it. But that's a different issue.)

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

thanks
-- PMM

Re: [PATCH 2/7] target/nios2: Replace MMU_LOG with tracepoints

[Peter Maydell]

On Sun, 27 Feb 2022 at 18:24, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  meson.build               |  1 +
>  target/nios2/mmu.c        | 96 ++++++++++++---------------------------
>  target/nios2/trace-events | 10 ++++
>  3 files changed, 39 insertions(+), 68 deletions(-)
>  create mode 100644 target/nios2/trace-events

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

thanks
-- PMM

Re: [PATCH 3/7] target/nios2: Only build mmu.c for system mode

[Peter Maydell]

On Sun, 27 Feb 2022 at 18:23, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> We can thus remove an ifdef covering the entire file.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/nios2/mmu.c       | 3 ---
>  target/nios2/meson.build | 3 +--

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

thanks
-- PMM

Re: [PATCH 4/7] target/nios2: Hoist R_ZERO check in rdctl

[Peter Maydell]

On Sun, 27 Feb 2022 at 18:25, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> This will avoid having to replicate the check to additional cases.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---


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

thanks
-- PMM

Re: [PATCH 5/7] target/nios2: Split mmu_write

[Peter Maydell]

On Sun, 27 Feb 2022 at 18:27, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Create three separate functions for the three separate registers.
> Avoid extra dispatch through op_helper.c.
> Dispatch to the correct function in translation.
> Clean up the ifdefs in wrctl.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/nios2/helper.h    |   4 +-
>  target/nios2/mmu.c       | 180 +++++++++++++++++++--------------------
>  target/nios2/op_helper.c |   5 --
>  target/nios2/translate.c |  26 +++---
>  4 files changed, 104 insertions(+), 111 deletions(-)

You forgot to delete the prototype for mmu_write() in mmu.h.
Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [PATCH 6/7] target/nios2: Special case ipending in rdctl and wrctl

[Peter Maydell]

On Sun, 27 Feb 2022 at 18:25, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> It was never correct to be able to write to ipending.
> Until the rest of the irq code is tidied, the read of ipending
> will generate an "unnecessary" mask.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/nios2/translate.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/target/nios2/translate.c b/target/nios2/translate.c
> index 52965ba17e..b17ce25a36 100644
> --- a/target/nios2/translate.c
> +++ b/target/nios2/translate.c
> @@ -452,6 +452,15 @@ static void rdctl(DisasContext *dc, uint32_t code, uint32_t flags)
>      }
>
>      switch (instr.imm5 + CR_BASE) {
> +    case CR_IPENDING:
> +        /*
> +         * The value of the ipending register is synthetic.
> +         * In hw, this is the AND of a set of hardware irq lines
> +         * with the ienable register.  In qemu, we re-use the space
> +         * of CR_IPENDING to store the set of irq lines.

maybe add
"and so we must perform the AND here, and anywhere else we need
the guest value of CR_IPENDING" ?

> +         */
> +        tcg_gen_and_tl(cpu_R[instr.c], cpu_R[CR_IPENDING], cpu_R[CR_IENABLE]);
> +        break;

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

thanks
-- PMM

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

[Peter Maydell]

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