[PATCH SLOF v3 0/5] GPT fixes/cleanup and LVM support with FAT

[PATCH SLOF v3 0/5] GPT fixes/cleanup and LVM support with FAT

[Nikunj A Dadhania]

Following patchset implements some improvements and cleanup for the
GPT booting code:

patch 1: Simplify the gpt detection code with lesser scopes and add
	 comments.

patch 2: Introduce 8byte LE helpers: x@-le and x!-le

patch 3: Rename block / read-sector to indicate it a allocated buffer

patch 4: As we need to detect FAT partition, implement a helper that
	 can be used both from GPT code and "fat-bootblock?"

patch 5: Implement GPT FAT for LVM suport and make GPT detection code
	 robust

Changelog V2:
* Optimize gpt detection code
* Fixed stack comments
* Rename verify-gpt-partition, as it was doing more than that. Added
  comments.

Nikunj A Dadhania (5):
  disk-label: simplify gpt-prep-partition? routine
  introduce 8-byte LE helpers
  disk-label: rename confusing "block" word
  disk-label: introduce helper to check fat filesystem
  disk-label: add support for booting from GPT FAT partition

 slof/fs/little-endian.fs       |   6 ++
 slof/fs/packages/disk-label.fs | 211 ++++++++++++++++++++++++++---------------
 2 files changed, 138 insertions(+), 79 deletions(-)

-- 
2.4.3

[PATCH SLOF v3 1/5] disk-label: simplify gpt-prep-partition? routine

[Nikunj A Dadhania]

Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 slof/fs/packages/disk-label.fs | 41 +++++++++++++++--------------------------
 1 file changed, 15 insertions(+), 26 deletions(-)

diff --git a/slof/fs/packages/disk-label.fs b/slof/fs/packages/disk-label.fs
index fe1c25e..f1f083a 100644
--- a/slof/fs/packages/disk-label.fs
+++ b/slof/fs/packages/disk-label.fs
@@ -352,42 +352,31 @@ CONSTANT /gpt-part-entry
    drop 0
 ;
 
-\ Check for GPT PReP partition GUID
-9E1A2D38     CONSTANT GPT-PREP-PARTITION-1
-C612         CONSTANT GPT-PREP-PARTITION-2
-4316         CONSTANT GPT-PREP-PARTITION-3
-AA26         CONSTANT GPT-PREP-PARTITION-4
-8B49521E5A8B CONSTANT GPT-PREP-PARTITION-5
+\ Check for GPT PReP partition GUID. Only first 3 blocks are
+\ byte-swapped treating last two blocks as contigous for simplifying
+\ comparison
+9E1A2D38            CONSTANT GPT-PREP-PARTITION-1
+C612                CONSTANT GPT-PREP-PARTITION-2
+4316                CONSTANT GPT-PREP-PARTITION-3
+AA268B49521E5A8B    CONSTANT GPT-PREP-PARTITION-4
 
 : gpt-prep-partition? ( -- true|false )
-   block gpt-part-entry>part-type-guid l@-le GPT-PREP-PARTITION-1 = IF
-      block gpt-part-entry>part-type-guid 4 + w@-le
-      GPT-PREP-PARTITION-2 = IF
-         block gpt-part-entry>part-type-guid 6 + w@-le
-         GPT-PREP-PARTITION-3 = IF
-            block gpt-part-entry>part-type-guid 8 + w@
-            GPT-PREP-PARTITION-4 = IF
-               block gpt-part-entry>part-type-guid a + w@
-               block gpt-part-entry>part-type-guid c + l@ swap lxjoin
-               GPT-PREP-PARTITION-5 = IF
-                   TRUE EXIT
-               THEN
-            THEN
-         THEN
-      THEN
-   THEN
-   FALSE
+   block gpt-part-entry>part-type-guid
+   dup l@-le     GPT-PREP-PARTITION-1 <> IF drop false EXIT THEN
+   dup 4 + w@-le GPT-PREP-PARTITION-2 <> IF drop false EXIT THEN
+   dup 6 + w@-le GPT-PREP-PARTITION-3 <> IF drop false EXIT THEN
+       8 + x@    GPT-PREP-PARTITION-4 =
 ;
 
 : load-from-gpt-prep-partition ( addr -- size )
-   no-gpt? IF drop FALSE EXIT THEN
+   no-gpt? IF drop false EXIT THEN
    debug-disk-label? IF
       cr ." GPT partition found " cr
    THEN
    1 read-sector block gpt>part-entry-lba l@-le
    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
+   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 gpt-prep-partition? IF
@@ -405,7 +394,7 @@ AA26         CONSTANT GPT-PREP-PARTITION-4
       THEN
       seek-pos gpt-part-size i * + to seek-pos
    LOOP
-   FALSE
+   false
 ;
 
 \ Extract the boot loader path from a bootinfo.txt file
-- 
2.4.3

[PATCH SLOF v3 2/5] introduce 8-byte LE helpers

[Nikunj A Dadhania]

Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 slof/fs/little-endian.fs       | 6 ++++++
 slof/fs/packages/disk-label.fs | 4 ++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/slof/fs/little-endian.fs b/slof/fs/little-endian.fs
index f2e4e8d..6b4779e 100644
--- a/slof/fs/little-endian.fs
+++ b/slof/fs/little-endian.fs
@@ -17,6 +17,9 @@ here c@ ef = CONSTANT ?littleendian
 
 ?bigendian [IF]
 
+: x!-le >r xbflip r> x! ;
+: x@-le x@ xbflip ;
+
 : l!-le  >r lbflip r> l! ;
 : l@-le  l@ lbflip ;
 
@@ -47,6 +50,9 @@ here c@ ef = CONSTANT ?littleendian
 
 [ELSE]
 
+: x!-le x! ;
+: x@-le x@ ;
+
 : l!-le  l! ;
 : l@-le  l@ ;
 
diff --git a/slof/fs/packages/disk-label.fs b/slof/fs/packages/disk-label.fs
index f1f083a..b346774 100644
--- a/slof/fs/packages/disk-label.fs
+++ b/slof/fs/packages/disk-label.fs
@@ -383,8 +383,8 @@ AA268B49521E5A8B    CONSTANT GPT-PREP-PARTITION-4
          debug-disk-label? IF
             ." GPT PReP partition found " cr
          THEN
-         block gpt-part-entry>first-lba x@ xbflip
-         block gpt-part-entry>last-lba x@ xbflip
+         block gpt-part-entry>first-lba x@-le
+         block gpt-part-entry>last-lba x@-le
          over - 1+                 ( addr offset len )
          swap                      ( addr len offset )
          block-size * to part-offset
-- 
2.4.3

[PATCH SLOF v3 3/5] disk-label: rename confusing "block" word

[Nikunj A Dadhania]

"block" word is not a block number, actually its an allocated host
address.  Rename it to disk-buf along with a associated
size(disk-buf-size=4096) for using during allocation/free.

Also renaming the helper routine read-sector to read-disk-buf. This
routine assumes the address to be disk-buf and only takes sector number
as argument.

Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 slof/fs/packages/disk-label.fs | 78 ++++++++++++++++++++++--------------------
 1 file changed, 41 insertions(+), 37 deletions(-)

diff --git a/slof/fs/packages/disk-label.fs b/slof/fs/packages/disk-label.fs
index b346774..e5a0546 100644
--- a/slof/fs/packages/disk-label.fs
+++ b/slof/fs/packages/disk-label.fs
@@ -33,7 +33,8 @@ s" disk-label" device-name
 0 INSTANCE VALUE dos-logical-partitions
 
 0 INSTANCE VALUE block-size
-0 INSTANCE VALUE block
+0 INSTANCE VALUE disk-buf
+d# 4096    VALUE disk-buf-size
 
 0 INSTANCE VALUE args
 0 INSTANCE VALUE args-len
@@ -126,11 +127,11 @@ CONSTANT /gpt-part-entry
 ;
 
 
-\ read sector to array "block"
-: read-sector ( sector-number -- )
+\ read sector to array "disk-buf"
+: read-disk-buf ( sector-number -- )
    \ block-size is 0x200 on disks, 0x800 on cdrom drives
    block-size * 0 seek drop      \ seek to sector
-   block block-size read drop    \ read sector
+   disk-buf block-size read drop    \ read sector
 ;
 
 : (.part-entry) ( part-entry )
@@ -149,35 +150,35 @@ CONSTANT /gpt-part-entry
 
 : (.name) r@ begin cell - dup @ <colon> = UNTIL xt>name cr type space ;
 
-: init-block ( -- )
+: init-disk-buf ( -- )
    s" block-size" ['] $call-parent CATCH IF ABORT" parent has no block-size." THEN
    to block-size
-   d# 4096 alloc-mem
-   dup d# 4096 erase
-   to block
+   disk-buf-size alloc-mem
+   dup disk-buf-size erase
+   to disk-buf
    debug-disk-label? IF
-      ." init-block: block-size=" block-size .d ." block=0x" block u. cr
+      ." init-disk-buf: block-size=" block-size .d ." disk-buf=0x" disk-buf u. cr
    THEN
 ;
 
 : partition>part-entry ( partition -- part-entry )
-   1- /partition-entry * block mbr>partition-table +
+   1- /partition-entry * disk-buf mbr>partition-table +
 ;
 
 : partition>start-sector ( partition -- sector-offset )
    partition>part-entry part-entry>sector-offset l@-le
 ;
 
-\ This word returns true if the currently loaded block has _NO_ MBR magic
+\ This word returns true if the currently loaded disk-buf has _NO_ MBR magic
 : no-mbr? ( -- true|false )
-   0 read-sector
+   0 read-disk-buf
    1 partition>part-entry part-entry>id c@ ee = IF TRUE EXIT THEN \ GPT partition found
-   block mbr>magic w@-le aa55 <>
+   disk-buf mbr>magic w@-le aa55 <>
 ;
 
-\ This word returns true if the currently loaded block has _NO_ GPT partition id
+\ This word returns true if the currently loaded disk-buf has _NO_ GPT partition id
 : no-gpt? ( -- true|false )
-   0 read-sector
+   0 read-disk-buf
    1 partition>part-entry part-entry>id c@ ee <>
 ;
 
@@ -197,7 +198,7 @@ CONSTANT /gpt-part-entry
          part-entry>sector-offset l@-le    ( current sector )
          dup to part-start to lpart-start  ( current )
          BEGIN
-            part-start read-sector          \ read EBR
+            part-start read-disk-buf          \ read EBR
             1 partition>start-sector IF
                \ ." Logical Partition found at " part-start .d cr
                1+
@@ -240,7 +241,7 @@ CONSTANT /gpt-part-entry
             part-entry>sector-offset l@-le        ( log-part current sector )
             dup to part-start to lpart-start      ( log-part current )
             BEGIN
-               part-start read-sector             \ read EBR
+               part-start read-disk-buf             \ read EBR
                1 partition>start-sector IF        \ first partition entry
                   1+ 2dup = IF                    ( log-part current )
                      2drop
@@ -306,13 +307,13 @@ CONSTANT /gpt-part-entry
 : has-iso9660-filesystem  ( -- TRUE|FALSE )
    \ Seek to the beginning of logical 2048-byte sector 16
    \ refer to Chapter C.11.1 in PAPR 2.0 Spec
-   \ was: 10 read-sector, but this might cause trouble if you
+   \ was: 10 read-disk-buf, but this might cause trouble if you
    \ try booting an ISO image from a device with 512b sectors.
    10 800 * 0 seek drop      \ seek to sector
-   block 800 read drop       \ read sector
+   disk-buf 800 read drop       \ read sector
    \ Check for CD-ROM volume magic:
-   block c@ 1 =
-   block 1+ 5 s" CD001"  str=
+   disk-buf c@ 1 =
+   disk-buf 1+ 5 s" CD001"  str=
    and
    dup IF 800 to block-size THEN
 ;
@@ -361,7 +362,7 @@ C612                CONSTANT GPT-PREP-PARTITION-2
 AA268B49521E5A8B    CONSTANT GPT-PREP-PARTITION-4
 
 : gpt-prep-partition? ( -- true|false )
-   block gpt-part-entry>part-type-guid
+   disk-buf gpt-part-entry>part-type-guid
    dup l@-le     GPT-PREP-PARTITION-1 <> IF drop false EXIT THEN
    dup 4 + w@-le GPT-PREP-PARTITION-2 <> IF drop false EXIT THEN
    dup 6 + w@-le GPT-PREP-PARTITION-3 <> IF drop false EXIT THEN
@@ -373,18 +374,18 @@ AA268B49521E5A8B    CONSTANT GPT-PREP-PARTITION-4
    debug-disk-label? IF
       cr ." GPT partition found " cr
    THEN
-   1 read-sector block gpt>part-entry-lba l@-le
+   1 read-disk-buf disk-buf gpt>part-entry-lba l@-le
    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
+   disk-buf gpt>part-entry-size l@-le to gpt-part-size
+   disk-buf 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 gpt-prep-partition? IF
+      disk-buf gpt-part-size read drop gpt-prep-partition? IF
          debug-disk-label? IF
             ." GPT PReP partition found " cr
          THEN
-         block gpt-part-entry>first-lba x@-le
-         block gpt-part-entry>last-lba x@-le
+         disk-buf gpt-part-entry>first-lba x@-le
+         disk-buf gpt-part-entry>last-lba x@-le
          over - 1+                 ( addr offset len )
          swap                      ( addr len offset )
          block-size * to part-offset
@@ -547,12 +548,12 @@ AA268B49521E5A8B    CONSTANT GPT-PREP-PARTITION-4
 : try-dos-files ( -- found? )
    no-mbr? IF false EXIT THEN
 
-   \ block 0 byte 0-2 is a jump instruction in all FAT
+   \ disk-buf 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
+   disk-buf c@ e9 <> IF
+      disk-buf c@ eb <>
+      disk-buf 2+ c@ 90 <> or
       IF false EXIT THEN
    THEN
    s" fat-files" (interpose-filesystem)
@@ -560,8 +561,8 @@ AA268B49521E5A8B    CONSTANT GPT-PREP-PARTITION-4
 ;
 
 : try-ext2-files ( -- found? )
-   2 read-sector               \ read first superblock
-   block d# 56 + w@-le         \ fetch s_magic
+   2 read-disk-buf               \ read first superblock
+   disk-buf d# 56 + w@-le         \ fetch s_magic
    ef53 <> IF false EXIT THEN  \ s_magic found?
    s" ext2-files" (interpose-filesystem)
    true
@@ -598,13 +599,16 @@ AA268B49521E5A8B    CONSTANT GPT-PREP-PARTITION-4
 \ as defined by IEEE 1275-1994 3.8.1
 
 : close ( -- )
-   debug-disk-label? IF ." Closing disk-label: block=0x" block u. ." block-size=" block-size .d cr THEN
-   block d# 4096 free-mem
+   debug-disk-label? IF
+      ." Closing disk-label: disk-buf=0x" disk-buf u.
+      ." block-size=" block-size .d cr
+   THEN
+   disk-buf disk-buf-size free-mem
 ;
 
 
 : open ( -- true|false )
-   init-block
+   init-disk-buf
 
    parse-partition 0= IF
       close
-- 
2.4.3

[PATCH SLOF v3 4/5] disk-label: introduce helper to check fat filesystem

[Nikunj A Dadhania]

Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 slof/fs/packages/disk-label.fs | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/slof/fs/packages/disk-label.fs b/slof/fs/packages/disk-label.fs
index e5a0546..347dc5d 100644
--- a/slof/fs/packages/disk-label.fs
+++ b/slof/fs/packages/disk-label.fs
@@ -321,6 +321,14 @@ CONSTANT /gpt-part-entry
 
 \ Load from first active DOS boot partition.
 
+: fat-bootblock? ( addr -- flag )
+   \ byte 0-2 of the bootblock is a jump instruction in
+   \ all FAT filesystems.
+   \ e9 and eb are jump instructions in x86 assembler.
+   dup c@ e9 = IF drop true EXIT THEN
+   dup c@ eb = swap 2+ c@ 90 = and
+;
+
 \ NOTE: block-size is always 512 bytes for DOS partition tables.
 
 : load-from-dos-boot-partition ( addr -- size )
@@ -548,14 +556,7 @@ AA268B49521E5A8B    CONSTANT GPT-PREP-PARTITION-4
 : try-dos-files ( -- found? )
    no-mbr? IF false EXIT THEN
 
-   \ disk-buf 0 byte 0-2 is a jump instruction in all FAT
-   \ filesystems.
-   \ e9 and eb are jump instructions in x86 assembler.
-   disk-buf c@ e9 <> IF
-      disk-buf c@ eb <>
-      disk-buf 2+ c@ 90 <> or
-      IF false EXIT THEN
-   THEN
+   disk-buf fat-bootblock? 0= IF false EXIT THEN
    s" fat-files" (interpose-filesystem)
    true
 ;
-- 
2.4.3

[PATCH SLOF v3 5/5] disk-label: add support for booting from GPT FAT partition

[Nikunj A Dadhania]

For a GPT+LVM combination disk, older bootloader that does not support
LVM, cannot load kernel from LVM.

The patch adds support to read from BASIC_DATA UUID partitions for the
case that the OS installer has installed the CHRP-BOOT config on a FAT
file system.

Makes GPT detection robust
* Check for Protective MBR Magic
* Check for valid GPT Signature
* Boundary check for allocated block size before reading into the
  buffer

Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 slof/fs/packages/disk-label.fs | 99 +++++++++++++++++++++++++++++++++---------
 1 file changed, 79 insertions(+), 20 deletions(-)

diff --git a/slof/fs/packages/disk-label.fs b/slof/fs/packages/disk-label.fs
index 347dc5d..5267ddb 100644
--- a/slof/fs/packages/disk-label.fs
+++ b/slof/fs/packages/disk-label.fs
@@ -179,7 +179,8 @@ CONSTANT /gpt-part-entry
 \ This word returns true if the currently loaded disk-buf has _NO_ GPT partition id
 : no-gpt? ( -- true|false )
    0 read-disk-buf
-   1 partition>part-entry part-entry>id c@ ee <>
+   1 partition>part-entry part-entry>id c@ ee <> IF true EXIT THEN
+   disk-buf mbr>magic w@-le aa55 <>
 ;
 
 : pc-extended-partition? ( part-entry-addr -- true|false )
@@ -267,7 +268,10 @@ CONSTANT /gpt-part-entry
 
 : try-dos-partition ( -- okay? )
    \ Read partition table and check magic.
-   no-mbr? IF cr ." No DOS disk-label found." cr false EXIT THEN
+   no-mbr? IF
+       debug-disk-label? IF cr ." No DOS disk-label found." cr THEN
+       false EXIT
+   THEN
 
    count-dos-logical-partitions TO dos-logical-partitions
 
@@ -377,28 +381,82 @@ AA268B49521E5A8B    CONSTANT GPT-PREP-PARTITION-4
        8 + x@    GPT-PREP-PARTITION-4 =
 ;
 
-: load-from-gpt-prep-partition ( addr -- size )
-   no-gpt? IF drop false EXIT THEN
-   debug-disk-label? IF
-      cr ." GPT partition found " cr
-   THEN
-   1 read-disk-buf disk-buf gpt>part-entry-lba l@-le
+\ Check for GPT MSFT BASIC DATA GUID - fat based
+EBD0A0A2            CONSTANT GPT-BASIC-DATA-PARTITION-1
+B9E5                CONSTANT GPT-BASIC-DATA-PARTITION-2
+4433                CONSTANT GPT-BASIC-DATA-PARTITION-3
+87C068B6B72699C7    CONSTANT GPT-BASIC-DATA-PARTITION-4
+
+: gpt-basic-data-partition? ( -- true|false )
+   disk-buf gpt-part-entry>part-type-guid
+   dup l@-le     GPT-BASIC-DATA-PARTITION-1 <> IF drop false EXIT THEN
+   dup 4 + w@-le GPT-BASIC-DATA-PARTITION-2 <> IF drop false EXIT THEN
+   dup 6 + w@-le GPT-BASIC-DATA-PARTITION-3 <> IF drop false EXIT THEN
+       8 + x@    GPT-BASIC-DATA-PARTITION-4 =
+;
+
+\
+\ GPT Signature
+\ ("EFI PART", 45h 46h 49h 20h 50h 41h 52h 54h)
+\
+4546492050415254 CONSTANT GPT-SIGNATURE
+
+\ The routine checks whether the protective MBR has GPT ID and then
+\ reads the gpt data from the sector. Also set the seek position and
+\ the partition size used in caller routines.
+
+: get-gpt-partition ( -- true|false )
+   no-gpt? IF false EXIT THEN
+   debug-disk-label? IF cr ." GPT partition found " cr  THEN
+   1 read-disk-buf
+   disk-buf gpt>part-entry-lba x@-le
    block-size * to seek-pos
    disk-buf gpt>part-entry-size l@-le to gpt-part-size
-   disk-buf gpt>num-part-entry l@-le dup 0= IF false EXIT THEN
+   gpt-part-size disk-buf-size > IF
+      cr ." GPT part size exceeds buffer allocated " cr
+      false exit
+   THEN
+   disk-buf gpt>signature x@ GPT-SIGNATURE =
+;
+
+: load-from-gpt-prep-partition ( addr -- size )
+   get-gpt-partition 0= IF false EXIT THEN
+   disk-buf gpt>num-part-entry l@-le dup 0= IF false exit THEN
    1+ 1 ?DO
       seek-pos 0 seek drop
       disk-buf gpt-part-size read drop gpt-prep-partition? IF
-         debug-disk-label? IF
-            ." GPT PReP partition found " cr
-         THEN
-         disk-buf gpt-part-entry>first-lba x@-le
-         disk-buf gpt-part-entry>last-lba x@-le
-         over - 1+                 ( addr offset len )
-         swap                      ( addr len offset )
-         block-size * to part-offset
-         0 0 seek drop             ( addr len )
-         block-size * read         ( size )
+         debug-disk-label? IF  ." GPT PReP partition found " cr THEN
+         disk-buf gpt-part-entry>first-lba x@-le ( addr first-lba )
+         disk-buf gpt-part-entry>last-lba x@-le  ( addr first-lba last-lba)
+         over - 1+                            ( addr first-lba blocks )
+         swap                                 ( addr blocks first-lba )
+         block-size * to part-offset          ( addr blocks )
+         0 0 seek drop                        ( addr blocks )
+         block-size * read                    ( size )
+         UNLOOP EXIT
+     THEN
+     seek-pos gpt-part-size i * + to seek-pos
+   LOOP
+   false
+;
+
+: try-gpt-dos-partition ( -- true|false )
+   get-gpt-partition 0= IF false EXIT THEN
+   disk-buf gpt>num-part-entry l@-le dup 0= IF false EXIT THEN
+   1+ 1 ?DO
+      seek-pos 0 seek drop
+      disk-buf gpt-part-size read drop
+      gpt-basic-data-partition? IF
+         debug-disk-label? IF ." GPT BASIC DATA partition found " cr THEN
+         disk-buf gpt-part-entry>first-lba x@-le       ( first-lba )
+         dup to part-start                          ( first-lba )
+         disk-buf gpt-part-entry>last-lba x@-le        ( first-lba last-lba )
+         over - 1+                                  ( first-lba s1 )
+         block-size * to part-size                  ( first-lba )
+         block-size * to part-offset                ( )
+         0 0 seek drop
+         disk-buf block-size read drop
+         disk-buf fat-bootblock?                    ( true|false )
          UNLOOP EXIT
       THEN
       seek-pos gpt-part-size i * + to seek-pos
@@ -491,7 +549,7 @@ AA268B49521E5A8B    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
@@ -591,6 +649,7 @@ AA268B49521E5A8B    CONSTANT GPT-PREP-PARTITION-4
 
 : try-partitions ( -- found? )
    try-dos-partition IF try-files EXIT THEN
+   try-gpt-dos-partition IF try-files EXIT THEN
    \ try-iso9660-partition IF try-files EXIT THEN
    \ ... more partition types here...
    false
-- 
2.4.3

Re: [PATCH SLOF v3 5/5] disk-label: add support for booting from GPT FAT partition

[Thomas Huth]

Sorry, every time I look at this gpt stuff, my eyes stumble
over something new ...

On Tue, 30 Jun 2015 16:31:21 +0530
Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> wrote:

> For a GPT+LVM combination disk, older bootloader that does not support
> LVM, cannot load kernel from LVM.
> 
> The patch adds support to read from BASIC_DATA UUID partitions for the
> case that the OS installer has installed the CHRP-BOOT config on a FAT
> file system.
> 
> Makes GPT detection robust
> * Check for Protective MBR Magic
> * Check for valid GPT Signature
> * Boundary check for allocated block size before reading into the
>   buffer
> 
> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> ---
>  slof/fs/packages/disk-label.fs | 99 +++++++++++++++++++++++++++++++++---------
>  1 file changed, 79 insertions(+), 20 deletions(-)
> 
> diff --git a/slof/fs/packages/disk-label.fs b/slof/fs/packages/disk-label.fs
> index 347dc5d..5267ddb 100644
> --- a/slof/fs/packages/disk-label.fs
> +++ b/slof/fs/packages/disk-label.fs
...
> +\ The routine checks whether the protective MBR has GPT ID and then
> +\ reads the gpt data from the sector. Also set the seek position and
> +\ the partition size used in caller routines.
> +
> +: get-gpt-partition ( -- true|false )
> +   no-gpt? IF false EXIT THEN
> +   debug-disk-label? IF cr ." GPT partition found " cr  THEN
> +   1 read-disk-buf
> +   disk-buf gpt>part-entry-lba x@-le
>     block-size * to seek-pos
>     disk-buf gpt>part-entry-size l@-le to gpt-part-size
> -   disk-buf gpt>num-part-entry l@-le dup 0= IF false EXIT THEN
> +   gpt-part-size disk-buf-size > IF
> +      cr ." GPT part size exceeds buffer allocated " cr
> +      false exit
> +   THEN
> +   disk-buf gpt>signature x@ GPT-SIGNATURE =
> +;
> +
> +: load-from-gpt-prep-partition ( addr -- size )
> +   get-gpt-partition 0= IF false EXIT THEN
> +   disk-buf gpt>num-part-entry l@-le dup 0= IF false exit THEN
>     1+ 1 ?DO
>        seek-pos 0 seek drop
>        disk-buf gpt-part-size read drop gpt-prep-partition? IF
> -         debug-disk-label? IF
> -            ." GPT PReP partition found " cr
> -         THEN
> -         disk-buf gpt-part-entry>first-lba x@-le
> -         disk-buf gpt-part-entry>last-lba x@-le
> -         over - 1+                 ( addr offset len )
> -         swap                      ( addr len offset )
> -         block-size * to part-offset
> -         0 0 seek drop             ( addr len )
> -         block-size * read         ( size )
> +         debug-disk-label? IF  ." GPT PReP partition found " cr THEN
> +         disk-buf gpt-part-entry>first-lba x@-le ( addr first-lba )
> +         disk-buf gpt-part-entry>last-lba x@-le  ( addr first-lba last-lba)
> +         over - 1+                            ( addr first-lba blocks )
> +         swap                                 ( addr blocks first-lba )
> +         block-size * to part-offset          ( addr blocks )
> +         0 0 seek drop                        ( addr blocks )
> +         block-size * read                    ( size )
> +         UNLOOP EXIT
> +     THEN
> +     seek-pos gpt-part-size i * + to seek-pos

Is this the right way to update the seek pos? Looks somewhat suspicious
to me, shouldn't this rather be:

	seek-pos gpt-part-size + to seek-pos

or maybe if you store the base value somewhere instead, something like:

	seek-pos-base gpt-part-size i * + to seek-pos

?

> +   LOOP
> +   false
> +;
> +
> +: try-gpt-dos-partition ( -- true|false )
> +   get-gpt-partition 0= IF false EXIT THEN
> +   disk-buf gpt>num-part-entry l@-le dup 0= IF false EXIT THEN
> +   1+ 1 ?DO
> +      seek-pos 0 seek drop
> +      disk-buf gpt-part-size read drop
> +      gpt-basic-data-partition? IF
> +         debug-disk-label? IF ." GPT BASIC DATA partition found " cr THEN
> +         disk-buf gpt-part-entry>first-lba x@-le       ( first-lba )
> +         dup to part-start                          ( first-lba )
> +         disk-buf gpt-part-entry>last-lba x@-le        ( first-lba last-lba )
> +         over - 1+                                  ( first-lba s1 )
> +         block-size * to part-size                  ( first-lba )
> +         block-size * to part-offset                ( )
> +         0 0 seek drop
> +         disk-buf block-size read drop
> +         disk-buf fat-bootblock?                    ( true|false )
>           UNLOOP EXIT
>        THEN
>        seek-pos gpt-part-size i * + to seek-pos

dito (so this bug was likely there before?)

 Thomas

Re: [PATCH SLOF v3 5/5] disk-label: add support for booting from GPT FAT partition

[Nikunj A Dadhania]

Thomas Huth <thuth@redhat.com> writes:

> Sorry, every time I look at this gpt stuff, my eyes stumble
> over something new ...

No worries :-)

> On Tue, 30 Jun 2015 16:31:21 +0530
> Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> wrote:
>
>> For a GPT+LVM combination disk, older bootloader that does not support
>> LVM, cannot load kernel from LVM.
>> 
>> The patch adds support to read from BASIC_DATA UUID partitions for the
>> case that the OS installer has installed the CHRP-BOOT config on a FAT
>> file system.
>> 
>> Makes GPT detection robust
>> * Check for Protective MBR Magic
>> * Check for valid GPT Signature
>> * Boundary check for allocated block size before reading into the
>>   buffer
>> 
>> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
>> ---
>>  slof/fs/packages/disk-label.fs | 99 +++++++++++++++++++++++++++++++++---------
>>  1 file changed, 79 insertions(+), 20 deletions(-)
>> 
>> diff --git a/slof/fs/packages/disk-label.fs b/slof/fs/packages/disk-label.fs
>> index 347dc5d..5267ddb 100644
>> --- a/slof/fs/packages/disk-label.fs
>> +++ b/slof/fs/packages/disk-label.fs
> ...
>> +\ The routine checks whether the protective MBR has GPT ID and then
>> +\ reads the gpt data from the sector. Also set the seek position and
>> +\ the partition size used in caller routines.
>> +
>> +: get-gpt-partition ( -- true|false )
>> +   no-gpt? IF false EXIT THEN
>> +   debug-disk-label? IF cr ." GPT partition found " cr  THEN
>> +   1 read-disk-buf
>> +   disk-buf gpt>part-entry-lba x@-le
>>     block-size * to seek-pos
>>     disk-buf gpt>part-entry-size l@-le to gpt-part-size
>> -   disk-buf gpt>num-part-entry l@-le dup 0= IF false EXIT THEN
>> +   gpt-part-size disk-buf-size > IF
>> +      cr ." GPT part size exceeds buffer allocated " cr
>> +      false exit
>> +   THEN
>> +   disk-buf gpt>signature x@ GPT-SIGNATURE =
>> +;
>> +
>> +: load-from-gpt-prep-partition ( addr -- size )
>> +   get-gpt-partition 0= IF false EXIT THEN
>> +   disk-buf gpt>num-part-entry l@-le dup 0= IF false exit THEN
>>     1+ 1 ?DO
>>        seek-pos 0 seek drop
>>        disk-buf gpt-part-size read drop gpt-prep-partition? IF
>> -         debug-disk-label? IF
>> -            ." GPT PReP partition found " cr
>> -         THEN
>> -         disk-buf gpt-part-entry>first-lba x@-le
>> -         disk-buf gpt-part-entry>last-lba x@-le
>> -         over - 1+                 ( addr offset len )
>> -         swap                      ( addr len offset )
>> -         block-size * to part-offset
>> -         0 0 seek drop             ( addr len )
>> -         block-size * read         ( size )
>> +         debug-disk-label? IF  ." GPT PReP partition found " cr THEN
>> +         disk-buf gpt-part-entry>first-lba x@-le ( addr first-lba )
>> +         disk-buf gpt-part-entry>last-lba x@-le  ( addr first-lba last-lba)
>> +         over - 1+                            ( addr first-lba blocks )
>> +         swap                                 ( addr blocks first-lba )
>> +         block-size * to part-offset          ( addr blocks )
>> +         0 0 seek drop                        ( addr blocks )
>> +         block-size * read                    ( size )
>> +         UNLOOP EXIT
>> +     THEN
>> +     seek-pos gpt-part-size i * + to seek-pos
>
> Is this the right way to update the seek pos?

Good catch !! its jumping ahead, should go in the order of 128byte
(gpt-part-size)

> Looks somewhat suspicious
> to me, shouldn't this rather be:
>
> 	seek-pos gpt-part-size + to seek-pos

Would use this. Updated patch attached.

> or maybe if you store the base value somewhere instead, something like:
>
> 	seek-pos-base gpt-part-size i * + to seek-pos
>
> ?
>
>> +   LOOP
>> +   false
>> +;
>> +
>> +: try-gpt-dos-partition ( -- true|false )
>> +   get-gpt-partition 0= IF false EXIT THEN
>> +   disk-buf gpt>num-part-entry l@-le dup 0= IF false EXIT THEN
>> +   1+ 1 ?DO
>> +      seek-pos 0 seek drop
>> +      disk-buf gpt-part-size read drop
>> +      gpt-basic-data-partition? IF
>> +         debug-disk-label? IF ." GPT BASIC DATA partition found " cr THEN
>> +         disk-buf gpt-part-entry>first-lba x@-le       ( first-lba )
>> +         dup to part-start                          ( first-lba )
>> +         disk-buf gpt-part-entry>last-lba x@-le        ( first-lba last-lba )
>> +         over - 1+                                  ( first-lba s1 )
>> +         block-size * to part-size                  ( first-lba )
>> +         block-size * to part-offset                ( )
>> +         0 0 seek drop
>> +         disk-buf block-size read drop
>> +         disk-buf fat-bootblock?                    ( true|false )
>>           UNLOOP EXIT
>>        THEN
>>        seek-pos gpt-part-size i * + to seek-pos
>
> dito (so this bug was likely there before?)

Seems was working because it happens to be the first partition. If not
would have failed.


    disk-label: add support for booting from GPT FAT partition
    
    For a GPT+LVM combination disk, older bootloader that does not support
    LVM, cannot load kernel from LVM.
    
    The patch adds support to read from BASIC_DATA UUID partitions for the
    case that the OS installer has installed the CHRP-BOOT config on a FAT
    file system.
    
    Makes GPT detection robust
    * Fix bug in seek-pos updation code
    * Check for Protective MBR Magic
    * Check for valid GPT Signature
    * Boundary check for allocated block size before reading into the
      buffer
    
    Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>

diff --git a/slof/fs/packages/disk-label.fs b/slof/fs/packages/disk-label.fs
index 347dc5d..fa5df27 100644
--- a/slof/fs/packages/disk-label.fs
+++ b/slof/fs/packages/disk-label.fs
@@ -179,7 +179,8 @@ CONSTANT /gpt-part-entry
 \ This word returns true if the currently loaded disk-buf has _NO_ GPT partition id
 : no-gpt? ( -- true|false )
    0 read-disk-buf
-   1 partition>part-entry part-entry>id c@ ee <>
+   1 partition>part-entry part-entry>id c@ ee <> IF true EXIT THEN
+   disk-buf mbr>magic w@-le aa55 <>
 ;
 
 : pc-extended-partition? ( part-entry-addr -- true|false )
@@ -267,7 +268,10 @@ CONSTANT /gpt-part-entry
 
 : try-dos-partition ( -- okay? )
    \ Read partition table and check magic.
-   no-mbr? IF cr ." No DOS disk-label found." cr false EXIT THEN
+   no-mbr? IF
+       debug-disk-label? IF cr ." No DOS disk-label found." cr THEN
+       false EXIT
+   THEN
 
    count-dos-logical-partitions TO dos-logical-partitions
 
@@ -377,31 +381,85 @@ AA268B49521E5A8B    CONSTANT GPT-PREP-PARTITION-4
        8 + x@    GPT-PREP-PARTITION-4 =
 ;
 
-: load-from-gpt-prep-partition ( addr -- size )
-   no-gpt? IF drop false EXIT THEN
-   debug-disk-label? IF
-      cr ." GPT partition found " cr
-   THEN
-   1 read-disk-buf disk-buf gpt>part-entry-lba l@-le
+\ Check for GPT MSFT BASIC DATA GUID - fat based
+EBD0A0A2            CONSTANT GPT-BASIC-DATA-PARTITION-1
+B9E5                CONSTANT GPT-BASIC-DATA-PARTITION-2
+4433                CONSTANT GPT-BASIC-DATA-PARTITION-3
+87C068B6B72699C7    CONSTANT GPT-BASIC-DATA-PARTITION-4
+
+: gpt-basic-data-partition? ( -- true|false )
+   disk-buf gpt-part-entry>part-type-guid
+   dup l@-le     GPT-BASIC-DATA-PARTITION-1 <> IF drop false EXIT THEN
+   dup 4 + w@-le GPT-BASIC-DATA-PARTITION-2 <> IF drop false EXIT THEN
+   dup 6 + w@-le GPT-BASIC-DATA-PARTITION-3 <> IF drop false EXIT THEN
+       8 + x@    GPT-BASIC-DATA-PARTITION-4 =
+;
+
+\
+\ GPT Signature
+\ ("EFI PART", 45h 46h 49h 20h 50h 41h 52h 54h)
+\
+4546492050415254 CONSTANT GPT-SIGNATURE
+
+\ The routine checks whether the protective MBR has GPT ID and then
+\ reads the gpt data from the sector. Also set the seek position and
+\ the partition size used in caller routines.
+
+: get-gpt-partition ( -- true|false )
+   no-gpt? IF false EXIT THEN
+   debug-disk-label? IF cr ." GPT partition found " cr  THEN
+   1 read-disk-buf
+   disk-buf gpt>part-entry-lba x@-le
    block-size * to seek-pos
    disk-buf gpt>part-entry-size l@-le to gpt-part-size
-   disk-buf gpt>num-part-entry l@-le dup 0= IF false EXIT THEN
+   gpt-part-size disk-buf-size > IF
+      cr ." GPT part size exceeds buffer allocated " cr
+      false exit
+   THEN
+   disk-buf gpt>signature x@ GPT-SIGNATURE =
+;
+
+: load-from-gpt-prep-partition ( addr -- size )
+   get-gpt-partition 0= IF false EXIT THEN
+   disk-buf gpt>num-part-entry l@-le dup 0= IF false exit THEN
    1+ 1 ?DO
       seek-pos 0 seek drop
       disk-buf gpt-part-size read drop gpt-prep-partition? IF
-         debug-disk-label? IF
-            ." GPT PReP partition found " cr
-         THEN
-         disk-buf gpt-part-entry>first-lba x@-le
-         disk-buf gpt-part-entry>last-lba x@-le
-         over - 1+                 ( addr offset len )
-         swap                      ( addr len offset )
-         block-size * to part-offset
-         0 0 seek drop             ( addr len )
-         block-size * read         ( size )
+         debug-disk-label? IF  ." GPT PReP partition found " cr THEN
+         disk-buf gpt-part-entry>first-lba x@-le ( addr first-lba )
+         disk-buf gpt-part-entry>last-lba x@-le  ( addr first-lba last-lba)
+         over - 1+                            ( addr first-lba blocks )
+         swap                                 ( addr blocks first-lba )
+         block-size * to part-offset          ( addr blocks )
+         0 0 seek drop                        ( addr blocks )
+         block-size * read                    ( size )
+         UNLOOP EXIT
+     THEN
+     seek-pos gpt-part-size + to seek-pos
+   LOOP
+   false
+;
+
+: try-gpt-dos-partition ( -- true|false )
+   get-gpt-partition 0= IF false EXIT THEN
+   disk-buf gpt>num-part-entry l@-le dup 0= IF false EXIT THEN
+   1+ 1 ?DO
+      seek-pos 0 seek drop
+      disk-buf gpt-part-size read drop
+      gpt-basic-data-partition? IF
+         debug-disk-label? IF ." GPT BASIC DATA partition found " cr THEN
+         disk-buf gpt-part-entry>first-lba x@-le       ( first-lba )
+         dup to part-start                          ( first-lba )
+         disk-buf gpt-part-entry>last-lba x@-le        ( first-lba last-lba )
+         over - 1+                                  ( first-lba s1 )
+         block-size * to part-size                  ( first-lba )
+         block-size * to part-offset                ( )
+         0 0 seek drop
+         disk-buf block-size read drop
+         disk-buf fat-bootblock?                    ( true|false )
          UNLOOP EXIT
       THEN
-      seek-pos gpt-part-size i * + to seek-pos
+      seek-pos gpt-part-size + to seek-pos
    LOOP
    false
 ;
@@ -491,7 +549,7 @@ AA268B49521E5A8B    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
@@ -591,6 +649,7 @@ AA268B49521E5A8B    CONSTANT GPT-PREP-PARTITION-4
 
 : try-partitions ( -- found? )
    try-dos-partition IF try-files EXIT THEN
+   try-gpt-dos-partition IF try-files EXIT THEN
    \ try-iso9660-partition IF try-files EXIT THEN
    \ ... more partition types here...
    false

Re: [PATCH SLOF v3 5/5] disk-label: add support for booting from GPT FAT partition

[Thomas Huth]

On Tue, 30 Jun 2015 17:54:14 +0530
Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> wrote:
> 
>     disk-label: add support for booting from GPT FAT partition
>     
>     For a GPT+LVM combination disk, older bootloader that does not support
>     LVM, cannot load kernel from LVM.
>     
>     The patch adds support to read from BASIC_DATA UUID partitions for the
>     case that the OS installer has installed the CHRP-BOOT config on a FAT
>     file system.
>     
>     Makes GPT detection robust
>     * Fix bug in seek-pos updation code
>     * Check for Protective MBR Magic
>     * Check for valid GPT Signature
>     * Boundary check for allocated block size before reading into the
>       buffer
>     
>     Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> 
> diff --git a/slof/fs/packages/disk-label.fs b/slof/fs/packages/disk-label.fs
> index 347dc5d..fa5df27 100644
> --- a/slof/fs/packages/disk-label.fs
> +++ b/slof/fs/packages/disk-label.fs
> @@ -179,7 +179,8 @@ CONSTANT /gpt-part-entry
>  \ This word returns true if the currently loaded disk-buf has _NO_ GPT partition id
>  : no-gpt? ( -- true|false )
>     0 read-disk-buf
> -   1 partition>part-entry part-entry>id c@ ee <>
> +   1 partition>part-entry part-entry>id c@ ee <> IF true EXIT THEN
> +   disk-buf mbr>magic w@-le aa55 <>
>  ;
>  
>  : pc-extended-partition? ( part-entry-addr -- true|false )
> @@ -267,7 +268,10 @@ CONSTANT /gpt-part-entry
>  
>  : try-dos-partition ( -- okay? )
>     \ Read partition table and check magic.
> -   no-mbr? IF cr ." No DOS disk-label found." cr false EXIT THEN
> +   no-mbr? IF
> +       debug-disk-label? IF cr ." No DOS disk-label found." cr THEN
> +       false EXIT
> +   THEN
>  
>     count-dos-logical-partitions TO dos-logical-partitions
>  
> @@ -377,31 +381,85 @@ AA268B49521E5A8B    CONSTANT GPT-PREP-PARTITION-4
>         8 + x@    GPT-PREP-PARTITION-4 =
>  ;
>  
> -: load-from-gpt-prep-partition ( addr -- size )
> -   no-gpt? IF drop false EXIT THEN
> -   debug-disk-label? IF
> -      cr ." GPT partition found " cr
> -   THEN
> -   1 read-disk-buf disk-buf gpt>part-entry-lba l@-le
> +\ Check for GPT MSFT BASIC DATA GUID - fat based
> +EBD0A0A2            CONSTANT GPT-BASIC-DATA-PARTITION-1
> +B9E5                CONSTANT GPT-BASIC-DATA-PARTITION-2
> +4433                CONSTANT GPT-BASIC-DATA-PARTITION-3
> +87C068B6B72699C7    CONSTANT GPT-BASIC-DATA-PARTITION-4
> +
> +: gpt-basic-data-partition? ( -- true|false )
> +   disk-buf gpt-part-entry>part-type-guid
> +   dup l@-le     GPT-BASIC-DATA-PARTITION-1 <> IF drop false EXIT THEN
> +   dup 4 + w@-le GPT-BASIC-DATA-PARTITION-2 <> IF drop false EXIT THEN
> +   dup 6 + w@-le GPT-BASIC-DATA-PARTITION-3 <> IF drop false EXIT THEN
> +       8 + x@    GPT-BASIC-DATA-PARTITION-4 =
> +;
> +
> +\
> +\ GPT Signature
> +\ ("EFI PART", 45h 46h 49h 20h 50h 41h 52h 54h)
> +\
> +4546492050415254 CONSTANT GPT-SIGNATURE
> +
> +\ The routine checks whether the protective MBR has GPT ID and then
> +\ reads the gpt data from the sector. Also set the seek position and
> +\ the partition size used in caller routines.
> +
> +: get-gpt-partition ( -- true|false )
> +   no-gpt? IF false EXIT THEN
> +   debug-disk-label? IF cr ." GPT partition found " cr  THEN
> +   1 read-disk-buf
> +   disk-buf gpt>part-entry-lba x@-le
>     block-size * to seek-pos
>     disk-buf gpt>part-entry-size l@-le to gpt-part-size
> -   disk-buf gpt>num-part-entry l@-le dup 0= IF false EXIT THEN
> +   gpt-part-size disk-buf-size > IF
> +      cr ." GPT part size exceeds buffer allocated " cr
> +      false exit
> +   THEN
> +   disk-buf gpt>signature x@ GPT-SIGNATURE =
> +;
> +
> +: load-from-gpt-prep-partition ( addr -- size )
> +   get-gpt-partition 0= IF false EXIT THEN
> +   disk-buf gpt>num-part-entry l@-le dup 0= IF false exit THEN
>     1+ 1 ?DO
>        seek-pos 0 seek drop
>        disk-buf gpt-part-size read drop gpt-prep-partition? IF
> -         debug-disk-label? IF
> -            ." GPT PReP partition found " cr
> -         THEN
> -         disk-buf gpt-part-entry>first-lba x@-le
> -         disk-buf gpt-part-entry>last-lba x@-le
> -         over - 1+                 ( addr offset len )
> -         swap                      ( addr len offset )
> -         block-size * to part-offset
> -         0 0 seek drop             ( addr len )
> -         block-size * read         ( size )
> +         debug-disk-label? IF  ." GPT PReP partition found " cr THEN
> +         disk-buf gpt-part-entry>first-lba x@-le ( addr first-lba )
> +         disk-buf gpt-part-entry>last-lba x@-le  ( addr first-lba last-lba)
> +         over - 1+                            ( addr first-lba blocks )
> +         swap                                 ( addr blocks first-lba )
> +         block-size * to part-offset          ( addr blocks )
> +         0 0 seek drop                        ( addr blocks )
> +         block-size * read                    ( size )
> +         UNLOOP EXIT
> +     THEN
> +     seek-pos gpt-part-size + to seek-pos
> +   LOOP
> +   false
> +;
> +
> +: try-gpt-dos-partition ( -- true|false )
> +   get-gpt-partition 0= IF false EXIT THEN
> +   disk-buf gpt>num-part-entry l@-le dup 0= IF false EXIT THEN
> +   1+ 1 ?DO
> +      seek-pos 0 seek drop
> +      disk-buf gpt-part-size read drop
> +      gpt-basic-data-partition? IF
> +         debug-disk-label? IF ." GPT BASIC DATA partition found " cr THEN
> +         disk-buf gpt-part-entry>first-lba x@-le       ( first-lba )
> +         dup to part-start                          ( first-lba )
> +         disk-buf gpt-part-entry>last-lba x@-le        ( first-lba last-lba )
> +         over - 1+                                  ( first-lba s1 )
> +         block-size * to part-size                  ( first-lba )
> +         block-size * to part-offset                ( )
> +         0 0 seek drop
> +         disk-buf block-size read drop
> +         disk-buf fat-bootblock?                    ( true|false )
>           UNLOOP EXIT
>        THEN
> -      seek-pos gpt-part-size i * + to seek-pos
> +      seek-pos gpt-part-size + to seek-pos
>     LOOP
>     false
>  ;
> @@ -491,7 +549,7 @@ AA268B49521E5A8B    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
> @@ -591,6 +649,7 @@ AA268B49521E5A8B    CONSTANT GPT-PREP-PARTITION-4
>  
>  : try-partitions ( -- found? )
>     try-dos-partition IF try-files EXIT THEN
> +   try-gpt-dos-partition IF try-files EXIT THEN
>     \ try-iso9660-partition IF try-files EXIT THEN
>     \ ... more partition types here...
>     false

This version now looks fine to me.

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH SLOF v3 3/5] disk-label: rename confusing "block" word

[Segher Boessenkool]

On Tue, Jun 30, 2015 at 04:31:19PM +0530, Nikunj A Dadhania wrote:
> "block" word is not a block number, actually its an allocated host
> address.  Rename it to disk-buf along with a associated
> size(disk-buf-size=4096) for using during allocation/free.
> 
> Also renaming the helper routine read-sector to read-disk-buf. This
> routine assumes the address to be disk-buf and only takes sector number
> as argument.

This isn't what I suggested, and I think it is a terrible idea.
Just FWIW :-)


Segher

Re: [PATCH SLOF v3 3/5] disk-label: rename confusing "block" word

[Nikunj A Dadhania]

Hi Segher,

Segher Boessenkool <segher@kernel.crashing.org> writes:
> On Tue, Jun 30, 2015 at 04:31:19PM +0530, Nikunj A Dadhania wrote:
>> "block" word is not a block number, actually its an allocated host
>> address.  Rename it to disk-buf along with a associated
>> size(disk-buf-size=4096) for using during allocation/free.
>> 
>> Also renaming the helper routine read-sector to read-disk-buf. This
>> routine assumes the address to be disk-buf and only takes sector number
>> as argument.
>
> This isn't what I suggested, and I think it is a terrible idea.

The comment was against the "has-fat-filesystem". As the complete
disk-label.fs had that same assumption, I went ahead and renamed "block"
across the file.

Are you suggesting to drop complete patch or just the rename of
"read-sector" ?

> Just FWIW :-)

Regards
Nikunj

Re: [PATCH SLOF v3 3/5] disk-label: rename confusing "block" word

[Segher Boessenkool]

On Thu, Jul 02, 2015 at 11:17:49AM +0530, Nikunj A Dadhania wrote:
> >> "block" word is not a block number, actually its an allocated host
> >> address.  Rename it to disk-buf along with a associated
> >> size(disk-buf-size=4096) for using during allocation/free.
> >> 
> >> Also renaming the helper routine read-sector to read-disk-buf. This
> >> routine assumes the address to be disk-buf and only takes sector number
> >> as argument.
> >
> > This isn't what I suggested, and I think it is a terrible idea.
> 
> The comment was against the "has-fat-filesystem". As the complete
> disk-label.fs had that same assumption, I went ahead and renamed "block"
> across the file.

No, I said that "block" in that stack comment was misleading.  Nothing more.

Since the word "block" is used all over the file (as your patch size shows),
a short name is much better than a longer name, esp. if that shorter name
actually is more expressive.


Segher

Re: [PATCH SLOF v3 3/5] disk-label: rename confusing "block" word

[Nikunj A Dadhania]

Segher Boessenkool <segher@kernel.crashing.org> writes:

> On Thu, Jul 02, 2015 at 11:17:49AM +0530, Nikunj A Dadhania wrote:
>> >> "block" word is not a block number, actually its an allocated host
>> >> address.  Rename it to disk-buf along with a associated
>> >> size(disk-buf-size=4096) for using during allocation/free.
>> >> 
>> >> Also renaming the helper routine read-sector to read-disk-buf. This
>> >> routine assumes the address to be disk-buf and only takes sector number
>> >> as argument.
>> >
>> > This isn't what I suggested, and I think it is a terrible idea.
>> 
>> The comment was against the "has-fat-filesystem". As the complete
>> disk-label.fs had that same assumption, I went ahead and renamed "block"
>> across the file.
>
> No, I said that "block" in that stack comment was misleading.  Nothing more.
>
> Since the word "block" is used all over the file (as your patch size shows),
> a short name is much better than a longer name, esp. if that shorter name
> actually is more expressive.

Sure, will resend dropping this patch.

Regards,
Nikunj