Making a bootable image

[MichaelPetch]

Okay, I actually disassembled the code in your disk image and see the problem as to why the code doesn't work: The disassembled code is not what is in boot.asm. `ndisasm -b16 BonsOS.img -o 0x7c00 -e 0x100000` shows this:

Code: Select all

00007C00  EB50              jmp short 0x7c52 <----- this jump actually jumps into the middle of an instruction
00007C02  90                nop
00007C03  6D                insw
00007C04  6B66732E          imul sp,[bp+0x73],byte +0x2e
00007C08  6661              popad
00007C0A  7400              jz 0x7c0c
00007C0C  0204              add al,[si]
00007C0E  0400              add al,0x0
00007C10  0200              add al,[bx+si]
00007C12  0200              add al,[bx+si]
00007C14  00F8              add al,bh
00007C16  40                inc ax
00007C17  0020              add [bx+si],ah
00007C19  004000            add [bx+si+0x0],al
00007C1C  0000              add [bx+si],al
00007C1E  0000              add [bx+si],al
00007C20  0000              add [bx+si],al
00007C22  0100              add [bx+si],ax
00007C24  800029            add byte [bx+si],0x29
00007C27  AD                lodsw
00007C28  7FD3              jg 0x7bfd
00007C2A  0F426F6E          cmovc bp,[bx+0x6e]
00007C2E  734F              jnc 0x7c7f
00007C30  53                push bx
00007C31  2020              and [bx+si],ah
00007C33  2020              and [bx+si],ah
00007C35  204641            and [bp+0x41],al
00007C38  54                push sp
00007C39  31362020          xor [0x2020],si
00007C3D  200E1FBE          and [0xbe1f],cl
00007C41  5B                pop bx
00007C42  7CAC              jl 0x7bf0
00007C44  22C0              and al,al
00007C46  740B              jz 0x7c53
00007C48  56                push si
00007C49  B40E              mov ah,0xe
00007C4B  BB0700            mov bx,0x7
00007C4E  CD10              int 0x10
00007C50  5E                pop si
00007C51  EBF0              jmp short 0x7c43 ; <------- This is a bogus jump!
00007C53  8CC8              mov ax,cs
00007C55  8ED8              mov ds,ax
00007C57  8EC0              mov es,ax
00007C59  8EE0              mov fs,ax
00007C5B  8EE8              mov gs,ax
00007C5D  31C0              xor ax,ax
00007C5F  8ED0              mov ss,ax
00007C61  BC007C            mov sp,0x7c00
00007C64  FB                sti
00007C65  BE6F7C            mov si,0x7c6f
00007C68  E81400            call 0x7c7f
00007C6B  FA                cli
00007C6C  F4                hlt
00007C6D  EBFC              jmp short 0x7c6b
00007C6F  48                dec ax
00007C70  656C              gs insb
00007C72  6C                insb
00007C73  6F                outsw

It appears that MKFS has been used to do something with the BIOS parameter block (BPB) or is involved with filling in the BPB and whatever you have done has created code that has an initial jump into the middle of an instruction at 0x7c52 and then another JMP back into the middle of the BPB. Although that second JMP won't be treated as a jump since the first jmp jumps into the middle of the second jump and looks like the code that would be executed by the processor might be something invalid like `lock mov ax, cs`

I see why I can't recreate the problem. I modified your createImage.sh file yesterday to avoid doing any of the stuff related to moving things around in the BPB. Here in particular:

Code: Select all

dd if=/dev/zero of=./bin/img/partition.dd bs=512 count=65536 # count = [ K = megabyte; K*(1024)^2/512 ]
mkfs.vfat -F 16 -n "BonsOS" ./bin/img/partition.dd

#Add file to the partition
#mcopy -i ./bin/img/partition.dd ./bin/boot/loader.bin ::/
#mcopy -i ./bin/img/partition.dd ./bin/kernel/kernel.sys ::/

#Add the bootloader to the partition
dd if=bin/boot/boot.bin of=./bin/img/partition.dd seek=0 count=1 conv=notrunc bs=3
dd if=bin/boot/boot.bin of=./bin/img/partition.dd seek=83 seek=83 skip=83 count=$[512-83] conv=notrunc bs=1

The final two DD commands have created a munged/incorrect version of your code.

As a test - if you were to write boot.bin directly to the start of partition.dd, the VBR (without modification) should work.

[Bonfra]

As a test - if you were to write boot.bin directly to the start of partition.dd, the VBR (without modification) should work.

Yes, it works perfectly. I've worked a bit and I came up with this solution:

Following this wiki I created a fat12_bpb.inc that contains only some labels and memory reserve to then use it in the program. This is the code:

Code: Select all

times 0x0B-($-$$) db 0  ; The BPB starts at 0x0B

; DOS 2.0 BPB
bpb_BytesPerSector:     resw 1
bpb_SectorsPerCluster:  resb 1
bpb_ReservedSectors:    resw 1
bpb_NumberOfFATs:       resb 1
bpb_RootEntries:        resw 1
bpb_TotalSectors:       resw 1
bpb_Media:              resb 1
bpb_SectorsPerFAT:      resw 1

; DOS 3.31 BPB
bpb_SectorsPerTrack:    resw 1
bpb_HeadsPerCylinder:   resw 1
bpb_HiddenSectors:      resd 1
bpb_TotalSectorsBig:    resd 1

; Extended BPB
bpb_DriveNumber:        resb 1
bpb_Unused:             resb 1
bpb_ExtBootSignature:   resb 1
bpb_SerialNumber:       resd 1
bpb_VolumeLabel:        resb 11
bpb_FileSystem:         resb 8

Which I can include at the start of my boot.bin and access the bpb data. Knowing that the fat12 bpb ends at 0x3E I can adjust the script that copy the bootloader as so:

Code: Select all

dd if=bin/boot/boot.bin of=./bin/img/partition.dd seek=62 skip=62 count=$[512-62] conv=notrunc bs=1

If I'll need to change the filesystem I just neet to create another *_bpb.inc file and calculate his size to adjust the dd copy.
I think this is the final solution.

[bzt]

If I'll need to change the filesystem I just neet to create another *_bpb.inc file and calculate his size to adjust the dd copy.
I think this is the final solution.

Or you could use a common struct for all. For example I place the code always at 0x5A, and I use this BPB (works for FAT12/16/32 too)

Code: Select all

    org 0
    bpb.jmp             db 3 dup 0
    bpb.oem             db 8 dup 0
    bpb.bps             dw 0
    bpb.spc             db 0
    bpb.rsc             dw 0
    bpb.nf              db 0 ;16
    bpb.nr              dw 0
    bpb.ts16            dw 0
    bpb.media           db 0
    bpb.spf16           dw 0 ;22
    bpb.spt             dw 0
    bpb.nh              dw 0
    bpb.hs              dd 0
    bpb.ts32            dd 0
    bpb.spf32           dd 0 ;36
    bpb.flg             dd 0
    bpb.rc              dd 0 ;44
    bpb.vol             db 6 dup 0
    bpb.fst             db 8 dup 0 ;54
    bpb.dmy             db 20 dup 0
    bpb.fst2            db 8 dup 0 ;82

start:  ; code at 0x5A (90)

To distinguish between FAT types, check if bpb.spf16 is zero. If not, then it's a FAT12/16, otherwise FAT32 (and use bpb.spf32). For FAT12/16, check if you have more than 4086 clusters, if so then it's a FAT16. If you always start your code at 0x5A, then it won't interfere with any of the BPBs, no matter the FAT type.

Cheers,
bzt

[zaval]

check if you have more than 4086 clusters, if so then it's a FAT16. If you always start your code at 0x5A, then it won't interfere with any of the BPBs, no matter the FAT type.

for FAT12, count of clusters must be less than 4085 clusters (to the author - see the FAT spec at pages 14-15, there is a pseudocode of how to calculate the count of clusters right and the FAT type determination algorithm).

[bzt]

for FAT12, count of clusters must be less than 4085 clusters (to the author - see the FAT spec at pages 14-15, there is a pseudocode of how to calculate the count of clusters right and the FAT type determination algorithm).

Yeah, or whatever. The whole let's count the clusters is a seriously flawed and particularly ugly workaround just because M$ forget to put a FATEntrySize or a version field in the BPB and they were stupid enough to mark the magic bytes "optional" (the spec says FilSysType does not determine the file system type, WTF? What is it for then? This is plain stupid and dumb.)

For example it's perfectly valid to create a FAT32 file system with 4 sectors per clusters on a 64 Mb partition, and most utilities will handle that just fine (because they check the BPB_FATSz16 field being zero as told on page 9 and 11), while using the number of clusters method (as suggested on page 14-15) it would falsely identify it as a FAT16. Epic fail!

Cheers,
bzt

[zaval]

bzt, some people over here had gotten a (wrong) impression, that my arguments with you have something to do with phallometry, whereas it's not the case, for me at least, - e.g. the only purpose of my remark here was to point to your off by one mistake, for the purpose of avoiding mistakes (the spec emphasizes on such a lot, and explains why the detection way is how it is and why it's not through the "magic" fields and why counts of clusters are like this - because of a million "off by X" mistakes done through the years by haxx0rs; it's ironical, how "dumb" ones have seen and overcame/fixed/mitigated tons of mistakes that the "smartest" ones had commited, isn't it? the spec says clearly, the field isn't used for identification because a lot of 3d party software doesn't set it (correct)) and to point Bonfra to the ultimate source of information for getting his FAT related code the proper way. if I found myself wanting to know your highly valuable opinion on the intellectual capabilities of software engineers of big software corporations, that would mean, this time I forgot to take my pills. fortunately, I didn't.

[nexos]

bzt, whether or not it make sense is irrelevant. It's the way things are, so we have to learn to live with it. And don't start a fight again!

[bzt]

bzt, whether or not it make sense is irrelevant. It's the way things are, so we have to learn to live with it. And don't start a fight again!

There was no fight at all. Zaval was more polite than usual

I've just pointed out a serious problem with the FAT specification. In order to correctly detect FAT size, you shouldn't follow the spec page 14-15, that's how things actually are. Just for the records, TianoCore does not use that method either. So if you want to create a FAT image which is compatible with UEFI, you must forget about the FAT spec page 14-15 method. Go on, give it a try: create a 64Mb FAT16 partition with 4 sectors per cluster, TianoCore will incorrectly identify it, and you won't have an FS0:. Ironically the FAT spec method would yield a good result in this case, but TianoCore does not follow the spec and uses its own heuristics. This is a constant problem with FAT which could have been avoided shouldn't the magic bytes in FilSysType be "optional".

So beware when you implement your own FAT driver and/or image creator!

Cheers,
bzt

[Octocontrabass]

Just for the records, TianoCore does not use that method either.

It looks like it does to me. At this point, it already knows whether to expect a FAT32 volume based on the "sectors per FAT" field.

Go on, give it a try: create a 64Mb FAT16 partition with 4 sectors per cluster, TianoCore will incorrectly identify it, and you won't have an FS0:.

Do you have a disk image that demonstrates this problem?

[nexos]

I found that to be a bit strange in the FAT spec myself. I want to create my own filesystem, as I can't get a long with FAT or Ext2 .

[nullplan]

I've just pointed out a serious problem with the FAT specification. In order to correctly detect FAT size, you shouldn't follow the spec page 14-15, that's how things actually are. Just for the records, TianoCore does not use that method either. So if you want to create a FAT image which is compatible with UEFI, you must forget about the FAT spec page 14-15 method. Go on, give it a try: create a 64Mb FAT16 partition with 4 sectors per cluster, TianoCore will incorrectly identify it, and you won't have an FS0:. Ironically the FAT spec method would yield a good result in this case, but TianoCore does not follow the spec and uses its own heuristics. This is a constant problem with FAT which could have been avoided shouldn't the magic bytes in FilSysType be "optional".

Your argument is invalid, since TianoCore is not an authoritative source on the design of the FAT file system. Microsoft is. If TianoCore cannot understand a 64MB FAT16 partition with 4 sectors per cluster, then this is a bug in TianoCore and should be reported. My god, it wouldn't be the first time we found a bug in firmware, and it likely won't be the last.

[bzt]

At this point, it already knows whether to expect a FAT32 volume based on the "sectors per FAT" field.

Yes, exactly my point, it does not use the number of clusters as told by the spec. Instead of seeing if there are more than 0xFFF5 clusters, it uses the other method I've suggested, checking BPB_FATSz16 field being zero to determine if it's a FAT32 (line 234). You've noticed that correctly.

I found that to be a bit strange in the FAT spec myself. I want to create my own filesystem, as I can't get a long with FAT or Ext2 .

Sadly you can't get away without FAT if your OS boots from UEFI as the ESP needs it. You can use an existing tool to create the boot partition in development, but sooner or later you'll need to mount it under your OS (meaning you'll need a FAT driver in your kernel). I would be happy too if we could avoid FAT entirely.

Your argument is invalid, since TianoCore is not an authoritative source on the design of the FAT file system.

No, the argument isn't invalid at all. Nobody cares if TianoCore is authoritative or not, or if it follows the spec or not, you _MUST_ comply with the UEFI firmware if you want to boot your OS. And there are many other software you must take into consideration for compatibility. Your OS must create a FAT file system that's readable under Win or Linux too. Most of them don't use the spec either.

Cheers,
bzt

[Octocontrabass]

Yes, exactly my point, it does not use the number of clusters as told by the spec. Instead of seeing if there are more than 0xFFF5 clusters, it uses the other method I've suggested, checking BPB_FATSz16 field being zero to determine if it's a FAT32 (line 234). You've noticed that correctly.

But they do check the number of clusters. What I'm trying to say is that the logic is still present, but it's been rearranged.

Here is some pseudocode to determine the type of a FAT filesystem and then check if BPB_FATSz16 is valid:

Code: Select all

if( clusters < 4085 )
{
    if( BPB_FATSz16 != 0 )
    {
        return FAT12;
    } else {
        return invalid;
    }
}
else if( clusters < 65525 )
{
    if( BPB_FATSz16 != 0 )
    {
        return FAT16;
    } else {
        return invalid;
    }
}
else
{
    if( BPB_FATSz16 != 0 )
    {
        return invalid;
    } else {
        return FAT32;
    }
}

Here's pseudocode for the logic in Tianocore, which performs the same two tests but in a different order:

Code: Select all

if( BPB_FATSz16 != 0 )
{
    if( clusters < 4085 )
    {
        return FAT12;
    }
    else if( clusters < 65525 )
    {
        return FAT16;
    }
    else
    {
        return invalid;
    }
}
else
{
    if( clusters < 4085 )
    {
        return invalid;
    }
    else if( clusters < 65525 )
    {
        return invalid;
    }
    else
    {
        return FAT32;
    }
}

The results are the same both ways.

[bzt]

Here is some pseudocode to determine the type of a FAT filesystem

Your first pseudocode is not the same as the spec. Take a closer look: in the spec on pages 14-15, BPB_FATSz16 being zero or not does not influence the FAT type. (The criteria that BPB_FATSz16 must be zero for FAT32 only appears on pages 9 and 11, but not in the "Determining FAT Type" section).

Here's pseudocode for the logic in Tianocore, which performs the same two tests but in a different order:

I don't know where you get that pseudocode either. This is clearly not how TianoCore logic works. Because that looks like

Code: Select all

  SectorsPerFat = FatBs.FatBsb.SectorsPerFat;
  if (SectorsPerFat == 0) {
    SectorsPerFat = FatBs.FatBse.Fat32Bse.LargeSectorsPerFat;
    FatType       = Fat32;
  }

so
a) we can see there's absolutely no number of cluster check before setting "FatType = Fat32", just a "BPB_FATSz16 == 0" check.
b) in your second pseudocode, you have the "if( clusters < 4085 )" check in both branches, however there's no such thing in TianoCore. The "FAT_MAX_FAT12_CLUSTER" define appears just one time, in one branch only, for the "if (FatType != Fat32) {" condition in a trinary operator, that's it (line 327).

Just get over it, FAT specification s*cks. You must be creative and use a heuristic to detect FAT types correctly, specially because not all tools use the hardwired sector per cluster table, and even the spec mentions that defining number of sectors per cluster differently is allowed. There's nothing in the on-disk format that would prevent you from creating a 64Mb FAT32 image with 64 sectors per clusters, and if that can be done, then you can bet some tools will create it (and the Linux kernel for example would parse that happily without probs).

Cheers,
bzt

[Octocontrabass]

Take a closer look: in the spec on pages 14-15, BPB_FATSz16 being zero or not does not influence the FAT type.

Correct: it only influences whether the filesystem is valid. It must be nonzero on all valid FAT12 or FAT16 filesystems, and it must be zero on all valid FAT32 filesystems.

I don't know where you get that pseudocode either. This is clearly not how TianoCore logic works.

You're missing the rest of the code, which appears later:

Code: Select all

  if (FatType != Fat32) {
    if (Volume->MaxCluster >= FAT_MAX_FAT16_CLUSTER) {
      return EFI_VOLUME_CORRUPTED;
    }

    FatType = Volume->MaxCluster < FAT_MAX_FAT12_CLUSTER ? Fat12 : Fat16;
    /* irrelevant code removed */
  } else {
    if (Volume->MaxCluster < FAT_MAX_FAT16_CLUSTER) {
      return EFI_VOLUME_CORRUPTED;
    }
    /* irrelevant code removed */
  }

a) we can see there's absolutely no number of cluster check before setting "FatType = Fat32", just a "BPB_FATSz16 == 0" check.

It doesn't need to be. Tianocore will refuse to read or write an invalid filesystem, so it doesn't matter if Tianocore incorrectly detects an invalid FAT16 filesystem as FAT32 (before rejecting it as invalid).

b) in your second pseudocode, you have the "if( clusters < 4085 )" check in both branches, however there's no such thing in TianoCore. The "FAT_MAX_FAT12_CLUSTER" define appears just one time, in one branch only, for the "if (FatType != Fat32) {" condition in a trinary operator, that's it (line 327).

The Tianocore developers found an optimization: 4085 is less than 65525, so you only need to check if there are less than 65525 clusters to know that it's not a valid FAT32 filesystem.

There's nothing in the on-disk format that would prevent you from creating a 64Mb FAT32 image with 64 sectors per clusters.

Sure, but it wouldn't have enough clusters to be valid.