From: Roman Bolshakov <r.bolshakov@yadro.com>
To: Richard Henderson <richard.henderson@linaro.org>
Cc: agraf@csgraf.de, qemu-devel@nongnu.org, j@getutm.app
Subject: Re: [PATCH v4] tcg: Toggle page execution for Apple Silicon
Date: Sat, 23 Jan 2021 14:53:48 +0300 [thread overview]
Message-ID: <YAwOTAljKMLvHd7M@SPB-NB-133.local> (raw)
In-Reply-To: <20210121184752.1395873-1-richard.henderson@linaro.org>
On Thu, Jan 21, 2021 at 08:47:52AM -1000, Richard Henderson wrote:
> From: Roman Bolshakov <r.bolshakov@yadro.com>
>
> Pages can't be both write and executable at the same time on Apple
> Silicon. macOS provides public API to switch write protection [1] for
> JIT applications, like TCG.
>
> 1. https://developer.apple.com/documentation/apple_silicon/porting_just-in-time_compilers_to_apple_silicon
>
> Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
> Message-Id: <20210113032806.18220-1-r.bolshakov@yadro.com>
> [rth: Inline the qemu_thread_jit_* functions;
> drop the MAP_JIT change for a follow-on patch.]
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>
> Supercedes: <20210113032806.18220-1-r.bolshakov@yadro.com>
>
> This is the version of Roman's patch that I'm queuing to tcg-next.
> What's missing from the full "Fix execution" patch is setting MAP_JIT
> for !splitwx in alloc_code_gen_buffer().
>
Richard, thanks for updating the patch. I have no objections against
moving the functions and inlining them. However I'm seeing an issue that
wasn't present in v3:
Process 37109 stopped * thread #6, stop reason = EXC_BAD_ACCESS (code=1, address=0xfffffffffffffd4f)
frame #0: 0x00000001002f1c90 qemu-system-x86_64`tcg_emit_op(opc=INDEX_op_add_i64) at tcg.c:2531:5 [opt] 2528 TCGOp *tcg_emit_op(TCGOpcode opc)
2529 { 2530 TCGOp *op = tcg_op_alloc(opc);
-> 2531 QTAILQ_INSERT_TAIL(&tcg_ctx->ops, op, link);
2532 return op;
2533 }
2534
Target 0: (qemu-system-x86_64) stopped.
(lldb) bt
* thread #6, stop reason = EXC_BAD_ACCESS (code=1, address=0xfffffffffffffd4f)
* frame #0: 0x00000001002f1c90 qemu-system-x86_64`tcg_emit_op(opc=INDEX_op_add_i64) at tcg.c:2531:5 [opt]
frame #1: 0x000000010026f040 qemu-system-x86_64`tcg_gen_addi_i64 [inlined] tcg_gen_op3(opc=INDEX_op_add_i64, a1=4430334952, a2=4430333440,
a3=4430361496) at tcg-op.c:60:17 [opt]
frame #2: 0x000000010026f038 qemu-system-x86_64`tcg_gen_addi_i64 [inlined] tcg_gen_op3_i64(opc=INDEX_op_add_i64, a1=<unavailable>, a2=<unav
ailable>, a3=<unavailable>) at tcg-op.h:94 [opt]
frame #3: 0x000000010026f030 qemu-system-x86_64`tcg_gen_addi_i64 [inlined] tcg_gen_add_i64(ret=<unavailable>, arg1=<unavailable>, arg2=<una
vailable>) at tcg-op.h:618 [opt]
frame #4: 0x000000010026f030 qemu-system-x86_64`tcg_gen_addi_i64(ret=<unavailable>, arg1=<unavailable>, arg2=<unavailable>) at tcg-op.c:123
5 [opt]
frame #5: 0x000000010021d1e0 qemu-system-x86_64`gen_lea_modrm_1(s=<unavailable>, a=(def_seg = 2, base = 5, index = -1, scale = 0, disp = -6
89)) at translate.c:2101:9 [opt]
frame #6: 0x000000010020eeec qemu-system-x86_64`disas_insn [inlined] gen_lea_modrm(env=0x0000000118610870, s=0x00000001700b6b00, modrm=<una
vailable>) at translate.c:2111:15 [opt]
frame #7: 0x000000010020eec0 qemu-system-x86_64`disas_insn(s=0x00000001700b6b00, cpu=<unavailable>) at translate.c:5509 [opt]
frame #8: 0x000000010020bb44 qemu-system-x86_64`i386_tr_translate_insn(dcbase=0x00000001700b6b00, cpu=<unavailable>) at translate.c:8573:15
[opt]
frame #9: 0x00000001002fbcf8 qemu-system-x86_64`translator_loop(ops=0x0000000100b209c8, db=0x00000001700b6b00, cpu=0x0000000118608000, tb=0
x0000000120017200, max_insns=512) at translator.c:0 [opt]
frame #10: 0x000000010020b73c qemu-system-x86_64`gen_intermediate_code(cpu=<unavailable>, tb=<unavailable>, max_insns=<unavailable>) at tra
nslate.c:8635:5 [opt]
frame #11: 0x0000000100257970 qemu-system-x86_64`tb_gen_code(cpu=0x0000000118608000, pc=<unavailable>, cs_base=0, flags=4194483, cflags=-16
777216) at translate-all.c:1931:5 [opt]
frame #12: 0x00000001002deb90 qemu-system-x86_64`cpu_exec [inlined] tb_find(cpu=0x0000000118608000, last_tb=0x0000000000000000, tb_exit=<un
available>, cf_mask=0) at cpu-exec.c:456:14 [opt]
frame #13: 0x00000001002deb54 qemu-system-x86_64`cpu_exec(cpu=0x0000000118608000) at cpu-exec.c:812 [opt]
frame #14: 0x00000001002bc0d0 qemu-system-x86_64`tcg_cpus_exec(cpu=0x0000000118608000) at tcg-cpus.c:57:11 [opt]
frame #15: 0x000000010024c2cc qemu-system-x86_64`rr_cpu_thread_fn(arg=<unavailable>) at tcg-cpus-rr.c:217:21 [opt]
frame #16: 0x00000001004b00b4 qemu-system-x86_64`qemu_thread_start(args=<unavailable>) at qemu-thread-posix.c:521:9 [opt]
frame #17: 0x0000000191c4d06c libsystem_pthread.dylib`_pthread_start + 320
I'm looking into the issue but perhaps we'll need v5.
Best regards,
Roman
>
> r~
>
> ---
> include/qemu/osdep.h | 28 ++++++++++++++++++++++++++++
> accel/tcg/cpu-exec.c | 2 ++
> accel/tcg/translate-all.c | 3 +++
> tcg/tcg.c | 1 +
> 4 files changed, 34 insertions(+)
>
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index a434382c58..b6ffdc15bf 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -119,6 +119,10 @@ extern int daemon(int, int);
> #include "sysemu/os-posix.h"
> #endif
>
> +#ifdef __APPLE__
> +#include <AvailabilityMacros.h>
> +#endif
> +
> #include "glib-compat.h"
> #include "qemu/typedefs.h"
>
> @@ -682,4 +686,28 @@ char *qemu_get_host_name(Error **errp);
> */
> size_t qemu_get_host_physmem(void);
>
> +/*
> + * Toggle write/execute on the pages marked MAP_JIT
> + * for the current thread.
> + */
> +#if defined(MAC_OS_VERSION_11_0) && \
> + MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_VERSION_11_0
> +static inline void qemu_thread_jit_execute(void)
> +{
> + if (__builtin_available(macOS 11.0, *)) {
> + pthread_jit_write_protect_np(true);
> + }
> +}
> +
> +static inline void qemu_thread_jit_write(void)
> +{
> + if (__builtin_available(macOS 11.0, *)) {
> + pthread_jit_write_protect_np(false);
> + }
> +}
> +#else
> +static inline void qemu_thread_jit_write(void) {}
> +static inline void qemu_thread_jit_execute(void) {}
> +#endif
> +
> #endif
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index 37d17c8e88..6d017e46dd 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -186,6 +186,7 @@ cpu_tb_exec(CPUState *cpu, TranslationBlock *itb, int *tb_exit)
> }
> #endif /* DEBUG_DISAS */
>
> + qemu_thread_jit_execute();
> ret = tcg_qemu_tb_exec(env, tb_ptr);
> cpu->can_do_io = 1;
> /*
> @@ -410,6 +411,7 @@ static inline void tb_add_jump(TranslationBlock *tb, int n,
> {
> uintptr_t old;
>
> + qemu_thread_jit_write();
> assert(n < ARRAY_SIZE(tb->jmp_list_next));
> qemu_spin_lock(&tb_next->jmp_lock);
>
> diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
> index 73fef47148..d09c187e0f 100644
> --- a/accel/tcg/translate-all.c
> +++ b/accel/tcg/translate-all.c
> @@ -1670,7 +1670,9 @@ static void do_tb_phys_invalidate(TranslationBlock *tb, bool rm_from_page_list)
>
> static void tb_phys_invalidate__locked(TranslationBlock *tb)
> {
> + qemu_thread_jit_write();
> do_tb_phys_invalidate(tb, true);
> + qemu_thread_jit_execute();
> }
>
> /* invalidate one TB
> @@ -1872,6 +1874,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
> #endif
>
> assert_memory_lock();
> + qemu_thread_jit_write();
>
> phys_pc = get_page_addr_code(env, pc);
>
> diff --git a/tcg/tcg.c b/tcg/tcg.c
> index 5110f6f39c..4d734130df 100644
> --- a/tcg/tcg.c
> +++ b/tcg/tcg.c
> @@ -1112,6 +1112,7 @@ void tcg_prologue_init(TCGContext *s)
> s->pool_labels = NULL;
> #endif
>
> + qemu_thread_jit_write();
> /* Generate the prologue. */
> tcg_target_qemu_prologue(s);
>
> --
> 2.25.1
>
next prev parent reply other threads:[~2021-01-23 11:55 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-21 18:47 [PATCH v4] tcg: Toggle page execution for Apple Silicon Richard Henderson
2021-01-21 20:01 ` Alexander Graf
2021-01-21 20:06 ` Alexander Graf
2021-01-23 11:53 ` Roman Bolshakov [this message]
2021-01-23 18:04 ` Roman Bolshakov
2021-01-23 18:33 ` BALATON Zoltan
2021-01-23 18:49 ` Roman Bolshakov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YAwOTAljKMLvHd7M@SPB-NB-133.local \
--to=r.bolshakov@yadro.com \
--cc=agraf@csgraf.de \
--cc=j@getutm.app \
--cc=qemu-devel@nongnu.org \
--cc=richard.henderson@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.