Merge pull request #188 from bunnei/refactor-buffer-descriptor

Refactor IPC buffer descriptor interface

src/core/hle/ipc.h

@@ -91,6 +91,10 @@ struct BufferDescriptorX {
         address |= static_cast<VAddr>(address_bits_36_38) << 36;
         return address;
     }
+
+    u64 Size() const {
+        return static_cast<u64>(size);
+    }
 };
 static_assert(sizeof(BufferDescriptorX) == 8, "BufferDescriptorX size is incorrect");
 

src/core/hle/kernel/hle_ipc.cpp

@@ -12,6 +12,7 @@
 #include "core/hle/kernel/kernel.h"
 #include "core/hle/kernel/process.h"
 #include "core/hle/kernel/server_session.h"
+#include "core/memory.h"
 
 namespace Kernel {
 
@@ -210,4 +211,47 @@ ResultCode HLERequestContext::WriteToOutgoingCommandBuffer(u32_le* dst_cmdbuf, P
     return RESULT_SUCCESS;
 }
 
+std::vector<u8> HLERequestContext::ReadBuffer() const {
+    std::vector<u8> buffer;
+    const bool is_buffer_a{BufferDescriptorA().size() && BufferDescriptorA()[0].Size()};
+
+    if (is_buffer_a) {
+        buffer.resize(BufferDescriptorA()[0].Size());
+        Memory::ReadBlock(BufferDescriptorA()[0].Address(), buffer.data(), buffer.size());
+    } else {
+        buffer.resize(BufferDescriptorX()[0].Size());
+        Memory::ReadBlock(BufferDescriptorX()[0].Address(), buffer.data(), buffer.size());
+    }
+
+    return buffer;
+}
+
+size_t HLERequestContext::WriteBuffer(const void* buffer, size_t size) const {
+    const bool is_buffer_b{BufferDescriptorB().size() && BufferDescriptorB()[0].Size()};
+
+    ASSERT_MSG(size <= GetWriteBufferSize(), "Size %d is too big", size);
+
+    if (is_buffer_b) {
+        Memory::WriteBlock(BufferDescriptorB()[0].Address(), buffer, size);
+    } else {
+        Memory::WriteBlock(BufferDescriptorC()[0].Address(), buffer, size);
+    }
+
+    return size;
+}
+
+size_t HLERequestContext::WriteBuffer(const std::vector<u8>& buffer) const {
+    return WriteBuffer(buffer.data(), buffer.size());
+}
+
+size_t HLERequestContext::GetReadBufferSize() const {
+    const bool is_buffer_a{BufferDescriptorA().size() && BufferDescriptorA()[0].Size()};
+    return is_buffer_a ? BufferDescriptorA()[0].Size() : BufferDescriptorX()[0].Size();
+}
+
+size_t HLERequestContext::GetWriteBufferSize() const {
+    const bool is_buffer_b{BufferDescriptorB().size() && BufferDescriptorB()[0].Size()};
+    return is_buffer_b ? BufferDescriptorB()[0].Size() : BufferDescriptorC()[0].Size();
+}
+
 } // namespace Kernel

src/core/hle/kernel/hle_ipc.h

@@ -143,6 +143,21 @@ public:
         return domain_message_header;
     }
 
+    /// Helper function to read a buffer using the appropriate buffer descriptor
+    std::vector<u8> ReadBuffer() const;
+
+    /// Helper function to write a buffer using the appropriate buffer descriptor
+    size_t WriteBuffer(const void* buffer, size_t size) const;
+
+    /// Helper function to write a buffer using the appropriate buffer descriptor
+    size_t WriteBuffer(const std::vector<u8>& buffer) const;
+
+    /// Helper function to get the size of the input buffer
+    size_t GetReadBufferSize() const;
+
+    /// Helper function to get the size of the output buffer
+    size_t GetWriteBufferSize() const;
+
     template <typename T>
     SharedPtr<T> GetCopyObject(size_t index) {
         ASSERT(index < copy_objects.size());

src/core/hle/service/acc/acc_u0.cpp

@@ -66,8 +66,7 @@ void ACC_U0::GetUserExistence(Kernel::HLERequestContext& ctx) {
 
 void ACC_U0::ListAllUsers(Kernel::HLERequestContext& ctx) {
     constexpr std::array<u128, 10> user_ids{DEFAULT_USER_ID};
-    const auto& output_buffer = ctx.BufferDescriptorC()[0];
+    ctx.WriteBuffer(user_ids.data(), user_ids.size());
-    Memory::WriteBlock(output_buffer.Address(), user_ids.data(), user_ids.size());
     IPC::ResponseBuilder rb{ctx, 2};
     rb.Push(RESULT_SUCCESS);
     LOG_DEBUG(Service_ACC, "called");

src/core/hle/service/am/am.cpp

@@ -306,11 +306,11 @@ private:
 
         u64 offset = rp.Pop<u64>();
 
-        const auto& output_buffer = ctx.BufferDescriptorC()[0];
+        const size_t size{ctx.GetWriteBufferSize()};
 
-        ASSERT(offset + output_buffer.Size() <= buffer.size());
+        ASSERT(offset + size <= buffer.size());
 
-        Memory::WriteBlock(output_buffer.Address(), buffer.data() + offset, output_buffer.Size());
+        ctx.WriteBuffer(buffer.data() + offset, size);
 
         IPC::ResponseBuilder rb{ctx, 2};
 

src/core/hle/service/audio/audout_u.cpp

@@ -99,8 +99,6 @@ private:
     void GetReleasedAudioOutBuffer_1(Kernel::HLERequestContext& ctx) {
         LOG_WARNING(Service_Audio, "(STUBBED) called");
 
-        const auto& buffer = ctx.BufferDescriptorB()[0];
-
         // TODO(st4rk): This is how libtransistor currently implements the
         // GetReleasedAudioOutBuffer, it should return the key (a VAddr) to the app and this address
         // is used to know which buffer should be filled with data and send again to the service
@@ -112,7 +110,7 @@ private:
             queue_keys.pop_back();
         }
 
-        Memory::WriteBlock(buffer.Address(), &key, sizeof(u64));
+        ctx.WriteBuffer(&key, sizeof(u64));
 
         IPC::ResponseBuilder rb{ctx, 3};
         rb.Push(RESULT_SUCCESS);
@@ -158,10 +156,8 @@ void AudOutU::ListAudioOuts(Kernel::HLERequestContext& ctx) {
     LOG_WARNING(Service_Audio, "(STUBBED) called");
     IPC::RequestParser rp{ctx};
 
-    auto& buffer = ctx.BufferDescriptorB()[0];
     const std::string audio_interface = "AudioInterface";
-
+    ctx.WriteBuffer(audio_interface.c_str(), audio_interface.size());
-    Memory::WriteBlock(buffer.Address(), &audio_interface[0], audio_interface.size());
 
     IPC::ResponseBuilder rb = rp.MakeBuilder(3, 0, 0);
 

src/core/hle/service/audio/audren_u.cpp

@@ -69,9 +69,7 @@ private:
             response_data.state_entries[i].state = 5;
         }
 
-        auto& buffer = ctx.BufferDescriptorB()[0];
+        ctx.WriteBuffer(&response_data, response_data.total_size);
-
-        Memory::WriteBlock(buffer.Address(), &response_data, response_data.total_size);
 
         IPC::ResponseBuilder rb{ctx, 2};
 

src/core/hle/service/filesystem/fsp_srv.cpp

@@ -33,12 +33,10 @@ private:
         IPC::RequestParser rp{ctx};
         const s64 offset = rp.Pop<s64>();
         const s64 length = rp.Pop<s64>();
-        const auto& descriptor = ctx.BufferDescriptorB()[0];
 
         LOG_DEBUG(Service_FS, "called, offset=0x%llx, length=0x%llx", offset, length);
 
         // Error checking
-        ASSERT_MSG(length == descriptor.Size(), "unexpected size difference");
         if (length < 0) {
             IPC::ResponseBuilder rb{ctx, 2};
             rb.Push(ResultCode(ErrorModule::FS, ErrorDescription::InvalidLength));
@@ -60,7 +58,7 @@ private:
         }
 
         // Write the data to memory
-        Memory::WriteBlock(descriptor.Address(), output.data(), descriptor.Size());
+        ctx.WriteBuffer(output);
 
         IPC::ResponseBuilder rb{ctx, 2};
         rb.Push(RESULT_SUCCESS);

src/core/hle/service/nvdrv/interface.cpp

@@ -15,9 +15,8 @@ namespace Nvidia {
 void NVDRV::Open(Kernel::HLERequestContext& ctx) {
     LOG_DEBUG(Service_NVDRV, "called");
 
-    auto buffer = ctx.BufferDescriptorA()[0];
+    const auto& buffer = ctx.ReadBuffer();
-
+    std::string device_name(buffer.begin(), buffer.end());
-    std::string device_name = Memory::ReadCString(buffer.Address(), buffer.Size());
 
     u32 fd = nvdrv->Open(device_name);
     IPC::ResponseBuilder rb{ctx, 4};
@@ -33,25 +32,13 @@ void NVDRV::Ioctl(Kernel::HLERequestContext& ctx) {
     u32 fd = rp.Pop<u32>();
     u32 command = rp.Pop<u32>();
 
+    std::vector<u8> output(ctx.GetWriteBufferSize());
+
     IPC::ResponseBuilder rb{ctx, 3};
     rb.Push(RESULT_SUCCESS);
-    if (ctx.BufferDescriptorA()[0].Size() != 0) {
+    rb.Push(nvdrv->Ioctl(fd, command, ctx.ReadBuffer(), output));
-        auto input_buffer = ctx.BufferDescriptorA()[0];
+
-        auto output_buffer = ctx.BufferDescriptorB()[0];
+    ctx.WriteBuffer(output);
-        std::vector<u8> input(input_buffer.Size());
-        std::vector<u8> output(output_buffer.Size());
-        Memory::ReadBlock(input_buffer.Address(), input.data(), input_buffer.Size());
-        rb.Push(nvdrv->Ioctl(fd, command, input, output));
-        Memory::WriteBlock(output_buffer.Address(), output.data(), output_buffer.Size());
-    } else {
-        auto input_buffer = ctx.BufferDescriptorX()[0];
-        auto output_buffer = ctx.BufferDescriptorC()[0];
-        std::vector<u8> input(input_buffer.size);
-        std::vector<u8> output(output_buffer.size);
-        Memory::ReadBlock(input_buffer.Address(), input.data(), input_buffer.size);
-        rb.Push(nvdrv->Ioctl(fd, command, input, output));
-        Memory::WriteBlock(output_buffer.Address(), output.data(), output_buffer.size);
-    }
 }
 
 void NVDRV::Close(Kernel::HLERequestContext& ctx) {

src/core/hle/service/set/set.cpp

@@ -17,9 +17,7 @@ void SET::GetAvailableLanguageCodes(Kernel::HLERequestContext& ctx) {
     u32 id = rp.Pop<u32>();
     constexpr std::array<u8, 13> lang_codes{};
 
-    const auto& output_buffer = ctx.BufferDescriptorC()[0];
+    ctx.WriteBuffer(lang_codes.data(), lang_codes.size());
-
-    Memory::WriteBlock(output_buffer.Address(), lang_codes.data(), lang_codes.size());
 
     IPC::ResponseBuilder rb{ctx, 2};
 

src/core/hle/service/vi/vi.cpp

@@ -429,7 +429,7 @@ public:
             {0, &IHOSBinderDriver::TransactParcel, "TransactParcel"},
             {1, &IHOSBinderDriver::AdjustRefcount, "AdjustRefcount"},
             {2, &IHOSBinderDriver::GetNativeHandle, "GetNativeHandle"},
-            {3, &IHOSBinderDriver::TransactParcelAuto, "TransactParcelAuto"},
+            {3, &IHOSBinderDriver::TransactParcel, "TransactParcelAuto"},
         };
         RegisterHandlers(functions);
     }
@@ -453,96 +453,62 @@ private:
         SetPreallocatedBuffer = 14
     };
 
-    void TransactParcel(u32 id, TransactionId transaction, const std::vector<u8>& input_data,
-                        VAddr output_addr, u64 output_size) {
-        auto buffer_queue = nv_flinger->GetBufferQueue(id);
-
-        if (transaction == TransactionId::Connect) {
-            IGBPConnectRequestParcel request{input_data};
-            IGBPConnectResponseParcel response{1280, 720};
-            std::vector<u8> response_buffer = response.Serialize();
-            Memory::WriteBlock(output_addr, response_buffer.data(), response_buffer.size());
-        } else if (transaction == TransactionId::SetPreallocatedBuffer) {
-            IGBPSetPreallocatedBufferRequestParcel request{input_data};
-
-            buffer_queue->SetPreallocatedBuffer(request.data.slot, request.buffer);
-
-            IGBPSetPreallocatedBufferResponseParcel response{};
-            std::vector<u8> response_buffer = response.Serialize();
-            Memory::WriteBlock(output_addr, response_buffer.data(), response_buffer.size());
-        } else if (transaction == TransactionId::DequeueBuffer) {
-            IGBPDequeueBufferRequestParcel request{input_data};
-
-            u32 slot = buffer_queue->DequeueBuffer(request.data.pixel_format, request.data.width,
-                                                   request.data.height);
-
-            IGBPDequeueBufferResponseParcel response{slot};
-            std::vector<u8> response_buffer = response.Serialize();
-            Memory::WriteBlock(output_addr, response_buffer.data(), response_buffer.size());
-        } else if (transaction == TransactionId::RequestBuffer) {
-            IGBPRequestBufferRequestParcel request{input_data};
-
-            auto& buffer = buffer_queue->RequestBuffer(request.slot);
-
-            IGBPRequestBufferResponseParcel response{buffer};
-            std::vector<u8> response_buffer = response.Serialize();
-            Memory::WriteBlock(output_addr, response_buffer.data(), response_buffer.size());
-        } else if (transaction == TransactionId::QueueBuffer) {
-            IGBPQueueBufferRequestParcel request{input_data};
-
-            buffer_queue->QueueBuffer(request.data.slot, request.data.transform);
-
-            IGBPQueueBufferResponseParcel response{1280, 720};
-            std::vector<u8> response_buffer = response.Serialize();
-            Memory::WriteBlock(output_addr, response_buffer.data(), response_buffer.size());
-        } else if (transaction == TransactionId::Query) {
-            IGBPQueryRequestParcel request{input_data};
-
-            u32 value =
-                buffer_queue->Query(static_cast<NVFlinger::BufferQueue::QueryType>(request.type));
-
-            IGBPQueryResponseParcel response{value};
-            std::vector<u8> response_buffer = response.Serialize();
-            Memory::WriteBlock(output_addr, response_buffer.data(), response_buffer.size());
-        } else if (transaction == TransactionId::CancelBuffer) {
-            LOG_WARNING(Service_VI, "(STUBBED) called, transaction=CancelBuffer");
-        } else {
-            ASSERT_MSG(false, "Unimplemented");
-        }
-    }
-
     void TransactParcel(Kernel::HLERequestContext& ctx) {
         IPC::RequestParser rp{ctx};
         u32 id = rp.Pop<u32>();
         auto transaction = static_cast<TransactionId>(rp.Pop<u32>());
         u32 flags = rp.Pop<u32>();
+        auto buffer_queue = nv_flinger->GetBufferQueue(id);
+
         LOG_DEBUG(Service_VI, "called, transaction=%x", transaction);
 
-        auto& input_buffer = ctx.BufferDescriptorA()[0];
+        if (transaction == TransactionId::Connect) {
-        auto& output_buffer = ctx.BufferDescriptorB()[0];
+            IGBPConnectRequestParcel request{ctx.ReadBuffer()};
-        std::vector<u8> input_data(input_buffer.Size());
+            IGBPConnectResponseParcel response{1280, 720};
-        Memory::ReadBlock(input_buffer.Address(), input_data.data(), input_buffer.Size());
+            ctx.WriteBuffer(response.Serialize());
+        } else if (transaction == TransactionId::SetPreallocatedBuffer) {
+            IGBPSetPreallocatedBufferRequestParcel request{ctx.ReadBuffer()};
 
-        TransactParcel(id, transaction, input_data, output_buffer.Address(), output_buffer.Size());
+            buffer_queue->SetPreallocatedBuffer(request.data.slot, request.buffer);
 
-        IPC::ResponseBuilder rb{ctx, 2};
+            IGBPSetPreallocatedBufferResponseParcel response{};
-        rb.Push(RESULT_SUCCESS);
+            ctx.WriteBuffer(response.Serialize());
+        } else if (transaction == TransactionId::DequeueBuffer) {
+            IGBPDequeueBufferRequestParcel request{ctx.ReadBuffer()};
+
+            u32 slot = buffer_queue->DequeueBuffer(request.data.pixel_format, request.data.width,
+                                                   request.data.height);
+
+            IGBPDequeueBufferResponseParcel response{slot};
+            ctx.WriteBuffer(response.Serialize());
+        } else if (transaction == TransactionId::RequestBuffer) {
+            IGBPRequestBufferRequestParcel request{ctx.ReadBuffer()};
+
+            auto& buffer = buffer_queue->RequestBuffer(request.slot);
+
+            IGBPRequestBufferResponseParcel response{buffer};
+            ctx.WriteBuffer(response.Serialize());
+        } else if (transaction == TransactionId::QueueBuffer) {
+            IGBPQueueBufferRequestParcel request{ctx.ReadBuffer()};
+
+            buffer_queue->QueueBuffer(request.data.slot, request.data.transform);
+
+            IGBPQueueBufferResponseParcel response{1280, 720};
+            ctx.WriteBuffer(response.Serialize());
+        } else if (transaction == TransactionId::Query) {
+            IGBPQueryRequestParcel request{ctx.ReadBuffer()};
+
+            u32 value =
+                buffer_queue->Query(static_cast<NVFlinger::BufferQueue::QueryType>(request.type));
+
+            IGBPQueryResponseParcel response{value};
+            ctx.WriteBuffer(response.Serialize());
+        } else if (transaction == TransactionId::CancelBuffer) {
+            LOG_WARNING(Service_VI, "(STUBBED) called, transaction=CancelBuffer");
+        } else {
+            ASSERT_MSG(false, "Unimplemented");
         }
 
-    void TransactParcelAuto(Kernel::HLERequestContext& ctx) {
-        IPC::RequestParser rp{ctx};
-        u32 id = rp.Pop<u32>();
-        auto transaction = static_cast<TransactionId>(rp.Pop<u32>());
-        u32 flags = rp.Pop<u32>();
-        LOG_DEBUG(Service_VI, "called, transaction=%x", transaction);
-
-        auto& input_buffer = ctx.BufferDescriptorX()[0];
-        auto& output_buffer = ctx.BufferDescriptorC()[0];
-        std::vector<u8> input_data(input_buffer.size);
-        Memory::ReadBlock(input_buffer.Address(), input_data.data(), input_buffer.size);
-
-        TransactParcel(id, transaction, input_data, output_buffer.Address(), output_buffer.Size());
-
         IPC::ResponseBuilder rb{ctx, 2};
         rb.Push(RESULT_SUCCESS);
     }
@@ -719,18 +685,13 @@ void IApplicationDisplayService::OpenLayer(Kernel::HLERequestContext& ctx) {
     u64 layer_id = rp.Pop<u64>();
     u64 aruid = rp.Pop<u64>();
 
-    auto& buffer = ctx.BufferDescriptorB()[0];
-
     u64 display_id = nv_flinger->OpenDisplay(display_name);
     u32 buffer_queue_id = nv_flinger->GetBufferQueueId(display_id, layer_id);
 
     NativeWindow native_window{buffer_queue_id};
-    auto data = native_window.Serialize();
-    Memory::WriteBlock(buffer.Address(), data.data(), data.size());
-
     IPC::ResponseBuilder rb = rp.MakeBuilder(4, 0, 0);
     rb.Push(RESULT_SUCCESS);
-    rb.Push<u64>(data.size());
+    rb.Push<u64>(ctx.WriteBuffer(native_window.Serialize()));
 }
 
 void IApplicationDisplayService::CreateStrayLayer(Kernel::HLERequestContext& ctx) {
@@ -741,21 +702,16 @@ void IApplicationDisplayService::CreateStrayLayer(Kernel::HLERequestContext& ctx
     rp.Pop<u32>(); // padding
     u64 display_id = rp.Pop<u64>();
 
-    auto& buffer = ctx.BufferDescriptorB()[0];
-
     // TODO(Subv): What's the difference between a Stray and a Managed layer?
 
     u64 layer_id = nv_flinger->CreateLayer(display_id);
     u32 buffer_queue_id = nv_flinger->GetBufferQueueId(display_id, layer_id);
 
     NativeWindow native_window{buffer_queue_id};
-    auto data = native_window.Serialize();
-    Memory::WriteBlock(buffer.Address(), data.data(), data.size());
-
     IPC::ResponseBuilder rb = rp.MakeBuilder(6, 0, 0);
     rb.Push(RESULT_SUCCESS);
     rb.Push(layer_id);
-    rb.Push<u64>(data.size());
+    rb.Push<u64>(ctx.WriteBuffer(native_window.Serialize()));
 }
 
 void IApplicationDisplayService::DestroyStrayLayer(Kernel::HLERequestContext& ctx) {
@@ -781,8 +737,7 @@ void IApplicationDisplayService::SetLayerScalingMode(Kernel::HLERequestContext&
 void IApplicationDisplayService::ListDisplays(Kernel::HLERequestContext& ctx) {
     IPC::RequestParser rp{ctx};
     DisplayInfo display_info;
-    auto& buffer = ctx.BufferDescriptorB()[0];
+    ctx.WriteBuffer(&display_info, sizeof(DisplayInfo));
-    Memory::WriteBlock(buffer.Address(), &display_info, sizeof(DisplayInfo));
     IPC::ResponseBuilder rb = rp.MakeBuilder(4, 0, 0);
     rb.Push(RESULT_SUCCESS);
     rb.Push<u64>(1);