[PATCH v3 0/5] target/nios2: Shadow register set, EIC and VIC

[PATCH v3 0/5] target/nios2: Shadow register set, EIC and VIC

[Amir Gonnen]

Based-on: 20220227182125.21809-1-richard.henderson@linaro.org
("target/nios2: Rewrite interrupt handling")

Implement nios2 Shadow register set, EIC and VIC.

Currently nios2 on QEMU contains an internal Interrupt Controller.
The nios2 architecture can support a more powerful External Interrupt
Controller (EIC) instead of the internal, and implements special cpu
features to support it: Shadow register set and External Interrupt
Controller Interface.

This patch series introduces the necessary changes to the nios2 cpu to
support an External Interrupt Controller, and includes a Vectored
Interrupt Controller (VIC) device that can be attached to the EIC.

Changes from v2
===============
- Rebase patchest on "target/nios2: Rewrite interrupt handling", which
  introduces fixes to nios2 interrupt handling
- Check supervisor on eret as a separate patch
- Check supervisor on rdprs and wrprs
- Use FIELD_EX32 and FIELD_DP32 to access IL, CRS and PRS
- Added a comment on helper_eret
- Compute rdprs and wrprs inline, without helper functions
- Check nios2_take_eic_irq on nios2_cpu_exec_interrupt
- Check regs[CR_IPENDING] instead of env->irq_pending
- Fix Kconfig (remove defaults and depends)
- Added URL to VIC documentaion on Intel website
- Removed LOG_VIC
- Added comments in nios_vic.c
- Report an error in case of write to invalid CSR address

Changes from v1
===============
- Splitted into several independant patches
- Added a board that wires up the VIC

Signed-off-by: Amir Gonnen

Amir Gonnen (5):
  target/nios2: Check supervisor on eret
  target/nios2: Shadow register set
  target/nios2: Exteral Interrupt Controller (EIC)
  hw/intc: Vectored Interrupt Controller (VIC)
  hw/nios2: Machine with a Vectored Interrupt Controller

 hw/intc/Kconfig           |   3 +
 hw/intc/meson.build       |   1 +
 hw/intc/nios2_vic.c       | 341 ++++++++++++++++++++++++++++++++++++++
 hw/nios2/10m50_devboard.c |  64 ++++++-
 hw/nios2/Kconfig          |   1 +
 target/nios2/cpu.c        |  55 ++++--
 target/nios2/cpu.h        |  69 +++++++-
 target/nios2/helper.c     |  33 +++-
 target/nios2/helper.h     |   1 +
 target/nios2/op_helper.c  |  18 ++
 target/nios2/translate.c  |  66 +++++++-
 11 files changed, 621 insertions(+), 31 deletions(-)
 create mode 100644 hw/intc/nios2_vic.c

-- 
2.25.1

[PATCH v3 1/5] target/nios2: Check supervisor on eret

[Amir Gonnen]

eret instruction is only allowed in supervisor mode.

Signed-off-by: Amir Gonnen <amir.gonnen@neuroblade.ai>
---
 target/nios2/translate.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/nios2/translate.c b/target/nios2/translate.c
index ce3aacf59d..007c17e6e9 100644
--- a/target/nios2/translate.c
+++ b/target/nios2/translate.c
@@ -384,6 +384,8 @@ static const Nios2Instruction i_type_instructions[] = {
  */
 static void eret(DisasContext *dc, uint32_t code, uint32_t flags)
 {
+    gen_check_supervisor(dc);
+
     tcg_gen_mov_tl(cpu_R[CR_STATUS], cpu_R[CR_ESTATUS]);
     tcg_gen_mov_tl(cpu_R[R_PC], cpu_R[R_EA]);
 
-- 
2.25.1

[PATCH v3 2/5] target/nios2: Shadow register set

[Amir Gonnen]

Implement shadow register set and related instructions
rdprs, wrprs. Fix eret to update either status or sstatus
according to current register set.
eret also changes register set when needed.

Signed-off-by: Amir Gonnen <amir.gonnen@neuroblade.ai>
---
 target/nios2/cpu.c       |  1 +
 target/nios2/cpu.h       | 48 +++++++++++++++++++++++++++---
 target/nios2/helper.h    |  1 +
 target/nios2/op_helper.c | 18 +++++++++++
 target/nios2/translate.c | 64 ++++++++++++++++++++++++++++++++++++----
 5 files changed, 123 insertions(+), 9 deletions(-)

diff --git a/target/nios2/cpu.c b/target/nios2/cpu.c
index 6975ae4bdb..026ee18b01 100644
--- a/target/nios2/cpu.c
+++ b/target/nios2/cpu.c
@@ -54,6 +54,7 @@ static void nios2_cpu_reset(DeviceState *dev)
     ncc->parent_reset(dev);
 
     memset(env->regs, 0, sizeof(uint32_t) * NUM_CORE_REGS);
+    memset(env->shadow_regs, 0, sizeof(uint32_t) * NUM_REG_SETS * NUM_GP_REGS);
     env->regs[R_PC] = cpu->reset_addr;
 
 #if defined(CONFIG_USER_ONLY)
diff --git a/target/nios2/cpu.h b/target/nios2/cpu.h
index a00e4229ce..dbb4c968df 100644
--- a/target/nios2/cpu.h
+++ b/target/nios2/cpu.h
@@ -23,6 +23,7 @@
 
 #include "exec/cpu-defs.h"
 #include "hw/core/cpu.h"
+#include "hw/registerfields.h"
 #include "qom/object.h"
 
 typedef struct CPUNios2State CPUNios2State;
@@ -57,9 +58,14 @@ struct Nios2CPUClass {
 #define EXCEPTION_ADDRESS     0x00000004
 #define FAST_TLB_MISS_ADDRESS 0x00000008
 
+#define NUM_GP_REGS 32
+#define NUM_CR_REGS 32
 
 /* GP regs + CR regs + PC */
-#define NUM_CORE_REGS (32 + 32 + 1)
+#define NUM_CORE_REGS (NUM_GP_REGS + NUM_CR_REGS + 1)
+
+/* 63 shadow register sets. 0 is the primary set */
+#define NUM_REG_SETS 64
 
 /* General purpose register aliases */
 #define R_ZERO   0
@@ -80,15 +86,15 @@ struct Nios2CPUClass {
 #define R_RA     31
 
 /* Control register aliases */
-#define CR_BASE  32
+#define CR_BASE  NUM_GP_REGS
 #define CR_STATUS    (CR_BASE + 0)
 #define   CR_STATUS_PIE  (1 << 0)
 #define   CR_STATUS_U    (1 << 1)
 #define   CR_STATUS_EH   (1 << 2)
 #define   CR_STATUS_IH   (1 << 3)
 #define   CR_STATUS_IL   (63 << 4)
-#define   CR_STATUS_CRS  (63 << 10)
-#define   CR_STATUS_PRS  (63 << 16)
+FIELD(CR_STATUS, CRS, 10, 6)
+FIELD(CR_STATUS, PRS, 16, 6)
 #define   CR_STATUS_NMI  (1 << 22)
 #define   CR_STATUS_RSIE (1 << 23)
 #define CR_ESTATUS   (CR_BASE + 1)
@@ -131,6 +137,7 @@ struct Nios2CPUClass {
 
 /* Other registers */
 #define R_PC         64
+#define R_SSTATUS    30
 
 /* Exceptions */
 #define EXCP_BREAK    0x1000
@@ -157,6 +164,7 @@ struct Nios2CPUClass {
 
 struct CPUNios2State {
     uint32_t regs[NUM_CORE_REGS];
+    uint32_t shadow_regs[NUM_REG_SETS][NUM_GP_REGS];
 
 #if !defined(CONFIG_USER_ONLY)
     Nios2MMU mmu;
@@ -245,4 +253,36 @@ static inline void cpu_get_tb_cpu_state(CPUNios2State *env, target_ulong *pc,
     *flags = (env->regs[CR_STATUS] & (CR_STATUS_EH | CR_STATUS_U));
 }
 
+static inline uint32_t cpu_get_crs(const CPUNios2State *env)
+{
+    return FIELD_EX32(env->regs[CR_STATUS], CR_STATUS, CRS);
+}
+
+static inline uint32_t cpu_get_prs(const CPUNios2State *env)
+{
+    return FIELD_EX32(env->regs[CR_STATUS], CR_STATUS, PRS);
+}
+
+static inline void cpu_change_reg_set(CPUNios2State *env, uint32_t prev_set,
+                                      uint32_t new_set)
+{
+    if (new_set == prev_set) {
+        return;
+    }
+    memcpy(env->shadow_regs[prev_set], env->regs,
+           sizeof(uint32_t) * NUM_GP_REGS);
+    memcpy(env->regs, env->shadow_regs[new_set],
+           sizeof(uint32_t) * NUM_GP_REGS);
+    env->regs[CR_STATUS] =
+        FIELD_DP32(env->regs[CR_STATUS], CR_STATUS, PRS, prev_set);
+    env->regs[CR_STATUS] =
+        FIELD_DP32(env->regs[CR_STATUS], CR_STATUS, CRS, new_set);
+}
+
+static inline void cpu_set_crs(CPUNios2State *env, uint32_t value)
+{
+    uint32_t crs = cpu_get_crs(env);
+    cpu_change_reg_set(env, crs, value);
+}
+
 #endif /* NIOS2_CPU_H */
diff --git a/target/nios2/helper.h b/target/nios2/helper.h
index a44ecfdf7a..2e400b1f12 100644
--- a/target/nios2/helper.h
+++ b/target/nios2/helper.h
@@ -18,6 +18,7 @@
  * <http://www.gnu.org/licenses/lgpl-2.1.html>
  */
 
+DEF_HELPER_2(eret, void, env, i32)
 DEF_HELPER_FLAGS_2(raise_exception, TCG_CALL_NO_WG, noreturn, env, i32)
 
 #if !defined(CONFIG_USER_ONLY)
diff --git a/target/nios2/op_helper.c b/target/nios2/op_helper.c
index caa885f7b4..c8ce399332 100644
--- a/target/nios2/op_helper.c
+++ b/target/nios2/op_helper.c
@@ -30,3 +30,21 @@ void helper_raise_exception(CPUNios2State *env, uint32_t index)
     cs->exception_index = index;
     cpu_loop_exit(cs);
 }
+
+void helper_eret(CPUNios2State *env, uint32_t new_pc)
+{
+    uint32_t crs = cpu_get_crs(env);
+    if (crs == 0) {
+        env->regs[CR_STATUS] = env->regs[CR_ESTATUS];
+    } else {
+        env->regs[CR_STATUS] = env->regs[R_SSTATUS];
+    }
+
+    /*
+     * At this point CRS was updated by the above assignment to CR_STATUS.
+     * Therefore we need to retrieve the new value of CRS and potentially
+     * switch the register set
+     */
+    cpu_change_reg_set(env, crs, cpu_get_crs(env));
+    env->regs[R_PC] = new_pc;
+}
diff --git a/target/nios2/translate.c b/target/nios2/translate.c
index 007c17e6e9..f1ac1bf126 100644
--- a/target/nios2/translate.c
+++ b/target/nios2/translate.c
@@ -205,6 +205,34 @@ static void call(DisasContext *dc, uint32_t code, uint32_t flags)
 /*
  * I-Type instructions
  */
+
+/*
+ * rB <- prs.rA + sigma(IMM16)
+ */
+static void rdprs(DisasContext *dc, uint32_t code, uint32_t flags)
+{
+    I_TYPE(instr, code);
+
+    gen_check_supervisor(dc);
+
+    TCGv_i32 t = tcg_temp_new_i32();
+    TCGv_ptr p = tcg_temp_new_ptr();
+
+    tcg_gen_extract_i32(t, cpu_R[CR_STATUS],
+                        R_CR_STATUS_PRS_SHIFT,
+                        R_CR_STATUS_PRS_LENGTH);
+    tcg_gen_muli_i32(t, t, sizeof(uint32_t) * NUM_GP_REGS);
+    tcg_gen_ext_i32_ptr(p, t);
+
+    tcg_gen_add_ptr(p, p, cpu_env);
+    tcg_gen_ld_i32(t, p, offsetof(CPUNios2State, shadow_regs)
+                    + sizeof(uint32_t) * instr.a);
+    tcg_gen_addi_i32(cpu_R[instr.b], t, instr.imm16.s);
+
+    tcg_temp_free_ptr(p);
+    tcg_temp_free_i32(t);
+}
+
 /* Load instructions */
 static void gen_ldx(DisasContext *dc, uint32_t code, uint32_t flags)
 {
@@ -365,7 +393,7 @@ static const Nios2Instruction i_type_instructions[] = {
     INSTRUCTION_FLG(gen_stx, MO_SL),                  /* stwio */
     INSTRUCTION_FLG(gen_bxx, TCG_COND_LTU),           /* bltu */
     INSTRUCTION_FLG(gen_ldx, MO_UL),                  /* ldwio */
-    INSTRUCTION_UNIMPLEMENTED(),                      /* rdprs */
+    INSTRUCTION(rdprs),                               /* rdprs */
     INSTRUCTION_ILLEGAL(),
     INSTRUCTION_FLG(handle_r_type_instr, 0),          /* R-Type */
     INSTRUCTION_NOP(),                                /* flushd */
@@ -378,16 +406,42 @@ static const Nios2Instruction i_type_instructions[] = {
 /*
  * R-Type instructions
  */
+
+/*
+ * prs.rC <- rA
+ */
+static void wrprs(DisasContext *dc, uint32_t code, uint32_t flags)
+{
+    R_TYPE(instr, code);
+
+    gen_check_supervisor(dc);
+
+    TCGv_i32 t = tcg_temp_new_i32();
+    TCGv_ptr p = tcg_temp_new_ptr();
+
+    tcg_gen_extract_i32(t, cpu_R[CR_STATUS],
+                        R_CR_STATUS_PRS_SHIFT,
+                        R_CR_STATUS_PRS_LENGTH);
+    tcg_gen_muli_i32(t, t, sizeof(uint32_t) * NUM_GP_REGS);
+    tcg_gen_ext_i32_ptr(p, t);
+
+    tcg_gen_add_ptr(p, p, cpu_env);
+    tcg_gen_st_i32(cpu_R[instr.a], p, offsetof(CPUNios2State, shadow_regs)
+                   + sizeof(uint32_t) * instr.c);
+
+    tcg_temp_free_ptr(p);
+    tcg_temp_free_i32(t);
+}
+
 /*
- * status <- estatus
+ * status <- CRS == 0? estatus: sstatus
  * PC <- ea
  */
 static void eret(DisasContext *dc, uint32_t code, uint32_t flags)
 {
     gen_check_supervisor(dc);
 
-    tcg_gen_mov_tl(cpu_R[CR_STATUS], cpu_R[CR_ESTATUS]);
-    tcg_gen_mov_tl(cpu_R[R_PC], cpu_R[R_EA]);
+    gen_helper_eret(cpu_env, cpu_R[R_EA]);
 
     dc->base.is_jmp = DISAS_JUMP;
 }
@@ -665,7 +719,7 @@ static const Nios2Instruction r_type_instructions[] = {
     INSTRUCTION_ILLEGAL(),
     INSTRUCTION(slli),                                /* slli */
     INSTRUCTION(sll),                                 /* sll */
-    INSTRUCTION_UNIMPLEMENTED(),                      /* wrprs */
+    INSTRUCTION(wrprs),                               /* wrprs */
     INSTRUCTION_ILLEGAL(),
     INSTRUCTION(or),                                  /* or */
     INSTRUCTION(mulxsu),                              /* mulxsu */
-- 
2.25.1

[PATCH v3 3/5] target/nios2: Exteral Interrupt Controller (EIC)

[Amir Gonnen]

Implement Exteral Interrupt Controller interface (EIC).
Added intc_present property, true by default. When set to false, nios2
uses the EIC interface when handling IRQ. When set to true (default)
it uses the internal interrupt controller.
When nios2 recieves irq, it first checks intc_present to decide whether
to use the internal interrupt controller or the EIC.

The EIC is triggered by IRQ gpio but also recieves additional data from
the external interrupt controller (such as VIC): rha, ril, rrs and rnmi.
The interrupt controller is expected to raise IRQ after setting these
fields on Nios2CPU.

rha, ril, rrs and rnmi are used when EIC handles external interrupt, in
order to decide if to take the interrupt now, which shadow register set
to use, which PC to jump to, whether to set NMI flag, etc.

Signed-off-by: Amir Gonnen <amir.gonnen@neuroblade.ai>
---
 target/nios2/cpu.c    | 54 ++++++++++++++++++++++++++++++++-----------
 target/nios2/cpu.h    | 21 ++++++++++++++++-
 target/nios2/helper.c | 33 ++++++++++++++++++++++----
 3 files changed, 90 insertions(+), 18 deletions(-)

diff --git a/target/nios2/cpu.c b/target/nios2/cpu.c
index 026ee18b01..d09f20c8db 100644
--- a/target/nios2/cpu.c
+++ b/target/nios2/cpu.c
@@ -55,6 +55,7 @@ static void nios2_cpu_reset(DeviceState *dev)
 
     memset(env->regs, 0, sizeof(uint32_t) * NUM_CORE_REGS);
     memset(env->shadow_regs, 0, sizeof(uint32_t) * NUM_REG_SETS * NUM_GP_REGS);
+    env->regs[CR_STATUS] |= CR_STATUS_RSIE;
     env->regs[R_PC] = cpu->reset_addr;
 
 #if defined(CONFIG_USER_ONLY)
@@ -65,6 +66,25 @@ static void nios2_cpu_reset(DeviceState *dev)
 #endif
 }
 
+static bool nios2_take_eic_irq(const Nios2CPU *cpu)
+{
+    const CPUNios2State *env = &cpu->env;
+
+    if (cpu->rnmi) {
+        return !(env->regs[CR_STATUS] & CR_STATUS_NMI);
+    }
+
+    if (((env->regs[CR_STATUS] & CR_STATUS_PIE) == 0) ||
+        (cpu->ril <= cpu_get_il(env)) ||
+        (cpu->rrs == cpu_get_crs(env) &&
+          !(env->regs[CR_STATUS] & CR_STATUS_RSIE))) {
+
+        return false;
+    }
+
+    return true;
+}
+
 #ifndef CONFIG_USER_ONLY
 static void nios2_cpu_set_irq(void *opaque, int irq, int level)
 {
@@ -91,13 +111,6 @@ static void nios2_cpu_initfn(Object *obj)
 #if !defined(CONFIG_USER_ONLY)
     mmu_init(&cpu->env);
 
-    /*
-     * These interrupt lines model the IIC (internal interrupt
-     * controller). QEMU does not currently support the EIC
-     * (external interrupt controller) -- if we did it would be
-     * a separate device in hw/intc with a custom interface to
-     * the CPU, and boards using it would not wire up these IRQ lines.
-     */
     qdev_init_gpio_in_named(DEVICE(cpu), nios2_cpu_set_irq, "IRQ", 32);
 #endif
 }
@@ -131,13 +144,26 @@ static bool nios2_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
     Nios2CPU *cpu = NIOS2_CPU(cs);
     CPUNios2State *env = &cpu->env;
 
-    if ((interrupt_request & CPU_INTERRUPT_HARD) &&
-        (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;
+    if (cpu->intc_present) {
+        if ((interrupt_request & CPU_INTERRUPT_HARD) &&
+            (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;
+        }
+    } else {
+        /*
+         * IPENDING does not exist with external interrupt controller
+         * but we still use it to signal an external interrupt
+         */
+        if (env->regs[CR_IPENDING] && nios2_take_eic_irq(cpu)) {
+            cs->exception_index = EXCP_IRQ;
+            nios2_cpu_do_interrupt(cs);
+            return true;
+        }
     }
+
     return false;
 }
 #endif /* !CONFIG_USER_ONLY */
@@ -200,6 +226,8 @@ static Property nios2_properties[] = {
     DEFINE_PROP_UINT32("mmu_tlb_num_ways", Nios2CPU, tlb_num_ways, 16),
     /* ALTR,tlb-num-entries */
     DEFINE_PROP_UINT32("mmu_pid_num_entries", Nios2CPU, tlb_num_entries, 256),
+    /* interrupt-controller (internal) */
+    DEFINE_PROP_BOOL("intc_present", Nios2CPU, intc_present, true),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/target/nios2/cpu.h b/target/nios2/cpu.h
index dbb4c968df..24d2d65aa9 100644
--- a/target/nios2/cpu.h
+++ b/target/nios2/cpu.h
@@ -92,11 +92,12 @@ struct Nios2CPUClass {
 #define   CR_STATUS_U    (1 << 1)
 #define   CR_STATUS_EH   (1 << 2)
 #define   CR_STATUS_IH   (1 << 3)
-#define   CR_STATUS_IL   (63 << 4)
+FIELD(CR_STATUS, IL, 4, 6)
 FIELD(CR_STATUS, CRS, 10, 6)
 FIELD(CR_STATUS, PRS, 16, 6)
 #define   CR_STATUS_NMI  (1 << 22)
 #define   CR_STATUS_RSIE (1 << 23)
+#define   CR_STATUS_SRS  (1 << 31)
 #define CR_ESTATUS   (CR_BASE + 1)
 #define CR_BSTATUS   (CR_BASE + 2)
 #define CR_IENABLE   (CR_BASE + 3)
@@ -187,6 +188,7 @@ struct Nios2CPU {
     CPUNios2State env;
 
     bool mmu_present;
+    bool intc_present;
     uint32_t pid_num_bits;
     uint32_t tlb_num_ways;
     uint32_t tlb_num_entries;
@@ -195,6 +197,12 @@ struct Nios2CPU {
     uint32_t reset_addr;
     uint32_t exception_addr;
     uint32_t fast_tlb_miss_addr;
+
+    /* External Interrupt Controller Interface */
+    uint32_t rha; /* Requested handler address */
+    uint32_t ril; /* Requested interrupt level */
+    uint32_t rrs; /* Requested register set */
+    uint32_t rnmi; /* Requested nonmaskable interrupt */
 };
 
 
@@ -253,6 +261,17 @@ static inline void cpu_get_tb_cpu_state(CPUNios2State *env, target_ulong *pc,
     *flags = (env->regs[CR_STATUS] & (CR_STATUS_EH | CR_STATUS_U));
 }
 
+static inline uint32_t cpu_get_il(const CPUNios2State *env)
+{
+    return FIELD_EX32(env->regs[CR_STATUS], CR_STATUS, IL);
+}
+
+static inline void cpu_set_il(CPUNios2State *env, uint32_t value)
+{
+    env->regs[CR_STATUS] =
+        FIELD_DP32(env->regs[CR_STATUS], CR_STATUS, IL, value);
+}
+
 static inline uint32_t cpu_get_crs(const CPUNios2State *env)
 {
     return FIELD_EX32(env->regs[CR_STATUS], CR_STATUS, CRS);
diff --git a/target/nios2/helper.c b/target/nios2/helper.c
index e5c98650e1..bc022e969d 100644
--- a/target/nios2/helper.c
+++ b/target/nios2/helper.c
@@ -54,21 +54,46 @@ void nios2_cpu_do_interrupt(CPUState *cs)
     Nios2CPU *cpu = NIOS2_CPU(cs);
     CPUNios2State *env = &cpu->env;
 
+    if (cs->exception_index != EXCP_IRQ) {
+        cpu_set_crs(env, 0);
+    }
+
     switch (cs->exception_index) {
     case EXCP_IRQ:
         assert(env->regs[CR_STATUS] & CR_STATUS_PIE);
 
         qemu_log_mask(CPU_LOG_INT, "interrupt at pc=%x\n", env->regs[R_PC]);
 
-        env->regs[CR_ESTATUS] = env->regs[CR_STATUS];
-        env->regs[CR_STATUS] |= CR_STATUS_IH;
+        uint32_t last_status = env->regs[CR_STATUS];
         env->regs[CR_STATUS] &= ~(CR_STATUS_PIE | CR_STATUS_U);
 
         env->regs[CR_EXCEPTION] &= ~(0x1F << 2);
         env->regs[CR_EXCEPTION] |= (cs->exception_index & 0x1F) << 2;
 
-        env->regs[R_EA] = env->regs[R_PC] + 4;
-        env->regs[R_PC] = cpu->exception_addr;
+        if (!cpu->intc_present) {
+            cpu_set_crs(env, cpu->rrs);
+            cpu_set_il(env, cpu->ril);
+            if (cpu->rnmi) {
+                env->regs[CR_STATUS] |= CR_STATUS_NMI;
+            } else {
+                env->regs[CR_STATUS] &= ~CR_STATUS_NMI;
+            }
+            if (cpu->rrs == 0) {
+                env->regs[CR_ESTATUS] = last_status;
+            } else {
+                env->regs[R_SSTATUS] = last_status;
+                env->regs[R_SSTATUS] |= CR_STATUS_SRS;
+            }
+            env->regs[CR_STATUS] |= CR_STATUS_IH;
+            env->regs[R_EA] = env->regs[R_PC] + 4;
+            env->regs[R_PC] = cpu->rha;
+
+        } else {
+            env->regs[CR_ESTATUS] = last_status;
+            env->regs[R_EA] = env->regs[R_PC] + 4;
+            env->regs[R_PC] = cpu->exception_addr;
+        }
+
         break;
 
     case EXCP_TLBD:
-- 
2.25.1

[PATCH v3 4/5] hw/intc: Vectored Interrupt Controller (VIC)

[Amir Gonnen]

Implement nios2 Vectored Interrupt Controller (VIC).
VIC is connected to EIC. It needs to update rha, ril, rrs and rnmi
fields on Nios2CPU before raising an IRQ.
For that purpose, VIC has a "cpu" property which should refer to the
nios2 cpu and set by the board that connects VIC.

Signed-off-by: Amir Gonnen <amir.gonnen@neuroblade.ai>
---
 hw/intc/Kconfig     |   3 +
 hw/intc/meson.build |   1 +
 hw/intc/nios2_vic.c | 341 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 345 insertions(+)
 create mode 100644 hw/intc/nios2_vic.c

diff --git a/hw/intc/Kconfig b/hw/intc/Kconfig
index 528e77b4a6..0a8e8d1d84 100644
--- a/hw/intc/Kconfig
+++ b/hw/intc/Kconfig
@@ -81,3 +81,6 @@ config GOLDFISH_PIC
 
 config M68K_IRQC
     bool
+
+config NIOS2_VIC
+    bool
diff --git a/hw/intc/meson.build b/hw/intc/meson.build
index d953197413..c33b075859 100644
--- a/hw/intc/meson.build
+++ b/hw/intc/meson.build
@@ -61,3 +61,4 @@ specific_ss.add(when: ['CONFIG_KVM', 'CONFIG_XIVE'],
 		if_true: files('spapr_xive_kvm.c'))
 specific_ss.add(when: 'CONFIG_GOLDFISH_PIC', if_true: files('goldfish_pic.c'))
 specific_ss.add(when: 'CONFIG_M68K_IRQC', if_true: files('m68k_irqc.c'))
+specific_ss.add(when: 'CONFIG_NIOS2_VIC', if_true: files('nios2_vic.c'))
diff --git a/hw/intc/nios2_vic.c b/hw/intc/nios2_vic.c
new file mode 100644
index 0000000000..b59d3f6f4c
--- /dev/null
+++ b/hw/intc/nios2_vic.c
@@ -0,0 +1,341 @@
+/*
+ * Vectored Interrupt Controller for nios2 processor
+ *
+ * Copyright (c) 2022 Neuroblade
+ *
+ * Interface:
+ * QOM property "cpu": link to the Nios2 CPU (must be set)
+ * Unnamed GPIO inputs 0..NIOS2_VIC_MAX_IRQ-1: input IRQ lines
+ * IRQ should be connected to nios2 IRQ0.
+ *
+ * Reference: "Embedded Peripherals IP User Guide
+ *             for Intel® Quartus® Prime Design Suite: 21.4"
+ * Chapter 38 "Vectored Interrupt Controller Core"
+ * See: https://www.intel.com/content/www/us/en/docs/programmable/683130/21-4/vectored-interrupt-controller-core.html
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+
+#include "hw/irq.h"
+#include "hw/qdev-properties.h"
+#include "hw/sysbus.h"
+#include "migration/vmstate.h"
+#include "qapi/error.h"
+#include "qemu/bitops.h"
+#include "qemu/log.h"
+#include "qom/object.h"
+#include "cpu.h"
+
+#define TYPE_NIOS2_VIC "nios2-vic"
+
+OBJECT_DECLARE_SIMPLE_TYPE(Nios2Vic, NIOS2_VIC)
+
+#define NIOS2_VIC_MAX_IRQ 32
+
+enum {
+    INT_CONFIG0 = 0,
+    INT_CONFIG31 = 31,
+    INT_ENABLE = 32,
+    INT_ENABLE_SET = 33,
+    INT_ENABLE_CLR = 34,
+    INT_PENDING = 35,
+    INT_RAW_STATUS = 36,
+    SW_INTERRUPT = 37,
+    SW_INTERRUPT_SET = 38,
+    SW_INTERRUPT_CLR = 39,
+    VIC_CONFIG = 40,
+    VIC_STATUS = 41,
+    VEC_TBL_BASE = 42,
+    VEC_TBL_ADDR = 43,
+    CSR_COUNT /* Last! */
+};
+
+struct Nios2Vic {
+    /*< private >*/
+    SysBusDevice parent_obj;
+
+    /*< public >*/
+    qemu_irq output_int;
+
+    /* properties */
+    CPUState *cpu;
+    MemoryRegion csr;
+
+    uint32_t int_config[32];
+    uint32_t vic_config;
+    uint32_t int_raw_status;
+    uint32_t int_enable;
+    uint32_t sw_int;
+    uint32_t vic_status;
+    uint32_t vec_tbl_base;
+    uint32_t vec_tbl_addr;
+};
+
+/* Requested interrupt level (INT_CONFIG[0:5]) */
+static inline uint32_t vic_int_config_ril(const Nios2Vic *vic, int irq_num)
+{
+    return extract32(vic->int_config[irq_num], 0, 6);
+}
+
+/* Requested NMI (INT_CONFIG[6]) */
+static inline uint32_t vic_int_config_rnmi(const Nios2Vic *vic, int irq_num)
+{
+    return extract32(vic->int_config[irq_num], 6, 1);
+}
+
+/* Requested register set (INT_CONFIG[7:12]) */
+static inline uint32_t vic_int_config_rrs(const Nios2Vic *vic, int irq_num)
+{
+    return extract32(vic->int_config[irq_num], 7, 6);
+}
+
+static inline uint32_t vic_config_vec_size(const Nios2Vic *vic)
+{
+    return 1 << (2 + extract32(vic->vic_config, 0, 3));
+}
+
+static inline uint32_t vic_int_pending(const Nios2Vic *vic)
+{
+    return (vic->int_raw_status | vic->sw_int) & vic->int_enable;
+}
+
+static void vic_update_irq(Nios2Vic *vic)
+{
+    Nios2CPU *cpu = NIOS2_CPU(vic->cpu);
+    uint32_t pending = vic_int_pending(vic);
+    int irq = -1;
+    int max_ril = 0;
+    /* Note that if RIL is 0 for an interrupt it is effectively disabled */
+
+    vic->vec_tbl_addr = 0;
+    vic->vic_status = 0;
+
+    if (pending == 0) {
+        qemu_irq_lower(vic->output_int);
+        return;
+    }
+
+    for (int i = 0; i < NIOS2_VIC_MAX_IRQ; i++) {
+        if (pending & BIT(i)) {
+            int ril = vic_int_config_ril(vic, i);
+            if (ril > max_ril) {
+                irq = i;
+                max_ril = ril;
+            }
+        }
+    }
+
+    if (irq < 0) {
+        qemu_irq_lower(vic->output_int);
+        return;
+    }
+
+    vic->vec_tbl_addr = irq * vic_config_vec_size(vic) + vic->vec_tbl_base;
+    vic->vic_status = irq | BIT(31);
+
+    /*
+     * In hardware, the interface between the VIC and the CPU is via the
+     * External Interrupt Controller interface, where the interrupt controller
+     * presents the CPU with a packet of data containing:
+     *  - Requested Handler Address (RHA): 32 bits
+     *  - Requested Register Set (RRS) : 6 bits
+     *  - Requested Interrupt Level (RIL) : 6 bits
+     *  - Requested NMI flag (RNMI) : 1 bit
+     * In our emulation, we implement this by writing the data directly to
+     * fields in the CPU object and then raising the IRQ line to tell
+     * the CPU that we've done so.
+     */
+
+    cpu->rha = vic->vec_tbl_addr;
+    cpu->ril = max_ril;
+    cpu->rrs = vic_int_config_rrs(vic, irq);
+    cpu->rnmi = vic_int_config_rnmi(vic, irq);
+
+    qemu_irq_raise(vic->output_int);
+}
+
+static void vic_set_irq(void *opaque, int irq_num, int level)
+{
+    Nios2Vic *vic = opaque;
+
+    if (level) {
+        vic->int_raw_status |= BIT(irq_num);
+    } else {
+        vic->int_raw_status &= ~BIT(irq_num);
+    }
+
+    vic_update_irq(vic);
+}
+
+static void nios2_vic_reset(DeviceState *dev)
+{
+    Nios2Vic *vic = NIOS2_VIC(dev);
+    memset(&vic->int_config, 0, sizeof(vic->int_config));
+    vic->vic_config = 0;
+    vic->int_raw_status = 0;
+    vic->int_enable = 0;
+    vic->sw_int = 0;
+    vic->vic_status = 0;
+    vic->vec_tbl_base = 0;
+    vic->vec_tbl_addr = 0;
+}
+
+static uint64_t nios2_vic_csr_read(void *opaque, hwaddr offset, unsigned size)
+{
+    Nios2Vic *vic = opaque;
+    int index = offset / 4;
+
+    switch (index) {
+    case INT_CONFIG0 ... INT_CONFIG31:
+        return vic->int_config[index - INT_CONFIG0];
+    case INT_ENABLE:
+        return vic->int_enable;
+    case INT_PENDING:
+        return vic_int_pending(vic);
+    case INT_RAW_STATUS:
+        return vic->int_raw_status;
+    case SW_INTERRUPT:
+        return vic->sw_int;
+    case VIC_CONFIG:
+        return vic->vic_config;
+    case VIC_STATUS:
+        return vic->vic_status;
+    case VEC_TBL_BASE:
+        return vic->vec_tbl_base;
+    case VEC_TBL_ADDR:
+        return vic->vec_tbl_addr;
+    default:
+        return 0;
+    }
+}
+
+static void nios2_vic_csr_write(void *opaque, hwaddr offset, uint64_t value,
+                                unsigned size)
+{
+    Nios2Vic *vic = opaque;
+    int index = offset / 4;
+
+    switch (index) {
+    case INT_CONFIG0 ... INT_CONFIG31:
+        vic->int_config[index - INT_CONFIG0] = value;
+        break;
+    case INT_ENABLE:
+        vic->int_enable = value;
+        break;
+    case INT_ENABLE_SET:
+        vic->int_enable |= value;
+        break;
+    case INT_ENABLE_CLR:
+        vic->int_enable &= ~value;
+        break;
+    case SW_INTERRUPT:
+        vic->sw_int = value;
+        break;
+    case SW_INTERRUPT_SET:
+        vic->sw_int |= value;
+        break;
+    case SW_INTERRUPT_CLR:
+        vic->sw_int &= ~value;
+        break;
+    case VIC_CONFIG:
+        vic->vic_config = value;
+        break;
+    case VEC_TBL_BASE:
+        vic->vec_tbl_base = value;
+        break;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+            "nios2-vic: write to invalid CSR address 0x%x\n", (int)offset);
+    }
+
+    vic_update_irq(vic);
+}
+
+static const MemoryRegionOps nios2_vic_csr_ops = {
+    .read = nios2_vic_csr_read,
+    .write = nios2_vic_csr_write,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .valid = { .min_access_size = 4, .max_access_size = 4 }
+};
+
+static void nios2_vic_realize(DeviceState *dev, Error **errp)
+{
+    Nios2Vic *vic = NIOS2_VIC(dev);
+
+    if (!vic->cpu) {
+        /* This is a programming error in the code using this device */
+        error_setg(errp, "nios2-vic 'cpu' link property was not set");
+        return;
+    }
+
+    sysbus_init_irq(SYS_BUS_DEVICE(dev), &vic->output_int);
+    qdev_init_gpio_in(dev, vic_set_irq, NIOS2_VIC_MAX_IRQ);
+
+    memory_region_init_io(&vic->csr, OBJECT(dev), &nios2_vic_csr_ops, vic,
+                          "nios2.vic.csr", CSR_COUNT * sizeof(uint32_t));
+    sysbus_init_mmio(SYS_BUS_DEVICE(dev), &vic->csr);
+}
+
+static Property nios2_vic_properties[] = {
+    DEFINE_PROP_LINK("cpu", Nios2Vic, cpu, TYPE_CPU, CPUState *),
+    DEFINE_PROP_END_OF_LIST()
+};
+
+static const VMStateDescription nios2_vic_vmstate = {
+    .name = "nios2-vic",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]){
+        VMSTATE_UINT32_ARRAY(int_config, Nios2Vic, 32),
+        VMSTATE_UINT32(vic_config, Nios2Vic),
+        VMSTATE_UINT32(int_raw_status, Nios2Vic),
+        VMSTATE_UINT32(int_enable, Nios2Vic),
+        VMSTATE_UINT32(sw_int, Nios2Vic),
+        VMSTATE_UINT32(vic_status, Nios2Vic),
+        VMSTATE_UINT32(vec_tbl_base, Nios2Vic),
+        VMSTATE_UINT32(vec_tbl_addr, Nios2Vic),
+        VMSTATE_END_OF_LIST()
+    },
+};
+
+static void nios2_vic_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->reset = nios2_vic_reset;
+    dc->realize = nios2_vic_realize;
+    dc->vmsd = &nios2_vic_vmstate;
+    device_class_set_props(dc, nios2_vic_properties);
+}
+
+static const TypeInfo nios2_vic_info = {
+    .name = TYPE_NIOS2_VIC,
+    .parent = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(Nios2Vic),
+    .class_init = nios2_vic_class_init,
+};
+
+static void nios2_vic_register_types(void)
+{
+    type_register_static(&nios2_vic_info);
+}
+
+type_init(nios2_vic_register_types);
-- 
2.25.1

[PATCH v3 5/5] hw/nios2: Machine with a Vectored Interrupt Controller

[Amir Gonnen]

Demonstrate how to use nios2 VIC on a machine.
Introduce a new machine "10m50-ghrd-vic" which is based on "10m50-ghrd"
with a VIC attached and internal interrupt controller removed.

When VIC is present, irq0 connects the VIC to the cpu, intc_present
is set to false to disable the internal interrupt controller, and the
devices on the machine are attached to the VIC (and not directly to cpu).
To allow VIC update EIC fields, we set the "cpu" property of the VIC
with a reference to the nios2 cpu.

Signed-off-by: Amir Gonnen <amir.gonnen@neuroblade.ai>
---
 hw/nios2/10m50_devboard.c | 64 ++++++++++++++++++++++++++++++++++++---
 hw/nios2/Kconfig          |  1 +
 2 files changed, 61 insertions(+), 4 deletions(-)

diff --git a/hw/nios2/10m50_devboard.c b/hw/nios2/10m50_devboard.c
index 3d1205b8bd..9f62a2993f 100644
--- a/hw/nios2/10m50_devboard.c
+++ b/hw/nios2/10m50_devboard.c
@@ -36,10 +36,23 @@
 
 #include "boot.h"
 
+#define TYPE_NIOS2_MACHINE  MACHINE_TYPE_NAME("10m50-ghrd")
+typedef struct Nios2MachineClass Nios2MachineClass;
+DECLARE_OBJ_CHECKERS(MachineState, Nios2MachineClass,
+                     NIOS2_MACHINE, TYPE_NIOS2_MACHINE)
+
 #define BINARY_DEVICE_TREE_FILE    "10m50-devboard.dtb"
 
+struct Nios2MachineClass {
+    MachineClass parent_obj;
+
+    bool vic;
+};
+
 static void nios2_10m50_ghrd_init(MachineState *machine)
 {
+    Nios2MachineClass *nmc = NIOS2_MACHINE_GET_CLASS(machine);
+
     Nios2CPU *cpu;
     DeviceState *dev;
     MemoryRegion *address_space_mem = get_system_memory();
@@ -74,8 +87,24 @@ static void nios2_10m50_ghrd_init(MachineState *machine)
 
     /* Create CPU -- FIXME */
     cpu = NIOS2_CPU(cpu_create(TYPE_NIOS2_CPU));
-    for (i = 0; i < 32; i++) {
-        irq[i] = qdev_get_gpio_in_named(DEVICE(cpu), "IRQ", i);
+
+    if (nmc->vic) {
+        DeviceState *dev = qdev_new("nios2-vic");
+
+        object_property_set_link(OBJECT(dev), "cpu", OBJECT(cpu), &error_fatal);
+        sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
+        cpu->intc_present = false;
+        qemu_irq cpu_irq = qdev_get_gpio_in_named(DEVICE(cpu), "IRQ", 0);
+        sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, cpu_irq);
+        for (int i = 0; i < 32; i++) {
+            irq[i] = qdev_get_gpio_in(dev, i);
+        }
+        MemoryRegion *dev_mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0);
+        memory_region_add_subregion(address_space_mem, 0x18002000, dev_mr);
+    } else {
+        for (i = 0; i < 32; i++) {
+            irq[i] = qdev_get_gpio_in_named(DEVICE(cpu), "IRQ", i);
+        }
     }
 
     /* Register: Altera 16550 UART */
@@ -105,11 +134,38 @@ static void nios2_10m50_ghrd_init(MachineState *machine)
                       BINARY_DEVICE_TREE_FILE, NULL);
 }
 
-static void nios2_10m50_ghrd_machine_init(struct MachineClass *mc)
+static void nios2_10m50_ghrd_machine_class_init(ObjectClass *oc, void *data)
 {
+    MachineClass *mc = MACHINE_CLASS(oc);
+    Nios2MachineClass *nmc = NIOS2_MACHINE_CLASS(oc);
     mc->desc = "Altera 10M50 GHRD Nios II design";
     mc->init = nios2_10m50_ghrd_init;
     mc->is_default = true;
+    nmc->vic = false;
 }
 
-DEFINE_MACHINE("10m50-ghrd", nios2_10m50_ghrd_machine_init);
+static void nios2_10m50_ghrd_vic_machine_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+    Nios2MachineClass *nmc = NIOS2_MACHINE_CLASS(oc);
+    mc->desc = "Altera 10M50 GHRD Nios II design with VIC";
+    mc->init = nios2_10m50_ghrd_init;
+    mc->is_default = false;
+    nmc->vic = true;
+}
+
+static const TypeInfo nios_machine_types[] = {
+    {
+        .name          = MACHINE_TYPE_NAME("10m50-ghrd"),
+        .parent        = TYPE_MACHINE,
+        .class_size    = sizeof(Nios2MachineClass),
+        .class_init    = nios2_10m50_ghrd_machine_class_init,
+    }, {
+        .name          = MACHINE_TYPE_NAME("10m50-ghrd-vic"),
+        .parent        = TYPE_MACHINE,
+        .class_size    = sizeof(Nios2MachineClass),
+        .class_init    = nios2_10m50_ghrd_vic_machine_class_init,
+    }
+};
+
+DEFINE_TYPES(nios_machine_types)
diff --git a/hw/nios2/Kconfig b/hw/nios2/Kconfig
index b10ea640da..4748ae27b6 100644
--- a/hw/nios2/Kconfig
+++ b/hw/nios2/Kconfig
@@ -3,6 +3,7 @@ config NIOS2_10M50
     select NIOS2
     select SERIAL
     select ALTERA_TIMER
+    select NIOS2_VIC
 
 config NIOS2_GENERIC_NOMMU
     bool
-- 
2.25.1

Re: [PATCH v3 4/5] hw/intc: Vectored Interrupt Controller (VIC)

[Peter Maydell]

On Thu, 3 Mar 2022 at 15:39, Amir Gonnen <amir.gonnen@neuroblade.ai> wrote:
>
> Implement nios2 Vectored Interrupt Controller (VIC).
> VIC is connected to EIC. It needs to update rha, ril, rrs and rnmi
> fields on Nios2CPU before raising an IRQ.
> For that purpose, VIC has a "cpu" property which should refer to the
> nios2 cpu and set by the board that connects VIC.
>
> Signed-off-by: Amir Gonnen <amir.gonnen@neuroblade.ai>

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

thanks
-- PMM

Re: [PATCH v3 1/5] target/nios2: Check supervisor on eret

[Peter Maydell]

On Thu, 3 Mar 2022 at 15:39, Amir Gonnen <amir.gonnen@neuroblade.ai> wrote:
>
> eret instruction is only allowed in supervisor mode.
>
> Signed-off-by: Amir Gonnen <amir.gonnen@neuroblade.ai>
> ---
>  target/nios2/translate.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/target/nios2/translate.c b/target/nios2/translate.c
> index ce3aacf59d..007c17e6e9 100644
> --- a/target/nios2/translate.c
> +++ b/target/nios2/translate.c
> @@ -384,6 +384,8 @@ static const Nios2Instruction i_type_instructions[] = {
>   */
>  static void eret(DisasContext *dc, uint32_t code, uint32_t flags)
>  {
> +    gen_check_supervisor(dc);
> +
>      tcg_gen_mov_tl(cpu_R[CR_STATUS], cpu_R[CR_ESTATUS]);
>      tcg_gen_mov_tl(cpu_R[R_PC], cpu_R[R_EA]);
>
> --

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

thanks
-- PMM

Re: [PATCH v3 5/5] hw/nios2: Machine with a Vectored Interrupt Controller

[Peter Maydell]

On Thu, 3 Mar 2022 at 15:39, Amir Gonnen <amir.gonnen@neuroblade.ai> wrote:
>
> Demonstrate how to use nios2 VIC on a machine.
> Introduce a new machine "10m50-ghrd-vic" which is based on "10m50-ghrd"
> with a VIC attached and internal interrupt controller removed.
>
> When VIC is present, irq0 connects the VIC to the cpu, intc_present
> is set to false to disable the internal interrupt controller, and the
> devices on the machine are attached to the VIC (and not directly to cpu).
> To allow VIC update EIC fields, we set the "cpu" property of the VIC
> with a reference to the nios2 cpu.
>
> Signed-off-by: Amir Gonnen <amir.gonnen@neuroblade.ai>
> ---
>  hw/nios2/10m50_devboard.c | 64 ++++++++++++++++++++++++++++++++++++---
>  hw/nios2/Kconfig          |  1 +
>  2 files changed, 61 insertions(+), 4 deletions(-)

My remarks about this from the earlier version of the patchset still
stand:
 (1) if this isn't an actual config of the real hardware, then
presumably there aren't any guests which will run on it with the
VIC enabled
 (2) if we do want to do this, we should have a machine property
for "vic=true" rather than a complete new machine type

thanks
-- PMM

Re: [PATCH v3 1/5] target/nios2: Check supervisor on eret

[Richard Henderson]

On 3/3/22 05:39, Amir Gonnen wrote:
> eret instruction is only allowed in supervisor mode.
> 
> Signed-off-by: Amir Gonnen<amir.gonnen@neuroblade.ai>
> ---
>   target/nios2/translate.c | 2 ++
>   1 file changed, 2 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH v3 2/5] target/nios2: Shadow register set

[Richard Henderson]

On 3/3/22 05:39, Amir Gonnen wrote:
> Implement shadow register set and related instructions
> rdprs, wrprs. Fix eret to update either status or sstatus
> according to current register set.
> eret also changes register set when needed.
> 
> Signed-off-by: Amir Gonnen <amir.gonnen@neuroblade.ai>
> ---
>   target/nios2/cpu.c       |  1 +
>   target/nios2/cpu.h       | 48 +++++++++++++++++++++++++++---
>   target/nios2/helper.h    |  1 +
>   target/nios2/op_helper.c | 18 +++++++++++
>   target/nios2/translate.c | 64 ++++++++++++++++++++++++++++++++++++----
>   5 files changed, 123 insertions(+), 9 deletions(-)
> 
> diff --git a/target/nios2/cpu.c b/target/nios2/cpu.c
> index 6975ae4bdb..026ee18b01 100644
> --- a/target/nios2/cpu.c
> +++ b/target/nios2/cpu.c
> @@ -54,6 +54,7 @@ static void nios2_cpu_reset(DeviceState *dev)
>       ncc->parent_reset(dev);
> 
>       memset(env->regs, 0, sizeof(uint32_t) * NUM_CORE_REGS);
> +    memset(env->shadow_regs, 0, sizeof(uint32_t) * NUM_REG_SETS * NUM_GP_REGS);

Zeroing registers is not part of processing 3.7.4 Reset Exception.
I'd be tempted to set all of the registers to 0xdeadbeef instead, to catch out incorrect 
code, especially wrt the shadowed r0.

> -#define   CR_STATUS_CRS  (63 << 10)
> -#define   CR_STATUS_PRS  (63 << 16)
> +FIELD(CR_STATUS, CRS, 10, 6)
> +FIELD(CR_STATUS, PRS, 16, 6)

This change needs to be done separately from introducing shadow registers.

Having read the specification more closely now, I think this implementation may be wrong. 
  In particular:

(1) r0 appears to be hardwired to 0 only in the normal register set (CRS = 0).  That's why 
software is directed to use wrprs to initialize r0 in each shadow register set to 0.

(2) Changes are needed to wrctl to protect the read-only bits of the control registers. 
In this case, especially status.NMI and status.CRS.

(3) The advice I gave you for rdprs/wrprs is wrong when CRS == PRS (you'd need to modify 
regs[n] not shadow_regs[prs][n]).

These 3 items need to be handled in separate patches.

> +static inline void cpu_change_reg_set(CPUNios2State *env, uint32_t prev_set,
> +                                      uint32_t new_set)
> +{
> +    if (new_set == prev_set) {
> +        return;
> +    }
> +    memcpy(env->shadow_regs[prev_set], env->regs,
> +           sizeof(uint32_t) * NUM_GP_REGS);
> +    memcpy(env->regs, env->shadow_regs[new_set],
> +           sizeof(uint32_t) * NUM_GP_REGS);
> +    env->regs[CR_STATUS] =
> +        FIELD_DP32(env->regs[CR_STATUS], CR_STATUS, PRS, prev_set);
> +    env->regs[CR_STATUS] =
> +        FIELD_DP32(env->regs[CR_STATUS], CR_STATUS, CRS, new_set);
> +}

Another possibility, that doesn't involve moving data around is to use a pointer to the 
CRS base, like sparc does for its register windows.  I'm not necessarily advocating this 
solution, merely pointing it out.


r~

Re: [PATCH v3 3/5] target/nios2: Exteral Interrupt Controller (EIC)

[Richard Henderson]

On 3/3/22 05:39, Amir Gonnen wrote:
> @@ -55,6 +55,7 @@ static void nios2_cpu_reset(DeviceState *dev)
> 
>       memset(env->regs, 0, sizeof(uint32_t) * NUM_CORE_REGS);
>       memset(env->shadow_regs, 0, sizeof(uint32_t) * NUM_REG_SETS * NUM_GP_REGS);
> +    env->regs[CR_STATUS] |= CR_STATUS_RSIE;

status is supposed to be reset to zero other than RSIE.

> @@ -131,13 +144,26 @@ static bool nios2_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
>       Nios2CPU *cpu = NIOS2_CPU(cs);
>       CPUNios2State *env = &cpu->env;
> 
> -    if ((interrupt_request & CPU_INTERRUPT_HARD) &&
> -        (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;
> +    if (cpu->intc_present) {
> +        if ((interrupt_request & CPU_INTERRUPT_HARD) &&
> +            (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;
> +        }
> +    } else {
> +        /*
> +         * IPENDING does not exist with external interrupt controller
> +         * but we still use it to signal an external interrupt
> +         */
> +        if (env->regs[CR_IPENDING] && nios2_take_eic_irq(cpu)) {

Why CR_IPENDING?  The ipending register isn't supposed to exist with the EIC.  Did you in 
fact mean interrupt_request & CPU_INTERRUPT_HARD, as set by nios2_cpu_set_irq?


> -    /*
> -     * These interrupt lines model the IIC (internal interrupt
> -     * controller). QEMU does not currently support the EIC
> -     * (external interrupt controller) -- if we did it would be
> -     * a separate device in hw/intc with a custom interface to
> -     * the CPU, and boards using it would not wire up these IRQ lines.
> -     */

You should note that this is still used for EIC, though only IRQ[0].

There's a fair amount of checking cpu->intc_present, then doing two completely different 
things.  I'm thinking that it might be best to split these into two separate functions, 
and then set up the pointers properly.

You could in fact replace intc_present with two separate cpu classes (which is where many 
of those pointers are registered).  That would be early enough for the cpu_init hook to 
*not* register 32 interrupt lines for the EIC, as per above.


r~