| Message ID | 20211015041053.2769193-16-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | user-only: Cleanup SIGSEGV and SIGBUS handling | expand |
On Thu, Oct 14, 2021 at 10:11 PM Richard Henderson < richard.henderson@linaro.org> wrote: > Split host_signal_pc and host_signal_write out of user-exec.c. > > Reviewed-by: Alistair Francis <alistair.francis@wdc.com> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > linux-user/host/riscv/host-signal.h | 85 +++++++++++++++++- > accel/tcg/user-exec.c | 134 ---------------------------- > 2 files changed, 84 insertions(+), 135 deletions(-) > Reviewed-by: Warner Losh <imp@bsdimp.com> > diff --git a/linux-user/host/riscv/host-signal.h > b/linux-user/host/riscv/host-signal.h > index f4b4d65031..5860dce7d7 100644 > --- a/linux-user/host/riscv/host-signal.h > +++ b/linux-user/host/riscv/host-signal.h > @@ -1 +1,84 @@ > -#define HOST_SIGNAL_PLACEHOLDER > +/* > + * host-signal.h: signal info dependent on the host architecture > + * > + * Copyright (C) 2021 Linaro Limited > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or > later. > + * See the COPYING file in the top-level directory. > + */ > + > +#ifndef RISCV_HOST_SIGNAL_H > +#define RISCV_HOST_SIGNAL_H > + > +static inline uintptr_t host_signal_pc(ucontext_t *uc) > +{ > + return uc->uc_mcontext.__gregs[REG_PC]; > +} > + > +static inline bool host_signal_write(siginfo_t *info, ucontext_t *uc) > +{ > + uint32_t insn = *(uint32_t *)host_signal_pc(uc); > + > + /* > + * Detect store by reading the instruction at the program > + * counter. Note: we currently only generate 32-bit > + * instructions so we thus only detect 32-bit stores > + */ > + switch (((insn >> 0) & 0b11)) { > + case 3: > + switch (((insn >> 2) & 0b11111)) { > + case 8: > + switch (((insn >> 12) & 0b111)) { > + case 0: /* sb */ > + case 1: /* sh */ > + case 2: /* sw */ > + case 3: /* sd */ > + case 4: /* sq */ > + return true; > + default: > + break; > + } > + break; > + case 9: > + switch (((insn >> 12) & 0b111)) { > + case 2: /* fsw */ > + case 3: /* fsd */ > + case 4: /* fsq */ > + return true; > + default: > + break; > + } > + break; > + default: > + break; > + } > + } > + > + /* Check for compressed instructions */ > + switch (((insn >> 13) & 0b111)) { > + case 7: > + switch (insn & 0b11) { > + case 0: /*c.sd */ > + case 2: /* c.sdsp */ > + return true; > + default: > + break; > + } > + break; > + case 6: > + switch (insn & 0b11) { > + case 0: /* c.sw */ > + case 3: /* c.swsp */ > + return true; > + default: > + break; > + } > + break; > + default: > + break; > + } > + > + return false; > +} > + > +#endif > diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c > index 287f03dac5..2d9ab0a8b8 100644 > --- a/accel/tcg/user-exec.c > +++ b/accel/tcg/user-exec.c > @@ -139,64 +139,6 @@ bool handle_sigsegv_accerr_write(CPUState *cpu, > sigset_t *old_set, > } > } > > -/* > - * 'pc' is the host PC at which the exception was raised. > - * 'address' is the effective address of the memory exception. > - * 'is_write' is 1 if a write caused the exception and otherwise 0. > - * 'old_set' is the signal set which should be restored. > - */ > -static inline int handle_cpu_signal(uintptr_t pc, siginfo_t *info, > - int is_write, sigset_t *old_set) > -{ > - CPUState *cpu = current_cpu; > - CPUClass *cc; > - unsigned long host_addr = (unsigned long)info->si_addr; > - MMUAccessType access_type = adjust_signal_pc(&pc, is_write); > - abi_ptr guest_addr; > - > - /* For synchronous signals we expect to be coming from the vCPU > - * thread (so current_cpu should be valid) and either from running > - * code or during translation which can fault as we cross pages. > - * > - * If neither is true then something has gone wrong and we should > - * abort rather than try and restart the vCPU execution. > - */ > - if (!cpu || !cpu->running) { > - printf("qemu:%s received signal outside vCPU context @ pc=0x%" > - PRIxPTR "\n", __func__, pc); > - abort(); > - } > - > -#if defined(DEBUG_SIGNAL) > - printf("qemu: SIGSEGV pc=0x%08lx address=%08lx w=%d oldset=0x%08lx\n", > - pc, host_addr, is_write, *(unsigned long *)old_set); > -#endif > - > - /* Convert forcefully to guest address space, invalid addresses > - are still valid segv ones */ > - guest_addr = h2g_nocheck(host_addr); > - > - /* XXX: locking issue */ > - if (is_write && > - info->si_signo == SIGSEGV && > - info->si_code == SEGV_ACCERR && > - h2g_valid(host_addr) && > - handle_sigsegv_accerr_write(cpu, old_set, pc, guest_addr)) { > - return 1; > - } > - > - /* > - * There is no way the target can handle this other than raising > - * an exception. Undo signal and retaddr state prior to longjmp. > - */ > - sigprocmask(SIG_SETMASK, old_set, NULL); > - > - cc = CPU_GET_CLASS(cpu); > - cc->tcg_ops->tlb_fill(cpu, guest_addr, 0, access_type, > - MMU_USER_IDX, false, pc); > - g_assert_not_reached(); > -} > - > static int probe_access_internal(CPUArchState *env, target_ulong addr, > int fault_size, MMUAccessType > access_type, > bool nonfault, uintptr_t ra) > @@ -255,82 +197,6 @@ void *probe_access(CPUArchState *env, target_ulong > addr, int size, > return size ? g2h(env_cpu(env), addr) : NULL; > } > > -#if defined(__riscv) > - > -int cpu_signal_handler(int host_signum, void *pinfo, > - void *puc) > -{ > - siginfo_t *info = pinfo; > - ucontext_t *uc = puc; > - greg_t pc = uc->uc_mcontext.__gregs[REG_PC]; > - uint32_t insn = *(uint32_t *)pc; > - int is_write = 0; > - > - /* Detect store by reading the instruction at the program > - counter. Note: we currently only generate 32-bit > - instructions so we thus only detect 32-bit stores */ > - switch (((insn >> 0) & 0b11)) { > - case 3: > - switch (((insn >> 2) & 0b11111)) { > - case 8: > - switch (((insn >> 12) & 0b111)) { > - case 0: /* sb */ > - case 1: /* sh */ > - case 2: /* sw */ > - case 3: /* sd */ > - case 4: /* sq */ > - is_write = 1; > - break; > - default: > - break; > - } > - break; > - case 9: > - switch (((insn >> 12) & 0b111)) { > - case 2: /* fsw */ > - case 3: /* fsd */ > - case 4: /* fsq */ > - is_write = 1; > - break; > - default: > - break; > - } > - break; > - default: > - break; > - } > - } > - > - /* Check for compressed instructions */ > - switch (((insn >> 13) & 0b111)) { > - case 7: > - switch (insn & 0b11) { > - case 0: /*c.sd */ > - case 2: /* c.sdsp */ > - is_write = 1; > - break; > - default: > - break; > - } > - break; > - case 6: > - switch (insn & 0b11) { > - case 0: /* c.sw */ > - case 3: /* c.swsp */ > - is_write = 1; > - break; > - default: > - break; > - } > - break; > - default: > - break; > - } > - > - return handle_cpu_signal(pc, info, is_write, &uc->uc_sigmask); > -} > -#endif > - > /* The softmmu versions of these helpers are in cputlb.c. */ > > /* > -- > 2.25.1 > > <div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Oct 14, 2021 at 10:11 PM Richard Henderson <<a href="mailto:richard.henderson@linaro.org">richard.henderson@linaro.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Split host_signal_pc and host_signal_write out of user-exec.c.<br> <br> Reviewed-by: Alistair Francis <<a href="mailto:alistair.francis@wdc.com" target="_blank">alistair.francis@wdc.com</a>><br> Signed-off-by: Richard Henderson <<a href="mailto:richard.henderson@linaro.org" target="_blank">richard.henderson@linaro.org</a>><br> ---<br> linux-user/host/riscv/host-signal.h | 85 +++++++++++++++++-<br> accel/tcg/user-exec.c | 134 ----------------------------<br> 2 files changed, 84 insertions(+), 135 deletions(-)<br></blockquote><div><br></div><div><div>Reviewed-by: Warner Losh <<a href="mailto:imp@bsdimp.com">imp@bsdimp.com</a>></div></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> diff --git a/linux-user/host/riscv/host-signal.h b/linux-user/host/riscv/host-signal.h<br> index f4b4d65031..5860dce7d7 100644<br> --- a/linux-user/host/riscv/host-signal.h<br> +++ b/linux-user/host/riscv/host-signal.h<br> @@ -1 +1,84 @@<br> -#define HOST_SIGNAL_PLACEHOLDER<br> +/*<br> + * host-signal.h: signal info dependent on the host architecture<br> + *<br> + * Copyright (C) 2021 Linaro Limited<br> + *<br> + * This work is licensed under the terms of the GNU GPL, version 2 or later.<br> + * See the COPYING file in the top-level directory.<br> + */<br> +<br> +#ifndef RISCV_HOST_SIGNAL_H<br> +#define RISCV_HOST_SIGNAL_H<br> +<br> +static inline uintptr_t host_signal_pc(ucontext_t *uc)<br> +{<br> + return uc->uc_mcontext.__gregs[REG_PC];<br> +}<br> +<br> +static inline bool host_signal_write(siginfo_t *info, ucontext_t *uc)<br> +{<br> + uint32_t insn = *(uint32_t *)host_signal_pc(uc);<br> +<br> + /*<br> + * Detect store by reading the instruction at the program<br> + * counter. Note: we currently only generate 32-bit<br> + * instructions so we thus only detect 32-bit stores<br> + */<br> + switch (((insn >> 0) & 0b11)) {<br> + case 3:<br> + switch (((insn >> 2) & 0b11111)) {<br> + case 8:<br> + switch (((insn >> 12) & 0b111)) {<br> + case 0: /* sb */<br> + case 1: /* sh */<br> + case 2: /* sw */<br> + case 3: /* sd */<br> + case 4: /* sq */<br> + return true;<br> + default:<br> + break;<br> + }<br> + break;<br> + case 9:<br> + switch (((insn >> 12) & 0b111)) {<br> + case 2: /* fsw */<br> + case 3: /* fsd */<br> + case 4: /* fsq */<br> + return true;<br> + default:<br> + break;<br> + }<br> + break;<br> + default:<br> + break;<br> + }<br> + }<br> +<br> + /* Check for compressed instructions */<br> + switch (((insn >> 13) & 0b111)) {<br> + case 7:<br> + switch (insn & 0b11) {<br> + case 0: /*<a href="http://c.sd" rel="noreferrer" target="_blank">c.sd</a> */<br> + case 2: /* c.sdsp */<br> + return true;<br> + default:<br> + break;<br> + }<br> + break;<br> + case 6:<br> + switch (insn & 0b11) {<br> + case 0: /* c.sw */<br> + case 3: /* c.swsp */<br> + return true;<br> + default:<br> + break;<br> + }<br> + break;<br> + default:<br> + break;<br> + }<br> +<br> + return false;<br> +}<br> +<br> +#endif<br> diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c<br> index 287f03dac5..2d9ab0a8b8 100644<br> --- a/accel/tcg/user-exec.c<br> +++ b/accel/tcg/user-exec.c<br> @@ -139,64 +139,6 @@ bool handle_sigsegv_accerr_write(CPUState *cpu, sigset_t *old_set,<br> }<br> }<br> <br> -/*<br> - * 'pc' is the host PC at which the exception was raised.<br> - * 'address' is the effective address of the memory exception.<br> - * 'is_write' is 1 if a write caused the exception and otherwise 0.<br> - * 'old_set' is the signal set which should be restored.<br> - */<br> -static inline int handle_cpu_signal(uintptr_t pc, siginfo_t *info,<br> - int is_write, sigset_t *old_set)<br> -{<br> - CPUState *cpu = current_cpu;<br> - CPUClass *cc;<br> - unsigned long host_addr = (unsigned long)info->si_addr;<br> - MMUAccessType access_type = adjust_signal_pc(&pc, is_write);<br> - abi_ptr guest_addr;<br> -<br> - /* For synchronous signals we expect to be coming from the vCPU<br> - * thread (so current_cpu should be valid) and either from running<br> - * code or during translation which can fault as we cross pages.<br> - *<br> - * If neither is true then something has gone wrong and we should<br> - * abort rather than try and restart the vCPU execution.<br> - */<br> - if (!cpu || !cpu->running) {<br> - printf("qemu:%s received signal outside vCPU context @ pc=0x%"<br> - PRIxPTR "\n", __func__, pc);<br> - abort();<br> - }<br> -<br> -#if defined(DEBUG_SIGNAL)<br> - printf("qemu: SIGSEGV pc=0x%08lx address=%08lx w=%d oldset=0x%08lx\n",<br> - pc, host_addr, is_write, *(unsigned long *)old_set);<br> -#endif<br> -<br> - /* Convert forcefully to guest address space, invalid addresses<br> - are still valid segv ones */<br> - guest_addr = h2g_nocheck(host_addr);<br> -<br> - /* XXX: locking issue */<br> - if (is_write &&<br> - info->si_signo == SIGSEGV &&<br> - info->si_code == SEGV_ACCERR &&<br> - h2g_valid(host_addr) &&<br> - handle_sigsegv_accerr_write(cpu, old_set, pc, guest_addr)) {<br> - return 1;<br> - }<br> -<br> - /*<br> - * There is no way the target can handle this other than raising<br> - * an exception. Undo signal and retaddr state prior to longjmp.<br> - */<br> - sigprocmask(SIG_SETMASK, old_set, NULL);<br> -<br> - cc = CPU_GET_CLASS(cpu);<br> - cc->tcg_ops->tlb_fill(cpu, guest_addr, 0, access_type,<br> - MMU_USER_IDX, false, pc);<br> - g_assert_not_reached();<br> -}<br> -<br> static int probe_access_internal(CPUArchState *env, target_ulong addr,<br> int fault_size, MMUAccessType access_type,<br> bool nonfault, uintptr_t ra)<br> @@ -255,82 +197,6 @@ void *probe_access(CPUArchState *env, target_ulong addr, int size,<br> return size ? g2h(env_cpu(env), addr) : NULL;<br> }<br> <br> -#if defined(__riscv)<br> -<br> -int cpu_signal_handler(int host_signum, void *pinfo,<br> - void *puc)<br> -{<br> - siginfo_t *info = pinfo;<br> - ucontext_t *uc = puc;<br> - greg_t pc = uc->uc_mcontext.__gregs[REG_PC];<br> - uint32_t insn = *(uint32_t *)pc;<br> - int is_write = 0;<br> -<br> - /* Detect store by reading the instruction at the program<br> - counter. Note: we currently only generate 32-bit<br> - instructions so we thus only detect 32-bit stores */<br> - switch (((insn >> 0) & 0b11)) {<br> - case 3:<br> - switch (((insn >> 2) & 0b11111)) {<br> - case 8:<br> - switch (((insn >> 12) & 0b111)) {<br> - case 0: /* sb */<br> - case 1: /* sh */<br> - case 2: /* sw */<br> - case 3: /* sd */<br> - case 4: /* sq */<br> - is_write = 1;<br> - break;<br> - default:<br> - break;<br> - }<br> - break;<br> - case 9:<br> - switch (((insn >> 12) & 0b111)) {<br> - case 2: /* fsw */<br> - case 3: /* fsd */<br> - case 4: /* fsq */<br> - is_write = 1;<br> - break;<br> - default:<br> - break;<br> - }<br> - break;<br> - default:<br> - break;<br> - }<br> - }<br> -<br> - /* Check for compressed instructions */<br> - switch (((insn >> 13) & 0b111)) {<br> - case 7:<br> - switch (insn & 0b11) {<br> - case 0: /*<a href="http://c.sd" rel="noreferrer" target="_blank">c.sd</a> */<br> - case 2: /* c.sdsp */<br> - is_write = 1;<br> - break;<br> - default:<br> - break;<br> - }<br> - break;<br> - case 6:<br> - switch (insn & 0b11) {<br> - case 0: /* c.sw */<br> - case 3: /* c.swsp */<br> - is_write = 1;<br> - break;<br> - default:<br> - break;<br> - }<br> - break;<br> - default:<br> - break;<br> - }<br> -<br> - return handle_cpu_signal(pc, info, is_write, &uc->uc_sigmask);<br> -}<br> -#endif<br> -<br> /* The softmmu versions of these helpers are in cputlb.c. */<br> <br> /*<br> -- <br> 2.25.1<br> <br> </blockquote></div></div>
diff --git a/linux-user/host/riscv/host-signal.h b/linux-user/host/riscv/host-signal.h index f4b4d65031..5860dce7d7 100644 --- a/linux-user/host/riscv/host-signal.h +++ b/linux-user/host/riscv/host-signal.h @@ -1 +1,84 @@ -#define HOST_SIGNAL_PLACEHOLDER +/* + * host-signal.h: signal info dependent on the host architecture + * + * Copyright (C) 2021 Linaro Limited + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ + +#ifndef RISCV_HOST_SIGNAL_H +#define RISCV_HOST_SIGNAL_H + +static inline uintptr_t host_signal_pc(ucontext_t *uc) +{ + return uc->uc_mcontext.__gregs[REG_PC]; +} + +static inline bool host_signal_write(siginfo_t *info, ucontext_t *uc) +{ + uint32_t insn = *(uint32_t *)host_signal_pc(uc); + + /* + * Detect store by reading the instruction at the program + * counter. Note: we currently only generate 32-bit + * instructions so we thus only detect 32-bit stores + */ + switch (((insn >> 0) & 0b11)) { + case 3: + switch (((insn >> 2) & 0b11111)) { + case 8: + switch (((insn >> 12) & 0b111)) { + case 0: /* sb */ + case 1: /* sh */ + case 2: /* sw */ + case 3: /* sd */ + case 4: /* sq */ + return true; + default: + break; + } + break; + case 9: + switch (((insn >> 12) & 0b111)) { + case 2: /* fsw */ + case 3: /* fsd */ + case 4: /* fsq */ + return true; + default: + break; + } + break; + default: + break; + } + } + + /* Check for compressed instructions */ + switch (((insn >> 13) & 0b111)) { + case 7: + switch (insn & 0b11) { + case 0: /*c.sd */ + case 2: /* c.sdsp */ + return true; + default: + break; + } + break; + case 6: + switch (insn & 0b11) { + case 0: /* c.sw */ + case 3: /* c.swsp */ + return true; + default: + break; + } + break; + default: + break; + } + + return false; +} + +#endif diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c index 287f03dac5..2d9ab0a8b8 100644 --- a/accel/tcg/user-exec.c +++ b/accel/tcg/user-exec.c @@ -139,64 +139,6 @@ bool handle_sigsegv_accerr_write(CPUState *cpu, sigset_t *old_set, } } -/* - * 'pc' is the host PC at which the exception was raised. - * 'address' is the effective address of the memory exception. - * 'is_write' is 1 if a write caused the exception and otherwise 0. - * 'old_set' is the signal set which should be restored. - */ -static inline int handle_cpu_signal(uintptr_t pc, siginfo_t *info, - int is_write, sigset_t *old_set) -{ - CPUState *cpu = current_cpu; - CPUClass *cc; - unsigned long host_addr = (unsigned long)info->si_addr; - MMUAccessType access_type = adjust_signal_pc(&pc, is_write); - abi_ptr guest_addr; - - /* For synchronous signals we expect to be coming from the vCPU - * thread (so current_cpu should be valid) and either from running - * code or during translation which can fault as we cross pages. - * - * If neither is true then something has gone wrong and we should - * abort rather than try and restart the vCPU execution. - */ - if (!cpu || !cpu->running) { - printf("qemu:%s received signal outside vCPU context @ pc=0x%" - PRIxPTR "\n", __func__, pc); - abort(); - } - -#if defined(DEBUG_SIGNAL) - printf("qemu: SIGSEGV pc=0x%08lx address=%08lx w=%d oldset=0x%08lx\n", - pc, host_addr, is_write, *(unsigned long *)old_set); -#endif - - /* Convert forcefully to guest address space, invalid addresses - are still valid segv ones */ - guest_addr = h2g_nocheck(host_addr); - - /* XXX: locking issue */ - if (is_write && - info->si_signo == SIGSEGV && - info->si_code == SEGV_ACCERR && - h2g_valid(host_addr) && - handle_sigsegv_accerr_write(cpu, old_set, pc, guest_addr)) { - return 1; - } - - /* - * There is no way the target can handle this other than raising - * an exception. Undo signal and retaddr state prior to longjmp. - */ - sigprocmask(SIG_SETMASK, old_set, NULL); - - cc = CPU_GET_CLASS(cpu); - cc->tcg_ops->tlb_fill(cpu, guest_addr, 0, access_type, - MMU_USER_IDX, false, pc); - g_assert_not_reached(); -} - static int probe_access_internal(CPUArchState *env, target_ulong addr, int fault_size, MMUAccessType access_type, bool nonfault, uintptr_t ra) @@ -255,82 +197,6 @@ void *probe_access(CPUArchState *env, target_ulong addr, int size, return size ? g2h(env_cpu(env), addr) : NULL; } -#if defined(__riscv) - -int cpu_signal_handler(int host_signum, void *pinfo, - void *puc) -{ - siginfo_t *info = pinfo; - ucontext_t *uc = puc; - greg_t pc = uc->uc_mcontext.__gregs[REG_PC]; - uint32_t insn = *(uint32_t *)pc; - int is_write = 0; - - /* Detect store by reading the instruction at the program - counter. Note: we currently only generate 32-bit - instructions so we thus only detect 32-bit stores */ - switch (((insn >> 0) & 0b11)) { - case 3: - switch (((insn >> 2) & 0b11111)) { - case 8: - switch (((insn >> 12) & 0b111)) { - case 0: /* sb */ - case 1: /* sh */ - case 2: /* sw */ - case 3: /* sd */ - case 4: /* sq */ - is_write = 1; - break; - default: - break; - } - break; - case 9: - switch (((insn >> 12) & 0b111)) { - case 2: /* fsw */ - case 3: /* fsd */ - case 4: /* fsq */ - is_write = 1; - break; - default: - break; - } - break; - default: - break; - } - } - - /* Check for compressed instructions */ - switch (((insn >> 13) & 0b111)) { - case 7: - switch (insn & 0b11) { - case 0: /*c.sd */ - case 2: /* c.sdsp */ - is_write = 1; - break; - default: - break; - } - break; - case 6: - switch (insn & 0b11) { - case 0: /* c.sw */ - case 3: /* c.swsp */ - is_write = 1; - break; - default: - break; - } - break; - default: - break; - } - - return handle_cpu_signal(pc, info, is_write, &uc->uc_sigmask); -} -#endif - /* The softmmu versions of these helpers are in cputlb.c. */ /*