[v1] target/riscv: Fix checking of whether instruciton at 'pc_next' spans pages

[PATCH] target/riscv: Fix checking of whether instruciton at 'pc_next' spans pages

Shaobo Song posted 1 patch 1 year, 2 months ago

Diff against v1
Download mbox

Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20230220072732.568-1-songshaobo@eswincomputing.com

Maintainers: Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Bin Meng <bin.meng@windriver.com>

There is a newer version of this series

target/riscv/translate.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

Expand all Fold all

[PATCH] target/riscv: Fix checking of whether instruciton at 'pc_next' spans pages

Posted by Shaobo Song 1 year, 2 months ago

This bug has a noticeable behavior of falling back to the main loop and
respawning a redundant translation block including a single instruction
when the end address of the compressive instruction is exactly on a page
boundary, and slows down running system performance.

Signed-off-by: Shaobo Song <songshaobo@eswincomputing.com>
---
 target/riscv/translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 772f9d7..8ffa211 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -1261,7 +1261,7 @@ static void riscv_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
                 uint16_t next_insn = cpu_lduw_code(env, ctx->base.pc_next);
                 int len = insn_len(next_insn);
 
-                if (!is_same_page(&ctx->base, ctx->base.pc_next + len)) {
+                if (!is_same_page(&ctx->base, ctx->base.pc_next + len - 1)) {
                     ctx->base.is_jmp = DISAS_TOO_MANY;
                 }
             }
-- 
2.33.1.windows.1

Re: [PATCH] target/riscv: Fix checking of whether instruciton at 'pc_next' spans pages

Posted by Palmer Dabbelt 1 year, 2 months ago

On Sun, 19 Feb 2023 23:27:32 PST (-0800), songshaobo@eswincomputing.com wrote:
> This bug has a noticeable behavior of falling back to the main loop and
> respawning a redundant translation block including a single instruction
> when the end address of the compressive instruction is exactly on a page
> boundary, and slows down running system performance.
>
> Signed-off-by: Shaobo Song <songshaobo@eswincomputing.com>
> ---
>  target/riscv/translate.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index 772f9d7..8ffa211 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -1261,7 +1261,7 @@ static void riscv_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
>                  uint16_t next_insn = cpu_lduw_code(env, ctx->base.pc_next);
>                  int len = insn_len(next_insn);
>
> -                if (!is_same_page(&ctx->base, ctx->base.pc_next + len)) {
> +                if (!is_same_page(&ctx->base, ctx->base.pc_next + len - 1)) {
>                      ctx->base.is_jmp = DISAS_TOO_MANY;
>                  }
>              }

Thanks, this is queued in riscv-to-apply.next .

Re: [PATCH] target/riscv: Fix checking of whether instruciton at 'pc_next' spans pages

Posted by Richard Henderson 1 year, 2 months ago

On 2/19/23 21:27, Shaobo Song wrote:
> This bug has a noticeable behavior of falling back to the main loop and
> respawning a redundant translation block including a single instruction
> when the end address of the compressive instruction is exactly on a page
> boundary, and slows down running system performance.
> 
> Signed-off-by: Shaobo Song<songshaobo@eswincomputing.com>
> ---
>   target/riscv/translate.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~