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

Heinrich Schuchardt xypron.glpk at gmx.de
Wed Nov 13 07:07:01 UTC 2019


On 11/12/19 11:55 PM, Gray Remlin wrote:
> 
> 
> On Tue, 12 Nov 2019 at 19:50, Heinrich Schuchardt <xypron.glpk at gmx.de 
> <mailto:xypron.glpk at gmx.de>> wrote:
> 
>     On 11/11/19 6:14 PM, Gray Remlin wrote:
>      >
>      > This content is getting very convoluted, if appropriate feel free to
>      > crop it.
>      > New point raised at very bottom.
>      >
>      > On Sun, 10 Nov 2019 at 08:41, 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 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>
>     <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/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>>
>      >     <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
>     <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/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>>>
>      >      >     <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 <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>
>      >     <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 <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> <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
>      >      >      >      >>
>      >      >      >      >>
>      >
>      >
>      > Distribution and version of GCC:
>      >
>      >      >      >      >> 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.
>      >      >      >      >>
>      >      >      >      >
>      >      >      >      >
>      >      >      >
>      >      >
>      >
>      >
>      > Looking at the source:
>      >
>      > struct efi_device_path *efi_dp_from_file(struct blk_desc *desc, int
>      > part, const char *path)
>      > ...
>      > if (desc)
>      >                  buf = dp_part_fill(buf, desc, part);
>      >                  // From this point on 'buf' can now be unaligned
>      > fp = buf;
>      > fp->dp.type = DEVICE_PATH_TYPE_MEDIA_DEVICE;
>      > fp->dp.sub_type = DEVICE_PATH_SUB_TYPE_FILE_PATH;
>      > ....
>      >
>      > Isn,t an unaligned structure an issue on the Kirkwood SoC ?
> 
>     U-Boot starts without support of unaligned data access. In
>     path_to_uefi() we call allow_unaligned() to switch the unaligned access
>     support on.
> 
>     At least my GCC 9.2.1 creates code for __packed structures that avoids
>     unaligned access but that may be different with your compiler.
> 
>     This is why I asked you to tell me which compiler version you are using
>     and to supply the efi_device_tree.o and charset.o files. I really need
>     your support to be able to understand what is happening as I can not
>     reproduce it on my systems. Cf.
>     https://lists.denx.de/pipermail/u-boot/2019-November/389847.html
> 
> 
> I marked the position of the cross compiler details I originally posted 
> above, I expect it was missed in the noise....
> 
>   gcc-linaro-7.4.1-2019.02-x86_64_arm-linux-gnueabi
> 
> be surprised if it was a compiler issue, the rest of U-Boot and the 
> Linux kernel compiles and runs.
> I will email you some broken object files (built from unmodified source) 
> when I have my workstation back up and running properly (thank Fedora 31).
> 
> 
>     I have prepared a patch
>     https://github.com/xypron/u-boot-patches/blob/efi-next/0001-efi_loader-call-allow_unaligned-in-efi_set_bootdev.patch
>     that moves the call to allow_unaligned() to efi_set_bootdev() but I
>     doubt that this will fix your problem.
> 
> I have not made myself clear. When I said:
> "Isn,t an unaligned structure an issue on the Kirkwood SoC ?"
> 
> I mean "I believe unaligned access does not work well with the Kirkwood 
> SoC!"
> This is from what I recall was happening back around 2012, obviously 
> things have changed.
> If so, the code would need to be changed so the pointer to the struct is 
> aligned.

The UEFI spec prescribes that unaligned access must be enabled. You 
cannot expect UEFI applications to use aligned access.

If there is a design bug in the Kirkwood processor such that it does not 
correctly implement the unaligned flag the only option will be to 
disable CONFIG_EFI_LOADER for the boards. Afterwards efi_set_bootdev() 
will not be called anymore.

But couldn't you, please, provide the information requested in the prior 
mail (pointer addresses, compiler version, object files) to elucidate if 
there is a toolchain issue.

Best regards

Heinrich

> 
> Prafulla would be able to clear this up
> 


More information about the U-Boot mailing list