OSDev.org

The Place to Start for Operating System Developers
It is currently Fri Apr 19, 2024 10:09 am

All times are UTC - 6 hours




Post new topic Reply to topic  [ 4 posts ] 
Author Message
 Post subject: My 4th bootloader, comments
PostPosted: Mon Apr 09, 2018 3:26 am 
Offline

Joined: Mon Apr 09, 2018 3:09 am
Posts: 1
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:
[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


Top
 Profile  
 
 Post subject: Re: My 4th bootloader, comments
PostPosted: Mon Apr 09, 2018 8:28 am 
Offline
Member
Member
User avatar

Joined: Sat Jan 15, 2005 12:00 am
Posts: 8561
Location: At his keyboard!
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

_________________
For all things; perfection is, and will always remain, impossible to achieve in practice. However; by striving for perfection we create things that are as perfect as practically possible. Let the pursuit of perfection be our guide.


Top
 Profile  
 
 Post subject: Re: My 4th bootloader, comments
PostPosted: Sun Apr 22, 2018 3:32 pm 
Offline
Member
Member

Joined: Fri Aug 26, 2016 1:41 pm
Posts: 692
Brendan wrote:
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


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?


Top
 Profile  
 
 Post subject: Re: My 4th bootloader, comments
PostPosted: Mon Apr 23, 2018 3:17 am 
Offline
Member
Member
User avatar

Joined: Sat Jan 15, 2005 12:00 am
Posts: 8561
Location: At his keyboard!
Hi,

MichaelPetch wrote:
Brendan wrote:
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


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

_________________
For all things; perfection is, and will always remain, impossible to achieve in practice. However; by striving for perfection we create things that are as perfect as practically possible. Let the pursuit of perfection be our guide.


Top
 Profile  
 
Display posts from previous:  Sort by  
Post new topic Reply to topic  [ 4 posts ] 

All times are UTC - 6 hours


Who is online

Users browsing this forum: No registered users and 85 guests


You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot post attachments in this forum

Search for:
Jump to:  
Powered by phpBB © 2000, 2002, 2005, 2007 phpBB Group