diff --git a/Source/Core/Core/HW/EXI/EXI.cpp b/Source/Core/Core/HW/EXI/EXI.cpp index c4be51e2edd2..4cd16ddd8bb0 100644 --- a/Source/Core/Core/HW/EXI/EXI.cpp +++ b/Source/Core/Core/HW/EXI/EXI.cpp @@ -152,13 +152,15 @@ static void ChangeDeviceCallback(u64 userdata, s64 cyclesLate) } void ChangeDevice(const u8 channel, const TEXIDevices device_type, const u8 device_num, - CoreTiming::FromThread from_thread) + CoreTiming::FromThread from_thread, s64 cycles_delay_change, + s64 cycles_no_device_visible) { - // Let the hardware see no device for 1 second - CoreTiming::ScheduleEvent(0, changeDevice, + // Let the hardware see no device for cycles_no_device_visible cycles after waiting for + // cycles_delay_change cycles + CoreTiming::ScheduleEvent(cycles_delay_change, changeDevice, ((u64)channel << 32) | ((u64)EXIDEVICE_NONE << 16) | device_num, from_thread); - CoreTiming::ScheduleEvent(SystemTimers::GetTicksPerSecond(), changeDevice, + CoreTiming::ScheduleEvent(cycles_delay_change + cycles_no_device_visible, changeDevice, ((u64)channel << 32) | ((u64)device_type << 16) | device_num, from_thread); } diff --git a/Source/Core/Core/HW/EXI/EXI.h b/Source/Core/Core/HW/EXI/EXI.h index f902c6e735f5..d974f1c381c9 100644 --- a/Source/Core/Core/HW/EXI/EXI.h +++ b/Source/Core/Core/HW/EXI/EXI.h @@ -5,6 +5,7 @@ #include "Common/CommonTypes.h" #include "Core/CoreTiming.h" +#include "Core/HW/SystemTimers.h" class PointerWrap; @@ -39,8 +40,13 @@ void RegisterMMIO(MMIO::Mapping* mmio, u32 base); void UpdateInterrupts(); void ScheduleUpdateInterrupts(CoreTiming::FromThread from, int cycles_late); +// Change a device over a period of time. +// This schedules a device change: First the device is 'ejected' cycles_delay_change cycles from +// now, then the new device is 'inserted' another cycles_no_device_visible cycles later. void ChangeDevice(const u8 channel, const TEXIDevices device_type, const u8 device_num, - CoreTiming::FromThread from_thread = CoreTiming::FromThread::NON_CPU); + CoreTiming::FromThread from_thread = CoreTiming::FromThread::NON_CPU, + s64 cycles_delay_change = 0, + s64 cycles_no_device_visible = SystemTimers::GetTicksPerSecond()); CEXIChannel* GetChannel(u32 index); diff --git a/Source/Core/Core/HW/EXI/EXI_Channel.cpp b/Source/Core/Core/HW/EXI/EXI_Channel.cpp index 5488cdad8ea8..6eacedbaaa7a 100644 --- a/Source/Core/Core/HW/EXI/EXI_Channel.cpp +++ b/Source/Core/Core/HW/EXI/EXI_Channel.cpp @@ -13,7 +13,9 @@ #include "Core/CoreTiming.h" #include "Core/HW/EXI/EXI.h" #include "Core/HW/EXI/EXI_Device.h" +#include "Core/HW/EXI/EXI_DeviceMemoryCard.h" #include "Core/HW/MMIO.h" +#include "Core/HW/SystemTimers.h" #include "Core/Movie.h" namespace ExpansionInterface @@ -238,48 +240,90 @@ void CEXIChannel::DoState(PointerWrap& p) p.Do(m_dma_length); p.Do(m_control); p.Do(m_imm_data); - - Memcard::HeaderData old_header_data = m_memcard_header_data; p.DoPOD(m_memcard_header_data); for (int device_index = 0; device_index < NUM_DEVICES; ++device_index) { - std::unique_ptr& device = m_devices[device_index]; - TEXIDevices type = device->m_device_type; + TEXIDevices type = m_devices[device_index]->m_device_type; p.Do(type); - if (type == device->m_device_type) - { - device->DoState(p); - } - else - { - std::unique_ptr save_device = - EXIDevice_Create(type, m_channel_id, m_memcard_header_data); - save_device->DoState(p); - AddDevice(std::move(save_device), device_index, false); - } + if (type != m_devices[device_index]->m_device_type) + AddDevice(EXIDevice_Create(type, m_channel_id, m_memcard_header_data), device_index, false); + + m_devices[device_index]->DoState(p); - if (type == EXIDEVICE_MEMORYCARDFOLDER && old_header_data != m_memcard_header_data && - !Movie::IsMovieActive()) + const bool is_memcard = type == EXIDEVICE_MEMORYCARD || type == EXIDEVICE_MEMORYCARDFOLDER; + if (is_memcard && !Movie::IsMovieActive()) { - // We have loaded a savestate that has a GCI folder memcard that is different to the virtual - // card that is currently active. In order to prevent the game from recognizing this card as a - // 'different' memory card and preventing saving on it, we need to reinitialize the GCI folder - // card here with the loaded header data. - // We're intentionally calling ExpansionInterface::ChangeDevice() here instead of changing it - // directly so we don't switch immediately but after a delay, as if changed in the GUI. This - // should prevent games from assuming any stale data about the memory card, such as location - // of the individual save blocks, which may be different on the reinitialized card. - // Additionally, we immediately force the memory card to None so that any 'in-flight' writes - // (if someone managed to savestate while saving...) don't happen to hit the card. - // TODO: It might actually be enough to just switch to the card with the - // notify_presence_changed flag set to true? Not sure how software behaves if the previous and - // the new device type are identical in this case. I assume there is a reason we have this - // grace period when switching in the GUI. - AddDevice(EXIDEVICE_NONE, device_index); - ExpansionInterface::ChangeDevice(m_channel_id, EXIDEVICE_MEMORYCARDFOLDER, device_index, - CoreTiming::FromThread::CPU); + // We are processing a savestate that has a memory card plugged into the system, but don't + // want to actually store the memory card in the savestate, so loading older savestates + // doesn't confusingly revert in-game saves done since then. + + // Handling this properly is somewhat complex. When loading a state, the game still needs to + // see the memory card device as it was (or at least close enough) when the state was made, + // but at the same time we need to tell the game that the data on the card may have been + // changed and it should not assume that the data (especially the file system blocks) it may + // or may not have read before is still valid. + + // To accomplish this we do the following: + + CEXIMemoryCard* card_device = static_cast(m_devices[device_index].get()); + constexpr s32 file_system_data_size = 0xa000; + + if (p.GetMode() == PointerWrap::MODE_READ) + { + // When loading a state, we compare the previously written file system block data with the + // data currently in the card. If it mismatches in any way, we make sure to notify the game. + bool should_notify_game = false; + bool has_card_file_system_data; + p.Do(has_card_file_system_data); + if (has_card_file_system_data) + { + std::array state_file_system_data; + p.DoPOD(state_file_system_data); + std::array card_file_system_data; + const s32 bytes_read = + card_device->ReadFromMemcard(0, file_system_data_size, card_file_system_data.data()); + bool read_success = bytes_read == file_system_data_size; + should_notify_game = !(read_success && state_file_system_data == card_file_system_data); + } + else + { + should_notify_game = true; + } + + if (should_notify_game) + { + // We want to tell the game that the card is different, but don't want to immediately + // destroy the memory card device, as there may be pending operations running on the CPU + // side (at the very least, I've seen memory accesses to 0 when switching the device + // immediately in F-Zero GX). In order to ensure data on the card isn't corrupted by + // these, we disable writes to the memory card device. + card_device->DisableWrites(); + + // And then we force a re-load of the memory card by switching to a null device a frame + // from now, then back to the memory card a few frames later. This also makes sure that + // the GCI folder header matches what the game expects, so the game never recognizes it as + // a 'different' memory card and prevents saving on it. + const u32 cycles_per_second = SystemTimers::GetTicksPerSecond(); + ExpansionInterface::ChangeDevice(m_channel_id, type, device_index, + CoreTiming::FromThread::CPU, cycles_per_second / 60, + cycles_per_second / 3); + } + } + else + { + // When saving a state (or when we're measuring or verifiying the state data) we read the + // file system blocks from the currently active memory card and push them into p, so we have + // a reference of how the card file system looked at the time of savestate creation. + std::array card_file_system_data; + const s32 bytes_read = + card_device->ReadFromMemcard(0, file_system_data_size, card_file_system_data.data()); + bool read_success = bytes_read == file_system_data_size; + p.Do(read_success); + if (read_success) + p.DoPOD(card_file_system_data); + } } } } diff --git a/Source/Core/Core/HW/EXI/EXI_DeviceMemoryCard.cpp b/Source/Core/Core/HW/EXI/EXI_DeviceMemoryCard.cpp index d9fe78b7427d..2c4186c71bb9 100644 --- a/Source/Core/Core/HW/EXI/EXI_DeviceMemoryCard.cpp +++ b/Source/Core/Core/HW/EXI/EXI_DeviceMemoryCard.cpp @@ -174,6 +174,19 @@ CEXIMemoryCard::GetGCIFolderPath(int card_index, AllowMovieFolder allow_movie_fo return {std::move(path), !use_movie_folder}; } +s32 CEXIMemoryCard::ReadFromMemcard(u32 memcard_offset, s32 length, u8* dest_address) const +{ + if (!m_memory_card) + return 0; + + return m_memory_card->Read(memcard_offset, length, dest_address); +} + +void CEXIMemoryCard::DisableWrites() +{ + m_allow_writes = false; +} + void CEXIMemoryCard::SetupGciFolder(const Memcard::HeaderData& header_data) { const std::string& game_id = SConfig::GetInstance().GetGameID(); @@ -294,7 +307,8 @@ void CEXIMemoryCard::SetCS(int cs) case Command::SectorErase: if (m_position > 2) { - m_memory_card->ClearBlock(m_address & (m_memory_card_size - 1)); + if (m_allow_writes) + m_memory_card->ClearBlock(m_address & (m_memory_card_size - 1)); m_status |= MC_STATUS_BUSY; m_status &= ~MC_STATUS_READY; @@ -309,7 +323,8 @@ void CEXIMemoryCard::SetCS(int cs) { // TODO: Investigate on HW, I (LPFaint99) believe that this only // erases the system area (Blocks 0-4) - m_memory_card->ClearAll(); + if (m_allow_writes) + m_memory_card->ClearAll(); m_status &= ~MC_STATUS_BUSY; } break; @@ -323,7 +338,8 @@ void CEXIMemoryCard::SetCS(int cs) while (count--) { - m_memory_card->Write(m_address, 1, &(m_programming_buffer[i++])); + if (m_allow_writes) + m_memory_card->Write(m_address, 1, &(m_programming_buffer[i++])); i &= 127; m_address = (m_address & ~0x1FF) | ((m_address + 1) & 0x1FF); } @@ -555,11 +571,13 @@ void CEXIMemoryCard::DMARead(u32 addr, u32 size) // write all at once instead of single byte at a time as done by IEXIDevice::DMAWrite void CEXIMemoryCard::DMAWrite(u32 addr, u32 size) { - m_memory_card->Write(m_address, size, Memory::GetPointer(addr)); + if (m_allow_writes) + m_memory_card->Write(m_address, size, Memory::GetPointer(addr)); if (((m_address + size) % Memcard::BLOCK_SIZE) == 0) { - INFO_LOG_FMT(EXPANSIONINTERFACE, "writing to block: {:x}", m_address / Memcard::BLOCK_SIZE); + INFO_LOG_FMT(EXPANSIONINTERFACE, "{}writing to block: {:x}", m_allow_writes ? "" : "fake ", + m_address / Memcard::BLOCK_SIZE); } // Schedule transfer complete later based on write speed diff --git a/Source/Core/Core/HW/EXI/EXI_DeviceMemoryCard.h b/Source/Core/Core/HW/EXI/EXI_DeviceMemoryCard.h index 01fc30a4eb89..5409d1f066ba 100644 --- a/Source/Core/Core/HW/EXI/EXI_DeviceMemoryCard.h +++ b/Source/Core/Core/HW/EXI/EXI_DeviceMemoryCard.h @@ -50,6 +50,15 @@ class CEXIMemoryCard : public IEXIDevice static std::pair GetGCIFolderPath(int card_index, AllowMovieFolder allow_movie_folder); + s32 ReadFromMemcard(u32 memcard_offset, s32 length, u8* dest_address) const; + + // After this call all writes to the card are disabled. + // This is used to have a 'grace period' after loading a savestate where the emulated software + // still sees the memory card but can't write to it anymore. + // It is expected that this device is destroyed and possibly recreated soon (within a few emulated + // frames) after this method has been called. + void DisableWrites(); + private: void SetupGciFolder(const Memcard::HeaderData& header_data); void SetupRawMemcard(u16 size_mb); @@ -105,6 +114,7 @@ class CEXIMemoryCard : public IEXIDevice unsigned int m_address; u32 m_memory_card_size; std::unique_ptr m_memory_card; + bool m_allow_writes = true; protected: void TransferByte(u8& byte) override; diff --git a/Source/Core/Core/State.cpp b/Source/Core/Core/State.cpp index 5d5f6e9d336c..cc1d7ae1e12c 100644 --- a/Source/Core/Core/State.cpp +++ b/Source/Core/Core/State.cpp @@ -73,7 +73,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 -constexpr u32 STATE_VERSION = 132; // Last changed in PR 9532 +constexpr u32 STATE_VERSION = 133; // Last changed in PR 9027 // Maps savestate versions to Dolphin versions. // Versions after 42 don't need to be added to this list,