From: Nikunj A Dadhania <hidden> Date: 2015-06-11 10:19:58
For a GPT+LVM combination disk, older bootloader that does not support
LVM, cannot load kernel from LVM.
The patch add support to read from BASIC_DATA UUID
partition. Installer has installed CHRP-BOOT config on a FAT file
system.
Signed-off-by: Nikunj A Dadhania <redacted>
---
slof/fs/packages/disk-label.fs | 72 ++++++++++++++++++++++++++++++++++++++++--
1 file changed, 70 insertions(+), 2 deletions(-)
@@ -266,7 +266,7 @@ CONSTANT /gpt-part-entry:try-dos-partition( -- okay? )\ Read partition table and check magic.-no-mbr?IFcr."No DOS disk-label found."crfalseEXITTHEN+no-mbr?IFfalseEXITTHENcount-dos-logical-partitionsTOdos-logical-partitions
@@ -408,6 +408,73 @@ AA26 CONSTANT GPT-PREP-PARTITION-4FALSE;+\ Check for GPT MSFT BASIC DATA GUID - vfat based+EBD0A0A2CONSTANTGPT-BASIC-DATA-PARTITION-1+B9E5CONSTANTGPT-BASIC-DATA-PARTITION-2+4433CONSTANTGPT-BASIC-DATA-PARTITION-3+87C0CONSTANTGPT-BASIC-DATA-PARTITION-4+68B6B72699C7CONSTANTGPT-BASIC-DATA-PARTITION-5++:gpt-basic-data-partition?( -- true|false )+blockgpt-part-entry>part-type-guidl@-leGPT-BASIC-DATA-PARTITION-1=IF+blockgpt-part-entry>part-type-guid4+w@-le+GPT-BASIC-DATA-PARTITION-2=IF+blockgpt-part-entry>part-type-guid6+w@-le+GPT-BASIC-DATA-PARTITION-3=IF+blockgpt-part-entry>part-type-guid8+w@+GPT-BASIC-DATA-PARTITION-4=IF+blockgpt-part-entry>part-type-guida+w@+blockgpt-part-entry>part-type-guidc+l@swaplxjoin+GPT-BASIC-DATA-PARTITION-5=IF+TRUEEXIT+THEN+THEN+THEN+THEN+THEN+FALSE+;++:try-gpt-vfat-partition( -- okay? )+\ Read partition table and check magic.+no-gpt?IFcr."No GPT disk-label found."crfalseEXITTHEN+1read-sectorblockgpt>part-entry-lbal@-le+block-size*toseek-pos+blockgpt>part-entry-sizel@-letogpt-part-size+blockgpt>num-part-entryl@-ledup0=IFFALSEEXITTHEN+1+1?DO+seek-pos0seekdrop+blockgpt-part-sizereaddrop+gpt-basic-data-partition?IF+debug-disk-label?IF+."GPT LINUX DATA partition found "cr+THEN+blockgpt-part-entry>first-lbax@xbflip+duptopart-start+blockgpt-part-entry>last-lbax@xbflip+over-1+( addr offset len )+dupblock-size*topart-size+swap( addr len offset )+block-size*topart-offset+00seek+blockblock-sizeread+3drop+\ block 0 byte 0-2 is a jump instruction in all FAT+\ filesystems.+\ e9 and eb are jump instructions in x86 assembler.+blockc@e9<>IF+blockc@eb<>+block2+c@90<>or+IFfalseEXITTHEN+THEN+TRUE+UNLOOPEXIT+THEN+seek-posgpt-part-sizei*+toseek-pos+LOOP+FALSE+;+\ Extract the boot loader path from a bootinfo.txt file\ In: address and length of buffer where the bootinfo.txt has been loaded to.\ Out: string address and length of the boot loader (within the input buffer)
From: Thomas Huth <hidden> Date: 2015-06-17 10:23:00
On Thu, 11 Jun 2015 15:48:49 +0530
Nikunj A Dadhania [off-list ref] wrote:
quoted hunk
For a GPT+LVM combination disk, older bootloader that does not support
LVM, cannot load kernel from LVM.
The patch add support to read from BASIC_DATA UUID
partition. Installer has installed CHRP-BOOT config on a FAT file
system.
Signed-off-by: Nikunj A Dadhania <redacted>
---
slof/fs/packages/disk-label.fs | 72 ++++++++++++++++++++++++++++++++++++++++--
1 file changed, 70 insertions(+), 2 deletions(-)
Don't you have to byte-swap (w@-le) here, too? Looks somehow strange
that the other UID parts are read byte-swapped but this one is not?
+ GPT-BASIC-DATA-PARTITION-4 = IF
+ block gpt-part-entry>part-type-guid a + w@
+ block gpt-part-entry>part-type-guid c + l@ swap lxjoin
dito ... here again no byteswap?
+ GPT-BASIC-DATA-PARTITION-5 = IF
+ TRUE EXIT
+ THEN
+ THEN
+ THEN
+ THEN
+ THEN
+ FALSE
+;
I'm not a fan of deep nesting, so I'd maybe write this function rather
like this instead:
: gpt-basic-data-partition? ( -- true|false )
block gpt-part-entry>part-type-guid
dup l@-le GPT-BASIC-DATA-PARTITION-1 <> IF FALSE EXIT THEN
dup 4 + w@-le GPT-BASIC-DATA-PARTITION-2 <> IF FALSE EXIT THEN
dup 6 + w@-le GPT-BASIC-DATA-PARTITION-3 <> IF FALSE EXIT THEN
...
TRUE
;
... but that's just cosmetics.
+: try-gpt-vfat-partition ( -- okay? )
+ \ Read partition table and check magic.
+ no-gpt? IF cr ." No GPT disk-label found." cr false EXIT THEN
Not directly related to this patch, but "no-gpt?" seems somewhat
imprecise to me ... what if the whole MBR is accidentially filled
with EE for example? That certainly does not indicate a valid GPT,
does it?
I might be wrong, but as far as I can say that function should also
check for the aa55 magic at the end of the MBR, shouldn't it?
Also I wonder whether you should check gpt>signature somewhere, either
in "no-gpt?" or here before touching gpt>part-entry-lba entry below?
I think that would contribute to the robustness of the code.
+ 1 read-sector block gpt>part-entry-lba l@-le
gpt>part-entry-lba seems to be an 8-bytes field ... so shouldn't you
access it with "x@ xbflip" instead?
By the way, it might be a good idea to add a "x@-le" helper for this to
little-endian.fs.
+ block-size * to seek-pos
+ block gpt>part-entry-size l@-le to gpt-part-size
+ block gpt>num-part-entry l@-le dup 0= IF FALSE EXIT THEN
+ 1+ 1 ?DO
+ seek-pos 0 seek drop
+ block gpt-part-size read drop
Can you be sure that gpt-part-size is only smaller than 4096 bytes
here? If not, you might overflow the block array, don't you?
+ gpt-basic-data-partition? IF
+ debug-disk-label? IF
+ ." GPT LINUX DATA partition found " cr
+ THEN
+ block gpt-part-entry>first-lba x@ xbflip
+ dup to part-start
+ block gpt-part-entry>last-lba x@ xbflip
+ over - 1+ ( addr offset len )
+ dup block-size * to part-size
+ swap ( addr len offset )
+ block-size * to part-offset
+ 0 0 seek
+ block block-size read
+ 3drop
+ \ block 0 byte 0-2 is a jump instruction in all FAT
+ \ filesystems.
+ \ e9 and eb are jump instructions in x86 assembler.
+ block c@ e9 <> IF
+ block c@ eb <>
+ block 2+ c@ 90 <> or
+ IF false EXIT THEN
+ THEN
That's a copy of the code in try-dos-files ... could you please factor
out that stuff into a separate function to avoid duplicate code? Thanks!
(especially you also lack a UNLOOP before above EXIT otherwise, I think)
quoted hunk
+ TRUE
+ UNLOOP EXIT
+ THEN
+ seek-pos gpt-part-size i * + to seek-pos
+ LOOP
+ FALSE
+;
+
\ Extract the boot loader path from a bootinfo.txt file
\ In: address and length of buffer where the bootinfo.txt has been loaded to.
\ Out: string address and length of the boot loader (within the input buffer)
@@ -493,7 +560,7 @@ AA26 CONSTANT GPT-PREP-PARTITION-4 debug-disk-label? IF ." Trying CHRP boot " .s cr THEN 1 disk-chrp-boot !- dup load-chrp-boot-file ?dup 0 <> IF .s cr nip EXIT THEN+ dup load-chrp-boot-file ?dup 0 <> IF nip EXIT THEN 0 disk-chrp-boot ! debug-disk-label? IF ." Trying GPT boot " .s cr THEN
@@ -600,6 +667,7 @@ AA26 CONSTANT GPT-PREP-PARTITION-4 : try-partitions ( -- found? ) try-dos-partition IF try-files EXIT THEN+ try-gpt-vfat-partition IF try-files EXIT THEN \ try-iso9660-partition IF try-files EXIT THEN \ ... more partition types here... false
From: Nikunj A Dadhania <hidden> Date: 2015-06-17 11:59:46
Thomas Huth [off-list ref] writes:
On Thu, 11 Jun 2015 15:48:49 +0530
Nikunj A Dadhania [off-list ref] wrote:
quoted
For a GPT+LVM combination disk, older bootloader that does not support
LVM, cannot load kernel from LVM.
The patch add support to read from BASIC_DATA UUID
partition. Installer has installed CHRP-BOOT config on a FAT file
system.
Signed-off-by: Nikunj A Dadhania <redacted>
---
slof/fs/packages/disk-label.fs | 72 ++++++++++++++++++++++++++++++++++++++++--
1 file changed, 70 insertions(+), 2 deletions(-)
Don't you have to byte-swap (w@-le) here, too? Looks somehow strange
that the other UID parts are read byte-swapped but this one is not?
Interesting observation, I had used code from gpt-prep-partition? and
did not doubt the validity of it. But that is how I see it in the memory
though.
4 > 7e50d000 10 dump
7e50d000: a2 a0 d0 eb e5 b9 33 44 87 c0 68 b6 b7 26 99 c7 ......3D..h..&.. ok
4 >
quoted
+ GPT-BASIC-DATA-PARTITION-4 = IF
+ block gpt-part-entry>part-type-guid a + w@
+ block gpt-part-entry>part-type-guid c + l@ swap lxjoin
dito ... here again no byteswap?
quoted
+ GPT-BASIC-DATA-PARTITION-5 = IF
+ TRUE EXIT
+ THEN
+ THEN
+ THEN
+ THEN
+ THEN
+ FALSE
+;
I'm not a fan of deep nesting, so I'd maybe write this function rather
like this instead:
Sure, looks much simpler.
: gpt-basic-data-partition? ( -- true|false )
block gpt-part-entry>part-type-guid
dup l@-le GPT-BASIC-DATA-PARTITION-1 <> IF FALSE EXIT THEN
dup 4 + w@-le GPT-BASIC-DATA-PARTITION-2 <> IF FALSE EXIT THEN
dup 6 + w@-le GPT-BASIC-DATA-PARTITION-3 <> IF FALSE EXIT THEN
...
TRUE
;
... but that's just cosmetics.
quoted
+: try-gpt-vfat-partition ( -- okay? )
+ \ Read partition table and check magic.
+ no-gpt? IF cr ." No GPT disk-label found." cr false EXIT THEN
Not directly related to this patch, but "no-gpt?" seems somewhat
imprecise to me ... what if the whole MBR is accidentially filled
with EE for example? That certainly does not indicate a valid GPT,
does it?
I might be wrong, but as far as I can say that function should also
check for the aa55 magic at the end of the MBR, shouldn't it?
Also I wonder whether you should check gpt>signature somewhere, either
in "no-gpt?" or here before touching gpt>part-entry-lba entry below?
I think that would contribute to the robustness of the code.
Sure, we can make this changes.
quoted
+ 1 read-sector block gpt>part-entry-lba l@-le
gpt>part-entry-lba seems to be an 8-bytes field ... so shouldn't you
access it with "x@ xbflip" instead?
I will verfy this and make appropriate changes.
By the way, it might be a good idea to add a "x@-le" helper for this to
little-endian.fs.
quoted
+ block-size * to seek-pos
+ block gpt>part-entry-size l@-le to gpt-part-size
+ block gpt>num-part-entry l@-le dup 0= IF FALSE EXIT THEN
+ 1+ 1 ?DO
+ seek-pos 0 seek drop
+ block gpt-part-size read drop
Can you be sure that gpt-part-size is only smaller than 4096 bytes
here? If not, you might overflow the block array, don't you?
True.
quoted
+ gpt-basic-data-partition? IF
+ debug-disk-label? IF
+ ." GPT LINUX DATA partition found " cr
+ THEN
+ block gpt-part-entry>first-lba x@ xbflip
+ dup to part-start
+ block gpt-part-entry>last-lba x@ xbflip
+ over - 1+ ( addr offset len )
+ dup block-size * to part-size
+ swap ( addr len offset )
+ block-size * to part-offset
+ 0 0 seek
+ block block-size read
+ 3drop
+ \ block 0 byte 0-2 is a jump instruction in all FAT
+ \ filesystems.
+ \ e9 and eb are jump instructions in x86 assembler.
+ block c@ e9 <> IF
+ block c@ eb <>
+ block 2+ c@ 90 <> or
+ IF false EXIT THEN
+ THEN
That's a copy of the code in try-dos-files ... could you please factor
out that stuff into a separate function to avoid duplicate code? Thanks!
(especially you also lack a UNLOOP before above EXIT otherwise, I
think)
Yes.
quoted
+ TRUE
+ UNLOOP EXIT
+ THEN
+ seek-pos gpt-part-size i * + to seek-pos
+ LOOP
+ FALSE
+;
+
\ Extract the boot loader path from a bootinfo.txt file
\ In: address and length of buffer where the bootinfo.txt has been loaded to.
\ Out: string address and length of the boot loader (within the input buffer)
@@ -493,7 +560,7 @@ AA26 CONSTANT GPT-PREP-PARTITION-4 debug-disk-label? IF ." Trying CHRP boot " .s cr THEN 1 disk-chrp-boot !- dup load-chrp-boot-file ?dup 0 <> IF .s cr nip EXIT THEN+ dup load-chrp-boot-file ?dup 0 <> IF nip EXIT THEN 0 disk-chrp-boot ! debug-disk-label? IF ." Trying GPT boot " .s cr THEN
@@ -600,6 +667,7 @@ AA26 CONSTANT GPT-PREP-PARTITION-4 : try-partitions ( -- found? ) try-dos-partition IF try-files EXIT THEN+ try-gpt-vfat-partition IF try-files EXIT THEN \ try-iso9660-partition IF try-files EXIT THEN \ ... more partition types here... false
Don't you have to byte-swap (w@-le) here, too? Looks somehow strange
that the other UID parts are read byte-swapped but this one is not?
Interesting observation, I had used code from gpt-prep-partition? and
did not doubt the validity of it. But that is how I see it in the memory
though.
4 > 7e50d000 10 dump=20
7e50d000: a2 a0 d0 eb e5 b9 33 44 87 c0 68 b6 b7 26 99 c7 ......3D..h..&=
.. ok
4 >
And here the answer for that:
https://en.wikipedia.org/wiki/GUID_Partition_Table#cite_note-26
=20=20=20=20
The GUIDs in this table are written assuming a little-endian byte
order. For example, the GUID for an EFI System partition is written
as C12A7328-F81F-11D2-BA4B-00A0C93EC93B here, which corresponds to
the 16 byte sequence 28h 73h 2Ah C1h 1Fh F8h D2h 11h BAh 4Bh 00h A0h
C9h 3Eh C9h 3Bh =E2=80=93 only the first three blocks are byte-swapped.
"only the first three blocks are byte-swapped"
Regards,
Nikunj
Don't you have to byte-swap (w@-le) here, too? Looks somehow strange
that the other UID parts are read byte-swapped but this one is not?
Interesting observation, I had used code from gpt-prep-partition? and
did not doubt the validity of it. But that is how I see it in the memory
though.
4 > 7e50d000 10 dump=20
7e50d000: a2 a0 d0 eb e5 b9 33 44 87 c0 68 b6 b7 26 99 c7 ......3D..h.=
.&.. ok
quoted
4 >
=20
And here the answer for that:
=20
https://en.wikipedia.org/wiki/GUID_Partition_Table#cite_note-26
=20
The GUIDs in this table are written assuming a little-endian byte
order. For example, the GUID for an EFI System partition is written
as C12A7328-F81F-11D2-BA4B-00A0C93EC93B here, which corresponds to
the 16 byte sequence 28h 73h 2Ah C1h 1Fh F8h D2h 11h BAh 4Bh 00h A0h
C9h 3Eh C9h 3Bh =E2=80=93 only the first three blocks are byte-swappe=
d.
=20
"only the first three blocks are byte-swapped"
Ok, this seems to be a GUID specialty (as opposed to UUIDs), also see:
https://en.wikipedia.org/wiki/Globally_unique_identifier#Binary_encoding
... but that means that the last 8 bytes are always "big endian", so I
think you could simplify your code here and check the last 8 bytes at
once instead of checking 2 + 6 bytes separately, can't you?
Thomas
Ok, so it should be fine when you have a valid GPT ... however, one
thing I learnt during my past years: Never ever blindly trust data that
you get from an external medium ... it could have been corrupted by
accident or on purpose. So if you want to make your code more robust,
maybe better check gpt>part-entry-size for being in the range of d# 128
to d# 4096 or you might have to debug very strange crashes one day...
Thomas
=20
And here the answer for that:
=20
https://en.wikipedia.org/wiki/GUID_Partition_Table#cite_note-26
=20=20=20=20=20
The GUIDs in this table are written assuming a little-endian byte
order. For example, the GUID for an EFI System partition is written
as C12A7328-F81F-11D2-BA4B-00A0C93EC93B here, which corresponds to
the 16 byte sequence 28h 73h 2Ah C1h 1Fh F8h D2h 11h BAh 4Bh 00h A0h
C9h 3Eh C9h 3Bh =E2=80=93 only the first three blocks are byte-swapp=
ed.
quoted
=20
"only the first three blocks are byte-swapped"
Ok, this seems to be a GUID specialty (as opposed to UUIDs), also see:
https://en.wikipedia.org/wiki/Globally_unique_identifier#Binary_encoding
... but that means that the last 8 bytes are always "big endian", so I
think you could simplify your code here and check the last 8 bytes at
once instead of checking 2 + 6 bytes separately, can't you?
Yes, I am making these changes, will be sending the series after
testing.
Regards
Nikunj