On 2/4/19 12:02 PM, Peter Maydell wrote: > On Mon, 28 Jan 2019 at 22:31, Richard Henderson > <richard.hender...@linaro.org> wrote: >> >> The value of btype for syscalls is CONSTRAINED UNPREDICTABLE, >> so we need to make sure that the value is 0 before clone, >> fork, or syscall return. >> >> The value of btype for signals is defined, but it does not make >> sense for a SIGILL handler to enter with the btype set as for >> the indirect branch that caused the SIGILL. >> >> Clearing the value early means that btype is zero within the pstate >> saved into the signal frame, and so is also zero on (normal) signal >> return, but also allows the signal handler to adjust the value as >> seen after the sigcontext restore. >> >> This last is a guess at a future kernel's user-space ABI. >> >> Signed-off-by: Richard Henderson <richard.hender...@linaro.org> >> --- >> linux-user/aarch64/cpu_loop.c | 15 +++++++++++++++ >> 1 file changed, 15 insertions(+) >> >> diff --git a/linux-user/aarch64/cpu_loop.c b/linux-user/aarch64/cpu_loop.c >> index 65d815f030..51ea9961ba 100644 >> --- a/linux-user/aarch64/cpu_loop.c >> +++ b/linux-user/aarch64/cpu_loop.c >> @@ -83,8 +83,19 @@ void cpu_loop(CPUARMState *env) >> cpu_exec_end(cs); >> process_queued_cpu_work(cs); >> >> + /* >> + * The state of BTYPE on syscall and interrupt entry is CONSTRAINED >> + * UNPREDICTABLE. The real kernel will need to tidy this up as >> well. >> + * Do this before syscalls and signals, so that the value is correct >> + * both within signal handlers, and on return from syscall >> (especially >> + * clone & fork) and from signal handlers. >> + * >> + * The SIGILL signal handler, for BTITrap, can see the failing BTYPE >> + * within the ESR value in the signal frame. >> + */ >> switch (trapnr) { >> case EXCP_SWI: >> + env->btype = 0; >> ret = do_syscall(env, >> env->xregs[8], >> env->xregs[0], > > If the idea is to give a particular value on return from > the syscall and on entry to a signal handler, shouldn't we be > setting it after the do_syscall() call returns, and in the > signal handler entry path ? > >> @@ -104,6 +115,7 @@ void cpu_loop(CPUARMState *env) >> /* just indicate that signals should be handled asap */ >> break; >> case EXCP_UDEF: >> + env->btype = 0; >> info.si_signo = TARGET_SIGILL; >> info.si_errno = 0; >> info.si_code = TARGET_ILL_ILLOPN; >> @@ -112,6 +124,7 @@ void cpu_loop(CPUARMState *env) >> break; >> case EXCP_PREFETCH_ABORT: >> case EXCP_DATA_ABORT: >> + env->btype = 0; >> info.si_signo = TARGET_SIGSEGV; >> info.si_errno = 0; >> /* XXX: check env->error_code */ > >> @@ -121,12 +134,14 @@ void cpu_loop(CPUARMState *env) >> break; >> case EXCP_DEBUG: >> case EXCP_BKPT: >> + env->btype = 0; >> info.si_signo = TARGET_SIGTRAP; >> info.si_errno = 0; >> info.si_code = TARGET_TRAP_BRKPT; >> queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info); >> break; >> case EXCP_SEMIHOST: >> + env->btype = 0; > > Leaving btype alone rather than clearing it here would be > consistent with how we handle semihosting in system emulation, > right ?
Er.. yes. I sort of forgot we had semi-hosting for aa64. r~