[U-Boot] [BUG] U-Boot hangs on fatload, commit ee88eacbdd840199a3dec707234579fb15ddd46a

Sat Nov 9 19:42:21 UTC 2019

On Sat, 9 Nov 2019 at 18:40, Heinrich Schuchardt <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>> 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> 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.

>
>  > 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.
> >>
> >
> >
>
>