Page 1 of 1

My 4th bootloader, comments

Posted: Mon Apr 09, 2018 3:26 am
by XsDos
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.

Code: Select all

[ORG 0x7C00]
[BITS 16]

%define PS2_DATA_PORT 0x60
%define PS2_STATUS_REG 0x64
%define PS2_CMD_REG 0x64

%define PS2_STATUS_OUTPUT_FULL 0x1 ; 0000`0001
%define PS2_STATUS_INPUT_FULL  0x2 ; 0000`0010

%define PS2_CMD_DISABLE_PORT1  0xAD
%define PS2_CMD_DISABLE_PORT2  0xA7
%define PS2_CMD_READ_OUTPUT    0xD0
%define PS2_CMD_WRITE_OUTPUT   0xD1 ; write byte to controler output port
%define PS2_CMD_ENABLE_PORT1   0xAE
%define PS2_CMD_ENABLE_PORT2   0xA8

%define PS2_OUTPUT_BIT_A20     0x2 ; 0000`0010

%define ERR_INT13_RESET 0x1
%define ERR_INT13_EX 0x2
%define ERR_INT13_READ 0x3
%define ERR_STAGE2_MAGIC 0x4
%define ERR_A20_LINE 0x5
%define ERR_STAGE2_FAILED 0x6

%define STAGE2_SECTOR_INDEX 1 ;sectors are counted from 0, the bootloader (this code) is in sector 0
%define STAGE2_SECTORS_COUNT 2
%define STAGE2_BASE_ADDRESS 0x7E00
%define STAGE2_MAGIC 0x66AA
%define STAGE2_ENTRYPOINT_ADDRESS STAGE2_BASE_ADDRESS + 2

Main:
	jmp 0x00:Init
	nop

cBootDrive: db 0
cLastError: db 0x0F
SzErrorMsgPart1: db "Critical error: 0x", 0x0
SzErrorMsgPart2: db 0xA, 0xA, 0xD, "Press any key to restart", 0x0
SzBooting: db "Booting...", 0xA, 0xD, 0x0

DapRead: db 0x10
	  db 0
	  dw STAGE2_SECTORS_COUNT
	  dd STAGE2_BASE_ADDRESS
	  dq STAGE2_SECTOR_INDEX

Gdt: dq 0 ; null desc
     dw 0xFFFF
     dw 0
     db 0
     db 0x92 ; 1001`0010
     db 0xCF ; 1100`1111
     db 0
GdtPtr: dw (GdtPtr - Gdt) - 1
	 dd Gdt

;assuming there is no keyboard
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

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

PutS:
	mov ah, 0x0E
.L2:
	lodsb
	test al, al
	jz .L1
		int 0x10
		jmp .L2
.L1:
	ret

FlushError:
	lea si, [SzErrorMsgPart1]
	call PutS

	mov ah, 0xE
	mov al, [cLastError]
	cmp al, 0xA
	jl .L1
		add al, 0x7
.L1:
	add al, '0'
	int 0x10

	lea si, [SzErrorMsgPart2]
	call PutS

	xor ah, ah
	int 0x16 ;read key, block mode

	jmp 0xFFFF:0x000 ;restart

Init:
	cli
	xor ax, ax
	mov ds, ax
	mov es, ax
	mov fs, ax
	mov gs, ax
	mov ss, ax
	mov sp, 0x7B00
	sti

	mov byte [cBootDrive], dl

ResetDisk:
	mov byte [cLastError], ERR_INT13_RESET ; set last error to not be 0
	xor ax, ax
	int 0x13
	jc FlushError

CheckInt13Ex:
	inc byte [cLastError] ; ERR_INT13_EX
	mov bx, 0x55AA
	mov ah, 0x41
	int 0x13
	jc FlushError

FindStage2:
	inc byte [cLastError] ; ERR_INT13_READ
	mov ah, 0x42
	lea si, [DapRead]
	int 0x13
	jc FlushError

CheckStage2Magic:
	inc byte [cLastError] ; ERR_STAGE2_MAGIC
	lea si, [STAGE2_BASE_ADDRESS]
	lodsw
	cmp ax, STAGE2_MAGIC
	jne FlushError

A20Enable:
	xor cl, cl
	inc byte [cLastError] ; ERR_A20_LINE

A20FromBios:
	call A20Test
	jnc EnterUnreal

	mov ax, 0x2401
	int 0x15
	jnc EnterUnreal

	call A20Test
	jnc EnterUnreal
	
A20FromPS2:
	call PS2WaitForCmd
	mov al, PS2_CMD_DISABLE_PORT1
	out PS2_CMD_REG, al

	call PS2WaitForCmd
	mov al, PS2_CMD_DISABLE_PORT2
	out PS2_CMD_REG, al

	call PS2WaitForCmd
	mov al, PS2_CMD_READ_OUTPUT
	out PS2_CMD_REG, al

	call PS2WaitForData
	in al, PS2_DATA_PORT ;get output port current value
	push ax ;save it
	
	call PS2WaitForCmd
	mov al, PS2_CMD_WRITE_OUTPUT
	out PS2_CMD_REG, al

	call PS2WaitForCmd
	pop ax
	or ax, PS2_OUTPUT_BIT_A20
	out PS2_DATA_PORT, al

	call PS2WaitForCmd
	mov al, PS2_CMD_ENABLE_PORT1
	out PS2_CMD_REG, al

	call PS2WaitForCmd
	mov al, PS2_CMD_ENABLE_PORT2
	out PS2_CMD_REG, al

	call A20Test
	jnc EnterUnreal

	mov ax, 0x0E41
	int 0x10

	inc cl
	cmp cl, 4
	jbe A20FromBios

; final try, risky
A20FastGate:
	in al, 0x92
	or al, 2
	out 0x92, al

	mov ax, 0x0E42
	int 0x10

	call A20Test
	jc FlushError

EnterUnreal:
	cli
	push ds

	lgdt [GdtPtr]
	mov eax, cr0
	or al, 1 ; enter protected mode
	mov cr0, eax

	mov bx, 0x08 ;use the first descriptor
	mov ds, bx

	and al, 0xFE ; remove protected mode bit
	mov cr0, eax

	pop ds
	sti

EnterStage2:
	lea si, [SzBooting]
	call PutS

	;@TODO: jmp to stage2

SysHlt:
	cli
	hlt
	jmp SysHlt

times 510 - ($ - $$) db 0
			db 0x55
			db 0xAA

Re: My 4th bootloader, comments

Posted: Mon Apr 09, 2018 8:28 am
by Brendan
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: Select all

[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: Select all

    ORG 0x7C00
    BITS 16

Code: Select all

%define PS2_STATUS_OUTPUT_FULL 0x1 ; 0000`0001
%define PS2_STATUS_INPUT_FULL  0x2 ; 0000`0010
NASM supports binary, so this can be:

Code: Select all

%define PS2_STATUS_OUTPUT_FULL 00000001b
%define PS2_STATUS_INPUT_FULL  00000010b

For this code:

Code: Select all

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: Select all

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: Select all

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: Select all

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: Select all

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: Select all

	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

Re: My 4th bootloader, comments

Posted: Sun Apr 22, 2018 3:32 pm
by MichaelPetch
Brendan wrote:

Code: Select all

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
I understand the logic here, but not quite the memory address you chose. Are you sure you mean 0x07DF or did you mean 0x7DFF. I'm assuming the 0xAA is the one at memory address 0x7DFF at the end of the bootloader that is read into memory. If the segment is 0xFFFF as you suggest it should be set to then 0xFFFF<<4+(0x7DF+0x10) = 1007DF . In your code, IMHO everywhere you have the value 0x07DF I think you mean 0x7DFF?

Re: My 4th bootloader, comments

Posted: Mon Apr 23, 2018 3:17 am
by Brendan
Hi,
MichaelPetch wrote:
Brendan wrote:

Code: Select all

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
I understand the logic here, but not quite the memory address you chose. Are you sure you mean 0x07DF or did you mean 0x7DFF. I'm assuming the 0xAA is the one at memory address 0x7DFF at the end of the bootloader that is read into memory. If the segment is 0xFFFF as you suggest it should be set to then 0xFFFF<<4+(0x7DF+0x10) = 1007DF . In your code, IMHO everywhere you have the value 0x07DF I think you mean 0x7DFF?
Yes; everywhere I wrote 0x7DF I should have wrote 0x7DFF - not just in the code, but in the text that preceded it too. :oops:


Cheers,

Brendan