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

feature: DOS LOAD AND/OR EXEC and DOS process management #871

Draft
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

maximilien-noal
Copy link
Member

@maximilien-noal maximilien-noal commented Sep 26, 2024

EDIT: For context, the previous description was essentially 'refactor: make the DOS INT21H load and execute the initial program', and the PR was limited to only just that. It isn't anymore, as it wasn't enough.

Description of Changes

Implements LOAD AND/OR EXEC (DOS INT21H function 0x4B) and DOS process management.

Rationale behind Changes

Makes LOAD AND/OR EXEC, used by a lot of DOS games, work. Also a lot of critical DOS kernel structures are present and initialized in emulated memory now, which at least makes our DOS implementation more like an expected one.

Suggested Testing Steps

Test this with your favorite game.
It should still load, even if you set a preferred start segment in the config, or even use program arguments.

Dune still works.

@maximilien-noal maximilien-noal self-assigned this Sep 26, 2024
@maximilien-noal maximilien-noal added DOS Related to DOS refactoring Involves refactoring existing code labels Sep 26, 2024
@maximilien-noal maximilien-noal changed the title refactor: DOSINT21H loads DOS executables refactor: DOSINT21H loads the initial DOS executable Sep 26, 2024
@JorisVanEijden
Copy link
Contributor

Krondor loads and runs.

But I don't think an interrupt handler is the right place for this file loading is it?
Feels like DosFileManager would be a better fit.

I'm also not quite sure how this makes it easier to implement DOS function INT21H 0x4B.

@maximilien-noal
Copy link
Member Author

Dosbox does this :

Call the command processor, which eventually calls dos int21h 0x4b to execute a DOS program with a DOS path and args as parameters given via CPU registers.

It also handles the SDA structure, PSP chains, DOS process management, TSRs and such.

What we do currently on master:

We completely side step the dos kernel and dos int handlers and the usage of a DOS path.

We directly load an executable in memory from an absolute host path, with class that are foreign to the OperatingSystem namespace.

We don't have the SDA, DOS process management, TSR support, etc.

What this does:

What I see as the first step : call the dos int21h class to load the DOS program, from an absolute host path.

It's a little bit better, because the rest of the modifications will be done in the dos kernel and the dos int21h handler.

@@ -776,13 +776,13 @@ public DosFileOperationResult RemoveDirectory(string dosDirectory) {

return DosFileOperationResult.NoValue();
} catch (IOException e) {
triedToDeleteCurrentDir = true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is deleting the currentDir really the only cause for IOExceptions when deleting a directory?

fullHostPath, e);
}
}

return PathNotFoundError(dosDirectory);
return triedToDeleteCurrentDir ? RemoveCurrentDirError(dosDirectory) : PathNotFoundError(dosDirectory);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not return this at the end of the codeblock where that boolean is set?

@JorisVanEijden
Copy link
Contributor

Dosbox does this :

Call the command processor, which eventually calls dos int21h 0x4b to execute a DOS program with a DOS path and args as parameters given via CPU registers.

It also handles the SDA structure, PSP chains, DOS process management, TSRs and such.

What we do currently on master:

We completely side step the dos kernel and dos int handlers and the usage of a DOS path.

We directly load an executable in memory from an absolute host path, with class that are foreign to the OperatingSystem namespace.

We don't have the SDA, DOS process management, TSR support, etc.

What this does:

What I see as the first step : call the dos int21h class to load the DOS program, from an absolute host path.

It's a little bit better, because the rest of the modifications will be done in the dos kernel and the dos int21h handler.

The way I imagine it, we'd have a "FileLoader" class (or set of classes) that are used by both the int21handler and the ProgramExecutor.
I dunno, to me it just feels more like it belongs somewhere under the Spice86.Core.Emulator.OperatingSystem namespace rather than under the Spice86.Core.Emulator.InterruptHandlers.Dos namespace.

@maximilien-noal
Copy link
Member Author

I can do that.

@maximilien-noal maximilien-noal removed the request for review from kevinferrare September 30, 2024 17:36
@maximilien-noal maximilien-noal added the feature request This would be good to have label Sep 30, 2024
@maximilien-noal maximilien-noal marked this pull request as draft September 30, 2024 17:36
@maximilien-noal maximilien-noal force-pushed the feature/dos_process_managment_init branch 2 times, most recently from 5086f67 to 85bfd02 Compare September 30, 2024 18:46
@maximilien-noal maximilien-noal force-pushed the feature/dos_process_managment_init branch 5 times, most recently from 4deb7d6 to 95849f2 Compare October 20, 2024 19:09
@maximilien-noal maximilien-noal force-pushed the feature/dos_process_managment_init branch 2 times, most recently from 5fee758 to a1d32fd Compare October 22, 2024 18:37
@maximilien-noal maximilien-noal changed the title refactor: DOSINT21H loads the initial DOS executable feature: DOS LOAD AND/OR EXEC and DOS process management Oct 22, 2024
@maximilien-noal maximilien-noal force-pushed the feature/dos_process_managment_init branch from 521dd48 to 4e768c3 Compare October 22, 2024 20:49
SetCarryFlag(false, calledFromVm);
return;
}
DosExecParameterBlock dosExecParameterBlock = new DosExecParameterBlock(Memory, MemoryUtils.ToPhysicalAddress(State.ES, State.BX));

Check warning

Code scanning / CodeQL

Useless assignment to local variable Warning

This assignment to
dosExecParameterBlock
is useless, since its value is never read.
Just like on a real DOS implementation.

This makes it easier to implement the following:
- The SDA
- PSP chains
- DOS function INT21H 0x4B (Load and/or exec file)

and eventually, TSR support.

Signed-off-by: Maximilien Noal <noal.maximilien@gmail.com>
Signed-off-by: Maximilien Noal <noal.maximilien@gmail.com>
Signed-off-by: Maximilien Noal <noal.maximilien@gmail.com>
Signed-off-by: Maximilien Noal <noal.maximilien@gmail.com>
Signed-off-by: Maximilien Noal <noal.maximilien@gmail.com>
Signed-off-by: Maximilien Noal <noal.maximilien@gmail.com>
Signed-off-by: Maximilien Noal <noal.maximilien@gmail.com>
@maximilien-noal maximilien-noal force-pushed the feature/dos_process_managment_init branch from 948b4a1 to 5fbcb8c Compare October 30, 2024 19:14
@maximilien-noal maximilien-noal force-pushed the feature/dos_process_managment_init branch from de7680d to 2141bc9 Compare December 29, 2024 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DOS Related to DOS feature request This would be good to have refactoring Involves refactoring existing code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants