Thomas Huth <th...@redhat.com> writes: > On Mon, 22 Jun 2015 13:29:46 +0530 > Nikunj A Dadhania <nik...@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 <nik...@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@-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 <> 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.
I will redo this word and split it so that do not have confusing signature. > >> +: 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@-le >> + 1 read-sector block gpt>part-entry-lba x@-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 >> 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) Not intentional, I try to make sure I do not change the indentation. > >> 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? Yes :-) Let me rework on this to break it up for readabilty and better stack debugging. Regards Nikunj _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
