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