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

[Thomas Huth]

On Mon, 22 Jun 2015 13:29:46 +0530
Nikunj A Dadhania <nikunj at 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 add support to read from BASIC_DATA UUID
> partition. Installer has installed CHRP-BOOT config on a FAT file
> system.

Maybe better: 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.

?

> Signed-off-by: Nikunj A Dadhania <nikunj at linux.vnet.ibm.com>
> ---
>  slof/fs/packages/disk-label.fs | 54 +++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 48 insertions(+), 6 deletions(-)
> 
> diff --git a/slof/fs/packages/disk-label.fs b/slof/fs/packages/disk-label.fs
> index e317e93..821e959 100644
> --- a/slof/fs/packages/disk-label.fs
> +++ b/slof/fs/packages/disk-label.fs
> @@ -266,7 +266,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
>  
> @@ -381,18 +384,38 @@ AA268B49521E5A8B    CONSTANT GPT-PREP-PARTITION-4
>     TRUE
>  ;
>  
> -: load-from-gpt-prep-partition ( addr -- size )
> +\ 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 )
> +   block gpt-part-entry>part-type-guid
> +   dup l at -le     GPT-BASIC-DATA-PARTITION-1 <> IF DROP FALSE EXIT THEN
> +   dup 4 + w at -le GPT-BASIC-DATA-PARTITION-2 <> IF DROP FALSE EXIT THEN
> +   dup 6 + w at -le GPT-BASIC-DATA-PARTITION-3 <> IF DROP FALSE EXIT THEN
> +       8 + x@    GPT-BASIC-DATA-PARTITION-4 <> IF FALSE EXIT THEN
> +   TRUE
> +;
> +
> +\ Can be called from two path
> +\ 1) load-from-boot-partition for GPT PReP partition
> +\ 2) try-partitions for gpt basic data partition having fat chrp-boot

What did you want to achieve with the above comment? Caller locations
can change with later patches, but it's unlikely that everybody
remembers to update such comments in that case. So unlikely you've got
a good reason for above comment (but in that case, it maybe should be
written in a different way), I'd suggest to drop it.

> +: load-from-gpt-partition ( [ addr ] -- size | TRUE )

What do you mean with addr in square brackets? Is it optional?

>     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 at -le
> +   1 read-sector block gpt>part-entry-lba x at -le
>     block-size * to seek-pos
>     block gpt>part-entry-size l at -le to gpt-part-size
>     block gpt>num-part-entry l at -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
> +       block gpt-part-size read drop
> +       gpt-prep-partition? IF

You've changed the level of indentation here. Please try to avoid that
(unless you've got a good reason, e.g. because the previous indentation
was obviously wrong)

>           debug-disk-label? IF
>              ." GPT PReP partition found " cr
>           THEN
> @@ -404,6 +427,24 @@ AA268B49521E5A8B    CONSTANT GPT-PREP-PARTITION-4
>           0 0 seek drop             ( addr len )
>           block-size * read         ( size )
>           UNLOOP EXIT
> +     THEN
> +     gpt-basic-data-partition? IF

Hmm, I wonder whether we need a proper coding conventions spec for
writing Forth code ... (at least about the indentation depths ...) ;-)

> +         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 has-fat-filesystem 0= IF false UNLOOP EXIT THEN
> +         TRUE
> +         UNLOOP EXIT
>        THEN
>        seek-pos gpt-part-size i * + to seek-pos
>     LOOP
> @@ -495,11 +536,11 @@ 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
> -   load-from-gpt-prep-partition
> +   load-from-gpt-partition

So here the function is called with an "addr" parameter on the stack ...

>     \ More boot partition formats ...
>  ;
>  
> @@ -594,6 +635,7 @@ AA268B49521E5A8B    CONSTANT GPT-PREP-PARTITION-4
>  
>  : try-partitions ( -- found? )
>     try-dos-partition IF try-files EXIT THEN
> +   load-from-gpt-partition IF try-files EXIT THEN

... but here it is called without an "addr" parameter? *Confusing*
How does this work?

 Thomas