From 6b74bcec7c43667009638cf10505d71694708884 Mon Sep 17 00:00:00 2001 From: svc64 Date: Wed, 13 Dec 2023 19:11:57 +0200 Subject: [PATCH] Refactor stepping code, fix stepping over a breakpoint --- src/ARMeilleure/State/ExecutionContext.cs | 10 +++- src/ARMeilleure/Translation/Translator.cs | 3 +- src/Ryujinx.Cpu/AppleHv/HvExecutionContext.cs | 12 ++--- src/Ryujinx.Cpu/ExceptionCallbacks.cs | 8 ++++ src/Ryujinx.Cpu/IExecutionContext.cs | 12 +---- src/Ryujinx.Cpu/Jit/JitExecutionContext.cs | 9 ++-- src/Ryujinx.HLE/Debugger/Debugger.cs | 29 +++-------- .../Debugger/IDebuggableProcess.cs | 4 +- .../HOS/Kernel/Process/KProcess.cs | 48 ++++++++++++++----- .../Kernel/Process/ProcessExecutionContext.cs | 1 - 10 files changed, 74 insertions(+), 62 deletions(-) diff --git a/src/ARMeilleure/State/ExecutionContext.cs b/src/ARMeilleure/State/ExecutionContext.cs index 2df13e286..4fa322eef 100644 --- a/src/ARMeilleure/State/ExecutionContext.cs +++ b/src/ARMeilleure/State/ExecutionContext.cs @@ -98,18 +98,19 @@ namespace ARMeilleure.State private readonly ExceptionCallbackNoArgs _interruptCallback; private readonly ExceptionCallback _breakCallback; + private readonly ExceptionCallbackNoArgs _stepCallback; private readonly ExceptionCallback _supervisorCallback; private readonly ExceptionCallback _undefinedCallback; internal int ShouldStep; public ulong DebugPc { get; set; } - public Barrier StepBarrier { get; } public ExecutionContext( IJitMemoryAllocator allocator, ICounter counter, ExceptionCallbackNoArgs interruptCallback = null, ExceptionCallback breakCallback = null, + ExceptionCallbackNoArgs stepCallback = null, ExceptionCallback supervisorCallback = null, ExceptionCallback undefinedCallback = null) { @@ -117,11 +118,11 @@ namespace ARMeilleure.State _counter = counter; _interruptCallback = interruptCallback; _breakCallback = breakCallback; + _stepCallback = stepCallback; _supervisorCallback = supervisorCallback; _undefinedCallback = undefinedCallback; Running = true; - StepBarrier = new Barrier(2); _nativeContext.SetCounter(MinCountForCheck); } @@ -155,6 +156,11 @@ namespace ARMeilleure.State Interrupted = true; } + public void StepHandler() + { + _stepCallback.Invoke(this); + } + public void RequestDebugStep() { Interlocked.Exchange(ref ShouldStep, 1); diff --git a/src/ARMeilleure/Translation/Translator.cs b/src/ARMeilleure/Translation/Translator.cs index e68fd34d9..df9fa8f01 100644 --- a/src/ARMeilleure/Translation/Translator.cs +++ b/src/ARMeilleure/Translation/Translator.cs @@ -148,8 +148,7 @@ namespace ARMeilleure.Translation if (Interlocked.CompareExchange(ref context.ShouldStep, 0, 1) == 1) { context.DebugPc = Step(context, context.DebugPc); - context.StepBarrier.SignalAndWait(); - context.StepBarrier.SignalAndWait(); + context.StepHandler(); } else { diff --git a/src/Ryujinx.Cpu/AppleHv/HvExecutionContext.cs b/src/Ryujinx.Cpu/AppleHv/HvExecutionContext.cs index 7b3b62c22..cc975fb53 100644 --- a/src/Ryujinx.Cpu/AppleHv/HvExecutionContext.cs +++ b/src/Ryujinx.Cpu/AppleHv/HvExecutionContext.cs @@ -93,7 +93,6 @@ namespace Ryujinx.Cpu.AppleHv _shadowContext = new HvExecutionContextShadow(); _impl = _shadowContext; _exceptionCallbacks = exceptionCallbacks; - StepBarrier = new(2); Running = true; } @@ -119,6 +118,11 @@ namespace Ryujinx.Cpu.AppleHv _exceptionCallbacks.BreakCallback?.Invoke(this, address, imm); } + private void StepHandler() + { + _exceptionCallbacks.StepCallback?.Invoke(this); + } + private void SupervisorCallHandler(ulong address, int imm) { _exceptionCallbacks.SupervisorCallback?.Invoke(this, address, imm); @@ -167,8 +171,6 @@ namespace Ryujinx.Cpu.AppleHv } } - public Barrier StepBarrier { get; } - /// public void StopRunning() { @@ -272,9 +274,7 @@ namespace Ryujinx.Cpu.AppleHv HvApi.hv_vcpu_set_sys_reg(vcpuHandle, HvSysReg.SPSR_EL1, spsr).ThrowOnError(); HvApi.hv_vcpu_set_sys_reg(vcpuHandle, HvSysReg.MDSCR_EL1, 0); ReturnToPool(vcpu); - StepBarrier.SignalAndWait(); - StepBarrier.SignalAndWait(); - InterruptHandler(); + StepHandler(); vcpu = RentFromPool(memoryManager.AddressSpace, vcpu); break; case ExceptionClass.BrkAarch64: diff --git a/src/Ryujinx.Cpu/ExceptionCallbacks.cs b/src/Ryujinx.Cpu/ExceptionCallbacks.cs index d9293302b..6e50b4d70 100644 --- a/src/Ryujinx.Cpu/ExceptionCallbacks.cs +++ b/src/Ryujinx.Cpu/ExceptionCallbacks.cs @@ -29,6 +29,11 @@ namespace Ryujinx.Cpu /// public readonly ExceptionCallback BreakCallback; + /// + /// Handler for CPU software interrupts caused by single-stepping. + /// + public readonly ExceptionCallbackNoArgs StepCallback; + /// /// Handler for CPU software interrupts caused by the Arm SVC instruction. /// @@ -47,16 +52,19 @@ namespace Ryujinx.Cpu /// /// Handler for CPU interrupts triggered using /// Handler for CPU software interrupts caused by the Arm BRK instruction + /// Handler for CPU software interrupts caused by single-stepping /// Handler for CPU software interrupts caused by the Arm SVC instruction /// Handler for CPU software interrupts caused by any undefined Arm instruction public ExceptionCallbacks( ExceptionCallbackNoArgs interruptCallback = null, ExceptionCallback breakCallback = null, + ExceptionCallbackNoArgs stepCallback = null, ExceptionCallback supervisorCallback = null, ExceptionCallback undefinedCallback = null) { InterruptCallback = interruptCallback; BreakCallback = breakCallback; + StepCallback = stepCallback; SupervisorCallback = supervisorCallback; UndefinedCallback = undefinedCallback; } diff --git a/src/Ryujinx.Cpu/IExecutionContext.cs b/src/Ryujinx.Cpu/IExecutionContext.cs index eb5f93147..df0c94278 100644 --- a/src/Ryujinx.Cpu/IExecutionContext.cs +++ b/src/Ryujinx.Cpu/IExecutionContext.cs @@ -121,20 +121,10 @@ namespace Ryujinx.Cpu /// /// The thread might not pause immediately. /// One must not assume that guest code is no longer being executed by the thread after calling this function. - /// After single stepping, the thread should signal and wait on twice to allow - /// changing the thread state after stepping. + /// After single stepping, the thread should call call . /// void RequestDebugStep(); - /// - /// Step barrier - /// - /// - /// The participant count should be 2. - /// Should be signaled and waited on twice after single-stepping. - /// - Barrier StepBarrier { get; } - /// /// Current Program Counter (for debugging). /// diff --git a/src/Ryujinx.Cpu/Jit/JitExecutionContext.cs b/src/Ryujinx.Cpu/Jit/JitExecutionContext.cs index e12c28d30..7bae07859 100644 --- a/src/Ryujinx.Cpu/Jit/JitExecutionContext.cs +++ b/src/Ryujinx.Cpu/Jit/JitExecutionContext.cs @@ -74,6 +74,7 @@ namespace Ryujinx.Cpu.Jit counter, InterruptHandler, BreakHandler, + StepHandler, SupervisorCallHandler, UndefinedHandler); @@ -102,6 +103,11 @@ namespace Ryujinx.Cpu.Jit _exceptionCallbacks.BreakCallback?.Invoke(this, address, imm); } + private void StepHandler(ExecutionContext context) + { + _exceptionCallbacks.StepCallback?.Invoke(this); + } + private void SupervisorCallHandler(ExecutionContext context, ulong address, int imm) { _exceptionCallbacks.SupervisorCallback?.Invoke(this, address, imm); @@ -128,9 +134,6 @@ namespace Ryujinx.Cpu.Jit set => _impl.DebugPc = value; } - /// - public Barrier StepBarrier => _impl.StepBarrier; - /// public void StopRunning() { diff --git a/src/Ryujinx.HLE/Debugger/Debugger.cs b/src/Ryujinx.HLE/Debugger/Debugger.cs index 5c5eb299f..4a4979a9c 100644 --- a/src/Ryujinx.HLE/Debugger/Debugger.cs +++ b/src/Ryujinx.HLE/Debugger/Debugger.cs @@ -737,34 +737,17 @@ namespace Ryujinx.HLE.Debugger } } - public void ThreadBreak(IExecutionContext ctx, ulong address, int imm) + public void BreakHandler(IExecutionContext ctx, ulong address, int imm) { Logger.Notice.Print(LogClass.GdbStub, $"Break hit on thread {ctx.ThreadUid} at pc {address:x016}"); Messages.Add(new ThreadBreakMessage(ctx, address, imm)); + DebugProcess.DebugInterruptHandler(ctx); + } - KThread currentThread = DebugProcess.GetThread(ctx.ThreadUid); - - if (currentThread.Context.Running && - currentThread.Owner != null && - currentThread.GetUserDisableCount() != 0 && - currentThread.Owner.PinnedThreads[currentThread.CurrentCore] == null) - { - KernelContext.CriticalSection.Enter(); - - currentThread.Owner.PinThread(currentThread); - - currentThread.SetUserInterruptFlag(); - - KernelContext.CriticalSection.Leave(); - } - - if (currentThread.IsSchedulable) - { - KernelContext.Schedulers[currentThread.CurrentCore].Schedule(); - } - - currentThread.HandlePostSyscall(); + public void StepHandler(IExecutionContext ctx) + { + DebugProcess.DebugInterruptHandler(ctx); } } } diff --git a/src/Ryujinx.HLE/Debugger/IDebuggableProcess.cs b/src/Ryujinx.HLE/Debugger/IDebuggableProcess.cs index 609b9c8fb..b8f8bd30f 100644 --- a/src/Ryujinx.HLE/Debugger/IDebuggableProcess.cs +++ b/src/Ryujinx.HLE/Debugger/IDebuggableProcess.cs @@ -1,4 +1,5 @@ -using Ryujinx.HLE.HOS.Kernel.Threading; +using Ryujinx.Cpu; +using Ryujinx.HLE.HOS.Kernel.Threading; using Ryujinx.Memory; namespace Ryujinx.HLE.Debugger @@ -11,6 +12,7 @@ namespace Ryujinx.HLE.Debugger KThread GetThread(ulong threadUid); DebugState GetDebugState(); ulong[] GetThreadUids(); + public void DebugInterruptHandler(IExecutionContext ctx); IVirtualMemoryManager CpuMemory { get; } void InvalidateCacheRegion(ulong address, ulong size); } diff --git a/src/Ryujinx.HLE/HOS/Kernel/Process/KProcess.cs b/src/Ryujinx.HLE/HOS/Kernel/Process/KProcess.cs index 394350d98..0dd280489 100644 --- a/src/Ryujinx.HLE/HOS/Kernel/Process/KProcess.cs +++ b/src/Ryujinx.HLE/HOS/Kernel/Process/KProcess.cs @@ -13,6 +13,7 @@ using System.Collections.Generic; using System.Linq; using System.Threading; using ExceptionCallback = Ryujinx.Cpu.ExceptionCallback; +using ExceptionCallbackNoArgs = Ryujinx.Cpu.ExceptionCallbackNoArgs; namespace Ryujinx.HLE.HOS.Kernel.Process { @@ -733,15 +734,18 @@ namespace Ryujinx.HLE.HOS.Kernel.Process public IExecutionContext CreateExecutionContext() { ExceptionCallback breakCallback = null; + ExceptionCallbackNoArgs stepCallback = null; if (KernelContext.Device.Configuration.EnableGdbStub) { - breakCallback = KernelContext.Device.Debugger.ThreadBreak; + breakCallback = KernelContext.Device.Debugger.BreakHandler; + stepCallback = KernelContext.Device.Debugger.StepHandler; } return Context?.CreateExecutionContext(new ExceptionCallbacks( InterruptHandler, breakCallback, + stepCallback, KernelContext.SyscallHandler.SvcCall, UndefinedInstructionHandler)); } @@ -1189,14 +1193,17 @@ namespace Ryujinx.HLE.HOS.Kernel.Process private class DebuggerInterface : IDebuggableProcess { + private Barrier StepBarrier; private readonly KProcess _parent; - private readonly KernelContext KernelContext; + private readonly KernelContext _kernelContext; + private KThread steppingThread; private int _debugState = (int)DebugState.Running; public DebuggerInterface(KProcess p) { _parent = p; - KernelContext = p.KernelContext; + _kernelContext = p.KernelContext; + StepBarrier = new(2); } public void DebugStop() @@ -1207,7 +1214,7 @@ namespace Ryujinx.HLE.HOS.Kernel.Process return; } - KernelContext.CriticalSection.Enter(); + _kernelContext.CriticalSection.Enter(); lock (_parent._threadingLock) { foreach (KThread thread in _parent._threads) @@ -1219,7 +1226,7 @@ namespace Ryujinx.HLE.HOS.Kernel.Process } _debugState = (int)DebugState.Stopped; - KernelContext.CriticalSection.Leave(); + _kernelContext.CriticalSection.Leave(); } public void DebugContinue() @@ -1230,7 +1237,7 @@ namespace Ryujinx.HLE.HOS.Kernel.Process return; } - KernelContext.CriticalSection.Enter(); + _kernelContext.CriticalSection.Enter(); lock (_parent._threadingLock) { foreach (KThread thread in _parent._threads) @@ -1238,7 +1245,7 @@ namespace Ryujinx.HLE.HOS.Kernel.Process thread.Resume(ThreadSchedState.ThreadPauseFlag); } } - KernelContext.CriticalSection.Leave(); + _kernelContext.CriticalSection.Leave(); } public bool DebugStep(KThread target) @@ -1247,7 +1254,8 @@ namespace Ryujinx.HLE.HOS.Kernel.Process { return false; } - KernelContext.CriticalSection.Enter(); + _kernelContext.CriticalSection.Enter(); + steppingThread = target; bool waiting = target.MutexOwner != null || target.WaitingSync || target.WaitingInArbitration; target.Context.RequestDebugStep(); if (waiting) @@ -1264,12 +1272,12 @@ namespace Ryujinx.HLE.HOS.Kernel.Process { target.Resume(ThreadSchedState.ThreadPauseFlag); } - KernelContext.CriticalSection.Leave(); + _kernelContext.CriticalSection.Leave(); - target.Context.StepBarrier.SignalAndWait(); - target.Context.StepBarrier.SignalAndWait(); + StepBarrier.SignalAndWait(); - KernelContext.CriticalSection.Enter(); + _kernelContext.CriticalSection.Enter(); + steppingThread = null; if (waiting) { lock (_parent._threadingLock) @@ -1284,7 +1292,8 @@ namespace Ryujinx.HLE.HOS.Kernel.Process { target.Suspend(ThreadSchedState.ThreadPauseFlag); } - KernelContext.CriticalSection.Leave(); + _kernelContext.CriticalSection.Leave(); + StepBarrier.SignalAndWait(); return true; } @@ -1309,6 +1318,19 @@ namespace Ryujinx.HLE.HOS.Kernel.Process } } + public void DebugInterruptHandler(IExecutionContext ctx) + { + _kernelContext.CriticalSection.Enter(); + bool stepping = steppingThread != null; + _kernelContext.CriticalSection.Leave(); + if (stepping) + { + StepBarrier.SignalAndWait(); + StepBarrier.SignalAndWait(); + } + _parent.InterruptHandler(ctx); + } + public IVirtualMemoryManager CpuMemory { get { return _parent.CpuMemory; } } public void InvalidateCacheRegion(ulong address, ulong size) diff --git a/src/Ryujinx.HLE/HOS/Kernel/Process/ProcessExecutionContext.cs b/src/Ryujinx.HLE/HOS/Kernel/Process/ProcessExecutionContext.cs index 717ed9c77..f0e44c4b7 100644 --- a/src/Ryujinx.HLE/HOS/Kernel/Process/ProcessExecutionContext.cs +++ b/src/Ryujinx.HLE/HOS/Kernel/Process/ProcessExecutionContext.cs @@ -25,7 +25,6 @@ namespace Ryujinx.HLE.HOS.Kernel.Process private readonly ulong[] _x = new ulong[32]; public ulong DebugPc { get; set; } - public Barrier StepBarrier { get; } public ulong GetX(int index) => _x[index]; public void SetX(int index, ulong value) => _x[index] = value;