-
-
Notifications
You must be signed in to change notification settings - Fork 229
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
Allow alternate BIOS via -B
option; make BIOS ROM area read-only
#128
Conversation
blink/biosimage.c
Outdated
FormatInt64(tmp, errno); | ||
WriteErrorString(tmp); | ||
WriteErrorString(")\n"); | ||
exit(127); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @tkchia,
I would say it's about time for a FatalError
to replace six function calls here and a number of other places? With blink
being the size it is, and already requiring sprintf
, IMO we should use a single function with varargs, similar to the LOGF
and ERRF
already in place.
Thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ghaerr : indeed. Thank you!
-B
option to Blinkenlights-B
option; make BIOS ROM area read-only
@tkchia: LGTM. Is the reason that |
Hello @ghaerr,
That is my intention. Admittedly though, the code probably needs more testing on platforms where the page size may be larger or even smaller than 4,096. Thank you! |
Hello @tkchia, When running on macOS after this commit, Here's the attached screenshot showing when it happens: Thank you! |
If I comment out the
|
Hello @ghaerr, Interesting. It seems that something inside the FreeDOS installer was trying to scribble bytes into the BIOS ROM area (but why)? I should look into this further to figure out the correct way to fix this. Thank you! |
Hello @tkchia,
I see. Do you know what the As far as scribbling bytes I guess I'm not surprised at what I've already seen, but perhaps it's trying its own way of determining the top of usable memory? If so, we'll probably be forced to your previous recommendation of ignoring writes, rather than prohibiting them (somehow). I'm not sure how easy it might be to add a "Watch range" to Thank you! |
Hello @ghaerr, The instructions in the disassembly seem to come from an earlier version of the
So it looks like it should be OK to ignore and discard the writes to the ROM area in this case. The problem is figuring out how. 😐 (Perhaps one simplistic way for now is to check for the special case of a Thank you! |
Hello @ghaerr, I think the original idea was to actually scribble the Thank you! |
Hello @tkchia,
I think you're right, the However, even if the code had been written without error and the
I would agree that having the capability to check read/write access of VM memory would be a feature that could be used to better debug programs being emulated. However,
The current feature is of quite limited capability, given that its only use is on page-aligned memory with callback handling byte ranges or flags not supported by Thank you! |
Hello @ghaerr, I have a patch in the works that modifies Blink's Then again, it is rare for programs to actually want to write to ROM — such programs are probably either buggy (e.g. Thank you! |
Hello @tkchia,
That sounds promising. I hadn't realized that all VM ram access went through a single routine.
I see. For I think your proposed
Definitely agree there. Adding the ability to restrict ROM write access is of quite limited usefulness, unless one just wants to become informed of where the buggy programs are out there (but still likely requires them to run as written).
Perhaps a command line option to enable it might be more useful. However, who would really use this feature when enabled? Anyone emulating FreeDOS with Thank you! |
Hello @ghaerr, OK... here is the patch; I think I will just leave it here for now, and perhaps submit it if and when it turns out to be more useful:
It turns out I was slightly mistaken. The buggy Thank you! |
Hello @tkchia,
Wow, that seems quite complicated for such a small feature. If such a feature were to be implemented, given it's complexity, it would be nice to add to it to allow for arbitrary byte ranges to be compared against read/write privileges, for instance to detect UMB and or XMS access for debugging, which could be more useful. I guess ultimately this comes down to what is the purpose of the emulator, to help with debugging, or to exactly emulate hardware?
Well that fine, except that the FreeDOS 1.3-rc1 image you uploaded for me to debug GWBASIC for you still has it, and it appears there are many versions of FreeDOS out there, given that I've got three already and they all have different installs and CPU checks. Thus, I think we should comment out the Thank you! |
Hello @ghaerr, Actually, I think it will in fact be better and more "user-friendly" for Blinkenlights to fail noisily, rather than silently try to do something bogus, if it cannot emulate a particular CPU feature. Such failures will be much easier for developers — e.g. us — to diagnose, when users do encounter and report them. (But perhaps the failure message can be improved.) Thank you! |
Hello @tkchia,
Well, that means I can't boot your GWBASIC image without changing the blink source code, even though it works on a real PC, so it's not fully bogus. That was the whole reason I brought this up after this last merge. There should at least be an option for it, so that I don't have to comment it out in order to run that particular version of FreeDOS installer. (And/or please upload something that I can use for GWBASIC, as I would like to continue using GWBASIC to help you get it running well).
This isn't a CPU feature, its an IBM PC architectural feature, which in the real hardware case is ignored, and in our case, execution is stopped.
Agreed it could be a feature, when wanted. However, once found out, there needs to be a way to turn it off, rather than saying that
That's a definite for sure! A major problem continuing with the current design is that all segmentation faults go to the same handler, and no one (neither the user nor the developers) know whether this is a Thank you! |
Hello @ghaerr, I think I get what you are getting at now — you would like some sort of run-time (rather than build-time) toggle to get Blinkenlights to flag attempts to write to the ROM area, as this may be useful for catching certain kinds of memory corruption bugs. I (or someone else?) might try to implement this later. 🙂 I probably also need to decide though, if this should be settable from the command line, or from the TUI...
I expect things such as UMB and EMS will be handled by the virtual memory paging mechanism (modern EMS is done using V86 mode). So — unless we are to emulate EMS in a different way — I expect the specific range checks will be orthogonal to checks on the physical memory addresses, the BIOS ROM range check being one of these. In any case, it might be interesting to think a bit about how to emulate EMS. Thank you! |
Hello @tkchia,
The ability to continue an emulation that has halted for a bad "check" result is very desirable - so that one can continue forward instead of being dead in the water, especially for checks that might differ based on an assumed platform. In general, once the problem "check" is understood, its nice for a way to continue through it without having to halt and have to manually continue, for those deeper debugging sessions. (As an example, merely testing GWBASIC requires a booting and running FreeDOS along with its install program, only to be aborted, to get a DOS prompt). Along these debug issues, we can add to the list the need to emulate a ^C input to the emulated program. I already tried replacing, say ^E with ^C in the The ability to set or remove a breakpoint (by inputted address) from the TUI rather than the command line would be nice. Also, you may have noticed, that the disassembly pane will always show the current IP as the first address displayed when out has to recalculate... this ends up such that the user can't see the instruction stream behind the current instruction (which happens on every So, not sure the answer on TUI vs command line, but I say we need to make it as easy as possible for actual debugging or reverse engineering.
I thought that if Thank you! |
No description provided.