[U-Boot] [BUG] U-Boot hangs on fatload, commit ee88eacbdd840199a3dec707234579fb15ddd46a
Heinrich Schuchardt
xypron.glpk at gmx.de
Sun Nov 10 08:41:03 UTC 2019
On 11/9/19 10:31 PM, Gray Remlin wrote:
> On Sat, 9 Nov 2019 at 20:50, Heinrich Schuchardt <xypron.glpk at gmx.de
> <mailto:xypron.glpk at gmx.de>> wrote:
>
> On 11/9/19 8:42 PM, Gray Remlin wrote:
> > On Sat, 9 Nov 2019 at 18:40, Heinrich Schuchardt
> <xypron.glpk at gmx.de <mailto:xypron.glpk at gmx.de>
> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk at gmx.de>>> wrote:
> >
> > On 11/9/19 6:08 PM, Heinrich Schuchardt wrote:
> > > On 11/9/19 4:11 PM, Gray Remlin wrote:
> > >> On Fri, 8 Nov 2019 at 20:08, Heinrich Schuchardt
> > <xypron.glpk at gmx.de <mailto:xypron.glpk at gmx.de>
> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk at gmx.de>>
> > >> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk at gmx.de>
> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk at gmx.de>>>> wrote:
> > >>
> > >> On 11/8/19 7:32 PM, Gray Remlin wrote:
> > >> > Please excuse the noise. I would like to file a
> bug report
> > >> against the
> > >> > above commit, a quick search of www.denx.de
> <http://www.denx.de>
> > <http://www.denx.de> <http://www.denx.de>
> > >> <http://www.denx.de> did not
> > >> > reveal how I should proceed. Please point me in
> the right
> > >> direction.
> > >> >
> > >> >
> > >> > Issue:
> > >> > U-Boot hangs (i.e. during boot) whenever the command
> > 'fatload' is
> > >> used.
> > >> >
> > >> > Details:
> > >> > U-Boot 2019.10 compiled with either
> dreamplug_defconfig or
> > >> > guruplug_defconfig.
> > >> >
> > >> > After the commit do_load() now additionally calls
> > >> efi_set_bootdev()
> > >> > which was moved out of do_load_wrapper() which is
> only called
> > >> by the
> > >> > 'load' command.
> > >> >
> > >> > Reverting the commit fixes this issue for me.
> > >> >
> > >>
> > >> Dear Gray,
> > >>
> > >> thanks for reporting the issue with commit
> > >> fs: do_load: pass device path for efi payload
> > >> ee88eacbdd840199a3dec707234579fb15ddd46a
> > >>
> > >> Is it only the fatload command that fails on your
> device or
> > also the
> > >> load command?
> > >>
> > >> There is no bug tracker for U-Boot. So sending a mail
> to the
> > U-Boot
> > >> mailing list, the patch author, and the maintainer is the
> > best way to
> > >> inform the developers about bugs.
> > >>
> > >> Best regards
> > >>
> > >> Heinrich
> > >>
> > >>
> > >> Additional information:
> > >> cross-compiler
> gcc-linaro-7.4.1-2019.02-x86_64_arm-linux-gnueabi
> > >>
> > >> The U-Boot environment being used is the default obtained by
> > >> compiling U-Boot 2020.01-rc1-00100-gee93ef0c4b as
> > dreamplug_defconfig
> > >>
> > >> => printenv
> > >> baudrate=115200
> > >> bootcmd=setenv ethact egiga0; ${x_bootcmd_ethernet};
> setenv ethact
> > >> egiga1; ${x_bootcmd_ethernet}; ${x_bootcmd_usb};
> > ${x_bootcmd_kernel};
> > >> setenv bootargs ${x_bootargs} ${x_bootargs_root}; bootm
> 0x6400000;
> > >> bootdelay=3
> > >> ethact=egiga0
> > >> fdtcontroladdr=1fb8e7c8
> > >> stderr=serial
> > >> stdin=serial
> > >> stdout=serial
> > >> x_bootargs=console=ttyS0,115200
> > >> x_bootargs_root=root=/dev/sda2 rootdelay=10
> > >> x_bootcmd_ethernet=ping 192.168.2.1
> > >> x_bootcmd_kernel=fatload usb 0 0x6400000 uImage
> > >> x_bootcmd_usb=usb start
> > >>
> > >> U-Boot hangs for other syntactically correct invocations
> of either
> > >> 'fatload' or 'load'
> > >> Other commands such as 'fatls' function as expected.
> > >>
> > >> Program flow is as follows:
> > >>
> > >> command 'fatload' (or 'load')
> > >> efi_set_bootdev()
> > >> ...
> > >> efi_dp_split_file_path()
> > >> ...
> > >> efi_dp_dup()
> > >> ....
> > >> efi_dp_size()
> > >> *while exit condition
> never met*
> > >> *infinite loop*
> > >>
> > >>
> > >> This is not an attempted EFI boot, why is EFI code being
> invoked ?
> > >
> > > Thanks for debugging.
> > >
> > > When booting from EFI we need to know from which device
> the EFI
> > binary
> > > was loaded. We use this information to install the loaded
> image
> > > protocol. At the time of the load command we do no know if
> you will
> > > invoke bootz or bootefi.
> >
> >
> > Isn't that the purpose of the 'load' command ?
> >
> > >
> > > It might be that we have a problem with creating device
> paths for
> > USB. I
> > > will try to reproduce this.
> > >
> > > You could add
> > >
> > > printf("efi_dp_split_file_path(%pD)\n", full_path);
> > >
> > > at the beginning of efi_dp_split_file_path() to identify
> what device
> > > path is passed to the function. This should produce an
> output like
> > >
> > > => load scsi 0:2 $kernel_addr_r description.txt
> > >
> >
> efi_dp_split_file_path(/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Scsi(0,0)/HD(2,MBR,0x6fe3a999,0x400,0x400)/description.txt)
> > >
> > >
> > > Best regards
> > >
> > > Heinrich
> >
> > I just tested on an OrangePi PC with v2019.10 and got this:
> >
> > => fatload usb 0:1 $kernel_addr_r test.txt
> >
> efi_dp_split_file_path(/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/UsbClass(0x0,0x0,0x9,0x0,0x1)/UsbClass(0x781,0x5571,0x0,0x0,0x0)/HD(1,MBR,0xfae8c6af,0x800,0x3b9f800)/test.txt)
> > device path =
> >
> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/UsbClass(0x0,0x0,0x9,0x0,0x1)/UsbClass(0x781,0x5571,0x0,0x0,0x0)/HD(1,MBR,0xfae8c6af,0x800,0x3b9f800)
> > file path = /test.txt
> > 12 bytes read in 26 ms (0 Bytes/s)
> > => md.b $kernel_addr_r 0c
> > 42000000: 4a 75 73 74 20 61 20 74 65 73 74 0a Just a test.
> >
> > So debugging on your specific device is needed.
> >
> >
> > Why do you want to debug EFI code on a device that does not
> support EFI ?
> > I am not reporting a bug with EFI, the issue is 'fatload' is now
> broken
> > by this commit.
> > Once 'fatload' is fixed I am willing to test U-Boot as required for
> > other bugs whilst the
> > dreamplug platform is available to me for such.
>
> Your system is compiled with EFI_LOADER. So you could be using fatload
> to load an EFI file. do_fatload() is the only place where we can get the
> device from which you load the file.
>
>
> No, that is (was before this commit) the purpose of 'load'.
> I ask again, why do you want two commands that perform exactly the same
> action ?
> Is it the intention to first unify them and then discard one ?
You are right that the commands ext2ls, ext2load, ext4ls, ext4load,
ext4save, ext4size, fatls, fatload, fatsave, and fatsize are rather
superfluous in the light of ls, load, save, and size. They are just kept
for backward compatibility.
>
>
>
> Could you, please, change the end of efi_dp_from_file() to
>
> printf("fpsize = %u\n", fpsize);
> printf("dpsize = %u\n", dpsize);
> size_t i;
> for (i = 0; i < dpsize + sizeof(END); ++i)
> printf("0x%02x ", ((char *)start)[i]):;
> printf("\n");
>
> return start;
>
> and provide the output.
>
> On my system the output is
>
> => fatload usb 0 $kernel_addr_r uImage
> fpsize = 18
> dpsize = 102
> 0x01 0x04 0x14 0x00 0xb9 0x73 0x1d 0xe6 0x84 0xa3 0xcc 0x4a 0xae 0xab
> 0x82 0xe8 0x28 0xf3 0x62 0x8b 0x03 0x0f 0x0b 0x00 0x00 0x00 0x00 0x00
> 0x09 0x00 0x01 0x03 0x0f 0x0b 0x00 0x81 0x07 0x71 0x55 0x00 0x00 0x00
> 0x04 0x01 0x2a 0x00 0x01 0x00 0x00 0x00 0x00 0x08 0x00 0x00 0x00 0x00
> 0x00 0x00 0x00 0x18 0x00 0x00 0x00 0x00 0x00 0x00 0xc3 0x43 0x04 0xa5
> 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x01 0x01
> 0x04 0x04 0x12 0x00 0x75 0x00 0x49 0x00 0x6d 0x00 0x61 0x00 0x67 0x00
> 0x65 0x00 0x00 0x00 0x7f 0xff 0x04 0x00
> efi_dp_split_file_path('/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/UsbClass(0x0,0x0,0x9,0x0,0x1)/UsbClass(0x781,0x5571,0x0,0x0,0x0)/HD(1,MBR,0xa50443c3,0x800,0x1800)/uImage')
> 20 bytes read in 2 ms (9.8 KiB/s)
> =>
>
> 0x7f 0xff 0x04 0x00
> is the end of device path that seems to be missing for you.
>
>
> This is the default (from the environment 'fatload' command)
> fpsize = 18
> dpsize = 113
> 0x01 0x04 0x14 0x00 0xb9 0x73 0x1d 0xe6 0x84 0xa3 0xcc 0x4a 0xae 0xab
> 0x82 0xe8 0x28 0xf3 0x62 0x8b 0x03 0x0f 0x0b 0x00 0x00 0x00 0x00 0x00
> 0x09 0x00 0x01 0x03 0x0f 0x0b 0x00 0x40 0x1a 0x01 0x01 0x09 0x00 0x01
> 0x03 0x0f 0x0b 0x00 0xe3 0x05 0x26 0x07 0x00 0x00 0x00 0x04 0x01 0x2a
> 0x00 0x01 0x00 0x00 0x00 0x00 0x08 0x00 0x00 0x00 0x00 0x00 0x00 0x00
> 0xf8 0x01 0x00 0x00 0x00 0x00 0x00 0x99 0x28 0x0f 0x00 0x00 0x00 0x00
> 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x01 0x01 0x04 0x04 0x12
> 0x75 0x00 0x49 0x00 0x6d 0x00 0x61 0x00 0x67 0x00 0x65 0x00 0x00 0x00
> 0x00 0x7f 0xff 0x04 0x00
So here "uImage" is copied in one byte left of where we would expect it
according to the structure definition and overlaps the dp->length field.
struct efi_device_path_file_path {
struct efi_device_path dp;
u16 str[];
} __packed;
Could you, please, send me files lib/charset.o and
lib/efi_loader/efi_device_path.o.
Which distribution and which version of GCC are you using?
Adding the following printf() statements might give some more insight:
fp->dp.length = fpsize;
printf("buf = %p\n", buf);
printf("fp->str = %p\n", fp->str);
path_to_uefi(fp->str, path);
buf += fpsize;
Should the above printf() statements have buf + 4 != fp->str:
What happens when you change the structure to have »u16 str[0];«? (This
is what was required before the C99 standard. Cf.
https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html)
Best regards
Heinrich
> efi_dp_split_file_path(/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/UsbClass(0x0,0x0,0x9,0x0,0x1)/UsbClass(0x1a40,0x101,0x9,0x0,0x1)/UsbClass(0x5e3,0x726,0x0,0x0,0x0)/HD(1,MBR,0x000f2899,0x800,0x1f800)/uImage/UNKNOWN(0000,0004)/UNKNOWN(0000,0004)/UNKNOWN(0000,0004)/UNKNOWN(0000,0004)/UNKNOWN(0000,0004)/UNKNOWN(0000,0004)/UNKNOWN(0000,0004)/UNKNOWN(0000,0004)/UNKNOWN(0000,0004)/UNKNOWN(0000,0004)/UNKNOWN(0000,0004)/UNKNOWN(0000,0004)/UNKNOWN(0000,0004)/UNKNOWN(0000,0004)/UNKNOWN(0000,0004)/UNKNOWN(0000,0004)/UNKNOWN(0000,0004)/UNKNOWN(0000,0004))
>
> This is from the U-Boot command prompt with partition specified
> => fatload usb 0:1 0x6400000 uImage
> fpsize = 18
> dpsize = 113
> 0x01 0x04 0x14 0x00 0xb9 0x73 0x1d 0xe6 0x84 0xa3 0xcc 0x4a 0xae 0xab
> 0x82 0xe8 0x28 0xf3 0x62 0x8b 0x03 0x0f 0x0b 0x00 0x00 0x00 0x00 0x00
> 0x09 0x00 0x01 0x03 0x0f 0x0b 0x00 0x40 0x1a 0x01 0x01 0x09 0x00 0x01
> 0x03 0x0f 0x0b 0x00 0xe3 0x05 0x26 0x07 0x00 0x00 0x00 0x04 0x01 0x2a
> 0x00 0x01 0x00 0x00 0x00 0x00 0x08 0x00 0x00 0x00 0x00 0x00 0x00 0x00
> 0xf8 0x01 0x00 0x00 0x00 0x00 0x00 0x99 0x28 0x0f 0x00 0x00 0x00 0x00
> 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x01 0x01 0x04 0x04 0x12
> 0x75 0x00 0x49 0x00 0x6d 0x00 0x61 0x00 0x67 0x00 0x65 0x00 0x00 0x00
> 0x00 0x7f 0xff 0x04 0x00
> efi_dp_split_file_path(/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/UsbClass(0x0,0x0,0x9,0x0,0x1)/UsbClass(0x1a40,0x101,0x9,0x0,0x1)/UsbClass(0x5e3,0x726,0x0,0x0,0x0)/HD(1,MBR,0x000f2899,0x800,0x1f800)/uImage/UNKNOWN(0000,0000)/UNKNOWN(0000,0000)/UNKNOWN(0000,0000)/UNKNOWN(0000,0000)/UNKNOWN(0000,0000)/UNKNOWN(0000,0000)/UNKNOWN(0000,0000)/UNKNOWN(0000,0000)/UNKNOWN(0000,0000)/UNKNOWN(0000,0000)/UNKNOWN(0000,0000)/UNKNOWN(0000,0000)/UNKNOWN(0000,0000)/UNKNOWN(0000,0000)/UNKNOWN(0000,0000)/UNKNOWN(0000,0000)/UNKNOWN(0000,0000)/UNKNOWN(0000,0000))
>
>
> Best regards
>
> Heinrich
>
> >
> >
> > > x_bootcmd_kernel=fatload usb 0 0x6400000 uImage
> > You do not specify a partition number. Do you have a
> partition table?
> > Than the partition defaults to 1. Or does the file system sit
> directly
> > on the device?
> >
> >
> > I also tested other syntactically correct invocations of
> 'fatload' which
> > included the partition number.
> > Execution does not return from efi_set_bootdev().
> >
> >
> > Best regards
> >
> > Heinrich
> >
> >
> > The issue is this commit forces 'fatload' and 'load' to behave
> > identically, it does nothing else.
> > 'git show ee88eacbdd840199a3dec707234579fb15ddd46a'
> > Why would that duplication even be desired ?
> >
> > Further, the current approach of identical behaviour is flawed, the
> > following scenario will fail for all platforms.
> >
> > load scsi 0:2 $kernel_addr_r kernimg
> > fatload scsi 0:1 $script_addr ubscript
> > source $script_addr
> >
> > With this commit reverted the above scenario would work as 'fatload'
> > would not reset the EFI path.
> >
> >
> > Teaching granny to suck eggs....this is where my head is at to
> clarify
> > what I am raising.
> >
> > The 'fatload' command is used to load discrete files from a FAT
> > filesystem into memory.
> > It is not exclusively to do with booting, it is often used to load a
> > script for later sourcing
> > to set variables such as IP's, or kernel command line arguments,
> or even
> > load the kernel
> > itself.
> >
> > 'fatload' has *nothing* to do with EFI, the fact that EFI is
> dependant
> > on a FAT filesystem is a different issue.
> > Invoking EFI code on non-EFI platforms is bad form and is going
> to bite
> > back later again and again.
> >
> > I feel all this confusion has come about over the misnaming of 'load'
> > (which for consistency should have been named 'loadefi').
> >
> >
> > >
> > >> Whilst the proposition 'EFI boot = FAT filesystem' is True
> > >> the converse 'FAT filesystem = EFI boot' is Not True
> > >>
> > >> I am willing to help, but that may require some EFI
> hand-holding.
> > >> Gray
> > >>
> > >> PS. If anyone knows how to set '>' on reply content in
> GMail, please
> > >> email me off list.
> > >>
> > >
> > >
> >
>
More information about the U-Boot
mailing list