MBR code comments

[ecm-pushbx]

Here is a lea that can be replaced by a mov to save a byte:

fdisk/source/fdisk/bootnorm.asm

Line 54 in f52198a

lea di, [bp + PARTTBL_OFFSET] ; start of partition table

You don't check for multiple partitions marked as active (lDOS oldmbr does):

fdisk/source/fdisk/bootnorm.asm

Line 65 in f52198a

; We found an active partition. Load its boot sector to 0:7c00,

If you address the relocated loader with segment zero then you only need one far jump (to relocate) and the jump to the next loader could be near instead:

fdisk/source/fdisk/bootnorm.asm

Line 73 in f52198a

jmp word 0x0:0x7c00 ; jump to volume boot code

The print function is only ever called to halt the machine so you could put the halting into this function rather than returning to after the ASCIZ message string:

fdisk/source/fdisk/bootnorm.asm

Lines 61 to 63 in f52198a

	call print
	db 'no active partition found', 0
	jmp $

The LBA packet could be aligned, at least on a word boundary (align 2 before the label):

fdisk/source/fdisk/bootnorm.asm

Lines 85 to 89 in f52198a

	;-----------------------------------------------------------------------------
	; BIOS disk access packet used by ext. INT13 LBA read function
	dap:
	.packet_size db 0x10

(The alignment is "free" if you move the packet to before the partition table as the table location must be aligned on a word boundary. lDOS MBR does not do this yet but it will.)

The word "shoud" is a misspelling:

fdisk/source/fdisk/bootnorm.asm

Line 111 in f52198a

mov bx, 0x55aa ; magic value shoud be changed after call

shr cx, 1 \ jnc ... is a byte shorter than using test because test needs an immediate byte:

fdisk/source/fdisk/bootnorm.asm

Line 119 in f52198a

test cl, 1 ; no support if LBA flag not set

The trail stc \ int 13h \ retn here can be shared with the CHS path, saving two bytes:

fdisk/source/fdisk/bootnorm.asm

Lines 128 to 130 in f52198a

	stc
	int 0x13
	ret

Function 42h doesn't use al so there is no need to init it to zero:

fdisk/source/fdisk/bootnorm.asm

Line 126 in f52198a

mov ax, 0x4200 ; LBA read function

You're depending on the CHS tuple from the partition table entry if using CHS access (lDOS MBRs calculate CHS address from the LBA start using the geometry queried from function 08h):

fdisk/source/fdisk/bootnorm.asm

Line 134 in f52198a

mov cx, [di + 2]

I don't really see a reason why not to relocate to 600h like is standard for MBR loaders. If you combine this with my earlier cs = ds = es = 0 suggestion the relocation even may save a byte or two.

Over here you assume that there is enough space for the partition table. I think you should cause a build error if there would be overlap:

fdisk/source/fdisk/bootnorm.asm

Lines 156 to 158 in f52198a

	; zero-fill rest of sector and put signature into place
	times 0x1fe - $ + $$ db 0
	db 0x55, 0xaa

Finally, the reason I wanted to write this was I want the MBR to pass ds:si -> partition table entry being booted. This is what lDOS MBR do and also lDOS boot attempts to detect unless patched out eg using the build option or instsect /P none (in the instsect help).

[boeckmann]

Thanks for the comments! This was the first iteration on trying to improve the old code. I will study your proposals to learn something from it :)

To make it short: as most of your comments are already implemented in the lDOS MBR code, may I substitute the current bootnorm.asm with your code?

[ecm-pushbx]

Sure, just make sure to keep the license and attribution (from syslinux which is what I based them on).

[ecm-pushbx]

NB, I didn't add the Xi8088 BIOS workaround to lDOS MBR yet. Is the Book8088 drive MBR-partitioned? If so I should add it. It is not a problem for the size of the code.

[boeckmann]

I am in the mood of doing some assembly, so I could not resist to apply most of your proposals to the bootnorm.asm :) 8539f43

Currently missing: multiple active detection and the LBA to CHS translation. The other stuff should be in.

[boeckmann]

I am searching for a MASM label (the keyword) equivalent for NASM. I have a driveno variable I would like to place at offset 0 (0:0x600) without loosing type information. Do you know a way of doing this? Currently it wastes a byte.

[ecm-pushbx]

There is no type information in NASM, a label is merely a scalar value or relocatable address. You can place the label driveno: at the beginning of your code and just omit the db. Or use driveno equ 600h, driveno equ start, or driveno equ RELOCATED_OFFSET

[ecm-pushbx]

I reviewed your changes. I would suggest moving the LBA packet down before the padding (just align it) because, as hpa put it:

; Maximum MBR size: 446 bytes; end-of-boot-sector signature also needed.
; Note that some operating systems (NT, DR-DOS) put additional stuff at
; the end of the MBR, so shorter is better.  Location 440 is known to
; have a 4-byte attempt-at-unique-ID for some OSes.

[ecm-pushbx]

There is no type information in NASM, a label is merely a scalar value or relocatable address. You can place the label driveno: at the beginning of your code and just omit the db. Or use driveno equ 600h, driveno equ start, or driveno equ RELOCATED_OFFSET

Of course, you can use the absolute start and in the resulting nobits section place driveno: resb 1 if you prefer =) Example: https://hg.pushbx.org/ecm/ldosmbr/file/dd717c3f54e0/oldmbr.asm#l261 and more explanation at https://bugzilla.nasm.us/show_bug.cgi?id=3392808

[boeckmann]

Played too much with JWasm in recent times to forget that NASM has basically no type info for labels (even no concept of variables).

That absolute is new to me, but seems to be nearly what I was looking for (apart from the type info). I'll try that out.

[boeckmann]

Regarding the multiple active partitions, did you encounter that anywhere? Converting the LBA values I guess is for the cases where multiple BIOSes may disagree over the CHS values?

[ecm-pushbx]

Regarding the multiple active partitions, did you encounter that anywhere?

No.

Converting the LBA values I guess is for the cases where multiple BIOSes may disagree over the CHS values?

Yes. In my MS-DOS fork I also deleted all uses of CHS values from the partition tables. When you can use LBA values there is no downside to doing so.