Merge pull request #740 from yuriks/gsp-shmem

Fix crashes due to un-initialized GSP shared memory

7 files changed, 67 insertions, 34 deletions

diff --git a/src/core/hle/kernel/shared_memory.cpp b/src/core/hle/kernel/shared_memory.cpp
index cb5c16696..0c59f4876 100644
--- a/src/core/hle/kernel/shared_memory.cpp
+++ b/src/core/hle/kernel/shared_memory.cpp
@@ -2,6 +2,8 @@
 // Licensed under GPLv2 or any later version
 // Refer to the license.txt file included.
 
+#include <cstring>
+
 #include "common/logging/log.h"
 
 #include "core/mem_map.h"
@@ -12,11 +14,15 @@ namespace Kernel {
 SharedMemory::SharedMemory() {}
 SharedMemory::~SharedMemory() {}
 
-SharedPtr<SharedMemory> SharedMemory::Create(std::string name) {
+SharedPtr<SharedMemory> SharedMemory::Create(u32 size, MemoryPermission permissions,
+        MemoryPermission other_permissions, std::string name) {
     SharedPtr<SharedMemory> shared_memory(new SharedMemory);
 
     shared_memory->name = std::move(name);
     shared_memory->base_address = 0x0;
+    shared_memory->size = size;
+    shared_memory->permissions = permissions;
+    shared_memory->other_permissions = other_permissions;
 
     return shared_memory;
 }
@@ -24,7 +30,7 @@ SharedPtr<SharedMemory> SharedMemory::Create(std::string name) {
 ResultCode SharedMemory::Map(VAddr address, MemoryPermission permissions,
         MemoryPermission other_permissions) {
 
-    if (address < Memory::SHARED_MEMORY_VADDR || address >= Memory::SHARED_MEMORY_VADDR_END) {
+    if (address < Memory::SHARED_MEMORY_VADDR || address + size >= Memory::SHARED_MEMORY_VADDR_END) {
         LOG_ERROR(Kernel, "cannot map id=%u, address=0x%08X outside of shared mem bounds!",
                 GetObjectId(), address);
         // TODO: Verify error code with hardware
@@ -32,21 +38,25 @@ ResultCode SharedMemory::Map(VAddr address, MemoryPermission permissions,
                 ErrorSummary::InvalidArgument, ErrorLevel::Permanent);
     }
 
+    // TODO: Test permissions
+
+    // HACK: Since there's no way to write to the memory block without mapping it onto the game
+    // process yet, at least initialize memory the first time it's mapped.
+    if (address != this->base_address) {
+        std::memset(Memory::GetPointer(address), 0, size);
+    }
+
     this->base_address = address;
-    this->permissions = permissions;
-    this->other_permissions = other_permissions;
 
     return RESULT_SUCCESS;
 }
 
-ResultVal<u8*> SharedMemory::GetPointer(u32 offset) {
+u8* SharedMemory::GetPointer(u32 offset) {
     if (base_address != 0)
-        return MakeResult<u8*>(Memory::GetPointer(base_address + offset));
+        return Memory::GetPointer(base_address + offset);
 
     LOG_ERROR(Kernel_SVC, "memory block id=%u not mapped!", GetObjectId());
-    // TODO(yuriks): Verify error code.
-    return ResultCode(ErrorDescription::InvalidAddress, ErrorModule::Kernel,
-            ErrorSummary::InvalidState, ErrorLevel::Permanent);
+    return nullptr;
 }
 
 } // namespace
diff --git a/src/core/hle/kernel/shared_memory.h b/src/core/hle/kernel/shared_memory.h
index 5833b411c..204266896 100644
--- a/src/core/hle/kernel/shared_memory.h
+++ b/src/core/hle/kernel/shared_memory.h
@@ -27,11 +27,16 @@ class SharedMemory final : public Object {
 public:
     /**
      * Creates a shared memory object
-     * @param name Optional object name, used only for debugging purposes.
+     * @param size Size of the memory block. Must be page-aligned.
+     * @param permissions Permission restrictions applied to the process which created the block.
+     * @param other_permissions Permission restrictions applied to other processes mapping the block.
+     * @param name Optional object name, used for debugging purposes.
      */
-    static SharedPtr<SharedMemory> Create(std::string name = "Unknown");
+    static SharedPtr<SharedMemory> Create(u32 size, MemoryPermission permissions,
+            MemoryPermission other_permissions, std::string name = "Unknown");
 
     std::string GetTypeName() const override { return "SharedMemory"; }
+    std::string GetName() const override { return name; }
 
     static const HandleType HANDLE_TYPE = HandleType::SharedMemory;
     HandleType GetHandleType() const override { return HANDLE_TYPE; }
@@ -49,12 +54,18 @@ public:
     * @param offset Offset from the start of the shared memory block to get pointer
     * @return Pointer to the shared memory block from the specified offset
     */
-    ResultVal<u8*> GetPointer(u32 offset = 0);
+    u8* GetPointer(u32 offset = 0);
 
-    VAddr base_address;                 ///< Address of shared memory block in RAM
-    MemoryPermission permissions;       ///< Permissions of shared memory block (SVC field)
-    MemoryPermission other_permissions; ///< Other permissions of shared memory block (SVC field)
-    std::string name;                   ///< Name of shared memory object (optional)
+    /// Address of shared memory block in the process.
+    VAddr base_address;
+    /// Size of the memory block. Page-aligned.
+    u32 size;
+    /// Permission restrictions applied to the process which created the block.
+    MemoryPermission permissions;
+    /// Permission restrictions applied to other processes mapping the block.
+    MemoryPermission other_permissions;
+    /// Name of shared memory object.
+    std::string name;
 
 private:
     SharedMemory();
diff --git a/src/core/hle/service/apt/apt.cpp b/src/core/hle/service/apt/apt.cpp
index 560c9dcf6..09d463dd5 100644
--- a/src/core/hle/service/apt/apt.cpp
+++ b/src/core/hle/service/apt/apt.cpp
@@ -304,7 +304,9 @@ void Init() {
         file.ReadBytes(shared_font.data(), (size_t)file.GetSize());
 
         // Create shared font memory object
-        shared_font_mem = Kernel::SharedMemory::Create("APT_U:shared_font_mem");
+        using Kernel::MemoryPermission;
+        shared_font_mem = Kernel::SharedMemory::Create(3 * 1024 * 1024, // 3MB
+                MemoryPermission::ReadWrite, MemoryPermission::Read, "APT_U:shared_font_mem");
     } else {
         LOG_WARNING(Service_APT, "Unable to load shared font: %s", filepath.c_str());
         shared_font_mem = nullptr;
diff --git a/src/core/hle/service/gsp_gpu.cpp b/src/core/hle/service/gsp_gpu.cpp
index 8da063bd2..c6252a03b 100644
--- a/src/core/hle/service/gsp_gpu.cpp
+++ b/src/core/hle/service/gsp_gpu.cpp
@@ -30,13 +30,12 @@ namespace GSP_GPU {
 Kernel::SharedPtr<Kernel::Event> g_interrupt_event;
 /// GSP shared memoryings
 Kernel::SharedPtr<Kernel::SharedMemory> g_shared_memory;
-/// Thread index into interrupt relay queue, 1 is arbitrary
-u32 g_thread_id = 1;
+/// Thread index into interrupt relay queue
+u32 g_thread_id = 0;
 
 /// Gets a pointer to a thread command buffer in GSP shared memory
 static inline u8* GetCommandBuffer(u32 thread_id) {
-    ResultVal<u8*> ptr = g_shared_memory->GetPointer(0x800 + (thread_id * sizeof(CommandBuffer)));
-    return ptr.ValueOr(nullptr);
+    return g_shared_memory->GetPointer(0x800 + (thread_id * sizeof(CommandBuffer)));
 }
 
 static inline FrameBufferUpdate* GetFrameBufferInfo(u32 thread_id, u32 screen_index) {
@@ -44,14 +43,14 @@ static inline FrameBufferUpdate* GetFrameBufferInfo(u32 thread_id, u32 screen_in
 
     // For each thread there are two FrameBufferUpdate fields
     u32 offset = 0x200 + (2 * thread_id + screen_index) * sizeof(FrameBufferUpdate);
-    ResultVal<u8*> ptr = g_shared_memory->GetPointer(offset);
-    return reinterpret_cast<FrameBufferUpdate*>(ptr.ValueOr(nullptr));
+    u8* ptr = g_shared_memory->GetPointer(offset);
+    return reinterpret_cast<FrameBufferUpdate*>(ptr);
 }
 
 /// Gets a pointer to the interrupt relay queue for a given thread index
 static inline InterruptRelayQueue* GetInterruptRelayQueue(u32 thread_id) {
-    ResultVal<u8*> ptr = g_shared_memory->GetPointer(sizeof(InterruptRelayQueue) * thread_id);
-    return reinterpret_cast<InterruptRelayQueue*>(ptr.ValueOr(nullptr));
+    u8* ptr = g_shared_memory->GetPointer(sizeof(InterruptRelayQueue) * thread_id);
+    return reinterpret_cast<InterruptRelayQueue*>(ptr);
 }
 
 /**
@@ -278,7 +277,7 @@ static void FlushDataCache(Service::Interface* self) {
  *      1 : "Flags" field, purpose is unknown
  *      3 : Handle to GSP synchronization event
  *  Outputs:
- *      0 : Result of function, 0 on success, otherwise error code
+ *      1 : Result of function, 0x2A07 on success, otherwise error code
  *      2 : Thread index into GSP command buffer
  *      4 : Handle to GSP shared memory
  */
@@ -288,11 +287,12 @@ static void RegisterInterruptRelayQueue(Service::Interface* self) {
 
     g_interrupt_event = Kernel::g_handle_table.Get<Kernel::Event>(cmd_buff[3]);
     ASSERT_MSG((g_interrupt_event != nullptr), "handle is not valid!");
-    g_shared_memory = Kernel::SharedMemory::Create("GSPSharedMem");
 
     Handle shmem_handle = Kernel::g_handle_table.Create(g_shared_memory).MoveFrom();
 
-    cmd_buff[1] = 0x2A07; // Value verified by 3dmoo team, purpose unknown, but needed for GSP init
+    // This specific code is required for a successful initialization, rather than 0
+    cmd_buff[1] = ResultCode((ErrorDescription)519, ErrorModule::GX,
+                             ErrorSummary::Success, ErrorLevel::Success).raw;
     cmd_buff[2] = g_thread_id++; // Thread ID
     cmd_buff[4] = shmem_handle; // GSP shared memory
 
@@ -527,8 +527,12 @@ Interface::Interface() {
     Register(FunctionTable);
 
     g_interrupt_event = 0;
-    g_shared_memory = 0;
-    g_thread_id = 1;
+
+    using Kernel::MemoryPermission;
+    g_shared_memory = Kernel::SharedMemory::Create(0x1000, MemoryPermission::ReadWrite,
+            MemoryPermission::ReadWrite, "GSPSharedMem");
+
+    g_thread_id = 0;
 }
 
 } // namespace
diff --git a/src/core/hle/service/hid/hid.cpp b/src/core/hle/service/hid/hid.cpp
index dd85848d0..9695f7e56 100644
--- a/src/core/hle/service/hid/hid.cpp
+++ b/src/core/hle/service/hid/hid.cpp
@@ -48,7 +48,7 @@ static u32 next_touch_index;
 //     * Set PadData.current_state.circle_right = 1 if current PadEntry.circle_pad_y <= -41
 
 void Update() {
-    SharedMem* mem = reinterpret_cast<SharedMem*>(shared_mem->GetPointer().ValueOr(nullptr));
+    SharedMem* mem = reinterpret_cast<SharedMem*>(shared_mem->GetPointer());
     const PadState state = VideoCore::g_emu_window->GetPadState();
 
     if (mem == nullptr) {
@@ -163,7 +163,9 @@ void Init() {
     AddService(new HID_U_Interface);
     AddService(new HID_SPVR_Interface);
 
-    shared_mem = SharedMemory::Create("HID:SharedMem");
+    using Kernel::MemoryPermission;
+    shared_mem = SharedMemory::Create(0x1000, MemoryPermission::ReadWrite,
+            MemoryPermission::Read, "HID:SharedMem");
 
     next_pad_index = 0;
     next_touch_index = 0;
diff --git a/src/core/hle/service/ir/ir.cpp b/src/core/hle/service/ir/ir.cpp
index 15ac477ef..adfbb258d 100644
--- a/src/core/hle/service/ir/ir.cpp
+++ b/src/core/hle/service/ir/ir.cpp
@@ -34,7 +34,9 @@ void Init() {
     AddService(new IR_U_Interface);
     AddService(new IR_User_Interface);
 
-    shared_memory = SharedMemory::Create("IR:SharedMemory");
+    using Kernel::MemoryPermission;
+    shared_memory = SharedMemory::Create(0x1000, Kernel::MemoryPermission::ReadWrite,
+            Kernel::MemoryPermission::ReadWrite, "IR:SharedMemory");
 
     // Create event handle(s)
     handle_event  = Event::Create(RESETTYPE_ONESHOT, "IR:HandleEvent");
diff --git a/src/core/hle/svc.cpp b/src/core/hle/svc.cpp
index 393cfbe79..1ec6599c7 100644
--- a/src/core/hle/svc.cpp
+++ b/src/core/hle/svc.cpp
@@ -601,7 +601,9 @@ static ResultCode CreateMemoryBlock(Handle* out_handle, u32 addr, u32 size, u32
     using Kernel::SharedMemory;
     // TODO(Subv): Implement this function
 
-    SharedPtr<SharedMemory> shared_memory = SharedMemory::Create();
+    using Kernel::MemoryPermission;
+    SharedPtr<SharedMemory> shared_memory = SharedMemory::Create(size,
+            (MemoryPermission)my_permission, (MemoryPermission)other_permission);
     CASCADE_RESULT(*out_handle, Kernel::g_handle_table.Create(std::move(shared_memory)));
 
     LOG_WARNING(Kernel_SVC, "(STUBBED) called addr=0x%08X", addr);