Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix CfgCpu messing up graph on external interruptions. Make interrupt vector table a memory based array. #855

Merged
merged 1 commit into from
Sep 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions src/Spice86.Core/Emulator/CPU/CfgCpu/CfgCpu.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,18 @@ public class CfgCpu : IInstructionExecutor {
private readonly InstructionExecutionHelper _instructionExecutionHelper;
private readonly State _state;
private readonly DualPic _dualPic;
private readonly ExecutionContextManager _executionContextManager;

private readonly CfgNodeFeeder _cfgNodeFeeder;
private readonly ExecutionContextManager _executionContextManager;
private readonly InstructionReplacerRegistry _replacerRegistry = new();

public CfgCpu(IMemory memory, State state, IOPortDispatcher ioPortDispatcher, CallbackHandler callbackHandler,
DualPic dualPic, EmulatorBreakpointsManager emulatorBreakpointsManager, ILoggerService loggerService) {
_instructionExecutionHelper = new(state, memory, ioPortDispatcher, callbackHandler, loggerService);
_state = state;
_dualPic = dualPic;
_executionContextManager = new(emulatorBreakpointsManager);
_cfgNodeFeeder = new(memory, state, emulatorBreakpointsManager);

_cfgNodeFeeder = new(memory, state, emulatorBreakpointsManager, _replacerRegistry);
_executionContextManager = new(emulatorBreakpointsManager, _cfgNodeFeeder, _replacerRegistry);
}

public ExecutionContextManager ExecutionContextManager => _executionContextManager;
Expand Down Expand Up @@ -56,6 +57,13 @@ public void ExecuteNext() {
CurrentExecutionContext.NodeToExecuteNextAccordingToGraph = nextToExecute;
HandleExternalInterrupt();
}

/// <summary>
/// Signal to the cfg cpu that we are at the entry point of the program
/// </summary>
public void SignalEntry() {
_executionContextManager.SignalNewExecutionContext(_state.IpSegmentedAddress, null);
}

private void HandleExternalInterrupt() {
if (!_state.InterruptFlag) {
Expand Down
54 changes: 36 additions & 18 deletions src/Spice86.Core/Emulator/CPU/CfgCpu/ExecutionContextManager.cs
Original file line number Diff line number Diff line change
@@ -1,43 +1,61 @@
namespace Spice86.Core.Emulator.CPU.CfgCpu;

using Spice86.Core.Emulator.CPU.CfgCpu.ControlFlowGraph;
using Spice86.Core.Emulator.CPU.CfgCpu.Feeder;
using Spice86.Core.Emulator.CPU.CfgCpu.Linker;
using Spice86.Core.Emulator.CPU.CfgCpu.ParsedInstruction;
using Spice86.Core.Emulator.VM;
using Spice86.Core.Emulator.VM.Breakpoint;
using Spice86.Shared.Emulator.Memory;

public class ExecutionContextManager {
public class ExecutionContextManager : InstructionReplacer {
private readonly EmulatorBreakpointsManager _emulatorBreakpointsManager;
private readonly Dictionary<uint, ExecutionContext> _executionContextEntryPoints = new();

public ExecutionContextManager(EmulatorBreakpointsManager emulatorBreakpointsManager) {
private readonly Dictionary<SegmentedAddress, ISet<ICfgNode>> _executionContextEntryPoints = new();
private readonly CfgNodeFeeder _cfgNodeFeeder;
private int _currentDepth;

public ExecutionContextManager(EmulatorBreakpointsManager emulatorBreakpointsManager, CfgNodeFeeder cfgNodeFeeder,
InstructionReplacerRegistry replacerRegistry) : base(replacerRegistry) {
_emulatorBreakpointsManager = emulatorBreakpointsManager;
_cfgNodeFeeder = cfgNodeFeeder;
// Initial context at init
CurrentExecutionContext = new();
InitialExecutionContext = CurrentExecutionContext;
CurrentExecutionContext = new(_currentDepth);
}

public ExecutionContext InitialExecutionContext { get; }

public ExecutionContext CurrentExecutionContext { get; private set; }

public void SignalNewExecutionContext(SegmentedAddress entryAddress, SegmentedAddress? expectedReturnAddress) {
uint physicalEntryAddress = entryAddress.ToPhysical();
if (!_executionContextEntryPoints.TryGetValue(physicalEntryAddress, out ExecutionContext? executionContext)) {
executionContext = new ExecutionContext();
_executionContextEntryPoints.Add(entryAddress.ToPhysical(), executionContext);
}
// Reset the execution context so that nodes it last executed are not linked to the new ones that will come
executionContext.LastExecuted = null;
executionContext.NodeToExecuteNextAccordingToGraph = null;
// Save current execution context
ExecutionContext previousExecutionContext = CurrentExecutionContext;
CurrentExecutionContext = executionContext;
// Create a new one at a higher depth
_currentDepth++;
CurrentExecutionContext = new(_currentDepth);
if (expectedReturnAddress != null) {
// breakpoint that deletes itself on reach. Should be triggered when the return address is reached and before it starts execution.
_emulatorBreakpointsManager.ToggleBreakPoint(new AddressBreakPoint(BreakPointType.EXECUTION, expectedReturnAddress.Value.ToPhysical(), (_) => {
// Restore previous execution context
// Restore previous execution context and depth
CurrentExecutionContext = previousExecutionContext;
_currentDepth--;
}, true), true);
}

RegisterCurrentInstructionAsEntryPoint(entryAddress);
}

private void RegisterCurrentInstructionAsEntryPoint(SegmentedAddress entryAddress) {
// Register a new entry point
ICfgNode toExecute = _cfgNodeFeeder.GetLinkedCfgNodeToExecute(CurrentExecutionContext);
if (!_executionContextEntryPoints.TryGetValue(entryAddress, out ISet<ICfgNode>? nodes)) {
nodes = new HashSet<ICfgNode>();
_executionContextEntryPoints.Add(entryAddress, nodes);
}
nodes.Add(toExecute);
}

public override void ReplaceInstruction(CfgInstruction old, CfgInstruction instruction) {
if (_executionContextEntryPoints.TryGetValue(instruction.Address, out ISet<ICfgNode>? entriesAtAddress)
&& entriesAtAddress.Remove(old)) {
entriesAtAddress.Add(instruction);
}
}
}
19 changes: 12 additions & 7 deletions src/Spice86.Core/Emulator/CPU/CfgCpu/Feeder/CfgNodeFeeder.cs
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
namespace Spice86.Core.Emulator.CPU.CfgCpu.Feeder;

using Spice86.Core.Emulator.CPU.CfgCpu.ControlFlowGraph;
using Spice86.Core.Emulator.CPU.CfgCpu.Exceptions;
using Spice86.Core.Emulator.CPU.CfgCpu.Linker;
using Spice86.Core.Emulator.CPU.CfgCpu.ParsedInstruction;
using Spice86.Core.Emulator.CPU.CfgCpu.ParsedInstruction.SelfModifying;
using Spice86.Core.Emulator.Memory;
using Spice86.Core.Emulator.VM;

using System.Linq;

/// <summary>
/// Handles coherency between the memory and the graph of instructions executed by the CPU.
/// Next node to execute is normally the next node from the graph but several checks are done to make sure it is really it:
Expand All @@ -20,14 +19,15 @@ namespace Spice86.Core.Emulator.CPU.CfgCpu.Feeder;
public class CfgNodeFeeder {
private readonly State _state;
private readonly InstructionsFeeder _instructionsFeeder;
private readonly NodeLinker _nodeLinker = new();
private readonly NodeLinker _nodeLinker;
private readonly DiscriminatorReducer _discriminatorReducer;

public CfgNodeFeeder(IMemory memory, State state, EmulatorBreakpointsManager emulatorBreakpointsManager) {
public CfgNodeFeeder(IMemory memory, State state, EmulatorBreakpointsManager emulatorBreakpointsManager,
InstructionReplacerRegistry replacerRegistry) {
_state = state;
_instructionsFeeder = new(emulatorBreakpointsManager, memory, state);
_discriminatorReducer = new(new List<IInstructionReplacer<CfgInstruction>>()
{ _nodeLinker, _instructionsFeeder });
_instructionsFeeder = new(emulatorBreakpointsManager, memory, state, replacerRegistry);
_nodeLinker = new(replacerRegistry);
_discriminatorReducer = new(replacerRegistry);
}

private CfgInstruction CurrentNodeFromInstructionFeeder =>
Expand Down Expand Up @@ -61,6 +61,11 @@ private ICfgNode DetermineToExecute(ICfgNode? currentFromGraph) {
return currentFromGraph;
}

if (fromMemory.Address != currentFromGraph.Address) {
// should never happen
throw new UnhandledCfgDiscrepancyException("Nodes from memory and from graph don't have the same address. This should never happen.");
}

// Graph and memory are not aligned ... Need to inject Node with discriminator
// If previous was Discriminated and current was not in its successors we would not be there because
// currentFromGraph would have been null and the linker would then link it to DiscriminatedNode
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,25 +10,24 @@ namespace Spice86.Core.Emulator.CPU.CfgCpu.Feeder;
/// Cache of current instructions in memory.
/// Cache coherency is managed by breakpoints, as soon as an instruction is written in memory it is evicted.
/// </summary>
public class CurrentInstructions : IInstructionReplacer<CfgInstruction> {
public class CurrentInstructions : InstructionReplacer {
private readonly IMemory _memory;
private readonly EmulatorBreakpointsManager _emulatorBreakpointsManager;

/// <summary>
/// Instruction currently known to be in memory at a given address.
/// Memory write breakpoints invalidate this cache when CPU writes there.
/// </summary>
private readonly Dictionary<SegmentedAddress, CfgInstruction> _currentInstructionAtAddress =
new Dictionary<SegmentedAddress, CfgInstruction>();
private readonly Dictionary<SegmentedAddress, CfgInstruction> _currentInstructionAtAddress = new();


/// <summary>
/// Breakpoints that have been installed to monitor instruction at a given address. So that we can reset them when we want.
/// </summary>
private readonly Dictionary<SegmentedAddress, List<AddressBreakPoint>> _breakpointsForInstruction =
new Dictionary<SegmentedAddress, List<AddressBreakPoint>>();
private readonly Dictionary<SegmentedAddress, List<AddressBreakPoint>> _breakpointsForInstruction = new();

public CurrentInstructions(IMemory memory, EmulatorBreakpointsManager emulatorBreakpointsManager) {
public CurrentInstructions(IMemory memory, EmulatorBreakpointsManager emulatorBreakpointsManager,
InstructionReplacerRegistry replacerRegistry) : base(replacerRegistry) {
_memory = memory;
_emulatorBreakpointsManager = emulatorBreakpointsManager;
}
Expand All @@ -38,7 +37,7 @@ public CurrentInstructions(IMemory memory, EmulatorBreakpointsManager emulatorBr
return res;
}

public void ReplaceInstruction(CfgInstruction old, CfgInstruction instruction) {
public override void ReplaceInstruction(CfgInstruction old, CfgInstruction instruction) {
SegmentedAddress instructionAddress = instruction.Address;
if (_currentInstructionAtAddress.ContainsKey(instructionAddress)) {
ClearCurrentInstruction(old);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ namespace Spice86.Core.Emulator.CPU.CfgCpu.Feeder;
using System.Linq;

public class DiscriminatorReducer {
private readonly IList<IInstructionReplacer<CfgInstruction>> _instructionReplacers;
private readonly InstructionReplacerRegistry _replacerRegistry;

public DiscriminatorReducer(IList<IInstructionReplacer<CfgInstruction>> instructionReplacers) {
_instructionReplacers = instructionReplacers;
public DiscriminatorReducer(InstructionReplacerRegistry replacerRegistry) {
_replacerRegistry = replacerRegistry;
}

private static Dictionary<Type, List<CfgInstruction>> GroupByType(List<CfgInstruction> instructions) {
Expand Down Expand Up @@ -98,9 +98,7 @@ private void ReplaceWithReference(CfgInstruction reference, IList<CfgInstruction
continue;
}

foreach (var instructionReplacer in _instructionReplacers) {
instructionReplacer.ReplaceInstruction(instruction, reference);
}
_replacerRegistry.ReplaceInstruction(instruction, reference);
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
namespace Spice86.Core.Emulator.CPU.CfgCpu.Feeder;

using Spice86.Core.Emulator.CPU.CfgCpu.ControlFlowGraph;
using Spice86.Core.Emulator.CPU.CfgCpu.ParsedInstruction;

public interface IInstructionReplacer<in T> where T : ICfgNode {
void ReplaceInstruction(T old, T instruction);
public interface IInstructionReplacer {
void ReplaceInstruction(CfgInstruction old, CfgInstruction instruction);
}
10 changes: 10 additions & 0 deletions src/Spice86.Core/Emulator/CPU/CfgCpu/Feeder/InstructionReplacer.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
namespace Spice86.Core.Emulator.CPU.CfgCpu.Feeder;

using Spice86.Core.Emulator.CPU.CfgCpu.ParsedInstruction;

public abstract class InstructionReplacer : IInstructionReplacer {
protected InstructionReplacer(InstructionReplacerRegistry replacerRegistry) {
replacerRegistry.Register(this);
}
public abstract void ReplaceInstruction(CfgInstruction old, CfgInstruction instruction);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
namespace Spice86.Core.Emulator.CPU.CfgCpu.Feeder;

using Spice86.Core.Emulator.CPU.CfgCpu.ParsedInstruction;

public class InstructionReplacerRegistry : IInstructionReplacer {
private List<InstructionReplacer> _replacers = new();

Check notice

Code scanning / CodeQL

Missed 'readonly' opportunity Note

Field '_replacers' can be 'readonly'.

public void Register(InstructionReplacer replacer) {
_replacers.Add(replacer);
}

public void ReplaceInstruction(CfgInstruction old, CfgInstruction newInstruction) {
foreach (InstructionReplacer instructionReplacer in _replacers) {
instructionReplacer.ReplaceInstruction(old, newInstruction);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,16 @@ namespace Spice86.Core.Emulator.CPU.CfgCpu.Feeder;
/// If an instruction is modified and then put back in its original version, it is the original instance of the parsed instruction that will be returned for the address
/// Instructions can be replaced in the cache (see method ReplaceInstruction)
/// </summary>
public class InstructionsFeeder : IInstructionReplacer<CfgInstruction> {
public class InstructionsFeeder {
private readonly InstructionParser _instructionParser;
private readonly CurrentInstructions _currentInstructions;
private readonly PreviousInstructions _previousInstructions;

public InstructionsFeeder(EmulatorBreakpointsManager emulatorBreakpointsManager, IMemory memory, State cpuState) {
_currentInstructions = new(memory, emulatorBreakpointsManager);
public InstructionsFeeder(EmulatorBreakpointsManager emulatorBreakpointsManager, IMemory memory, State cpuState,
InstructionReplacerRegistry replacerRegistry) {
_currentInstructions = new(memory, emulatorBreakpointsManager, replacerRegistry);
_instructionParser = new(memory, cpuState);
_previousInstructions = new(memory);
_previousInstructions = new(memory, replacerRegistry);
}

public CfgInstruction GetInstructionFromMemory(SegmentedAddress address) {
Expand All @@ -48,9 +49,4 @@ public CfgInstruction GetInstructionFromMemory(SegmentedAddress address) {
_previousInstructions.AddInstructionInPrevious(parsed);
return parsed;
}

public void ReplaceInstruction(CfgInstruction old, CfgInstruction instruction) {
_currentInstructions.ReplaceInstruction(old, instruction);
_previousInstructions.ReplaceInstruction(old, instruction);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,16 @@ namespace Spice86.Core.Emulator.CPU.CfgCpu.Feeder;
/// <summary>
/// Cache of previous instructions that existed in a memory address at a time.
/// </summary>
public class PreviousInstructions : IInstructionReplacer<CfgInstruction> {
public class PreviousInstructions : InstructionReplacer {
private readonly MemoryInstructionMatcher _memoryInstructionMatcher;

/// <summary>
/// Instructions that were parsed at a given address. List for address is ordered by instruction decreasing length
/// </summary>
private readonly Dictionary<SegmentedAddress, HashSet<CfgInstruction>> _previousInstructionsAtAddress = new();

public PreviousInstructions(IMemory memory) {
public PreviousInstructions(IMemory memory, InstructionReplacerRegistry replacerRegistry) : base(
replacerRegistry) {
_memoryInstructionMatcher = new MemoryInstructionMatcher(memory);
}

Expand All @@ -28,7 +29,7 @@ public PreviousInstructions(IMemory memory) {
return null;
}

public void ReplaceInstruction(CfgInstruction old, CfgInstruction instruction) {
public override void ReplaceInstruction(CfgInstruction old, CfgInstruction instruction) {
SegmentedAddress instructionAddress = instruction.Address;

if (_previousInstructionsAtAddress.TryGetValue(instructionAddress,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@ namespace Spice86.Core.Emulator.CPU.CfgCpu.Linker;
using Spice86.Core.Emulator.CPU.CfgCpu.ControlFlowGraph;

public class ExecutionContext {
private static int _nextIndex = 0;

public ExecutionContext() {
Index = _nextIndex++;
public ExecutionContext(int depth) {
Depth = depth;
}

public int Index { get; }
public int Depth { get; }

/// <summary>
/// Last node actually executed by the CPU
/// </summary>
Expand Down
Loading
Loading