Re: [RFC PATCH 11/11] target/riscv: Update interrupt return in CLIC mode

From: Frank Chang <frank.chang@sifive.com>
To: LIU Zhiwei <zhiwei_liu@c-sky.com>
Cc: Palmer Dabbelt <palmer@dabbelt.com>,
	Alistair Francis <Alistair.Francis@wdc.com>,
	"open list:RISC-V" <qemu-riscv@nongnu.org>,
	"qemu-devel@nongnu.org Developers" <qemu-devel@nongnu.org>,
	wxy194768@alibaba-inc.com
Subject: Re: [RFC PATCH 11/11] target/riscv: Update interrupt return in CLIC mode
Date: Sun, 27 Jun 2021 20:08:29 +0800	[thread overview]
Message-ID: <CANzO1D3oQJHHDzXgFN6yb3xsp=WrVwohmkNAG01W+HGVnCCZZA@mail.gmail.com> (raw)
In-Reply-To: <20210409074857.166082-12-zhiwei_liu@c-sky.com>

[-- Attachment #1: Type: text/plain, Size: 3056 bytes --]

LIU Zhiwei <zhiwei_liu@c-sky.com> 於 2021年4月9日 週五 下午3:55寫道：

> When a vectored interrupt is selected and serviced, the hardware will
> automatically clear the corresponding pending bit in edge-triggered mode.
> This may lead to a lower priviledge interrupt pending forever.
>
> Therefore when interrupts return, pull a pending interrupt to service.
>
> Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
> ---
>  target/riscv/op_helper.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>
> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> index 1eddcb94de..42563b22ba 100644
> --- a/target/riscv/op_helper.c
> +++ b/target/riscv/op_helper.c
> @@ -24,6 +24,10 @@
>  #include "exec/exec-all.h"
>  #include "exec/helper-proto.h"
>
> +#if !defined(CONFIG_USER_ONLY)
> +#include "hw/intc/riscv_clic.h"
> +#endif
> +
>  /* Exceptions processing helpers */
>  void QEMU_NORETURN riscv_raise_exception(CPURISCVState *env,
>                                            uint32_t exception, uintptr_t
> pc)
> @@ -130,6 +134,17 @@ target_ulong helper_sret(CPURISCVState *env,
> target_ulong cpu_pc_deb)
>          mstatus = set_field(mstatus, MSTATUS_SPIE, 1);
>          mstatus = set_field(mstatus, MSTATUS_SPP, PRV_U);
>          env->mstatus = mstatus;
> +
> +        if (riscv_clic_is_clic_mode(env)) {
> +            CPUState *cs = env_cpu(env);
> +            target_ulong spil = get_field(env->scause, SCAUSE_SPIL);
> +            env->mintstatus = set_field(env->mintstatus, MINTSTATUS_SIL,
> spil);
> +            env->scause = set_field(env->scause, SCAUSE_SPIE, 0);
>

Should scause.spie set to 1?

> +            env->scause = set_field(env->scause, SCAUSE_SPP, PRV_U);
> +            qemu_mutex_lock_iothread();
> +            riscv_clic_get_next_interrupt(env->clic, cs->cpu_index);
> +            qemu_mutex_unlock_iothread();
> +        }
>      }
>
>      riscv_cpu_set_mode(env, prev_priv);
> @@ -172,6 +187,16 @@ target_ulong helper_mret(CPURISCVState *env,
> target_ulong cpu_pc_deb)
>          riscv_cpu_set_virt_enabled(env, prev_virt);
>      }
>
> +    if (riscv_clic_is_clic_mode(env)) {
> +        CPUState *cs = env_cpu(env);
> +        target_ulong mpil = get_field(env->mcause, MCAUSE_MPIL);
> +        env->mintstatus = set_field(env->mintstatus, MINTSTATUS_MIL,
> mpil);
> +        env->mcause = set_field(env->mcause, MCAUSE_MPIE, 0);
>

Should mcause.mpie set to 1?
  The xcause.xpp and xcause.xpie fields are modified following the behavior
  previously defined for xstatus.xpp and xstatus.xpie respectively.

RISC-V Privilege spec:
  When executing an xRET instruction, xPIE is set to 1.

Regards,
Frank Chang

> +        env->mcause = set_field(env->mcause, MCAUSE_MPP, PRV_U);
> +        qemu_mutex_lock_iothread();
> +        riscv_clic_get_next_interrupt(env->clic, cs->cpu_index);
> +        qemu_mutex_unlock_iothread();
> +    }
>      return retpc;
>  }
>
> --
> 2.25.1
>
>
>

[-- Attachment #2: Type: text/html, Size: 4162 bytes --]