Merge pull request #855 from OpenRakis/fix/hw_interrupts_for_cfg_cpu

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

src/Spice86.Core/Emulator/CPU/CfgCpu/CfgCpu.cs

@@ -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;
@@ -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) {

src/Spice86.Core/Emulator/CPU/CfgCpu/ExecutionContextManager.cs

@@ -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);
+        }
+    }
 }

src/Spice86.Core/Emulator/CPU/CfgCpu/Feeder/CfgNodeFeeder.cs

@@ -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:
@@ -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 =>
@@ -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

src/Spice86.Core/Emulator/CPU/CfgCpu/Feeder/CurrentInstructions.cs

@@ -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;
     }
@@ -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);

src/Spice86.Core/Emulator/CPU/CfgCpu/Feeder/DiscriminatorReducer.cs

@@ -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) {
@@ -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);
         }
     }
 

src/Spice86.Core/Emulator/CPU/CfgCpu/Feeder/IInstructionReplacer.cs

@@ -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);
 }

src/Spice86.Core/Emulator/CPU/CfgCpu/Feeder/InstructionReplacer.cs

@@ -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);
+}

src/Spice86.Core/Emulator/CPU/CfgCpu/Feeder/InstructionReplacerRegistry.cs

@@ -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();
+
+    public void Register(InstructionReplacer replacer) {
+        _replacers.Add(replacer);
+    }
+
+    public void ReplaceInstruction(CfgInstruction old, CfgInstruction newInstruction) {
+        foreach (InstructionReplacer instructionReplacer in _replacers) {
+            instructionReplacer.ReplaceInstruction(old, newInstruction);
+        }
+    }
+}

src/Spice86.Core/Emulator/CPU/CfgCpu/Feeder/InstructionsFeeder.cs

@@ -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) {
@@ -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);
-    }
 }

src/Spice86.Core/Emulator/CPU/CfgCpu/Feeder/PreviousInstructions.cs

@@ -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);
     }
 
@@ -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,

src/Spice86.Core/Emulator/CPU/CfgCpu/Linker/ExecutionContext.cs

@@ -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>