Merge pull request #10448 from Pokechu22/prince-of-persia-rival-swords-unknown-opcode

Ignore unknown opcode for 0x3f
This commit is contained in:
JMC47 2022-02-13 13:58:58 -05:00 committed by GitHub
commit eed7d3b692
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 75 additions and 67 deletions

View File

@ -29,7 +29,7 @@ namespace GPFifo
// the same function could use both methods. Compile 2 different versions of each such block?
// More room for the fastmodes
alignas(32) static u8 s_gather_pipe[GATHER_PIPE_SIZE * 16];
alignas(GATHER_PIPE_SIZE) static u8 s_gather_pipe[GATHER_PIPE_EXTRA_SIZE];
static size_t GetGatherPipeCount()
{

View File

@ -9,10 +9,8 @@ class PointerWrap;
namespace GPFifo
{
enum
{
GATHER_PIPE_SIZE = 32
};
constexpr u32 GATHER_PIPE_SIZE = 32;
constexpr u32 GATHER_PIPE_EXTRA_SIZE = GATHER_PIPE_SIZE * 16;
// Init
void Init();

View File

@ -1018,7 +1018,8 @@ bool Jit64::DoJit(u32 em_address, JitBlock* b, u32 nextPC)
bool gatherPipeIntCheck = js.fifoWriteAddresses.find(op.address) != js.fifoWriteAddresses.end();
// Gather pipe writes using an immediate address are explicitly tracked.
if (jo.optimizeGatherPipe && (js.fifoBytesSinceCheck >= 32 || js.mustCheckFifo))
if (jo.optimizeGatherPipe &&
(js.fifoBytesSinceCheck >= GPFifo::GATHER_PIPE_SIZE || js.mustCheckFifo))
{
js.fifoBytesSinceCheck = 0;
js.mustCheckFifo = false;

View File

@ -835,7 +835,8 @@ bool JitArm64::DoJit(u32 em_address, JitBlock* b, u32 nextPC)
// Gather pipe writes using a non-immediate address are discovered by profiling.
bool gatherPipeIntCheck = js.fifoWriteAddresses.find(op.address) != js.fifoWriteAddresses.end();
if (jo.optimizeGatherPipe && (js.fifoBytesSinceCheck >= 32 || js.mustCheckFifo))
if (jo.optimizeGatherPipe &&
(js.fifoBytesSinceCheck >= GPFifo::GATHER_PIPE_SIZE || js.mustCheckFifo))
{
js.fifoBytesSinceCheck = 0;
js.mustCheckFifo = false;

View File

@ -17,6 +17,7 @@
#include "Core/HW/GPFifo.h"
#include "Core/HW/MMIO.h"
#include "Core/HW/ProcessorInterface.h"
#include "Core/PowerPC/PowerPC.h"
#include "Core/System.h"
#include "VideoCommon/Fifo.h"
@ -382,7 +383,7 @@ void GatherPipeBursted()
}
else
{
fifo.CPWritePointer.fetch_add(GATHER_PIPE_SIZE, std::memory_order_relaxed);
fifo.CPWritePointer.fetch_add(GPFifo::GATHER_PIPE_SIZE, std::memory_order_relaxed);
}
if (m_CPCtrlReg.GPReadEnable && m_CPCtrlReg.GPLinkEnable)
@ -396,7 +397,7 @@ void GatherPipeBursted()
if (fifo.bFF_HiWatermark.load(std::memory_order_relaxed) != 0)
CoreTiming::ForceExceptionCheck(0);
fifo.CPReadWriteDistance.fetch_add(GATHER_PIPE_SIZE, std::memory_order_seq_cst);
fifo.CPReadWriteDistance.fetch_add(GPFifo::GATHER_PIPE_SIZE, std::memory_order_seq_cst);
Fifo::RunGpu();
@ -615,12 +616,19 @@ void SetCpClearRegister()
void HandleUnknownOpcode(u8 cmd_byte, const u8* buffer, bool preprocess)
{
// Datel software uses 0x01 during startup, and Mario Party 5's Wiggler capsule
// accidentally uses 0x01-0x03 due to sending 4 more vertices than intended.
// Hardware testing indicates that 0x01-0x07 do nothing, so to avoid annoying the user with
// Datel software uses 0x01 during startup, and Mario Party 5's Wiggler capsule accidentally uses
// 0x01-0x03 due to sending 4 more vertices than intended (see https://dolp.in/i8104).
// Prince of Persia: Rival Swords sends 0x3f if the home menu is opened during the intro cutscene
// due to a game bug resulting in an incorrect vertex desc that results in the float value 1.0,
// encoded as 0x3f800000, being parsed as an opcode (see https://dolp.in/i9203).
//
// Hardware testing indicates that these opcodes do nothing, so to avoid annoying the user with
// spurious popups, we don't create a panic alert in those cases. Other unknown opcodes
// (such as 0x18) seem to result in hangs.
if (!s_is_fifo_error_seen && cmd_byte > 0x07)
// (such as 0x18) seem to result in actual hangs on real hardware, so the alert still is important
// to keep around for unexpected cases.
const bool suppress_panic_alert = (cmd_byte <= 0x7) || (cmd_byte == 0x3f);
if (!s_is_fifo_error_seen && !suppress_panic_alert)
{
s_is_fifo_error_seen = true;
@ -634,26 +642,28 @@ void HandleUnknownOpcode(u8 cmd_byte, const u8* buffer, bool preprocess)
"Further errors will be sent to the Video Backend log and\n"
"Dolphin will now likely crash or hang. Enjoy.",
cmd_byte, fmt::ptr(buffer), preprocess);
}
PanicAlertFmt("Illegal command {:02x}\n"
"CPBase: {:#010x}\n"
"CPEnd: {:#010x}\n"
"CPHiWatermark: {:#010x}\n"
"CPLoWatermark: {:#010x}\n"
"CPReadWriteDistance: {:#010x}\n"
"CPWritePointer: {:#010x}\n"
"CPReadPointer: {:#010x}\n"
"CPBreakpoint: {:#010x}\n"
"bFF_GPReadEnable: {}\n"
"bFF_BPEnable: {}\n"
"bFF_BPInt: {}\n"
"bFF_Breakpoint: {}\n"
"bFF_GPLinkEnable: {}\n"
"bFF_HiWatermarkInt: {}\n"
"bFF_LoWatermarkInt: {}\n",
cmd_byte, fifo.CPBase.load(std::memory_order_relaxed),
fifo.CPEnd.load(std::memory_order_relaxed), fifo.CPHiWatermark,
fifo.CPLoWatermark, fifo.CPReadWriteDistance.load(std::memory_order_relaxed),
// We always generate this log message, though we only generate the panic alerts once.
//
// PC and LR are generally inaccurate in dual-core and are still misleading in single-core
// due to the gather pipe queueing data. Changing GATHER_PIPE_SIZE to 1 and
// GATHER_PIPE_EXTRA_SIZE to 16 * 32 in GPFifo.h, and using the cached interpreter CPU emulation
// engine, can result in more accurate information (though it is still a bit delayed).
// PC and LR are meaningless when using the fifoplayer, and will generally not be helpful if the
// unknown opcode is inside of a display list. Also note that the changes in GPFifo.h are not
// accurate and may introduce timing issues.
ERROR_LOG_FMT(VIDEO,
"FIFO: Unknown Opcode {:#04x} @ {}, preprocessing = {}, CPBase: {:#010x}, CPEnd: "
"{:#010x}, CPHiWatermark: {:#010x}, CPLoWatermark: {:#010x}, CPReadWriteDistance: "
"{:#010x}, CPWritePointer: {:#010x}, CPReadPointer: {:#010x}, CPBreakpoint: "
"{:#010x}, bFF_GPReadEnable: {}, bFF_BPEnable: {}, bFF_BPInt: {}, bFF_Breakpoint: "
"{}, bFF_GPLinkEnable: {}, bFF_HiWatermarkInt: {}, bFF_LoWatermarkInt: {}, "
"approximate PC: {:08x}, approximate LR: {:08x}",
cmd_byte, fmt::ptr(buffer), preprocess ? "yes" : "no",
fifo.CPBase.load(std::memory_order_relaxed),
fifo.CPEnd.load(std::memory_order_relaxed), fifo.CPHiWatermark, fifo.CPLoWatermark,
fifo.CPReadWriteDistance.load(std::memory_order_relaxed),
fifo.CPWritePointer.load(std::memory_order_relaxed),
fifo.CPReadPointer.load(std::memory_order_relaxed),
fifo.CPBreakpoint.load(std::memory_order_relaxed),
@ -663,12 +673,7 @@ void HandleUnknownOpcode(u8 cmd_byte, const u8* buffer, bool preprocess)
fifo.bFF_Breakpoint.load(std::memory_order_relaxed) ? "true" : "false",
fifo.bFF_GPLinkEnable.load(std::memory_order_relaxed) ? "true" : "false",
fifo.bFF_HiWatermarkInt.load(std::memory_order_relaxed) ? "true" : "false",
fifo.bFF_LoWatermarkInt.load(std::memory_order_relaxed) ? "true" : "false");
}
// We always generate this log message, though we only generate the panic alerts once.
ERROR_LOG_FMT(VIDEO, "FIFO: Unknown Opcode ({:#04x} @ {}, preprocessing = {})", cmd_byte,
fmt::ptr(buffer), preprocess ? "yes" : "no");
fifo.bFF_LoWatermarkInt.load(std::memory_order_relaxed) ? "true" : "false", PC, LR);
}
} // namespace CommandProcessor

View File

@ -97,7 +97,6 @@ enum
enum
{
GATHER_PIPE_SIZE = 32,
INT_CAUSE_CP = 0x800
};

View File

@ -17,6 +17,7 @@
#include "Core/Config/MainSettings.h"
#include "Core/ConfigManager.h"
#include "Core/CoreTiming.h"
#include "Core/HW/GPFifo.h"
#include "Core/HW/Memmap.h"
#include "Core/Host.h"
#include "Core/System.h"
@ -249,13 +250,14 @@ void* PopFifoAuxBuffer(size_t size)
// Description: RunGpuLoop() sends data through this function.
static void ReadDataFromFifo(u32 readPtr)
{
constexpr size_t len = 32;
if (len > static_cast<size_t>(s_video_buffer + FIFO_SIZE - s_video_buffer_write_ptr))
if (GPFifo::GATHER_PIPE_SIZE >
static_cast<size_t>(s_video_buffer + FIFO_SIZE - s_video_buffer_write_ptr))
{
const size_t existing_len = s_video_buffer_write_ptr - s_video_buffer_read_ptr;
if (len > static_cast<size_t>(FIFO_SIZE - existing_len))
if (GPFifo::GATHER_PIPE_SIZE > static_cast<size_t>(FIFO_SIZE - existing_len))
{
PanicAlertFmt("FIFO out of bounds (existing {} + new {} > {})", existing_len, len, FIFO_SIZE);
PanicAlertFmt("FIFO out of bounds (existing {} + new {} > {})", existing_len,
GPFifo::GATHER_PIPE_SIZE, FIFO_SIZE);
return;
}
memmove(s_video_buffer, s_video_buffer_read_ptr, existing_len);
@ -263,16 +265,15 @@ static void ReadDataFromFifo(u32 readPtr)
s_video_buffer_read_ptr = s_video_buffer;
}
// Copy new video instructions to s_video_buffer for future use in rendering the new picture
Memory::CopyFromEmu(s_video_buffer_write_ptr, readPtr, len);
s_video_buffer_write_ptr += len;
Memory::CopyFromEmu(s_video_buffer_write_ptr, readPtr, GPFifo::GATHER_PIPE_SIZE);
s_video_buffer_write_ptr += GPFifo::GATHER_PIPE_SIZE;
}
// The deterministic_gpu_thread version.
static void ReadDataFromFifoOnCPU(u32 readPtr)
{
constexpr size_t len = 32;
u8* write_ptr = s_video_buffer_write_ptr;
if (len > static_cast<size_t>(s_video_buffer + FIFO_SIZE - write_ptr))
if (GPFifo::GATHER_PIPE_SIZE > static_cast<size_t>(s_video_buffer + FIFO_SIZE - write_ptr))
{
// We can't wrap around while the GPU is working on the data.
// This should be very rare due to the reset in SyncGPU.
@ -290,17 +291,18 @@ static void ReadDataFromFifoOnCPU(u32 readPtr)
}
write_ptr = s_video_buffer_write_ptr;
const size_t existing_len = write_ptr - s_video_buffer_pp_read_ptr;
if (len > static_cast<size_t>(FIFO_SIZE - existing_len))
if (GPFifo::GATHER_PIPE_SIZE > static_cast<size_t>(FIFO_SIZE - existing_len))
{
PanicAlertFmt("FIFO out of bounds (existing {} + new {} > {})", existing_len, len, FIFO_SIZE);
PanicAlertFmt("FIFO out of bounds (existing {} + new {} > {})", existing_len,
GPFifo::GATHER_PIPE_SIZE, FIFO_SIZE);
return;
}
}
Memory::CopyFromEmu(s_video_buffer_write_ptr, readPtr, len);
Memory::CopyFromEmu(s_video_buffer_write_ptr, readPtr, GPFifo::GATHER_PIPE_SIZE);
s_video_buffer_pp_read_ptr = OpcodeDecoder::RunFifo<true>(
DataReader(s_video_buffer_pp_read_ptr, write_ptr + len), nullptr);
DataReader(s_video_buffer_pp_read_ptr, write_ptr + GPFifo::GATHER_PIPE_SIZE), nullptr);
// This would have to be locked if the GPU thread didn't spin.
s_video_buffer_write_ptr = write_ptr + len;
s_video_buffer_write_ptr = write_ptr + GPFifo::GATHER_PIPE_SIZE;
}
void ResetVideoBuffer()
@ -362,20 +364,22 @@ void RunGpuLoop()
if (readPtr == fifo.CPEnd.load(std::memory_order_relaxed))
readPtr = fifo.CPBase.load(std::memory_order_relaxed);
else
readPtr += 32;
readPtr += GPFifo::GATHER_PIPE_SIZE;
ASSERT_MSG(COMMANDPROCESSOR,
(s32)fifo.CPReadWriteDistance.load(std::memory_order_relaxed) - 32 >= 0,
const s32 distance =
static_cast<s32>(fifo.CPReadWriteDistance.load(std::memory_order_relaxed)) -
GPFifo::GATHER_PIPE_SIZE;
ASSERT_MSG(COMMANDPROCESSOR, distance >= 0,
"Negative fifo.CPReadWriteDistance = {} in FIFO Loop !\nThat can produce "
"instability in the game. Please report it.",
fifo.CPReadWriteDistance.load(std::memory_order_relaxed) - 32);
distance);
u8* write_ptr = s_video_buffer_write_ptr;
s_video_buffer_read_ptr = OpcodeDecoder::RunFifo(
DataReader(s_video_buffer_read_ptr, write_ptr), &cyclesExecuted);
fifo.CPReadPointer.store(readPtr, std::memory_order_relaxed);
fifo.CPReadWriteDistance.fetch_sub(32, std::memory_order_seq_cst);
fifo.CPReadWriteDistance.fetch_sub(GPFifo::GATHER_PIPE_SIZE, std::memory_order_seq_cst);
if ((write_ptr - s_video_buffer_read_ptr) == 0)
{
fifo.SafeCPReadPointer.store(fifo.CPReadPointer.load(std::memory_order_relaxed),
@ -498,10 +502,10 @@ static int RunGpuOnCpu(int ticks)
}
else
{
fifo.CPReadPointer.fetch_add(32, std::memory_order_relaxed);
fifo.CPReadPointer.fetch_add(GPFifo::GATHER_PIPE_SIZE, std::memory_order_relaxed);
}
fifo.CPReadWriteDistance.fetch_sub(32, std::memory_order_relaxed);
fifo.CPReadWriteDistance.fetch_sub(GPFifo::GATHER_PIPE_SIZE, std::memory_order_relaxed);
}
CommandProcessor::SetCPStatusFromGPU();