[PATCH v8 2/5] target/riscv: Handle Smrnmi interrupt and exception

frank.chang@sifive.com posted 5 patches 1 year, 3 months ago
There is a newer version of this series
[PATCH v8 2/5] target/riscv: Handle Smrnmi interrupt and exception
Posted by frank.chang@sifive.com 1 year, 3 months ago
From: Tommy Wu <tommy.wu@sifive.com>

Because the RNMI interrupt trap handler address is implementation defined.
We add the 'rnmi-interrupt-vector' and 'rnmi-exception-vector' as the property
of the harts. It’s very easy for users to set the address based on their
expectation. This patch also adds the functionality to handle the RNMI signals.

Signed-off-by: Frank Chang <frank.chang@sifive.com>
Signed-off-by: Tommy Wu <tommy.wu@sifive.com>
---
 hw/riscv/riscv_hart.c         | 18 ++++++++
 include/hw/riscv/riscv_hart.h |  4 ++
 target/riscv/cpu.c            | 11 +++++
 target/riscv/cpu.h            |  6 +++
 target/riscv/cpu_bits.h       | 12 +++++
 target/riscv/cpu_helper.c     | 82 ++++++++++++++++++++++++++++++++---
 6 files changed, 128 insertions(+), 5 deletions(-)

diff --git a/hw/riscv/riscv_hart.c b/hw/riscv/riscv_hart.c
index 613ea2aaa0..b7d73f7a82 100644
--- a/hw/riscv/riscv_hart.c
+++ b/hw/riscv/riscv_hart.c
@@ -33,6 +33,12 @@ static Property riscv_harts_props[] = {
     DEFINE_PROP_STRING("cpu-type", RISCVHartArrayState, cpu_type),
     DEFINE_PROP_UINT64("resetvec", RISCVHartArrayState, resetvec,
                        DEFAULT_RSTVEC),
+    DEFINE_PROP_ARRAY("rnmi-interrupt-vector", RISCVHartArrayState,
+                      num_rnmi_irqvec, rnmi_irqvec, qdev_prop_uint64,
+                      uint64_t),
+    DEFINE_PROP_ARRAY("rnmi-exception-vector", RISCVHartArrayState,
+                      num_rnmi_excpvec, rnmi_excpvec, qdev_prop_uint64,
+                      uint64_t),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -47,6 +53,18 @@ static bool riscv_hart_realize(RISCVHartArrayState *s, int idx,
 {
     object_initialize_child(OBJECT(s), "harts[*]", &s->harts[idx], cpu_type);
     qdev_prop_set_uint64(DEVICE(&s->harts[idx]), "resetvec", s->resetvec);
+    if (s->harts[idx].cfg.ext_smrnmi) {
+        if (s->rnmi_irqvec) {
+            qdev_prop_set_uint64(DEVICE(&s->harts[idx]),
+                                 "rnmi-interrupt-vector",
+                                 s->rnmi_irqvec[idx]);
+        }
+        if (s->rnmi_excpvec) {
+            qdev_prop_set_uint64(DEVICE(&s->harts[idx]),
+                                 "rnmi-exception-vector",
+                                 s->rnmi_excpvec[idx]);
+        }
+    }
     s->harts[idx].env.mhartid = s->hartid_base + idx;
     qemu_register_reset(riscv_harts_cpu_reset, &s->harts[idx]);
     return qdev_realize(DEVICE(&s->harts[idx]), NULL, errp);
diff --git a/include/hw/riscv/riscv_hart.h b/include/hw/riscv/riscv_hart.h
index 912b4a2682..a6ed73a195 100644
--- a/include/hw/riscv/riscv_hart.h
+++ b/include/hw/riscv/riscv_hart.h
@@ -38,6 +38,10 @@ struct RISCVHartArrayState {
     uint32_t hartid_base;
     char *cpu_type;
     uint64_t resetvec;
+    uint32_t num_rnmi_irqvec;
+    uint64_t *rnmi_irqvec;
+    uint32_t num_rnmi_excpvec;
+    uint64_t *rnmi_excpvec;
     RISCVCPU *harts;
 };
 
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 2a78cd639b..5ce6ed31ec 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1297,6 +1297,11 @@ static void riscv_cpu_set_irq(void *opaque, int irq, int level)
         g_assert_not_reached();
     }
 }
+
+static void riscv_cpu_set_nmi(void *opaque, int irq, int level)
+{
+    riscv_cpu_set_rnmi(RISCV_CPU(opaque), irq, level);
+}
 #endif /* CONFIG_USER_ONLY */
 
 static bool riscv_cpu_is_dynamic(Object *cpu_obj)
@@ -1320,6 +1325,8 @@ static void riscv_cpu_init(Object *obj)
 #ifndef CONFIG_USER_ONLY
     qdev_init_gpio_in(DEVICE(obj), riscv_cpu_set_irq,
                       IRQ_LOCAL_MAX + IRQ_LOCAL_GUEST_MAX);
+    qdev_init_gpio_in_named(DEVICE(cpu), riscv_cpu_set_nmi,
+                            "riscv.cpu.rnmi", RNMI_MAX);
 #endif /* CONFIG_USER_ONLY */
 
     general_user_opts = g_hash_table_new(g_str_hash, g_str_equal);
@@ -2670,6 +2677,10 @@ static Property riscv_cpu_properties[] = {
 
 #ifndef CONFIG_USER_ONLY
     DEFINE_PROP_UINT64("resetvec", RISCVCPU, env.resetvec, DEFAULT_RSTVEC),
+    DEFINE_PROP_UINT64("rnmi-interrupt-vector", RISCVCPU, env.rnmi_irqvec,
+                       DEFAULT_RNMI_IRQVEC),
+    DEFINE_PROP_UINT64("rnmi-exception-vector", RISCVCPU, env.rnmi_excpvec,
+                       DEFAULT_RNMI_EXCPVEC),
 #endif
 
     DEFINE_PROP_BOOL("short-isa-string", RISCVCPU, cfg.short_isa_string, false),
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index a63a29744c..8b2a863c50 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -472,6 +472,11 @@ struct CPUArchState {
     uint64_t kvm_timer_state;
     uint64_t kvm_timer_frequency;
 #endif /* CONFIG_KVM */
+
+    /* RNMI */
+    target_ulong rnmip;
+    uint64_t rnmi_irqvec;
+    uint64_t rnmi_excpvec;
 };
 
 /*
@@ -568,6 +573,7 @@ void riscv_cpu_swap_hypervisor_regs(CPURISCVState *env);
 int riscv_cpu_claim_interrupts(RISCVCPU *cpu, uint64_t interrupts);
 uint64_t riscv_cpu_update_mip(CPURISCVState *env, uint64_t mask,
                               uint64_t value);
+void riscv_cpu_set_rnmi(RISCVCPU *cpu, uint32_t irq, bool level);
 void riscv_cpu_interrupt(CPURISCVState *env);
 #define BOOL_TO_MASK(x) (-!!(x)) /* helper for riscv_cpu_update_mip value */
 void riscv_cpu_set_rdtime_fn(CPURISCVState *env, uint64_t (*fn)(void *),
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 7e3f629356..6df2cbc2be 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -662,6 +662,12 @@ typedef enum {
 /* Default Reset Vector address */
 #define DEFAULT_RSTVEC      0x1000
 
+/* Default RNMI Interrupt Vector address */
+#define DEFAULT_RNMI_IRQVEC     0x0
+
+/* Default RNMI Exception Vector address */
+#define DEFAULT_RNMI_EXCPVEC    0x0
+
 /* Exception causes */
 typedef enum RISCVException {
     RISCV_EXCP_NONE = -1, /* sentinel value */
@@ -711,6 +717,9 @@ typedef enum RISCVException {
 /* -1 is due to bit zero of hgeip and hgeie being ROZ. */
 #define IRQ_LOCAL_GUEST_MAX                (TARGET_LONG_BITS - 1)
 
+/* RNMI causes */
+#define RNMI_MAX                           16
+
 /* mip masks */
 #define MIP_USIP                           (1 << IRQ_U_SOFT)
 #define MIP_SSIP                           (1 << IRQ_S_SOFT)
@@ -942,6 +951,9 @@ typedef enum RISCVException {
 #define MHPMEVENT_IDX_MASK                 0xFFFFF
 #define MHPMEVENT_SSCOF_RESVD              16
 
+/* RISC-V-specific interrupt pending bits. */
+#define CPU_INTERRUPT_RNMI                 CPU_INTERRUPT_TGT_EXT_0
+
 /* JVT CSR bits */
 #define JVT_MODE                           0x3F
 #define JVT_BASE                           (~0x3F)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 203c0a92ab..977bf97c16 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -434,6 +434,18 @@ static int riscv_cpu_local_irq_pending(CPURISCVState *env)
     uint64_t vsbits, irq_delegated;
     int virq;
 
+    /* Priority: RNMI > Other interrupt. */
+    if (riscv_cpu_cfg(env)->ext_smrnmi) {
+        /* If mnstatus.NMIE == 0, all interrupts are disabled. */
+        if (!get_field(env->mnstatus, MNSTATUS_NMIE)) {
+            return RISCV_EXCP_NONE;
+        }
+
+        if (env->rnmip) {
+            return ctz64(env->rnmip); /* since non-zero */
+        }
+    }
+
     /* Determine interrupt enable state of all privilege modes */
     if (env->virt_enabled) {
         mie = 1;
@@ -496,7 +508,9 @@ static int riscv_cpu_local_irq_pending(CPURISCVState *env)
 
 bool riscv_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
 {
-    if (interrupt_request & CPU_INTERRUPT_HARD) {
+    uint32_t mask = CPU_INTERRUPT_HARD | CPU_INTERRUPT_RNMI;
+
+    if (interrupt_request & mask) {
         RISCVCPU *cpu = RISCV_CPU(cs);
         CPURISCVState *env = &cpu->env;
         int interruptno = riscv_cpu_local_irq_pending(env);
@@ -619,6 +633,30 @@ void riscv_cpu_set_geilen(CPURISCVState *env, target_ulong geilen)
     env->geilen = geilen;
 }
 
+void riscv_cpu_set_rnmi(RISCVCPU *cpu, uint32_t irq, bool level)
+{
+    CPURISCVState *env = &cpu->env;
+    CPUState *cs = CPU(cpu);
+    bool release_lock = false;
+
+    if (!bql_locked()) {
+        release_lock = true;
+        bql_lock();
+    }
+
+    if (level) {
+        env->rnmip |= 1 << irq;
+        cpu_interrupt(cs, CPU_INTERRUPT_RNMI);
+    } else {
+        env->rnmip &= ~(1 << irq);
+        cpu_reset_interrupt(cs, CPU_INTERRUPT_RNMI);
+    }
+
+    if (release_lock) {
+        bql_unlock();
+    }
+}
+
 int riscv_cpu_claim_interrupts(RISCVCPU *cpu, uint64_t interrupts)
 {
     CPURISCVState *env = &cpu->env;
@@ -1661,6 +1699,7 @@ void riscv_cpu_do_interrupt(CPUState *cs)
     bool virt = env->virt_enabled;
     bool write_gva = false;
     uint64_t s;
+    int mode;
 
     /*
      * cs->exception is 32-bits wide unlike mcause which is XLEN-bits wide
@@ -1679,6 +1718,23 @@ void riscv_cpu_do_interrupt(CPUState *cs)
     target_ulong mtval2 = 0;
     int sxlen = 0;
     int mxlen = 0;
+    bool nnmi_excep = false;
+
+    if (cpu->cfg.ext_smrnmi && env->rnmip && async) {
+        env->mnstatus = set_field(env->mnstatus, MNSTATUS_NMIE, false);
+        env->mnstatus = set_field(env->mnstatus, MNSTATUS_MNPV,
+                                  env->virt_enabled);
+        env->mnstatus = set_field(env->mnstatus, MNSTATUS_MNPP,
+                                  env->priv);
+        env->mncause = cause | ((target_ulong)1U << (TARGET_LONG_BITS - 1));
+        env->mnepc = env->pc;
+        env->pc = env->rnmi_irqvec;
+
+        /* Trapping to M mode, virt is disabled */
+        riscv_cpu_set_mode(env, PRV_M, false);
+
+        return;
+    }
 
     if (!async) {
         /* set tval to badaddr for traps with address information */
@@ -1766,8 +1822,10 @@ void riscv_cpu_do_interrupt(CPUState *cs)
                   __func__, env->mhartid, async, cause, env->pc, tval,
                   riscv_cpu_get_trap_name(cause, async));
 
-    if (env->priv <= PRV_S && cause < 64 &&
-        (((deleg >> cause) & 1) || s_injected || vs_injected)) {
+    mode = env->priv <= PRV_S && cause < 64 &&
+        (((deleg >> cause) & 1) || s_injected || vs_injected) ? PRV_S : PRV_M;
+
+    if (mode == PRV_S) {
         /* handle the trap in S-mode */
         if (riscv_has_ext(env, RVH)) {
             uint64_t hdeleg = async ? env->hideleg : env->hedeleg;
@@ -1845,8 +1903,22 @@ void riscv_cpu_do_interrupt(CPUState *cs)
         env->mtval = tval;
         env->mtval2 = mtval2;
         env->mtinst = tinst;
-        env->pc = (env->mtvec >> 2 << 2) +
-                  ((async && (env->mtvec & 3) == 1) ? cause * 4 : 0);
+
+        /*
+         * If the hart encounters an exception while executing in M-mode,
+         * with the mnstatus.NMIE bit clear, the program counter is set to
+         * the RNMI exception trap handler address.
+         */
+        nnmi_excep = cpu->cfg.ext_smrnmi &&
+                     !get_field(env->mnstatus, MNSTATUS_NMIE) &&
+                     !async;
+
+        if (nnmi_excep) {
+            env->pc = env->rnmi_excpvec;
+        } else {
+            env->pc = (env->mtvec >> 2 << 2) +
+                      ((async && (env->mtvec & 3) == 1) ? cause * 4 : 0);
+        }
         riscv_cpu_set_mode(env, PRV_M, virt);
     }
 
-- 
2.43.2


Re: [PATCH v8 2/5] target/riscv: Handle Smrnmi interrupt and exception
Posted by Alistair Francis 1 year, 2 months ago
On Mon, Oct 21, 2024 at 1:06 PM <frank.chang@sifive.com> wrote:
>
> From: Tommy Wu <tommy.wu@sifive.com>
>
> Because the RNMI interrupt trap handler address is implementation defined.
> We add the 'rnmi-interrupt-vector' and 'rnmi-exception-vector' as the property
> of the harts. It’s very easy for users to set the address based on their
> expectation. This patch also adds the functionality to handle the RNMI signals.
>
> Signed-off-by: Frank Chang <frank.chang@sifive.com>
> Signed-off-by: Tommy Wu <tommy.wu@sifive.com>
> ---
>  hw/riscv/riscv_hart.c         | 18 ++++++++
>  include/hw/riscv/riscv_hart.h |  4 ++
>  target/riscv/cpu.c            | 11 +++++
>  target/riscv/cpu.h            |  6 +++
>  target/riscv/cpu_bits.h       | 12 +++++
>  target/riscv/cpu_helper.c     | 82 ++++++++++++++++++++++++++++++++---
>  6 files changed, 128 insertions(+), 5 deletions(-)
>
> diff --git a/hw/riscv/riscv_hart.c b/hw/riscv/riscv_hart.c
> index 613ea2aaa0..b7d73f7a82 100644
> --- a/hw/riscv/riscv_hart.c
> +++ b/hw/riscv/riscv_hart.c
> @@ -33,6 +33,12 @@ static Property riscv_harts_props[] = {
>      DEFINE_PROP_STRING("cpu-type", RISCVHartArrayState, cpu_type),
>      DEFINE_PROP_UINT64("resetvec", RISCVHartArrayState, resetvec,
>                         DEFAULT_RSTVEC),
> +    DEFINE_PROP_ARRAY("rnmi-interrupt-vector", RISCVHartArrayState,
> +                      num_rnmi_irqvec, rnmi_irqvec, qdev_prop_uint64,
> +                      uint64_t),
> +    DEFINE_PROP_ARRAY("rnmi-exception-vector", RISCVHartArrayState,
> +                      num_rnmi_excpvec, rnmi_excpvec, qdev_prop_uint64,
> +                      uint64_t),

These will need some documentation somewhere, it's not really clear
from the names what these do.

>      DEFINE_PROP_END_OF_LIST(),
>  };
>
> @@ -47,6 +53,18 @@ static bool riscv_hart_realize(RISCVHartArrayState *s, int idx,
>  {
>      object_initialize_child(OBJECT(s), "harts[*]", &s->harts[idx], cpu_type);
>      qdev_prop_set_uint64(DEVICE(&s->harts[idx]), "resetvec", s->resetvec);
> +    if (s->harts[idx].cfg.ext_smrnmi) {
> +        if (s->rnmi_irqvec) {
> +            qdev_prop_set_uint64(DEVICE(&s->harts[idx]),
> +                                 "rnmi-interrupt-vector",
> +                                 s->rnmi_irqvec[idx]);
> +        }
> +        if (s->rnmi_excpvec) {
> +            qdev_prop_set_uint64(DEVICE(&s->harts[idx]),
> +                                 "rnmi-exception-vector",
> +                                 s->rnmi_excpvec[idx]);
> +        }
> +    }

We should probably warn the user if we are ignoring the values that
they specified because the extension isn't enabled

Alistair

>      s->harts[idx].env.mhartid = s->hartid_base + idx;
>      qemu_register_reset(riscv_harts_cpu_reset, &s->harts[idx]);
>      return qdev_realize(DEVICE(&s->harts[idx]), NULL, errp);
> diff --git a/include/hw/riscv/riscv_hart.h b/include/hw/riscv/riscv_hart.h
> index 912b4a2682..a6ed73a195 100644
> --- a/include/hw/riscv/riscv_hart.h
> +++ b/include/hw/riscv/riscv_hart.h
> @@ -38,6 +38,10 @@ struct RISCVHartArrayState {
>      uint32_t hartid_base;
>      char *cpu_type;
>      uint64_t resetvec;
> +    uint32_t num_rnmi_irqvec;
> +    uint64_t *rnmi_irqvec;
> +    uint32_t num_rnmi_excpvec;
> +    uint64_t *rnmi_excpvec;
>      RISCVCPU *harts;
>  };
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 2a78cd639b..5ce6ed31ec 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -1297,6 +1297,11 @@ static void riscv_cpu_set_irq(void *opaque, int irq, int level)
>          g_assert_not_reached();
>      }
>  }
> +
> +static void riscv_cpu_set_nmi(void *opaque, int irq, int level)
> +{
> +    riscv_cpu_set_rnmi(RISCV_CPU(opaque), irq, level);
> +}
>  #endif /* CONFIG_USER_ONLY */
>
>  static bool riscv_cpu_is_dynamic(Object *cpu_obj)
> @@ -1320,6 +1325,8 @@ static void riscv_cpu_init(Object *obj)
>  #ifndef CONFIG_USER_ONLY
>      qdev_init_gpio_in(DEVICE(obj), riscv_cpu_set_irq,
>                        IRQ_LOCAL_MAX + IRQ_LOCAL_GUEST_MAX);
> +    qdev_init_gpio_in_named(DEVICE(cpu), riscv_cpu_set_nmi,
> +                            "riscv.cpu.rnmi", RNMI_MAX);
>  #endif /* CONFIG_USER_ONLY */
>
>      general_user_opts = g_hash_table_new(g_str_hash, g_str_equal);
> @@ -2670,6 +2677,10 @@ static Property riscv_cpu_properties[] = {
>
>  #ifndef CONFIG_USER_ONLY
>      DEFINE_PROP_UINT64("resetvec", RISCVCPU, env.resetvec, DEFAULT_RSTVEC),
> +    DEFINE_PROP_UINT64("rnmi-interrupt-vector", RISCVCPU, env.rnmi_irqvec,
> +                       DEFAULT_RNMI_IRQVEC),
> +    DEFINE_PROP_UINT64("rnmi-exception-vector", RISCVCPU, env.rnmi_excpvec,
> +                       DEFAULT_RNMI_EXCPVEC),
>  #endif
>
>      DEFINE_PROP_BOOL("short-isa-string", RISCVCPU, cfg.short_isa_string, false),
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index a63a29744c..8b2a863c50 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -472,6 +472,11 @@ struct CPUArchState {
>      uint64_t kvm_timer_state;
>      uint64_t kvm_timer_frequency;
>  #endif /* CONFIG_KVM */
> +
> +    /* RNMI */
> +    target_ulong rnmip;
> +    uint64_t rnmi_irqvec;
> +    uint64_t rnmi_excpvec;
>  };
>
>  /*
> @@ -568,6 +573,7 @@ void riscv_cpu_swap_hypervisor_regs(CPURISCVState *env);
>  int riscv_cpu_claim_interrupts(RISCVCPU *cpu, uint64_t interrupts);
>  uint64_t riscv_cpu_update_mip(CPURISCVState *env, uint64_t mask,
>                                uint64_t value);
> +void riscv_cpu_set_rnmi(RISCVCPU *cpu, uint32_t irq, bool level);
>  void riscv_cpu_interrupt(CPURISCVState *env);
>  #define BOOL_TO_MASK(x) (-!!(x)) /* helper for riscv_cpu_update_mip value */
>  void riscv_cpu_set_rdtime_fn(CPURISCVState *env, uint64_t (*fn)(void *),
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index 7e3f629356..6df2cbc2be 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -662,6 +662,12 @@ typedef enum {
>  /* Default Reset Vector address */
>  #define DEFAULT_RSTVEC      0x1000
>
> +/* Default RNMI Interrupt Vector address */
> +#define DEFAULT_RNMI_IRQVEC     0x0
> +
> +/* Default RNMI Exception Vector address */
> +#define DEFAULT_RNMI_EXCPVEC    0x0
> +
>  /* Exception causes */
>  typedef enum RISCVException {
>      RISCV_EXCP_NONE = -1, /* sentinel value */
> @@ -711,6 +717,9 @@ typedef enum RISCVException {
>  /* -1 is due to bit zero of hgeip and hgeie being ROZ. */
>  #define IRQ_LOCAL_GUEST_MAX                (TARGET_LONG_BITS - 1)
>
> +/* RNMI causes */
> +#define RNMI_MAX                           16
> +
>  /* mip masks */
>  #define MIP_USIP                           (1 << IRQ_U_SOFT)
>  #define MIP_SSIP                           (1 << IRQ_S_SOFT)
> @@ -942,6 +951,9 @@ typedef enum RISCVException {
>  #define MHPMEVENT_IDX_MASK                 0xFFFFF
>  #define MHPMEVENT_SSCOF_RESVD              16
>
> +/* RISC-V-specific interrupt pending bits. */
> +#define CPU_INTERRUPT_RNMI                 CPU_INTERRUPT_TGT_EXT_0
> +
>  /* JVT CSR bits */
>  #define JVT_MODE                           0x3F
>  #define JVT_BASE                           (~0x3F)
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 203c0a92ab..977bf97c16 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -434,6 +434,18 @@ static int riscv_cpu_local_irq_pending(CPURISCVState *env)
>      uint64_t vsbits, irq_delegated;
>      int virq;
>
> +    /* Priority: RNMI > Other interrupt. */
> +    if (riscv_cpu_cfg(env)->ext_smrnmi) {
> +        /* If mnstatus.NMIE == 0, all interrupts are disabled. */
> +        if (!get_field(env->mnstatus, MNSTATUS_NMIE)) {
> +            return RISCV_EXCP_NONE;
> +        }
> +
> +        if (env->rnmip) {
> +            return ctz64(env->rnmip); /* since non-zero */
> +        }
> +    }
> +
>      /* Determine interrupt enable state of all privilege modes */
>      if (env->virt_enabled) {
>          mie = 1;
> @@ -496,7 +508,9 @@ static int riscv_cpu_local_irq_pending(CPURISCVState *env)
>
>  bool riscv_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
>  {
> -    if (interrupt_request & CPU_INTERRUPT_HARD) {
> +    uint32_t mask = CPU_INTERRUPT_HARD | CPU_INTERRUPT_RNMI;
> +
> +    if (interrupt_request & mask) {
>          RISCVCPU *cpu = RISCV_CPU(cs);
>          CPURISCVState *env = &cpu->env;
>          int interruptno = riscv_cpu_local_irq_pending(env);
> @@ -619,6 +633,30 @@ void riscv_cpu_set_geilen(CPURISCVState *env, target_ulong geilen)
>      env->geilen = geilen;
>  }
>
> +void riscv_cpu_set_rnmi(RISCVCPU *cpu, uint32_t irq, bool level)
> +{
> +    CPURISCVState *env = &cpu->env;
> +    CPUState *cs = CPU(cpu);
> +    bool release_lock = false;
> +
> +    if (!bql_locked()) {
> +        release_lock = true;
> +        bql_lock();
> +    }
> +
> +    if (level) {
> +        env->rnmip |= 1 << irq;
> +        cpu_interrupt(cs, CPU_INTERRUPT_RNMI);
> +    } else {
> +        env->rnmip &= ~(1 << irq);
> +        cpu_reset_interrupt(cs, CPU_INTERRUPT_RNMI);
> +    }
> +
> +    if (release_lock) {
> +        bql_unlock();
> +    }
> +}
> +
>  int riscv_cpu_claim_interrupts(RISCVCPU *cpu, uint64_t interrupts)
>  {
>      CPURISCVState *env = &cpu->env;
> @@ -1661,6 +1699,7 @@ void riscv_cpu_do_interrupt(CPUState *cs)
>      bool virt = env->virt_enabled;
>      bool write_gva = false;
>      uint64_t s;
> +    int mode;
>
>      /*
>       * cs->exception is 32-bits wide unlike mcause which is XLEN-bits wide
> @@ -1679,6 +1718,23 @@ void riscv_cpu_do_interrupt(CPUState *cs)
>      target_ulong mtval2 = 0;
>      int sxlen = 0;
>      int mxlen = 0;
> +    bool nnmi_excep = false;
> +
> +    if (cpu->cfg.ext_smrnmi && env->rnmip && async) {
> +        env->mnstatus = set_field(env->mnstatus, MNSTATUS_NMIE, false);
> +        env->mnstatus = set_field(env->mnstatus, MNSTATUS_MNPV,
> +                                  env->virt_enabled);
> +        env->mnstatus = set_field(env->mnstatus, MNSTATUS_MNPP,
> +                                  env->priv);
> +        env->mncause = cause | ((target_ulong)1U << (TARGET_LONG_BITS - 1));
> +        env->mnepc = env->pc;
> +        env->pc = env->rnmi_irqvec;
> +
> +        /* Trapping to M mode, virt is disabled */
> +        riscv_cpu_set_mode(env, PRV_M, false);
> +
> +        return;
> +    }
>
>      if (!async) {
>          /* set tval to badaddr for traps with address information */
> @@ -1766,8 +1822,10 @@ void riscv_cpu_do_interrupt(CPUState *cs)
>                    __func__, env->mhartid, async, cause, env->pc, tval,
>                    riscv_cpu_get_trap_name(cause, async));
>
> -    if (env->priv <= PRV_S && cause < 64 &&
> -        (((deleg >> cause) & 1) || s_injected || vs_injected)) {
> +    mode = env->priv <= PRV_S && cause < 64 &&
> +        (((deleg >> cause) & 1) || s_injected || vs_injected) ? PRV_S : PRV_M;
> +
> +    if (mode == PRV_S) {
>          /* handle the trap in S-mode */
>          if (riscv_has_ext(env, RVH)) {
>              uint64_t hdeleg = async ? env->hideleg : env->hedeleg;
> @@ -1845,8 +1903,22 @@ void riscv_cpu_do_interrupt(CPUState *cs)
>          env->mtval = tval;
>          env->mtval2 = mtval2;
>          env->mtinst = tinst;
> -        env->pc = (env->mtvec >> 2 << 2) +
> -                  ((async && (env->mtvec & 3) == 1) ? cause * 4 : 0);
> +
> +        /*
> +         * If the hart encounters an exception while executing in M-mode,
> +         * with the mnstatus.NMIE bit clear, the program counter is set to
> +         * the RNMI exception trap handler address.
> +         */
> +        nnmi_excep = cpu->cfg.ext_smrnmi &&
> +                     !get_field(env->mnstatus, MNSTATUS_NMIE) &&
> +                     !async;
> +
> +        if (nnmi_excep) {
> +            env->pc = env->rnmi_excpvec;
> +        } else {
> +            env->pc = (env->mtvec >> 2 << 2) +
> +                      ((async && (env->mtvec & 3) == 1) ? cause * 4 : 0);
> +        }
>          riscv_cpu_set_mode(env, PRV_M, virt);
>      }
>
> --
> 2.43.2
>
>
Re: [PATCH v8 2/5] target/riscv: Handle Smrnmi interrupt and exception
Posted by Frank Chang 1 year, 2 months ago
On Mon, Nov 18, 2024 at 11:13 AM Alistair Francis <alistair23@gmail.com>
wrote:

> On Mon, Oct 21, 2024 at 1:06 PM <frank.chang@sifive.com> wrote:
> >
> > From: Tommy Wu <tommy.wu@sifive.com>
> >
> > Because the RNMI interrupt trap handler address is implementation
> defined.
> > We add the 'rnmi-interrupt-vector' and 'rnmi-exception-vector' as the
> property
> > of the harts. It’s very easy for users to set the address based on their
> > expectation. This patch also adds the functionality to handle the RNMI
> signals.
> >
> > Signed-off-by: Frank Chang <frank.chang@sifive.com>
> > Signed-off-by: Tommy Wu <tommy.wu@sifive.com>
> > ---
> >  hw/riscv/riscv_hart.c         | 18 ++++++++
> >  include/hw/riscv/riscv_hart.h |  4 ++
> >  target/riscv/cpu.c            | 11 +++++
> >  target/riscv/cpu.h            |  6 +++
> >  target/riscv/cpu_bits.h       | 12 +++++
> >  target/riscv/cpu_helper.c     | 82 ++++++++++++++++++++++++++++++++---
> >  6 files changed, 128 insertions(+), 5 deletions(-)
> >
> > diff --git a/hw/riscv/riscv_hart.c b/hw/riscv/riscv_hart.c
> > index 613ea2aaa0..b7d73f7a82 100644
> > --- a/hw/riscv/riscv_hart.c
> > +++ b/hw/riscv/riscv_hart.c
> > @@ -33,6 +33,12 @@ static Property riscv_harts_props[] = {
> >      DEFINE_PROP_STRING("cpu-type", RISCVHartArrayState, cpu_type),
> >      DEFINE_PROP_UINT64("resetvec", RISCVHartArrayState, resetvec,
> >                         DEFAULT_RSTVEC),
> > +    DEFINE_PROP_ARRAY("rnmi-interrupt-vector", RISCVHartArrayState,
> > +                      num_rnmi_irqvec, rnmi_irqvec, qdev_prop_uint64,
> > +                      uint64_t),
> > +    DEFINE_PROP_ARRAY("rnmi-exception-vector", RISCVHartArrayState,
> > +                      num_rnmi_excpvec, rnmi_excpvec, qdev_prop_uint64,
> > +                      uint64_t),
>
> These will need some documentation somewhere, it's not really clear
> from the names what these do.
>

Hi Alistair,

Thanks for the suggestion, I will add more comment to describe the purposes
of these new properties.


>
> >      DEFINE_PROP_END_OF_LIST(),
> >  };
> >
> > @@ -47,6 +53,18 @@ static bool riscv_hart_realize(RISCVHartArrayState
> *s, int idx,
> >  {
> >      object_initialize_child(OBJECT(s), "harts[*]", &s->harts[idx],
> cpu_type);
> >      qdev_prop_set_uint64(DEVICE(&s->harts[idx]), "resetvec",
> s->resetvec);
> > +    if (s->harts[idx].cfg.ext_smrnmi) {
> > +        if (s->rnmi_irqvec) {
> > +            qdev_prop_set_uint64(DEVICE(&s->harts[idx]),
> > +                                 "rnmi-interrupt-vector",
> > +                                 s->rnmi_irqvec[idx]);
> > +        }
> > +        if (s->rnmi_excpvec) {
> > +            qdev_prop_set_uint64(DEVICE(&s->harts[idx]),
> > +                                 "rnmi-exception-vector",
> > +                                 s->rnmi_excpvec[idx]);
> > +        }
> > +    }
>
> We should probably warn the user if we are ignoring the values that
> they specified because the extension isn't enabled
>

I think we could probably remove if (s->rnmi_irqvec) check as it's still
possible
that people define their RNMI interrupt and exception handler at address:
0x0,
and we don't have a way to distinguish them from the default values (0x0).

Regards,
Frank Chang


>
> Alistair
>
> >      s->harts[idx].env.mhartid = s->hartid_base + idx;
> >      qemu_register_reset(riscv_harts_cpu_reset, &s->harts[idx]);
> >      return qdev_realize(DEVICE(&s->harts[idx]), NULL, errp);
> > diff --git a/include/hw/riscv/riscv_hart.h
> b/include/hw/riscv/riscv_hart.h
> > index 912b4a2682..a6ed73a195 100644
> > --- a/include/hw/riscv/riscv_hart.h
> > +++ b/include/hw/riscv/riscv_hart.h
> > @@ -38,6 +38,10 @@ struct RISCVHartArrayState {
> >      uint32_t hartid_base;
> >      char *cpu_type;
> >      uint64_t resetvec;
> > +    uint32_t num_rnmi_irqvec;
> > +    uint64_t *rnmi_irqvec;
> > +    uint32_t num_rnmi_excpvec;
> > +    uint64_t *rnmi_excpvec;
> >      RISCVCPU *harts;
> >  };
> >
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index 2a78cd639b..5ce6ed31ec 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -1297,6 +1297,11 @@ static void riscv_cpu_set_irq(void *opaque, int
> irq, int level)
> >          g_assert_not_reached();
> >      }
> >  }
> > +
> > +static void riscv_cpu_set_nmi(void *opaque, int irq, int level)
> > +{
> > +    riscv_cpu_set_rnmi(RISCV_CPU(opaque), irq, level);
> > +}
> >  #endif /* CONFIG_USER_ONLY */
> >
> >  static bool riscv_cpu_is_dynamic(Object *cpu_obj)
> > @@ -1320,6 +1325,8 @@ static void riscv_cpu_init(Object *obj)
> >  #ifndef CONFIG_USER_ONLY
> >      qdev_init_gpio_in(DEVICE(obj), riscv_cpu_set_irq,
> >                        IRQ_LOCAL_MAX + IRQ_LOCAL_GUEST_MAX);
> > +    qdev_init_gpio_in_named(DEVICE(cpu), riscv_cpu_set_nmi,
> > +                            "riscv.cpu.rnmi", RNMI_MAX);
> >  #endif /* CONFIG_USER_ONLY */
> >
> >      general_user_opts = g_hash_table_new(g_str_hash, g_str_equal);
> > @@ -2670,6 +2677,10 @@ static Property riscv_cpu_properties[] = {
> >
> >  #ifndef CONFIG_USER_ONLY
> >      DEFINE_PROP_UINT64("resetvec", RISCVCPU, env.resetvec,
> DEFAULT_RSTVEC),
> > +    DEFINE_PROP_UINT64("rnmi-interrupt-vector", RISCVCPU,
> env.rnmi_irqvec,
> > +                       DEFAULT_RNMI_IRQVEC),
> > +    DEFINE_PROP_UINT64("rnmi-exception-vector", RISCVCPU,
> env.rnmi_excpvec,
> > +                       DEFAULT_RNMI_EXCPVEC),
> >  #endif
> >
> >      DEFINE_PROP_BOOL("short-isa-string", RISCVCPU,
> cfg.short_isa_string, false),
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index a63a29744c..8b2a863c50 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -472,6 +472,11 @@ struct CPUArchState {
> >      uint64_t kvm_timer_state;
> >      uint64_t kvm_timer_frequency;
> >  #endif /* CONFIG_KVM */
> > +
> > +    /* RNMI */
> > +    target_ulong rnmip;
> > +    uint64_t rnmi_irqvec;
> > +    uint64_t rnmi_excpvec;
> >  };
> >
> >  /*
> > @@ -568,6 +573,7 @@ void riscv_cpu_swap_hypervisor_regs(CPURISCVState
> *env);
> >  int riscv_cpu_claim_interrupts(RISCVCPU *cpu, uint64_t interrupts);
> >  uint64_t riscv_cpu_update_mip(CPURISCVState *env, uint64_t mask,
> >                                uint64_t value);
> > +void riscv_cpu_set_rnmi(RISCVCPU *cpu, uint32_t irq, bool level);
> >  void riscv_cpu_interrupt(CPURISCVState *env);
> >  #define BOOL_TO_MASK(x) (-!!(x)) /* helper for riscv_cpu_update_mip
> value */
> >  void riscv_cpu_set_rdtime_fn(CPURISCVState *env, uint64_t (*fn)(void *),
> > diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> > index 7e3f629356..6df2cbc2be 100644
> > --- a/target/riscv/cpu_bits.h
> > +++ b/target/riscv/cpu_bits.h
> > @@ -662,6 +662,12 @@ typedef enum {
> >  /* Default Reset Vector address */
> >  #define DEFAULT_RSTVEC      0x1000
> >
> > +/* Default RNMI Interrupt Vector address */
> > +#define DEFAULT_RNMI_IRQVEC     0x0
> > +
> > +/* Default RNMI Exception Vector address */
> > +#define DEFAULT_RNMI_EXCPVEC    0x0
> > +
> >  /* Exception causes */
> >  typedef enum RISCVException {
> >      RISCV_EXCP_NONE = -1, /* sentinel value */
> > @@ -711,6 +717,9 @@ typedef enum RISCVException {
> >  /* -1 is due to bit zero of hgeip and hgeie being ROZ. */
> >  #define IRQ_LOCAL_GUEST_MAX                (TARGET_LONG_BITS - 1)
> >
> > +/* RNMI causes */
> > +#define RNMI_MAX                           16
> > +
> >  /* mip masks */
> >  #define MIP_USIP                           (1 << IRQ_U_SOFT)
> >  #define MIP_SSIP                           (1 << IRQ_S_SOFT)
> > @@ -942,6 +951,9 @@ typedef enum RISCVException {
> >  #define MHPMEVENT_IDX_MASK                 0xFFFFF
> >  #define MHPMEVENT_SSCOF_RESVD              16
> >
> > +/* RISC-V-specific interrupt pending bits. */
> > +#define CPU_INTERRUPT_RNMI                 CPU_INTERRUPT_TGT_EXT_0
> > +
> >  /* JVT CSR bits */
> >  #define JVT_MODE                           0x3F
> >  #define JVT_BASE                           (~0x3F)
> > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > index 203c0a92ab..977bf97c16 100644
> > --- a/target/riscv/cpu_helper.c
> > +++ b/target/riscv/cpu_helper.c
> > @@ -434,6 +434,18 @@ static int
> riscv_cpu_local_irq_pending(CPURISCVState *env)
> >      uint64_t vsbits, irq_delegated;
> >      int virq;
> >
> > +    /* Priority: RNMI > Other interrupt. */
> > +    if (riscv_cpu_cfg(env)->ext_smrnmi) {
> > +        /* If mnstatus.NMIE == 0, all interrupts are disabled. */
> > +        if (!get_field(env->mnstatus, MNSTATUS_NMIE)) {
> > +            return RISCV_EXCP_NONE;
> > +        }
> > +
> > +        if (env->rnmip) {
> > +            return ctz64(env->rnmip); /* since non-zero */
> > +        }
> > +    }
> > +
> >      /* Determine interrupt enable state of all privilege modes */
> >      if (env->virt_enabled) {
> >          mie = 1;
> > @@ -496,7 +508,9 @@ static int riscv_cpu_local_irq_pending(CPURISCVState
> *env)
> >
> >  bool riscv_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
> >  {
> > -    if (interrupt_request & CPU_INTERRUPT_HARD) {
> > +    uint32_t mask = CPU_INTERRUPT_HARD | CPU_INTERRUPT_RNMI;
> > +
> > +    if (interrupt_request & mask) {
> >          RISCVCPU *cpu = RISCV_CPU(cs);
> >          CPURISCVState *env = &cpu->env;
> >          int interruptno = riscv_cpu_local_irq_pending(env);
> > @@ -619,6 +633,30 @@ void riscv_cpu_set_geilen(CPURISCVState *env,
> target_ulong geilen)
> >      env->geilen = geilen;
> >  }
> >
> > +void riscv_cpu_set_rnmi(RISCVCPU *cpu, uint32_t irq, bool level)
> > +{
> > +    CPURISCVState *env = &cpu->env;
> > +    CPUState *cs = CPU(cpu);
> > +    bool release_lock = false;
> > +
> > +    if (!bql_locked()) {
> > +        release_lock = true;
> > +        bql_lock();
> > +    }
> > +
> > +    if (level) {
> > +        env->rnmip |= 1 << irq;
> > +        cpu_interrupt(cs, CPU_INTERRUPT_RNMI);
> > +    } else {
> > +        env->rnmip &= ~(1 << irq);
> > +        cpu_reset_interrupt(cs, CPU_INTERRUPT_RNMI);
> > +    }
> > +
> > +    if (release_lock) {
> > +        bql_unlock();
> > +    }
> > +}
> > +
> >  int riscv_cpu_claim_interrupts(RISCVCPU *cpu, uint64_t interrupts)
> >  {
> >      CPURISCVState *env = &cpu->env;
> > @@ -1661,6 +1699,7 @@ void riscv_cpu_do_interrupt(CPUState *cs)
> >      bool virt = env->virt_enabled;
> >      bool write_gva = false;
> >      uint64_t s;
> > +    int mode;
> >
> >      /*
> >       * cs->exception is 32-bits wide unlike mcause which is XLEN-bits
> wide
> > @@ -1679,6 +1718,23 @@ void riscv_cpu_do_interrupt(CPUState *cs)
> >      target_ulong mtval2 = 0;
> >      int sxlen = 0;
> >      int mxlen = 0;
> > +    bool nnmi_excep = false;
> > +
> > +    if (cpu->cfg.ext_smrnmi && env->rnmip && async) {
> > +        env->mnstatus = set_field(env->mnstatus, MNSTATUS_NMIE, false);
> > +        env->mnstatus = set_field(env->mnstatus, MNSTATUS_MNPV,
> > +                                  env->virt_enabled);
> > +        env->mnstatus = set_field(env->mnstatus, MNSTATUS_MNPP,
> > +                                  env->priv);
> > +        env->mncause = cause | ((target_ulong)1U << (TARGET_LONG_BITS -
> 1));
> > +        env->mnepc = env->pc;
> > +        env->pc = env->rnmi_irqvec;
> > +
> > +        /* Trapping to M mode, virt is disabled */
> > +        riscv_cpu_set_mode(env, PRV_M, false);
> > +
> > +        return;
> > +    }
> >
> >      if (!async) {
> >          /* set tval to badaddr for traps with address information */
> > @@ -1766,8 +1822,10 @@ void riscv_cpu_do_interrupt(CPUState *cs)
> >                    __func__, env->mhartid, async, cause, env->pc, tval,
> >                    riscv_cpu_get_trap_name(cause, async));
> >
> > -    if (env->priv <= PRV_S && cause < 64 &&
> > -        (((deleg >> cause) & 1) || s_injected || vs_injected)) {
> > +    mode = env->priv <= PRV_S && cause < 64 &&
> > +        (((deleg >> cause) & 1) || s_injected || vs_injected) ? PRV_S :
> PRV_M;
> > +
> > +    if (mode == PRV_S) {
> >          /* handle the trap in S-mode */
> >          if (riscv_has_ext(env, RVH)) {
> >              uint64_t hdeleg = async ? env->hideleg : env->hedeleg;
> > @@ -1845,8 +1903,22 @@ void riscv_cpu_do_interrupt(CPUState *cs)
> >          env->mtval = tval;
> >          env->mtval2 = mtval2;
> >          env->mtinst = tinst;
> > -        env->pc = (env->mtvec >> 2 << 2) +
> > -                  ((async && (env->mtvec & 3) == 1) ? cause * 4 : 0);
> > +
> > +        /*
> > +         * If the hart encounters an exception while executing in
> M-mode,
> > +         * with the mnstatus.NMIE bit clear, the program counter is set
> to
> > +         * the RNMI exception trap handler address.
> > +         */
> > +        nnmi_excep = cpu->cfg.ext_smrnmi &&
> > +                     !get_field(env->mnstatus, MNSTATUS_NMIE) &&
> > +                     !async;
> > +
> > +        if (nnmi_excep) {
> > +            env->pc = env->rnmi_excpvec;
> > +        } else {
> > +            env->pc = (env->mtvec >> 2 << 2) +
> > +                      ((async && (env->mtvec & 3) == 1) ? cause * 4 :
> 0);
> > +        }
> >          riscv_cpu_set_mode(env, PRV_M, virt);
> >      }
> >
> > --
> > 2.43.2
> >
> >
>
Re: [PATCH v8 2/5] target/riscv: Handle Smrnmi interrupt and exception
Posted by Frank Chang 1 year, 2 months ago
On Wed, Nov 20, 2024 at 11:29 AM Frank Chang <frank.chang@sifive.com> wrote:

> On Mon, Nov 18, 2024 at 11:13 AM Alistair Francis <alistair23@gmail.com>
> wrote:
>
>> On Mon, Oct 21, 2024 at 1:06 PM <frank.chang@sifive.com> wrote:
>> >
>> > From: Tommy Wu <tommy.wu@sifive.com>
>> >
>> > Because the RNMI interrupt trap handler address is implementation
>> defined.
>> > We add the 'rnmi-interrupt-vector' and 'rnmi-exception-vector' as the
>> property
>> > of the harts. It’s very easy for users to set the address based on their
>> > expectation. This patch also adds the functionality to handle the RNMI
>> signals.
>> >
>> > Signed-off-by: Frank Chang <frank.chang@sifive.com>
>> > Signed-off-by: Tommy Wu <tommy.wu@sifive.com>
>> > ---
>> >  hw/riscv/riscv_hart.c         | 18 ++++++++
>> >  include/hw/riscv/riscv_hart.h |  4 ++
>> >  target/riscv/cpu.c            | 11 +++++
>> >  target/riscv/cpu.h            |  6 +++
>> >  target/riscv/cpu_bits.h       | 12 +++++
>> >  target/riscv/cpu_helper.c     | 82 ++++++++++++++++++++++++++++++++---
>> >  6 files changed, 128 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/hw/riscv/riscv_hart.c b/hw/riscv/riscv_hart.c
>> > index 613ea2aaa0..b7d73f7a82 100644
>> > --- a/hw/riscv/riscv_hart.c
>> > +++ b/hw/riscv/riscv_hart.c
>> > @@ -33,6 +33,12 @@ static Property riscv_harts_props[] = {
>> >      DEFINE_PROP_STRING("cpu-type", RISCVHartArrayState, cpu_type),
>> >      DEFINE_PROP_UINT64("resetvec", RISCVHartArrayState, resetvec,
>> >                         DEFAULT_RSTVEC),
>> > +    DEFINE_PROP_ARRAY("rnmi-interrupt-vector", RISCVHartArrayState,
>> > +                      num_rnmi_irqvec, rnmi_irqvec, qdev_prop_uint64,
>> > +                      uint64_t),
>> > +    DEFINE_PROP_ARRAY("rnmi-exception-vector", RISCVHartArrayState,
>> > +                      num_rnmi_excpvec, rnmi_excpvec, qdev_prop_uint64,
>> > +                      uint64_t),
>>
>> These will need some documentation somewhere, it's not really clear
>> from the names what these do.
>>
>
> Hi Alistair,
>
> Thanks for the suggestion, I will add more comment to describe the purposes
> of these new properties.
>
>
>>
>> >      DEFINE_PROP_END_OF_LIST(),
>> >  };
>> >
>> > @@ -47,6 +53,18 @@ static bool riscv_hart_realize(RISCVHartArrayState
>> *s, int idx,
>> >  {
>> >      object_initialize_child(OBJECT(s), "harts[*]", &s->harts[idx],
>> cpu_type);
>> >      qdev_prop_set_uint64(DEVICE(&s->harts[idx]), "resetvec",
>> s->resetvec);
>> > +    if (s->harts[idx].cfg.ext_smrnmi) {
>> > +        if (s->rnmi_irqvec) {
>> > +            qdev_prop_set_uint64(DEVICE(&s->harts[idx]),
>> > +                                 "rnmi-interrupt-vector",
>> > +                                 s->rnmi_irqvec[idx]);
>> > +        }
>> > +        if (s->rnmi_excpvec) {
>> > +            qdev_prop_set_uint64(DEVICE(&s->harts[idx]),
>> > +                                 "rnmi-exception-vector",
>> > +                                 s->rnmi_excpvec[idx]);
>> > +        }
>> > +    }
>>
>> We should probably warn the user if we are ignoring the values that
>> they specified because the extension isn't enabled
>>
>
> I think we could probably remove if (s->rnmi_irqvec) check as it's still
> possible
> that people define their RNMI interrupt and exception handler at address:
> 0x0,
> and we don't have a way to distinguish them from the default values (0x0).
>
> Regards,
> Frank Chang
>

Hi Alistair,

Sorry that I was confused about "rnmi-interrupt-vector" and
"rnmi-exception-vector" properties
defined in RISCVHartArrayState and RISCVCPU.

Yes, you're correct, we should add the warning message if Smrnmi extension
is not enabled.
I've added it in the v9 patchset:
https://lore.kernel.org/qemu-riscv/20241122032217.3816540-1-frank.chang@sifive.com/T/#t
I also updated the existence checks, replacing "s->rnmi_irqvec" and
"s->rnmi_excpvec" with
"s->num_rnmi_irqvec" and "s->num_rnmi_excpvec".

Regards,
Frank Chang


>
>
>>
>> Alistair
>>
>> >      s->harts[idx].env.mhartid = s->hartid_base + idx;
>> >      qemu_register_reset(riscv_harts_cpu_reset, &s->harts[idx]);
>> >      return qdev_realize(DEVICE(&s->harts[idx]), NULL, errp);
>> > diff --git a/include/hw/riscv/riscv_hart.h
>> b/include/hw/riscv/riscv_hart.h
>> > index 912b4a2682..a6ed73a195 100644
>> > --- a/include/hw/riscv/riscv_hart.h
>> > +++ b/include/hw/riscv/riscv_hart.h
>> > @@ -38,6 +38,10 @@ struct RISCVHartArrayState {
>> >      uint32_t hartid_base;
>> >      char *cpu_type;
>> >      uint64_t resetvec;
>> > +    uint32_t num_rnmi_irqvec;
>> > +    uint64_t *rnmi_irqvec;
>> > +    uint32_t num_rnmi_excpvec;
>> > +    uint64_t *rnmi_excpvec;
>> >      RISCVCPU *harts;
>> >  };
>> >
>> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> > index 2a78cd639b..5ce6ed31ec 100644
>> > --- a/target/riscv/cpu.c
>> > +++ b/target/riscv/cpu.c
>> > @@ -1297,6 +1297,11 @@ static void riscv_cpu_set_irq(void *opaque, int
>> irq, int level)
>> >          g_assert_not_reached();
>> >      }
>> >  }
>> > +
>> > +static void riscv_cpu_set_nmi(void *opaque, int irq, int level)
>> > +{
>> > +    riscv_cpu_set_rnmi(RISCV_CPU(opaque), irq, level);
>> > +}
>> >  #endif /* CONFIG_USER_ONLY */
>> >
>> >  static bool riscv_cpu_is_dynamic(Object *cpu_obj)
>> > @@ -1320,6 +1325,8 @@ static void riscv_cpu_init(Object *obj)
>> >  #ifndef CONFIG_USER_ONLY
>> >      qdev_init_gpio_in(DEVICE(obj), riscv_cpu_set_irq,
>> >                        IRQ_LOCAL_MAX + IRQ_LOCAL_GUEST_MAX);
>> > +    qdev_init_gpio_in_named(DEVICE(cpu), riscv_cpu_set_nmi,
>> > +                            "riscv.cpu.rnmi", RNMI_MAX);
>> >  #endif /* CONFIG_USER_ONLY */
>> >
>> >      general_user_opts = g_hash_table_new(g_str_hash, g_str_equal);
>> > @@ -2670,6 +2677,10 @@ static Property riscv_cpu_properties[] = {
>> >
>> >  #ifndef CONFIG_USER_ONLY
>> >      DEFINE_PROP_UINT64("resetvec", RISCVCPU, env.resetvec,
>> DEFAULT_RSTVEC),
>> > +    DEFINE_PROP_UINT64("rnmi-interrupt-vector", RISCVCPU,
>> env.rnmi_irqvec,
>> > +                       DEFAULT_RNMI_IRQVEC),
>> > +    DEFINE_PROP_UINT64("rnmi-exception-vector", RISCVCPU,
>> env.rnmi_excpvec,
>> > +                       DEFAULT_RNMI_EXCPVEC),
>> >  #endif
>> >
>> >      DEFINE_PROP_BOOL("short-isa-string", RISCVCPU,
>> cfg.short_isa_string, false),
>> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
>> > index a63a29744c..8b2a863c50 100644
>> > --- a/target/riscv/cpu.h
>> > +++ b/target/riscv/cpu.h
>> > @@ -472,6 +472,11 @@ struct CPUArchState {
>> >      uint64_t kvm_timer_state;
>> >      uint64_t kvm_timer_frequency;
>> >  #endif /* CONFIG_KVM */
>> > +
>> > +    /* RNMI */
>> > +    target_ulong rnmip;
>> > +    uint64_t rnmi_irqvec;
>> > +    uint64_t rnmi_excpvec;
>> >  };
>> >
>> >  /*
>> > @@ -568,6 +573,7 @@ void riscv_cpu_swap_hypervisor_regs(CPURISCVState
>> *env);
>> >  int riscv_cpu_claim_interrupts(RISCVCPU *cpu, uint64_t interrupts);
>> >  uint64_t riscv_cpu_update_mip(CPURISCVState *env, uint64_t mask,
>> >                                uint64_t value);
>> > +void riscv_cpu_set_rnmi(RISCVCPU *cpu, uint32_t irq, bool level);
>> >  void riscv_cpu_interrupt(CPURISCVState *env);
>> >  #define BOOL_TO_MASK(x) (-!!(x)) /* helper for riscv_cpu_update_mip
>> value */
>> >  void riscv_cpu_set_rdtime_fn(CPURISCVState *env, uint64_t (*fn)(void
>> *),
>> > diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
>> > index 7e3f629356..6df2cbc2be 100644
>> > --- a/target/riscv/cpu_bits.h
>> > +++ b/target/riscv/cpu_bits.h
>> > @@ -662,6 +662,12 @@ typedef enum {
>> >  /* Default Reset Vector address */
>> >  #define DEFAULT_RSTVEC      0x1000
>> >
>> > +/* Default RNMI Interrupt Vector address */
>> > +#define DEFAULT_RNMI_IRQVEC     0x0
>> > +
>> > +/* Default RNMI Exception Vector address */
>> > +#define DEFAULT_RNMI_EXCPVEC    0x0
>> > +
>> >  /* Exception causes */
>> >  typedef enum RISCVException {
>> >      RISCV_EXCP_NONE = -1, /* sentinel value */
>> > @@ -711,6 +717,9 @@ typedef enum RISCVException {
>> >  /* -1 is due to bit zero of hgeip and hgeie being ROZ. */
>> >  #define IRQ_LOCAL_GUEST_MAX                (TARGET_LONG_BITS - 1)
>> >
>> > +/* RNMI causes */
>> > +#define RNMI_MAX                           16
>> > +
>> >  /* mip masks */
>> >  #define MIP_USIP                           (1 << IRQ_U_SOFT)
>> >  #define MIP_SSIP                           (1 << IRQ_S_SOFT)
>> > @@ -942,6 +951,9 @@ typedef enum RISCVException {
>> >  #define MHPMEVENT_IDX_MASK                 0xFFFFF
>> >  #define MHPMEVENT_SSCOF_RESVD              16
>> >
>> > +/* RISC-V-specific interrupt pending bits. */
>> > +#define CPU_INTERRUPT_RNMI                 CPU_INTERRUPT_TGT_EXT_0
>> > +
>> >  /* JVT CSR bits */
>> >  #define JVT_MODE                           0x3F
>> >  #define JVT_BASE                           (~0x3F)
>> > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
>> > index 203c0a92ab..977bf97c16 100644
>> > --- a/target/riscv/cpu_helper.c
>> > +++ b/target/riscv/cpu_helper.c
>> > @@ -434,6 +434,18 @@ static int
>> riscv_cpu_local_irq_pending(CPURISCVState *env)
>> >      uint64_t vsbits, irq_delegated;
>> >      int virq;
>> >
>> > +    /* Priority: RNMI > Other interrupt. */
>> > +    if (riscv_cpu_cfg(env)->ext_smrnmi) {
>> > +        /* If mnstatus.NMIE == 0, all interrupts are disabled. */
>> > +        if (!get_field(env->mnstatus, MNSTATUS_NMIE)) {
>> > +            return RISCV_EXCP_NONE;
>> > +        }
>> > +
>> > +        if (env->rnmip) {
>> > +            return ctz64(env->rnmip); /* since non-zero */
>> > +        }
>> > +    }
>> > +
>> >      /* Determine interrupt enable state of all privilege modes */
>> >      if (env->virt_enabled) {
>> >          mie = 1;
>> > @@ -496,7 +508,9 @@ static int
>> riscv_cpu_local_irq_pending(CPURISCVState *env)
>> >
>> >  bool riscv_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
>> >  {
>> > -    if (interrupt_request & CPU_INTERRUPT_HARD) {
>> > +    uint32_t mask = CPU_INTERRUPT_HARD | CPU_INTERRUPT_RNMI;
>> > +
>> > +    if (interrupt_request & mask) {
>> >          RISCVCPU *cpu = RISCV_CPU(cs);
>> >          CPURISCVState *env = &cpu->env;
>> >          int interruptno = riscv_cpu_local_irq_pending(env);
>> > @@ -619,6 +633,30 @@ void riscv_cpu_set_geilen(CPURISCVState *env,
>> target_ulong geilen)
>> >      env->geilen = geilen;
>> >  }
>> >
>> > +void riscv_cpu_set_rnmi(RISCVCPU *cpu, uint32_t irq, bool level)
>> > +{
>> > +    CPURISCVState *env = &cpu->env;
>> > +    CPUState *cs = CPU(cpu);
>> > +    bool release_lock = false;
>> > +
>> > +    if (!bql_locked()) {
>> > +        release_lock = true;
>> > +        bql_lock();
>> > +    }
>> > +
>> > +    if (level) {
>> > +        env->rnmip |= 1 << irq;
>> > +        cpu_interrupt(cs, CPU_INTERRUPT_RNMI);
>> > +    } else {
>> > +        env->rnmip &= ~(1 << irq);
>> > +        cpu_reset_interrupt(cs, CPU_INTERRUPT_RNMI);
>> > +    }
>> > +
>> > +    if (release_lock) {
>> > +        bql_unlock();
>> > +    }
>> > +}
>> > +
>> >  int riscv_cpu_claim_interrupts(RISCVCPU *cpu, uint64_t interrupts)
>> >  {
>> >      CPURISCVState *env = &cpu->env;
>> > @@ -1661,6 +1699,7 @@ void riscv_cpu_do_interrupt(CPUState *cs)
>> >      bool virt = env->virt_enabled;
>> >      bool write_gva = false;
>> >      uint64_t s;
>> > +    int mode;
>> >
>> >      /*
>> >       * cs->exception is 32-bits wide unlike mcause which is XLEN-bits
>> wide
>> > @@ -1679,6 +1718,23 @@ void riscv_cpu_do_interrupt(CPUState *cs)
>> >      target_ulong mtval2 = 0;
>> >      int sxlen = 0;
>> >      int mxlen = 0;
>> > +    bool nnmi_excep = false;
>> > +
>> > +    if (cpu->cfg.ext_smrnmi && env->rnmip && async) {
>> > +        env->mnstatus = set_field(env->mnstatus, MNSTATUS_NMIE, false);
>> > +        env->mnstatus = set_field(env->mnstatus, MNSTATUS_MNPV,
>> > +                                  env->virt_enabled);
>> > +        env->mnstatus = set_field(env->mnstatus, MNSTATUS_MNPP,
>> > +                                  env->priv);
>> > +        env->mncause = cause | ((target_ulong)1U << (TARGET_LONG_BITS
>> - 1));
>> > +        env->mnepc = env->pc;
>> > +        env->pc = env->rnmi_irqvec;
>> > +
>> > +        /* Trapping to M mode, virt is disabled */
>> > +        riscv_cpu_set_mode(env, PRV_M, false);
>> > +
>> > +        return;
>> > +    }
>> >
>> >      if (!async) {
>> >          /* set tval to badaddr for traps with address information */
>> > @@ -1766,8 +1822,10 @@ void riscv_cpu_do_interrupt(CPUState *cs)
>> >                    __func__, env->mhartid, async, cause, env->pc, tval,
>> >                    riscv_cpu_get_trap_name(cause, async));
>> >
>> > -    if (env->priv <= PRV_S && cause < 64 &&
>> > -        (((deleg >> cause) & 1) || s_injected || vs_injected)) {
>> > +    mode = env->priv <= PRV_S && cause < 64 &&
>> > +        (((deleg >> cause) & 1) || s_injected || vs_injected) ? PRV_S
>> : PRV_M;
>> > +
>> > +    if (mode == PRV_S) {
>> >          /* handle the trap in S-mode */
>> >          if (riscv_has_ext(env, RVH)) {
>> >              uint64_t hdeleg = async ? env->hideleg : env->hedeleg;
>> > @@ -1845,8 +1903,22 @@ void riscv_cpu_do_interrupt(CPUState *cs)
>> >          env->mtval = tval;
>> >          env->mtval2 = mtval2;
>> >          env->mtinst = tinst;
>> > -        env->pc = (env->mtvec >> 2 << 2) +
>> > -                  ((async && (env->mtvec & 3) == 1) ? cause * 4 : 0);
>> > +
>> > +        /*
>> > +         * If the hart encounters an exception while executing in
>> M-mode,
>> > +         * with the mnstatus.NMIE bit clear, the program counter is
>> set to
>> > +         * the RNMI exception trap handler address.
>> > +         */
>> > +        nnmi_excep = cpu->cfg.ext_smrnmi &&
>> > +                     !get_field(env->mnstatus, MNSTATUS_NMIE) &&
>> > +                     !async;
>> > +
>> > +        if (nnmi_excep) {
>> > +            env->pc = env->rnmi_excpvec;
>> > +        } else {
>> > +            env->pc = (env->mtvec >> 2 << 2) +
>> > +                      ((async && (env->mtvec & 3) == 1) ? cause * 4 :
>> 0);
>> > +        }
>> >          riscv_cpu_set_mode(env, PRV_M, virt);
>> >      }
>> >
>> > --
>> > 2.43.2
>> >
>> >
>>
>