[Qemu-devel] Re: [RESEND][PATCH] gdbstub: Add vCont support

[Jan Kiszka <jan.kiszka@siemens.com>]

Krumme, Chris wrote:
>  
> 
>> -----Original Message-----
>> From: 
>> qemu-devel-bounces+chris.krumme=windriver.com@nongnu.org 
>> [mailto:qemu-devel-bounces+chris.krumme=windriver.com@nongnu.o
>> rg] On Behalf Of Jan Kiszka
>> Sent: Wednesday, January 14, 2009 8:44 AM
>> To: qemu-devel@nongnu.org
>> Subject: [Qemu-devel] [RESEND][PATCH] gdbstub: Add vCont support
>>
>> [ Also available via git://git.kiszka.org/qemu.git queue/gdb ]
>>
>> In order to set the VCPU for the next single-step command, 
>> you need gdb
>> 6.8 or better - and this patch. It enhances the existing support for
>> representing VCPUs as threads to the gdb frontend by introducing the
>> vCont remote gdb command. This is used by gdb to switch the debugging
>> focus for single-stepping multi-threaded targets.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>
>>  gdbstub.c |   56 
>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 files changed, 56 insertions(+), 0 deletions(-)
>>
>> diff --git a/gdbstub.c b/gdbstub.c
>> index 0bcd5d5..1cb20b7 100644
>> --- a/gdbstub.c
>> +++ b/gdbstub.c
>> @@ -1542,6 +1542,62 @@ static int gdb_handle_packet(GDBState 
>> *s, const char *line_buf)
>>              s->signal = 0;
>>          gdb_continue(s);
>>          return RS_IDLE;
>> +    case 'v':
>> +        if (strncmp(p, "Cont", 4) == 0) {
>> +            int res_signal, res_thread;
>> +
>> +            p += 4;
>> +            if (*p == '?') {
>> +                put_packet(s, "vCont;c;C;s;S");
>> +                break;
>> +            }
>> +            res = 0;
>> +            res_signal = 0;
>> +            res_thread = 0;
>> +            while (*p) {
>> +                int action, signal;
>> +
>> +                if (*p++ != ';') {
>> +                    res = 0;
>> +                    break;
>> +                }
>> +                action = *p++;
>> +                signal = 0;
>> +                if (action == 'C' || action == 'S')
>> +                    signal = strtoul(p, (char **)&p, 16);
>> +                else if (action != 'c' && action != 's') {
>> +                    res = 0;
>> +                    break;
>> +                }
>> +                thread = 0;
>> +                if (*p == ':')
>> +                    thread = strtoull(p+1, (char **)&p, 16);
>> +
>> +                action = tolower(action);
>> +                if (res == 0 || (res == 'c' && action == 's')) {
>> +                    res = action;
>> +                    res_signal = signal;
>> +                    res_thread = thread;
>> +                }
>> +            }
>> +            if (res) {
>> +                if (res_thread != -1 && res_thread != 0) {
>> +                    for (env = first_cpu; env != NULL; env = 
>> env->next_cpu)
>> +                        if (env->cpu_index + 1 == res_thread)
>> +                            break;
>> +                    if (env == NULL) {
>> +                        put_packet(s, "E22");
>> +                        break;
>> +                    }
>> +                    s->c_cpu = env;
>> +                }
>> +                if (res == 's')
>> +                    cpu_single_step(s->c_cpu, sstep_flags);
>> +                gdb_continue(s);
> 
> Where did res_signal go?

Good question... Looks like I forgot some 's->signal = res_signal;'
here. Will add this.

>  (btw: some OS use signal 0 along with the
> rest.)

In case of the 'c' command, gdbstub also sets s->signal to 0, so I don't
think this default value can be problematic.

> 
>> +                return RS_IDLE;
>> +            }
> 
> If the command is not vCont do you need to return an error?

True, this screams for 'goto unknown_command;'

Thanks for reviewing!

Jan

-- 
Siemens AG, Corporate Technology, CT SE 26
Corporate Competence Center Embedded Linux