[kernel] Make mailbox non-fixed-length again

Going back to letting mailboxes use variable-length data. Note that this
requires extra copies, so shared memory channels should be used for
anything in the hot path. But this allows better RPC over mailboxes and
other flexibility.

Other changes:
- added a j6::proto::sl::client class to act as a service locator
  client, instead of duplicating that code in every program.
- moved protocol ids into j6/tables/protocols.inc so that C++ clients
  can easily have their own API

definitions/objects/mailbox.def

@@ -1,5 +1,6 @@
-# Mailboxes are objects that enable synchronous IPC via short message-passing
-# of tagged handles.
+# Mailboxes are objects that enable synchronous IPC via arbitrary
+# message-passing of tagged data and/or handles. Not as efficient
+# as shared memory channels, but more flexible.
 
 object mailbox : object {
     uid 99934ad04ece1e07
@@ -13,13 +14,14 @@ object mailbox : object {
     method create [constructor]
     method close [destructor cap:close]
 
-    # Send a message to the reciever, and block until a
-    # response is sent. Note that getting this response
-    # does not require the receive capability.
+    # Send a message to the reciever, and block until a response is
+    # sent. Note that getting this response does not require the
+    # receive capability.
     method call [cap:send] {
         param tag uint64 [inout]
-        param subtag uint64 [inout]
-        param give_handle ref object [optional inout handle]
+        param data buffer [optional inout]
+        param data_in_len size  # number of bytes in data used for input
+        param handles ref object [optional inout handle list]
     }
 
     # Respond to a message sent using call, and wait for another
@@ -28,8 +30,9 @@ object mailbox : object {
     # to waiting for a new message.
     method respond [cap:receive] {
         param tag uint64 [inout]
-        param subtag uint64 [inout]
-        param give_handle ref object [optional inout handle]
+        param data buffer [optional inout]
+        param data_in_len size  # number of bytes in data used for input
+        param handles ref object [optional inout handle list]
         param reply_tag uint64 [inout]
         param flags uint64
     }

src/kernel/ipc_message.cpp

@@ -0,0 +1,49 @@
+#include <j6/memutils.h>
+#include <util/basic_types.h>
+
+#include "ipc_message.h"
+
+namespace ipc {
+
+message::message() : tag {0}, data {nullptr, 0}, handles {nullptr, 0} {}
+
+message::message(
+    uint64_t in_tag,
+    const util::buffer &in_data,
+    const util::counted<j6_handle_t> &in_handles) :
+        tag {in_tag}, data {nullptr, in_data.count}, handles {nullptr, in_handles.count}
+{
+    if (data.count) {
+        data.pointer = new uint8_t [data.count];
+        memcpy(data.pointer, in_data.pointer, data.count);
+    }
+
+    if (handles.count) {
+        handles.pointer = new j6_handle_t [handles.count];
+        memcpy(handles.pointer, in_handles.pointer, handles.count * sizeof(j6_handle_t));
+    }
+}
+
+message::message(message &&other) { *this = util::move(other); }
+
+message::~message()
+{
+    delete [] reinterpret_cast<uint8_t*>(data.pointer);
+    delete [] handles.pointer;
+}
+
+message &
+message::operator=(message &&other)
+{
+    tag = other.tag;
+    other.tag = 0;
+
+    data = other.data;
+    other.data = {nullptr, 0};
+
+    handles = other.handles;
+    other.handles = {nullptr, 0};
+    return *this;
+}
+
+} // namespace ipc

src/kernel/ipc_message.h

@@ -0,0 +1,25 @@
+#pragma once
+/// \file ipc_message.h
+/// Definition of shared message structure
+
+#include <stdint.h>
+#include <j6/types.h>
+#include <util/counted.h>
+
+namespace ipc {
+
+struct message
+{
+    uint64_t tag;
+    util::buffer data;
+    util::counted<j6_handle_t> handles;
+
+    message();
+    message(uint64_t in_tag, const util::buffer &in_data, const util::counted<j6_handle_t> &in_handles);
+    message(message &&other);
+    ~message();
+
+    message & operator=(message &&other);
+};
+
+} // namespace ipc

src/kernel/kernel.module

@@ -27,6 +27,7 @@ kernel = module("kernel",
         "interrupts.cpp",
         "interrupts.s",
         "io.cpp",
+        "ipc_message.cpp",
         "kernel_main.cpp",
         "logger.cpp",
         "memory.cpp",

src/kernel/objects/mailbox.cpp

@@ -1,4 +1,6 @@
+#include <util/basic_types.h>
 #include <util/counted.h>
+#include <j6/memutils.h>
 
 #include "objects/mailbox.h"
 #include "objects/thread.h"
@@ -49,7 +51,7 @@ mailbox::call()
 }
 
 j6_status_t
-mailbox::receive(thread::message_data &data, reply_tag_t &reply_tag, bool block)
+mailbox::receive(ipc::message &data, reply_tag_t &reply_tag, bool block)
 {
     if (closed())
         return j6_status_closed;
@@ -81,7 +83,7 @@ mailbox::receive(thread::message_data &data, reply_tag_t &reply_tag, bool block)
 }
 
 j6_status_t
-mailbox::reply(reply_tag_t reply_tag, const thread::message_data &data)
+mailbox::reply(reply_tag_t reply_tag, ipc::message &&data)
 {
     if (closed())
         return j6_status_closed;
@@ -95,7 +97,7 @@ mailbox::reply(reply_tag_t reply_tag, const thread::message_data &data)
     m_reply_map.erase(reply_tag);
     lock.release();
 
-    caller->get_message_data() = data;
+    caller->set_message_data(util::move(data));
     caller->wake(j6_status_ok);
     return j6_status_ok;
 }

src/kernel/objects/mailbox.h

@@ -8,10 +8,9 @@
 #include <util/spinlock.h>
 
 #include "heap_allocator.h"
+#include "ipc_message.h"
 #include "memory.h"
 #include "objects/kobject.h"
-#include "objects/thread.h"
-#include "slab_allocated.h"
 #include "wait_queue.h"
 
 namespace obj {
@@ -23,6 +22,7 @@ class mailbox :
     public kobject
 {
 public:
+
     using reply_tag_t = uint64_t;
 
     /// Capabilities on a newly constructed mailbox handle
@@ -43,23 +43,23 @@ public:
 
     /// Send a message to a thread waiting to receive on this mailbox, and block the
     /// current thread awaiting a response. The message contents should be in the calling
-    /// thread's message_data.
+    /// thread's message data.
     /// \returns      j6_status_ok if a reply was received
     j6_status_t call();
 
     /// Receive the next available message, optionally blocking if no messages are available.
-    /// \arg data         [out] a thread::message_data structure to fill
+    /// \arg data         [out] an ipc::message structure to fill
     /// \arg reply_tag    [out] the reply_tag to use when replying to this message
     /// \arg block        True if this call should block when no messages are available.
     /// \returns          j6_status_ok if a message was received
-    j6_status_t receive(thread::message_data &data, reply_tag_t &reply_tag, bool block);
+    j6_status_t receive(ipc::message &data, reply_tag_t &reply_tag, bool block);
 
     /// Find a given pending message to be responded to. Returns a replyer object, which will
-    /// wake the calling thread upon destruction.
+    /// wake the calling read upon destruction.
     /// \arg reply_tag  The reply tag in the original message
     /// \arg data       Message data to pass on to the caller
     /// \returns        j6_status_ok if the reply was successfully sent
-    j6_status_t reply(reply_tag_t reply_tag, const thread::message_data &data);
+    j6_status_t reply(reply_tag_t reply_tag, ipc::message &&data);
 
 private:
     wait_queue m_callers;

src/kernel/objects/process.cpp

@@ -118,6 +118,10 @@ process::thread_exited(thread *th)
 void
 process::add_handle(j6_handle_t handle)
 {
+    // Passing the invalid handle is fine, just don't add anything
+    if (handle == j6_handle_invalid)
+        return;
+
     capability *c = g_cap_table.retain(handle);
     kassert(c, "Trying to add a non-existant handle to a process!");
 

src/kernel/objects/thread.cpp

@@ -1,3 +1,4 @@
+#include <util/basic_types.h>
 #include <util/pointers.h>
 
 #include "kassert.h"
@@ -110,6 +111,9 @@ thread::wake_only()
     set_state(state::ready);
 }
 
+void thread::set_message_data(ipc::message &&md) { m_message = util::move(md); }
+ipc::message && thread::get_message_data() { return util::move(m_message); }
+
 void
 thread::exit()
 {

src/kernel/objects/thread.h

@@ -8,6 +8,7 @@
 #include <util/spinlock.h>
 
 #include "cpu.h"
+#include "ipc_message.h"
 #include "objects/kobject.h"
 #include "wait_queue.h"
 
@@ -121,13 +122,8 @@ public:
     /// \returns  The clock time at which to wake. 0 for no timeout.
     inline uint64_t wake_timeout() const { return m_wake_timeout; }
 
-    struct message_data
-    {
-        uint64_t tag, subtag;
-        j6_handle_t handle;
-    };
-
-    message_data & get_message_data() { return m_message_data; }
+    void set_message_data(ipc::message &&md);
+    ipc::message && get_message_data();
 
     inline bool has_state(state s) const {
         return __atomic_load_n(reinterpret_cast<const uint8_t*>(&m_state), __ATOMIC_ACQUIRE) &
@@ -200,7 +196,8 @@ private:
 
     uint64_t m_wake_value;
     uint64_t m_wake_timeout;
-    message_data m_message_data;
+
+    ipc::message m_message;
 
     wait_queue m_join_queue;
 };

src/kernel/syscalls/mailbox.cpp

@@ -1,7 +1,9 @@
 #include <j6/errors.h>
 #include <j6/flags.h>
+#include <util/counted.h>
 #include <util/util.h>
 
+#include "ipc_message.h"
 #include "objects/mailbox.h"
 #include "objects/thread.h"
 #include "syscalls/helpers.h"
@@ -31,30 +33,39 @@ j6_status_t
 mailbox_call(
         mailbox *self,
         uint64_t *tag,
-        uint64_t *subtag,
-        j6_handle_t *handle)
+        void *in_data,
+        size_t *data_len,
+        size_t data_in_len,
+        j6_handle_t *in_handles,
+        size_t *handles_count)
 {
-    thread::message_data &data =
-        thread::current().get_message_data();
+    thread &cur = thread::current();
 
-    data.tag = *tag;
-    data.subtag = *subtag;
+    util::buffer data {in_data, data_in_len};
+    util::counted<j6_handle_t> handles {in_handles, *handles_count};
 
-    if (handle)
-        data.handle = *handle;
+    ipc::message message(*tag, data, handles);
+    cur.set_message_data(util::move(message));
 
     j6_status_t s = self->call();
     if (s != j6_status_ok)
         return s;
 
-    *tag = data.tag;
-    *subtag = data.subtag;
+    message = cur.get_message_data();
 
-    if (handle) {
-        *handle = data.handle;
-        process::current().add_handle(*handle);
+    if (message.handles) {
+        for (unsigned i = 0; i < message.handles.count; ++i)
+            process::current().add_handle(message.handles[i]);
     }
 
+    *tag = message.tag;
+    *data_len = *data_len > message.data.count ? message.data.count : *data_len;
+    memcpy(in_data, message.data.pointer, *data_len);
+
+    size_t handles_min = *handles_count > message.handles.count ? message.handles.count : *handles_count;
+    *handles_count = handles_min;
+    memcpy(in_handles, message.handles.pointer, handles_min * sizeof(j6_handle_t));
+
     return j6_status_ok;
 }
 
@@ -62,28 +73,42 @@ j6_status_t
 mailbox_respond(
         mailbox *self,
         uint64_t *tag,
-        uint64_t *subtag,
-        j6_handle_t *handle,
+        void *in_data,
+        size_t *data_len,
+        size_t data_in_len,
+        j6_handle_t *in_handles,
+        size_t *handles_count,
         uint64_t *reply_tag,
         uint64_t flags)
 {
-    thread::message_data data { *tag, *subtag, *handle };
+    util::buffer data {in_data, data_in_len};
+    util::counted<j6_handle_t> handles {in_handles, *handles_count};
+
+    ipc::message message(*tag, data, handles);
 
     if (*reply_tag) {
-        j6_status_t s = self->reply(*reply_tag, data);
+        j6_status_t s = self->reply(*reply_tag, util::move(message));
         if (s != j6_status_ok)
             return s;
     }
 
     bool block = flags & j6_flag_block;
-    j6_status_t s = self->receive(data, *reply_tag, block);
+    j6_status_t s = self->receive(message, *reply_tag, block);
     if (s != j6_status_ok)
         return s;
 
-    *tag = data.tag;
-    *subtag = data.subtag;
-    *handle = data.handle;
-    process::current().add_handle(*handle);
+    if (message.handles) {
+        for (unsigned i = 0; i < message.handles.count; ++i)
+            process::current().add_handle(message.handles[i]);
+    }
+
+    *tag = message.tag;
+    *data_len = *data_len > message.data.count ? message.data.count : *data_len;
+    memcpy(in_data, message.data.pointer, *data_len);
+
+    size_t handles_min = *handles_count > message.handles.count ? message.handles.count : *handles_count;
+    *handles_count = handles_min;
+    memcpy(in_handles, message.handles.pointer, handles_min * sizeof(j6_handle_t));
 
     return j6_status_ok;
 }

src/libraries/j6/include/j6/protocols.h

@@ -7,6 +7,34 @@ enum j6_proto_base_tag
     j6_proto_base_status,
     j6_proto_base_get_proto_id,
     j6_proto_base_proto_id,
+    j6_proto_base_open_channel,
+    j6_proto_base_opened_channel,
 
     j6_proto_base_first_proto_id /// The first protocol-specific ID
 };
+
+#ifdef __cplusplus
+#include <util/hash.h>
+namespace j6::proto {
+
+enum class style { mailbox, channel };
+
+#define PROTOCOL(name, desc, type) namespace name { \
+    inline constexpr uint64_t id = #desc##_id; \
+    inline constexpr proto::style style = proto::style:: type; }
+
+#include <j6/tables/protocols.inc>
+#undef PROTOCOL
+
+} // namespace j6::proto
+#endif // __cplusplus
+
+extern "C" const uint64_t j6_proto_style_mailbox;
+extern "C" const uint64_t j6_proto_style_channel;
+
+#define PROTOCOL(name, desc, type) \
+    extern "C" const uint64_t j6_proto_ ## name ## _id; \
+    extern "C" const uint64_t j6_proto_ ## name ## _style;
+
+#include <j6/tables/protocols.inc>
+#undef PROTOCOL

src/libraries/j6/include/j6/protocols.hh

@@ -0,0 +1,13 @@
+#pragma once
+/// \file protocols.hh
+/// High-level C++ base protocol interface
+
+namespace j6::proto {
+
+class client
+{
+public:
+    
+};
+
+} // namespace j6::proto

src/libraries/j6/include/j6/protocols/service_locator.h

@@ -4,8 +4,6 @@
 
 #include <j6/protocols.h>
 
-extern const uint64_t j6_proto_sl_id;
-
 enum j6_proto_sl_tag
 {
     j6_proto_sl_register = j6_proto_base_first_proto_id,

src/libraries/j6/include/j6/protocols/service_locator.hh

@@ -0,0 +1,27 @@
+#include <j6/protocols/service_locator.h>
+#include <j6/types.h>
+
+namespace j6::proto::sl {
+
+class client
+{
+public:
+    /// Constructor.
+    /// \arg slp_mb  Handle to the service locator service's mailbox
+    client(j6_handle_t slp_mb);
+
+    /// Register a handle as a service with the locator.
+    /// \arg proto_id  The protocol this handle supports
+    /// \arg handle    The mailbox or channel handle
+    j6_status_t register_service(uint64_t proto_id, j6_handle_t handle);
+
+    /// Look up a handle with the locator service.
+    /// \arg proto_id  The protocol to look for
+    /// \arg handle    [out] The mailbox or channel handle
+    j6_status_t lookup_service(uint64_t proto_id, j6_handle_t &handle);
+
+private:
+    j6_handle_t m_service;
+};
+
+} // namespace j6::proto::sl

src/libraries/j6/include/j6/tables/protocols.inc

@@ -0,0 +1,2 @@
+PROTOCOL(sl,    service_locator,    mailbox)
+PROTOCOL(vfs,   virtual_filesystem, channel)

src/libraries/j6/j6.module

@@ -11,6 +11,7 @@ j6 = module("j6",
         "memutils.cpp",
         "mutex.cpp",
         "protocol_ids.cpp",
+        "protocols/service_locator.cpp",
         "syscalls.s.cog",
         "sysconf.cpp.cog",
         "syslog.cpp",
@@ -27,6 +28,7 @@ j6 = module("j6",
         "j6/memutils.h",
         "j6/protocols.h",
         "j6/protocols/service_locator.h",
+        "j6/protocols/service_locator.hh",
         "j6/syscalls.h.cog",
         "j6/sysconf.h.cog",
         "j6/syslog.hh",

src/libraries/j6/protocol_ids.cpp

@@ -1,4 +1,10 @@
-#include <util/hash.h>
+#include <j6/protocols.h>
 
-extern "C"
-const uint64_t j6_proto_sl_id = "jsix.protocol.service_locator"_id;
+extern "C" const uint64_t j6_proto_style_mailbox = (uint64_t)j6::proto::style::mailbox;
+extern "C" const uint64_t j6_proto_style_channel = (uint64_t)j6::proto::style::channel;
+
+#define PROTOCOL(name, desc, type) \
+    extern "C" const uint64_t j6_proto_##name##_id = j6::proto::name::id; \
+    extern "C" const uint64_t j6_proto_##name##_style = j6_proto_style_##type;
+
+#include <j6/tables/protocols.inc>

src/libraries/j6/protocols/service_locator.cpp

@@ -0,0 +1,64 @@
+#include <j6/errors.h>
+#include <j6/protocols/service_locator.hh>
+#include <j6/syscalls.h>
+#include <j6/syslog.hh>
+
+#ifndef __j6kernel
+
+namespace j6::proto::sl {
+
+client::client(j6_handle_t slp_mb) :
+    m_service {slp_mb}
+{
+}
+
+j6_status_t
+client::register_service(uint64_t proto_id, j6_handle_t handle)
+{
+    uint64_t tag = j6_proto_sl_register;
+    size_t handle_count = 1;
+    size_t data = proto_id;
+    size_t data_size = sizeof(proto_id);
+
+    j6_status_t s = j6_mailbox_call(m_service, &tag,
+        &data, &data_size, data_size,
+        &handle, &handle_count);
+
+    if (s != j6_status_ok)
+        return s;
+
+    if (tag == j6_proto_base_status)
+        return data; // contains a status
+
+    return j6_err_unexpected;
+}
+
+j6_status_t
+client::lookup_service(uint64_t proto_id, j6_handle_t &handle)
+{
+    uint64_t tag = j6_proto_sl_find;
+    size_t handle_count = 1;
+    size_t data = proto_id;
+    size_t data_size = sizeof(proto_id);
+    handle = j6_handle_invalid;
+
+    j6::syslog("Looking up service for %x", proto_id);
+    j6_status_t s = j6_mailbox_call(m_service, &tag,
+        &data, &data_size, data_size,
+        &handle, &handle_count);
+
+    if (s != j6_status_ok)
+        return s;
+
+    if (tag == j6_proto_sl_result)
+        return j6_status_ok; // handle is already in `handle`
+
+    else if (tag == j6_proto_base_status)
+        return data; // contains a status
+
+    return j6_err_unexpected;
+}
+
+
+} // namespace j6::proto::sl
+#endif // __j6kernel

src/user/drv.uart/main.cpp

@@ -8,7 +8,7 @@
 #include <j6/errors.h>
 #include <j6/flags.h>
 #include <j6/init.h>
-#include <j6/protocols/service_locator.h>
+#include <j6/protocols/service_locator.hh>
 #include <j6/syscalls.h>
 #include <j6/sysconf.h>
 #include <j6/syslog.hh>
@@ -76,16 +76,13 @@ main(int argc, const char **argv)
     if (!cout)
         return 2;
 
-    uint64_t tag = j6_proto_sl_register;
     uint64_t proto_id = "jsix.protocol.stream.ouput"_id;
     uint64_t handle = cout->handle();
-    result = j6_mailbox_call(slp, &tag, &proto_id, &handle);
+    j6::proto::sl::client slp_client {slp};
+    result = slp_client.register_service(proto_id, handle);
     if (result != j6_status_ok)
         return 4;
 
-    if (tag != j6_proto_base_status)
-        return 5;
-
     result = j6_system_request_iopl(g_handle_sys, 3);
     if (result != j6_status_ok)
         return 6;

src/user/srv.init/service_locator.cpp

@@ -1,4 +1,5 @@
 #include <unordered_map>
+#include <stdlib.h>
 
 #include <j6/errors.h>
 #include <j6/flags.h>
@@ -24,45 +25,44 @@ service_locator_start(j6_handle_t mb)
     std::unordered_map<uint64_t, j6_handle_t> services;
 
     uint64_t tag = 0;
-    uint64_t subtag = 0;
+    uint64_t data = 0;
+    uint64_t handle_count = 1;
     uint64_t reply_tag = 0;
 
     j6_handle_t give_handle = j6_handle_invalid;
     uint64_t proto_id;
 
     while (true) {
-        j6_status_t s = j6_mailbox_respond(mb, &tag, &subtag, &give_handle,
+        uint64_t data_len = sizeof(uint64_t);
+        j6_status_t s = j6_mailbox_respond(mb, &tag,
+            &data, &data_len, data_len,
+            &give_handle, &handle_count,
             &reply_tag, j6_flag_block);
 
         if (s != j6_status_ok)
-            while (1);
+            exit(128);
 
         handle_entry *found = nullptr;
 
         switch (tag) {
-        case j6_proto_base_get_proto_id:
-            tag = j6_proto_base_proto_id;
-            subtag = j6_proto_sl_id;
-            give_handle = j6_handle_invalid;
-            break;
-
         case j6_proto_sl_register:
-            proto_id = subtag;
+            proto_id = data;
             if (give_handle == j6_handle_invalid) {
                 tag = j6_proto_base_status;
-                subtag = j6_err_invalid_arg;
+                data = j6_err_invalid_arg;
                 break;
             }
 
             services.insert( {proto_id, give_handle} );
             tag = j6_proto_base_status;
-            subtag = j6_status_ok;
+            data = j6_status_ok;
             give_handle = j6_handle_invalid;
             break;
 
         case j6_proto_sl_find:
-            proto_id = subtag;
+            proto_id = data;
             tag = j6_proto_sl_result;
+            data = 0;
 
             {
                 auto found = services.find(proto_id);
@@ -75,7 +75,7 @@ service_locator_start(j6_handle_t mb)
 
         default:
             tag = j6_proto_base_status;
-            subtag = j6_err_invalid_arg;
+            data = j6_err_invalid_arg;
             give_handle = j6_handle_invalid;
             break;
         }

src/user/srv.init/service_locator.h

@@ -2,4 +2,6 @@
 /// \file service_locator.h
 /// Definitions for srv.init's implementation of the service locator protocol
 
+#include <j6/types.h>
+
 void service_locator_start(j6_handle_t mb);

src/user/srv.logger/main.cpp

@@ -8,7 +8,7 @@
 #include <j6/errors.h>
 #include <j6/flags.h>
 #include <j6/init.h>
-#include <j6/protocols/service_locator.h>
+#include <j6/protocols/service_locator.hh>
 #include <j6/syscalls.h>
 #include <j6/sysconf.h>
 #include <j6/types.h>
@@ -106,13 +106,12 @@ main(int argc, const char **argv)
 
     j6_handle_t cout_vma = j6_handle_invalid;
 
-    for (unsigned i = 0; i < 100; ++i) {
-        uint64_t tag = j6_proto_sl_find;
     uint64_t proto_id = "jsix.protocol.stream.ouput"_id;
+    j6::proto::sl::client slp_client {slp};
 
-        j6_status_t s = j6_mailbox_call(slp, &tag, &proto_id, &cout_vma);
+    for (unsigned i = 0; i < 100; ++i) {
+        j6_status_t s = slp_client.lookup_service(proto_id, cout_vma);
         if (s == j6_status_ok &&
-            tag == j6_proto_sl_result &&
             cout_vma != j6_handle_invalid)
             break;