* [Qemu-devel] [PATCH] Add support for the arm breakpoint syscall.
@ 2014-06-20 9:14 Hunter Laux
2014-06-20 9:51 ` Peter Maydell
0 siblings, 1 reply; 3+ messages in thread
From: Hunter Laux @ 2014-06-20 9:14 UTC (permalink / raw)
To: qemu-devel, qemu-trivial; +Cc: Hunter Laux
---
linux-user/arm/syscall.h | 1 +
linux-user/main.c | 4 ++++
2 files changed, 5 insertions(+)
diff --git a/linux-user/arm/syscall.h b/linux-user/arm/syscall.h
index ce2c2a8..e0d2cc3 100644
--- a/linux-user/arm/syscall.h
+++ b/linux-user/arm/syscall.h
@@ -29,6 +29,7 @@ struct target_pt_regs {
#define ARM_THUMB_SYSCALL 0
#define ARM_NR_BASE 0xf0000
+#define ARM_NR_breakpoint (ARM_NR_BASE + 1)
#define ARM_NR_cacheflush (ARM_NR_BASE + 2)
#define ARM_NR_set_tls (ARM_NR_BASE + 5)
diff --git a/linux-user/main.c b/linux-user/main.c
index a87c6f7..831b363 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -807,6 +807,9 @@ void cpu_loop(CPUARMState *env)
cpu_set_tls(env, env->regs[0]);
env->regs[0] = 0;
break;
+ case ARM_NR_breakpoint:
+ env->regs[15] -= env->thumb ? 2 : 4;
+ goto excp_debug;
default:
gemu_log("qemu: Unsupported ARM syscall: 0x%x\n",
n);
@@ -850,6 +853,7 @@ void cpu_loop(CPUARMState *env)
}
break;
case EXCP_DEBUG:
+ excp_debug:
{
int sig;
--
1.9.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] Add support for the arm breakpoint syscall.
2014-06-20 9:14 [Qemu-devel] [PATCH] Add support for the arm breakpoint syscall Hunter Laux
@ 2014-06-20 9:51 ` Peter Maydell
2014-06-21 12:44 ` [Qemu-devel] [Qemu-trivial] " Michael Tokarev
0 siblings, 1 reply; 3+ messages in thread
From: Peter Maydell @ 2014-06-20 9:51 UTC (permalink / raw)
To: Hunter Laux; +Cc: QEMU Trivial, Riku Voipio, QEMU Developers
[cc'ing Riku as the linux-user maintainer.]
On 20 June 2014 10:14, Hunter Laux <hunterlaux@gmail.com> wrote:
Thanks for writing this patch. It mostly looks good
but there are some minor admin/format tweaks we need.
You need to provide a Signed-off-by: line here, or
we can't take this patch, I'm afraid. (We work the same
way as the Linux kernel for this; see
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches?id=f6f94e2ab1b33f0082ac22d71f66385a60d8157f#n297
for more details, but basically it says you wrote the code
and are willing to contribute it under our licensing terms.)
It could also use a little more explanation in the commit
message.
FWIW, this is an obscure syscall that dates back to before
the architecture provided an official breakpoint instruction;
apparently it's still used by Steel Bank Common Lisp.
This is the kernel implementation:
http://lxr.free-electrons.com/source/arch/arm/kernel/traps.c#L598
> ---
> linux-user/arm/syscall.h | 1 +
> linux-user/main.c | 4 ++++
> 2 files changed, 5 insertions(+)
>
> diff --git a/linux-user/arm/syscall.h b/linux-user/arm/syscall.h
> index ce2c2a8..e0d2cc3 100644
> --- a/linux-user/arm/syscall.h
> +++ b/linux-user/arm/syscall.h
> @@ -29,6 +29,7 @@ struct target_pt_regs {
> #define ARM_THUMB_SYSCALL 0
>
> #define ARM_NR_BASE 0xf0000
> +#define ARM_NR_breakpoint (ARM_NR_BASE + 1)
> #define ARM_NR_cacheflush (ARM_NR_BASE + 2)
> #define ARM_NR_set_tls (ARM_NR_BASE + 5)
>
> diff --git a/linux-user/main.c b/linux-user/main.c
> index a87c6f7..831b363 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -807,6 +807,9 @@ void cpu_loop(CPUARMState *env)
> cpu_set_tls(env, env->regs[0]);
> env->regs[0] = 0;
> break;
> + case ARM_NR_breakpoint:
> + env->regs[15] -= env->thumb ? 2 : 4;
> + goto excp_debug;
> default:
> gemu_log("qemu: Unsupported ARM syscall: 0x%x\n",
> n);
> @@ -850,6 +853,7 @@ void cpu_loop(CPUARMState *env)
> }
> break;
> case EXCP_DEBUG:
> + excp_debug:
The indent on this label isn't quite right; it should
be 4 columns to the left.
> {
> int sig;
Those minor issues aside, the code looks right to me.
thanks
-- PMM
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [Qemu-trivial] [PATCH] Add support for the arm breakpoint syscall.
2014-06-20 9:51 ` Peter Maydell
@ 2014-06-21 12:44 ` Michael Tokarev
0 siblings, 0 replies; 3+ messages in thread
From: Michael Tokarev @ 2014-06-21 12:44 UTC (permalink / raw)
To: Peter Maydell, Hunter Laux; +Cc: QEMU Trivial, Riku Voipio, QEMU Developers
20.06.2014 13:51, Peter Maydell пишет:
> [cc'ing Riku as the linux-user maintainer.]
>
> On 20 June 2014 10:14, Hunter Laux <hunterlaux@gmail.com> wrote:
>
> Thanks for writing this patch. It mostly looks good
> but there are some minor admin/format tweaks we need.
>
> You need to provide a Signed-off-by: line here, or
> we can't take this patch, I'm afraid. (We work the same
> way as the Linux kernel for this; see
> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches?id=f6f94e2ab1b33f0082ac22d71f66385a60d8157f#n297
> for more details, but basically it says you wrote the code
> and are willing to contribute it under our licensing terms.)
>
> It could also use a little more explanation in the commit
> message.
>
> FWIW, this is an obscure syscall that dates back to before
> the architecture provided an official breakpoint instruction;
> apparently it's still used by Steel Bank Common Lisp.
>
> This is the kernel implementation:
> http://lxr.free-electrons.com/source/arch/arm/kernel/traps.c#L598
>
>> ---
>> linux-user/arm/syscall.h | 1 +
>> linux-user/main.c | 4 ++++
>> 2 files changed, 5 insertions(+)
>>
>> diff --git a/linux-user/arm/syscall.h b/linux-user/arm/syscall.h
>> index ce2c2a8..e0d2cc3 100644
>> --- a/linux-user/arm/syscall.h
>> +++ b/linux-user/arm/syscall.h
>> @@ -29,6 +29,7 @@ struct target_pt_regs {
>> #define ARM_THUMB_SYSCALL 0
>>
>> #define ARM_NR_BASE 0xf0000
>> +#define ARM_NR_breakpoint (ARM_NR_BASE + 1)
>> #define ARM_NR_cacheflush (ARM_NR_BASE + 2)
>> #define ARM_NR_set_tls (ARM_NR_BASE + 5)
>>
>> diff --git a/linux-user/main.c b/linux-user/main.c
>> index a87c6f7..831b363 100644
>> --- a/linux-user/main.c
>> +++ b/linux-user/main.c
>> @@ -807,6 +807,9 @@ void cpu_loop(CPUARMState *env)
>> cpu_set_tls(env, env->regs[0]);
>> env->regs[0] = 0;
>> break;
>> + case ARM_NR_breakpoint:
>> + env->regs[15] -= env->thumb ? 2 : 4;
>> + goto excp_debug;
>> default:
>> gemu_log("qemu: Unsupported ARM syscall: 0x%x\n",
>> n);
>> @@ -850,6 +853,7 @@ void cpu_loop(CPUARMState *env)
>> }
>> break;
>> case EXCP_DEBUG:
>> + excp_debug:
>
> The indent on this label isn't quite right; it should
> be 4 columns to the left.
Isn't whole thing - using label and goto like this, intermixing
two switch statements with goto - a bit flawed? At best it makes
the code unclean...
>> {
>> int sig;
>
> Those minor issues aside, the code looks right to me.
There was another submission of the same patch, but with
a S-o-b line.
Thanks,
/mjt
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-06-21 12:44 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-20 9:14 [Qemu-devel] [PATCH] Add support for the arm breakpoint syscall Hunter Laux
2014-06-20 9:51 ` Peter Maydell
2014-06-21 12:44 ` [Qemu-devel] [Qemu-trivial] " Michael Tokarev
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.