MBR code comments

[ecm-pushbx]

Here is a lea that can be replaced by a mov to save a byte: https://github.com/FDOS/fdisk/blob/f52198a93b5e48ab56a93a5613723d65ada23251/source/fdisk/bootnorm.asm#L54

You don't check for multiple partitions marked as active (lDOS oldmbr does): https://github.com/FDOS/fdisk/blob/f52198a93b5e48ab56a93a5613723d65ada23251/source/fdisk/bootnorm.asm#L65

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: https://github.com/FDOS/fdisk/blob/f52198a93b5e48ab56a93a5613723d65ada23251/source/fdisk/bootnorm.asm#L73

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: https://github.com/FDOS/fdisk/blob/f52198a93b5e48ab56a93a5613723d65ada23251/source/fdisk/bootnorm.asm#L61-L63

The LBA packet could be aligned, at least on a word boundary (align 2 before the label): https://github.com/FDOS/fdisk/blob/f52198a93b5e48ab56a93a5613723d65ada23251/source/fdisk/bootnorm.asm#L85-L89 (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: https://github.com/FDOS/fdisk/blob/f52198a93b5e48ab56a93a5613723d65ada23251/source/fdisk/bootnorm.asm#L111

shr cx, 1 \ jnc ... is a byte shorter than using test because test needs an immediate byte: https://github.com/FDOS/fdisk/blob/f52198a93b5e48ab56a93a5613723d65ada23251/source/fdisk/bootnorm.asm#L119

The trail stc \ int 13h \ retn here can be shared with the CHS path, saving two bytes: https://github.com/FDOS/fdisk/blob/f52198a93b5e48ab56a93a5613723d65ada23251/source/fdisk/bootnorm.asm#L128-L130

Function 42h doesn't use al so there is no need to init it to zero: https://github.com/FDOS/fdisk/blob/f52198a93b5e48ab56a93a5613723d65ada23251/source/fdisk/bootnorm.asm#L126

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): https://github.com/FDOS/fdisk/blob/f52198a93b5e48ab56a93a5613723d65ada23251/source/fdisk/bootnorm.asm#L134

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: https://github.com/FDOS/fdisk/blob/f52198a93b5e48ab56a93a5613723d65ada23251/source/fdisk/bootnorm.asm#L156-L158

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.