Hi - Regarding patch 2, it looks good overall. Comments inline below.
> From 44e1f7b6303886cfdd705a64a9d457cb924c83ee Mon Sep 17 00:00:00 2001 > From: Christopher Koch <[email protected]> > Date: Thu, 9 Jun 2016 11:12:19 -0700 > Subject: Defines protocol between gdbserver and a 2LS. > > The protocol is called D9. > > This will be part of parlib and in its current form can be used with any > 2LS. Also sets up some helpful structures to deal with indexing / listing > threads in parlib. > > The protocol is intended to be asynchronous and looks a lot like 9P (hence, > the name). Some of the issues around async are not solved yet; e.g. the > client side (gdbserver) currently expects all operations to be serialized > without interleaving responses. E.g. it could not currently handle this > sequence of operations: Does the server side currently have similar limitations? > TWAIT > TREADMEM > RWAIT > RREADMEM > > Change-Id: Ib7c8fd800b1550f6b72ce0feae57c7f5dbdf11f6 > Signed-off-by: Christopher Koch <[email protected]> > --- > kern/arch/x86/trap.c | 1 + > user/parlib/debug.c | 470 > ++++++++++++++++++++++++++++++++- > user/parlib/include/parlib/debug.h | 67 +++++ > user/parlib/include/parlib/x86/debug.h | 32 +++ > user/parlib/uthread.c | 8 + > user/parlib/x86/debug.c | 18 ++ > 6 files changed, 594 insertions(+), 2 deletions(-) > create mode 100644 user/parlib/include/parlib/debug.h > create mode 100644 user/parlib/include/parlib/x86/debug.h > create mode 100644 user/parlib/x86/debug.c > > diff --git a/kern/arch/x86/trap.c b/kern/arch/x86/trap.c > index 83ad3532edbf..53615af930d2 100644 > --- a/kern/arch/x86/trap.c > +++ b/kern/arch/x86/trap.c > @@ -549,6 +549,7 @@ static void trap_dispatch(struct hw_trapframe *hw_tf) > > // Handle processor exceptions. > switch(hw_tf->tf_trapno) { > + case T_DEBUG: > case T_BRKPT: > enable_irq(); > monitor(hw_tf); > diff --git a/user/parlib/debug.c b/user/parlib/debug.c > index 46a3e4ed72c1..9f967f09f109 100644 > --- a/user/parlib/debug.c > +++ b/user/parlib/debug.c > @@ -1,9 +1,17 @@ > +#include <fcntl.h> > +#include <parlib/assert.h> > #include <parlib/common.h> > +#include <parlib/debug.h> > +#include <parlib/ros_debug.h> > +#include <parlib/event.h> > #include <parlib/parlib.h> > -#include <stdio.h> > -#include <unistd.h> > #include <parlib/spinlock.h> > #include <ros/common.h> > +#include <stdio.h> > +#include <stdlib.h> > +#include <sys/types.h> > +#include <sys/stat.h> > +#include <unistd.h> > > int akaros_printf(const char *format, ...) > { > @@ -75,3 +83,461 @@ void __print_func_exit(const char *func, const char *file) > printf("---- %s()\n", func); > spinlock_unlock(&lock); > } > + > +static void handle_debug_msg(struct syscall *sysc); > +static void debug_read(int fd, void *buf, size_t len); > +static int handle_one_msg(struct event_mbox *ev_mbox); > +static void debug_read_handler(struct event_queue *ev_q); > +static int debug_send_packet(int fd, void *data, size_t size); > +static int debug_send_error(uint32_t errnum); > +static int debug_send_response(struct d9_header *packet); > +static void *d9_read_packet(struct d9_header *hdr); > +static int d9_treadmem(struct d9_header *hdr, void *data); > +static int d9_tstoremem(struct d9_header *hdr, void *data); > +static int d9_tfetchreg(struct d9_header *hdr, void *data); The definitions for these functions (below) are usually missing the 'static' part. Same for some helpers. > + > +/* ev_q for read syscall on debug pipe. */ > +static struct event_queue *debug_read_ev_q; > +static struct syscall debug_read_sysc; > +static int debug_fd; > + > +/* async_read_buf is the buffer used to read the header of a packet. > + * > + * We can only have one of these reads at a time anyway. */ > +static char async_read_buf[sizeof(struct d9_header)]; > + > +/* d9_ops are user-supplied routines that fill in the response packet with > the > + * appropriate information and/or do the requested operation. These may > block. > + */ > +static struct d9_ops *d9_ops; > + > +/* A message handler is an internal routine that allocates the appropriate > + * response packet and coordinates sending the appropriate response to > + * gdbserver. */ > +typedef int (*message_handler)(struct d9_header *hdr, void *data); > +#define D9_HANDLER(x) (x - D9_TREADMEM) > +static message_handler msg_handlers[D9_HANDLER(D9_NUM_MESSAGES)]; > + > +/* queue_handle_one_message extracts one message from the mbox and calls the > + * appropriate handler for that message. */ > +int queue_handle_one_msg(struct event_mbox *ev_mbox) This func can be static. > +{ > + struct event_msg msg; > + struct syscall *sysc; > + > + if (!extract_one_mbox_msg(ev_mbox, &msg)) > + return 0; > + > + assert(msg.ev_type == EV_SYSCALL); > + sysc = msg.ev_arg3; > + assert(sysc); > + handle_debug_msg(sysc); > + return 1; > +} > + > +/* queue_read_handler is the event queue handler for the async read evq. */ > +void queue_read_handler(struct event_queue *ev_q) This func can be static. > +{ > + assert(ev_q); > + assert(ev_q->ev_mbox); > + while (queue_handle_one_msg(ev_q->ev_mbox)) > + ; > +} > + > +void d9_server_init(struct schedule_ops *ops, struct d9_ops *dops) > +{ > + int fd; > + int p[2]; > + char buf[60]; > + int ret; > + > + /* Set up parlib queue for asynchronous read notifications. */ > + debug_read_ev_q = get_eventq(EV_MBOX_UCQ); > + debug_read_ev_q->ev_flags = EVENT_IPI | EVENT_INDIR | EVENT_SPAM_INDIR | > + EVENT_WAKEUP; > + debug_read_ev_q->ev_handler = queue_read_handler; > + > + /* Set up d9 handlers. */ > + msg_handlers[D9_HANDLER(D9_TREADMEM)] = d9_treadmem; > + msg_handlers[D9_HANDLER(D9_TSTOREMEM)] = d9_tstoremem; > + msg_handlers[D9_HANDLER(D9_TFETCHREG)] = d9_tfetchreg; > + > + /* Set d9 ops. */ > + d9_ops = dops; Are we ever going to change d9_ops at runtime? I noticed down in uthread_2ls_init() that you dynamically set the values for dops, then here we assign it to d9_ops. My guess is this never changes, (i.e. no specific 2LS will override them), so we can just set up d9_ops statically. Also, if these aren't dynamic, then you won't need to do things like this: + if (d9_ops == NULL || d9_ops->store_memory == NULL) + return debug_send_error(EBADF /* TODO: better error code */); Since there will always be a store_memory operation. > + > + /* Open a pipe and post it in #srv. > + * TODO(chrisko): add special file #proc/PID/debug that works like #srv. > + */ When are these srv files removed? After running for a bit, do you have a large number of srv files (one for every process that ever existed)? We probably can deal with this with the #proc file in the future. > + ret = pipe(p); > + if (ret < 0) > + return; > + > + snprintf(buf, sizeof(buf), "#srv/debug-%d", getpid()); > + fd = open(buf, O_WRONLY|O_CREAT, 0666); > + if (fd < 0) > + return; > + > + snprintf(buf, sizeof(buf), "%d", p[1]); > + if (write(fd, buf, strlen(buf)) != strlen(buf)) > + return; For all of these error cases, I'd just assert success / panic on failure. If you just return, we won't notice the bug. Since this is startup code, it should always succeed. > + > + close(p[1]); > + > + debug_fd = p[0]; > + debug_read(debug_fd, async_read_buf, sizeof(struct d9_header)); > + > + asm("int $3"); You can call breakpoint(), which is arch-independent. Though what is the point of this breakpoint? > +} > + > +void debug_read(int fd, void *buf, size_t len) This func can be static. > +{ > + memset(&debug_read_sysc, 0, sizeof(struct syscall)); > + syscall_async(&debug_read_sysc, SYS_read, fd, buf, len); > + > + if (!register_evq(&debug_read_sysc, debug_read_ev_q)) > + handle_debug_msg(&debug_read_sysc); This is correct, and no need to change it now, but for those curious, you can sign up for an event on a syscall before you submit it, and then *only* handle the completion from the event handler. > +} > + > +void handle_debug_msg(struct syscall *sysc) This func can be static. > +{ > + struct d9_header *hdr; > + > + switch (sysc->retval) { > + case 0: > + panic("read syscall: got 0 bytes."); > + break; > + > + case -1: > + panic("read failed"); > + break; > + > + default: > + if (sysc->retval != sizeof(struct d9_header)) > + panic("2LS debug: should have received D9 header."); > + > + /* The buffer used will be reused before we are done handling > this call. > + */ This might have issues. If you delayed debug_read until after you ran the handlers, then you could avoid these mallocs, memcpys, and other stuff. You'd just have the one async_read_buf, which always has your current command. Other than code complexity, there might be problems with having multiple commands operating at the same time. Since you debug_read() before running a handler, there might be a scenario where another command arrives and we run two handlers in parallel (say another vcore handled the message). If you do the debug_read() after handling the message, that will never happen - you enforce the synchronicity. The current gdb setup might not trigger this, but other "2LS-interrogations" in the future might not have the same assumptions. > + hdr = malloc(sizeof(struct d9_header)); > + if (hdr == NULL) > + panic("2LS debug: failed to malloc."); > + > + memcpy(hdr, (void *) sysc->arg1, sizeof(struct d9_header)); > + > + void *data = d9_read_packet(hdr); > + > + /* It is now safe to set off another async read syscall, > reusing the > + * buffer. */ > + debug_read(debug_fd, async_read_buf, sizeof(struct d9_header)); > + > + /* Call the appropriate handler for this packet. */ > + if (msg_handlers[D9_HANDLER(hdr->msg_type)] != NULL) > + msg_handlers[D9_HANDLER(hdr->msg_type)](hdr, data); > + else > + panic("2LS debug: no message handler found."); > + > + if (data) > + free(data); > + free(hdr); > + break; > + } > +} > + > +/* alloc_packet allocates memory for a packet of given type. */ > +struct d9_header *alloc_packet(size_t pck_len, enum d9_msg_t msg_type) Given the type, it seems like you usually know the pck_len. Other than D9_RREADMEM, the other callers are all passing in a static value. Is there a reasonable limit to the size of the readlen? For instance, can we tell GDB that PacketSize=1024 or something? Not a huge deal, but it would avoid any confusion with getting the wrong pck_len and would remove this error check: > +{ > + struct d9_header *rhdr; > + > + if (pck_len >= UINT_MAX - sizeof(struct d9_header)) > + panic("2LS debug: packet too long."); Incidentally, pck_len is a size_t, not a uint. Basically, we're saying you can't ask for more than about 4GB of RAM, which is ridiculously large. > + > + rhdr = calloc(sizeof(struct d9_header) + pck_len, 1); > + if (rhdr == NULL) > + return NULL; > + > + rhdr->msg_type = msg_type; > + rhdr->size = sizeof(struct d9_header) + pck_len; > + return rhdr; > +} > + > +/* d9_read_memory is the d9_ops func called to fulfill a TREADMEM request. */ > +int d9_read_memory(const struct d9_treadmem *req, struct d9_rreadmem *resp) > +{ > + memcpy(resp->data, (void *) req->address, req->length); > + return 0; > +} > + > +/* d9_store_memory is the d9_ops func called to fulfill a TSTOREMEM request. > */ > +int d9_store_memory(const struct d9_tstoremem *req, uint32_t length) > +{ > + memcpy((void *) req->address, req->data, length); > + return 0; > +} I would have expected length to come from the d9_tstoremem struct, instead of being implied by the packet size (down below). Though you might want a sanity check regardless. > + > +/* d9_tstoremem allocates the response packet, calls the user-supplied ops > + * function for storing memory, and sends the response packet. */ > +int d9_tstoremem(struct d9_header *hdr, void *data) > +{ > + int length, ret; > + struct d9_header *resp; > + struct d9_tstoremem *tstoremem = (struct d9_tstoremem *) data; > + > + if (d9_ops == NULL || d9_ops->store_memory == NULL) > + return debug_send_error(EBADF /* TODO: better error code */); > + > + length = hdr->size - sizeof(struct d9_header) - 1 - > + sizeof(struct d9_tstoremem); Are you sure you want to subtract 1 here? I imagine that 1 came from data[1] in the struct definition. I'd just remove it from the struct (which I mention below). If anything, I think you'd add the 1 Since when you subtract sizeof(struct d9_tstoremem), that removed the extra 1 byte that actually belonged to length. Anyway, I'd just remove the '1' completely. > + > + /* Call user-supplied routine for filling in response packet. */ > + ret = d9_ops->store_memory(tstoremem, length); > + > + if (ret < 0) { > + /* Error with errno -ret. */ > + return debug_send_error(-ret); > + } > + > + /* Successful response. */ > + resp = alloc_packet(0, D9_RSTOREMEM); > + if (resp == NULL) > + return debug_send_error(ENOMEM); > + > + ret = debug_send_response(resp); > + free(resp); > + return ret; > +} > + > +/* d9_treadmem allocates the response packet, calls the user-supplied ops > + * function for reading from memory, and sends the response packet. */ > +int d9_treadmem(struct d9_header *hdr, void *data) > +{ > + int ret; > + struct d9_header *rpack; > + struct d9_rreadmem *rreadmem; > + struct d9_treadmem *treadmem = (struct d9_treadmem *) data; > + > + if (d9_ops == NULL || d9_ops->read_memory == NULL) > + return debug_send_error(EBADF /* TODO */); > + > + /* Allocate response packet. */ > + rpack = alloc_packet(treadmem->length, D9_RREADMEM); > + if (rpack == NULL) > + return debug_send_error(ENOMEM); > + > + rreadmem = (struct d9_rreadmem *) (rpack + 1); As an example, if the d9_rreadmem contains the header, you won't need to do this pointer arithmetic - you just alloc the struct directly. Not a big deal though. > + > + /* Call user-supplied routine for filling in response packet. */ > + ret = d9_ops->read_memory(treadmem, rreadmem); > + > + if (ret < 0) { > + /* Error with errno -ret. */ > + free(rpack); > + return debug_send_error(-ret); > + } > + > + /* Successful response. */ > + ret = debug_send_response(rpack); > + free(rpack); > + return ret; > +} > + > +/* d9_tfetchreg allocates the response packet, finds the appropriate thread > + * structure, calls the user-supplied ops function for reading its registers, > + * and sends the response packet. */ > +int d9_tfetchreg(struct d9_header *hdr, void *data) > +{ > + int ret; > + struct uthread *t; > + struct d9_header *rpack; > + struct d9_rfetchreg *resp; > + struct d9_tfetchreg *req = (struct d9_tfetchreg *) data; > + > + if (d9_ops == NULL || d9_ops->fetch_registers == NULL) > + return debug_send_error(EBADF /* TODO */); > + > + /* Find the appropriate thread. */ > + t = uthread_get_thread_by_id(req->threadid); > + if (t == NULL) > + return debug_send_error(EBADF /* TODO */); > + > + /* Allocate response packet. */ > + rpack = alloc_packet(sizeof(struct d9_rfetchreg), D9_RFETCHREG); > + if (rpack == NULL) > + return debug_send_error(ENOMEM); > + > + resp = (struct d9_rfetchreg *) (rpack + 1); > + > + /* Call user-supplied routine for filling in response packet. */ > + ret = d9_ops->fetch_registers(t, &(resp->regs)); > + > + if (ret < 0) { > + /* Error with errno -ret. */ > + free(rpack); > + return debug_send_error(-ret); > + } > + > + /* Successful response. */ > + ret = debug_send_response(rpack); > + free(rpack); > + return ret; > +} > + > +/* debug_send_error sends an error response to gdbserver. */ > +int debug_send_error(uint32_t errnum) > +{ > + int ret; > + struct d9_rerror *rerror; > + struct d9_header *rpack; > + > + /* Allocate error response packet. */ > + rpack = alloc_packet(sizeof(struct d9_rerror), D9_RERROR); > + if (rpack == NULL) > + return -1; /* What now? */ > + > + /* Fill in error number. */ > + rerror = (struct d9_rerror *) (rpack + 1); > + rerror->errnum = errnum; > + > + ret = debug_send_response(rpack); > + free(rpack); > + return ret; > +} > + > +/* debug_send_response sends a response to gdbserver. */ > +int debug_send_response(struct d9_header *packet) > +{ > + return debug_send_packet(debug_fd, (void *) packet, packet->size); > +} > + > +/* d9_read_packet reads the rest of the packet given the packet header. > + * > + * The header contains the size of the packet. */ > +void *d9_read_packet(struct d9_header *hdr) > +{ > + size_t msg_size = hdr->size - sizeof(struct d9_header); > + void *data = NULL; > + int ret; > + > + if (msg_size > 0) { > + /* Read the remaining data of this packet. */ > + data = calloc(msg_size, 1); > + if (data == NULL) > + return NULL; > + > + if (read(debug_fd, data, msg_size) != msg_size) { > + free(data); > + return NULL; > + } We should handle partial reads here. It's probably unlikely with gdb and the current pipe implementation, but they might only write a byte at a time or something. Especially if we ever have bigger payloads coming in for a read (maybe a storemem?). > + } > + > + return data; > +} > + > +int debug_send_packet(int fd, void *data, size_t size) > +{ > + ssize_t wlen, len_written = 0; > + > + while (len_written < size) { > + wlen = write(fd, (uint8_t *) data + len_written, size - > len_written); > + if (wlen < 0) { > + if (errno == EINTR) > + continue; > + else > + return -1; > + } > + len_written += wlen; > + } > + return 0; > +} > + > +int check_error_packet(struct d9_header *hdr, enum d9_msg_t msg_type) > +{ > + struct d9_rerror *rerror = NULL; > + > + /* Msg is of expected type -- no error. */ > + if (hdr->msg_type == msg_type) > + return 0; > + > + if (hdr->msg_type == D9_RERROR) { > + /* Got error message. */ > + rerror = (struct d9_rerror *) (hdr + 1); > + errno = rerror->errnum; > + } else { > + /* Neither got the expected message nor an error message. */ > + errno = EIO; > + } > + return 1; > +} > + > +int debug_read_memory(int fd, uintptr_t address, uint32_t length, uint8_t > *buf) Not sure, but do we want to somehow mark the client side operations by a naming convention? > +{ > + size_t return_packet_size; > + struct d9_header *hdr, *rhdr; > + struct d9_treadmem *req; > + struct d9_rreadmem *resp; > + > + hdr = alloc_packet(sizeof(struct d9_treadmem), D9_TREADMEM); > + if (hdr == NULL) > + return -1; > + > + req = (struct d9_treadmem *) (hdr + 1); > + req->address = address; > + req->length = length; > + > + int ret = debug_send_packet(fd, hdr, hdr->size); Don't forget to free hdr (looks you aren't). > + > + /* Block on return packet. */ > + > + /* TODO(chrisko): need to do this asynchronously */ What's the deal here? Does the client need to be async? > + return_packet_size = sizeof(struct d9_header) + length; > + char pkt_buf[return_packet_size]; Depending on the size of return_packet_size, this might be a little dangerous. A lot of our 2LSs have a limit to the stacksize. For pthreads, which gdbserver uses, it is currently 4MB. How big are these return packets? > + > + if (read(fd, pkt_buf, return_packet_size) <= 0) { > + perror("d9 read memory"); > + return -1; > + } As mentioned above, you want to handle partial reads. You should be able to use the same helper function that the server side ops use. > + > + rhdr = (struct d9_header *) pkt_buf; > + if (check_error_packet(rhdr, D9_RREADMEM)) { > + perror("d9 read memory"); > + return -1; > + } > + > + resp = (struct d9_rreadmem *) (rhdr + 1); > + memcpy(buf, resp->data, length); > + return 0; > +} > + > +int debug_fetch_registers(int fd, uint64_t tid, struct d9_regs *regs) > +{ > + size_t return_packet_size; > + struct d9_header *hdr, *rhdr; > + struct d9_tfetchreg *req; > + > + hdr = alloc_packet(sizeof(struct d9_tfetchreg), D9_TFETCHREG); > + if (hdr == NULL) > + return -1; > + > + req = (struct d9_tfetchreg *) (hdr + 1); > + req->threadid = tid; > + > + int ret = debug_send_packet(fd, hdr, hdr->size); Don't forget to free hdr (looks you aren't). > + > + /* Block on return packet. */ > + > + /* TODO(chrisko): need to do this asynchronously */ > + return_packet_size = sizeof(struct d9_header) + sizeof(struct > d9_rfetchreg); > + char pkt_buf[return_packet_size]; > + > + if (read(fd, pkt_buf, return_packet_size) != return_packet_size) { > + perror("d9 fetch registers"); > + return -1; > + } > + > + rhdr = (struct d9_header *) pkt_buf; > + if (check_error_packet(rhdr, D9_RFETCHREG)) { > + perror("d9 fetch registers"); > + return -1; > + } > + > + memcpy(regs, rhdr + 1, sizeof(struct d9_regs)); > + return 0; > +} > diff --git a/user/parlib/include/parlib/debug.h > b/user/parlib/include/parlib/debug.h > new file mode 100644 > index 000000000000..68b462dca58f > --- /dev/null > +++ b/user/parlib/include/parlib/debug.h > @@ -0,0 +1,67 @@ > +#pragma once > + > +#include <parlib/arch/debug.h> > +#include <parlib/uthread.h> > + > +enum d9_msg_t { > + D9_TREADMEM = 10, > + D9_RREADMEM, > + D9_TSTOREMEM, > + D9_RSTOREMEM, > + D9_TFETCHREG, > + D9_RFETCHREG, > + D9_TERROR, /* Do not use. */ > + D9_RERROR, > + D9_NUM_MESSAGES, /* Do not use. */ > +}; > + > +struct d9_header { > + uint32_t size; > + uint32_t msg_type; > +}; > + > +struct d9_rerror { > + uint32_t errnum; > +}; Is the actual format for all of these messages something like: struct d9_foo { struct d9_header hdr; something_specific_to_foo; } If so, it's usually better to embed the header into the outer struct. See how Linux did it (with perf) for details: tools/dev-util/perf/perf_format.h. I think we want to make sure they are __attribute__((packed)), to avoid confusion in the future. On a related note, in the future, we probably want to make these message structures endian-independent. Right now, everything has to be host-endian. For an endian-independent example, check out what Davide did with Akaros's perf: kern/arch/x86/ros/perfmon.h > + > +struct d9_treadmem { > + uintptr_t address; > + uint32_t length; > +}; > + > +struct d9_rreadmem { > + uint8_t data[1]; /* Must be the last member. */ > +}; > + > +struct d9_tstoremem { > + uintptr_t address; > + uint8_t data[1]; /* Must be the last member. */ For these two, I think you don't need the '1'. It makes the size calculations confusing too. > +}; > + > +struct d9_rstoremem { > +}; > + > +struct d9_tfetchreg { > + uint64_t threadid; > +}; > + > +struct d9_rfetchreg { > + struct d9_regs regs; /* Architecture-dependent. */ > +}; > + > +/* Server-side ops. */ > +struct d9_ops { > + int (*read_memory)(const struct d9_treadmem *req, struct d9_rreadmem > *resp); > + int (*store_memory)(const struct d9_tstoremem *req, uint32_t length); > + int (*fetch_registers)(struct uthread *t, struct d9_regs *resp); > +}; > + > +/* Server-side functions. */ > +void d9_server_init(struct schedule_ops *ops, struct d9_ops *debug_ops); > +int d9_read_memory(const struct d9_treadmem *req, struct d9_rreadmem *resp); > +int d9_store_memory(const struct d9_tstoremem *req, uint32_t length); If you statically set up d9_ops, then these don't need to be exposed outside debug.c, I think. Though I think you'd still need the arch-dependent d9_fetch_registers in a debug header. > + > +/* Client-side ops. */ > +int debug_read_memory(int fd, uintptr_t address, uint32_t length, uint8_t > *buf); > +int debug_store_memory(int fd, uintptr_t address, void *data, uint32_t > length); > +int debug_fetch_registers(int fd, uint64_t tid, struct d9_regs *regs); > diff --git a/user/parlib/include/parlib/x86/debug.h > b/user/parlib/include/parlib/x86/debug.h > new file mode 100644 > index 000000000000..2fe452744abf > --- /dev/null > +++ b/user/parlib/include/parlib/x86/debug.h > @@ -0,0 +1,32 @@ > +#pragma once > + > +#include <parlib/uthread.h> > + > +struct d9_regs { > + uint64_t reg_rax; > + uint64_t reg_rbx; > + uint64_t reg_rcx; > + uint64_t reg_rdx; > + uint64_t reg_rsp; > + uint64_t reg_rbp; > + uint64_t reg_rsi; > + uint64_t reg_rdi; > + uint64_t reg_rip; > + uint64_t reg_r8; > + uint64_t reg_r9; > + uint64_t reg_r10; > + uint64_t reg_r11; > + uint64_t reg_r12; > + uint64_t reg_r13; > + uint64_t reg_r14; > + uint64_t reg_r15; > + uint64_t reg_eflags; > + uint64_t reg_cs; > + uint64_t reg_ss; > + uint64_t reg_ds; > + uint64_t reg_es; > + uint64_t reg_fs; > + uint64_t reg_gs; > +}; > + > +int d9_fetch_registers(struct uthread *t, struct d9_regs *resp); > diff --git a/user/parlib/uthread.c b/user/parlib/uthread.c > index 582e4cb50ec1..4b53d0f2b141 100644 > --- a/user/parlib/uthread.c > +++ b/user/parlib/uthread.c > @@ -5,6 +5,7 @@ > #include <parlib/arch/atomic.h> > #include <parlib/arch/trap.h> > #include <parlib/assert.h> > +#include <parlib/debug.h> > #include <parlib/event.h> > #include <parlib/spinlock.h> > #include <parlib/parlib.h> > @@ -16,6 +17,7 @@ > * pthreads, should override sched_ops in its init code. */ > extern struct schedule_ops thread0_2ls_ops; > struct schedule_ops *sched_ops = &thread0_2ls_ops; > +struct d9_ops d9ops; > > __thread struct uthread *current_uthread = 0; > /* ev_q for all preempt messages (handled here to keep 2LSs from worrying > @@ -178,6 +180,12 @@ void uthread_2ls_init(struct uthread *uthread, struct > schedule_ops *ops) > cmb(); > __vcore_context = FALSE; > enable_notifs(0); /* will trigger a self_notif if we missed a > notif */ > + > + d9ops.read_memory = &d9_read_memory; > + d9ops.store_memory = &d9_store_memory; > + d9ops.fetch_registers = &d9_fetch_registers; > + /* Make ourselves available for debugging */ > + d9_server_init(ops, &d9ops); It might be better to do this at the end of uthread_lib_init(), instead of in uthread_2ls_init. (More syscall and event infrastructure is set up). And you probably don't need the d9ops stuff in uthread.c at all. > } > > /* Helper: tells the kernel our SCP is capable of going into vcore context on > diff --git a/user/parlib/x86/debug.c b/user/parlib/x86/debug.c > new file mode 100644 > index 000000000000..0cc976b7f4f1 > --- /dev/null > +++ b/user/parlib/x86/debug.c > @@ -0,0 +1,18 @@ > +#include <parlib/arch/debug.h> > + > +int d9_fetch_registers(struct uthread *t, struct d9_regs *regs) > +{ > + switch (t->u_ctx.type) { > + case ROS_HW_CTX: > + regs->reg_rax = t->u_ctx.tf.hw_tf.tf_rax; > + regs->reg_rbx = t->u_ctx.tf.hw_tf.tf_rbx; > + regs->reg_rip = t->u_ctx.tf.hw_tf.tf_rip; > + break; > + case ROS_SW_CTX: > + regs->reg_rbx = t->u_ctx.tf.sw_tf.tf_rbx; > + regs->reg_rip = t->u_ctx.tf.sw_tf.tf_rip; > + break; Don't forget about all the other registers (e.g. r15). We also ought to have support for reading the VM ctx registers too. They aren't going to hit breakpoints (for now!), but when we stop-the-world, they'll get caught up too. Barret > + } > + > + return 0; > +} > -- > 2.8.0.rc3.226.g39d4020 > -- You received this message because you are subscribed to the Google Groups "Akaros" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. For more options, visit https://groups.google.com/d/optout.
