Open ecm-pushbx opened 3 months ago
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?
Sure, just make sure to keep the license and attribution (from syslinux which is what I based them on).
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.
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.
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.
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
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.
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 thedb
. Or usedriveno equ 600h
,driveno equ start
, ordriveno 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
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.
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?
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.
Here is a
lea
that can be replaced by amov
to save a byte: https://github.com/FDOS/fdisk/blob/f52198a93b5e48ab56a93a5613723d65ada23251/source/fdisk/bootnorm.asm#L54You 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 usingtest
becausetest
needs an immediate byte: https://github.com/FDOS/fdisk/blob/f52198a93b5e48ab56a93a5613723d65ada23251/source/fdisk/bootnorm.asm#L119The 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-L130Function 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#L126You'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 orinstsect /P none
(in the instsect help).