From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:54884) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ry6Eh-0006sC-W8 for qemu-devel@nongnu.org; Thu, 16 Feb 2012 13:39:24 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Ry6Ec-0007g1-RF for qemu-devel@nongnu.org; Thu, 16 Feb 2012 13:39:19 -0500 Received: from relay1.mentorg.com ([192.94.38.131]:47440) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ry6Ec-0007eG-Jg for qemu-devel@nongnu.org; Thu, 16 Feb 2012 13:39:14 -0500 Message-ID: <4F3D4D4D.7050607@codesourcery.com> Date: Thu, 16 Feb 2012 12:39:09 -0600 From: Meador Inge MIME-Version: 1.0 References: <1329324914-12296-1-git-send-email-meadori@codesourcery.com> In-Reply-To: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v1 0/1] Fix GDB semihosting List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: qemu-devel@nongnu.org On 02/15/2012 02:14 PM, Peter Maydell wrote: > Here's tracing when it goes wrong: > gdb_do_syscall: vm_stop(RUN_STATE_DEBUG) > reply=3D'Fread,00000003,04000188,00000200' > gdb_chr_receive bytes 1042 > # got the data back before the state change > Got ACK > dropping char $, vm_stop(RUN_STATE_PAUSED) > # ...so gdb_read_byte drops this $ and sends a spurious state change: > gdb_vm_state_change: to 0 > <5:M><1:4><1:0><1:0><1:0><1:1><1:8><1:8><1:,><1:2><1:0><1:0><1::><1:3><1:= 6><1:3><1:3><1:3><1:9><1:0><1:9><1:3><1:1><1:3><1:3><1:3><1:8><1:3><1:9><1:= 3><1:0><1:3><1:7><1:3><1:9><1:3> > # and we end up ignoring the whole packet > <1:1><1:2>gdb_chr_receive bytes 1041 > # gdb got bored and retransmitted > <1:$><2:M><2:4><2:0><2:0> > # snip again; this time we got it, though. > <2:#><3:1><4:2>command=3D'M4000188,200:3633390931333839303739333432093533= 353238363134310a31363432363633313938093437343432363309313538323438323433370= a313033333230363230320938343431363939333909313135333236333539300a3139393238= 363531323809323836373931363331093138313232363531330a31363530393934353731093= 1343835353131383034093938363437383235370a3231323438393831333809383438393334= 36383309313133313335323334360a313534313431373534300939343331393034393509313= 134353135313232350a33303338373232360938373730363839373209313234353033363432= 310a313339303836353732350939353631333431353809313630383334303633340a3833323= 0373736343509313733313139303935320936353132303335360a3733363733333339093131= 3839393334343609313232303538353437320a3738343137363033093137303134373538383= 309313636333536383131310a39323235383735343209373037323535383235093131353837= 34373636310a31323039333739313734093838383438323333390934343437303231360a353= 437343037333330093138373439363035393609323033373333353334340a31333936333432= 30313309383538383239323934 09313534303834363236370a3139323034383836300932303033393830353139' > # etc Ah, I see. In my current patch a reply to the syscall request can still co= me in while the CPU is in the running state. Thank you very much for the anal= ysis. > I think the right way to deal with both the problem you were seeing > and this related issue is simply not to try to send the syscall > request until we have really stopped the CPU. That is, when not > in CONFIG_USER_ONLY we should send the syscall request from > gdb_vm_state_change(). I agree. I am doing some more testing and will send an official v2 patch later, but just to make sure I am on the right track something like (this worked in the basic testing I have done so far and avoids the pitfall point= ed out above): diff --git a/gdbstub.c b/gdbstub.c index 7d470b6..66c3760 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -347,6 +347,7 @@ static int get_char(GDBState *s) #endif static gdb_syscall_complete_cb gdb_current_syscall_cb; +static char gdb_syscall_buf[256]; static enum { GDB_SYS_UNKNOWN, @@ -2396,7 +2397,12 @@ static void gdb_vm_state_change(void *opaque, int running, RunState state) const char *type; int ret; - if (running || s->state =3D=3D RS_INACTIVE || s->state =3D=3D RS_SYSCA= LL) { + if (running || s->state =3D=3D RS_INACTIVE) { + return; + } + if (s->state =3D=3D RS_SYSCALL) { + put_packet(s, gdb_syscall_buf); + s->state =3D RS_IDLE; return; } switch (state) { @@ -2466,7 +2472,6 @@ send_packet: void gdb_do_syscall(gdb_syscall_complete_cb cb, const char *fmt, ...) { va_list va; - char buf[256]; char *p; target_ulong addr; uint64_t i64; @@ -2480,9 +2485,8 @@ void gdb_do_syscall(gdb_syscall_complete_cb cb, const= char *fmt, ...) #ifndef CONFIG_USER_ONLY vm_stop(RUN_STATE_DEBUG); #endif - s->state =3D RS_IDLE; va_start(va, fmt); - p =3D buf; + p =3D gdb_syscall_buf; *(p++) =3D 'F'; while (*fmt) { if (*fmt =3D=3D '%') { @@ -2490,18 +2494,20 @@ void gdb_do_syscall(gdb_syscall_complete_cb cb, con= st char *fmt, ...) switch (*fmt++) { case 'x': addr =3D va_arg(va, target_ulong); - p +=3D snprintf(p, &buf[sizeof(buf)] - p, TARGET_FMT_lx, a= ddr); + p +=3D snprintf(p, &gdb_syscall_buf[sizeof(gdb_syscall_buf= )] - p, + TARGET_FMT_lx, addr); break; case 'l': if (*(fmt++) !=3D 'x') goto bad_format; i64 =3D va_arg(va, uint64_t); - p +=3D snprintf(p, &buf[sizeof(buf)] - p, "%" PRIx64, i64)= ; + p +=3D snprintf(p, &gdb_syscall_buf[sizeof(gdb_syscall_buf= )] - p, + "%" PRIx64, i64); break; case 's': addr =3D va_arg(va, target_ulong); - p +=3D snprintf(p, &buf[sizeof(buf)] - p, TARGET_FMT_lx "/= %x", - addr, va_arg(va, int)); + p +=3D snprintf(p, &gdb_syscall_buf[sizeof(gdb_syscall_buf= )] - p, + TARGET_FMT_lx "/%x", addr, va_arg(va, int)); break; default: bad_format: @@ -2515,10 +2521,17 @@ void gdb_do_syscall(gdb_syscall_complete_cb cb, con= st char *fmt, ...) } *p =3D 0; va_end(va); - put_packet(s, buf); #ifdef CONFIG_USER_ONLY + s->state =3D RS_IDLE; + put_packet(s, gdb_syscall_buf); gdb_handlesig(s->c_cpu, 0); #else + /* In this case wait to send the syscall packet until notification tha= t + the CPU has stopped. This must be done because if the packet is se= nt + now the reply from the syscall request could be received while the = CPU + is still in the running state, which can cause packets to be droppe= d + and state transition 'T' packets to be sent while the syscall is st= ill + being processed. */ cpu_exit(s->c_cpu); #endif } --=20 Meador Inge CodeSourcery / Mentor Embedded http://www.mentor.com/embedded-software