On Thu, 11 Jun 2015 15:48:49 +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. > > Signed-off-by: Nikunj A Dadhania <nik...@linux.vnet.ibm.com> > --- > slof/fs/packages/disk-label.fs | 72 > ++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 70 insertions(+), 2 deletions(-) > > diff --git a/slof/fs/packages/disk-label.fs b/slof/fs/packages/disk-label.fs > index fe1c25e..bf5fb5c 100644 > --- a/slof/fs/packages/disk-label.fs > +++ b/slof/fs/packages/disk-label.fs > @@ -266,7 +266,7 @@ 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 false EXIT THEN Maybe keep the output when "debug-disk-label?" is set? > count-dos-logical-partitions TO dos-logical-partitions > > @@ -408,6 +408,73 @@ AA26 CONSTANT GPT-PREP-PARTITION-4 > FALSE > ; > > +\ Check for GPT MSFT BASIC DATA GUID - vfat based > +EBD0A0A2 CONSTANT GPT-BASIC-DATA-PARTITION-1 > +B9E5 CONSTANT GPT-BASIC-DATA-PARTITION-2 > +4433 CONSTANT GPT-BASIC-DATA-PARTITION-3 > +87C0 CONSTANT GPT-BASIC-DATA-PARTITION-4 > +68B6B72699C7 CONSTANT GPT-BASIC-DATA-PARTITION-5 > + > +: gpt-basic-data-partition? ( -- true|false ) > + block gpt-part-entry>part-type-guid l@-le GPT-BASIC-DATA-PARTITION-1 = IF > + block gpt-part-entry>part-type-guid 4 + w@-le > + GPT-BASIC-DATA-PARTITION-2 = IF > + block gpt-part-entry>part-type-guid 6 + w@-le > + GPT-BASIC-DATA-PARTITION-3 = IF > + block gpt-part-entry>part-type-guid 8 + w@ 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) > + 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 Thomas _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
