diff --git a/CMakeLists.txt b/CMakeLists.txt index 23b026ff..81caa900 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -45,6 +45,7 @@ set(AC6RECOMP_SOURCES src/ac6_backend_fixes/ac6_fps_physics_fix.cpp src/ac6_backend_fixes/ac6_fullres_effects.cpp src/ac6_backend_fixes/ac6_kbm_input.cpp + src/ac6_backend_fixes/ac6_storage_submit_order_fix.cpp src/ac6_backend_fixes/ac6_widescreen.cpp ) diff --git a/ac6recomp_config.toml b/ac6recomp_config.toml index 2411fbc6..153028d2 100644 --- a/ac6recomp_config.toml +++ b/ac6recomp_config.toml @@ -10796,3 +10796,82 @@ registers = ["r3"] address = 0x822CD758 name = "ac6PacBlockConsumerProbe" registers = ["r3"] + +# Storage job submit-order fix (ac6_fix_storage_submit_order). +# +# Twelve call sites submit an async storage job to JobPoolB::Submit (0x821D4A78) +# and only store the job's operation code to [job+4] AFTER the call returns; the +# pool-B worker runs job->vtbl[1] as soon as the job is queued, so a fast worker +# reads a stale operation code (previous op on the reused object). The retail +# title update reorders every one of these sites to store the code BEFORE the +# submit call (verified instruction-level at all 12). These hooks fire at each +# `bl` site, where r5 already holds the job pointer (Submit's third argument), +# and perform the same early store. The original post-call store still runs and +# re-stores the same value, which is harmless: the code is a submit-time input +# the worker only reads. Implementation: src/ac6_backend_fixes/ +# ac6_storage_submit_order_fix.cpp. +# +# Sites: 7 per-operation wrappers 0x821D3BE8..0x821D4078 (ops 2,3,6,7,A,B,C), +# 0x821C72F8 (op D), 0x821C88C0 (ops 8,9), 0x821C8FF0 (op 0x10) -- these are the +# save/replay/content-cache layer under sub_82158D90 and CResourceLoaderDpl -- +# plus 0x821C59C0 where the op code is carried in r29. + +[[midasm_hook]] +address = 0x821D3C8C +name = "ac6StorageOrderHookOp2" +registers = ["r5"] + +[[midasm_hook]] +address = 0x821D3D0C +name = "ac6StorageOrderHookOp3" +registers = ["r5"] + +[[midasm_hook]] +address = 0x821D3DC8 +name = "ac6StorageOrderHookOp6" +registers = ["r5"] + +[[midasm_hook]] +address = 0x821D3E58 +name = "ac6StorageOrderHookOp7" +registers = ["r5"] + +[[midasm_hook]] +address = 0x821D3F70 +name = "ac6StorageOrderHookOpA" +registers = ["r5"] + +[[midasm_hook]] +address = 0x821D4030 +name = "ac6StorageOrderHookOpB" +registers = ["r5"] + +[[midasm_hook]] +address = 0x821D40F0 +name = "ac6StorageOrderHookOpC" +registers = ["r5"] + +[[midasm_hook]] +address = 0x821C7538 +name = "ac6StorageOrderHookOpD" +registers = ["r5"] + +[[midasm_hook]] +address = 0x821C8EC4 +name = "ac6StorageOrderHookOp8" +registers = ["r5"] + +[[midasm_hook]] +address = 0x821C8F78 +name = "ac6StorageOrderHookOp9" +registers = ["r5"] + +[[midasm_hook]] +address = 0x821C9298 +name = "ac6StorageOrderHookOp10" +registers = ["r5"] + +[[midasm_hook]] +address = 0x821C5A78 +name = "ac6StorageOrderHookVar" +registers = ["r5", "r29"] diff --git a/src/ac6_backend_fixes/ac6_storage_submit_order_fix.cpp b/src/ac6_backend_fixes/ac6_storage_submit_order_fix.cpp new file mode 100644 index 00000000..fb6a90c0 --- /dev/null +++ b/src/ac6_backend_fixes/ac6_storage_submit_order_fix.cpp @@ -0,0 +1,131 @@ +// AC6 storage job submit-order race - preventive fix, ported from the retail +// title update. +// +// MECHANISM (found by an offline diff of the retail title update against the +// base executable this port is compiled from): +// +// The async save/storage layer - save.dat / replay.dat / dlcolorname.dat I/O +// and CResourceLoaderDpl's file reads, all reached through the save/load +// manager sub_82158D90 and its per-operation submitters - queues jobs on the +// game's second worker pool via JobPoolB::Submit (rex_sub_821D4A78). At twelve +// call sites the submitter stores the job's OPERATION CODE to [job+4] only +// AFTER Submit returns: +// +// mr r5, r31 ; job +// bl JobPoolB::Submit ; worker may run job->vtbl[1] from here on +// li r10, +// stw r3, 0x61A4(r31) ; ticket (submitter-side only, safe late) +// stw r10, 4(r31) ; operation code - TOO LATE +// +// The pool-B worker (rex_sub_821D4F20) invokes job->vtbl[1](job, arg) as soon +// as it dequeues the job. The job objects are reused, so a worker that wins +// the race reads the PREVIOUS operation's code and performs the wrong storage +// operation. This is the same submit-side race family as the effect mode-word +// flicker (ac6_effect_mode_fix): on the 360 fixed core assignment and guest +// priorities made the window effectively unhittable in the common case; the +// port discards guest priorities and affinities, and its instant I/O keeps +// workers hot during the burst submits the save-select and DLC-content-check +// screens produce. +// +// The retail title update fixes exactly this: at all twelve sites it moves the +// [job+4] store BEFORE the Submit call (verified instruction-by-instruction on +// the patched image; the update leaves JobPoolB itself untouched). The port +// runs the base executable, so it inherits the unfixed ordering. +// +// THE FIX: a midasm hook at each of the twelve `bl` sites stores the operation +// code to [job+4] before the call - r5 already holds the job pointer (Submit's +// job argument, verified at every site), and the code is a per-site constant +// (2,3,6,7,8,9,0xA,0xB,0xC,0xD,0x10) except at 0x821C5A78 where it is carried +// in r29. The original post-call store still executes and re-stores the same +// value; that is harmless - the code is a submit-time input the worker reads, +// exactly why the title update wants it written before submission. +// +// No in-port failure has ever been attributed to this race; the fix is +// insurance against a rare, unreproducible wrong-operation on save data, taken +// because the vendor considered the reorder worth shipping. Semantics with the +// fix are identical to the unfixed path whenever the race does not fire. + +#include +#include + +#include +#include +#include +#include +#include + +REXCVAR_DEFINE_BOOL( + ac6_fix_storage_submit_order, true, "AC6", + "Store each async storage job's operation code before JobPoolB::Submit instead of " + "after it, as the retail title update does at the same twelve call sites. Prevents " + "a fast pool-B worker from reading a stale operation code on a reused job object " + "(rare wrong save/replay/content-cache operation). On by default; semantics are " + "identical to stock whenever the race does not occur."); + +namespace { + +struct SiteInfo { + uint32_t guest_bl; // guest address of the bl JobPoolB::Submit instruction + uint32_t op_code; // operation code stored to [job+4]; 0 = carried in r29 +}; + +// Indices must match the kSite* constants below. +constexpr SiteInfo kSites[] = { + {0x821D3C8C, 0x2}, {0x821D3D0C, 0x3}, {0x821D3DC8, 0x6}, {0x821D3E58, 0x7}, + {0x821D3F70, 0xA}, {0x821D4030, 0xB}, {0x821D40F0, 0xC}, {0x821C7538, 0xD}, + {0x821C8EC4, 0x8}, {0x821C8F78, 0x9}, {0x821C9298, 0x10}, {0x821C5A78, 0x0}, +}; +constexpr size_t kSiteCount = sizeof(kSites) / sizeof(kSites[0]); + +std::atomic g_engaged_total{0}; +std::atomic g_site_logged[kSiteCount] = {}; + +void StoreOpCodeEarly(unsigned site, uint32_t job_va, uint32_t op_code) { + if (!REXCVAR_GET(ac6_fix_storage_submit_order)) { + return; + } + if (!job_va) { + // Defensive: never seen (r5 is always a live object here), but a null job + // would mean our model of the site is wrong - log once rather than fault. + if (!g_site_logged[site].exchange(true, std::memory_order_relaxed)) { + REXLOG_WARN("ac6_fix_storage_submit_order: null job at site 0x{:08X} - site skipped", + kSites[site].guest_bl); + } + return; + } + auto* memory = REX_KERNEL_MEMORY(); + rex::memory::store_and_swap(memory->TranslateVirtual(job_va + 4), op_code); + const uint32_t total = g_engaged_total.fetch_add(1, std::memory_order_relaxed) + 1; + if (!g_site_logged[site].exchange(true, std::memory_order_relaxed)) { + // Verified live 2026-08-06: six sites engaged within 100 ms of the boot + // storage check, all on one reused job object cycling operation codes - + // exactly the burst-submit shape the race analysis predicted. + REXLOG_INFO( + "ac6_fix_storage_submit_order ENGAGED: site 0x{:08X} job=0x{:08X} op=0x{:X} " + "(engagements so far: {})", + kSites[site].guest_bl, job_va, op_code, total); + } +} + +} // namespace + +// One hook per site; the constant is baked here so the hook needs only r5. +// Signatures must match the registers lists in ac6recomp_config.toml. +void ac6StorageOrderHookOp2(PPCRegister& r5) { StoreOpCodeEarly(0, r5.u32, 0x2); } +void ac6StorageOrderHookOp3(PPCRegister& r5) { StoreOpCodeEarly(1, r5.u32, 0x3); } +void ac6StorageOrderHookOp6(PPCRegister& r5) { StoreOpCodeEarly(2, r5.u32, 0x6); } +void ac6StorageOrderHookOp7(PPCRegister& r5) { StoreOpCodeEarly(3, r5.u32, 0x7); } +void ac6StorageOrderHookOpA(PPCRegister& r5) { StoreOpCodeEarly(4, r5.u32, 0xA); } +void ac6StorageOrderHookOpB(PPCRegister& r5) { StoreOpCodeEarly(5, r5.u32, 0xB); } +void ac6StorageOrderHookOpC(PPCRegister& r5) { StoreOpCodeEarly(6, r5.u32, 0xC); } +void ac6StorageOrderHookOpD(PPCRegister& r5) { StoreOpCodeEarly(7, r5.u32, 0xD); } +void ac6StorageOrderHookOp8(PPCRegister& r5) { StoreOpCodeEarly(8, r5.u32, 0x8); } +void ac6StorageOrderHookOp9(PPCRegister& r5) { StoreOpCodeEarly(9, r5.u32, 0x9); } +void ac6StorageOrderHookOp10(PPCRegister& r5) { StoreOpCodeEarly(10, r5.u32, 0x10); } + +// 0x821C5A78: the operation code is computed by the caller and carried in r29 +// (a nonvolatile, already live before the call - the stock post-call store is +// `stw r29,4(r31)`). +void ac6StorageOrderHookVar(PPCRegister& r5, PPCRegister& r29) { + StoreOpCodeEarly(11, r5.u32, r29.u32); +}