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.
m-labs · Aug 7, 2015 · b5cf1e3 · b5cf1e3
1 parent 1a969aa
commit b5cf1e3
Show file tree

Hide file tree

Showing 6 changed files with 41 additions and 62 deletions.
diff --git a/artiq/coredevice/comm_generic.py b/artiq/coredevice/comm_generic.py
@@ -130,10 +130,9 @@ def load(self, kcode):
         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
@@ -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
@@ -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
@@ -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();
-
-    if(kernel) {
-        void (*run_closure)(void *) = msg->library_info->init;
-        run_closure(kernel);
+        mailbox_send_and_wait(&load_reply);
+        kernel_init();
 
-        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
@@ -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
@@ -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;