adamperkowski / highlightos

🌄 x86_64 OS (kernel) made from scratch in Rust & Assembly
https://os.adamperkowski.dev
GNU General Public License v3.0
47 stars 4 forks source link

Update boot.asm #18

Closed franzageek closed 4 months ago

franzageek commented 4 months ago
franzageek commented 4 months ago

Oop so sorry, lemme try to fix it. It could be that no entry point was specified, I'll give it a try.

adamperkowski commented 4 months ago

I'll add a boot check to this assembly CI, because now it only compiles and that could be misleading.

franzageek commented 4 months ago

Adding an extra boot check sounds like a good idea. I'm on a Mac rn so I couldn't test the code before committing, thinking that because the changes were not invasive, it should've booted just fine. I've just updated it - lmk if it works fine :) EDIT: didn't see the previous comment - my bad. Working on a fix rn. EDIT2: omw to install QEMU rn.

franzageek commented 4 months ago

I think I've found the bug, trying to fix it.

adamperkowski commented 4 months ago

Added a job that checks if a DOS boot sector is present. It's not a full "boot check" but it'll do. https://github.com/adamperkowski/highlightos/blob/4346236180e39512335f32ba9e25e9a4fff452e7/.github/workflows/asm.yml#L26-L30

adamperkowski commented 4 months ago

I'll update your branch.

franzageek commented 4 months ago

There you go, fixed. I forgot that 16 bit Real Mode assembly does not support splitting code in text/data sections. I'm used to program in 32/64 bit Protected mode. Very sorry, should be fixed now.

While debugging, I found another bug - after pressing enter the program flow remained trapped in an infinite loop, because I replaced a jump with a ret.

Fixed that as well.

Try it out for yourself and let me know if it sticks!

adamperkowski commented 4 months ago

Seems like you forgot about this https://github.com/adamperkowski/highlightos/pull/18/commits/bc369ce4a1a3c4ba9efd12798dd64bc55f794e3e . Added.

input wasn't looping because of that

franzageek commented 4 months ago

Actually no - the reason was the jmp shell line on line 116/117. EDIT: yeah, forgot abt both. I'm sorry man, bit tired these days.

adamperkowski commented 4 months ago

No problem at all. Happens. So yeah thanks for taking your time and contributing again. Much appreciated. I'll merge this if you don't want to add anything.

franzageek commented 4 months ago

Sure! Really like the vision & the direction of this project. Let me review the code - will add anything if necessary.

adamperkowski commented 4 months ago

Message me privately please through Discord, Signal, e-mail or whatever you like. Merging.

franzageek commented 4 months ago

Alright - just sent you a DM request on Discord.