diff --git a/Core/Dialog/PSPSaveDialog.cpp b/Core/Dialog/PSPSaveDialog.cpp index 7254f6c217..271f3baae9 100755 --- a/Core/Dialog/PSPSaveDialog.cpp +++ b/Core/Dialog/PSPSaveDialog.cpp @@ -770,11 +770,8 @@ int PSPSaveDialog::Update(int animSpeed) { EndDraw(); break; case DS_SAVE_SAVING: - if (ioThreadStatus != SAVEIO_PENDING) { - if (ioThread.joinable()) { - ioThread.join(); - } - } + // The dialog keeps drawing while the IO runs, and takes the results once it's done. + FinishIO(false); StartDraw(); @@ -788,9 +785,6 @@ int PSPSaveDialog::Update(int animSpeed) { EndDraw(); break; case DS_SAVE_FAILED: - if (ioThread.joinable()) { - ioThread.join(); - } StartDraw(); DisplaySaveIcon(true); @@ -814,10 +808,6 @@ int PSPSaveDialog::Update(int animSpeed) { EndDraw(); break; case DS_SAVE_DONE: - if (ioThread.joinable()) { - ioThread.join(); - param.SetPspParam(param.GetPspParam()); - } StartDraw(); DisplaySaveIcon(true); @@ -879,11 +869,8 @@ int PSPSaveDialog::Update(int animSpeed) { EndDraw(); break; case DS_LOAD_LOADING: - if (ioThreadStatus != SAVEIO_PENDING) { - if (ioThread.joinable()) { - ioThread.join(); - } - } + // The dialog keeps drawing while the IO runs, and takes the results once it's done. + FinishIO(false); StartDraw(); @@ -897,9 +884,6 @@ int PSPSaveDialog::Update(int animSpeed) { EndDraw(); break; case DS_LOAD_FAILED: - if (ioThread.joinable()) { - ioThread.join(); - } StartDraw(); DisplaySaveIcon(true); @@ -922,9 +906,6 @@ int PSPSaveDialog::Update(int animSpeed) { EndDraw(); break; case DS_LOAD_DONE: - if (ioThread.joinable()) { - ioThread.join(); - } StartDraw(); DisplaySaveIcon(true); @@ -1010,11 +991,8 @@ int PSPSaveDialog::Update(int animSpeed) { EndDraw(); break; case DS_DELETE_DELETING: - if (ioThreadStatus != SAVEIO_PENDING) { - if (ioThread.joinable()) { - ioThread.join(); - } - } + // The dialog keeps drawing while the IO runs, and takes the results once it's done. + FinishIO(false); StartDraw(); @@ -1025,9 +1003,6 @@ int PSPSaveDialog::Update(int animSpeed) { EndDraw(); break; case DS_DELETE_FAILED: - if (ioThread.joinable()) { - ioThread.join(); - } StartDraw(); DisplayMessage(di->T("DeleteFailed", "Unable to delete data.")); @@ -1045,10 +1020,6 @@ int PSPSaveDialog::Update(int animSpeed) { EndDraw(); break; case DS_DELETE_DONE: - if (ioThread.joinable()) { - ioThread.join(); - param.SetPspParam(param.GetPspParam()); - } StartDraw(); DisplayMessage(di->T("Delete completed")); @@ -1088,28 +1059,10 @@ int PSPSaveDialog::Update(int animSpeed) { break; case DS_NONE: // For action which display nothing - switch (ioThreadStatus) { - case SAVEIO_NONE: - if (g_Config.iIOTimingMethod == IOTIMING_HOST) { - StartIOThread(); - } else { - // The IO thread writes the results into PSP memory while the game runs, landing at - // an arbitrary point in its code. On a PSP they're all there when Update returns. - ExecuteIOAction(); - } - break; - case SAVEIO_PENDING: - case SAVEIO_DONE: - // To make sure there aren't any timing variations, we sync the next frame. - if (g_Config.iIOTimingMethod == IOTIMING_HOST && ioThreadStatus == SAVEIO_PENDING) { - // ... except in Host IO timing, where we wait as long as needed. - break; - } - if (ioThread.joinable()) { - ioThread.join(); - } + if (ioThreadStatus == SAVEIO_NONE) { + StartIOThread(); + } else if (FinishIO(g_Config.iIOTimingMethod != IOTIMING_HOST)) { ChangeStatus(SCE_UTILITY_STATUS_FINISHED, 0); - break; } break; @@ -1125,37 +1078,26 @@ int PSPSaveDialog::Update(int animSpeed) { return 0; } -// It's kinda ugly how this uses the "global" 'display'... +// Runs on the IO thread. Of the dialog, it only touches the io* members (see StartIOThread and FinishIO). void PSPSaveDialog::ExecuteIOAction() { - param.ClearSFOCache(); - auto &result = param.GetPspParam()->common.result; - std::lock_guard guard(paramLock); - switch (display) { + ioParam_.ClearSFOCache(); + auto &result = ioRequest_.common.result; + ioDisplay_ = ioAction_; + switch (ioAction_) { case DS_LOAD_LOADING: - result = param.Load(param.GetPspParam(), GetSelectedSaveDirName(), currentSelectedSave); - if (result == 0) { - display = DS_LOAD_DONE; - g_lastSaveTime = time_now_d(); - } else { - display = DS_LOAD_FAILED; - } + result = ioParam_.Load(&ioRequest_, ioSaveDirName_, ioSaveId_); + ioDisplay_ = result == 0 ? DS_LOAD_DONE : DS_LOAD_FAILED; break; case DS_SAVE_SAVING: - SaveState::NotifySaveData(); - if (param.Save(param.GetPspParam(), GetSelectedSaveDirName()) == 0) { - display = DS_SAVE_DONE; - g_lastSaveTime = time_now_d(); - } else { - display = DS_SAVE_FAILED; - } + ioDisplay_ = ioParam_.Save(&ioRequest_, ioSaveDirName_) == 0 ? DS_SAVE_DONE : DS_SAVE_FAILED; break; case DS_DELETE_DELETING: - if (param.Delete(param.GetPspParam(), currentSelectedSave)) { + if (ioParam_.Delete(&ioRequest_, ioDeleteDir_)) { result = 0; - display = DS_DELETE_DONE; + ioDisplay_ = DS_DELETE_DONE; } else { //result = SCE_UTILITY_SAVEDATA_ERROR_DELETE_NO_DATA;// What the result should be? - display = DS_DELETE_FAILED; + ioDisplay_ = DS_DELETE_FAILED; } break; case DS_NONE: @@ -1167,42 +1109,36 @@ void PSPSaveDialog::ExecuteIOAction() { break; } - ioThreadStatus = SAVEIO_DONE; - param.ClearSFOCache(); + ioParam_.ClearSFOCache(); + ioThreadStatus = SAVEIO_READY; } void PSPSaveDialog::ExecuteNotVisibleIOAction() { - param.ClearSFOCache(); - auto &result = param.GetPspParam()->common.result; + auto &result = ioRequest_.common.result; - SceUtilitySavedataType utilityMode = (SceUtilitySavedataType)(u32)param.GetPspParam()->mode; + SceUtilitySavedataType utilityMode = (SceUtilitySavedataType)(u32)ioRequest_.mode; switch (utilityMode) { case SCE_UTILITY_SAVEDATA_TYPE_LOAD: // Only load and exit case SCE_UTILITY_SAVEDATA_TYPE_AUTOLOAD: - result = param.Load(param.GetPspParam(), GetSelectedSaveDirName(), currentSelectedSave); - ResetSecondsSinceLastGameSave(); - ShowSaveLoadIndicator(false); + result = ioParam_.Load(&ioRequest_, ioSaveDirName_, ioSaveId_); break; case SCE_UTILITY_SAVEDATA_TYPE_SAVE: // Only save and exit case SCE_UTILITY_SAVEDATA_TYPE_AUTOSAVE: - SaveState::NotifySaveData(); - result = param.Save(param.GetPspParam(), GetSelectedSaveDirName()); - ResetSecondsSinceLastGameSave(); - ShowSaveLoadIndicator(true); + result = ioParam_.Save(&ioRequest_, ioSaveDirName_); break; case SCE_UTILITY_SAVEDATA_TYPE_SIZES: - result = param.GetSizes(param.GetPspParam()); + result = ioParam_.GetSizes(&ioRequest_); break; case SCE_UTILITY_SAVEDATA_TYPE_LIST: - param.GetList(param.GetPspParam()); + ioParam_.GetList(&ioRequest_); result = 0; break; case SCE_UTILITY_SAVEDATA_TYPE_FILES: - result = param.GetFilesList(param.GetPspParam(), requestAddr); + result = ioParam_.GetFilesList(&ioRequest_, requestAddr, ioListSaveDirName_); break; case SCE_UTILITY_SAVEDATA_TYPE_GETSIZE: { - bool sizeResult = param.GetSize(param.GetPspParam()); + bool sizeResult = ioParam_.GetSize(&ioRequest_); // TODO: According to JPCSP, should test/verify this part but seems edge casey. if (MemoryStick_State() != PSP_MEMORYSTICK_STATE_INSERTED) { result = SCE_UTILITY_SAVEDATA_ERROR_RW_NO_MEMSTICK; @@ -1214,8 +1150,8 @@ void PSPSaveDialog::ExecuteNotVisibleIOAction() { } break; case SCE_UTILITY_SAVEDATA_TYPE_DELETEDATA: - DEBUG_LOG(Log::sceUtility, "sceUtilitySavedata DELETEDATA: %s", param.GetPspParam()->saveName); - if (param.Delete(param.GetPspParam(), param.GetSelectedSave())) { + DEBUG_LOG(Log::sceUtility, "sceUtilitySavedata DELETEDATA: %s", ioRequest_.saveName); + if (ioParam_.Delete(&ioRequest_, ioDeleteDir_)) { result = 0; } else { result = SCE_UTILITY_SAVEDATA_ERROR_RW_NO_DATA; @@ -1223,7 +1159,7 @@ void PSPSaveDialog::ExecuteNotVisibleIOAction() { break; case SCE_UTILITY_SAVEDATA_TYPE_AUTODELETE: case SCE_UTILITY_SAVEDATA_TYPE_DELETE: - if (param.Delete(param.GetPspParam(), param.GetSelectedSave())) { + if (ioParam_.Delete(&ioRequest_, ioDeleteDir_)) { result = 0; } else { result = SCE_UTILITY_SAVEDATA_ERROR_DELETE_NO_DATA; @@ -1232,41 +1168,30 @@ void PSPSaveDialog::ExecuteNotVisibleIOAction() { // TODO: Should reset the directory's other files. case SCE_UTILITY_SAVEDATA_TYPE_MAKEDATA: case SCE_UTILITY_SAVEDATA_TYPE_MAKEDATASECURE: - result = param.Save(param.GetPspParam(), GetSelectedSaveDirName(), param.GetPspParam()->mode == SCE_UTILITY_SAVEDATA_TYPE_MAKEDATASECURE); + result = ioParam_.Save(&ioRequest_, ioSaveDirName_, ioRequest_.mode == SCE_UTILITY_SAVEDATA_TYPE_MAKEDATASECURE); if (result == SCE_UTILITY_SAVEDATA_ERROR_SAVE_MS_NOSPACE) { result = SCE_UTILITY_SAVEDATA_ERROR_RW_MEMSTICK_FULL; - } else { - SaveState::NotifySaveData(); - ResetSecondsSinceLastGameSave(); - ShowSaveLoadIndicator(true); } break; case SCE_UTILITY_SAVEDATA_TYPE_WRITEDATA: case SCE_UTILITY_SAVEDATA_TYPE_WRITEDATASECURE: - SaveState::NotifySaveData(); - result = param.Save(param.GetPspParam(), GetSelectedSaveDirName(), param.GetPspParam()->mode == SCE_UTILITY_SAVEDATA_TYPE_WRITEDATASECURE); - ResetSecondsSinceLastGameSave(); - ShowSaveLoadIndicator(true); + result = ioParam_.Save(&ioRequest_, ioSaveDirName_, ioRequest_.mode == SCE_UTILITY_SAVEDATA_TYPE_WRITEDATASECURE); break; case SCE_UTILITY_SAVEDATA_TYPE_READDATA: case SCE_UTILITY_SAVEDATA_TYPE_READDATASECURE: - result = param.Load(param.GetPspParam(), GetSelectedSaveDirName(), currentSelectedSave, param.GetPspParam()->mode == SCE_UTILITY_SAVEDATA_TYPE_READDATASECURE); + result = ioParam_.Load(&ioRequest_, ioSaveDirName_, ioSaveId_, ioRequest_.mode == SCE_UTILITY_SAVEDATA_TYPE_READDATASECURE); if (result == SCE_UTILITY_SAVEDATA_ERROR_LOAD_DATA_BROKEN) result = SCE_UTILITY_SAVEDATA_ERROR_RW_DATA_BROKEN; if (result == SCE_UTILITY_SAVEDATA_ERROR_LOAD_NO_DATA) result = SCE_UTILITY_SAVEDATA_ERROR_RW_NO_DATA; - ResetSecondsSinceLastGameSave(); - ShowSaveLoadIndicator(false); break; case SCE_UTILITY_SAVEDATA_TYPE_ERASE: case SCE_UTILITY_SAVEDATA_TYPE_ERASESECURE: - result = param.DeleteData(param.GetPspParam()); + result = ioParam_.DeleteData(&ioRequest_); break; default: break; } - - param.ClearSFOCache(); } void PSPSaveDialog::StartIOThread() { @@ -1282,6 +1207,24 @@ void PSPSaveDialog::StartIOThread() { ShowSaveLoadIndicator(save); } + // Everything the IO thread needs from the dialog, taken now: it doesn't look at the dialog's state. + const SceUtilitySavedataType mode = (SceUtilitySavedataType)(u32)request.mode; + ioAction_ = display; + ioRequest_ = request; + ioRequestStart_ = request; + ioSaveId_ = currentSelectedSave; + ioSaveDirName_ = GetSelectedSaveDirName(); + ioListSaveDirName_.clear(); + if (display == DS_NONE && mode == SCE_UTILITY_SAVEDATA_TYPE_FILES) { + ioListSaveDirName_ = param.GetSaveDirName(&request, 0); + } + ioDeleteDir_.clear(); + if (display == DS_DELETE_DELETING) { + ioDeleteDir_ = param.GetSaveDir(currentSelectedSave); + } else if (display == DS_NONE && (mode == SCE_UTILITY_SAVEDATA_TYPE_DELETEDATA || mode == SCE_UTILITY_SAVEDATA_TYPE_AUTODELETE || mode == SCE_UTILITY_SAVEDATA_TYPE_DELETE)) { + ioDeleteDir_ = param.GetSaveDir(param.GetSelectedSave()); + } + ioThreadStatus = SAVEIO_PENDING; ioThread = std::thread([this]() { SetCurrentThreadName("SaveIO"); @@ -1291,6 +1234,93 @@ void PSPSaveDialog::StartIOThread() { }); } +// Takes the IO thread's results back. Without wait, only if it's done: returns whether it was. +bool PSPSaveDialog::FinishIO(bool wait) { + if (ioThreadStatus == SAVEIO_PENDING && !wait) { + return false; + } + if (ioThread.joinable()) { + ioThread.join(); + } + if (ioThreadStatus != SAVEIO_READY) { + return true; + } + + { + // Only what the IO changed: the game may have changed its request meanwhile (Update reloads it). + std::lock_guard guard(paramLock); + const u8 *before = (const u8 *)&ioRequestStart_; + const u8 *after = (const u8 *)&ioRequest_; + u8 *live = (u8 *)&request; + for (size_t i = 0; i < sizeof(request); ++i) { + if (after[i] != before[i]) { + live[i] = after[i]; + } + } + } + + switch (ioAction_) { + case DS_LOAD_LOADING: + if (ioDisplay_ == DS_LOAD_DONE) { + g_lastSaveTime = time_now_d(); + } + break; + case DS_SAVE_SAVING: + SaveState::NotifySaveData(); + if (ioDisplay_ == DS_SAVE_DONE) { + g_lastSaveTime = time_now_d(); + } + break; + case DS_NONE: + switch ((SceUtilitySavedataType)(u32)request.mode) { + case SCE_UTILITY_SAVEDATA_TYPE_LOAD: + case SCE_UTILITY_SAVEDATA_TYPE_AUTOLOAD: + case SCE_UTILITY_SAVEDATA_TYPE_READDATA: + case SCE_UTILITY_SAVEDATA_TYPE_READDATASECURE: + ResetSecondsSinceLastGameSave(); + ShowSaveLoadIndicator(false); + break; + case SCE_UTILITY_SAVEDATA_TYPE_MAKEDATA: + case SCE_UTILITY_SAVEDATA_TYPE_MAKEDATASECURE: + if (request.common.result == SCE_UTILITY_SAVEDATA_ERROR_RW_MEMSTICK_FULL) { + break; + } + [[fallthrough]]; + case SCE_UTILITY_SAVEDATA_TYPE_SAVE: + case SCE_UTILITY_SAVEDATA_TYPE_AUTOSAVE: + case SCE_UTILITY_SAVEDATA_TYPE_WRITEDATA: + case SCE_UTILITY_SAVEDATA_TYPE_WRITEDATASECURE: + SaveState::NotifySaveData(); + ResetSecondsSinceLastGameSave(); + ShowSaveLoadIndicator(true); + break; + default: + break; + } + break; + default: + break; + } + + if (ioAction_ != DS_NONE) { + display = ioDisplay_; + // Show what was saved or deleted in the list. + if (display == DS_SAVE_DONE || display == DS_DELETE_DONE) { + std::lock_guard guard(paramLock); + param.SetPspParam(param.GetPspParam()); + } + } + + ioThreadStatus = SAVEIO_DONE; + return true; +} + +void PSPSaveDialog::WaitForIO() { + if (ioThread.joinable()) { + ioThread.join(); + } +} + int PSPSaveDialog::Shutdown(bool force) { if (GetStatus() != SCE_UTILITY_STATUS_FINISHED && !force) return SCE_ERROR_UTILITY_INVALID_STATUS; @@ -1310,6 +1340,28 @@ int PSPSaveDialog::Shutdown(bool force) { return 0; } +// Version 4 of the dialog's state (never in a release) kept the IO's writes to PSP memory until the +// results were taken. Late is better than never. +static void DoStateOldPendingWrites(PointerWrap &p) { + auto s = p.Section("SavedataMemory", 1); + if (!s) { + return; + } + u32 count = 0; + Do(p, count); + for (u32 i = 0; i < count; ++i) { + u32 addr = 0; + std::string tag; + std::vector data; + Do(p, addr); + Do(p, tag); + Do(p, data); + if (p.mode == PointerWrap::MODE_READ && Memory::IsValidRange(addr, (u32)data.size())) { + Memory::Memcpy(addr, data.data(), (u32)data.size(), tag.c_str(), tag.size()); + } + } +} + void PSPSaveDialog::DoState(PointerWrap &p) { if (ioThread.joinable()) { ioThread.join(); @@ -1318,11 +1370,11 @@ void PSPSaveDialog::DoState(PointerWrap &p) { // Version 3 activates the s > 2 branch below, so ioThreadStatus survives // a savestate. Safe to restore: the IO thread was joined above, so the - // value is only ever SAVEIO_NONE or SAVEIO_DONE, and the operation's - // effects are already part of the serialized state. Without this, loading - // a state taken while a savedata operation was in flight would restart - // the operation instead of resuming from its recorded status. - auto s = p.Section("PSPSaveDialog", 1, 3); + // value is never SAVEIO_PENDING. Without this, loading a state taken + // while a savedata operation was in flight would restart the operation + // instead of resuming from its recorded status. Version 4 keeps the + // results of a finished operation that haven't been taken yet. + auto s = p.Section("PSPSaveDialog", 1, 5); if (!s) { return; } @@ -1339,10 +1391,21 @@ void PSPSaveDialog::DoState(PointerWrap &p) { Do(p, requestAddr); Do(p, currentSelectedSave); Do(p, yesnoChoice); + SaveIOStatus ioStatus = ioThreadStatus; if (s > 2) { - Do(p, ioThreadStatus); + Do(p, ioStatus); } else { - ioThreadStatus = SAVEIO_NONE; + ioStatus = SAVEIO_NONE; + } + ioThreadStatus = ioStatus; + if (s >= 4) { + Do(p, ioAction_); + Do(p, ioDisplay_); + Do(p, ioRequest_); + Do(p, ioRequestStart_); + } + if (s == 4) { + DoStateOldPendingWrites(p); } } diff --git a/Core/Dialog/PSPSaveDialog.h b/Core/Dialog/PSPSaveDialog.h index 8fe2b2d23e..0dbaf84467 100644 --- a/Core/Dialog/PSPSaveDialog.h +++ b/Core/Dialog/PSPSaveDialog.h @@ -17,6 +17,8 @@ #pragma once +#include +#include #include #include @@ -33,8 +35,8 @@ public: int Shutdown(bool force = false) override; void DoState(PointerWrap &p) override; pspUtilityDialogCommon *GetCommonParam() override; - - void ExecuteIOAction(); + // Waits for the IO thread, if any, to be done with PSP memory. Its results are still taken as usual. + void WaitForIO(); protected: bool UseAutoStatus() override { @@ -51,6 +53,8 @@ private: std::string GetSelectedSaveDirName() const; void StartIOThread(); + bool FinishIO(bool wait); + void ExecuteIOAction(); void ExecuteNotVisibleIOAction(); enum DisplayState { @@ -98,12 +102,29 @@ private: enum SaveIOStatus { SAVEIO_NONE, SAVEIO_PENDING, + // Finished, and the results taken. SAVEIO_DONE, + // Finished, but the request changes and bookkeeping haven't been taken yet. + SAVEIO_READY, }; std::thread ioThread; std::mutex paramLock; - volatile SaveIOStatus ioThreadStatus = SAVEIO_NONE; + std::atomic ioThreadStatus{ SAVEIO_NONE }; + + // The IO thread uses these instead of the dialog's own state (it writes its results to PSP + // memory directly). StartIOThread sets them up and FinishIO takes the results back, both on the + // emulator thread. + DisplayState ioAction_ = DS_NONE; + DisplayState ioDisplay_ = DS_NONE; + SceUtilitySavedataParam ioRequest_{}; + // The request when the IO started, to tell what the IO changed. + SceUtilitySavedataParam ioRequestStart_{}; + SavedataParam ioParam_; + int ioSaveId_ = 0; + std::string ioSaveDirName_; + std::string ioListSaveDirName_; + std::string ioDeleteDir_; }; void ResetSecondsSinceLastGameSave(); diff --git a/Core/Dialog/SavedataParam.cpp b/Core/Dialog/SavedataParam.cpp index 9b9b410a2e..f4923fc041 100644 --- a/Core/Dialog/SavedataParam.cpp +++ b/Core/Dialog/SavedataParam.cpp @@ -313,7 +313,7 @@ bool SavedataParam::HasKey(const SceUtilitySavedataParam *param) const return false; } -bool SavedataParam::Delete(SceUtilitySavedataParam* param, int saveId) { +bool SavedataParam::Delete(SceUtilitySavedataParam* param, const std::string &saveDir) { if (!param) { return false; } @@ -324,7 +324,7 @@ bool SavedataParam::Delete(SceUtilitySavedataParam* param, int saveId) { return false; } - std::string dirPath = GetSaveFilePath(param, GetSaveDir(saveId)); + std::string dirPath = GetSaveFilePath(param, saveDir); if (dirPath.size() == 0) { ERROR_LOG(Log::sceUtility, "GetSaveFilePath (%.*s) returned empty - cannot delete save directory. Might already be deleted?", (int)sizeof(param->gameName), param->gameName); return false; @@ -1281,7 +1281,7 @@ bool SavedataParam::GetList(SceUtilitySavedataParam *param) return true; } -int SavedataParam::GetFilesList(SceUtilitySavedataParam *param, u32 requestAddr) { +int SavedataParam::GetFilesList(SceUtilitySavedataParam *param, u32 requestAddr, const std::string &saveDirName) { if (!param) { return SCE_UTILITY_SAVEDATA_ERROR_RW_BAD_STATUS; } @@ -1339,7 +1339,7 @@ int SavedataParam::GetFilesList(SceUtilitySavedataParam *param, u32 requestAddr) requestPtr->bind = 1021; // Does not list directories, nor recurse into them, and ignores files not ALL UPPERCASE. - bool isCrypted = GetSaveCryptMode(param, GetSaveDirName(param, 0)) != 0; + bool isCrypted = GetSaveCryptMode(param, saveDirName) != 0; for (const auto &file : files) { if (file.type == FILETYPE_DIRECTORY) { continue; @@ -1795,6 +1795,9 @@ std::string SavedataParam::GetFilename(int idx) const } std::string SavedataParam::GetSaveDir(int idx) const { + if (!saveDataList || idx < 0 || idx >= saveDataListCount) { + return ""; + } return saveDataList[idx].saveDir; } diff --git a/Core/Dialog/SavedataParam.h b/Core/Dialog/SavedataParam.h index 65f47d7e8d..5689b51954 100644 --- a/Core/Dialog/SavedataParam.h +++ b/Core/Dialog/SavedataParam.h @@ -311,13 +311,13 @@ public: std::string GetSaveDirName(const SceUtilitySavedataParam *param, int saveId = -1) const; std::string GetSaveDir(const SceUtilitySavedataParam *param, int saveId = -1) const; std::string GetSaveDir(const SceUtilitySavedataParam *param, const std::string &saveDirName) const; - bool Delete(SceUtilitySavedataParam* param, int saveId = -1); + bool Delete(SceUtilitySavedataParam* param, const std::string &saveDir); int DeleteData(SceUtilitySavedataParam* param); int Save(SceUtilitySavedataParam* param, const std::string &saveDirName, bool secureMode = true); int Load(SceUtilitySavedataParam* param, const std::string &saveDirName, int saveId = -1, bool secureMode = true); int GetSizes(SceUtilitySavedataParam* param); bool GetList(SceUtilitySavedataParam* param); - int GetFilesList(SceUtilitySavedataParam* param, u32 requestAddr); + int GetFilesList(SceUtilitySavedataParam* param, u32 requestAddr, const std::string &saveDirName); bool GetSize(SceUtilitySavedataParam* param); int GetSaveCryptMode(const SceUtilitySavedataParam *param, const std::string &saveDirName); bool IsInSaveDataList(const std::string &saveName, int count); diff --git a/Core/HLE/sceChnnlsv.cpp b/Core/HLE/sceChnnlsv.cpp index 1ca310015b..918ca97696 100644 --- a/Core/HLE/sceChnnlsv.cpp +++ b/Core/HLE/sceChnnlsv.cpp @@ -15,6 +15,8 @@ // Official git repository and contact information can be found at // https://github.com/hrydgard/ppsspp and http://www.ppsspp.org/. +#include + #include "Core/MemMapHelpers.h" #include "Core/HLE/HLE.h" #include "Core/HLE/FunctionWrappers.h" @@ -28,6 +30,9 @@ KirkState *__ChnnlsvKirkState() { return &g_kirk; } +// The savedata IO thread uses these through the sceSd functions below, while the game can call them +// (and the kirk ones) on the emulator thread. +static std::mutex g_lock; static u8 dataBuf[2048+20]; static u8 *dataBuf2 = dataBuf + 20; @@ -226,6 +231,7 @@ static int sceSdGetLastIndex(u32 addressCtx, u32 addressHash, u32 addressKey) { int sceSdMacFinal(pspChnnlsvContext1& ctx, u8* in_hash, const u8* in_key) { + std::lock_guard guard(g_lock); if(ctx.keyLength >= 17) return -1026; @@ -352,6 +358,7 @@ static int sceSdRemoveValue(u32 addressCtx, u32 addressData, int length) { int sceSdMacUpdate(pspChnnlsvContext1& ctx, const u8* data, int length) { + std::lock_guard guard(g_lock); if(ctx.keyLength >= 17) return -1026; @@ -404,6 +411,7 @@ static int sceSdCreateList(u32 ctx2Addr, int mode, int unkwn, u32 dataAddr, u32 int sceSdCipherInit(pspChnnlsvContext2& ctx2, int mode, int uknw, u8* data, const u8* cryptkey) { + std::lock_guard guard(g_lock); ctx2.mode = mode; ctx2.unkn = 1; if (uknw == 2) @@ -467,6 +475,7 @@ static int sceSdSetMember(u32 ctxAddr, u32 dataAddr, int alignedLen) { int sceSdCipherUpdate(pspChnnlsvContext2& ctx, u8* data, int alignedLen) { + std::lock_guard guard(g_lock); if (alignedLen == 0) { return 0; @@ -538,6 +547,7 @@ void Register_sceChnnlsv() static u32 sceUtilsBufferCopyWithRange(u32 outAddr, int outSize, u32 inAddr, int inSize, int cmd) { u8 *outAddress = Memory::IsValidRange(outAddr, outSize) ? Memory::GetPointerWriteUnchecked(outAddr) : nullptr; u8 *inAddress = Memory::IsValidRange(inAddr, inSize) ? Memory::GetPointerWriteUnchecked(inAddr) : nullptr; + std::lock_guard guard(g_lock); int temp = kirk_sceUtilsBufferCopyWithRange(&g_kirk, outAddress, outSize, inAddress, inSize, cmd); if (temp != 0) { ERROR_LOG(Log::sceKernel, "hleUtilsBufferCopyWithRange: Failed with %d", temp); @@ -549,6 +559,7 @@ static u32 sceUtilsBufferCopyWithRange(u32 outAddr, int outSize, u32 inAddr, int static int sceUtilsBufferCopyByPollingWithRange(u32 outAddr, int outSize, u32 inAddr, int inSize, int cmd) { u8 *outAddress = Memory::IsValidRange(outAddr, outSize) ? Memory::GetPointerWriteUnchecked(outAddr) : nullptr; u8 *inAddress = Memory::IsValidRange(inAddr, inSize) ? Memory::GetPointerWriteUnchecked(inAddr) : nullptr; + std::lock_guard guard(g_lock); return hleNoLog(kirk_sceUtilsBufferCopyWithRange(&g_kirk, outAddress, outSize, inAddress, inSize, cmd)); } diff --git a/Core/HLE/sceUtility.cpp b/Core/HLE/sceUtility.cpp index 67251a22a6..054b2b0a5e 100644 --- a/Core/HLE/sceUtility.cpp +++ b/Core/HLE/sceUtility.cpp @@ -626,6 +626,12 @@ void __UtilityDoState(PointerWrap &p) { } } +void __UtilityWaitForIO() { + if (saveDialog) { + saveDialog->WaitForIO(); + } +} + void __UtilityShutdown() { saveDialog->Shutdown(true); msgDialog->Shutdown(true); @@ -648,6 +654,7 @@ void __UtilityShutdown() { lastSaveStateVersion = -1; delete saveDialog; + saveDialog = nullptr; delete msgDialog; delete oskDialog; delete netDialog; diff --git a/Core/HLE/sceUtility.h b/Core/HLE/sceUtility.h index 2240708796..93b20b9a2a 100644 --- a/Core/HLE/sceUtility.h +++ b/Core/HLE/sceUtility.h @@ -123,6 +123,8 @@ enum class UtilityDialogType { void __UtilityInit(); void __UtilityDoState(PointerWrap &p); void __UtilityShutdown(); +// Before a savestate saves or loads memory: the savedata IO thread reads and writes it directly. +void __UtilityWaitForIO(); void UtilityDialogInitialize(UtilityDialogType type, int delayUs, int accessPriority, int graphicsPriority); void UtilityDialogShutdown(UtilityDialogType type, int delayUs, int accessPriority, int graphicsPriority); diff --git a/Core/HW/MemoryStick.cpp b/Core/HW/MemoryStick.cpp index 4e74526a37..6ba306115d 100644 --- a/Core/HW/MemoryStick.cpp +++ b/Core/HW/MemoryStick.cpp @@ -16,6 +16,8 @@ // https://github.com/hrydgard/ppsspp and http://www.ppsspp.org/. #include +#include +#include #include #include #include @@ -42,8 +44,12 @@ static MemStickFatState memStickFatState; static bool memStickNeedsAssign = false; static uint64_t memStickInsertedAt = 0; static uint64_t memstickInitialFree = 0; +// The savedata IO thread asks for the free space too, so the cached use is guarded, and a write +// during the calculation leaves it stale rather than marked current. +static std::mutex memstickCurrentUseLock; static uint64_t memstickCurrentUse = 0; -static bool memstickCurrentUseValid = false; +static uint32_t memstickCurrentUseGeneration = 0; +static std::atomic memstickWriteGeneration{ 1 }; enum FreeCalcStatus { NONE, @@ -122,15 +128,21 @@ u64 MemoryStick_FreeSpace(std::string gameID) { const u64 memStickSize = flags.ReportSmallMemstick ? smallMemstickSize : (u64)g_Config.iMemStickSizeGB * 1024 * 1024 * 1024; // Assume the memory stick is only used to store savedata, for the current game only. - if (!memstickCurrentUseValid) { - Path saveFolder = GetSysDirectory(DIRECTORY_SAVEDATA); - memstickCurrentUse = ComputeSizeOfSavedataForGame(saveFolder, gameID); - memstickCurrentUseValid = true; + u64 currentUse; + { + std::lock_guard guard(memstickCurrentUseLock); + const uint32_t generation = memstickWriteGeneration; + if (memstickCurrentUseGeneration != generation) { + Path saveFolder = GetSysDirectory(DIRECTORY_SAVEDATA); + memstickCurrentUse = ComputeSizeOfSavedataForGame(saveFolder, gameID); + memstickCurrentUseGeneration = generation; + } + currentUse = memstickCurrentUse; } u64 simulatedFreeSpace = 0; - if (memstickCurrentUse < memStickSize) { - simulatedFreeSpace = memStickSize - memstickCurrentUse; + if (currentUse < memStickSize) { + simulatedFreeSpace = memStickSize - currentUse; } else if (flags.ReportSmallMemstick) { // There's more stuff in the memstick than the size we report. // This doesn't work, so we'll just have to lie. Not sure what the best way is. @@ -144,8 +156,8 @@ u64 MemoryStick_FreeSpace(std::string gameID) { // Assassin's Creed: Bloodlines fails to save if free space changes incorrectly during game. // See issue #12761 u64 realFreeSpace = 0; - if (memstickCurrentUse <= memstickInitialFree) { - realFreeSpace = memstickInitialFree - memstickCurrentUse; + if (currentUse <= memstickInitialFree) { + realFreeSpace = memstickInitialFree - currentUse; } space = std::min(simulatedFreeSpace, realFreeSpace); } else if (System_GetPropertyBool(SYSPROP_CAN_GET_FREE_SPACE_FAST) || g_Config.bReportAccurateFreeStorageSpace) { @@ -161,7 +173,7 @@ u64 MemoryStick_FreeSpace(std::string gameID) { } void MemoryStick_NotifyWrite() { - memstickCurrentUseValid = false; + memstickWriteGeneration++; } void MemoryStick_SetFatState(MemStickFatState state) { diff --git a/Core/SaveState.cpp b/Core/SaveState.cpp index 810b82f0db..b2f706b426 100644 --- a/Core/SaveState.cpp +++ b/Core/SaveState.cpp @@ -137,6 +137,9 @@ int g_screenshotFailures; // when in-game it's just not an issue. void SaveStart::DoState(PointerWrap &p) { + // Nothing may still be writing PSP memory while it's saved, or be left to write into what's loaded. + __UtilityWaitForIO(); + auto s = p.Section("SaveStart", 1, 3); if (!s) return; diff --git a/pspautotests b/pspautotests index 3dc4690e7d..2c804f5cb3 160000 --- a/pspautotests +++ b/pspautotests @@ -1 +1 @@ -Subproject commit 3dc4690e7d4f704764d23ef1ef0eb82ccc3dc30d +Subproject commit 2c804f5cb3cd97b5ca3e11242060fee73e50bc47