[PATCH] target/riscv: Fix itrigger when icount is used

LIU Zhiwei posted 1 patch 1 year, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20230324064011.976-1-zhiwei._5Fliu@linux.alibaba.com
Maintainers: Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Bin Meng <bin.meng@windriver.com>, Weiwei Li <liweiwei@iscas.ac.cn>, Daniel Henrique Barboza <dbarboza@ventanamicro.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>
target/riscv/insn_trans/trans_privileged.c.inc | 6 ++++++
1 file changed, 6 insertions(+)
[PATCH] target/riscv: Fix itrigger when icount is used
Posted by LIU Zhiwei 1 year, 1 month ago
When I boot a ubuntu image, QEMU output a "Bad icount read" message and exit.
The reason is that when execute helper_mret or helper_sret, it will
cause a call to icount_get_raw_locked (), which needs set can_do_io flag
on cpustate.

Thus we setting this flag when execute these two instructions.

Signed-off-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
---
 target/riscv/insn_trans/trans_privileged.c.inc | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/target/riscv/insn_trans/trans_privileged.c.inc b/target/riscv/insn_trans/trans_privileged.c.inc
index 59501b2780..e3bee971c6 100644
--- a/target/riscv/insn_trans/trans_privileged.c.inc
+++ b/target/riscv/insn_trans/trans_privileged.c.inc
@@ -77,6 +77,9 @@ static bool trans_sret(DisasContext *ctx, arg_sret *a)
 #ifndef CONFIG_USER_ONLY
     if (has_ext(ctx, RVS)) {
         decode_save_opc(ctx);
+        if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
+            gen_io_start();
+        }
         gen_helper_sret(cpu_pc, cpu_env);
         exit_tb(ctx); /* no chaining */
         ctx->base.is_jmp = DISAS_NORETURN;
@@ -93,6 +96,9 @@ static bool trans_mret(DisasContext *ctx, arg_mret *a)
 {
 #ifndef CONFIG_USER_ONLY
     decode_save_opc(ctx);
+    if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
+        gen_io_start();
+    }
     gen_helper_mret(cpu_pc, cpu_env);
     exit_tb(ctx); /* no chaining */
     ctx->base.is_jmp = DISAS_NORETURN;
-- 
2.17.1
Re: [PATCH] target/riscv: Fix itrigger when icount is used
Posted by Alistair Francis 1 year, 1 month ago
On Sat, Mar 25, 2023 at 2:04 AM LIU Zhiwei <zhiwei_liu@linux.alibaba.com> wrote:
>
> When I boot a ubuntu image, QEMU output a "Bad icount read" message and exit.
> The reason is that when execute helper_mret or helper_sret, it will
> cause a call to icount_get_raw_locked (), which needs set can_do_io flag
> on cpustate.
>
> Thus we setting this flag when execute these two instructions.
>
> Signed-off-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
>  target/riscv/insn_trans/trans_privileged.c.inc | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/target/riscv/insn_trans/trans_privileged.c.inc b/target/riscv/insn_trans/trans_privileged.c.inc
> index 59501b2780..e3bee971c6 100644
> --- a/target/riscv/insn_trans/trans_privileged.c.inc
> +++ b/target/riscv/insn_trans/trans_privileged.c.inc
> @@ -77,6 +77,9 @@ static bool trans_sret(DisasContext *ctx, arg_sret *a)
>  #ifndef CONFIG_USER_ONLY
>      if (has_ext(ctx, RVS)) {
>          decode_save_opc(ctx);
> +        if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
> +            gen_io_start();
> +        }
>          gen_helper_sret(cpu_pc, cpu_env);
>          exit_tb(ctx); /* no chaining */
>          ctx->base.is_jmp = DISAS_NORETURN;
> @@ -93,6 +96,9 @@ static bool trans_mret(DisasContext *ctx, arg_mret *a)
>  {
>  #ifndef CONFIG_USER_ONLY
>      decode_save_opc(ctx);
> +    if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
> +        gen_io_start();
> +    }
>      gen_helper_mret(cpu_pc, cpu_env);
>      exit_tb(ctx); /* no chaining */
>      ctx->base.is_jmp = DISAS_NORETURN;
> --
> 2.17.1
>
>
Re: [PATCH] target/riscv: Fix itrigger when icount is used
Posted by Alistair Francis 1 year, 1 month ago
On Sat, Mar 25, 2023 at 2:04 AM LIU Zhiwei <zhiwei_liu@linux.alibaba.com> wrote:
>
> When I boot a ubuntu image, QEMU output a "Bad icount read" message and exit.
> The reason is that when execute helper_mret or helper_sret, it will
> cause a call to icount_get_raw_locked (), which needs set can_do_io flag
> on cpustate.
>
> Thus we setting this flag when execute these two instructions.
>
> Signed-off-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>

Acked-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/insn_trans/trans_privileged.c.inc | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/target/riscv/insn_trans/trans_privileged.c.inc b/target/riscv/insn_trans/trans_privileged.c.inc
> index 59501b2780..e3bee971c6 100644
> --- a/target/riscv/insn_trans/trans_privileged.c.inc
> +++ b/target/riscv/insn_trans/trans_privileged.c.inc
> @@ -77,6 +77,9 @@ static bool trans_sret(DisasContext *ctx, arg_sret *a)
>  #ifndef CONFIG_USER_ONLY
>      if (has_ext(ctx, RVS)) {
>          decode_save_opc(ctx);
> +        if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
> +            gen_io_start();
> +        }
>          gen_helper_sret(cpu_pc, cpu_env);
>          exit_tb(ctx); /* no chaining */
>          ctx->base.is_jmp = DISAS_NORETURN;
> @@ -93,6 +96,9 @@ static bool trans_mret(DisasContext *ctx, arg_mret *a)
>  {
>  #ifndef CONFIG_USER_ONLY
>      decode_save_opc(ctx);
> +    if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
> +        gen_io_start();
> +    }
>      gen_helper_mret(cpu_pc, cpu_env);
>      exit_tb(ctx); /* no chaining */
>      ctx->base.is_jmp = DISAS_NORETURN;
> --
> 2.17.1
>
>
Re: [PATCH] target/riscv: Fix itrigger when icount is used
Posted by liweiwei 1 year, 1 month ago
On 2023/3/24 14:40, LIU Zhiwei wrote:
> When I boot a ubuntu image, QEMU output a "Bad icount read" message and exit.
> The reason is that when execute helper_mret or helper_sret, it will
> cause a call to icount_get_raw_locked (), which needs set can_do_io flag
> on cpustate.
>
> Thus we setting this flag when execute these two instructions.
>
> Signed-off-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
> ---

LGTM.

Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn>

Weiwei L

>   target/riscv/insn_trans/trans_privileged.c.inc | 6 ++++++
>   1 file changed, 6 insertions(+)
>
> diff --git a/target/riscv/insn_trans/trans_privileged.c.inc b/target/riscv/insn_trans/trans_privileged.c.inc
> index 59501b2780..e3bee971c6 100644
> --- a/target/riscv/insn_trans/trans_privileged.c.inc
> +++ b/target/riscv/insn_trans/trans_privileged.c.inc
> @@ -77,6 +77,9 @@ static bool trans_sret(DisasContext *ctx, arg_sret *a)
>   #ifndef CONFIG_USER_ONLY
>       if (has_ext(ctx, RVS)) {
>           decode_save_opc(ctx);
> +        if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
> +            gen_io_start();
> +        }
>           gen_helper_sret(cpu_pc, cpu_env);
>           exit_tb(ctx); /* no chaining */
>           ctx->base.is_jmp = DISAS_NORETURN;
> @@ -93,6 +96,9 @@ static bool trans_mret(DisasContext *ctx, arg_mret *a)
>   {
>   #ifndef CONFIG_USER_ONLY
>       decode_save_opc(ctx);
> +    if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
> +        gen_io_start();
> +    }
>       gen_helper_mret(cpu_pc, cpu_env);
>       exit_tb(ctx); /* no chaining */
>       ctx->base.is_jmp = DISAS_NORETURN;