Page 6 of 7
Re: Making a bootable image
Posted: Tue Nov 03, 2020 3:23 pm
by 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.
Re: Making a bootable image
Posted: Tue Nov 03, 2020 4:05 pm
by Bonfra
MichaelPetch wrote: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.
Re: Making a bootable image
Posted: Wed Nov 04, 2020 1:06 pm
by bzt
Bonfra wrote: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
Re: Making a bootable image
Posted: Wed Nov 04, 2020 4:10 pm
by zaval
bzt wrote:
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).
Re: Making a bootable image
Posted: Thu Nov 05, 2020 5:53 am
by bzt
zaval wrote: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
Re: Making a bootable image
Posted: Thu Nov 05, 2020 3:53 pm
by 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.
Re: Making a bootable image
Posted: Thu Nov 05, 2020 4:48 pm
by 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!
Re: Making a bootable image
Posted: Fri Nov 06, 2020 1:07 pm
by bzt
nexos wrote: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
Re: Making a bootable image
Posted: Fri Nov 06, 2020 2:02 pm
by Octocontrabass
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.
bzt wrote: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?
Re: Making a bootable image
Posted: Fri Nov 06, 2020 4:22 pm
by 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
.
Re: Making a bootable image
Posted: Fri Nov 06, 2020 11:54 pm
by nullplan
bzt wrote: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.
Re: Making a bootable image
Posted: Sat Nov 07, 2020 5:50 am
by bzt
Octocontrabass wrote: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.
nexos wrote: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.
nullplan wrote: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
Re: Making a bootable image
Posted: Sat Nov 07, 2020 4:05 pm
by Octocontrabass
bzt wrote: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.
Re: Making a bootable image
Posted: Sat Nov 07, 2020 8:39 pm
by bzt
Octocontrabass wrote: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).
Octocontrabass wrote: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
Re: Making a bootable image
Posted: Sat Nov 07, 2020 9:24 pm
by Octocontrabass
bzt wrote: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.
bzt wrote: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 */
}
bzt wrote: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).
bzt wrote: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.
bzt wrote: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.