Skip to content

Commit 370a1e5

Browse files
Copilotmaximilien-noalCopilot
authored
fix: correct COMMAND.COM PSP[0x02] size, and DOS media ID tables segment and informations returned by DOS INT21H (#2138)
* fix: correct COMMAND.COM PSP CurrentSize to prevent TSR memory exhaustion The DosProgramSegmentPrefix constructor was writing CurrentSize = LastFreeSegment (0x9FFF = 40959 paragraphs) to memory on every instantiation, acting as a destructive write rather than a pure memory view. The fake COMMAND.COM PSP at segment 0x60 had PSP[0x02] = 0x9FFF because: 1. CreateRootCommandComPsp set it to 0x9FFF (old code) 2. Every call to `new DosProgramSegmentPrefix(memory, 0x60_address)` re-wrote it to 0x9FFF (even pure read wrappers) Games like Maupiti Island read the parent (COMMAND.COM) PSP[0x02] to compute DX for INT 21h/31h (Terminate and Stay Resident). With the buggy 0x9FFF value, TryModifyBlock would fail (0x9E8D block < 0x9FFF requested), leaving the entire program block allocated and no conventional memory free for subsequent allocations. Fix: - Remove the destructive write from DosProgramSegmentPrefix constructor: it is a memory view/wrapper and must not initialise fields on construction. - Set CreateRootCommandComPsp to use a realistic value: CommandComSegment + PspSizeInParagraphs = 0x60 + 0x10 = 0x70 (reflecting only the 16-paragraph PSP area, as real COMMAND.COM would have after releasing its extra memory before loading the game). TDD evidence (MCP-server style ASM test): - Before fix: parent PSP[0x02] = 0x9FFF; largest free block = 12 paragraphs - After fix: parent PSP[0x02] = 0x0070; largest free block = 0x9E2D paragraphs Agent-Logs-Url: https://github.com/OpenRakis/Spice86/sessions/788b07bb-2457-4112-9e30-cbc5fa5319e4 Co-authored-by: maximilien-noal <1087524+maximilien-noal@users.noreply.github.com> * fix: remove null-forgiving operator and clarify comment per review feedback Agent-Logs-Url: https://github.com/OpenRakis/Spice86/sessions/788b07bb-2457-4112-9e30-cbc5fa5319e4 Co-authored-by: maximilien-noal <1087524+maximilien-noal@users.noreply.github.com> * refactor: use Arrange/Act/Assert and share init code in TsrIntegrationTests Agent-Logs-Url: https://github.com/OpenRakis/Spice86/sessions/3d9d957b-4161-4500-b596-c0a7316cd3d9 Co-authored-by: maximilien-noal <1087524+maximilien-noal@users.noreply.github.com> * refactor: extract shared ResourceDir field in TsrIntegrationTests Agent-Logs-Url: https://github.com/OpenRakis/Spice86/sessions/61eac897-87f7-4491-8a9a-5322f190d3dd Co-authored-by: maximilien-noal <1087524+maximilien-noal@users.noreply.github.com> * feat: implement INT 21h, AH=1Ch and related tests for drive allocation info Co-authored-by: Copilot <copilot@github.com> * refactor: DosTables constructor * refactor: removed 'default!' * refactor: streamline DOS media ID handling and improve drive manager initialization Co-authored-by: Copilot <copilot@github.com> * refactor: DosDriveManager initializes itself * refactor: Removed unused field * chore: Format DOSInt21Handler to respect java style code blocks --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: maximilien-noal <1087524+maximilien-noal@users.noreply.github.com> Co-authored-by: Maximilien Noal <noal.maximilien@gmail.com> Co-authored-by: Copilot <copilot@github.com>
1 parent 3289185 commit 370a1e5

20 files changed

Lines changed: 397 additions & 187 deletions

src/Spice86.Core/Emulator/InterruptHandlers/Dos/DosInt21Handler.cs

Lines changed: 63 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,9 @@ public class DosInt21Handler : InterruptHandler {
4646

4747
private const ushort OffsetMask = 0x0F;
4848
private const byte ExpectedValueOfALInCreateChildPsp = 0xF0;
49+
private const ushort DefaultBytesPerSector = 0x200;
50+
private const byte DefaultSectorsPerCluster = 0x20;
51+
private const ushort DefaultTotalClusters = 0x7FFD;
4952

5053
/// <summary>
5154
/// Initializes a new instance.
@@ -621,35 +624,74 @@ public void RemoveDirectory(bool calledFromVm) {
621624
}
622625

623626
/// <summary>
624-
/// Returns the bytes per sector (in CX), sectors per cluster (in AX), total clusters (in DX), media id (in DS), <br/>
625-
/// and drive parameter block address for the default drive (in BX). <br/>
626-
/// Sets the AH register to 0.
627+
/// INT 21h, AH=1Ch - Get allocation info for a specific drive.
628+
/// Returns AL=0xFF if the drive is invalid or not mounted as a block drive.
627629
/// </summary>
628-
/// <remarks>
629-
/// TODO: Implement it for real. This is just a stub that returns the same information<br/>
630-
/// as <see cref="GetAllocationInfoForDefaultDrive"/> as we can only mount C: !
631-
/// </remarks>
632630
public void GetAllocationInfoForAnyDrive() {
633-
GetAllocationInfoForDefaultDrive();
631+
if (!TryGetAllocationInfoDrive(State.DL, out byte driveIndex, out _)) {
632+
if (LoggerService.IsEnabled(LogEventLevel.Verbose)) {
633+
LoggerService.Verbose("INT 21h AH=1Ch invalid/unmounted drive request DL={DriveRequest:X2}", State.DL);
634+
}
635+
State.AL = 0xFF;
636+
return;
637+
}
638+
SetAllocationInfoRegisters(driveIndex);
634639
}
635640

636641
/// <summary>
637-
/// Returns the bytes per sector (in CX), sectors per cluster (in AX), total clusters (in DX), media id (in DS),<br/>
638-
/// and drive parameter block address for the default drive (in BX). <br/>
639-
/// Sets the AH register to 0. <br/>
640-
/// Notes: always returns the same values.
642+
/// INT 21h, AH=1Bh - Get allocation info for the default drive.
643+
/// Returns AL=0xFF if the current default drive is invalid for FAT allocation queries.
641644
/// </summary>
642645
public void GetAllocationInfoForDefaultDrive() {
643-
// Bytes per sector
644-
State.CX = 0x200;
645-
// Sectors per cluster
646-
State.AX = 1;
647-
// Total clusters
648-
State.DX = 0xEA0;
649-
// Media Id
650-
State.DS = 0x8010;
651-
State.BX = (ushort)(0x8010 + _dosDriveManager.CurrentDriveIndex * 9);
646+
if (!TryGetAllocationInfoDrive(0, out byte driveIndex, out _)) {
647+
if (LoggerService.IsEnabled(LogEventLevel.Verbose)) {
648+
LoggerService.Verbose("INT 21h AH=1Bh default drive invalid/unmounted (CurrentDriveIndex={DriveIndex})",
649+
_dosDriveManager.CurrentDriveIndex);
650+
}
651+
State.AL = 0xFF;
652+
return;
653+
}
654+
SetAllocationInfoRegisters(driveIndex);
655+
}
656+
657+
private bool TryGetAllocationInfoDrive(byte driveRequest, out byte driveIndex, out string mountedHostDirectory) {
658+
mountedHostDirectory = string.Empty;
659+
if (driveRequest == 0) {
660+
driveIndex = _dosDriveManager.CurrentDriveIndex;
661+
} else {
662+
driveIndex = (byte)(driveRequest - 1);
663+
}
664+
665+
if (driveIndex >= DosDriveManager.MaxDriveCount) {
666+
return false;
667+
}
668+
669+
char driveLetter = (char)('A' + driveIndex);
670+
if (!_dosDriveManager.TryGetValue(driveLetter, out VirtualDrive? resolvedDrive) || resolvedDrive == null) {
671+
return false;
672+
}
673+
674+
if (string.IsNullOrWhiteSpace(resolvedDrive.MountedHostDirectory)) {
675+
return false;
676+
}
677+
678+
mountedHostDirectory = resolvedDrive.MountedHostDirectory;
679+
return true;
680+
}
681+
682+
private void SetAllocationInfoRegisters(byte driveIndex) {
683+
State.CX = DefaultBytesPerSector;
684+
State.AL = DefaultSectorsPerCluster;
652685
State.AH = 0;
686+
State.DX = DefaultTotalClusters;
687+
State.DS = _dosDriveManager.MediaIdTableSegment;
688+
State.BX = _dosDriveManager.MediaIdEntryOffset(driveIndex);
689+
690+
if (LoggerService.IsEnabled(LogEventLevel.Verbose)) {
691+
LoggerService.Verbose(
692+
"INT 21h AH=1Bh/1Ch allocation info drive={DriveLetter}: AL(spc)={SectorsPerCluster:X2} CX(bps)={BytesPerSector:X4} DX(totalClusters)={TotalClusters:X4} DS:BX={Segment:X4}:{Offset:X4}",
693+
(char)('A' + driveIndex), State.AL, State.CX, State.DX, State.DS, State.BX);
694+
}
653695
}
654696

655697
/// <summary>

src/Spice86.Core/Emulator/OperatingSystem/Dos.cs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -182,17 +182,16 @@ public Dos(Configuration configuration, IMemory memory,
182182
_biosDataArea = biosDataArea;
183183
_vgaFunctionality = vgaFunctionality;
184184

185-
DosDriveManager = new(_loggerService, configuration.CDrive, configuration.Exe);
185+
DosTables = new DosTables(memory);
186+
ushort mediaIdSegment = DosTables.ReserveDosPrivateSegment((ushort)DosMediaIdTable.TableSizeInParagraphs);
187+
DosMediaIdTable mediaIdTable = new(memory, MemoryUtils.ToPhysicalAddress(mediaIdSegment, 0), mediaIdSegment);
188+
DosDriveManager = new(_loggerService, configuration.CDrive, configuration.Exe, mediaIdTable);
186189

187190
VirtualFileBase[] dosDevices = AddDefaultDevices(state, keyboardInt16Handler);
188191
DosSysVars = new DosSysVars(configuration, (NullDevice)dosDevices[0], memory,
189192
MemoryUtils.ToPhysicalAddress(DosSysVars.Segment, 0x0));
190193

191194
DosSysVars.ConsoleDeviceHeaderPointer = ((IVirtualDevice)dosDevices[1]).Header.BaseAddress;
192-
193-
DosTables = new DosTables();
194-
DosTables.Initialize(memory);
195-
196195
DosSysVars.CurrentDirectoryStructureListPointer = DosTables.CurrentDirectoryStructure.BaseAddress;
197196
DosSysVars.CurrentDirectoryStructureCount = 26;
198197

src/Spice86.Core/Emulator/OperatingSystem/DosDriveManager.cs

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,25 +15,27 @@
1515
public class DosDriveManager : IDictionary<char, VirtualDrive> {
1616
private readonly SortedDictionary<char, VirtualDrive> _driveMap = new();
1717
private readonly Dictionary<char, MemoryDrive> _memoryDriveMap = new();
18-
private readonly ILoggerService _loggerService;
18+
private readonly DosMediaIdTable _mediaIdTable;
1919

2020
/// <summary>
2121
/// Initializes a new instance.
2222
/// </summary>
2323
/// <param name="loggerService">The service used to log messages.</param>
2424
/// <param name="cDriveFolderPath">The host path to be mounted as C:.</param>
2525
/// <param name="executablePath">The host path to the DOS executable to be launched.</param>
26-
public DosDriveManager(ILoggerService loggerService, string? cDriveFolderPath, string? executablePath) {
26+
/// <param name="mediaIdTable">The DOS private-segment media ID table owned by this manager.</param>
27+
public DosDriveManager(ILoggerService loggerService, string? cDriveFolderPath, string? executablePath, DosMediaIdTable mediaIdTable) {
28+
_mediaIdTable = mediaIdTable;
2729
if (string.IsNullOrWhiteSpace(cDriveFolderPath)) {
2830
cDriveFolderPath = DosPathResolver.GetExeParentFolder(executablePath);
2931
}
30-
_loggerService = loggerService;
3132
cDriveFolderPath = ConvertUtils.ToSlashFolderPath(cDriveFolderPath);
3233
_driveMap.Add('A', new() { DriveLetter = 'A', CurrentDosDirectory = "", MountedHostDirectory = "" });
3334
_driveMap.Add('B', new() { DriveLetter = 'B', CurrentDosDirectory = "", MountedHostDirectory = "" });
3435
var cDrive = new VirtualDrive { DriveLetter = 'C', MountedHostDirectory = cDriveFolderPath, CurrentDosDirectory = "" };
3536
_driveMap.Add('C', cDrive);
3637
CurrentDrive = cDrive;
38+
InitializeMediaDescriptors();
3739
if (loggerService.IsEnabled(Serilog.Events.LogEventLevel.Verbose)) {
3840
loggerService.Verbose("DOS Drives initialized: {@Drives}", _driveMap.Values);
3941
}
@@ -86,6 +88,9 @@ internal bool HasDriveAtIndex(ushort zeroBasedIndex) {
8688
return true;
8789
}
8890

91+
/// <suummary>
92+
/// Gets the number of DOS drive letters assigned.
93+
/// </summary>
8994
public byte NumberOfPotentiallyValidDriveLetters {
9095
get {
9196
// At least A: and B:
@@ -106,6 +111,29 @@ public byte NumberOfPotentiallyValidDriveLetters {
106111

107112
public const int MaxDriveCount = 26;
108113

114+
private const byte FloppyMediaDescriptor = 0xF0;
115+
private const byte FixedDiskMediaDescriptor = 0xF8;
116+
117+
/// <summary>Writes the FAT media descriptor byte for every drive into the media ID table.</summary>
118+
private void InitializeMediaDescriptors() {
119+
for (byte driveIndex = 0; driveIndex < MaxDriveCount; driveIndex++) {
120+
_mediaIdTable[driveIndex] = MediaDescriptor(driveIndex);
121+
}
122+
}
123+
124+
/// <summary>The segment of the media ID table, used as DS in AH=1Bh/1Ch returns.</summary>
125+
public ushort MediaIdTableSegment => _mediaIdTable.Segment;
126+
127+
/// <summary>In-segment offset of the given drive's entry, used as BX in AH=1Bh/1Ch returns.</summary>
128+
public ushort MediaIdEntryOffset(byte driveIndex) => _mediaIdTable.EntryOffset(driveIndex);
129+
130+
private byte MediaDescriptor(byte driveIndex) {
131+
if (driveIndex <= 1) {
132+
return FloppyMediaDescriptor;
133+
}
134+
return FixedDiskMediaDescriptor;
135+
}
136+
109137
public void Add(char key, VirtualDrive value) {
110138
((IDictionary<char, VirtualDrive>)_driveMap).Add(key, value);
111139
}

src/Spice86.Core/Emulator/OperatingSystem/DosProcessManager.cs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,13 @@ public DosProgramSegmentPrefix CreateRootCommandComPsp() {
258258

259259
rootPsp.Exit[0] = IntOpcode;
260260
rootPsp.Exit[1] = Int20TerminateNumber;
261-
rootPsp.CurrentSize = DosMemoryManager.LastFreeSegment;
261+
// CurrentSize reflects only the PSP itself (16 paragraphs), not all of conventional memory.
262+
// Programs that read the parent (COMMAND.COM) PSP[0x02] to compute their TSR resident
263+
// size (DX for INT 21h/31h) must receive a realistic small value, not LastFreeSegment
264+
// (0x9FFF = 40959 decimal). Using 0x9FFF caused games like Maupiti Island to pass 0x9FFF
265+
// as DX, which made TryModifyBlock fail after expanding the block to maximum, leaving
266+
// all conventional memory consumed and subsequent allocations impossible.
267+
rootPsp.CurrentSize = (ushort)(CommandComSegment + DosProgramSegmentPrefix.PspSizeInParagraphs);
262268

263269
// Root PSP: parent points to itself
264270
rootPsp.ParentProgramSegmentPrefix = CommandComSegment;
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
namespace Spice86.Core.Emulator.OperatingSystem.Structures;
2+
3+
using Spice86.Core.Emulator.Memory.ReaderWriter;
4+
using Spice86.Core.Emulator.ReverseEngineer.DataStructure;
5+
6+
/// <summary>
7+
/// Represents the DOS media ID table stored in the DOS private segment area.
8+
/// Each of the 26 possible drives has a 9-byte entry; the first byte is the FAT
9+
/// media descriptor returned by INT 21h AH=1Bh/1Ch via DS:BX.
10+
/// </summary>
11+
/// <remarks>
12+
/// The 9-byte stride matches DOSBox's DPB layout (<c>mediaid = RealMake(dpb, 0x17)</c>)
13+
/// where media IDs are spaced at <c>i*9</c> offsets.
14+
/// </remarks>
15+
public class DosMediaIdTable : MemoryBasedDataStructure {
16+
/// <summary>Bytes per drive entry.</summary>
17+
public const int EntrySize = 9;
18+
19+
/// <summary>Total bytes required for all 26 drive entries.</summary>
20+
public const int TableSizeInBytes = 26 * EntrySize;
21+
22+
/// <summary>Paragraphs (16-byte blocks) required to hold the full table.</summary>
23+
public const int TableSizeInParagraphs = (TableSizeInBytes + 15) / 16;
24+
25+
/// <summary>The segment of this table in the DOS private area.</summary>
26+
public ushort Segment { get; }
27+
28+
/// <summary>
29+
/// Initializes a new instance.
30+
/// </summary>
31+
/// <param name="byteReaderWriter">The memory bus.</param>
32+
/// <param name="baseAddress">The physical base address of the table.</param>
33+
/// <param name="segment">The segment value used to build DS:BX pointers for callers.</param>
34+
public DosMediaIdTable(IByteReaderWriter byteReaderWriter, uint baseAddress, ushort segment)
35+
: base(byteReaderWriter, baseAddress) {
36+
Segment = segment;
37+
}
38+
39+
/// <summary>Gets or sets the FAT media descriptor byte for the given zero-based drive index.</summary>
40+
public byte this[byte driveIndex] {
41+
get => UInt8[driveIndex * EntrySize];
42+
set => UInt8[driveIndex * EntrySize] = value;
43+
}
44+
45+
/// <summary>
46+
/// Returns the in-segment offset of the given drive's entry, for use as BX in the DS:BX pointer.
47+
/// </summary>
48+
public ushort EntryOffset(byte driveIndex) => (ushort)(driveIndex * EntrySize);
49+
}

src/Spice86.Core/Emulator/OperatingSystem/Structures/DosProgramSegmentPrefix.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ public sealed class DosProgramSegmentPrefix : MemoryBasedDataStructure {
2626
public const ushort PspSizeInParagraphs = 0x10;
2727

2828
public DosProgramSegmentPrefix(IByteReaderWriter byteReaderWriter, uint baseAddress) : base(byteReaderWriter, baseAddress) {
29-
CurrentSize = DosMemoryManager.LastFreeSegment;
3029
}
3130

3231
/// <summary>

src/Spice86.Core/Emulator/OperatingSystem/Structures/DosTables.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,18 +24,18 @@ public class DosTables {
2424
/// <summary>
2525
/// Gets the Current Directory Structure (CDS) for DOS drives.
2626
/// </summary>
27-
public CurrentDirectoryStructure CurrentDirectoryStructure { get; private set; } = default!;
27+
public CurrentDirectoryStructure CurrentDirectoryStructure { get; private set; }
2828

2929
/// <summary>
3030
/// Gets the Double Byte Character Set (DBCS) lead-byte table.
3131
/// </summary>
32-
public DosDoubleByteCharacterSet DoubleByteCharacterSet { get; private set; } = default!;
32+
public DosDoubleByteCharacterSet DoubleByteCharacterSet { get; private set; }
3333

3434
/// <summary>
3535
/// Initializes the DOS table structures in memory.
3636
/// </summary>
3737
/// <param name="memory">The memory interface to write structures to.</param>
38-
public void Initialize(IByteReaderWriter memory) {
38+
public DosTables(IByteReaderWriter memory) {
3939
uint cdsAddress = MemoryUtils.ToPhysicalAddress(MemoryMap.DosCdsSegment, 0);
4040
CurrentDirectoryStructure = new CurrentDirectoryStructure(memory, cdsAddress);
4141

src/Spice86/Spice86DependencyInjection.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,7 @@ internal Spice86DependencyInjection(Configuration configuration, MainWindow? mai
356356

357357
ExtendedMemoryManager? xms = null;
358358

359-
DosTables dosTables = new();
359+
DosTables dosTables = new(memory);
360360

361361
SharedMouseData sharedMouseData = new();
362362

0 commit comments

Comments
 (0)