| Message ID | 20200420212206.12776-4-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | linux-user/arm: Fix BKPT, SVC immediate handling | expand |
On 4/20/20 11:22 PM, Peter Maydell wrote: > The kernel has different handling for syscalls with invalid > numbers that are in the "arm-specific" range 0x9f0000 and up: > * 0x9f0000..0x9f07ff return -ENOSYS if not implemented > * other out of range syscalls cause a SIGILL > (see the kernel's arch/arm/kernel/traps.c:arm_syscall()) > > Implement this distinction. (Note that our code doesn't look > quite like the kernel's, because we have removed the > 0x900000 prefix by this point, whereas the kernel retains > it in arm_syscall().) > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > linux-user/arm/cpu_loop.c | 30 ++++++++++++++++++++++++++---- > 1 file changed, 26 insertions(+), 4 deletions(-) > > diff --git a/linux-user/arm/cpu_loop.c b/linux-user/arm/cpu_loop.c > index 025887d6b86..f042108b0be 100644 > --- a/linux-user/arm/cpu_loop.c > +++ b/linux-user/arm/cpu_loop.c > @@ -332,10 +332,32 @@ void cpu_loop(CPUARMState *env) > env->regs[0] = cpu_get_tls(env); > break; > default: > - qemu_log_mask(LOG_UNIMP, > - "qemu: Unsupported ARM syscall: 0x%x\n", > - n); > - env->regs[0] = -TARGET_ENOSYS; > + if (n < 0xf0800) { > + /* > + * Syscalls 0xf0000..0xf07ff (or 0x9f0000.. > + * 0x9f07ff in OABI numbering) are defined > + * to return -ENOSYS rather than raising > + * SIGILL. Note that we have already > + * removed the 0x900000 prefix. > + */ > + qemu_log_mask(LOG_UNIMP, > + "qemu: Unsupported ARM syscall: 0x%x\n", > + n); > + env->regs[0] = -TARGET_ENOSYS; > + } else { > + /* Otherwise SIGILL */ > + info.si_signo = TARGET_SIGILL; > + info.si_errno = 0; > + info.si_code = TARGET_ILL_ILLTRP; > + info._sifields._sigfault._addr = env->regs[15]; > + if (env->thumb) { > + info._sifields._sigfault._addr -= 2; > + } else { > + info._sifields._sigfault._addr -= 2; > + } > + queue_signal(env, info.si_signo, > + QEMU_SI_FAULT, &info); > + } > break; > } > } else { > Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
On Mon, Apr 20, 2020 at 10:22:05PM +0100, Peter Maydell wrote: > The kernel has different handling for syscalls with invalid > numbers that are in the "arm-specific" range 0x9f0000 and up: > * 0x9f0000..0x9f07ff return -ENOSYS if not implemented > * other out of range syscalls cause a SIGILL > (see the kernel's arch/arm/kernel/traps.c:arm_syscall()) > > Implement this distinction. (Note that our code doesn't look > quite like the kernel's, because we have removed the > 0x900000 prefix by this point, whereas the kernel retains > it in arm_syscall().) > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > linux-user/arm/cpu_loop.c | 30 ++++++++++++++++++++++++++---- > 1 file changed, 26 insertions(+), 4 deletions(-) > > diff --git a/linux-user/arm/cpu_loop.c b/linux-user/arm/cpu_loop.c > index 025887d6b86..f042108b0be 100644 > --- a/linux-user/arm/cpu_loop.c > +++ b/linux-user/arm/cpu_loop.c > @@ -332,10 +332,32 @@ void cpu_loop(CPUARMState *env) > env->regs[0] = cpu_get_tls(env); > break; > default: > - qemu_log_mask(LOG_UNIMP, > - "qemu: Unsupported ARM syscall: 0x%x\n", > - n); > - env->regs[0] = -TARGET_ENOSYS; > + if (n < 0xf0800) { > + /* > + * Syscalls 0xf0000..0xf07ff (or 0x9f0000.. > + * 0x9f07ff in OABI numbering) are defined > + * to return -ENOSYS rather than raising > + * SIGILL. Note that we have already > + * removed the 0x900000 prefix. > + */ > + qemu_log_mask(LOG_UNIMP, > + "qemu: Unsupported ARM syscall: 0x%x\n", > + n); > + env->regs[0] = -TARGET_ENOSYS; > + } else { > + /* Otherwise SIGILL */ > + info.si_signo = TARGET_SIGILL; > + info.si_errno = 0; > + info.si_code = TARGET_ILL_ILLTRP; > + info._sifields._sigfault._addr = env->regs[15]; > + if (env->thumb) { > + info._sifields._sigfault._addr -= 2; > + } else { > + info._sifields._sigfault._addr -= 2; > + } Am I missing some detail or are both branches of the if-else doing the same thing? Cheers, Edgar > + queue_signal(env, info.si_signo, > + QEMU_SI_FAULT, &info); > + } > break; > } > } else { > -- > 2.20.1 > >
On 4/21/20 9:44 AM, Edgar E. Iglesias wrote: > On Mon, Apr 20, 2020 at 10:22:05PM +0100, Peter Maydell wrote: >> The kernel has different handling for syscalls with invalid >> numbers that are in the "arm-specific" range 0x9f0000 and up: >> * 0x9f0000..0x9f07ff return -ENOSYS if not implemented >> * other out of range syscalls cause a SIGILL >> (see the kernel's arch/arm/kernel/traps.c:arm_syscall()) >> >> Implement this distinction. (Note that our code doesn't look >> quite like the kernel's, because we have removed the >> 0x900000 prefix by this point, whereas the kernel retains >> it in arm_syscall().) >> >> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> >> --- >> linux-user/arm/cpu_loop.c | 30 ++++++++++++++++++++++++++---- >> 1 file changed, 26 insertions(+), 4 deletions(-) >> >> diff --git a/linux-user/arm/cpu_loop.c b/linux-user/arm/cpu_loop.c >> index 025887d6b86..f042108b0be 100644 >> --- a/linux-user/arm/cpu_loop.c >> +++ b/linux-user/arm/cpu_loop.c >> @@ -332,10 +332,32 @@ void cpu_loop(CPUARMState *env) >> env->regs[0] = cpu_get_tls(env); >> break; >> default: >> - qemu_log_mask(LOG_UNIMP, >> - "qemu: Unsupported ARM syscall: 0x%x\n", >> - n); >> - env->regs[0] = -TARGET_ENOSYS; >> + if (n < 0xf0800) { >> + /* >> + * Syscalls 0xf0000..0xf07ff (or 0x9f0000.. >> + * 0x9f07ff in OABI numbering) are defined >> + * to return -ENOSYS rather than raising >> + * SIGILL. Note that we have already >> + * removed the 0x900000 prefix. >> + */ >> + qemu_log_mask(LOG_UNIMP, >> + "qemu: Unsupported ARM syscall: 0x%x\n", >> + n); >> + env->regs[0] = -TARGET_ENOSYS; >> + } else { >> + /* Otherwise SIGILL */ >> + info.si_signo = TARGET_SIGILL; >> + info.si_errno = 0; >> + info.si_code = TARGET_ILL_ILLTRP; >> + info._sifields._sigfault._addr = env->regs[15]; >> + if (env->thumb) { >> + info._sifields._sigfault._addr -= 2; >> + } else { >> + info._sifields._sigfault._addr -= 2; >> + } > > > Am I missing some detail or are both branches of the if-else doing the > same thing? Oops good catch. R-b stands using '-= 4' on 2nd line. > > Cheers, > Edgar > > > >> + queue_signal(env, info.si_signo, >> + QEMU_SI_FAULT, &info); >> + } >> break; >> } >> } else { >> -- >> 2.20.1 >> >> >
On Tue, 21 Apr 2020 at 08:42, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote: > > On Mon, Apr 20, 2020 at 10:22:05PM +0100, Peter Maydell wrote: > > + if (env->thumb) { > > + info._sifields._sigfault._addr -= 2; > > + } else { > > + info._sifields._sigfault._addr -= 2; > > + } > > > Am I missing some detail or are both branches of the if-else doing the > same thing? Oops, yes: cut-n-paste error; as Philippe says, the not-thumb branch should be "-= 4". thanks -- PMM
пон, 20. апр 2020. у 23:25 Peter Maydell <peter.maydell@linaro.org> је написао/ла: > > The kernel has different handling for syscalls with invalid > numbers that are in the "arm-specific" range 0x9f0000 and up: > * 0x9f0000..0x9f07ff return -ENOSYS if not implemented > * other out of range syscalls cause a SIGILL > (see the kernel's arch/arm/kernel/traps.c:arm_syscall()) > > Implement this distinction. (Note that our code doesn't look > quite like the kernel's, because we have removed the > 0x900000 prefix by this point, whereas the kernel retains > it in arm_syscall().) > Hmm, I suspect other targets could have a similar problem. I am definitely going to take a look at the mips target, but did you Peter have a chance to take a more global look whether this problem is actually widespread? Regards, Aleksandar > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > linux-user/arm/cpu_loop.c | 30 ++++++++++++++++++++++++++---- > 1 file changed, 26 insertions(+), 4 deletions(-) > > diff --git a/linux-user/arm/cpu_loop.c b/linux-user/arm/cpu_loop.c > index 025887d6b86..f042108b0be 100644 > --- a/linux-user/arm/cpu_loop.c > +++ b/linux-user/arm/cpu_loop.c > @@ -332,10 +332,32 @@ void cpu_loop(CPUARMState *env) > env->regs[0] = cpu_get_tls(env); > break; > default: > - qemu_log_mask(LOG_UNIMP, > - "qemu: Unsupported ARM syscall: 0x%x\n", > - n); > - env->regs[0] = -TARGET_ENOSYS; > + if (n < 0xf0800) { > + /* > + * Syscalls 0xf0000..0xf07ff (or 0x9f0000.. > + * 0x9f07ff in OABI numbering) are defined > + * to return -ENOSYS rather than raising > + * SIGILL. Note that we have already > + * removed the 0x900000 prefix. > + */ > + qemu_log_mask(LOG_UNIMP, > + "qemu: Unsupported ARM syscall: 0x%x\n", > + n); > + env->regs[0] = -TARGET_ENOSYS; > + } else { > + /* Otherwise SIGILL */ > + info.si_signo = TARGET_SIGILL; > + info.si_errno = 0; > + info.si_code = TARGET_ILL_ILLTRP; > + info._sifields._sigfault._addr = env->regs[15]; > + if (env->thumb) { > + info._sifields._sigfault._addr -= 2; > + } else { > + info._sifields._sigfault._addr -= 2; > + } > + queue_signal(env, info.si_signo, > + QEMU_SI_FAULT, &info); > + } > break; > } > } else { > -- > 2.20.1 > >
On Tue, 21 Apr 2020 at 10:32, Aleksandar Markovic <aleksandar.qemu.devel@gmail.com> wrote: > > пон, 20. апр 2020. у 23:25 Peter Maydell <peter.maydell@linaro.org> је > написао/ла: > > > > The kernel has different handling for syscalls with invalid > > numbers that are in the "arm-specific" range 0x9f0000 and up: > > * 0x9f0000..0x9f07ff return -ENOSYS if not implemented > > * other out of range syscalls cause a SIGILL > > (see the kernel's arch/arm/kernel/traps.c:arm_syscall()) > > > > Implement this distinction. (Note that our code doesn't look > > quite like the kernel's, because we have removed the > > 0x900000 prefix by this point, whereas the kernel retains > > it in arm_syscall().) > > > > Hmm, I suspect other targets could have a similar problem. > > I am definitely going to take a look at the mips target, but did > you Peter have a chance to take a more global look whether > this problem is actually widespread? My guess is that this is Arm-specific, because both the OABI-vs-EABI "do we pass the syscall number in the insn immediate field or via a register" changeover and also the oddball "arm-specific handful of syscalls in a distinct range" are Arm hacks, not something the kernel deals with in generic code. thanks -- PMM
diff --git a/linux-user/arm/cpu_loop.c b/linux-user/arm/cpu_loop.c index 025887d6b86..f042108b0be 100644 --- a/linux-user/arm/cpu_loop.c +++ b/linux-user/arm/cpu_loop.c @@ -332,10 +332,32 @@ void cpu_loop(CPUARMState *env) env->regs[0] = cpu_get_tls(env); break; default: - qemu_log_mask(LOG_UNIMP, - "qemu: Unsupported ARM syscall: 0x%x\n", - n); - env->regs[0] = -TARGET_ENOSYS; + if (n < 0xf0800) { + /* + * Syscalls 0xf0000..0xf07ff (or 0x9f0000.. + * 0x9f07ff in OABI numbering) are defined + * to return -ENOSYS rather than raising + * SIGILL. Note that we have already + * removed the 0x900000 prefix. + */ + qemu_log_mask(LOG_UNIMP, + "qemu: Unsupported ARM syscall: 0x%x\n", + n); + env->regs[0] = -TARGET_ENOSYS; + } else { + /* Otherwise SIGILL */ + info.si_signo = TARGET_SIGILL; + info.si_errno = 0; + info.si_code = TARGET_ILL_ILLTRP; + info._sifields._sigfault._addr = env->regs[15]; + if (env->thumb) { + info._sifields._sigfault._addr -= 2; + } else { + info._sifields._sigfault._addr -= 2; + } + queue_signal(env, info.si_signo, + QEMU_SI_FAULT, &info); + } break; } } else {
The kernel has different handling for syscalls with invalid numbers that are in the "arm-specific" range 0x9f0000 and up: * 0x9f0000..0x9f07ff return -ENOSYS if not implemented * other out of range syscalls cause a SIGILL (see the kernel's arch/arm/kernel/traps.c:arm_syscall()) Implement this distinction. (Note that our code doesn't look quite like the kernel's, because we have removed the 0x900000 prefix by this point, whereas the kernel retains it in arm_syscall().) Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- linux-user/arm/cpu_loop.c | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) -- 2.20.1