From 277b0d3cccaf0b845bd169fecd7fed5d5c5ac527 Mon Sep 17 00:00:00 2001 From: Drew Galbraith Date: Thu, 2 Nov 2023 23:31:08 -0700 Subject: [PATCH] [Zion] Use the glacier ArrayView class for sending IPC msgs. --- zion/interrupt/interrupt.cpp | 2 +- zion/lib/message_queue.cpp | 64 ++++++++++++++---------------------- zion/lib/message_queue.h | 24 +++++++------- zion/object/endpoint.cpp | 15 --------- zion/object/endpoint.h | 6 ---- zion/object/ipc_object.cpp | 22 ++++++++++--- zion/object/ipc_object.h | 11 +++++-- zion/syscall/ipc.cpp | 23 ++++++++++--- 8 files changed, 82 insertions(+), 85 deletions(-) diff --git a/zion/interrupt/interrupt.cpp b/zion/interrupt/interrupt.cpp index b00e69a..5c053e7 100644 --- a/zion/interrupt/interrupt.cpp +++ b/zion/interrupt/interrupt.cpp @@ -153,7 +153,7 @@ glcr::RefPtr pci1_port; extern "C" void isr_pci1(); extern "C" void interrupt_pci1(InterruptFrame*) { dbgln("Interrupt PCI line 1"); - pci1_port->Send(0, nullptr, 0, nullptr); + pci1_port->Send({}, {}); gApic->SignalEOI(); } diff --git a/zion/lib/message_queue.cpp b/zion/lib/message_queue.cpp index 9b3340c..6b53d70 100644 --- a/zion/lib/message_queue.cpp +++ b/zion/lib/message_queue.cpp @@ -3,30 +3,24 @@ #include "debug/debug.h" #include "scheduler/scheduler.h" -glcr::ErrorCode UnboundedMessageQueue::PushBack(uint64_t num_bytes, - const void* bytes, - uint64_t num_caps, - const z_cap_t* caps, - z_cap_t reply_cap) { - if (num_bytes > 0x1000) { - dbgln("Large message size unimplemented: %x", num_bytes); +glcr::ErrorCode UnboundedMessageQueue::PushBack( + const glcr::ArrayView& message, + const glcr::ArrayView& caps, z_cap_t reply_cap) { + if (message.size() > 0x1000) { + dbgln("Large message size unimplemented: %x", message.size()); return glcr::UNIMPLEMENTED; } - auto message = glcr::MakeShared(); - message->num_bytes = num_bytes; - message->bytes = new uint8_t[num_bytes]; - for (uint64_t i = 0; i < num_bytes; i++) { - message->bytes[i] = static_cast(bytes)[i]; - } + auto msg_struct = glcr::MakeShared(); + msg_struct->message = glcr::Array(message); if (reply_cap != kZionInvalidCapability) { // FIXME: We're just trusting that capability has the correct permissions. - message->reply_cap = + msg_struct->reply_cap = gScheduler->CurrentProcess().ReleaseCapability(reply_cap); } - for (uint64_t i = 0; i < num_caps; i++) { + for (uint64_t i = 0; i < caps.size(); i++) { // FIXME: This would feel safer closer to the relevant syscall. // FIXME: Race conditions on get->check->release here. Would be better to // have that as a single call on the process. (This pattern repeats other @@ -39,11 +33,11 @@ glcr::ErrorCode UnboundedMessageQueue::PushBack(uint64_t num_bytes, return glcr::CAP_PERMISSION_DENIED; } cap = gScheduler->CurrentProcess().ReleaseCapability(caps[i]); - message->caps.PushBack(cap); + msg_struct->caps.PushBack(cap); } MutexHolder h(mutex_); - pending_messages_.PushBack(message); + pending_messages_.PushBack(msg_struct); if (blocked_threads_.size() > 0) { auto thread = blocked_threads_.PopFront(); @@ -70,7 +64,7 @@ glcr::ErrorCode UnboundedMessageQueue::PopFront(uint64_t* num_bytes, MutexHolder lock(mutex_); auto next_msg = pending_messages_.PeekFront(); - if (next_msg->num_bytes > *num_bytes) { + if (next_msg->message.size() > *num_bytes) { return glcr::BUFFER_SIZE; } if (next_msg->caps.size() > *num_caps) { @@ -79,10 +73,10 @@ glcr::ErrorCode UnboundedMessageQueue::PopFront(uint64_t* num_bytes, next_msg = pending_messages_.PopFront(); - *num_bytes = next_msg->num_bytes; + *num_bytes = next_msg->message.size(); for (uint64_t i = 0; i < *num_bytes; i++) { - static_cast(bytes)[i] = next_msg->bytes[i]; + static_cast(bytes)[i] = next_msg->message[i]; } auto& proc = gScheduler->CurrentProcess(); @@ -105,40 +99,32 @@ void UnboundedMessageQueue::WriteKernel(uint64_t init, glcr::RefPtr cap) { // FIXME: Add synchronization here in case it is ever used outside of init. auto msg = glcr::MakeShared(); - msg->bytes = new uint8_t[8]; - msg->num_bytes = sizeof(init); + msg->message = glcr::Array(sizeof(init)); uint8_t* data = reinterpret_cast(&init); for (uint8_t i = 0; i < sizeof(init); i++) { - msg->bytes[i] = data[i]; + msg->message[i] = data[i]; } msg->caps.PushBack(cap); pending_messages_.PushBack(msg); } -glcr::ErrorCode SingleMessageQueue::PushBack(uint64_t num_bytes, - const void* bytes, - uint64_t num_caps, - const z_cap_t* caps, - z_cap_t reply_port) { +glcr::ErrorCode SingleMessageQueue::PushBack( + const glcr::ArrayView& message, + const glcr::ArrayView& caps, z_cap_t reply_port) { MutexHolder h(mutex_); if (has_written_) { return glcr::FAILED_PRECONDITION; } - num_bytes_ = num_bytes; - bytes_ = new uint8_t[num_bytes]; - - for (uint64_t i = 0; i < num_bytes; i++) { - bytes_[i] = reinterpret_cast(bytes)[i]; - } + message_ = message; if (reply_port != kZionInvalidCapability) { dbgln("Sent a reply port to a single message queue"); return glcr::INTERNAL; } - for (uint64_t i = 0; i < num_caps; i++) { + for (uint64_t i = 0; i < caps.size(); i++) { // FIXME: This would feel safer closer to the relevant syscall. auto cap = gScheduler->CurrentProcess().GetCapability(caps[i]); if (!cap) { @@ -181,16 +167,16 @@ glcr::ErrorCode SingleMessageQueue::PopFront(uint64_t* num_bytes, void* bytes, return glcr::FAILED_PRECONDITION; } - if (num_bytes_ > *num_bytes) { + if (message_.size() > *num_bytes) { return glcr::BUFFER_SIZE; } if (caps_.size() > *num_caps) { return glcr::BUFFER_SIZE; } - *num_bytes = num_bytes_; - for (uint64_t i = 0; i < num_bytes_; i++) { - reinterpret_cast(bytes)[i] = bytes_[i]; + *num_bytes = message_.size(); + for (uint64_t i = 0; i < message_.size(); i++) { + reinterpret_cast(bytes)[i] = message_[i]; } if (reply_port != nullptr) { diff --git a/zion/lib/message_queue.h b/zion/lib/message_queue.h index 3bd5733..d06dfac 100644 --- a/zion/lib/message_queue.h +++ b/zion/lib/message_queue.h @@ -1,5 +1,7 @@ #pragma once +#include +#include #include #include #include @@ -14,12 +16,12 @@ class MessageQueue { public: virtual ~MessageQueue() {} - virtual glcr::ErrorCode PushBack(uint64_t num_bytes, const void* bytes, - uint64_t num_caps, const z_cap_t* caps, - z_cap_t reply_cap = 0) = 0; + virtual glcr::ErrorCode PushBack(const glcr::ArrayView& message, + const glcr::ArrayView& caps, + z_cap_t reply_cap) = 0; virtual glcr::ErrorCode PopFront(uint64_t* num_bytes, void* bytes, uint64_t* num_caps, z_cap_t* caps, - z_cap_t* reply_cap = nullptr) = 0; + z_cap_t* reply_cap) = 0; virtual bool empty() = 0; protected: @@ -36,8 +38,8 @@ class UnboundedMessageQueue : public MessageQueue { UnboundedMessageQueue& operator=(const UnboundedMessageQueue&) = delete; virtual ~UnboundedMessageQueue() override {} - glcr::ErrorCode PushBack(uint64_t num_bytes, const void* bytes, - uint64_t num_caps, const z_cap_t* caps, + glcr::ErrorCode PushBack(const glcr::ArrayView& message, + const glcr::ArrayView& caps, z_cap_t reply_cap) override; glcr::ErrorCode PopFront(uint64_t* num_bytes, void* bytes, uint64_t* num_caps, z_cap_t* caps, z_cap_t* reply_cap) override; @@ -51,8 +53,7 @@ class UnboundedMessageQueue : public MessageQueue { private: struct Message { - uint64_t num_bytes; - uint8_t* bytes; + glcr::Array message; glcr::LinkedList> caps; glcr::RefPtr reply_cap; @@ -68,8 +69,8 @@ class SingleMessageQueue : public MessageQueue { SingleMessageQueue(SingleMessageQueue&&) = delete; virtual ~SingleMessageQueue() override {} - glcr::ErrorCode PushBack(uint64_t num_bytes, const void* bytes, - uint64_t num_caps, const z_cap_t* caps, + glcr::ErrorCode PushBack(const glcr::ArrayView& message, + const glcr::ArrayView& caps, z_cap_t reply_cap) override; glcr::ErrorCode PopFront(uint64_t* num_bytes, void* bytes, uint64_t* num_caps, z_cap_t* caps, z_cap_t* reply_cap) override; @@ -82,7 +83,6 @@ class SingleMessageQueue : public MessageQueue { private: bool has_written_ = false; bool has_read_ = false; - uint64_t num_bytes_; - uint8_t* bytes_; + glcr::Array message_; glcr::LinkedList> caps_; }; diff --git a/zion/object/endpoint.cpp b/zion/object/endpoint.cpp index 7036aae..7689876 100644 --- a/zion/object/endpoint.cpp +++ b/zion/object/endpoint.cpp @@ -5,18 +5,3 @@ glcr::RefPtr Endpoint::Create() { return glcr::AdoptPtr(new Endpoint); } - -glcr::ErrorCode Endpoint::Send(uint64_t num_bytes, const void* data, - uint64_t num_caps, const z_cap_t* caps, - z_cap_t reply_port_cap) { - auto& message_queue = GetSendMessageQueue(); - return message_queue.PushBack(num_bytes, data, num_caps, caps, - reply_port_cap); -} -glcr::ErrorCode Endpoint::Recv(uint64_t* num_bytes, void* data, - uint64_t* num_caps, z_cap_t* caps, - z_cap_t* reply_port_cap) { - auto& message_queue = GetRecvMessageQueue(); - return message_queue.PopFront(num_bytes, data, num_caps, caps, - reply_port_cap); -} diff --git a/zion/object/endpoint.h b/zion/object/endpoint.h index 20c706a..9112e4b 100644 --- a/zion/object/endpoint.h +++ b/zion/object/endpoint.h @@ -26,12 +26,6 @@ class Endpoint : public IpcObject { static glcr::RefPtr Create(); - // FIXME: These are hacky "almost" overrides that could lead to bugs. - glcr::ErrorCode Send(uint64_t num_bytes, const void* data, uint64_t num_caps, - const z_cap_t* caps, z_cap_t reply_port_cap); - glcr::ErrorCode Recv(uint64_t* num_bytes, void* data, uint64_t* num_caps, - z_cap_t* caps, z_cap_t* reply_port_cap); - virtual MessageQueue& GetSendMessageQueue() override { return message_queue_; } diff --git a/zion/object/ipc_object.cpp b/zion/object/ipc_object.cpp index a10f0fb..d82d178 100644 --- a/zion/object/ipc_object.cpp +++ b/zion/object/ipc_object.cpp @@ -2,14 +2,26 @@ #include "scheduler/scheduler.h" -glcr::ErrorCode IpcObject::Send(uint64_t num_bytes, const void* bytes, - uint64_t num_caps, const z_cap_t* caps) { +glcr::ErrorCode IpcObject::Send(const glcr::ArrayView& message, + const glcr::ArrayView& caps) { + return Send(message, caps, kZionInvalidCapability); +} + +glcr::ErrorCode IpcObject::Send(const glcr::ArrayView& message, + const glcr::ArrayView& caps, + const z_cap_t reply_port) { auto& message_queue = GetSendMessageQueue(); - return message_queue.PushBack(num_bytes, bytes, num_caps, caps); + return message_queue.PushBack(message, caps, reply_port); } glcr::ErrorCode IpcObject::Recv(uint64_t* num_bytes, void* bytes, uint64_t* num_caps, z_cap_t* caps) { - auto& message_queue = GetRecvMessageQueue(); - return message_queue.PopFront(num_bytes, bytes, num_caps, caps); + return Recv(num_bytes, bytes, num_caps, caps, nullptr); +} + +glcr::ErrorCode IpcObject::Recv(uint64_t* num_bytes, void* bytes, + uint64_t* num_caps, z_cap_t* caps, + z_cap_t* reply_port) { + auto& message_queue = GetRecvMessageQueue(); + return message_queue.PopFront(num_bytes, bytes, num_caps, caps, reply_port); } diff --git a/zion/object/ipc_object.h b/zion/object/ipc_object.h index 69595a7..cdc927f 100644 --- a/zion/object/ipc_object.h +++ b/zion/object/ipc_object.h @@ -11,10 +11,17 @@ class IpcObject : public KernelObject { IpcObject(){}; virtual ~IpcObject() {} - virtual glcr::ErrorCode Send(uint64_t num_bytes, const void* bytes, - uint64_t num_caps, const z_cap_t* caps) final; + virtual glcr::ErrorCode Send(const glcr::ArrayView& message, + const glcr::ArrayView& caps) final; + virtual glcr::ErrorCode Send(const glcr::ArrayView& message, + const glcr::ArrayView& caps, + const z_cap_t reply_port) final; + virtual glcr::ErrorCode Recv(uint64_t* num_bytes, void* bytes, uint64_t* num_caps, z_cap_t* caps) final; + virtual glcr::ErrorCode Recv(uint64_t* num_bytes, void* bytes, + uint64_t* num_caps, z_cap_t* caps, + z_cap_t* reply_port) final; bool HasMessages() { return !GetRecvMessageQueue().empty(); } diff --git a/zion/syscall/ipc.cpp b/zion/syscall/ipc.cpp index 5c40bfc..c234211 100644 --- a/zion/syscall/ipc.cpp +++ b/zion/syscall/ipc.cpp @@ -6,6 +6,14 @@ #include "object/reply_port.h" #include "scheduler/scheduler.h" +namespace { + +glcr::ArrayView Buffer(const void* bytes, uint64_t num_bytes) { + return glcr::ArrayView(reinterpret_cast(const_cast(bytes)), + num_bytes); +} +} // namespace + glcr::ErrorCode ChannelCreate(ZChannelCreateReq* req) { auto& proc = gScheduler->CurrentProcess(); auto chan_pair = Channel::CreateChannelPair(); @@ -20,7 +28,8 @@ glcr::ErrorCode ChannelSend(ZChannelSendReq* req) { RET_ERR(ValidateCapability(chan_cap, kZionPerm_Write)); auto chan = chan_cap->obj(); - return chan->Send(req->num_bytes, req->data, req->num_caps, req->caps); + return chan->Send(Buffer(req->data, req->num_bytes), + glcr::ArrayView(req->caps, req->num_caps)); } glcr::ErrorCode ChannelRecv(ZChannelRecvReq* req) { @@ -45,7 +54,8 @@ glcr::ErrorCode PortSend(ZPortSendReq* req) { RET_ERR(ValidateCapability(port_cap, kZionPerm_Write)); auto port = port_cap->obj(); - return port->Send(req->num_bytes, req->data, req->num_caps, req->caps); + return port->Send(Buffer(req->data, req->num_bytes), + glcr::ArrayView(req->caps, req->num_caps)); } glcr::ErrorCode PortRecv(ZPortRecvReq* req) { @@ -100,8 +110,10 @@ glcr::ErrorCode EndpointSend(ZEndpointSendReq* req) { *req->reply_port_cap = proc.AddNewCapability(reply_port, kZionPerm_Read); uint64_t reply_port_cap_to_send = proc.AddNewCapability(reply_port, kZionPerm_Write | kZionPerm_Transmit); - return endpoint->Send(req->num_bytes, req->data, req->num_caps, req->caps, - reply_port_cap_to_send); + return endpoint->Send( + Buffer(req->data, req->num_bytes), + glcr::ArrayView(const_cast(req->caps), req->num_caps), + reply_port_cap_to_send); } glcr::ErrorCode EndpointRecv(ZEndpointRecvReq* req) { @@ -126,7 +138,8 @@ glcr::ErrorCode ReplyPortSend(ZReplyPortSendReq* req) { ValidateCapability(reply_port_cap, kZionPerm_Read); auto reply_port = reply_port_cap->obj(); - return reply_port->Send(req->num_bytes, req->data, req->num_caps, req->caps); + return reply_port->Send(Buffer(req->data, req->num_bytes), + glcr::ArrayView(req->caps, req->num_caps)); } glcr::ErrorCode ReplyPortRecv(ZReplyPortRecvReq* req) { auto& proc = gScheduler->CurrentProcess();