[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 11/12] linux-user/aarch64: Reset btype for sy
From: |
Richard Henderson |
Subject: |
Re: [Qemu-devel] [PATCH v2 11/12] linux-user/aarch64: Reset btype for syscalls and signals |
Date: |
Mon, 4 Feb 2019 12:06:21 +0000 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 |
On 2/4/19 12:02 PM, Peter Maydell wrote:
> On Mon, 28 Jan 2019 at 22:31, Richard Henderson
> <address@hidden> 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 <address@hidden>
>> ---
>> 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~