diff mbox series

[v6,21/21] target/riscv: Make translator stop before the end of a page

Message ID 20220819032615.884847-22-richard.henderson@linaro.org
State Superseded
Headers show
Series linux-user: Fix siginfo_t contents when jumping to non-readable pages | expand

Commit Message

Richard Henderson Aug. 19, 2022, 3:26 a.m. UTC 
Right now the translator stops right *after* the end of a page, which
breaks reporting of fault locations when the last instruction of a
multi-insn translation block crosses a page boundary.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1155
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/riscv/translate.c          | 17 +++++--
 tests/tcg/riscv64/noexec.c        | 79 +++++++++++++++++++++++++++++++
 tests/tcg/riscv64/Makefile.target |  1 +
 3 files changed, 93 insertions(+), 4 deletions(-)
 create mode 100644 tests/tcg/riscv64/noexec.c

Comments

Alistair Francis Aug. 21, 2022, 11:47 p.m. UTC | #1 
On Fri, Aug 19, 2022 at 1:39 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Right now the translator stops right *after* the end of a page, which
> breaks reporting of fault locations when the last instruction of a
> multi-insn translation block crosses a page boundary.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1155
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

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

Alistair

> ---
>  target/riscv/translate.c          | 17 +++++--
>  tests/tcg/riscv64/noexec.c        | 79 +++++++++++++++++++++++++++++++
>  tests/tcg/riscv64/Makefile.target |  1 +
>  3 files changed, 93 insertions(+), 4 deletions(-)
>  create mode 100644 tests/tcg/riscv64/noexec.c
>
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index a719aa6e63..f8af6daa70 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -1154,12 +1154,21 @@ static void riscv_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
>      }
>      ctx->nftemp = 0;
>
> +    /* Only the first insn within a TB is allowed to cross a page boundary. */
>      if (ctx->base.is_jmp == DISAS_NEXT) {
> -        target_ulong page_start;
> -
> -        page_start = ctx->base.pc_first & TARGET_PAGE_MASK;
> -        if (ctx->base.pc_next - page_start >= TARGET_PAGE_SIZE) {
> +        if (!is_same_page(&ctx->base, ctx->base.pc_next)) {
>              ctx->base.is_jmp = DISAS_TOO_MANY;
> +        } else {
> +            unsigned page_ofs = ctx->base.pc_next & ~TARGET_PAGE_MASK;
> +
> +            if (page_ofs > TARGET_PAGE_SIZE - MAX_INSN_LEN) {
> +                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)) {
> +                    ctx->base.is_jmp = DISAS_TOO_MANY;
> +                }
> +            }
>          }
>      }
>  }
> diff --git a/tests/tcg/riscv64/noexec.c b/tests/tcg/riscv64/noexec.c
> new file mode 100644
> index 0000000000..86f64b28db
> --- /dev/null
> +++ b/tests/tcg/riscv64/noexec.c
> @@ -0,0 +1,79 @@
> +#include "../multiarch/noexec.c.inc"
> +
> +static void *arch_mcontext_pc(const mcontext_t *ctx)
> +{
> +    return (void *)ctx->__gregs[REG_PC];
> +}
> +
> +static int arch_mcontext_arg(const mcontext_t *ctx)
> +{
> +    return ctx->__gregs[REG_A0];
> +}
> +
> +static void arch_flush(void *p, int len)
> +{
> +    __builtin___clear_cache(p, p + len);
> +}
> +
> +extern char noexec_1[];
> +extern char noexec_2[];
> +extern char noexec_end[];
> +
> +asm(".option push\n"
> +    ".option norvc\n"
> +    "noexec_1:\n"
> +    "   li a0,1\n"       /* a0 is 0 on entry, set 1. */
> +    "noexec_2:\n"
> +    "   li a0,2\n"      /* a0 is 0/1; set 2. */
> +    "   ret\n"
> +    "noexec_end:\n"
> +    ".option pop");
> +
> +int main(void)
> +{
> +    struct noexec_test noexec_tests[] = {
> +        {
> +            .name = "fallthrough",
> +            .test_code = noexec_1,
> +            .test_len = noexec_end - noexec_1,
> +            .page_ofs = noexec_1 - noexec_2,
> +            .entry_ofs = noexec_1 - noexec_2,
> +            .expected_si_ofs = 0,
> +            .expected_pc_ofs = 0,
> +            .expected_arg = 1,
> +        },
> +        {
> +            .name = "jump",
> +            .test_code = noexec_1,
> +            .test_len = noexec_end - noexec_1,
> +            .page_ofs = noexec_1 - noexec_2,
> +            .entry_ofs = 0,
> +            .expected_si_ofs = 0,
> +            .expected_pc_ofs = 0,
> +            .expected_arg = 0,
> +        },
> +        {
> +            .name = "fallthrough [cross]",
> +            .test_code = noexec_1,
> +            .test_len = noexec_end - noexec_1,
> +            .page_ofs = noexec_1 - noexec_2 - 2,
> +            .entry_ofs = noexec_1 - noexec_2 - 2,
> +            .expected_si_ofs = 0,
> +            .expected_pc_ofs = -2,
> +            .expected_arg = 1,
> +        },
> +        {
> +            .name = "jump [cross]",
> +            .test_code = noexec_1,
> +            .test_len = noexec_end - noexec_1,
> +            .page_ofs = noexec_1 - noexec_2 - 2,
> +            .entry_ofs = -2,
> +            .expected_si_ofs = 0,
> +            .expected_pc_ofs = -2,
> +            .expected_arg = 0,
> +        },
> +    };
> +
> +    return test_noexec(noexec_tests,
> +                       sizeof(noexec_tests) / sizeof(noexec_tests[0]));
> +}
> diff --git a/tests/tcg/riscv64/Makefile.target b/tests/tcg/riscv64/Makefile.target
> index d41bf6d60d..b5b89dfb0e 100644
> --- a/tests/tcg/riscv64/Makefile.target
> +++ b/tests/tcg/riscv64/Makefile.target
> @@ -3,3 +3,4 @@
>
>  VPATH += $(SRC_PATH)/tests/tcg/riscv64
>  TESTS += test-div
> +TESTS += noexec
> --
> 2.34.1
>
>
diff mbox series

Patch

diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index a719aa6e63..f8af6daa70 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -1154,12 +1154,21 @@  static void riscv_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
     }
     ctx->nftemp = 0;
 
+    /* Only the first insn within a TB is allowed to cross a page boundary. */
     if (ctx->base.is_jmp == DISAS_NEXT) {
-        target_ulong page_start;
-
-        page_start = ctx->base.pc_first & TARGET_PAGE_MASK;
-        if (ctx->base.pc_next - page_start >= TARGET_PAGE_SIZE) {
+        if (!is_same_page(&ctx->base, ctx->base.pc_next)) {
             ctx->base.is_jmp = DISAS_TOO_MANY;
+        } else {
+            unsigned page_ofs = ctx->base.pc_next & ~TARGET_PAGE_MASK;
+
+            if (page_ofs > TARGET_PAGE_SIZE - MAX_INSN_LEN) {
+                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)) {
+                    ctx->base.is_jmp = DISAS_TOO_MANY;
+                }
+            }
         }
     }
 }
diff --git a/tests/tcg/riscv64/noexec.c b/tests/tcg/riscv64/noexec.c
new file mode 100644
index 0000000000..86f64b28db
--- /dev/null
+++ b/tests/tcg/riscv64/noexec.c
@@ -0,0 +1,79 @@ 
+#include "../multiarch/noexec.c.inc"
+
+static void *arch_mcontext_pc(const mcontext_t *ctx)
+{
+    return (void *)ctx->__gregs[REG_PC];
+}
+
+static int arch_mcontext_arg(const mcontext_t *ctx)
+{
+    return ctx->__gregs[REG_A0];
+}
+
+static void arch_flush(void *p, int len)
+{
+    __builtin___clear_cache(p, p + len);
+}
+
+extern char noexec_1[];
+extern char noexec_2[];
+extern char noexec_end[];
+
+asm(".option push\n"
+    ".option norvc\n"
+    "noexec_1:\n"
+    "   li a0,1\n"       /* a0 is 0 on entry, set 1. */
+    "noexec_2:\n"
+    "   li a0,2\n"      /* a0 is 0/1; set 2. */
+    "   ret\n"
+    "noexec_end:\n"
+    ".option pop");
+
+int main(void)
+{
+    struct noexec_test noexec_tests[] = {
+        {
+            .name = "fallthrough",
+            .test_code = noexec_1,
+            .test_len = noexec_end - noexec_1,
+            .page_ofs = noexec_1 - noexec_2,
+            .entry_ofs = noexec_1 - noexec_2,
+            .expected_si_ofs = 0,
+            .expected_pc_ofs = 0,
+            .expected_arg = 1,
+        },
+        {
+            .name = "jump",
+            .test_code = noexec_1,
+            .test_len = noexec_end - noexec_1,
+            .page_ofs = noexec_1 - noexec_2,
+            .entry_ofs = 0,
+            .expected_si_ofs = 0,
+            .expected_pc_ofs = 0,
+            .expected_arg = 0,
+        },
+        {
+            .name = "fallthrough [cross]",
+            .test_code = noexec_1,
+            .test_len = noexec_end - noexec_1,
+            .page_ofs = noexec_1 - noexec_2 - 2,
+            .entry_ofs = noexec_1 - noexec_2 - 2,
+            .expected_si_ofs = 0,
+            .expected_pc_ofs = -2,
+            .expected_arg = 1,
+        },
+        {
+            .name = "jump [cross]",
+            .test_code = noexec_1,
+            .test_len = noexec_end - noexec_1,
+            .page_ofs = noexec_1 - noexec_2 - 2,
+            .entry_ofs = -2,
+            .expected_si_ofs = 0,
+            .expected_pc_ofs = -2,
+            .expected_arg = 0,
+        },
+    };
+
+    return test_noexec(noexec_tests,
+                       sizeof(noexec_tests) / sizeof(noexec_tests[0]));
+}
diff --git a/tests/tcg/riscv64/Makefile.target b/tests/tcg/riscv64/Makefile.target
index d41bf6d60d..b5b89dfb0e 100644
--- a/tests/tcg/riscv64/Makefile.target
+++ b/tests/tcg/riscv64/Makefile.target
@@ -3,3 +3,4 @@ 
 
 VPATH += $(SRC_PATH)/tests/tcg/riscv64
 TESTS += test-div
+TESTS += noexec