From b5cf1e395d0af8c89317568727581fe2f9e7d2b3 Mon Sep 17 00:00:00 2001 From: whitequark Date: Fri, 7 Aug 2015 08:49:57 +0300 Subject: [PATCH] runtime: avoid race condition when running kernel. Also, don't bother passing kernel name: entry point is already recorded in DT_INIT when the kernel is linked. --- artiq/coredevice/comm_generic.py | 7 ++--- soc/runtime/kloader.c | 23 +++++++++----- soc/runtime/kloader.h | 2 +- soc/runtime/ksupport.c | 54 ++++++++++++-------------------- soc/runtime/messages.h | 2 +- soc/runtime/session.c | 15 +-------- 6 files changed, 41 insertions(+), 62 deletions(-) diff --git a/artiq/coredevice/comm_generic.py b/artiq/coredevice/comm_generic.py index 2daa24cdc..b538c33f7 100644 --- a/artiq/coredevice/comm_generic.py +++ b/artiq/coredevice/comm_generic.py @@ -130,10 +130,9 @@ class CommGeneric: if ty != _D2HMsgType.LOAD_COMPLETED: raise IOError("Incorrect reply from device: "+str(ty)) - def run(self, kname): - self._write_header(len(kname) + 9, _H2DMsgType.RUN_KERNEL) - self.write(bytes(kname, "ascii")) - logger.debug("running kernel: %s", kname) + def run(self): + self._write_header(9, _H2DMsgType.RUN_KERNEL) + logger.debug("running kernel") def flash_storage_read(self, key): self._write_header(9+len(key), _H2DMsgType.FLASH_READ_REQUEST) diff --git a/soc/runtime/kloader.c b/soc/runtime/kloader.c index ea8e4f5df..547907728 100644 --- a/soc/runtime/kloader.c +++ b/soc/runtime/kloader.c @@ -30,22 +30,29 @@ void kloader_start_bridge() start_kernel_cpu(NULL); } -static int load_or_start_kernel(void *library, const char *kernel) +static int load_or_start_kernel(void *library, int run_kernel) { static struct dyld_info library_info; struct msg_load_request request = { .library = library, .library_info = &library_info, - .kernel = kernel, + .run_kernel = run_kernel, }; start_kernel_cpu(&request); struct msg_load_reply *reply = mailbox_wait_and_receive(); - if(reply != NULL && reply->type == MESSAGE_TYPE_LOAD_REPLY) { - log("cannot load/run kernel: %s", reply->error); + if(reply->type != MESSAGE_TYPE_LOAD_REPLY) { + log("BUG: unexpected reply to load/run request"); return 0; } + if(reply->error != NULL) { + log("cannot load kernel: %s", reply->error); + return 0; + } + + mailbox_acknowledge(); + return 1; } @@ -56,12 +63,12 @@ int kloader_load_library(void *library) return 0; } - return load_or_start_kernel(library, NULL); + return load_or_start_kernel(library, 0); } -int kloader_start_kernel(const char *name) +void kloader_start_kernel() { - return load_or_start_kernel(NULL, name); + load_or_start_kernel(NULL, 1); } int kloader_start_idle_kernel(void) @@ -74,7 +81,7 @@ int kloader_start_idle_kernel(void) if(length <= 0) return 0; - return load_or_start_kernel(buffer, "test.__modinit__"); + return load_or_start_kernel(buffer, 1); #else return 0; #endif diff --git a/soc/runtime/kloader.h b/soc/runtime/kloader.h index 08036456c..834fd8d3d 100644 --- a/soc/runtime/kloader.h +++ b/soc/runtime/kloader.h @@ -12,7 +12,7 @@ int kloader_load_library(void *code); void kloader_start_bridge(void); int kloader_start_idle_kernel(void); -int kloader_start_kernel(const char *name); +void kloader_start_kernel(void); void kloader_stop(void); int kloader_validate_kpointer(void *p); diff --git a/soc/runtime/ksupport.c b/soc/runtime/ksupport.c index c7d465c26..da02bc4a0 100644 --- a/soc/runtime/ksupport.c +++ b/soc/runtime/ksupport.c @@ -184,51 +184,37 @@ void exception_handler(unsigned long vect, unsigned long *regs, int main(void); int main(void) { - struct msg_load_request *msg = mailbox_receive(); + struct msg_load_request *request = mailbox_receive(); + struct msg_load_reply load_reply = { + .type = MESSAGE_TYPE_LOAD_REPLY, + .error = NULL + }; - if(msg == NULL) { + if(request == NULL) { bridge_main(); while(1); } - if(msg->library != NULL) { - const char *error; - if(!dyld_load(msg->library, KERNELCPU_PAYLOAD_ADDRESS, - resolve_runtime_export, msg->library_info, &error)) { - struct msg_load_reply msg = { - .type = MESSAGE_TYPE_LOAD_REPLY, - .error = error - }; - mailbox_send(&msg); + if(request->library != NULL) { + if(!dyld_load(request->library, KERNELCPU_PAYLOAD_ADDRESS, + resolve_runtime_export, request->library_info, + &load_reply.error)) { + mailbox_send(&load_reply); while(1); } } - void (*kernel)(void) = NULL; - if(msg->kernel != NULL) { - kernel = dyld_lookup(msg->kernel, msg->library_info); - if(kernel == NULL) { - char error[256]; - snprintf(error, sizeof(error), - "kernel '%s' not found in library", msg->kernel); - struct msg_load_reply msg = { - .type = MESSAGE_TYPE_LOAD_REPLY, - .error = error - }; - mailbox_send(&msg); - while(1); - } - } + if(request->run_kernel) { + void (*kernel_init)() = request->library_info->init; - mailbox_acknowledge(); + mailbox_send_and_wait(&load_reply); + kernel_init(); - if(kernel) { - void (*run_closure)(void *) = msg->library_info->init; - run_closure(kernel); - - struct msg_base msg; - msg.type = MESSAGE_TYPE_FINISHED; - mailbox_send_and_wait(&msg); + struct msg_base finished_reply; + finished_reply.type = MESSAGE_TYPE_FINISHED; + mailbox_send_and_wait(&finished_reply); + } else { + mailbox_send(&load_reply); } while(1); diff --git a/soc/runtime/messages.h b/soc/runtime/messages.h index 4b633a5d5..cb0c9c20b 100644 --- a/soc/runtime/messages.h +++ b/soc/runtime/messages.h @@ -39,7 +39,7 @@ struct msg_base { struct msg_load_request { void *library; struct dyld_info *library_info; - const char *kernel; + int run_kernel; }; struct msg_load_reply { diff --git a/soc/runtime/session.c b/soc/runtime/session.c index 511cd8353..bc4ad4545 100644 --- a/soc/runtime/session.c +++ b/soc/runtime/session.c @@ -184,21 +184,8 @@ static int process_input(void) break; } - if((buffer_in_index + 1) > BUFFER_OUT_SIZE) { - log("Kernel name too long"); - buffer_out[8] = REMOTEMSG_TYPE_KERNEL_STARTUP_FAILED; - submit_output(9); - break; - } - buffer_in[buffer_in_index] = 0; - watchdog_init(); - if(!kloader_start_kernel((char *)&buffer_in[9])) { - log("Failed to find kernel entry point '%s' in object", &buffer_in[9]); - buffer_out[8] = REMOTEMSG_TYPE_KERNEL_STARTUP_FAILED; - submit_output(9); - break; - } + kloader_start_kernel(); user_kernel_state = USER_KERNEL_RUNNING; break;