sim, arch, base: Refactor the base remote GDB class.

Fold the GDBListener class into the main BaseRemoteGDB class, move
around a bunch of functions, convert a lot of internal functions to
be private, move some functions into the .cc, make some functions
non-virtual which didn't really need to be overridden.

Change-Id: Id0832b730b0fdfb2eababa5067e72c66de1c147d
Reviewed-on: https://gem5-review.googlesource.com/7422
Reviewed-by: Jason Lowe-Power <jason@lowepower.com>
Reviewed-by: Gabe Black <gabeblack@google.com>
Maintainer: Gabe Black <gabeblack@google.com>

2 files changed, 507 insertions, 613 deletions

diff --git a/src/base/remote_gdb.cc b/src/base/remote_gdb.cc
index 6ed5957d7..09796f1be 100644
--- a/src/base/remote_gdb.cc
+++ b/src/base/remote_gdb.cc
@@ -154,179 +154,236 @@ static const char GDBBadP = '-';
 
 static const int GDBPacketBufLen = 1024;
 
-#ifndef NDEBUG
 vector<BaseRemoteGDB *> debuggers;
 
-void
-debugger()
+class HardBreakpoint : public PCEvent
 {
-    static int current_debugger = -1;
-    if (current_debugger >= 0 && current_debugger < (int)debuggers.size()) {
-        BaseRemoteGDB *gdb = debuggers[current_debugger];
-        if (!gdb->isattached())
-            gdb->listener->accept();
-        if (gdb->isattached())
-            gdb->trap(SIGILL);
+  private:
+    BaseRemoteGDB *gdb;
+
+  public:
+    int refcount;
+
+  public:
+    HardBreakpoint(BaseRemoteGDB *_gdb, PCEventQueue *q, Addr pc)
+        : PCEvent(q, "HardBreakpoint Event", pc),
+          gdb(_gdb), refcount(0)
+    {
+        DPRINTF(GDBMisc, "creating hardware breakpoint at %#x\n", evpc);
     }
-}
-#endif
 
-///////////////////////////////////////////////////////////
-//
-//
-//
+    const std::string name() const { return gdb->name() + ".hwbkpt"; }
 
-GDBListener::InputEvent::InputEvent(GDBListener *l, int fd, int e)
-    : PollEvent(fd, e), listener(l)
-{}
+    void
+    process(ThreadContext *tc) override
+    {
+        DPRINTF(GDBMisc, "handling hardware breakpoint at %#x\n", pc());
 
-void
-GDBListener::InputEvent::process(int revent)
+        if (tc == gdb->tc)
+            gdb->trap(SIGTRAP);
+    }
+};
+
+namespace {
+
+// Exception to throw when the connection to the client is broken.
+struct BadClient
 {
-    listener->accept();
-}
+    const char *warning;
+    BadClient(const char *_warning=NULL) : warning(_warning)
+    {}
+};
 
-GDBListener::GDBListener(BaseRemoteGDB *g, int p)
-    : inputEvent(NULL), gdb(g), port(p)
+// Exception to throw when an error needs to be reported to the client.
+struct CmdError
 {
-    assert(!gdb->listener);
-    gdb->listener = this;
-}
+    string error;
+    CmdError(std::string _error) : error(_error)
+    {}
+};
+
+// Exception to throw when something isn't supported.
+class Unsupported {};
 
-GDBListener::~GDBListener()
+// Convert a hex digit into an integer.
+// This returns -1 if the argument passed is no valid hex digit.
+int
+digit2i(char c)
 {
-    delete inputEvent;
+    if (c >= '0' && c <= '9')
+        return (c - '0');
+    else if (c >= 'a' && c <= 'f')
+        return (c - 'a' + 10);
+    else if (c >= 'A' && c <= 'F')
+        return (c - 'A' + 10);
+    else
+        return (-1);
 }
 
-string
-GDBListener::name()
+// Convert the low 4 bits of an integer into an hex digit.
+char
+i2digit(int n)
 {
-    return gdb->name() + ".listener";
+    return ("0123456789abcdef"[n & 0x0f]);
 }
 
+// Convert a byte array into an hex string.
 void
-GDBListener::listen()
+mem2hex(char *vdst, const char *vsrc, int len)
 {
-    if (ListenSocket::allDisabled()) {
-        warn_once("Sockets disabled, not accepting gdb connections");
-        return;
-    }
+    char *dst = vdst;
+    const char *src = vsrc;
 
-    while (!listener.listen(port, true)) {
-        DPRINTF(GDBMisc, "Can't bind port %d\n", port);
-        port++;
+    while (len--) {
+        *dst++ = i2digit(*src >> 4);
+        *dst++ = i2digit(*src++);
     }
-
-    inputEvent = new InputEvent(this, listener.getfd(), POLLIN);
-    pollQueue.schedule(inputEvent);
-
-#ifndef NDEBUG
-    gdb->number = debuggers.size();
-    debuggers.push_back(gdb);
-#endif
-
-#ifndef NDEBUG
-    ccprintf(cerr, "%d: %s: listening for remote gdb #%d on port %d\n",
-             curTick(), name(), gdb->number, port);
-#else
-    ccprintf(cerr, "%d: %s: listening for remote gdb on port %d\n",
-             curTick(), name(), port);
-#endif
+    *dst = '\0';
 }
 
-void
-GDBListener::accept()
+// Convert an hex string into a byte array.
+// This returns a pointer to the character following the last valid
+// hex digit. If the string ends in the middle of a byte, NULL is
+// returned.
+const char *
+hex2mem(char *vdst, const char *src, int maxlen)
 {
-    if (!listener.islistening())
-        panic("GDBListener::accept(): cannot accept if we're not listening!");
-
-    int sfd = listener.accept(true);
+    char *dst = vdst;
+    int msb, lsb;
 
-    if (sfd != -1) {
-        if (gdb->isattached())
-            close(sfd);
-        else
-            gdb->attach(sfd);
+    while (*src && maxlen--) {
+        msb = digit2i(*src++);
+        if (msb < 0)
+            return (src - 1);
+        lsb = digit2i(*src++);
+        if (lsb < 0)
+            return (NULL);
+        *dst++ = (msb << 4) | lsb;
     }
+    return src;
 }
 
-int
-GDBListener::getPort() const
+// Convert an hex string into an integer.
+// This returns a pointer to the character following the last valid
+// hex digit.
+Addr
+hex2i(const char **srcp)
 {
-    panic_if(!listener.islistening(),
-             "Remote GDB port is unknown until GDBListener::listen() has "
-             "been called.\n");
+    const char *src = *srcp;
+    Addr r = 0;
+    int nibble;
 
-    return port;
+    while ((nibble = digit2i(*src)) >= 0) {
+        r *= 16;
+        r += nibble;
+        src++;
+    }
+    *srcp = src;
+    return r;
 }
 
-BaseRemoteGDB::InputEvent::InputEvent(BaseRemoteGDB *g, int fd, int e)
-    : PollEvent(fd, e), gdb(g)
-{}
+enum GdbBreakpointType {
+    GdbSoftBp = '0',
+    GdbHardBp = '1',
+    GdbWriteWp = '2',
+    GdbReadWp = '3',
+    GdbAccWp = '4',
+};
 
-void
-BaseRemoteGDB::InputEvent::process(int revent)
+const char *
+break_type(char c)
 {
-    if (gdb->trapEvent.scheduled()) {
-        warn("GDB trap event has already been scheduled! "
-             "Ignoring this input event.");
-        return;
-    }
-
-    if (revent & POLLIN) {
-        gdb->trapEvent.type(SIGILL);
-        gdb->scheduleInstCommitEvent(&gdb->trapEvent, 0);
-    } else if (revent & POLLNVAL) {
-        gdb->descheduleInstCommitEvent(&gdb->trapEvent);
-        gdb->detach();
+    switch(c) {
+      case GdbSoftBp: return "software breakpoint";
+      case GdbHardBp: return "hardware breakpoint";
+      case GdbWriteWp: return "write watchpoint";
+      case GdbReadWp: return "read watchpoint";
+      case GdbAccWp: return "access watchpoint";
+      default: return "unknown breakpoint/watchpoint";
     }
 }
 
-void
-BaseRemoteGDB::TrapEvent::process()
+std::map<Addr, HardBreakpoint *> hardBreakMap;
+
+EventQueue *
+getComInstEventQueue(ThreadContext *tc)
 {
-    gdb->trap(_type);
+    return tc->getCpuPtr()->comInstEventQueue[tc->threadId()];
 }
 
-void
-BaseRemoteGDB::processSingleStepEvent()
-{
-    if (!singleStepEvent.scheduled())
-        scheduleInstCommitEvent(&singleStepEvent, 1);
-    trap(SIGTRAP);
 }
 
-BaseRemoteGDB::BaseRemoteGDB(System *_system, ThreadContext *c) :
-        inputEvent(NULL), trapEvent(this), listener(NULL), number(-1),
-        fd(-1), active(false), attached(false), system(_system),
-        context(c),
-        singleStepEvent([this]{ processSingleStepEvent(); }, name())
+BaseRemoteGDB::BaseRemoteGDB(System *_system, ThreadContext *c, int _port) :
+        connectEvent(nullptr), dataEvent(nullptr), _port(_port), fd(-1),
+        active(false), attached(false), sys(_system), tc(c),
+        trapEvent(this), singleStepEvent(*this)
 {
+    debuggers.push_back(this);
 }
 
 BaseRemoteGDB::~BaseRemoteGDB()
 {
-    if (inputEvent)
-        delete inputEvent;
+    delete connectEvent;
+    delete dataEvent;
 }
 
 string
 BaseRemoteGDB::name()
 {
-    return system->name() + ".remote_gdb";
+    return sys->name() + ".remote_gdb";
 }
 
-bool
-BaseRemoteGDB::isattached()
-{ return attached; }
+void
+BaseRemoteGDB::listen()
+{
+    if (ListenSocket::allDisabled()) {
+        warn_once("Sockets disabled, not accepting gdb connections");
+        return;
+    }
+
+    while (!listener.listen(_port, true)) {
+        DPRINTF(GDBMisc, "Can't bind port %d\n", _port);
+        _port++;
+    }
+
+    connectEvent = new ConnectEvent(this, listener.getfd(), POLLIN);
+    pollQueue.schedule(connectEvent);
+
+    ccprintf(cerr, "%d: %s: listening for remote gdb on port %d\n",
+             curTick(), name(), _port);
+}
+
+void
+BaseRemoteGDB::connect()
+{
+    panic_if(!listener.islistening(),
+             "Cannot accept GDB connections if we're not listening!");
+
+    int sfd = listener.accept(true);
+
+    if (sfd != -1) {
+        if (isAttached())
+            close(sfd);
+        else
+            attach(sfd);
+    }
+}
+
+int
+BaseRemoteGDB::port() const
+{
+    panic_if(!listener.islistening(),
+             "Remote GDB port is unknown until listen() has been called.\n");
+    return _port;
+}
 
 void
 BaseRemoteGDB::attach(int f)
 {
     fd = f;
 
-    inputEvent = new InputEvent(this, fd, POLLIN);
-    pollQueue.schedule(inputEvent);
+    dataEvent = new DataEvent(this, fd, POLLIN);
+    pollQueue.schedule(dataEvent);
 
     attached = true;
     DPRINTFN("remote gdb attached\n");
@@ -341,13 +398,106 @@ BaseRemoteGDB::detach()
     close(fd);
     fd = -1;
 
-    pollQueue.remove(inputEvent);
+    pollQueue.remove(dataEvent);
     DPRINTFN("remote gdb detached\n");
 }
 
-/////////////////////////
-//
-//
+// This function does all command processing for interfacing to a
+// remote gdb.  Note that the error codes are ignored by gdb at
+// present, but might eventually become meaningful. (XXX) It might
+// makes sense to use POSIX errno values, because that is what the
+// gdb/remote.c functions want to return.
+bool
+BaseRemoteGDB::trap(int type)
+{
+
+    if (!attached)
+        return false;
+
+    DPRINTF(GDBMisc, "trap: PC=%s\n", tc->pcState());
+
+    clearSingleStep();
+
+    /*
+     * The first entry to this function is normally through
+     * a breakpoint trap in kgdb_connect(), in which case we
+     * must advance past the breakpoint because gdb will not.
+     *
+     * On the first entry here, we expect that gdb is not yet
+     * listening to us, so just enter the interaction loop.
+     * After the debugger is "active" (connected) it will be
+     * waiting for a "signaled" message from us.
+     */
+    if (!active) {
+        active = true;
+    } else {
+        // Tell remote host that an exception has occurred.
+        send(csprintf("S%02x", type).c_str());
+    }
+
+    // Stick frame regs into our reg cache.
+    regCachePtr = gdbRegs();
+    regCachePtr->getRegs(tc);
+
+    char data[GDBPacketBufLen + 1];
+    GdbCommand::Context cmdCtx;
+    cmdCtx.type = type;
+    cmdCtx.data = &data[1];
+
+    for (;;) {
+        try {
+            size_t datalen = recv(data, sizeof(data));
+            if (datalen < 1)
+                throw BadClient();
+
+            data[datalen] = 0; // Sentinel
+            cmdCtx.cmd_byte = data[0];
+            cmdCtx.len = datalen - 1;
+
+            auto cmdIt = command_map.find(cmdCtx.cmd_byte);
+            if (cmdIt == command_map.end()) {
+                DPRINTF(GDBMisc, "Unknown command: %c(%#x)\n",
+                        cmdCtx.cmd_byte, cmdCtx.cmd_byte);
+                throw Unsupported();
+            }
+            cmdCtx.cmd = &(cmdIt->second);
+
+            if (!(this->*(cmdCtx.cmd->func))(cmdCtx))
+                break;
+
+        } catch (BadClient &e) {
+            if (e.warning)
+                warn(e.warning);
+            detach();
+            break;
+        } catch (Unsupported &e) {
+            send("");
+        } catch (CmdError &e) {
+            send(e.error.c_str());
+        } catch (...) {
+            panic("Unrecognzied GDB exception.");
+        }
+    }
+
+    return true;
+}
+
+void
+BaseRemoteGDB::incomingData(int revent)
+{
+    if (trapEvent.scheduled()) {
+        warn("GDB trap event has already been scheduled!");
+        return;
+    }
+
+    if (revent & POLLIN) {
+        trapEvent.type(SIGILL);
+        scheduleInstCommitEvent(&trapEvent, 0);
+    } else if (revent & POLLNVAL) {
+        descheduleInstCommitEvent(&trapEvent);
+        detach();
+    }
+}
 
 uint8_t
 BaseRemoteGDB::getbyte()
@@ -368,35 +518,6 @@ BaseRemoteGDB::putbyte(uint8_t b)
     throw BadClient("Couldn't write data to the debugger.");
 }
 
-// Send a packet to gdb
-void
-BaseRemoteGDB::send(const char *bp)
-{
-    const char *p;
-    uint8_t csum, c;
-
-    DPRINTF(GDBSend, "send:  %s\n", bp);
-
-    do {
-        p = bp;
-        // Start sending a packet
-        putbyte(GDBStart);
-        // Send the contents, and also keep a check sum.
-        for (csum = 0; (c = *p); p++) {
-            putbyte(c);
-            csum += c;
-        }
-        // Send the ending character.
-        putbyte(GDBEnd);
-        // Send the checksum.
-        putbyte(i2digit(csum >> 4));
-        putbyte(i2digit(csum));
-        // Try transmitting over and over again until the other end doesn't
-        // send an error back.
-        c = getbyte();
-    } while ((c & 0x7f) == GDBBadP);
-}
-
 // Receive a packet from gdb
 int
 BaseRemoteGDB::recv(char *bp, int maxlen)
@@ -460,6 +581,35 @@ BaseRemoteGDB::recv(char *bp, int maxlen)
     return len;
 }
 
+// Send a packet to gdb
+void
+BaseRemoteGDB::send(const char *bp)
+{
+    const char *p;
+    uint8_t csum, c;
+
+    DPRINTF(GDBSend, "send:  %s\n", bp);
+
+    do {
+        p = bp;
+        // Start sending a packet
+        putbyte(GDBStart);
+        // Send the contents, and also keep a check sum.
+        for (csum = 0; (c = *p); p++) {
+            putbyte(c);
+            csum += c;
+        }
+        // Send the ending character.
+        putbyte(GDBEnd);
+        // Send the checksum.
+        putbyte(i2digit(csum >> 4));
+        putbyte(i2digit(csum));
+        // Try transmitting over and over again until the other end doesn't
+        // send an error back.
+        c = getbyte();
+    } while ((c & 0x7f) == GDBBadP);
+}
+
 // Read bytes from kernel address space for debugger.
 bool
 BaseRemoteGDB::read(Addr vaddr, size_t size, char *data)
@@ -475,10 +625,10 @@ BaseRemoteGDB::read(Addr vaddr, size_t size, char *data)
     DPRINTF(GDBRead, "read:  addr=%#x, size=%d", vaddr, size);
 
     if (FullSystem) {
-        FSTranslatingPortProxy &proxy = context->getVirtProxy();
+        FSTranslatingPortProxy &proxy = tc->getVirtProxy();
         proxy.readBlob(vaddr, (uint8_t*)data, size);
     } else {
-        SETranslatingPortProxy &proxy = context->getMemProxy();
+        SETranslatingPortProxy &proxy = tc->getMemProxy();
         proxy.readBlob(vaddr, (uint8_t*)data, size);
     }
 
@@ -518,10 +668,10 @@ BaseRemoteGDB::write(Addr vaddr, size_t size, const char *data)
             DPRINTFNR("\n");
     }
     if (FullSystem) {
-        FSTranslatingPortProxy &proxy = context->getVirtProxy();
+        FSTranslatingPortProxy &proxy = tc->getVirtProxy();
         proxy.writeBlob(vaddr, (uint8_t*)data, size);
     } else {
-        SETranslatingPortProxy &proxy = context->getMemProxy();
+        SETranslatingPortProxy &proxy = tc->getMemProxy();
         proxy.writeBlob(vaddr, (uint8_t*)data, size);
     }
 
@@ -529,66 +679,24 @@ BaseRemoteGDB::write(Addr vaddr, size_t size, const char *data)
 }
 
 void
-BaseRemoteGDB::clearSingleStep()
-{
-    descheduleInstCommitEvent(&singleStepEvent);
-}
-
-void
-BaseRemoteGDB::setSingleStep()
+BaseRemoteGDB::singleStep()
 {
     if (!singleStepEvent.scheduled())
         scheduleInstCommitEvent(&singleStepEvent, 1);
-}
-
-PCEventQueue *BaseRemoteGDB::getPcEventQueue()
-{
-    return &system->pcEventQueue;
-}
-
-EventQueue *
-BaseRemoteGDB::getComInstEventQueue()
-{
-    BaseCPU *cpu = context->getCpuPtr();
-    return cpu->comInstEventQueue[context->threadId()];
-}
-
-void
-BaseRemoteGDB::scheduleInstCommitEvent(Event *ev, int delta)
-{
-    EventQueue *eq = getComInstEventQueue();
-    // Here "ticks" aren't simulator ticks which measure time, they're
-    // instructions committed by the CPU.
-    eq->schedule(ev, eq->getCurTick() + delta);
+    trap(SIGTRAP);
 }
 
 void
-BaseRemoteGDB::descheduleInstCommitEvent(Event *ev)
-{
-    if (ev->scheduled())
-        getComInstEventQueue()->deschedule(ev);
-}
-
-bool
-BaseRemoteGDB::checkBpLen(size_t len)
-{
-    return len == sizeof(MachInst);
-}
-
-BaseRemoteGDB::HardBreakpoint::HardBreakpoint(BaseRemoteGDB *_gdb, Addr pc)
-    : PCEvent(_gdb->getPcEventQueue(), "HardBreakpoint Event", pc),
-      gdb(_gdb), refcount(0)
+BaseRemoteGDB::clearSingleStep()
 {
-    DPRINTF(GDBMisc, "creating hardware breakpoint at %#x\n", evpc);
+    descheduleInstCommitEvent(&singleStepEvent);
 }
 
 void
-BaseRemoteGDB::HardBreakpoint::process(ThreadContext *tc)
+BaseRemoteGDB::setSingleStep()
 {
-    DPRINTF(GDBMisc, "handling hardware breakpoint at %#x\n", pc());
-
-    if (tc == gdb->context)
-        gdb->trap(SIGTRAP);
+    if (!singleStepEvent.scheduled())
+        scheduleInstCommitEvent(&singleStepEvent, 1);
 }
 
 void
@@ -619,7 +727,7 @@ BaseRemoteGDB::insertHardBreak(Addr addr, size_t len)
 
     HardBreakpoint *&bkpt = hardBreakMap[addr];
     if (bkpt == 0)
-        bkpt = new HardBreakpoint(this, addr);
+        bkpt = new HardBreakpoint(this, &sys->pcEventQueue, addr);
 
     bkpt->refcount++;
 }
@@ -632,7 +740,7 @@ BaseRemoteGDB::removeHardBreak(Addr addr, size_t len)
 
     DPRINTF(GDBMisc, "Removing hardware breakpoint at %#x\n", addr);
 
-    break_iter_t i = hardBreakMap.find(addr);
+    auto i = hardBreakMap.find(addr);
     if (i == hardBreakMap.end())
         throw CmdError("E0C");
 
@@ -644,42 +752,37 @@ BaseRemoteGDB::removeHardBreak(Addr addr, size_t len)
 }
 
 void
-BaseRemoteGDB::setTempBreakpoint(Addr bkpt)
+BaseRemoteGDB::clearTempBreakpoint(Addr &bkpt)
 {
     DPRINTF(GDBMisc, "setTempBreakpoint: addr=%#x\n", bkpt);
-    insertHardBreak(bkpt, sizeof(TheISA::MachInst));
+    removeHardBreak(bkpt, sizeof(TheISA::MachInst));
+    bkpt = 0;
 }
 
 void
-BaseRemoteGDB::clearTempBreakpoint(Addr &bkpt)
+BaseRemoteGDB::setTempBreakpoint(Addr bkpt)
 {
     DPRINTF(GDBMisc, "setTempBreakpoint: addr=%#x\n", bkpt);
-    removeHardBreak(bkpt, sizeof(TheISA::MachInst));
-    bkpt = 0;
+    insertHardBreak(bkpt, sizeof(TheISA::MachInst));
 }
 
-enum GdbBreakpointType {
-    GdbSoftBp = '0',
-    GdbHardBp = '1',
-    GdbWriteWp = '2',
-    GdbReadWp = '3',
-    GdbAccWp = '4',
-};
+void
+BaseRemoteGDB::scheduleInstCommitEvent(Event *ev, int delta)
+{
+    EventQueue *eq = getComInstEventQueue(tc);
+    // Here "ticks" aren't simulator ticks which measure time, they're
+    // instructions committed by the CPU.
+    eq->schedule(ev, eq->getCurTick() + delta);
+}
 
-const char *
-BaseRemoteGDB::break_type(char c)
+void
+BaseRemoteGDB::descheduleInstCommitEvent(Event *ev)
 {
-    switch(c) {
-      case GdbSoftBp: return "software breakpoint";
-      case GdbHardBp: return "hardware breakpoint";
-      case GdbWriteWp: return "write watchpoint";
-      case GdbReadWp: return "read watchpoint";
-      case GdbAccWp: return "access watchpoint";
-      default: return "unknown breakpoint/watchpoint";
-    }
+    if (ev->scheduled())
+        getComInstEventQueue(tc)->deschedule(ev);
 }
 
-std::map<char, GdbCommand> BaseRemoteGDB::command_map = {
+std::map<char, BaseRemoteGDB::GdbCommand> BaseRemoteGDB::command_map = {
     // last signal
     { '?', { "KGDB_SIGNAL", &BaseRemoteGDB::cmd_signal } },
     // set baud (deprecated)
@@ -736,6 +839,11 @@ std::map<char, GdbCommand> BaseRemoteGDB::command_map = {
     { 'Z', { "KGDB_SET_HW_BKPT", &BaseRemoteGDB::cmd_set_hw_bkpt } },
 };
 
+bool
+BaseRemoteGDB::checkBpLen(size_t len)
+{
+    return len == sizeof(MachInst);
+}
 
 bool
 BaseRemoteGDB::cmd_unsupported(GdbCommand::Context &ctx)
@@ -759,7 +867,7 @@ BaseRemoteGDB::cmd_cont(GdbCommand::Context &ctx)
     const char *p = ctx.data;
     if (ctx.len) {
         Addr newPc = hex2i(&p);
-        context->pcState(newPc);
+        tc->pcState(newPc);
     }
     clearSingleStep();
     return false;
@@ -772,7 +880,7 @@ BaseRemoteGDB::cmd_async_cont(GdbCommand::Context &ctx)
     hex2i(&p);
     if (*p++ == ';') {
         Addr newPc = hex2i(&p);
-        context->pcState(newPc);
+        tc->pcState(newPc);
     }
     clearSingleStep();
     return false;
@@ -803,7 +911,7 @@ BaseRemoteGDB::cmd_reg_w(GdbCommand::Context &ctx)
     if (p == NULL || *p != '\0')
         throw CmdError("E01");
 
-    regCachePtr->setRegs(context);
+    regCachePtr->setRegs(tc);
     send("OK");
 
     return true;
@@ -883,7 +991,7 @@ BaseRemoteGDB::cmd_async_step(GdbCommand::Context &ctx)
     hex2i(&p); // Ignore the subcommand byte.
     if (*p++ == ';') {
         Addr newPc = hex2i(&p);
-        context->pcState(newPc);
+        tc->pcState(newPc);
     }
     setSingleStep();
     return false;
@@ -895,7 +1003,7 @@ BaseRemoteGDB::cmd_step(GdbCommand::Context &ctx)
     if (ctx.len) {
         const char *p = ctx.data;
         Addr newPc = hex2i(&p);
-        context->pcState(newPc);
+        tc->pcState(newPc);
     }
     setSingleStep();
     return false;
@@ -966,161 +1074,3 @@ BaseRemoteGDB::cmd_set_hw_bkpt(GdbCommand::Context &ctx)
 
     return true;
 }
-
-
-// This function does all command processing for interfacing to a
-// remote gdb.  Note that the error codes are ignored by gdb at
-// present, but might eventually become meaningful. (XXX) It might
-// makes sense to use POSIX errno values, because that is what the
-// gdb/remote.c functions want to return.
-bool
-BaseRemoteGDB::trap(int type)
-{
-
-    if (!attached)
-        return false;
-
-    DPRINTF(GDBMisc, "trap: PC=%s\n", context->pcState());
-
-    clearSingleStep();
-
-    /*
-     * The first entry to this function is normally through
-     * a breakpoint trap in kgdb_connect(), in which case we
-     * must advance past the breakpoint because gdb will not.
-     *
-     * On the first entry here, we expect that gdb is not yet
-     * listening to us, so just enter the interaction loop.
-     * After the debugger is "active" (connected) it will be
-     * waiting for a "signaled" message from us.
-     */
-    if (!active) {
-        active = true;
-    } else {
-        // Tell remote host that an exception has occurred.
-        send(csprintf("S%02x", type).c_str());
-    }
-
-    // Stick frame regs into our reg cache.
-    regCachePtr = gdbRegs();
-    regCachePtr->getRegs(context);
-
-    char data[GDBPacketBufLen + 1];
-    GdbCommand::Context cmdCtx;
-    cmdCtx.type = type;
-    cmdCtx.data = &data[1];
-
-    for (;;) {
-        try {
-            size_t datalen = recv(data, sizeof(data));
-            if (datalen < 1)
-                throw BadClient();
-
-            data[datalen] = 0; // Sentinel
-            cmdCtx.cmd_byte = data[0];
-            cmdCtx.len = datalen - 1;
-
-            auto cmdIt = command_map.find(cmdCtx.cmd_byte);
-            if (cmdIt == command_map.end()) {
-                DPRINTF(GDBMisc, "Unknown command: %c(%#x)\n",
-                        cmdCtx.cmd_byte, cmdCtx.cmd_byte);
-                throw Unsupported();
-            }
-            cmdCtx.cmd = &(cmdIt->second);
-
-            if (!(this->*(cmdCtx.cmd->func))(cmdCtx))
-                break;
-
-        } catch (BadClient &e) {
-            if (e.warning)
-                warn(e.warning);
-            detach();
-            break;
-        } catch (Unsupported &e) {
-            send("");
-        } catch (CmdError &e) {
-            send(e.error.c_str());
-        } catch (...) {
-            panic("Unrecognzied GDB exception.");
-        }
-    }
-
-    return true;
-}
-
-// Convert a hex digit into an integer.
-// This returns -1 if the argument passed is no valid hex digit.
-int
-BaseRemoteGDB::digit2i(char c)
-{
-    if (c >= '0' && c <= '9')
-        return (c - '0');
-    else if (c >= 'a' && c <= 'f')
-        return (c - 'a' + 10);
-    else if (c >= 'A' && c <= 'F')
-        return (c - 'A' + 10);
-    else
-        return (-1);
-}
-
-// Convert the low 4 bits of an integer into an hex digit.
-char
-BaseRemoteGDB::i2digit(int n)
-{
-    return ("0123456789abcdef"[n & 0x0f]);
-}
-
-// Convert a byte array into an hex string.
-void
-BaseRemoteGDB::mem2hex(char *vdst, const char *vsrc, int len)
-{
-    char *dst = vdst;
-    const char *src = vsrc;
-
-    while (len--) {
-        *dst++ = i2digit(*src >> 4);
-        *dst++ = i2digit(*src++);
-    }
-    *dst = '\0';
-}
-
-// Convert an hex string into a byte array.
-// This returns a pointer to the character following the last valid
-// hex digit. If the string ends in the middle of a byte, NULL is
-// returned.
-const char *
-BaseRemoteGDB::hex2mem(char *vdst, const char *src, int maxlen)
-{
-    char *dst = vdst;
-    int msb, lsb;
-
-    while (*src && maxlen--) {
-        msb = digit2i(*src++);
-        if (msb < 0)
-            return (src - 1);
-        lsb = digit2i(*src++);
-        if (lsb < 0)
-            return (NULL);
-        *dst++ = (msb << 4) | lsb;
-    }
-    return src;
-}
-
-// Convert an hex string into an integer.
-// This returns a pointer to the character following the last valid
-// hex digit.
-Addr
-BaseRemoteGDB::hex2i(const char **srcp)
-{
-    const char *src = *srcp;
-    Addr r = 0;
-    int nibble;
-
-    while ((nibble = digit2i(*src)) >= 0) {
-        r *= 16;
-        r += nibble;
-        src++;
-    }
-    *srcp = src;
-    return r;
-}
diff --git a/src/base/remote_gdb.hh b/src/base/remote_gdb.hh
index 121faaf2e..27b4ab488 100644
--- a/src/base/remote_gdb.hh
+++ b/src/base/remote_gdb.hh
@@ -49,265 +49,239 @@
 class System;
 class ThreadContext;
 
-class GDBListener;
-
 class BaseRemoteGDB;
-
-struct GdbCommand
+class HardBreakpoint;
+
+/**
+ * Concrete subclasses of this abstract class represent how the
+ * register values are transmitted on the wire.  Usually each
+ * architecture should define one subclass, but there can be more
+ * if there is more than one possible wire format.  For example,
+ * ARM defines both AArch32GdbRegCache and AArch64GdbRegCache.
+ */
+class BaseGdbRegCache
 {
   public:
-    struct Context
-    {
-        const GdbCommand *cmd;
-        char cmd_byte;
-        int type;
-        char *data;
-        int len;
-    };
 
-    typedef bool (BaseRemoteGDB::*Func)(Context &ctx);
+    /**
+     * Return the pointer to the raw bytes buffer containing the
+     * register values.  Each byte of this buffer is literally
+     * encoded as two hex digits in the g or G RSP packet.
+     */
+    virtual char *data() const = 0;
+
+    /**
+     * Return the size of the raw buffer, in bytes
+     * (i.e., half of the number of digits in the g/G packet).
+     */
+    virtual size_t size() const = 0;
+
+    /**
+     * Fill the raw buffer from the registers in the ThreadContext.
+     */
+    virtual void getRegs(ThreadContext*) = 0;
+
+    /**
+     * Set the ThreadContext's registers from the values
+     * in the raw buffer.
+     */
+    virtual void setRegs(ThreadContext*) const = 0;
 
-    const char * const name;
-    const Func func;
+    /**
+     * Return the name to use in places like DPRINTF.
+     * Having each concrete superclass redefine this member
+     * is useful in situations where the class of the regCache
+     * can change on the fly.
+     */
+    virtual const std::string name() const = 0;
 
-    GdbCommand(const char *_name, Func _func) : name(_name), func(_func)
+    BaseGdbRegCache(BaseRemoteGDB *g) : gdb(g)
     {}
+    virtual ~BaseGdbRegCache()
+    {}
+
+  protected:
+    BaseRemoteGDB *gdb;
 };
 
 class BaseRemoteGDB
 {
-  private:
-    friend void debugger();
-    friend class GDBListener;
+    friend class HardBreakpoint;
+  public:
 
-  protected:
-    /// Exception to throw when the connection to the client is broken.
-    struct BadClient
-    {
-        const char *warning;
-        BadClient(const char *_warning=NULL) : warning(_warning)
-        {}
-    };
-    /// Exception to throw when an error needs to be reported to the client.
-    struct CmdError
-    {
-        std::string error;
-        CmdError(std::string _error) : error(_error)
-        {}
-    };
-    /// Exception to throw when something isn't supported.
-    class Unsupported {};
+    /*
+     * Interface to other parts of the simulator.
+     */
+    BaseRemoteGDB(System *system, ThreadContext *context, int _port);
+    virtual ~BaseRemoteGDB();
 
-    // Helper functions
-  protected:
-    int digit2i(char);
-    char i2digit(int);
-    Addr hex2i(const char **);
-    // Address formats, break types, and gdb commands may change
-    // between architectures, so they're defined as virtual
-    // functions.
-    virtual void mem2hex(char *, const char *, int);
-    virtual const char * hex2mem(char *, const char *, int);
-    virtual const char * break_type(char c);
+    std::string name();
 
-  protected:
-    static std::map<char, GdbCommand> command_map;
+    void listen();
+    void connect();
 
-    bool cmd_unsupported(GdbCommand::Context &ctx);
+    int port() const;
 
-    bool cmd_signal(GdbCommand::Context &ctx);
-    bool cmd_cont(GdbCommand::Context &ctx);
-    bool cmd_async_cont(GdbCommand::Context &ctx);
-    bool cmd_detach(GdbCommand::Context &ctx);
-    bool cmd_reg_r(GdbCommand::Context &ctx);
-    bool cmd_reg_w(GdbCommand::Context &ctx);
-    bool cmd_set_thread(GdbCommand::Context &ctx);
-    bool cmd_mem_r(GdbCommand::Context &ctx);
-    bool cmd_mem_w(GdbCommand::Context &ctx);
-    bool cmd_query_var(GdbCommand::Context &ctx);
-    bool cmd_step(GdbCommand::Context &ctx);
-    bool cmd_async_step(GdbCommand::Context &ctx);
-    bool cmd_clr_hw_bkpt(GdbCommand::Context &ctx);
-    bool cmd_set_hw_bkpt(GdbCommand::Context &ctx);
+    void attach(int fd);
+    void detach();
+    bool isAttached() { return attached; }
 
-  protected:
-    class InputEvent : public PollEvent
-    {
-      protected:
-        BaseRemoteGDB *gdb;
+    void replaceThreadContext(ThreadContext *_tc) { tc = _tc; }
 
-      public:
-        InputEvent(BaseRemoteGDB *g, int fd, int e);
-        void process(int revent);
-    };
+    bool trap(int type);
+    bool breakpoint() { return trap(SIGTRAP); }
 
-    class TrapEvent : public Event
+  private:
+    /*
+     * Connection to the external GDB.
+     */
+    void incomingData(int revent);
+    void connectWrapper(int revent) { connect(); }
+
+    template <void (BaseRemoteGDB::*F)(int revent)>
+    class SocketEvent : public PollEvent
     {
       protected:
-        int _type;
         BaseRemoteGDB *gdb;
 
       public:
-        TrapEvent(BaseRemoteGDB *g) : gdb(g)
+        SocketEvent(BaseRemoteGDB *gdb, int fd, int e) :
+            PollEvent(fd, e), gdb(gdb)
         {}
 
-        void type(int t) { _type = t; }
-        void process();
+        void process(int revent) { (gdb->*F)(revent); }
     };
 
-    friend class InputEvent;
-    InputEvent *inputEvent;
-    TrapEvent trapEvent;
-    GDBListener *listener;
-    int number;
+    typedef SocketEvent<&BaseRemoteGDB::connectWrapper> ConnectEvent;
+    typedef SocketEvent<&BaseRemoteGDB::incomingData> DataEvent;
 
-  protected:
-    // The socket commands come in through
-    int fd;
-
-  protected:
-    bool active;
-    bool attached;
+    friend ConnectEvent;
+    friend DataEvent;
 
-    System *system;
-    ThreadContext *context;
-
-  protected:
-    /**
-     * Concrete subclasses of this abstract class represent how the
-     * register values are transmitted on the wire.  Usually each
-     * architecture should define one subclass, but there can be more
-     * if there is more than one possible wire format.  For example,
-     * ARM defines both AArch32GdbRegCache and AArch64GdbRegCache.
-     */
-    class BaseGdbRegCache
-    {
-      public:
+    ConnectEvent *connectEvent;
+    DataEvent *dataEvent;
 
-        /**
-         * Return the pointer to the raw bytes buffer containing the
-         * register values.  Each byte of this buffer is literally
-         * encoded as two hex digits in the g or G RSP packet.
-         */
-        virtual char *data() const = 0;
-
-        /**
-         * Return the size of the raw buffer, in bytes
-         * (i.e., half of the number of digits in the g/G packet).
-         */
-        virtual size_t size() const = 0;
-
-        /**
-         * Fill the raw buffer from the registers in the ThreadContext.
-         */
-        virtual void getRegs(ThreadContext*) = 0;
-
-        /**
-         * Set the ThreadContext's registers from the values
-         * in the raw buffer.
-         */
-        virtual void setRegs(ThreadContext*) const = 0;
-
-        /**
-         * Return the name to use in places like DPRINTF.
-         * Having each concrete superclass redefine this member
-         * is useful in situations where the class of the regCache
-         * can change on the fly.
-         */
-        virtual const std::string name() const = 0;
-
-        BaseGdbRegCache(BaseRemoteGDB *g) : gdb(g)
-        {}
-        virtual ~BaseGdbRegCache()
-        {}
-
-      protected:
-        BaseRemoteGDB *gdb;
-    };
+    ListenSocket listener;
+    int _port;
 
-    BaseGdbRegCache *regCachePtr;
+    // The socket commands come in through.
+    int fd;
 
-  protected:
+    // Transfer data to/from GDB.
     uint8_t getbyte();
     void putbyte(uint8_t b);
 
     int recv(char *data, int len);
     void send(const char *data);
 
-  protected:
-    // Machine memory
-    virtual bool read(Addr addr, size_t size, char *data);
-    virtual bool write(Addr addr, size_t size, const char *data);
+    /*
+     * Simulator side debugger state.
+     */
+    bool active;
+    bool attached;
 
-    template <class T> T read(Addr addr);
-    template <class T> void write(Addr addr, T data);
+    System *sys;
+    ThreadContext *tc;
 
-  public:
-    BaseRemoteGDB(System *system, ThreadContext *context);
-    virtual ~BaseRemoteGDB();
-    virtual BaseGdbRegCache *gdbRegs() = 0;
+    BaseGdbRegCache *regCachePtr;
 
-    void replaceThreadContext(ThreadContext *tc) { context = tc; }
+    class TrapEvent : public Event
+    {
+      protected:
+        int _type;
+        BaseRemoteGDB *gdb;
 
-    void attach(int fd);
-    void detach();
-    bool isattached();
+      public:
+        TrapEvent(BaseRemoteGDB *g) : gdb(g)
+        {}
 
-    virtual bool acc(Addr addr, size_t len) = 0;
-    bool trap(int type);
-    virtual bool breakpoint()
-    {
-        return trap(SIGTRAP);
-    }
+        void type(int t) { _type = t; }
+        void process() { gdb->trap(_type); }
+    } trapEvent;
 
-    void processSingleStepEvent();
-    EventFunctionWrapper singleStepEvent;
+    /*
+     * The interface to the simulated system.
+     */
+    // Machine memory.
+    bool read(Addr addr, size_t size, char *data);
+    bool write(Addr addr, size_t size, const char *data);
+
+    template <class T> T read(Addr addr);
+    template <class T> void write(Addr addr, T data);
+
+    // Single step.
+    void singleStep();
+    EventWrapper<BaseRemoteGDB, &BaseRemoteGDB::singleStep> singleStepEvent;
 
     void clearSingleStep();
     void setSingleStep();
 
-    PCEventQueue *getPcEventQueue();
-    EventQueue *getComInstEventQueue();
-
     /// Schedule an event which will be triggered "delta" instructions later.
     void scheduleInstCommitEvent(Event *ev, int delta);
     /// Deschedule an instruction count based event.
     void descheduleInstCommitEvent(Event *ev);
 
-  protected:
-    virtual bool checkBpLen(size_t len);
+    // Breakpoints.
+    void insertSoftBreak(Addr addr, size_t len);
+    void removeSoftBreak(Addr addr, size_t len);
+    void insertHardBreak(Addr addr, size_t len);
+    void removeHardBreak(Addr addr, size_t len);
 
-    class HardBreakpoint : public PCEvent
-    {
-      private:
-        BaseRemoteGDB *gdb;
+    void clearTempBreakpoint(Addr &bkpt);
+    void setTempBreakpoint(Addr bkpt);
 
+    /*
+     * GDB commands.
+     */
+    struct GdbCommand
+    {
       public:
-        int refcount;
+        struct Context
+        {
+            const GdbCommand *cmd;
+            char cmd_byte;
+            int type;
+            char *data;
+            int len;
+        };
 
-      public:
-        HardBreakpoint(BaseRemoteGDB *_gdb, Addr addr);
-        const std::string name() const { return gdb->name() + ".hwbkpt"; }
+        typedef bool (BaseRemoteGDB::*Func)(Context &ctx);
 
-        virtual void process(ThreadContext *tc);
+        const char * const name;
+        const Func func;
+
+        GdbCommand(const char *_name, Func _func) : name(_name), func(_func) {}
     };
-    friend class HardBreakpoint;
 
-    typedef std::map<Addr, HardBreakpoint *> break_map_t;
-    typedef break_map_t::iterator break_iter_t;
-    break_map_t hardBreakMap;
+    static std::map<char, GdbCommand> command_map;
 
-    void insertSoftBreak(Addr addr, size_t len);
-    void removeSoftBreak(Addr addr, size_t len);
-    virtual void insertHardBreak(Addr addr, size_t len);
-    void removeHardBreak(Addr addr, size_t len);
+    bool cmd_unsupported(GdbCommand::Context &ctx);
+
+    bool cmd_signal(GdbCommand::Context &ctx);
+    bool cmd_cont(GdbCommand::Context &ctx);
+    bool cmd_async_cont(GdbCommand::Context &ctx);
+    bool cmd_detach(GdbCommand::Context &ctx);
+    bool cmd_reg_r(GdbCommand::Context &ctx);
+    bool cmd_reg_w(GdbCommand::Context &ctx);
+    bool cmd_set_thread(GdbCommand::Context &ctx);
+    bool cmd_mem_r(GdbCommand::Context &ctx);
+    bool cmd_mem_w(GdbCommand::Context &ctx);
+    bool cmd_query_var(GdbCommand::Context &ctx);
+    bool cmd_step(GdbCommand::Context &ctx);
+    bool cmd_async_step(GdbCommand::Context &ctx);
+    bool cmd_clr_hw_bkpt(GdbCommand::Context &ctx);
+    bool cmd_set_hw_bkpt(GdbCommand::Context &ctx);
 
   protected:
-    void clearTempBreakpoint(Addr &bkpt);
-    void setTempBreakpoint(Addr bkpt);
+    ThreadContext *context() { return tc; }
+    System *system() { return sys; }
 
-  public:
-    std::string name();
+    // To be implemented by subclasses.
+    virtual bool checkBpLen(size_t len);
+
+    virtual BaseGdbRegCache *gdbRegs() = 0;
+
+    virtual bool acc(Addr addr, size_t len) = 0;
 };
 
 template <class T>
@@ -322,38 +296,8 @@ BaseRemoteGDB::read(Addr addr)
 template <class T>
 inline void
 BaseRemoteGDB::write(Addr addr, T data)
-{ write(addr, sizeof(T), (const char *)&data); }
-
-class GDBListener
 {
-  protected:
-    class InputEvent : public PollEvent
-    {
-      protected:
-        GDBListener *listener;
-
-      public:
-        InputEvent(GDBListener *l, int fd, int e);
-        void process(int revent);
-    };
-
-    friend class InputEvent;
-    InputEvent *inputEvent;
-
-  protected:
-    ListenSocket listener;
-    BaseRemoteGDB *gdb;
-    int port;
-
-  public:
-    GDBListener(BaseRemoteGDB *g, int p);
-    ~GDBListener();
-
-    void accept();
-    void listen();
-    std::string name();
-
-    int getPort() const;
-};
+    write(addr, sizeof(T), (const char *)&data);
+}
 
 #endif /* __REMOTE_GDB_H__ */