[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v6 2/5] target/riscv: Handle Smrnmi interrupt and exception.
|
From: |
Clément Léger |
|
Subject: |
Re: [PATCH v6 2/5] target/riscv: Handle Smrnmi interrupt and exception. |
|
Date: |
Mon, 2 Sep 2024 14:49:34 +0200 |
|
User-agent: |
Mozilla Thunderbird |
On 02/09/2024 12:34, Clément Léger wrote:
>
>
> On 02/09/2024 09:13, Tommy Wu wrote:
>> 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 | 80 ++++++++++++++++++++++++++++++++---
>> 6 files changed, 126 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 a90808a3ba..2f64b3df22 100644
>> --- a/target/riscv/cpu.c
>> +++ b/target/riscv/cpu.c
>> @@ -1309,6 +1309,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)
>> @@ -1332,6 +1337,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);
>> @@ -2681,6 +2688,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 1619c3acb6..d8ad04ec31 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 32b068f18a..e14b654c35 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 395a1d9140..9d0400035f 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;
>> @@ -1654,6 +1692,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
>> @@ -1670,6 +1709,20 @@ void riscv_cpu_do_interrupt(CPUState *cs)
>> target_ulong tinst = 0;
>> target_ulong htval = 0;
>> target_ulong mtval2 = 0;
>> + bool nmi_execp = 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;
>> + riscv_cpu_set_mode(env, PRV_M, virt);
>> + return;
>> + }
>>
>> if (!async) {
>> /* set tval to badaddr for traps with address information */
>> @@ -1755,8 +1808,20 @@ 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 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.
>> + */
>> + nmi_execp = cpu->cfg.ext_smrnmi &&
>> + !get_field(env->mnstatus, MNSTATUS_NMIE) &&
>> + !async &&
>> + mode == PRV_M;
>
> Since nmi_excep is only use in the else statement (mode == PRV_M), maybe
> it would be better to move the computation there. And it will also allow
> to get rid of the mode == PRV_M check in the boolean expression.
>
> BTW, I think the variable should be named "nnmi_excep" rather than
> "nmi_execp".
>
> Thanks,
>
> Clément
>
>> +
>> + if (mode == PRV_S) {
>> /* handle the trap in S-mode */
>> if (riscv_has_ext(env, RVH)) {
>> uint64_t hdeleg = async ? env->hideleg : env->hedeleg;
>> @@ -1832,8 +1897,13 @@ 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 (cpu->cfg.ext_smrnmi && nmi_execp) {
BTW, since cpu->cfg.ext_smrnmi is already part of nmi_execp boolean
expression, you can remove it from there.
Thanks,
Clément
>> + 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);
>> }
>>
>
- [PATCH v6 0/5] target/riscv: Add Smrnmi support., Tommy Wu, 2024/09/02
- [PATCH v6 1/5] target/riscv: Add `ext_smrnmi` in the RISCVCPUConfig., Tommy Wu, 2024/09/02
- [PATCH v6 2/5] target/riscv: Handle Smrnmi interrupt and exception., Tommy Wu, 2024/09/02
- [PATCH v6 3/5] target/riscv: Add Smrnmi CSRs., Tommy Wu, 2024/09/02
- [PATCH v6 4/5] target/riscv: Add Smrnmi mnret instruction., Tommy Wu, 2024/09/02
- [PATCH v6 5/5] target/riscv: Add Smrnmi cpu extension., Tommy Wu, 2024/09/02
- Re: [PATCH v6 0/5] target/riscv: Add Smrnmi support., Alistair Francis, 2024/09/05