Hi,
XsDos wrote:
This is my 5th bootloader, can you give advises or comments or did i miss something, or something will cause a problem in the future. i am still learning i am not sure about this code.
A thorough examination...
Code:
[ORG 0x7C00]
[BITS 16]
NASM's directives have a "system version" (the lowest level directive, that should only be used in macros) and a "user version" (which can be replaced/enhanced by macros to add new features). You should always use the latter, like:
Code:
ORG 0x7C00
BITS 16
Code:
%define PS2_STATUS_OUTPUT_FULL 0x1 ; 0000`0001
%define PS2_STATUS_INPUT_FULL 0x2 ; 0000`0010
NASM supports binary, so this can be:
Code:
%define PS2_STATUS_OUTPUT_FULL 00000001b
%define PS2_STATUS_INPUT_FULL 00000010b
For this code:
Code:
A20Test: ; if CF is set, A20 is disaled
push ds
push es
push di
push si
cli
clc
xor ax, ax
mov es, ax
mov di, 0x500 ; 0000:0500
mov ax, 0xFFFF
mov ds, ax
mov si, 0x510 ; (0xFFFF + 0x510) - (0x10 "16 bytes")
mov al, byte [es:di]
push ax
mov al, byte [ds:si]
push ax
mov byte [es:di], 0x00
mov byte [ds:si], 0xFF
cmp byte [es:di], 0xFF
jne .L1
stc
.L1:
pop ax
mov byte [es:di], al
pop ax
mov byte [ds:si], al
sti
pop es
pop di
pop si
pop ds
ret
You don't need to use SI or DI and can do (e.g.) " mov al, byte [es:0x500]" instead to avoid saving them on the stack and popping them after; and you know that all of the segment registers are zero and don't need to save, set or restore ES. You could also use (e.g.) FS instead of DS and leave it "trashed" (because nothing else uses FS) to avoid saving/loading that, and set FS to 0xFFFF during the "init" code (instead of setting it to zero). You also shouldn't need to disable and re-enable IRQs. Note that a comparison will overwrite the previous value in the carry flag, so there's no point doing an initial "CLC".
By using a memory location that has a known value (e.g. the byte at 0x07DF which must contain the value 0xAA) you could reduce the code further. For example (with all these changes) the code could be:
Code:
A20Test: ; if CF is set, A20 is disabled
cmp byte [fs:0x07DF+0x0010],0xAA
jne .disabled2
inc byte [0x07DF]
cmp byte [fs:0x07DF+0x0010],0xAA
jne .disabled1
dec byte [0x07DF]
clc
ret
.disabled1:
dec byte [0x07DF]
.disabled2:
stc
ret
For the code that asks BIOS to enable A20; I wouldn't rely on BIOS returning carry to indicate success/failure. You can delete the "jnc EnterUnreal" instruction (immediately after then "int 0x15") to always test if A20 is enabled or not instead.
For the initialisation code; there's no point setting GS because it's never used, and setting SS:SP to 0x0000:0x7B00 wastes 256 bytes (from 0x7B00 to 0x7C00). Taking into account the FS changes above; it could be:
Code:
Init:
xor ax, ax
mov ds, ax
mov es, ax
cli
mov ss, ax
mov sp, 0x7C00
sti
dec ax
mov fs, ax
Both of these routines need time-outs (and need to return some kind of error if the time out expires) so that the OS doesn't unexpectedly lock up when there's problems with the PS/2 controller:
Code:
PS2WaitForData:
in al, PS2_STATUS_REG
test al, PS2_STATUS_OUTPUT_FULL
jnz PS2WaitForCmd
ret
PS2WaitForCmd:
in al, PS2_STATUS_REG
test al, PS2_STATUS_INPUT_FULL
jnz PS2WaitForCmd
ret
This code assumes that the direction flag is clear:
Code:
PutS:
mov ah, 0x0E
.L2:
lodsb
test al, al
jz .L1
int 0x10
jmp .L2
.L1:
ret
You need to make sure that the direction flag is clear (instead of assuming it is) by putting a CLD instruction somewhere.
For this piece of code:
Code:
xor ah, ah
int 0x16 ;read key, block mode
jmp 0xFFFF:0x000 ;restart
I wouldn't assume that the user has a keyboard (and is able to press a key); and I wouldn't assume that "jmp 0xFFFF:0x000" safely resets the computer (and doesn't cause a crash or infinite loop). Instead I'd display a "Reset your computer" error message and do "HLT" in a loop, and let the user use control+alt+delete (if they can) or reset the computer some other way.
Note: I also wouldn't assume that the computer has a video card or has a monitor attached. For this reason I do "continuous tone on error" in early boot code (and "continuous beeping" in later boot code, so there's an easily described audible hint indicating where it failed).For disk IO; there's no need to reset BIOS disk services before using it the first time, because you know that the BIOS just successfully used the disk to load your boot sector.
For the code itself; it's obviously intended for hard disks (because there's no BPB, and it uses "int 0x13 extensions" which typically don't support floppy); but hard disks are always partitioned and the code does not expect or support partitions. This makes it unusable in practice. To create more space for code that handles partitions properly (and improve the error handling - e.g. more different messages to tell the user why loading from disk failed and/or why their partitions didn't make sense) I'd shift all of the code to enter unreal mode and all of the A20 code into the second stage.
Cheers,
Brendan