I see a number of things that may help you improve your code.
Eliminate "magic numbers"
This code has a number of "magic numbers," that is, unnamed constants such as 2, 0x0e, 0x10, etc. Generally it's better to avoid that and give such constants meaningful names. That way, if anything ever needs to be changed, you won't have to go hunting through the code for all instances of "7" and then trying to determine if this particular 0x07 is relevant to the desired change or if it is some other constant that happens to have the same value. With NASM, you can use the %define
directive:
%define KBDINT 16h
Then in the code:
int KBDINT
Use XOR
to clear a register
The idiomatic way to clear a register in x86 assembly language is to use xor
:
xor ax,ax ; ax = 0
This instruction coding is shorter than mov ax,0000h
.
Use comments to indicate register usage
Keeping track of register usage is one of the most important tasks for an assembly language programmer. A useful technique for tracking this is the use of comments. For example, instead of this:
bgetkey: mov ax, 0 ; clear register a
mov ah, 0x10 ;
int KBDINT ; interrupt bios keyboard
ret ; return
write this:
;****************************************************************************
;
; bgetkey - use BIOS call to get a keystroke; blocks until key available
;
; INPUT: none
; OUTPUT: ah = BIOS scan code, al = ASCII char
; DESTROYED: none
;****************************************************************************
bgetkey:
mov ah, 0x10 ;
int KBDINT ; interrupt bios keyboard
ret ; return
Use macros to simplify code
Although the macro support in NASM is not very good, it does exist and can be used to simplify your code. For instance, the routine above is only accessed once. The shortened version above is only three instructions but would be only two instructions and eliminate a call
if it were placed inline in the code. I'd write it like this:
%macro BIOSWAITKEY 0
mov ah, 0x10 ;
int KBDINT ; interrupt bios keyboard
%endmacro
Then it's used in the code like this:
type:
mov si, qbuf ; set byte buffer ptr for printing
BIOSWAITKEY
mov [qbuf], al ; copy key byte to buffer
Set the segment registers explicitly
The BIOS calling your loader should have CS
and DS
both set to 0
but it's unfortunately not guaranteed. Some old BIOS would call 7C0:0
rather than 0:7C00
so most boot loader code explictly sets the segment registers. Your code sets the DS
register only and not SS
. For a robust bootloader, set the segment registers explictly to either 0
or to equal whatever CS
happens to be.
Carefully consider stack usage
The prints
routine currently pushes and then pops all registers. This routine is not speed critical, but it's useful to get into the habit of thinking carefully about stack usage. In this case, I would probably only save AX
and BX
instead or maybe just BX
. The code does not really need to preserve the value of SI
and probably not AX
either, with some small changes to the calling code.
Put each label on its own line
Maintaining code which has code on the same line as labels is a pain. Better practice is to have each label be on a line by itself. This makes it much easier to maintain the code.
Avoid branching where practical
Branching is a costly operation to the processor, so avoiding branching (that is conditional or unconditional jumps) saves cycles and time. In this code, we have this:
prints: push ax ; modified per previous point
push bx ;
.loop: mov ah, 0x0e ;
mov al, [si] ;
cmp al, 0 ; check for null terminator
jz print_end ; stop printing
mov bh, 0x00 ;
mov bl, 0x07 ;
int 0x10 ; interrupt bios tty
inc si ; next character
jmp .loop ; jump beginning
print_end: pop bx ;
pop ax ;
ret ; return
This means that the the unconditional jmp
at the end of the loop is always executed. Instead, it would be better to restructure the code to only have a single conditional branch within the loop.
prints:
push ax ;
push bx ;
jmp .begin ; skip over loop first iteration
.loop:
mov bx, PAGE0WHTBLK ; page 0, white on black
mov ah, TTYOUT ;
int VIDINT ; interrupt bios tty
inc si ; next character
.begin:
mov al, [si] ;
cmp al, 0 ; check for null terminator
jnz .loop ; keep printing
pop bx ;
pop ax ;
ret ; return
Set the AX
register close to the INT
instruction
To make it easier for another programmer to understand your code, it's a good idea to set the AX
(or AH
register) just before the INT
instruction the calls a BIOS or operating system function. That way the two most important pieces of information, namely "which interrupt" and "which service" are near each other, making it easy to look them up. Better still, use named constants as well as putting them near each other.
Eliminate unused variables
The .buf
area in bgetkey
is never used and should be eliminated.
Prefer assemble-time to runtime mathematics
The newinput
label starts with these three lines:
mov bx, mem ; set register b to memory start
add bx, word 2 ; increment by size of memory ptr
mov word [mem], bx ; set pointer at first memory byte
Better would be to let the assembler do the calculation instead:
mov bx, mem+2 ; point to available space
mov word [mem], bx ; save pointer to available space
Better still would be to eliminate the use of mem
to store the pointer. That is, just use bx
to store the pointer and use all of mem
as the buffer area.
Simplify calling mechanisms where appropriate
The qbuf
structure appears to be set up solely for the purpose of printing single character output. In the code, the first byte of qbuf
is set to a value and then si
pointed to qbuf
and then the prints
routine is called. Better would be to simplify by creating a routine to simply and directly print a single character. In fact, this is what the BIOS video TTY output routine (that you're already using) actually does already, so make a function for that:
;****************************************************************************
;
; printch: prints a single character to screen
;
; INPUT: al = character to print
; OUTPUT: none
; DESTROYED: none
;****************************************************************************
printch:
push ax ;
push bx ;
mov bx, PAGE0WHTBLK ; page 0, white on black
mov ah, TTYOUT ;
int VIDINT ; interrupt bios tty
pop bx ;
pop ax ;
ret ; return
Again, a macro is now useful for using this routine:
%macro PRINTCHAR 1
%ifnidni %1,al
mov al, %1
%endif
call printch
%endmacro
Now it can be used like any of these:
PRINTCHAR al
PRINTCHAR [bx]
PRINTCHAR CR
Name important memory locations
One very important but unnamed memory location is the end of the mem
area. I would modify the existing code to look instead like this:
mem db 0
times 0200h - 3 - ($ - $$)db 0
endofmem db 0
stop dw 0xAA55
Take advantage of every byte
Especially in a bootloader, where every byte matters, it's useful to take advantage of every available byte. For instance, after your code begins, the "welcome" messsage is no longer needed. You could overlay that message within the mem
buffer to allow it to be overwritten by input from the user. Similarly, technically, once the sector has been read into memory, the signature 0xAA55
bytes could be overwritten as well. Similarly, the error message strings can also be combined to save a few bytes.
Consider reformatting the code
You may, of course, do it any way that pleases you, but typical x86 assembly language code is formatted with labels in column 1 and code indented to column 9 (that is, the size of one traditional tab stop). After that, comments are most often aligned (as you have done) at some multiple of tab stops.
Carefully consider register allocation
By using another register, such as di
rather than bx
to point to the memory buffer, we can avoid having to save the bx
register.
Comments should tell why, not what
Comments should generally explain why you're doing what you do, and not simply repeat what the instruction does. So this is not a good comment:
hlt ; halt the cpu
This, however, is a better comment:
je readmemdone ; if found begin another input line
Putting it all together
Applying all of these suggestions yields a program that is easier to maintain, easier to read, smaller and better structured:
boot.asm
bits 16 ; 16 bit real mode
org 0x7C00 ; loader start in memory
%define KBDINT 16h
%define VIDINT 10h
%define TTYOUT 0eh
%define PAGE0WHTBLK 0x0007
%define CR 0x0D
%define LF 0x0A
%define NUL 0x00
%macro BIOSWAITKEY 0
mov ah, 0x10 ;
int KBDINT ; interrupt bios keyboard
%endmacro
%macro PRINTSTR 1
mov si, %1
call prints
%endmacro
%macro PRINTCHAR 1
%ifnidni %1,al
mov al, %1
%endif
call printch
%endmacro
start: jmp main ; goto main
;****************************************************************************
;
; prints: prints a NUL-terminated string to screen
;
; INPUT: ds:si ==> NUL-terminated buffer to print
; OUTPUT: none
; DESTROYED: bx, si
;****************************************************************************
prints:
jmp .begin ;
.loop:
call printch ; print the char in AL
inc si ; next character
.begin:
mov al, [si] ;
cmp al, NUL ; check for NUL terminator
jnz .loop ; keep printing
ret ; return
;****************************************************************************
;
; printch: prints a single character to screen
;
; INPUT: al = character to print
; OUTPUT: none
; DESTROYED: bx
;****************************************************************************
printch:
push ax ;
mov bx, PAGE0WHTBLK ; page 0, white on black
mov ah, TTYOUT ;
int VIDINT ; interrupt bios tty
pop ax ;
ret ;
main:
xor ax, ax ;
mov ds, ax ;
mov es, ax ; both es and ds are now 0
PRINTSTR welcome ; print welcome string
newinput:
mov di, inputbuffer ; point to memory buffer
type:
BIOSWAITKEY ; fetch a key
PRINTCHAR al ; echo to screen
mov ah, NUL ; make sure string is always terminated
cmp di, endofmem ; check for overflow
je oom ; halt if Out Of Memory (oom)
mov [di], ax ; copy keystroke + NUL to memory
inc di ; advance to next byte in buffer
cmp al, CR ; CR signals end of input
jne type ; goto next key if not found
PRINTSTR newline ; print newline
PRINTSTR inputbuffer ; print string from memory
PRINTSTR newline ; another newline
jmp newinput ; keep getting lines forever
oom:
PRINTSTR outomem ; print out of memory message
here:
jmp here ; sit and spin
outomem db CR, LF, "out of memory", CR ; terminated by newline below
db LF, "halting", CR
newline db LF, NUL
inputbuffer db NUL
; welcome will be overwritten by input
welcome db "Welcome to the loaderless bootloader.", CR, LF,
db "Type anything and it will be repeated by the magic of "
db "assembly language!", CR, LF, NUL
times 0200h - 2 - ($ - $$) db NUL
endofmem dw 0xAA55
Use real version control rather than just file copies
While it's important to have backup files when you're changing source rapidly and experimentally, you might consider using a more appropriate mechanism for that. I'd suggest that you might want to use git
instead of creating multiple gzip files. That way it will be easier to document why changes were made, even if you don't plan on sharing the code.