diff --git a/Source/Core/Core/HW/EXI/EXI.cpp b/Source/Core/Core/HW/EXI/EXI.cpp index fd371fa076..8d7c02dafb 100644 --- a/Source/Core/Core/HW/EXI/EXI.cpp +++ b/Source/Core/Core/HW/EXI/EXI.cpp @@ -205,14 +205,25 @@ void ExpansionInterfaceManager::ChangeDevice(Slot slot, EXIDeviceType device_typ void ExpansionInterfaceManager::ChangeDevice(u8 channel, u8 device_num, EXIDeviceType device_type, CoreTiming::FromThread from_thread) { - // Let the hardware see no device for 1 second + ChangeDevice(channel, device_num, device_type, 0, m_system.GetSystemTimers().GetTicksPerSecond(), + from_thread); +} + +void ExpansionInterfaceManager::ChangeDevice(u8 channel, u8 device_num, EXIDeviceType device_type, + s64 cycles_delay_change, s64 cycles_no_device_visible, + CoreTiming::FromThread from_thread) +{ + // Let the hardware see no device for cycles_no_device_visible cycles after waiting for + // cycles_delay_change cycles auto& core_timing = m_system.GetCoreTiming(); - core_timing.ScheduleEvent(0, m_event_type_change_device, - ((u64)channel << 32) | ((u64)EXIDeviceType::None << 16) | device_num, + core_timing.ScheduleEvent(cycles_delay_change, m_event_type_change_device, + (static_cast(channel) << 32) | + (static_cast(EXIDeviceType::None) << 16) | device_num, from_thread); core_timing.ScheduleEvent( - m_system.GetSystemTimers().GetTicksPerSecond(), m_event_type_change_device, - ((u64)channel << 32) | ((u64)device_type << 16) | device_num, from_thread); + cycles_delay_change + cycles_no_device_visible, m_event_type_change_device, + (static_cast(channel) << 32) | (static_cast(device_type) << 16) | device_num, + from_thread); } CEXIChannel* ExpansionInterfaceManager::GetChannel(u32 index) diff --git a/Source/Core/Core/HW/EXI/EXI.h b/Source/Core/Core/HW/EXI/EXI.h index a4f2b899d0..36e7a9ebf8 100644 --- a/Source/Core/Core/HW/EXI/EXI.h +++ b/Source/Core/Core/HW/EXI/EXI.h @@ -10,6 +10,7 @@ #include "Common/CommonTypes.h" #include "Common/EnumFormatter.h" #include "Core/CoreTiming.h" +#include "Core/HW/SystemTimers.h" class PointerWrap; struct Sram; @@ -82,6 +83,9 @@ public: CoreTiming::FromThread from_thread = CoreTiming::FromThread::NON_CPU); void ChangeDevice(u8 channel, u8 device_num, EXIDeviceType device_type, CoreTiming::FromThread from_thread = CoreTiming::FromThread::NON_CPU); + void ChangeDevice(u8 channel, u8 device_num, EXIDeviceType device_type, s64 cycles_delay_change, + s64 cycles_no_device_visible, + CoreTiming::FromThread from_thread = CoreTiming::FromThread::NON_CPU); CEXIChannel* GetChannel(u32 index); IEXIDevice* GetDevice(Slot slot); diff --git a/Source/Core/Core/HW/EXI/EXI_Channel.cpp b/Source/Core/Core/HW/EXI/EXI_Channel.cpp index 8d47a816e4..cc91e8068b 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" #include "Core/System.h" @@ -241,48 +243,94 @@ 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.Do(m_memcard_header_data); for (int device_index = 0; device_index < NUM_DEVICES; ++device_index) { - std::unique_ptr& device = m_devices[device_index]; - EXIDeviceType type = device->m_device_type; + EXIDeviceType type = m_devices[device_index]->m_device_type; p.Do(type); - if (type == device->m_device_type) + if (type != m_devices[device_index]->m_device_type) { - device->DoState(p); - } - else - { - std::unique_ptr save_device = - EXIDevice_Create(m_system, type, m_channel_id, m_memcard_header_data); - save_device->DoState(p); - AddDevice(std::move(save_device), device_index, false); + AddDevice(EXIDevice_Create(m_system, type, m_channel_id, m_memcard_header_data), device_index, + false); } - if (type == EXIDeviceType::MemoryCardFolder && old_header_data != m_memcard_header_data && - !m_system.GetMovie().IsMovieActive()) + m_devices[device_index]->DoState(p); + + const bool is_memcard = + (type == EXIDeviceType::MemoryCard || type == EXIDeviceType::MemoryCardFolder); + if (is_memcard && !m_system.GetMovie().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(EXIDeviceType::None, device_index); - m_system.GetExpansionInterface().ChangeDevice( - m_channel_id, device_index, EXIDeviceType::MemoryCardFolder, 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.IsReadMode()) + { + // 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.Do(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 = m_system.GetSystemTimers().GetTicksPerSecond(); + m_system.GetExpansionInterface().ChangeDevice( + m_channel_id, device_index, type, cycles_per_second / 60, cycles_per_second / 3, + CoreTiming::FromThread::CPU); + } + } + 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.Do(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 13a06da652..5c86b06464 100644 --- a/Source/Core/Core/HW/EXI/EXI_DeviceMemoryCard.cpp +++ b/Source/Core/Core/HW/EXI/EXI_DeviceMemoryCard.cpp @@ -172,6 +172,19 @@ CEXIMemoryCard::GetGCIFolderPath(Slot card_slot, AllowMovieFolder allow_movie_fo return {Config::GetGCIFolderPath(card_slot, region), true}; } +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(); @@ -289,7 +302,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; @@ -302,7 +316,8 @@ void CEXIMemoryCard::SetCS(int cs) case Command::ChipErase: if (m_position > 2) { - m_memory_card->ClearAll(); + if (m_allow_writes) + m_memory_card->ClearAll(); m_status &= ~MC_STATUS_BUSY; } break; @@ -316,7 +331,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); } @@ -542,12 +558,16 @@ 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) { - auto& memory = m_system.GetMemory(); - m_memory_card->Write(m_address, size, memory.GetPointerForRange(addr, size)); + if (m_allow_writes) + { + auto& memory = m_system.GetMemory(); + m_memory_card->Write(m_address, size, memory.GetPointerForRange(addr, size)); + } 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 2264abb076..7c6ef9c1a5 100644 --- a/Source/Core/Core/HW/EXI/EXI_DeviceMemoryCard.h +++ b/Source/Core/Core/HW/EXI/EXI_DeviceMemoryCard.h @@ -64,6 +64,15 @@ public: static std::pair GetGCIFolderPath(Slot card_slot, AllowMovieFolder allow_movie_folder, Movie::MovieManager& movie); + 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); @@ -119,6 +128,7 @@ private: 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 aac9d05221..4e95301c13 100644 --- a/Source/Core/Core/State.cpp +++ b/Source/Core/Core/State.cpp @@ -99,7 +99,7 @@ static size_t s_state_writes_in_queue; static std::condition_variable s_state_write_queue_is_empty; // Don't forget to increase this after doing changes on the savestate system -constexpr u32 STATE_VERSION = 169; // Last changed in PR 13074 +constexpr u32 STATE_VERSION = 170; // Last changed in PR 9027 // Increase this if the StateExtendedHeader definition changes constexpr u32 EXTENDED_HEADER_VERSION = 1; // Last changed in PR 12217