On Wed, Nov 20, 2024 at 11:29 AM Frank Chang <frank.ch...@sifive.com> wrote:
> On Mon, Nov 18, 2024 at 11:13 AM Alistair Francis <alistai...@gmail.com>
> wrote:
>
>> On Mon, Oct 21, 2024 at 1:06 PM <frank.ch...@sifive.com> wrote:
>> >
>> > From: Tommy Wu <tommy...@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.ch...@sifive.com>
>> > Signed-off-by: Tommy Wu <tommy...@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.ch...@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
>> >
>> >
>>
>
