From b65ad538bab6d56bda2a0e6dbea785c0cb291f43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9o=20Lam?= Date: Sat, 3 Dec 2016 22:23:15 +0100 Subject: [PATCH 1/4] IOS HLE: Refactor ExecuteCommand ExecuteCommand was becoming pretty confusing with unused variables for some commands, confusing names (device ID != IOS file descriptor), duplicated checks, not keeping the indentation level low, and having tons of things into a single function. This commit gives more correct names to variables, deduplicates the device checking code, and splits ExecuteCommand so that it's easier to read. It's worth noting that some device checks have been forgotten in the past, which has caused a bug (which was recently fixed in 288e75f6). --- Source/Core/Core/IPC_HLE/WII_IPC_HLE.cpp | 267 ++++++++--------------- Source/Core/Core/IPC_HLE/WII_IPC_HLE.h | 2 - 2 files changed, 90 insertions(+), 179 deletions(-) diff --git a/Source/Core/Core/IPC_HLE/WII_IPC_HLE.cpp b/Source/Core/Core/IPC_HLE/WII_IPC_HLE.cpp index b59bb7f552..bb9c1c61ec 100644 --- a/Source/Core/Core/IPC_HLE/WII_IPC_HLE.cpp +++ b/Source/Core/Core/IPC_HLE/WII_IPC_HLE.cpp @@ -283,14 +283,6 @@ std::shared_ptr AccessDeviceByID(u32 id) return nullptr; } -// This is called from ExecuteCommand() COMMAND_OPEN_DEVICE -std::shared_ptr CreateFileIO(u32 device_id, const std::string& device_name) -{ - // scan device name and create the right one - INFO_LOG(WII_IPC_FILEIO, "IOP: Create FileIO %s", device_name.c_str()); - return std::make_shared(device_id, device_name); -} - void DoState(PointerWrap& p) { p.Do(s_request_queue); @@ -377,188 +369,109 @@ void DoState(PointerWrap& p) } } -void ExecuteCommand(u32 address) +static std::shared_ptr GetUnusedESDevice() { - IPCCommandResult result = IWII_IPC_HLE_Device::GetNoReply(); - - IPCCommandType Command = static_cast(Memory::Read_U32(address)); - s32 DeviceID = Memory::Read_U32(address + 8); - - std::shared_ptr device = - (DeviceID >= 0 && DeviceID < IPC_MAX_FDS) ? s_fdmap[DeviceID] : nullptr; - - DEBUG_LOG(WII_IPC_HLE, "-->> Execute Command Address: 0x%08x (code: %x, device: %x) %p", address, - Command, DeviceID, device.get()); - - switch (Command) + for (u32 es_number = 0; es_number < ES_MAX_COUNT; ++es_number) { - case IPC_CMD_OPEN: - { - u32 Mode = Memory::Read_U32(address + 0x10); - DeviceID = GetFreeDeviceID(); - - std::string device_name = Memory::GetString(Memory::Read_U32(address + 0xC)); - - INFO_LOG(WII_IPC_HLE, "Trying to open %s as %d", device_name.c_str(), DeviceID); - if (DeviceID >= 0) - { - if (device_name.find("/dev/es") == 0) - { - u32 j; - for (j = 0; j < ES_MAX_COUNT; j++) - { - if (!s_es_inuse[j]) - { - s_es_inuse[j] = true; - s_fdmap[DeviceID] = s_es_handles[j]; - result = s_es_handles[j]->Open(address, Mode); - Memory::Write_U32(DeviceID, address + 4); - break; - } - } - - if (j == ES_MAX_COUNT) - { - Memory::Write_U32(FS_EESEXHAUSTED, address + 4); - result = IWII_IPC_HLE_Device::GetDefaultReply(); - } - } - else if (device_name.find("/dev/") == 0) - { - device = GetDeviceByName(device_name); - if (device) - { - s_fdmap[DeviceID] = device; - result = device->Open(address, Mode); - DEBUG_LOG(WII_IPC_FILEIO, "IOP: ReOpen (Device=%s, DeviceID=%08x, Mode=%i)", - device->GetDeviceName().c_str(), DeviceID, Mode); - Memory::Write_U32(DeviceID, address + 4); - } - else - { - WARN_LOG(WII_IPC_HLE, "Unimplemented device: %s", device_name.c_str()); - Memory::Write_U32(FS_ENOENT, address + 4); - result = IWII_IPC_HLE_Device::GetDefaultReply(); - } - } - else if (device_name.find('/') == 0) - { - device = CreateFileIO(DeviceID, device_name); - result = device->Open(address, Mode); - - DEBUG_LOG(WII_IPC_FILEIO, "IOP: Open File (Device=%s, ID=%08x, Mode=%i)", - device->GetDeviceName().c_str(), DeviceID, Mode); - if (Memory::Read_U32(address + 4) == (u32)DeviceID) - s_fdmap[DeviceID] = device; - } - else - { - WARN_LOG(WII_IPC_HLE, "Invalid device: %s", device_name.c_str()); - Memory::Write_U32(FS_ENOENT, address + 4); - result = IWII_IPC_HLE_Device::GetDefaultReply(); - } - } - else - { - Memory::Write_U32(FS_EFDEXHAUSTED, address + 4); - result = IWII_IPC_HLE_Device::GetDefaultReply(); - } - break; + if (s_es_inuse[es_number]) + continue; + s_es_inuse[es_number] = true; + return s_es_handles[es_number]; } + return nullptr; +} + +// Returns the FD for the newly opened device (on success) or an error code. +static s32 OpenDevice(const u32 address) +{ + const std::string device_name = Memory::GetString(Memory::Read_U32(address + 0xC)); + const u32 open_mode = Memory::Read_U32(address + 0x10); + const s32 new_fd = GetFreeDeviceID(); + INFO_LOG(WII_IPC_HLE, "Opening %s (mode %d, fd %d)", device_name.c_str(), open_mode, new_fd); + if (new_fd < 0 || new_fd >= IPC_MAX_FDS) + { + ERROR_LOG(WII_IPC_HLE, "Couldn't get a free fd, too many open files"); + return FS_EFDEXHAUSTED; + } + + std::shared_ptr device; + if (device_name.find("/dev/es") == 0) + { + device = GetUnusedESDevice(); + if (!device) + return FS_EESEXHAUSTED; + } + else if (device_name.find("/dev/") == 0) + { + device = GetDeviceByName(device_name); + } + else if (device_name.find('/') == 0) + { + device = std::make_shared(new_fd, device_name); + } + + if (!device) + { + ERROR_LOG(WII_IPC_HLE, "Unknown device: %s", device_name.c_str()); + return FS_ENOENT; + } + + Memory::Write_U32(new_fd, address + 4); + device->Open(address, open_mode); + const s32 open_return_code = Memory::Read_U32(address + 4); + if (open_return_code < 0) + return open_return_code; + s_fdmap[new_fd] = device; + return new_fd; +} + +static IPCCommandResult HandleCommand(const u32 address) +{ + const auto command = static_cast(Memory::Read_U32(address)); + if (command == IPC_CMD_OPEN) + { + const s32 new_fd = OpenDevice(address); + Memory::Write_U32(new_fd, address + 4); + return IWII_IPC_HLE_Device::GetDefaultReply(); + } + + const s32 fd = Memory::Read_U32(address + 8); + const auto device = (fd >= 0 && fd < IPC_MAX_FDS) ? s_fdmap[fd] : nullptr; + if (!device) + { + Memory::Write_U32(FS_EINVAL, address + 4); + return IWII_IPC_HLE_Device::GetDefaultReply(); + } + + switch (command) + { case IPC_CMD_CLOSE: - { - if (device) + for (u32 j = 0; j < ES_MAX_COUNT; j++) { - result = device->Close(address); - - for (u32 j = 0; j < ES_MAX_COUNT; j++) - { - if (s_es_handles[j] == s_fdmap[DeviceID]) - { - s_es_inuse[j] = false; - } - } - - s_fdmap[DeviceID].reset(); + if (s_es_handles[j] == s_fdmap[fd]) + s_es_inuse[j] = false; } - else - { - Memory::Write_U32(FS_EINVAL, address + 4); - result = IWII_IPC_HLE_Device::GetDefaultReply(); - } - break; - } + s_fdmap[fd].reset(); + return device->Close(address); case IPC_CMD_READ: - { - if (device) - { - result = device->Read(address); - } - else - { - Memory::Write_U32(FS_EINVAL, address + 4); - result = IWII_IPC_HLE_Device::GetDefaultReply(); - } - break; - } + return device->Read(address); case IPC_CMD_WRITE: - { - if (device) - { - result = device->Write(address); - } - else - { - Memory::Write_U32(FS_EINVAL, address + 4); - result = IWII_IPC_HLE_Device::GetDefaultReply(); - } - break; - } + return device->Write(address); case IPC_CMD_SEEK: - { - if (device) - { - result = device->Seek(address); - } - else - { - Memory::Write_U32(FS_EINVAL, address + 4); - result = IWII_IPC_HLE_Device::GetDefaultReply(); - } - break; - } + return device->Seek(address); case IPC_CMD_IOCTL: - { - if (device) - { - result = device->IOCtl(address); - } - else - { - Memory::Write_U32(FS_EINVAL, address + 4); - result = IWII_IPC_HLE_Device::GetDefaultReply(); - } - break; - } + return device->IOCtl(address); case IPC_CMD_IOCTLV: - { - if (device) - { - result = device->IOCtlV(address); - } - else - { - Memory::Write_U32(FS_EINVAL, address + 4); - result = IWII_IPC_HLE_Device::GetDefaultReply(); - } - break; - } + return device->IOCtlV(address); default: - { - _dbg_assert_msg_(WII_IPC_HLE, 0, "Unknown IPC Command %i (0x%08x)", Command, address); - break; - } + _assert_msg_(WII_IPC_HLE, false, "Unexpected command: %x", command); + return IWII_IPC_HLE_Device::GetDefaultReply(); } +} + +void ExecuteCommand(const u32 address) +{ + IPCCommandResult result = HandleCommand(address); // Ensure replies happen in order const s64 ticks_until_last_reply = s_last_reply_time - CoreTiming::GetTicks(); diff --git a/Source/Core/Core/IPC_HLE/WII_IPC_HLE.h b/Source/Core/Core/IPC_HLE/WII_IPC_HLE.h index fc567662f3..13a36b3daf 100644 --- a/Source/Core/Core/IPC_HLE/WII_IPC_HLE.h +++ b/Source/Core/Core/IPC_HLE/WII_IPC_HLE.h @@ -62,8 +62,6 @@ void ES_DIVerify(const std::vector& tmd); void SDIO_EventNotify(); -std::shared_ptr CreateFileIO(u32 _DeviceID, const std::string& _rDeviceName); - std::shared_ptr GetDeviceByName(const std::string& _rDeviceName); std::shared_ptr AccessDeviceByID(u32 _ID); From 9abfa54c9dae0ddd2979413504ae567955be1354 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9o=20Lam?= Date: Mon, 5 Dec 2016 22:31:51 +0100 Subject: [PATCH 2/4] IOS HLE: Remove s_es_inuse We don't really have to keep track of device opens/closes manually, since we can already check that by calling IsOpened() on the device. This also replaces some loops with for range loops. --- Source/Core/Core/IPC_HLE/WII_IPC_HLE.cpp | 46 +++++++----------------- Source/Core/Core/State.cpp | 2 +- 2 files changed, 13 insertions(+), 35 deletions(-) diff --git a/Source/Core/Core/IPC_HLE/WII_IPC_HLE.cpp b/Source/Core/Core/IPC_HLE/WII_IPC_HLE.cpp index bb9c1c61ec..ea22ea17ef 100644 --- a/Source/Core/Core/IPC_HLE/WII_IPC_HLE.cpp +++ b/Source/Core/Core/IPC_HLE/WII_IPC_HLE.cpp @@ -71,7 +71,6 @@ static std::mutex s_device_map_mutex; #define IPC_MAX_FDS 0x18 #define ES_MAX_COUNT 2 static std::shared_ptr s_fdmap[IPC_MAX_FDS]; -static bool s_es_inuse[ES_MAX_COUNT]; static std::shared_ptr s_es_handles[ES_MAX_COUNT]; using IPCMsgQueue = std::deque; @@ -141,11 +140,8 @@ void Reinit() AddDevice("/dev/fs"); // IOS allows two ES devices at a time - for (u32 j = 0; j < ES_MAX_COUNT; j++) - { - s_es_handles[j] = AddDevice("/dev/es"); - s_es_inuse[j] = false; - } + for (auto& es_device : s_es_handles) + es_device = AddDevice("/dev/es"); AddDevice("/dev/di"); AddDevice("/dev/net/kd/request"); @@ -189,11 +185,6 @@ void Reset(bool hard) dev.reset(); } - for (bool& in_use : s_es_inuse) - { - in_use = false; - } - { std::lock_guard lock(s_device_map_mutex); for (const auto& entry : s_device_map) @@ -329,13 +320,11 @@ void DoState(PointerWrap& p) } } - for (u32 i = 0; i < ES_MAX_COUNT; i++) + for (auto& es_device : s_es_handles) { - p.Do(s_es_inuse[i]); - u32 handleID = s_es_handles[i]->GetDeviceID(); - p.Do(handleID); - - s_es_handles[i] = AccessDeviceByID(handleID); + const u32 handle_id = es_device->GetDeviceID(); + p.Do(handle_id); + es_device = AccessDeviceByID(handle_id); } } else @@ -360,25 +349,19 @@ void DoState(PointerWrap& p) } } - for (u32 i = 0; i < ES_MAX_COUNT; i++) + for (const auto& es_device : s_es_handles) { - p.Do(s_es_inuse[i]); - u32 handleID = s_es_handles[i]->GetDeviceID(); - p.Do(handleID); + const u32 handle_id = es_device->GetDeviceID(); + p.Do(handle_id); } } } static std::shared_ptr GetUnusedESDevice() { - for (u32 es_number = 0; es_number < ES_MAX_COUNT; ++es_number) - { - if (s_es_inuse[es_number]) - continue; - s_es_inuse[es_number] = true; - return s_es_handles[es_number]; - } - return nullptr; + const auto iterator = std::find_if(std::begin(s_es_handles), std::end(s_es_handles), + [](const auto& es_device) { return !es_device->IsOpened(); }); + return (iterator != std::end(s_es_handles)) ? *iterator : nullptr; } // Returns the FD for the newly opened device (on success) or an error code. @@ -446,11 +429,6 @@ static IPCCommandResult HandleCommand(const u32 address) switch (command) { case IPC_CMD_CLOSE: - for (u32 j = 0; j < ES_MAX_COUNT; j++) - { - if (s_es_handles[j] == s_fdmap[fd]) - s_es_inuse[j] = false; - } s_fdmap[fd].reset(); return device->Close(address); case IPC_CMD_READ: diff --git a/Source/Core/Core/State.cpp b/Source/Core/Core/State.cpp index 183ef9ff81..3d4a9b3744 100644 --- a/Source/Core/Core/State.cpp +++ b/Source/Core/Core/State.cpp @@ -71,7 +71,7 @@ static Common::Event g_compressAndDumpStateSyncEvent; static std::thread g_save_thread; // Don't forget to increase this after doing changes on the savestate system -static const u32 STATE_VERSION = 65; // Last changed in PR 4120 +static const u32 STATE_VERSION = 66; // Last changed in PR 4607 // Maps savestate versions to Dolphin versions. // Versions after 42 don't need to be added to this list, From 00268443caebd13e1b6ccac6c9539869acabf7d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9o=20Lam?= Date: Thu, 5 Jan 2017 00:41:42 +0100 Subject: [PATCH 3/4] WII_IPC_HLE: Replace #defines with constexpr --- Source/Core/Core/IPC_HLE/WII_IPC_HLE.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Source/Core/Core/IPC_HLE/WII_IPC_HLE.cpp b/Source/Core/Core/IPC_HLE/WII_IPC_HLE.cpp index ea22ea17ef..a10818c711 100644 --- a/Source/Core/Core/IPC_HLE/WII_IPC_HLE.cpp +++ b/Source/Core/Core/IPC_HLE/WII_IPC_HLE.cpp @@ -68,8 +68,8 @@ static std::map> s_device_map; static std::mutex s_device_map_mutex; // STATE_TO_SAVE -#define IPC_MAX_FDS 0x18 -#define ES_MAX_COUNT 2 +constexpr u8 IPC_MAX_FDS = 0x18; +constexpr u8 ES_MAX_COUNT = 2; static std::shared_ptr s_fdmap[IPC_MAX_FDS]; static std::shared_ptr s_es_handles[ES_MAX_COUNT]; From a15be890e1603cdaf86a4c4fea67f3880fda4a78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9o=20Lam?= Date: Thu, 5 Jan 2017 01:10:03 +0100 Subject: [PATCH 4/4] WII_IPC_HLE: Update the "IOS basics" comment The codebase has changed since it was written, so it needed a small update. --- Source/Core/Core/IPC_HLE/WII_IPC_HLE.cpp | 30 ++++++++++-------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/Source/Core/Core/IPC_HLE/WII_IPC_HLE.cpp b/Source/Core/Core/IPC_HLE/WII_IPC_HLE.cpp index a10818c711..b011f51d4e 100644 --- a/Source/Core/Core/IPC_HLE/WII_IPC_HLE.cpp +++ b/Source/Core/Core/IPC_HLE/WII_IPC_HLE.cpp @@ -2,23 +2,19 @@ // Licensed under GPLv2+ // Refer to the license.txt file included. -/* -This is the main Wii IPC file that handles all incoming IPC calls and directs them -to the right function. - -IPC basics (IOS' usage): - -Return values for file handles: All IPC calls will generate a return value to 0x04, -in case of success they are - Open: DeviceID - Close: 0 - Read: Bytes read - Write: Bytes written - Seek: Seek position - Ioctl: 0 (in addition to that there may be messages to the out buffers) - Ioctlv: 0 (in addition to that there may be messages to the out buffers) -They will also generate a true or false return for UpdateInterrupts() in WII_IPC.cpp. -*/ +// This is the main Wii IPC file that handles all incoming IPC requests and directs them +// to the right function. +// +// IPC basics (IOS' usage): +// All IPC request handlers will write a return value to 0x04. +// Open: Device file descriptor or error code +// Close: IPC_SUCCESS +// Read: Bytes read +// Write: Bytes written +// Seek: Seek position +// Ioctl: Depends on the handler +// Ioctlv: Depends on the handler +// Replies may be sent immediately or asynchronously for ioctls and ioctlvs. #include #include