diff --git a/src/xenia/kernel/xboxkrnl/xboxkrnl_threading.cc b/src/xenia/kernel/xboxkrnl/xboxkrnl_threading.cc index 6dec1c20e..83bb83e18 100644 --- a/src/xenia/kernel/xboxkrnl/xboxkrnl_threading.cc +++ b/src/xenia/kernel/xboxkrnl/xboxkrnl_threading.cc @@ -1138,11 +1138,34 @@ uint32_t xeKeKfAcquireSpinLock(PPCContext* ctx, X_KSPINLOCK* lock, PrefetchForCAS(lock); assert_true(lock->prcb_of_owner != static_cast(ctx->r[13])); + + uint32_t our_pcr = static_cast(ctx->r[13]); + uint8_t our_cpu = + ctx->TranslateVirtualGPR(our_pcr)->prcb_data.current_cpu; + // Lock. - while (!xe::atomic_cas(0, xe::byte_swap(static_cast(ctx->r[13])), - &lock->prcb_of_owner.value)) { - // Spin! - // TODO(benvanik): error on deadlock? + while ( + !xe::atomic_cas(0, xe::byte_swap(our_pcr), &lock->prcb_of_owner.value)) { + // On real hardware, threads sharing a Xenon HW thread are serialized by + // the kernel scheduler — the spinner would be preempted within one + // timeslice (~1ms) so the holder can make progress. In the naive + // host-thread model both threads run truly in parallel, so the spinner + // can burn its entire host quantum without giving the holder a chance. + // + // Check whether the lock holder is assigned to the same guest CPU as us. + // If so, yield the host thread aggressively (Sleep(0)) to force a host + // context switch and give the holder a chance to run and release. + // The relationship is stable — affinity doesn't change while a thread + // holds a spinlock — so one check per contention episode is sufficient. + uint32_t owner_pcr_be = lock->prcb_of_owner.value; + if (owner_pcr_be) { + uint32_t owner_pcr = xe::byte_swap(owner_pcr_be); + auto* owner_kpcr = ctx->TranslateVirtual(owner_pcr); + if (owner_kpcr->prcb_data.current_cpu == our_cpu) { + xe::threading::Sleep(std::chrono::milliseconds(0)); + continue; + } + } xe::threading::MaybeYield(); } diff --git a/src/xenia/kernel/xthread.cc b/src/xenia/kernel/xthread.cc index 19be6a6cd..f7ec64bcb 100644 --- a/src/xenia/kernel/xthread.cc +++ b/src/xenia/kernel/xthread.cc @@ -155,13 +155,17 @@ static uint8_t next_cpu = 0; static uint8_t GetFakeCpuNumber(uint8_t proc_mask) { // NOTE: proc_mask is logical processors, not physical processors or cores. if (!proc_mask) { - next_cpu = (next_cpu + 1) % 6; - return next_cpu; // is this reasonable? - // TODO(Triang3l): Does the following apply here? + // On Xbox 360, threads without an explicit processor assignment stay on + // the same hardware thread as the parent. Preserve this so that the + // guest CPU assignment reflects the game's intent — parent-child thread + // pairs that share a HW thread may rely on implicit serialization. // https://docs.microsoft.com/en-us/windows/win32/dxtecharts/coding-for-multiple-cores - // "On Xbox 360, you must explicitly assign software threads to a particular - // hardware thread by using XSetThreadProcessor. Otherwise, all child - // threads will stay on the same hardware thread as the parent." + XThread* parent = current_xthread_tls_; + if (parent) { + return parent->active_cpu(); + } + next_cpu = (next_cpu + 1) % 6; + return next_cpu; } assert_false(proc_mask & 0xC0);