| Message ID | 20220109161923.85683-6-imp@bsdimp.com |
|---|---|
| State | New |
| Headers | show |
| Series | bsd-user: upstream our signal implementation | expand |
On Sun, 9 Jan 2022 at 16:26, Warner Losh <imp@bsdimp.com> wrote: > > Implement EXCP_DEBUG and EXCP_BKPT the same, as is done in > linux-user. The prior adjustment of register 15 isn't needed, so remove > that. Remove a redunant comment (that code in FreeBSD never handled > break points). > > Signed-off-by: Warner Losh <imp@bsdimp.com> > --- > bsd-user/arm/target_arch_cpu.h | 23 +++-------------------- > 1 file changed, 3 insertions(+), 20 deletions(-) > > diff --git a/bsd-user/arm/target_arch_cpu.h b/bsd-user/arm/target_arch_cpu.h > index c526fc73502..05b19ce6119 100644 > --- a/bsd-user/arm/target_arch_cpu.h > +++ b/bsd-user/arm/target_arch_cpu.h > @@ -21,6 +21,7 @@ > #define _TARGET_ARCH_CPU_H_ > > #include "target_arch.h" > +#include "signal-common.h" > > #define TARGET_DEFAULT_CPU_MODEL "any" > > @@ -64,19 +65,7 @@ static inline void target_cpu_loop(CPUARMState *env) > } > break; > case EXCP_SWI: > - case EXCP_BKPT: > { > - /* > - * system call > - * See arm/arm/trap.c cpu_fetch_syscall_args() > - */ > - if (trapnr == EXCP_BKPT) { > - if (env->thumb) { > - env->regs[15] += 2; > - } else { > - env->regs[15] += 4; > - } > - } So the previous code was implementing BKPT as a way to do a syscall (added in commit 8d450c9a30). Was that just a mistake ? > n = env->regs[7]; > if (bsd_type == target_freebsd) { > int ret; > @@ -171,14 +160,8 @@ static inline void target_cpu_loop(CPUARMState *env) > queue_signal(env, info.si_signo, &info); > break; > case EXCP_DEBUG: > - { > - > - info.si_signo = TARGET_SIGTRAP; > - info.si_errno = 0; > - info.si_code = TARGET_TRAP_BRKPT; > - info.si_addr = env->exception.vaddress; > - queue_signal(env, info.si_signo, &info); > - } > + case EXCP_BKPT: > + force_sig_fault(TARGET_SIGTRAP, TARGET_TRAP_BRKPT, env->regs[15]); > break; > case EXCP_YIELD: > /* nothing to do here for user-mode, just resume guest code */ Looks like it now matches the freebsd kernel behaviour, anyway. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> thanks -- PMM
On Thu, Jan 13, 2022 at 10:13 AM Peter Maydell <peter.maydell@linaro.org> wrote: > On Sun, 9 Jan 2022 at 16:26, Warner Losh <imp@bsdimp.com> wrote: > > > > Implement EXCP_DEBUG and EXCP_BKPT the same, as is done in > > linux-user. The prior adjustment of register 15 isn't needed, so remove > > that. Remove a redunant comment (that code in FreeBSD never handled > > break points). > > > > Signed-off-by: Warner Losh <imp@bsdimp.com> > > --- > > bsd-user/arm/target_arch_cpu.h | 23 +++-------------------- > > 1 file changed, 3 insertions(+), 20 deletions(-) > > > > diff --git a/bsd-user/arm/target_arch_cpu.h > b/bsd-user/arm/target_arch_cpu.h > > index c526fc73502..05b19ce6119 100644 > > --- a/bsd-user/arm/target_arch_cpu.h > > +++ b/bsd-user/arm/target_arch_cpu.h > > @@ -21,6 +21,7 @@ > > #define _TARGET_ARCH_CPU_H_ > > > > #include "target_arch.h" > > +#include "signal-common.h" > > > > #define TARGET_DEFAULT_CPU_MODEL "any" > > > > @@ -64,19 +65,7 @@ static inline void target_cpu_loop(CPUARMState *env) > > } > > break; > > case EXCP_SWI: > > - case EXCP_BKPT: > > { > > - /* > > - * system call > > - * See arm/arm/trap.c cpu_fetch_syscall_args() > > - */ > > - if (trapnr == EXCP_BKPT) { > > - if (env->thumb) { > > - env->regs[15] += 2; > > - } else { > > - env->regs[15] += 4; > > - } > > - } > > So the previous code was implementing BKPT as a way to do > a syscall (added in commit 8d450c9a30). Was that just a mistake ? > I did some digging and I'm at a loss for why this code was ever here. > > n = env->regs[7]; > > if (bsd_type == target_freebsd) { > > int ret; > > @@ -171,14 +160,8 @@ static inline void target_cpu_loop(CPUARMState *env) > > queue_signal(env, info.si_signo, &info); > > break; > > case EXCP_DEBUG: > > - { > > - > > - info.si_signo = TARGET_SIGTRAP; > > - info.si_errno = 0; > > - info.si_code = TARGET_TRAP_BRKPT; > > - info.si_addr = env->exception.vaddress; > > - queue_signal(env, info.si_signo, &info); > > - } > > + case EXCP_BKPT: > > + force_sig_fault(TARGET_SIGTRAP, TARGET_TRAP_BRKPT, > env->regs[15]); > > break; > > case EXCP_YIELD: > > /* nothing to do here for user-mode, just resume guest code > */ > > Looks like it now matches the freebsd kernel behaviour, anyway. > Yea. That's why I went ahead and made the change rather than slavishly carry it over for something weird I couldn't find out about... I think it's an old mistake... I'll update the commit message to specifically note it. > Reviewed-by: Peter Maydell <peter.maydell@linaro.org> > > thanks > -- PMM >
On 1/10/22 3:18 AM, Warner Losh wrote: > Implement EXCP_DEBUG and EXCP_BKPT the same, as is done in > linux-user. The prior adjustment of register 15 isn't needed, so remove > that. Remove a redunant comment (that code in FreeBSD never handled > break points). > > Signed-off-by: Warner Losh<imp@bsdimp.com> > --- > bsd-user/arm/target_arch_cpu.h | 23 +++-------------------- > 1 file changed, 3 insertions(+), 20 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
diff --git a/bsd-user/arm/target_arch_cpu.h b/bsd-user/arm/target_arch_cpu.h index c526fc73502..05b19ce6119 100644 --- a/bsd-user/arm/target_arch_cpu.h +++ b/bsd-user/arm/target_arch_cpu.h @@ -21,6 +21,7 @@ #define _TARGET_ARCH_CPU_H_ #include "target_arch.h" +#include "signal-common.h" #define TARGET_DEFAULT_CPU_MODEL "any" @@ -64,19 +65,7 @@ static inline void target_cpu_loop(CPUARMState *env) } break; case EXCP_SWI: - case EXCP_BKPT: { - /* - * system call - * See arm/arm/trap.c cpu_fetch_syscall_args() - */ - if (trapnr == EXCP_BKPT) { - if (env->thumb) { - env->regs[15] += 2; - } else { - env->regs[15] += 4; - } - } n = env->regs[7]; if (bsd_type == target_freebsd) { int ret; @@ -171,14 +160,8 @@ static inline void target_cpu_loop(CPUARMState *env) queue_signal(env, info.si_signo, &info); break; case EXCP_DEBUG: - { - - info.si_signo = TARGET_SIGTRAP; - info.si_errno = 0; - info.si_code = TARGET_TRAP_BRKPT; - info.si_addr = env->exception.vaddress; - queue_signal(env, info.si_signo, &info); - } + case EXCP_BKPT: + force_sig_fault(TARGET_SIGTRAP, TARGET_TRAP_BRKPT, env->regs[15]); break; case EXCP_YIELD: /* nothing to do here for user-mode, just resume guest code */
Implement EXCP_DEBUG and EXCP_BKPT the same, as is done in linux-user. The prior adjustment of register 15 isn't needed, so remove that. Remove a redunant comment (that code in FreeBSD never handled break points). Signed-off-by: Warner Losh <imp@bsdimp.com> --- bsd-user/arm/target_arch_cpu.h | 23 +++-------------------- 1 file changed, 3 insertions(+), 20 deletions(-)