I began this month with trying to discover how shellcode is made. My search led me to wanting to learn assembly, so I wrote a simple bootloader with NASM:

                bits       16               ; 16 bit real mode
org        0x7C00           ; loader start in memory

start:          jmp        main             ; goto main

bgetkey:        mov        ax, 0            ; clear register a
mov        ah, 0x10         ;
int        16h              ; interrupt bios keyboard
ret                         ; return
.buf       dw 0             ; buffer size one word

prints:         pusha                       ;
.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:      popa                        ;
ret                         ; return

main:           mov        ax, 0x0000       ; clear register a
mov        ds, ax           ;
mov        si, welcome      ; copy welcome string pointer
call       prints           ; print string
newinput:       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
type:           mov        si, qbuf         ; set byte buffer ptr for printing
call       bgetkey          ; capture keyboard input
mov        [qbuf], al       ; copy key byte to buffer
call       prints           ; print the character
mov        bx, [mem]        ; copy memory stop to b, decrement
cmp        bx, stop - 1     ; check for overflow preserve null
je         oom              ; halt - no more memory
mov        byte [bx], al    ; copy keystroke to memory
add        bx, byte 1       ; increment memory pointer
mov        [mem], bx        ; restore memory pointer to memory
cmp        byte [qbuf], 0x0D; check for carriage return
mov        si, newline      ; copy pointer to line feed string
call       prints           ; print the line feed

mov        bx, mem + 2      ; restore start of valid memory
readmem:        cmp        byte [bx], 0x0D  ; check for carriage return
je         readmemdone      ; if found begin another input line
mov        cl, [bx]         ; register character byte
mov        byte [qbuf], cl  ; copy byte to string buffer
mov        si, qbuf         ; copy buffer ptr for printing
call       prints           ; print the character
inc        bx               ; increment memory pointer
call       prints           ; print carriage return
mov        si, newline      ; copy line feed string to buffer
call       prints           ; print line feed
jmp        newinput         ; ready new line

oom:            mov        si, outomem      ; copy out of memory message ptr
call       prints           ; print message

halt:           mov        si, halting      ; copy halting mesage ptr
call       prints           ; print final message
hlt                         ; halt the cpu

welcome db "boot", 0x0A, 0x0D, 0x00
newline db 0x0A, 0x00
outomem db 0x0A, 0x0D, "out of memory", 0x0A, 0x0D, 0x00
halting db 0x0A, 0x0D, "halting", 0x00
qbuf       dw 0, 0
mem        db 0

times 0200h - 2 - (- $$)db 0 stop dw 0xAA55  I use a script to compile and run the loader. It makes a fallback copy of all my files each time: nasm -o boot.bin -f bin boot.asm && \ tar -zcvf ~/os-(date +%Y%m%d%H%M%S).tar.gz ../os && \ qemu boot.bin  If I type: testing 123 hello loader overflow test [press . until it halts] I get the following correct output: At an earlier point in time I thought I would never need to learn a low level language. I was wrong. The blank comments are placeholders for once I learn more about what they do. I am primarily asking for feedback on the accuracy of the comments, then secondarily asking for efficiency of size and correctness of overall flow and layout. ## 5 Answers Lots of review of the raw mechanics of your code. Not much review of the design. So here are just a few design tips. # Use BPBs, because other people will. start: jmp main ; goto main There are two reasons that Volume Boot Record programs do this. The first and foremost is in order to skip over an embedded BIOS Parameter Block. You may not think that you need a BPB in your volume. You almost certainly will. Others have thought that over the years, and later been bitten when they do things like not have a BPB and use tools from operating systems that require that various types of partition have a BPB in the VBR. It bit IBM in the OS/2 Boot Manager. It will bite you. The second is that there are even tools (broken ones, that are fortunately not in wide use any more) that key off the first instruction in a VBR, expecting it to be a jmp. # Will your code ever be run on an original 8088 manufactured in 1979? Note that the cli and the sti are super important. But not for the reason stated. They stop interrupts from occurring in the window where you have loaded only half of ss:sp, and the stack pointer is thus invalid. This piece of 808x wisdom has been unnecessarily and erroneously extended to the other segment registers in that answer, even though there are no accompanying offsets being loaded with those other segments. In truth, those other registers are not expected to have any particular values by interrupt handlers, and you don't need to temporarily disable interrupts in order to manipulate them. 8086 "far" memory model DOS programs manipulate ds and es all of the time without masking interrupts, just fine. And the importance is overstated. Since the 8088, a mov ss has implicitly caused interrupts (even some that cli cannot mask) to be deferred for one instruction, so that one can slip in a mov sp immediately after it and the two instructions will be implicitly interrupt-safe without an explicit cli/sti pair. (Early 8088s had a bug, but the 8088 reference manual documented this as the behaviour. It's still in the Intel reference manuals today.) Since the 80386, we've had the lss esp instruction that loads the register pair in one instruction. ## Interesting historical perspective In PC Magazine in 1987 Robert L. Hummel called the 8088 bug "a serious bug that should be attended to" and explained the (by then already folkloric) workaround of a cli and sti pairing. In fact Intel had fixed the bug in 1981. The 80386 had arrived in 1985. Thirty years later people are still handing around folk rules for coding this stuff, which have even become distorted. # Exit properly.  hlt ; halt the cpu welcome db "boot", 0x0A, 0x0D, 0x00 Congratulations! You just executed a prompt string as code. The IBM PC Compatible firmware provides two software interrupts for properly "exiting" a bootstrap program back to the firmware. The BIOS Boot Specification explains their use. Use them. # Further reading • Good advice except that the BIOS Boot Specification does not describe how to exit back to the firmware from a bootloader. Instead, it describes what the BIOS does, not what the bootloader is required to do. I'd also offer this real bootloader source code as inspiration. – Edward Jun 21 '15 at 14:58 • You haven't read it properly. Read it slowly and in full, paying particular attention to Appendix D. – JdeBP Jun 21 '15 at 15:06 • I'm actually quite familiar with it -- I was working at Compaq doing BIOS development when the document was written. What it says is that if the OS is not present or is unable to load, it should execute int 18h. Neither condition is true in this bootloader, so that advice is not applicable. – Edward Jun 21 '15 at 15:12 You are very inconsistent with how you clear a register: bgetkey: mov ax, 0 ; clear register a  and main: mov ax, 0x0000 ; clear register a  I recommend xoring the register with itself. Example: xor ax, ax  As recommended by @icktoofay, in this section: bgetkey: mov ax, 0 ; clear register a mov ah, 0x10 ; int 16h ; interrupt bios keyboard ret  Instead of clearing all of ax and changing ax separately, just do this: mov ax, 0x1000  That way, you are changing the upper and lower bytes of ax in one instruction. I find that it is common in bootloaders that I've seen to not use org 0x7C00 as people find that it is taking the "easy way out". Instead, people often manuall setup the segments like this: cli mov ax,07C0h mov ds,ax mov gs,ax mov fs,ax mov es,ax  and then this to set up the stack(see the bios memory mapping where where the stack is located): mov ax,07E0h mov ss,ax mov bp,ax mov sp,0xff sti  Note that the cli and the sti are super important. When messing around with segments, interrupts might "accidentally" fire, which could mess up your program. cli will disable interrupts and sti will re-enable interrupts. On some lines like this: mov byte [bx], al ; copy keystroke to memory  I do not think that the byte is necessary since al is already a byte in size, although I have not tested that theory. Ascii numbers like 0x0A and 0x0D can be a little confusing, because some people don't know what they are, and others it can take a second to process. To improve readability, I recommend that you use %define to define a macro for these numbers, like in C. For example, %define NEWLINE 0x0A  In your prints routine, each iteration you are restoring ah with 0x0e, even though ah isn't touched at all during the routine. I recommend putting mov ah, 0x0e  before the .loop label, so you are unnecessarily updating ah. I recommend you check out this post for a good example of a bootloader. I will continue to look through the code and add more improvements that I see necessary. • Re. the recommendation to replace mov ax, 0 with mov al, 0: that’s true, but I’d go even further and say that if you’re changing both the high and the low byte, change them both at once with a mov ax, 0x1000. – icktoofay Jun 21 '15 at 2:24 • @icktoofay Ah yes! That is a great idea. Do you mind if I modify my post to accomodate to your recommendation? – SirPython Jun 21 '15 at 2:31 • By all means, please do! – icktoofay Jun 21 '15 at 2:32 • The only need I see for disabling interrupts in that code is that MOV can't update both SS and SP atomically. An interrupt coming between the two updates would use an inconsistent stack pointer. Updating the other segment registers can safely be done without disabling interrupts. – kasperd Jun 21 '15 at 16:50 • @SirPython I'd actually forgotten it was 0000:7C00... it's been ages. However, my main point is that CS will be 0000, not 07C0, on any hardware the OP cares to use, including VMs. It hasn't been broken since the early 8086's, so there's really no point in carrying on with that code anymore. It's a leftover "folklore" that people keep dredging up long after its expiration date. If you're going to relocate your loader, then you need to eventually far jump, but not because your registers are incorrect. – phyrfox Jun 23 '15 at 18:26 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.

I echo most of SirPython’s recommendations (excepting possibly a detail about the recommended segment-setup code—I prefer setting up a flat memory space with all segment registers zero), but there’s a few other things I might change as well. I should note that a few of these may be more a matter of taste.

You are inconsistent with notation for hexadecimal constants. You use the 0xABCD syntax in some places, and the 0ABCDh syntax in others. I prefer the 0ABCDh syntax, but that’s just me.

## (Essentially) using .data where .bss would suffice

You’re putting your line buffer into the space allotted for your bootloader. As you write your code, the amount of space available will dwindle. If you need some space initialized to some particular initial values, sure, it’s rather convenient, so you ought to keep it there, but if you don’t need it initialized to anything in particular, you don’t need to cram it into those 512 bytes; you’ve got nearly the entire address space at your disposal. (Starting at 500h up ’till you hit your bootloader code itself, and then up until… gee, something else. But you’ve got a lot of space.) Keep in mind, though, that if you want it to be zero-initialized, now you have to zero-initialize it yourself.

## String instructions

I see lots of code accessing a buffer, doing something with it, and then incrementing that pointer. Do yourself a favor and read up on the string instructions made available to you by the x86 architecture: in particular, lodsb and stosb may become some of your best friends.

## inc

add reg, 1 is almost never necessary. Save some space and use inc reg. (With add, you need space to store the 1. Not so for inc.) There’s also dec to decrement, should you need it later.

## Testing equality to zero

Along the same lines, consider using test reg, reg rather than cmp reg, 0. Again, this avoids the need to store an immediate 0. (This is also why xor reg, reg is better than mov reg, 0 when reg is bigger than a byte.)

## Indentation

Here, a matter of style. When I use the local dot-labels, I usually indent them to indicate some sort of structure, e.g.:

read_line:
.next_character:
.done:


Also, the named db/dw definitions are introducing a label, so I would flush those left as I would for any other label.

## Operand size hints

This was mentioned by SirPython, but I assert more strongly that you can absolutely remove the operand size hint on, e.g., mov byte [bx], al. There are some places you can’t remove them (most commonly when using an instruction with a memory operand and an immediate operand), but I see it as clutter when it’s inferrable otherwise.

## Choice of assembler

This is entirely personal preference, but I used NASM for a while, until I came across what I believe is a very odd bug where it started having off-by-one errors in jump targets across 16- and 32-bit boundaries. When I hit that, I switched to FASM (which has a very similar syntax). That fixed my problem, and I’ve since grown to like it simply for the few syntactic differences it does make1. You may want to try it too.

1 One concrete thing FASM does that NASM doesn’t is it will remember what type you declared a data definition to be. So, for example:

some_variable db 0

some_function:
mov     [some_variable], 123
ret


…will work; the mov will be a byte-mov because some_variable was defined as a byte. (Of course, this can be overridden with an explicit operand size specified.) FASM’s style of macro I also think looks a bit nicer than NASM’s, and FASM’s virtual is quite nice for laying out memory (like your line buffer) outside your program.

• It's good to know lodsb and stosb (and scasb) and friends, but it's also worth knowing that they aren't necessarily faster than the corresponding equivalent instructions. Testing in context is the best way to know. – Edward Jun 22 '15 at 23:50
• @Edward: I was recommending them not for speed, but for code size, as space is at a premium inside a bootloader. Plus they’re rather convenient. – icktoofay Jun 25 '15 at 1:39

Everything from SyrPython and icktoofay, plus ...

Four instructions:

mov        ah, 0x0e         ;
mov        al, [si]         ;
cmp        al, 0            ; check for null terminator
jz         print_end        ; stop printing


I would suggest:

mov        ax,0x0e00        ; clears al
or         al,[si]          ; sets Z accordingly (if [si] is 0)
jz         print_end        ; stop printing


If I understand correctly (I used another assembler back in the days. Borland I think)

newinput:       mov        bx, mem          ; set register b to memory start
add        bx, word 2       ; increment by size of memory ptr


You move the offset of mem in bx. Then why not do

newinput:       mov bx, mem + 2


Like you do elsewhere ?

It seems you never use DX nor DI (and you still will not if you use the well suggested lodsb, that you may or may not use with rep, depending on your future direction)

You have 8 registers Ax Cx Dx Bx SP BP SI DI SP, you will mostly not play with SI and DI are usually used for offsets (what you use BX for) AX is your main register CX is mostly used for counting (and shifts) DX is for other data BP is mostly used for stack frame, which you don't care about so, another free one.

For using xor reg,reg, this is indeed a preferred way, with the only downside that it affects the flags so you cannot very well use it while pipelining your code (putting the operations in such order they are executed in parallel)

ie:

cmp bx, 2 <- U
mov ax, 1 <- V
je  label


cmp will be executed in pipeline U, and the mov in pipeline V because it does not affect the cmp operation.

But this may be a bit over the top for a loader.

• The U/V bit I'd agree is over the top in this case, but the info would surely be decent when they get around to, for example, trying to optimize the video hardware stuff. – phyrfox Jun 22 '15 at 19:43