* [Qemu-devel] [PATCH 1/2] Improve Monitor disas with symbol lookup
@ 2013-08-02 12:48 ` Fabien Chouteau
0 siblings, 0 replies; 20+ messages in thread
From: Fabien Chouteau @ 2013-08-02 12:48 UTC (permalink / raw)
To: qemu-trivial; +Cc: qemu-devel, lcapitulino
Part of M731-018.
Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
---
disas.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/disas.c b/disas.c
index 71007fb..3ffb3ae 100644
--- a/disas.c
+++ b/disas.c
@@ -480,11 +480,19 @@ void monitor_disas(Monitor *mon, CPUArchState *env,
#endif
for(i = 0; i < nb_insn; i++) {
- monitor_printf(mon, "0x" TARGET_FMT_lx ": ", pc);
+ const char *sym = lookup_symbol(pc);
+
+ monitor_printf(mon, "0x" TARGET_FMT_lx " ", pc);
+ if (sym[0] != '\0') {
+ monitor_printf(mon, "<%s>: ", sym);
+ } else {
+ monitor_printf(mon, ": ");
+ }
count = print_insn(pc, &s.info);
- monitor_printf(mon, "\n");
- if (count < 0)
- break;
+ monitor_printf(mon, "\n");
+ if (count < 0) {
+ break;
+ }
pc += count;
}
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 20+ messages in thread* [Qemu-trivial] [PATCH 2/2] Add ARM registers definitions in Monitor commands
2013-08-02 12:48 ` [Qemu-devel] " Fabien Chouteau
@ 2013-08-02 12:48 ` Fabien Chouteau
-1 siblings, 0 replies; 20+ messages in thread
From: Fabien Chouteau @ 2013-08-02 12:48 UTC (permalink / raw)
To: qemu-trivial; +Cc: qemu-devel, lcapitulino
Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
---
monitor.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/monitor.c b/monitor.c
index 5dc0aa9..78e93af 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3167,6 +3167,23 @@ static const MonitorDef monitor_defs[] = {
{ "cleanwin", offsetof(CPUSPARCState, cleanwin) },
{ "fprs", offsetof(CPUSPARCState, fprs) },
#endif
+#elif defined(TARGET_ARM)
+ { "r0", offsetof(CPUARMState, regs[0]) },
+ { "r1", offsetof(CPUARMState, regs[1]) },
+ { "r2", offsetof(CPUARMState, regs[2]) },
+ { "r3", offsetof(CPUARMState, regs[3]) },
+ { "r4", offsetof(CPUARMState, regs[4]) },
+ { "r5", offsetof(CPUARMState, regs[5]) },
+ { "r6", offsetof(CPUARMState, regs[6]) },
+ { "r7", offsetof(CPUARMState, regs[7]) },
+ { "r8", offsetof(CPUARMState, regs[8]) },
+ { "r9", offsetof(CPUARMState, regs[9]) },
+ { "r10", offsetof(CPUARMState, regs[10]) },
+ { "r11", offsetof(CPUARMState, regs[11]) },
+ { "r12", offsetof(CPUARMState, regs[12]) },
+ { "r13|sp", offsetof(CPUARMState, regs[13]) },
+ { "r14|lr", offsetof(CPUARMState, regs[14]) },
+ { "r15|pc", offsetof(CPUARMState, regs[15]) },
#endif
{ NULL },
};
--
1.7.9.5
^ permalink raw reply related [flat|nested] 20+ messages in thread* [Qemu-devel] [PATCH 2/2] Add ARM registers definitions in Monitor commands
@ 2013-08-02 12:48 ` Fabien Chouteau
0 siblings, 0 replies; 20+ messages in thread
From: Fabien Chouteau @ 2013-08-02 12:48 UTC (permalink / raw)
To: qemu-trivial; +Cc: qemu-devel, lcapitulino
Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
---
monitor.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/monitor.c b/monitor.c
index 5dc0aa9..78e93af 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3167,6 +3167,23 @@ static const MonitorDef monitor_defs[] = {
{ "cleanwin", offsetof(CPUSPARCState, cleanwin) },
{ "fprs", offsetof(CPUSPARCState, fprs) },
#endif
+#elif defined(TARGET_ARM)
+ { "r0", offsetof(CPUARMState, regs[0]) },
+ { "r1", offsetof(CPUARMState, regs[1]) },
+ { "r2", offsetof(CPUARMState, regs[2]) },
+ { "r3", offsetof(CPUARMState, regs[3]) },
+ { "r4", offsetof(CPUARMState, regs[4]) },
+ { "r5", offsetof(CPUARMState, regs[5]) },
+ { "r6", offsetof(CPUARMState, regs[6]) },
+ { "r7", offsetof(CPUARMState, regs[7]) },
+ { "r8", offsetof(CPUARMState, regs[8]) },
+ { "r9", offsetof(CPUARMState, regs[9]) },
+ { "r10", offsetof(CPUARMState, regs[10]) },
+ { "r11", offsetof(CPUARMState, regs[11]) },
+ { "r12", offsetof(CPUARMState, regs[12]) },
+ { "r13|sp", offsetof(CPUARMState, regs[13]) },
+ { "r14|lr", offsetof(CPUARMState, regs[14]) },
+ { "r15|pc", offsetof(CPUARMState, regs[15]) },
#endif
{ NULL },
};
--
1.7.9.5
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [Qemu-trivial] [Qemu-devel] [PATCH 2/2] Add ARM registers definitions in Monitor commands
2013-08-02 12:48 ` [Qemu-devel] " Fabien Chouteau
@ 2013-08-19 8:26 ` Fabien Chouteau
-1 siblings, 0 replies; 20+ messages in thread
From: Fabien Chouteau @ 2013-08-19 8:26 UTC (permalink / raw)
To: qemu-trivial; +Cc: qemu-devel, lcapitulino
Any comments?
Regards,
On 08/02/2013 02:48 PM, Fabien Chouteau wrote:
> Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
> ---
> monitor.c | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/monitor.c b/monitor.c
> index 5dc0aa9..78e93af 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -3167,6 +3167,23 @@ static const MonitorDef monitor_defs[] = {
> { "cleanwin", offsetof(CPUSPARCState, cleanwin) },
> { "fprs", offsetof(CPUSPARCState, fprs) },
> #endif
> +#elif defined(TARGET_ARM)
> + { "r0", offsetof(CPUARMState, regs[0]) },
> + { "r1", offsetof(CPUARMState, regs[1]) },
> + { "r2", offsetof(CPUARMState, regs[2]) },
> + { "r3", offsetof(CPUARMState, regs[3]) },
> + { "r4", offsetof(CPUARMState, regs[4]) },
> + { "r5", offsetof(CPUARMState, regs[5]) },
> + { "r6", offsetof(CPUARMState, regs[6]) },
> + { "r7", offsetof(CPUARMState, regs[7]) },
> + { "r8", offsetof(CPUARMState, regs[8]) },
> + { "r9", offsetof(CPUARMState, regs[9]) },
> + { "r10", offsetof(CPUARMState, regs[10]) },
> + { "r11", offsetof(CPUARMState, regs[11]) },
> + { "r12", offsetof(CPUARMState, regs[12]) },
> + { "r13|sp", offsetof(CPUARMState, regs[13]) },
> + { "r14|lr", offsetof(CPUARMState, regs[14]) },
> + { "r15|pc", offsetof(CPUARMState, regs[15]) },
> #endif
> { NULL },
> };
>
--
Fabien Chouteau
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [Qemu-devel] [PATCH 2/2] Add ARM registers definitions in Monitor commands
@ 2013-08-19 8:26 ` Fabien Chouteau
0 siblings, 0 replies; 20+ messages in thread
From: Fabien Chouteau @ 2013-08-19 8:26 UTC (permalink / raw)
To: qemu-trivial; +Cc: qemu-devel, lcapitulino
Any comments?
Regards,
On 08/02/2013 02:48 PM, Fabien Chouteau wrote:
> Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
> ---
> monitor.c | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/monitor.c b/monitor.c
> index 5dc0aa9..78e93af 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -3167,6 +3167,23 @@ static const MonitorDef monitor_defs[] = {
> { "cleanwin", offsetof(CPUSPARCState, cleanwin) },
> { "fprs", offsetof(CPUSPARCState, fprs) },
> #endif
> +#elif defined(TARGET_ARM)
> + { "r0", offsetof(CPUARMState, regs[0]) },
> + { "r1", offsetof(CPUARMState, regs[1]) },
> + { "r2", offsetof(CPUARMState, regs[2]) },
> + { "r3", offsetof(CPUARMState, regs[3]) },
> + { "r4", offsetof(CPUARMState, regs[4]) },
> + { "r5", offsetof(CPUARMState, regs[5]) },
> + { "r6", offsetof(CPUARMState, regs[6]) },
> + { "r7", offsetof(CPUARMState, regs[7]) },
> + { "r8", offsetof(CPUARMState, regs[8]) },
> + { "r9", offsetof(CPUARMState, regs[9]) },
> + { "r10", offsetof(CPUARMState, regs[10]) },
> + { "r11", offsetof(CPUARMState, regs[11]) },
> + { "r12", offsetof(CPUARMState, regs[12]) },
> + { "r13|sp", offsetof(CPUARMState, regs[13]) },
> + { "r14|lr", offsetof(CPUARMState, regs[14]) },
> + { "r15|pc", offsetof(CPUARMState, regs[15]) },
> #endif
> { NULL },
> };
>
--
Fabien Chouteau
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [Qemu-trivial] [Qemu-devel] [PATCH 2/2] Add ARM registers definitions in Monitor commands
2013-08-19 8:26 ` Fabien Chouteau
@ 2013-08-19 8:31 ` Peter Maydell
-1 siblings, 0 replies; 20+ messages in thread
From: Peter Maydell @ 2013-08-19 8:31 UTC (permalink / raw)
To: Fabien Chouteau
Cc: qemu-trivial, qemu-devel, Andreas Färber, lcapitulino
On 19 August 2013 09:26, Fabien Chouteau <chouteau@adacore.com> wrote:
> Any comments?
>
> Regards,
>
> On 08/02/2013 02:48 PM, Fabien Chouteau wrote:
>> Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
>> ---
>> monitor.c | 17 +++++++++++++++++
>> 1 file changed, 17 insertions(+)
>>
>> diff --git a/monitor.c b/monitor.c
>> index 5dc0aa9..78e93af 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -3167,6 +3167,23 @@ static const MonitorDef monitor_defs[] = {
>> { "cleanwin", offsetof(CPUSPARCState, cleanwin) },
>> { "fprs", offsetof(CPUSPARCState, fprs) },
>> #endif
>> +#elif defined(TARGET_ARM)
>> + { "r0", offsetof(CPUARMState, regs[0]) },
>> + { "r1", offsetof(CPUARMState, regs[1]) },
Rather than adding yet another entry to this target-ifdef
ladder in common code, maybe we can abstract this out to be
a method/field of the CPU object?
Andreas can probably suggest the best approach.
thanks
-- PMM
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [Qemu-devel] [PATCH 2/2] Add ARM registers definitions in Monitor commands
@ 2013-08-19 8:31 ` Peter Maydell
0 siblings, 0 replies; 20+ messages in thread
From: Peter Maydell @ 2013-08-19 8:31 UTC (permalink / raw)
To: Fabien Chouteau
Cc: qemu-trivial, qemu-devel, Andreas Färber, lcapitulino
On 19 August 2013 09:26, Fabien Chouteau <chouteau@adacore.com> wrote:
> Any comments?
>
> Regards,
>
> On 08/02/2013 02:48 PM, Fabien Chouteau wrote:
>> Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
>> ---
>> monitor.c | 17 +++++++++++++++++
>> 1 file changed, 17 insertions(+)
>>
>> diff --git a/monitor.c b/monitor.c
>> index 5dc0aa9..78e93af 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -3167,6 +3167,23 @@ static const MonitorDef monitor_defs[] = {
>> { "cleanwin", offsetof(CPUSPARCState, cleanwin) },
>> { "fprs", offsetof(CPUSPARCState, fprs) },
>> #endif
>> +#elif defined(TARGET_ARM)
>> + { "r0", offsetof(CPUARMState, regs[0]) },
>> + { "r1", offsetof(CPUARMState, regs[1]) },
Rather than adding yet another entry to this target-ifdef
ladder in common code, maybe we can abstract this out to be
a method/field of the CPU object?
Andreas can probably suggest the best approach.
thanks
-- PMM
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [Qemu-trivial] [Qemu-devel] [PATCH 2/2] Add ARM registers definitions in Monitor commands
2013-08-19 8:31 ` Peter Maydell
@ 2013-08-19 14:24 ` Andreas Färber
-1 siblings, 0 replies; 20+ messages in thread
From: Andreas Färber @ 2013-08-19 14:24 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-trivial, qemu-devel, lcapitulino
Am 19.08.2013 10:31, schrieb Peter Maydell:
> On 19 August 2013 09:26, Fabien Chouteau <chouteau@adacore.com> wrote:
>> Any comments?
>>
>> Regards,
>>
>> On 08/02/2013 02:48 PM, Fabien Chouteau wrote:
>>> Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
>>> ---
>>> monitor.c | 17 +++++++++++++++++
>>> 1 file changed, 17 insertions(+)
>>>
>>> diff --git a/monitor.c b/monitor.c
>>> index 5dc0aa9..78e93af 100644
>>> --- a/monitor.c
>>> +++ b/monitor.c
>>> @@ -3167,6 +3167,23 @@ static const MonitorDef monitor_defs[] = {
>>> { "cleanwin", offsetof(CPUSPARCState, cleanwin) },
>>> { "fprs", offsetof(CPUSPARCState, fprs) },
>>> #endif
>>> +#elif defined(TARGET_ARM)
>>> + { "r0", offsetof(CPUARMState, regs[0]) },
>>> + { "r1", offsetof(CPUARMState, regs[1]) },
>
> Rather than adding yet another entry to this target-ifdef
> ladder in common code, maybe we can abstract this out to be
> a method/field of the CPU object?
>
> Andreas can probably suggest the best approach.
I had similar thoughts when I first saw this patch... I haven't looked
closely into monitor yet though (other than seeing it has too much
target dependencies).
Either a MonitorDef* field (null-terminating the array then) or,
depending on where/how this is used, a method in CPUClass might be
options to better abstract this. Given that there is going to be a
little bit of time before 1.7 now we could take some time to rethink our
design.
A related question that remained unanswered for the Program Counter in
the gdbstub context was whether we want QOM properties for such
registers, which would hint at further abstracting such a register list
to reuse it outside monitor.
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [Qemu-devel] [PATCH 2/2] Add ARM registers definitions in Monitor commands
@ 2013-08-19 14:24 ` Andreas Färber
0 siblings, 0 replies; 20+ messages in thread
From: Andreas Färber @ 2013-08-19 14:24 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-trivial, qemu-devel, Fabien Chouteau, lcapitulino
Am 19.08.2013 10:31, schrieb Peter Maydell:
> On 19 August 2013 09:26, Fabien Chouteau <chouteau@adacore.com> wrote:
>> Any comments?
>>
>> Regards,
>>
>> On 08/02/2013 02:48 PM, Fabien Chouteau wrote:
>>> Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
>>> ---
>>> monitor.c | 17 +++++++++++++++++
>>> 1 file changed, 17 insertions(+)
>>>
>>> diff --git a/monitor.c b/monitor.c
>>> index 5dc0aa9..78e93af 100644
>>> --- a/monitor.c
>>> +++ b/monitor.c
>>> @@ -3167,6 +3167,23 @@ static const MonitorDef monitor_defs[] = {
>>> { "cleanwin", offsetof(CPUSPARCState, cleanwin) },
>>> { "fprs", offsetof(CPUSPARCState, fprs) },
>>> #endif
>>> +#elif defined(TARGET_ARM)
>>> + { "r0", offsetof(CPUARMState, regs[0]) },
>>> + { "r1", offsetof(CPUARMState, regs[1]) },
>
> Rather than adding yet another entry to this target-ifdef
> ladder in common code, maybe we can abstract this out to be
> a method/field of the CPU object?
>
> Andreas can probably suggest the best approach.
I had similar thoughts when I first saw this patch... I haven't looked
closely into monitor yet though (other than seeing it has too much
target dependencies).
Either a MonitorDef* field (null-terminating the array then) or,
depending on where/how this is used, a method in CPUClass might be
options to better abstract this. Given that there is going to be a
little bit of time before 1.7 now we could take some time to rethink our
design.
A related question that remained unanswered for the Program Counter in
the gdbstub context was whether we want QOM properties for such
registers, which would hint at further abstracting such a register list
to reuse it outside monitor.
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-trivial] [Qemu-devel] [PATCH 1/2] Improve Monitor disas with symbol lookup
2013-08-02 12:48 ` [Qemu-devel] " Fabien Chouteau
@ 2013-08-19 8:27 ` Fabien Chouteau
-1 siblings, 0 replies; 20+ messages in thread
From: Fabien Chouteau @ 2013-08-19 8:27 UTC (permalink / raw)
To: qemu-trivial; +Cc: qemu-devel, lcapitulino
Any comments?
Regards,
On 08/02/2013 02:48 PM, Fabien Chouteau wrote:
> Part of M731-018.
>
> Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
> ---
> disas.c | 16 ++++++++++++----
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/disas.c b/disas.c
> index 71007fb..3ffb3ae 100644
> --- a/disas.c
> +++ b/disas.c
> @@ -480,11 +480,19 @@ void monitor_disas(Monitor *mon, CPUArchState *env,
> #endif
>
> for(i = 0; i < nb_insn; i++) {
> - monitor_printf(mon, "0x" TARGET_FMT_lx ": ", pc);
> + const char *sym = lookup_symbol(pc);
> +
> + monitor_printf(mon, "0x" TARGET_FMT_lx " ", pc);
> + if (sym[0] != '\0') {
> + monitor_printf(mon, "<%s>: ", sym);
> + } else {
> + monitor_printf(mon, ": ");
> + }
> count = print_insn(pc, &s.info);
> - monitor_printf(mon, "\n");
> - if (count < 0)
> - break;
> + monitor_printf(mon, "\n");
> + if (count < 0) {
> + break;
> + }
> pc += count;
> }
> }
>
--
Fabien Chouteau
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [Qemu-devel] [PATCH 1/2] Improve Monitor disas with symbol lookup
@ 2013-08-19 8:27 ` Fabien Chouteau
0 siblings, 0 replies; 20+ messages in thread
From: Fabien Chouteau @ 2013-08-19 8:27 UTC (permalink / raw)
To: qemu-trivial; +Cc: qemu-devel, lcapitulino
Any comments?
Regards,
On 08/02/2013 02:48 PM, Fabien Chouteau wrote:
> Part of M731-018.
>
> Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
> ---
> disas.c | 16 ++++++++++++----
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/disas.c b/disas.c
> index 71007fb..3ffb3ae 100644
> --- a/disas.c
> +++ b/disas.c
> @@ -480,11 +480,19 @@ void monitor_disas(Monitor *mon, CPUArchState *env,
> #endif
>
> for(i = 0; i < nb_insn; i++) {
> - monitor_printf(mon, "0x" TARGET_FMT_lx ": ", pc);
> + const char *sym = lookup_symbol(pc);
> +
> + monitor_printf(mon, "0x" TARGET_FMT_lx " ", pc);
> + if (sym[0] != '\0') {
> + monitor_printf(mon, "<%s>: ", sym);
> + } else {
> + monitor_printf(mon, ": ");
> + }
> count = print_insn(pc, &s.info);
> - monitor_printf(mon, "\n");
> - if (count < 0)
> - break;
> + monitor_printf(mon, "\n");
> + if (count < 0) {
> + break;
> + }
> pc += count;
> }
> }
>
--
Fabien Chouteau
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-trivial] [Qemu-devel] [PATCH 1/2] Improve Monitor disas with symbol lookup
2013-08-02 12:48 ` [Qemu-devel] " Fabien Chouteau
@ 2013-08-19 8:33 ` Peter Maydell
-1 siblings, 0 replies; 20+ messages in thread
From: Peter Maydell @ 2013-08-19 8:33 UTC (permalink / raw)
To: Fabien Chouteau; +Cc: qemu-trivial, qemu-devel, lcapitulino
On 2 August 2013 13:48, Fabien Chouteau <chouteau@adacore.com> wrote:
> Part of M731-018.
What is this a reference to?
> Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
> ---
> disas.c | 16 ++++++++++++----
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/disas.c b/disas.c
> index 71007fb..3ffb3ae 100644
> --- a/disas.c
> +++ b/disas.c
> @@ -480,11 +480,19 @@ void monitor_disas(Monitor *mon, CPUArchState *env,
> #endif
>
> for(i = 0; i < nb_insn; i++) {
> - monitor_printf(mon, "0x" TARGET_FMT_lx ": ", pc);
> + const char *sym = lookup_symbol(pc);
> +
> + monitor_printf(mon, "0x" TARGET_FMT_lx " ", pc);
> + if (sym[0] != '\0') {
> + monitor_printf(mon, "<%s>: ", sym);
> + } else {
> + monitor_printf(mon, ": ");
> + }
It feels to me like this is at the wrong level:
shouldn't it be in the disassembly layer so that you
can get symbols in both monitor disassembly and
debug-log disassembly?
thanks
-- PMM
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [Qemu-devel] [PATCH 1/2] Improve Monitor disas with symbol lookup
@ 2013-08-19 8:33 ` Peter Maydell
0 siblings, 0 replies; 20+ messages in thread
From: Peter Maydell @ 2013-08-19 8:33 UTC (permalink / raw)
To: Fabien Chouteau; +Cc: qemu-trivial, qemu-devel, lcapitulino
On 2 August 2013 13:48, Fabien Chouteau <chouteau@adacore.com> wrote:
> Part of M731-018.
What is this a reference to?
> Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
> ---
> disas.c | 16 ++++++++++++----
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/disas.c b/disas.c
> index 71007fb..3ffb3ae 100644
> --- a/disas.c
> +++ b/disas.c
> @@ -480,11 +480,19 @@ void monitor_disas(Monitor *mon, CPUArchState *env,
> #endif
>
> for(i = 0; i < nb_insn; i++) {
> - monitor_printf(mon, "0x" TARGET_FMT_lx ": ", pc);
> + const char *sym = lookup_symbol(pc);
> +
> + monitor_printf(mon, "0x" TARGET_FMT_lx " ", pc);
> + if (sym[0] != '\0') {
> + monitor_printf(mon, "<%s>: ", sym);
> + } else {
> + monitor_printf(mon, ": ");
> + }
It feels to me like this is at the wrong level:
shouldn't it be in the disassembly layer so that you
can get symbols in both monitor disassembly and
debug-log disassembly?
thanks
-- PMM
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [Qemu-trivial] [Qemu-devel] [PATCH 1/2] Improve Monitor disas with symbol lookup
2013-08-19 8:33 ` Peter Maydell
@ 2013-08-19 14:08 ` Fabien Chouteau
-1 siblings, 0 replies; 20+ messages in thread
From: Fabien Chouteau @ 2013-08-19 14:08 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-trivial, qemu-devel, lcapitulino
On 08/19/2013 10:33 AM, Peter Maydell wrote:
> On 2 August 2013 13:48, Fabien Chouteau <chouteau@adacore.com> wrote:
>> Part of M731-018.
>
> What is this a reference to?
>
I'm sorry this is not supposed to be here. Do you have comments before I
resend the patch?
Regards,
--
Fabien Chouteau
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-trivial] [Qemu-devel] [PATCH 1/2] Improve Monitor disas with symbol lookup
2013-08-02 12:48 ` [Qemu-devel] " Fabien Chouteau
@ 2013-08-19 14:14 ` Andreas Färber
-1 siblings, 0 replies; 20+ messages in thread
From: Andreas Färber @ 2013-08-19 14:14 UTC (permalink / raw)
To: Fabien Chouteau; +Cc: qemu-trivial, Peter Maydell, qemu-devel, lcapitulino
Am 02.08.2013 14:48, schrieb Fabien Chouteau:
> Part of M731-018.
>
> Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
> ---
> disas.c | 16 ++++++++++++----
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/disas.c b/disas.c
> index 71007fb..3ffb3ae 100644
> --- a/disas.c
> +++ b/disas.c
> @@ -480,11 +480,19 @@ void monitor_disas(Monitor *mon, CPUArchState *env,
> #endif
>
> for(i = 0; i < nb_insn; i++) {
> - monitor_printf(mon, "0x" TARGET_FMT_lx ": ", pc);
> + const char *sym = lookup_symbol(pc);
> +
> + monitor_printf(mon, "0x" TARGET_FMT_lx " ", pc);
> + if (sym[0] != '\0') {
> + monitor_printf(mon, "<%s>: ", sym);
> + } else {
> + monitor_printf(mon, ": ");
> + }
Independent of PMM's comment, I think you meant
+ monitor_printf(mon, "0x" TARGET_FMT_lx, pc);
+ if (sym[0] != '\0') {
+ monitor_printf(mon, " <%s>: ", sym);
+ } else {
+ monitor_printf(mon, ": ");
+ }
to keep the output unchanged.
Could you please put the tab fixing into a preceding patch for
readability and prepend a cover letter?
Regards,
Andreas
> count = print_insn(pc, &s.info);
> - monitor_printf(mon, "\n");
> - if (count < 0)
> - break;
> + monitor_printf(mon, "\n");
> + if (count < 0) {
> + break;
> + }
> pc += count;
> }
> }
>
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [Qemu-devel] [PATCH 1/2] Improve Monitor disas with symbol lookup
@ 2013-08-19 14:14 ` Andreas Färber
0 siblings, 0 replies; 20+ messages in thread
From: Andreas Färber @ 2013-08-19 14:14 UTC (permalink / raw)
To: Fabien Chouteau; +Cc: qemu-trivial, Peter Maydell, qemu-devel, lcapitulino
Am 02.08.2013 14:48, schrieb Fabien Chouteau:
> Part of M731-018.
>
> Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
> ---
> disas.c | 16 ++++++++++++----
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/disas.c b/disas.c
> index 71007fb..3ffb3ae 100644
> --- a/disas.c
> +++ b/disas.c
> @@ -480,11 +480,19 @@ void monitor_disas(Monitor *mon, CPUArchState *env,
> #endif
>
> for(i = 0; i < nb_insn; i++) {
> - monitor_printf(mon, "0x" TARGET_FMT_lx ": ", pc);
> + const char *sym = lookup_symbol(pc);
> +
> + monitor_printf(mon, "0x" TARGET_FMT_lx " ", pc);
> + if (sym[0] != '\0') {
> + monitor_printf(mon, "<%s>: ", sym);
> + } else {
> + monitor_printf(mon, ": ");
> + }
Independent of PMM's comment, I think you meant
+ monitor_printf(mon, "0x" TARGET_FMT_lx, pc);
+ if (sym[0] != '\0') {
+ monitor_printf(mon, " <%s>: ", sym);
+ } else {
+ monitor_printf(mon, ": ");
+ }
to keep the output unchanged.
Could you please put the tab fixing into a preceding patch for
readability and prepend a cover letter?
Regards,
Andreas
> count = print_insn(pc, &s.info);
> - monitor_printf(mon, "\n");
> - if (count < 0)
> - break;
> + monitor_printf(mon, "\n");
> + if (count < 0) {
> + break;
> + }
> pc += count;
> }
> }
>
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [Qemu-trivial] [Qemu-devel] [PATCH 1/2] Improve Monitor disas with symbol lookup
2013-08-19 14:14 ` Andreas Färber
@ 2013-08-19 16:05 ` Fabien Chouteau
-1 siblings, 0 replies; 20+ messages in thread
From: Fabien Chouteau @ 2013-08-19 16:05 UTC (permalink / raw)
To: Andreas Färber; +Cc: qemu-trivial, Peter Maydell, qemu-devel, lcapitulino
On 08/19/2013 04:14 PM, Andreas Färber wrote:
> Am 02.08.2013 14:48, schrieb Fabien Chouteau:
>> Part of M731-018.
>>
>> Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
>> ---
>> disas.c | 16 ++++++++++++----
>> 1 file changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/disas.c b/disas.c
>> index 71007fb..3ffb3ae 100644
>> --- a/disas.c
>> +++ b/disas.c
>> @@ -480,11 +480,19 @@ void monitor_disas(Monitor *mon, CPUArchState *env,
>> #endif
>>
>> for(i = 0; i < nb_insn; i++) {
>> - monitor_printf(mon, "0x" TARGET_FMT_lx ": ", pc);
>> + const char *sym = lookup_symbol(pc);
>> +
>> + monitor_printf(mon, "0x" TARGET_FMT_lx " ", pc);
>> + if (sym[0] != '\0') {
>> + monitor_printf(mon, "<%s>: ", sym);
>> + } else {
>> + monitor_printf(mon, ": ");
>> + }
>
> Independent of PMM's comment, I think you meant
>
> + monitor_printf(mon, "0x" TARGET_FMT_lx, pc);
> + if (sym[0] != '\0') {
> + monitor_printf(mon, " <%s>: ", sym);
> + } else {
> + monitor_printf(mon, ": ");
> + }
>
> to keep the output unchanged.
>
> Could you please put the tab fixing into a preceding patch for
> readability and prepend a cover letter?
OK, will do.
--
Fabien Chouteau
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [Qemu-devel] [PATCH 1/2] Improve Monitor disas with symbol lookup
@ 2013-08-19 16:05 ` Fabien Chouteau
0 siblings, 0 replies; 20+ messages in thread
From: Fabien Chouteau @ 2013-08-19 16:05 UTC (permalink / raw)
To: Andreas Färber; +Cc: qemu-trivial, Peter Maydell, qemu-devel, lcapitulino
On 08/19/2013 04:14 PM, Andreas Färber wrote:
> Am 02.08.2013 14:48, schrieb Fabien Chouteau:
>> Part of M731-018.
>>
>> Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
>> ---
>> disas.c | 16 ++++++++++++----
>> 1 file changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/disas.c b/disas.c
>> index 71007fb..3ffb3ae 100644
>> --- a/disas.c
>> +++ b/disas.c
>> @@ -480,11 +480,19 @@ void monitor_disas(Monitor *mon, CPUArchState *env,
>> #endif
>>
>> for(i = 0; i < nb_insn; i++) {
>> - monitor_printf(mon, "0x" TARGET_FMT_lx ": ", pc);
>> + const char *sym = lookup_symbol(pc);
>> +
>> + monitor_printf(mon, "0x" TARGET_FMT_lx " ", pc);
>> + if (sym[0] != '\0') {
>> + monitor_printf(mon, "<%s>: ", sym);
>> + } else {
>> + monitor_printf(mon, ": ");
>> + }
>
> Independent of PMM's comment, I think you meant
>
> + monitor_printf(mon, "0x" TARGET_FMT_lx, pc);
> + if (sym[0] != '\0') {
> + monitor_printf(mon, " <%s>: ", sym);
> + } else {
> + monitor_printf(mon, ": ");
> + }
>
> to keep the output unchanged.
>
> Could you please put the tab fixing into a preceding patch for
> readability and prepend a cover letter?
OK, will do.
--
Fabien Chouteau
^ permalink raw reply [flat|nested] 20+ messages in thread