From d9df1212b70b0f7c95bcf927db674c138160fe89 Mon Sep 17 00:00:00 2001 From: Drew Galbraith Date: Fri, 3 Nov 2023 00:37:53 -0700 Subject: [PATCH] [Zion] Pass data to message queue as IpcMessage obj. --- zion/interrupt/interrupt.cpp | 2 +- zion/lib/message_queue.cpp | 67 ++++++------------------------------ zion/lib/message_queue.h | 12 ++----- zion/object/ipc_object.cpp | 12 ++----- zion/object/ipc_object.h | 6 +--- zion/syscall/ipc.cpp | 55 ++++++++++++++++++++++------- 6 files changed, 60 insertions(+), 94 deletions(-) diff --git a/zion/interrupt/interrupt.cpp b/zion/interrupt/interrupt.cpp index 5c053e7..1a0cb2b 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({}, {}); + pci1_port->Send({}); gApic->SignalEOI(); } diff --git a/zion/lib/message_queue.cpp b/zion/lib/message_queue.cpp index b830947..3b619be 100644 --- a/zion/lib/message_queue.cpp +++ b/zion/lib/message_queue.cpp @@ -3,42 +3,9 @@ #include "debug/debug.h" #include "scheduler/scheduler.h" -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; - } - - IpcMessage msg_struct; - msg_struct.data = glcr::Array(message); - - if (reply_cap != kZionInvalidCapability) { - // FIXME: We're just trusting that capability has the correct permissions. - msg_struct.reply_cap = - gScheduler->CurrentProcess().ReleaseCapability(reply_cap); - } - - msg_struct.caps.Resize(caps.size()); - 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 - // places too). - auto cap = gScheduler->CurrentProcess().GetCapability(caps[i]); - if (!cap) { - return glcr::CAP_NOT_FOUND; - } - if (!cap->HasPermissions(kZionPerm_Transmit)) { - return glcr::CAP_PERMISSION_DENIED; - } - cap = gScheduler->CurrentProcess().ReleaseCapability(caps[i]); - msg_struct.caps.PushBack(cap); - } - +glcr::ErrorCode UnboundedMessageQueue::PushBack(IpcMessage&& message) { MutexHolder h(mutex_); - pending_messages_.PushBack(glcr::Move(msg_struct)); + pending_messages_.PushBack(glcr::Move(message)); if (blocked_threads_.size() > 0) { auto thread = blocked_threads_.PopFront(); @@ -88,34 +55,19 @@ void UnboundedMessageQueue::WriteKernel(uint64_t init, pending_messages_.PushBack(glcr::Move(msg)); } -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; - } - message_.data = message; - - if (reply_port != kZionInvalidCapability) { +glcr::ErrorCode SingleMessageQueue::PushBack(IpcMessage&& message) { + if (message.reply_cap) { dbgln("Sent a reply port to a single message queue"); return glcr::INTERNAL; } - message_.caps.Resize(caps.size()); - 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) { - return glcr::CAP_NOT_FOUND; - } - if (!cap->HasPermissions(kZionPerm_Transmit)) { - return glcr::CAP_PERMISSION_DENIED; - } - cap = gScheduler->CurrentProcess().ReleaseCapability(caps[i]); - message_.caps.PushBack(cap); + MutexHolder h(mutex_); + if (has_written_) { + dbgln("Double write to reply port."); + return glcr::FAILED_PRECONDITION; } + message_ = glcr::Move(message); has_written_ = true; if (blocked_threads_.size() > 0) { @@ -142,6 +94,7 @@ glcr::ErrorOr SingleMessageQueue::PopFront(uint64_t data_buf_size, MutexHolder lock(mutex_); if (has_read_) { + dbgln("Double read from reply port."); return glcr::FAILED_PRECONDITION; } diff --git a/zion/lib/message_queue.h b/zion/lib/message_queue.h index af6c025..35636ce 100644 --- a/zion/lib/message_queue.h +++ b/zion/lib/message_queue.h @@ -26,9 +26,7 @@ class MessageQueue { public: virtual ~MessageQueue() {} - virtual glcr::ErrorCode PushBack(const glcr::ArrayView& message, - const glcr::ArrayView& caps, - z_cap_t reply_cap) = 0; + virtual glcr::ErrorCode PushBack(IpcMessage&&) = 0; virtual glcr::ErrorOr PopFront(uint64_t data_buf_size, uint64_t cap_buf_size) = 0; virtual bool empty() = 0; @@ -47,9 +45,7 @@ class UnboundedMessageQueue : public MessageQueue { UnboundedMessageQueue& operator=(const UnboundedMessageQueue&) = delete; virtual ~UnboundedMessageQueue() override {} - glcr::ErrorCode PushBack(const glcr::ArrayView& message, - const glcr::ArrayView& caps, - z_cap_t reply_cap) override; + glcr::ErrorCode PushBack(IpcMessage&& message) override; glcr::ErrorOr PopFront(uint64_t data_buf_size, uint64_t cap_buf_size) override; @@ -71,9 +67,7 @@ class SingleMessageQueue : public MessageQueue { SingleMessageQueue(SingleMessageQueue&&) = delete; virtual ~SingleMessageQueue() override {} - glcr::ErrorCode PushBack(const glcr::ArrayView& message, - const glcr::ArrayView& caps, - z_cap_t reply_cap) override; + glcr::ErrorCode PushBack(IpcMessage&&) override; glcr::ErrorOr PopFront(uint64_t data_buf_size, uint64_t cap_buf_size) override; diff --git a/zion/object/ipc_object.cpp b/zion/object/ipc_object.cpp index fed7c72..174cc87 100644 --- a/zion/object/ipc_object.cpp +++ b/zion/object/ipc_object.cpp @@ -2,16 +2,8 @@ #include "scheduler/scheduler.h" -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(message, caps, reply_port); +glcr::ErrorCode IpcObject::Send(IpcMessage&& message) { + return GetSendMessageQueue().PushBack(glcr::Move(message)); } glcr::ErrorOr IpcObject::Recv(uint64_t data_buf_size, diff --git a/zion/object/ipc_object.h b/zion/object/ipc_object.h index 9f78618..c50abc5 100644 --- a/zion/object/ipc_object.h +++ b/zion/object/ipc_object.h @@ -11,11 +11,7 @@ class IpcObject : public KernelObject { IpcObject(){}; virtual ~IpcObject() {} - 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 Send(IpcMessage&& message) final; virtual glcr::ErrorOr Recv(uint64_t data_buf_size, uint64_t cap_buf_size) final; diff --git a/zion/syscall/ipc.cpp b/zion/syscall/ipc.cpp index 28a25b1..9c0ada3 100644 --- a/zion/syscall/ipc.cpp +++ b/zion/syscall/ipc.cpp @@ -14,6 +14,38 @@ glcr::ArrayView Buffer(const void* bytes, uint64_t num_bytes) { num_bytes); } +template +glcr::ErrorOr TranslateRequestToIpcMessage(const T& req) { + if (req.num_bytes > 0x1000) { + dbgln("Large message size unimplemented: %x", req.num_bytes); + return glcr::UNIMPLEMENTED; + } + + IpcMessage message; + message.data = Buffer(req.data, req.num_bytes); + + glcr::ArrayView caps(req.caps, req.num_caps); + + message.caps.Resize(caps.size()); + 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 + // places too). + auto cap = gScheduler->CurrentProcess().GetCapability(caps[i]); + if (!cap) { + return glcr::CAP_NOT_FOUND; + } + if (!cap->HasPermissions(kZionPerm_Transmit)) { + return glcr::CAP_PERMISSION_DENIED; + } + message.caps.PushBack( + gScheduler->CurrentProcess().ReleaseCapability(caps[i])); + } + + return message; +} + template glcr::ErrorCode TranslateIpcMessageToResponse(const IpcMessage& message, T* resp) { @@ -67,8 +99,8 @@ glcr::ErrorCode ChannelSend(ZChannelSendReq* req) { RET_ERR(ValidateCapability(chan_cap, kZionPerm_Write)); auto chan = chan_cap->obj(); - return chan->Send(Buffer(req->data, req->num_bytes), - glcr::ArrayView(req->caps, req->num_caps)); + ASSIGN_OR_RETURN(IpcMessage message, TranslateRequestToIpcMessage(*req)); + return chan->Send(glcr::Move(message)); } glcr::ErrorCode ChannelRecv(ZChannelRecvReq* req) { @@ -94,8 +126,8 @@ glcr::ErrorCode PortSend(ZPortSendReq* req) { RET_ERR(ValidateCapability(port_cap, kZionPerm_Write)); auto port = port_cap->obj(); - return port->Send(Buffer(req->data, req->num_bytes), - glcr::ArrayView(req->caps, req->num_caps)); + ASSIGN_OR_RETURN(IpcMessage message, TranslateRequestToIpcMessage(*req)); + return port->Send(glcr::Move(message)); } glcr::ErrorCode PortRecv(ZPortRecvReq* req) { @@ -150,12 +182,11 @@ glcr::ErrorCode EndpointSend(ZEndpointSendReq* req) { auto reply_port = ReplyPort::Create(); *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( - Buffer(req->data, req->num_bytes), - glcr::ArrayView(const_cast(req->caps), req->num_caps), - reply_port_cap_to_send); + + ASSIGN_OR_RETURN(IpcMessage message, TranslateRequestToIpcMessage(*req)); + message.reply_cap = glcr::MakeRefCounted( + reply_port, kZionPerm_Write | kZionPerm_Transmit); + return endpoint->Send(glcr::Move(message)); } glcr::ErrorCode EndpointRecv(ZEndpointRecvReq* req) { @@ -176,8 +207,8 @@ glcr::ErrorCode ReplyPortSend(ZReplyPortSendReq* req) { ValidateCapability(reply_port_cap, kZionPerm_Read); auto reply_port = reply_port_cap->obj(); - return reply_port->Send(Buffer(req->data, req->num_bytes), - glcr::ArrayView(req->caps, req->num_caps)); + ASSIGN_OR_RETURN(IpcMessage message, TranslateRequestToIpcMessage(*req)); + return reply_port->Send(glcr::Move(message)); } glcr::ErrorCode ReplyPortRecv(ZReplyPortRecvReq* req) { auto& proc = gScheduler->CurrentProcess();