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='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='M4000188,200:3633390931333839303739333432093533353238363134310a31363432363633313938093437343432363309313538323438323433370a313033333230363230320938343431363939333909313135333236333539300a3139393238363531323809323836373931363331093138313232363531330a313635303939343537310931343835353131383034093938363437383235370a323132343839383133380938343839333436383309313133313335323334360a313534313431373534300939343331393034393509313134353135313232350a33303338373232360938373730363839373209313234353033363432310a313339303836353732350939353631333431353809313630383334303633340a38333230373736343509313733313139303935320936353132303335360a37333637333333390931313839393334343609313232303538353437320a3738343137363033093137303134373538383309313636333536383131310a3932323538373534320937303732353538323509313135383734373636310a31323039333739313734093838383438323333390934343437303231360a353437343037333330093138373439363035393609323033373333353334340a3133393633343230313309383538383239323934
09313534303834363236370a3139323034383836300932303033393830353139'
> # etc
Ah, I see. In my current patch a reply to the syscall request can still come
in while the CPU is in the running state. Thank you very much for the analysis.
> 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 pointed
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 == RS_INACTIVE || s->state == RS_SYSCALL) {
+ if (running || s->state == RS_INACTIVE) {
+ return;
+ }
+ if (s->state == RS_SYSCALL) {
+ put_packet(s, gdb_syscall_buf);
+ s->state = 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 = RS_IDLE;
va_start(va, fmt);
- p = buf;
+ p = gdb_syscall_buf;
*(p++) = 'F';
while (*fmt) {
if (*fmt == '%') {
@@ -2490,18 +2494,20 @@ void gdb_do_syscall(gdb_syscall_complete_cb cb, const
char *fmt, ...)
switch (*fmt++) {
case 'x':
addr = va_arg(va, target_ulong);
- p += snprintf(p, &buf[sizeof(buf)] - p, TARGET_FMT_lx, addr);
+ p += snprintf(p, &gdb_syscall_buf[sizeof(gdb_syscall_buf)] - p,
+ TARGET_FMT_lx, addr);
break;
case 'l':
if (*(fmt++) != 'x')
goto bad_format;
i64 = va_arg(va, uint64_t);
- p += snprintf(p, &buf[sizeof(buf)] - p, "%" PRIx64, i64);
+ p += snprintf(p, &gdb_syscall_buf[sizeof(gdb_syscall_buf)] - p,
+ "%" PRIx64, i64);
break;
case 's':
addr = va_arg(va, target_ulong);
- p += snprintf(p, &buf[sizeof(buf)] - p, TARGET_FMT_lx "/%x",
- addr, va_arg(va, int));
+ p += 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, const
char *fmt, ...)
}
*p = 0;
va_end(va);
- put_packet(s, buf);
#ifdef CONFIG_USER_ONLY
+ s->state = 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 that
+ the CPU has stopped. This must be done because if the packet is sent
+ 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 dropped
+ and state transition 'T' packets to be sent while the syscall is still
+ being processed. */
cpu_exit(s->c_cpu);
#endif
}
--
Meador Inge
CodeSourcery / Mentor Embedded
http://www.mentor.com/embedded-software
