* [U-Boot] [BUG] U-Boot hangs on fatload, commit ee88eacbdd840199a3dec707234579fb15ddd46a [not found] <CACwrecKgrDAoSOrmWmEawFUbj_c1o2j0T=CitkR9+Cn-qAMfTg@mail.gmail.com> @ 2019-11-08 20:08 ` Heinrich Schuchardt 2019-11-09 15:11 ` Gray Remlin 0 siblings, 1 reply; 17+ messages in thread From: Heinrich Schuchardt @ 2019-11-08 20:08 UTC (permalink / raw) To: u-boot 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> 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [BUG] U-Boot hangs on fatload, commit ee88eacbdd840199a3dec707234579fb15ddd46a 2019-11-08 20:08 ` [U-Boot] [BUG] U-Boot hangs on fatload, commit ee88eacbdd840199a3dec707234579fb15ddd46a Heinrich Schuchardt @ 2019-11-09 15:11 ` Gray Remlin 2019-11-09 17:08 ` Heinrich Schuchardt 0 siblings, 1 reply; 17+ messages in thread From: Gray Remlin @ 2019-11-09 15:11 UTC (permalink / raw) To: u-boot On Fri, 8 Nov 2019 at 20:08, Heinrich Schuchardt <xypron.glpk@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> 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 ? 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. ^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [BUG] U-Boot hangs on fatload, commit ee88eacbdd840199a3dec707234579fb15ddd46a 2019-11-09 15:11 ` Gray Remlin @ 2019-11-09 17:08 ` Heinrich Schuchardt 2019-11-09 17:45 ` Gray Remlin 2019-11-09 18:40 ` Heinrich Schuchardt 0 siblings, 2 replies; 17+ messages in thread From: Heinrich Schuchardt @ 2019-11-09 17:08 UTC (permalink / raw) To: u-boot On 11/9/19 4:11 PM, Gray Remlin wrote: > On Fri, 8 Nov 2019 at 20:08, Heinrich Schuchardt <xypron.glpk@gmx.de > <mailto:xypron.glpk@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. 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 > 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. > ^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [BUG] U-Boot hangs on fatload, commit ee88eacbdd840199a3dec707234579fb15ddd46a 2019-11-09 17:08 ` Heinrich Schuchardt @ 2019-11-09 17:45 ` Gray Remlin 2019-11-09 18:40 ` Heinrich Schuchardt 1 sibling, 0 replies; 17+ messages in thread From: Gray Remlin @ 2019-11-09 17:45 UTC (permalink / raw) To: u-boot On Sat, 9 Nov 2019 at 17:08, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > On 11/9/19 4:11 PM, Gray Remlin wrote: > > On Fri, 8 Nov 2019 at 20:08, Heinrich Schuchardt <xypron.glpk@gmx.de > > <mailto:xypron.glpk@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. > Wasn't that the reason for the load wrapper ? 'load' for bootefi and 'fatload' for 'bootz' | 'bootm' > > 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) > > 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 > > > > 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. > > > > ^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [BUG] U-Boot hangs on fatload, commit ee88eacbdd840199a3dec707234579fb15ddd46a 2019-11-09 17:08 ` Heinrich Schuchardt 2019-11-09 17:45 ` Gray Remlin @ 2019-11-09 18:40 ` Heinrich Schuchardt 2019-11-09 19:42 ` Gray Remlin 1 sibling, 1 reply; 17+ messages in thread From: Heinrich Schuchardt @ 2019-11-09 18:40 UTC (permalink / raw) To: u-boot 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@gmx.de >> <mailto:xypron.glpk@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. > > 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. > 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? Best regards Heinrich > >> 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. >> > > ^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [BUG] U-Boot hangs on fatload, commit ee88eacbdd840199a3dec707234579fb15ddd46a 2019-11-09 18:40 ` Heinrich Schuchardt @ 2019-11-09 19:42 ` Gray Remlin 2019-11-09 20:50 ` Heinrich Schuchardt 0 siblings, 1 reply; 17+ messages in thread From: Gray Remlin @ 2019-11-09 19:42 UTC (permalink / raw) To: u-boot On Sat, 9 Nov 2019 at 18:40, Heinrich Schuchardt <xypron.glpk@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@gmx.de > >> <mailto:xypron.glpk@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. > >> > > > > > > ^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [BUG] U-Boot hangs on fatload, commit ee88eacbdd840199a3dec707234579fb15ddd46a 2019-11-09 19:42 ` Gray Remlin @ 2019-11-09 20:50 ` Heinrich Schuchardt 2019-11-09 21:31 ` Gray Remlin 0 siblings, 1 reply; 17+ messages in thread From: Heinrich Schuchardt @ 2019-11-09 20:50 UTC (permalink / raw) To: u-boot On 11/9/19 8:42 PM, Gray Remlin wrote: > On Sat, 9 Nov 2019 at 18:40, Heinrich Schuchardt <xypron.glpk@gmx.de > <mailto:xypron.glpk@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@gmx.de> > >> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@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> 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. 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. 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. > >> > > > > > ^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [BUG] U-Boot hangs on fatload, commit ee88eacbdd840199a3dec707234579fb15ddd46a 2019-11-09 20:50 ` Heinrich Schuchardt @ 2019-11-09 21:31 ` Gray Remlin 2019-11-10 8:41 ` Heinrich Schuchardt 0 siblings, 1 reply; 17+ messages in thread From: Gray Remlin @ 2019-11-09 21:31 UTC (permalink / raw) To: u-boot On Sat, 9 Nov 2019 at 20:50, Heinrich Schuchardt <xypron.glpk@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@gmx.de > > <mailto:xypron.glpk@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@gmx.de> > > >> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@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> 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 ? > > 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 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. > > >> > > > > > > > > > > ^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [BUG] U-Boot hangs on fatload, commit ee88eacbdd840199a3dec707234579fb15ddd46a 2019-11-09 21:31 ` Gray Remlin @ 2019-11-10 8:41 ` Heinrich Schuchardt 2019-11-11 17:14 ` Gray Remlin 0 siblings, 1 reply; 17+ messages in thread From: Heinrich Schuchardt @ 2019-11-10 8:41 UTC (permalink / raw) To: u-boot On 11/9/19 10:31 PM, Gray Remlin wrote: > On Sat, 9 Nov 2019 at 20:50, Heinrich Schuchardt <xypron.glpk@gmx.de > <mailto:xypron.glpk@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@gmx.de> > > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@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@gmx.de> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>> > > >> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@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. > > >> > > > > > > > > > ^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [BUG] U-Boot hangs on fatload, commit ee88eacbdd840199a3dec707234579fb15ddd46a 2019-11-10 8:41 ` Heinrich Schuchardt @ 2019-11-11 17:14 ` Gray Remlin 2019-11-12 19:50 ` Heinrich Schuchardt 0 siblings, 1 reply; 17+ messages in thread From: Gray Remlin @ 2019-11-11 17:14 UTC (permalink / raw) To: u-boot 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@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@gmx.de > > <mailto:xypron.glpk@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@gmx.de> > > > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@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@gmx.de> > > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>> > > > >> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> > > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@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 > > > >> > > > >> > 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 ? ^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [BUG] U-Boot hangs on fatload, commit ee88eacbdd840199a3dec707234579fb15ddd46a 2019-11-11 17:14 ` Gray Remlin @ 2019-11-12 19:50 ` Heinrich Schuchardt 2019-11-12 22:55 ` Gray Remlin 0 siblings, 1 reply; 17+ messages in thread From: Heinrich Schuchardt @ 2019-11-12 19:50 UTC (permalink / raw) To: u-boot 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@gmx.de > <mailto:xypron.glpk@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@gmx.de> > > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@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@gmx.de> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>> > > > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@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@gmx.de> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>> > > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>> > > > >> <mailto:xypron.glpk@gmx.de > <mailto:xypron.glpk@gmx.de> <mailto:xypron.glpk@gmx.de > <mailto:xypron.glpk@gmx.de>> > > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@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> 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 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. Best regards Heinrich ^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [BUG] U-Boot hangs on fatload, commit ee88eacbdd840199a3dec707234579fb15ddd46a 2019-11-12 19:50 ` Heinrich Schuchardt @ 2019-11-12 22:55 ` Gray Remlin 2019-11-13 7:07 ` Heinrich Schuchardt 0 siblings, 1 reply; 17+ messages in thread From: Gray Remlin @ 2019-11-12 22:55 UTC (permalink / raw) To: u-boot On Tue, 12 Nov 2019 at 19:50, Heinrich Schuchardt <xypron.glpk@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@gmx.de > > <mailto:xypron.glpk@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@gmx.de> > > > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@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@gmx.de> > > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>> > > > > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> > > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@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@gmx.de> > > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>> > > > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> > > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>> > > > > >> <mailto:xypron.glpk@gmx.de > > <mailto:xypron.glpk@gmx.de> <mailto:xypron.glpk@gmx.de > > <mailto:xypron.glpk@gmx.de>> > > > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> > > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@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> 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. Prafulla would be able to clear this up > Best regards > > Heinrich > ^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [BUG] U-Boot hangs on fatload, commit ee88eacbdd840199a3dec707234579fb15ddd46a 2019-11-12 22:55 ` Gray Remlin @ 2019-11-13 7:07 ` Heinrich Schuchardt 2019-11-16 10:11 ` Heinrich Schuchardt 0 siblings, 1 reply; 17+ messages in thread From: Heinrich Schuchardt @ 2019-11-13 7:07 UTC (permalink / raw) To: u-boot On 11/12/19 11:55 PM, Gray Remlin wrote: > > > On Tue, 12 Nov 2019 at 19:50, Heinrich Schuchardt <xypron.glpk@gmx.de > <mailto:xypron.glpk@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@gmx.de> > > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@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@gmx.de> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>> > > > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@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@gmx.de> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>> > > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>> > > > > <mailto:xypron.glpk@gmx.de > <mailto:xypron.glpk@gmx.de> <mailto:xypron.glpk@gmx.de > <mailto:xypron.glpk@gmx.de>> > > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@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@gmx.de> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>> > > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>> > > > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>> > > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>>> > > > > >> <mailto:xypron.glpk@gmx.de > <mailto:xypron.glpk@gmx.de> > > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> > > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>> > > > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>> > > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@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 > ^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [BUG] U-Boot hangs on fatload, commit ee88eacbdd840199a3dec707234579fb15ddd46a 2019-11-13 7:07 ` Heinrich Schuchardt @ 2019-11-16 10:11 ` Heinrich Schuchardt 2019-11-17 7:50 ` Stefan Roese 0 siblings, 1 reply; 17+ messages in thread From: Heinrich Schuchardt @ 2019-11-16 10:11 UTC (permalink / raw) To: u-boot Hello Stefan, Gray reporting this bug unfortunately did not provide enough information to analyze his issue. Are you aware of any restrictions of the Kirkwood 88F6281 SoC concerning unaligned access after enabling the unaligned flag? Do you have access to a device with a Kirkwood processor (e.g. Sheevaplug)? One possible solution would be to let CONFIG_EFI_LOADER depend on CONFIG_KIRKWOOD=n. This would concern 36 devices. Best regards Heinrich On 11/13/19 8:07 AM, Heinrich Schuchardt wrote: > On 11/12/19 11:55 PM, Gray Remlin wrote: >> >> >> On Tue, 12 Nov 2019 at 19:50, Heinrich Schuchardt <xypron.glpk@gmx.de >> <mailto:xypron.glpk@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@gmx.de> >> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@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@gmx.de> >> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>> >> > > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> >> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@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@gmx.de> >> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>> >> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> >> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>> >> > > > <mailto:xypron.glpk@gmx.de >> <mailto:xypron.glpk@gmx.de> <mailto:xypron.glpk@gmx.de >> <mailto:xypron.glpk@gmx.de>> >> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> >> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@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@gmx.de> >> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>> >> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> >> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>> >> > > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> >> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>> >> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> >> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>>> >> > > > >> <mailto:xypron.glpk@gmx.de >> <mailto:xypron.glpk@gmx.de> >> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>> >> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> >> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>> >> > > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> >> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>> >> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> >> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@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 >> ^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [BUG] U-Boot hangs on fatload, commit ee88eacbdd840199a3dec707234579fb15ddd46a 2019-11-16 10:11 ` Heinrich Schuchardt @ 2019-11-17 7:50 ` Stefan Roese 2019-11-17 9:42 ` Heinrich Schuchardt 2019-11-18 7:18 ` Chris Packham 0 siblings, 2 replies; 17+ messages in thread From: Stefan Roese @ 2019-11-17 7:50 UTC (permalink / raw) To: u-boot Hi Heinrich, (+Chris) On 16.11.19 11:11, Heinrich Schuchardt wrote: > Hello Stefan, > > Gray reporting this bug unfortunately did not provide enough information > to analyze his issue. > > Are you aware of any restrictions of the Kirkwood 88F6281 SoC concerning > unaligned access after enabling the unaligned flag? > > Do you have access to a device with a Kirkwood processor (e.g. Sheevaplug)? No, unfortunately I don't have have a Kirkwood based board. Chris has access to some boards though. Chris, do you have a chance to comment on this? > One possible solution would be to let CONFIG_EFI_LOADER depend on > CONFIG_KIRKWOOD=n. This would concern 36 devices. That would be very unfortunate. Thanks, Stefan > Best regards > > Heinrich > > On 11/13/19 8:07 AM, Heinrich Schuchardt wrote: >> On 11/12/19 11:55 PM, Gray Remlin wrote: >>> >>> >>> On Tue, 12 Nov 2019 at 19:50, Heinrich Schuchardt <xypron.glpk@gmx.de >>> <mailto:xypron.glpk@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@gmx.de> >>> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@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@gmx.de> >>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>> >>> > > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> >>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@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@gmx.de> >>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>> >>> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> >>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>> >>> > > > <mailto:xypron.glpk@gmx.de >>> <mailto:xypron.glpk@gmx.de> <mailto:xypron.glpk@gmx.de >>> <mailto:xypron.glpk@gmx.de>> >>> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> >>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@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@gmx.de> >>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>> >>> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> >>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>> >>> > > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> >>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>> >>> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> >>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>>> >>> > > > >> <mailto:xypron.glpk@gmx.de >>> <mailto:xypron.glpk@gmx.de> >>> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>> >>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> >>> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>> >>> > > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> >>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>> >>> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> >>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@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 >>> > Viele Grüße, Stefan -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de ^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [BUG] U-Boot hangs on fatload, commit ee88eacbdd840199a3dec707234579fb15ddd46a 2019-11-17 7:50 ` Stefan Roese @ 2019-11-17 9:42 ` Heinrich Schuchardt 2019-11-18 7:18 ` Chris Packham 1 sibling, 0 replies; 17+ messages in thread From: Heinrich Schuchardt @ 2019-11-17 9:42 UTC (permalink / raw) To: u-boot On 11/17/19 8:50 AM, Stefan Roese wrote: > Hi Heinrich, > > (+Chris) > > On 16.11.19 11:11, Heinrich Schuchardt wrote: >> Hello Stefan, >> >> Gray reporting this bug unfortunately did not provide enough information >> to analyze his issue. >> >> Are you aware of any restrictions of the Kirkwood 88F6281 SoC concerning >> unaligned access after enabling the unaligned flag? >> >> Do you have access to a device with a Kirkwood processor (e.g. >> Sheevaplug)? > > No, unfortunately I don't have have a Kirkwood based board. Chris has > access to some boards though. Chris, do you have a chance to comment > on this? > >> One possible solution would be to let CONFIG_EFI_LOADER depend on >> CONFIG_KIRKWOOD=n. This would concern 36 devices. > > That would be very unfortunate. > > Thanks, > Stefan > >> Best regards >> >> Heinrich >> >> On 11/13/19 8:07 AM, Heinrich Schuchardt wrote: >>> On 11/12/19 11:55 PM, Gray Remlin wrote: >>>> >>>> >>>> On Tue, 12 Nov 2019 at 19:50, Heinrich Schuchardt <xypron.glpk@gmx.de >>>> <mailto:xypron.glpk@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@gmx.de> >>>> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@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@gmx.de> >>>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>> >>>> > > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> >>>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@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@gmx.de> >>>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>> >>>> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> >>>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>> >>>> > > > <mailto:xypron.glpk@gmx.de >>>> <mailto:xypron.glpk@gmx.de> <mailto:xypron.glpk@gmx.de >>>> <mailto:xypron.glpk@gmx.de>> >>>> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> >>>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@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@gmx.de >>>> <mailto:xypron.glpk@gmx.de> >>>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>> >>>> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> >>>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>> >>>> > > <mailto:xypron.glpk@gmx.de >>>> <mailto:xypron.glpk@gmx.de> >>>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>> >>>> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> >>>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>>> >>>> > > > >> <mailto:xypron.glpk@gmx.de >>>> <mailto:xypron.glpk@gmx.de> >>>> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>> >>>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> >>>> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>> >>>> > > <mailto:xypron.glpk@gmx.de >>>> <mailto:xypron.glpk@gmx.de> >>>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>> >>>> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> >>>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@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 >>G>> 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 >>>> >> > > Viele Grüße, > Stefan > According to "Marvell 88F6281 SoC with Sheeva Technology" (http://www.swissdutch.ch/Marvell-88F6281.pdf) the 88F6281 CPU of the Sheevaplug is "fully ARMv5TE compliant". ARMv5 does not support unaligned access at all. ARMv6 supports unaligned access when we clear the A flag and set the U flag. With ARMv7 unaligned access is possible when clearing the aligned flag, which we do in function allow_unaligned() (arch/arm/cpu/armv7/sctlr.S). For none of the other cpus in arch/arm/cpu/ we have implemented a similar function. ARMv8 allows unaligned access. UEFI requires unaligned access. So in Kconfig we currently have to restrict EFI_LOADER to only be available for armv7 and armv8 on ARM. config EFI_LOADER bool "Support running UEFI applications" depends on OF_LIBFDT && ( \ ARM && (SYS_CPU = armv7 || \ SYS_CPU = armv8) || \ X86 || RISCV || SANDBOX) Once allow_unaligned() is implemented for other armv6 and armv7 CPUs we can add these to Kconfig. Thanks to Gray for reporting the issue. Best regards Heinrich ^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [BUG] U-Boot hangs on fatload, commit ee88eacbdd840199a3dec707234579fb15ddd46a 2019-11-17 7:50 ` Stefan Roese 2019-11-17 9:42 ` Heinrich Schuchardt @ 2019-11-18 7:18 ` Chris Packham 1 sibling, 0 replies; 17+ messages in thread From: Chris Packham @ 2019-11-18 7:18 UTC (permalink / raw) To: u-boot On Sun, Nov 17, 2019 at 8:50 PM Stefan Roese <sr@denx.de> wrote: > > Hi Heinrich, > > (+Chris) > > On 16.11.19 11:11, Heinrich Schuchardt wrote: > > Hello Stefan, > > > > Gray reporting this bug unfortunately did not provide enough information > > to analyze his issue. > > > > Are you aware of any restrictions of the Kirkwood 88F6281 SoC concerning > > unaligned access after enabling the unaligned flag? > > > > Do you have access to a device with a Kirkwood processor (e.g. Sheevaplug)? > > No, unfortunately I don't have have a Kirkwood based board. Chris has > access to some boards though. Chris, do you have a chance to comment > on this? > I have got a few different Kirkwood boards. But unfortunately none have anything that I could get a fatfs on. I can confirm that unaligned access is just not possible (which I think is an ARMv5TE thing). The Linux kernel does have some logic to fix up unaligned userspace accesses by emulating the read/write[1] but I believe kernel-space accesses are still fatal. I don't know if intercepting these accesses by UEFI apps is something u-boot should be concerned with. [1] - https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/mm/alignment.c#n800 > > One possible solution would be to let CONFIG_EFI_LOADER depend on > > CONFIG_KIRKWOOD=n. This would concern 36 devices. > > That would be very unfortunate. > > Thanks, > Stefan > > > Best regards > > > > Heinrich > > > > On 11/13/19 8:07 AM, Heinrich Schuchardt wrote: > >> On 11/12/19 11:55 PM, Gray Remlin wrote: > >>> > >>> > >>> On Tue, 12 Nov 2019 at 19:50, Heinrich Schuchardt <xypron.glpk@gmx.de > >>> <mailto:xypron.glpk@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@gmx.de> > >>> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@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@gmx.de> > >>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>> > >>> > > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> > >>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@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@gmx.de> > >>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>> > >>> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> > >>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>> > >>> > > > <mailto:xypron.glpk@gmx.de > >>> <mailto:xypron.glpk@gmx.de> <mailto:xypron.glpk@gmx.de > >>> <mailto:xypron.glpk@gmx.de>> > >>> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> > >>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@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@gmx.de> > >>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>> > >>> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> > >>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>> > >>> > > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> > >>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>> > >>> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> > >>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>>> > >>> > > > >> <mailto:xypron.glpk@gmx.de > >>> <mailto:xypron.glpk@gmx.de> > >>> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>> > >>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> > >>> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>>> > >>> > > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> > >>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>> > >>> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de> > >>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk@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 > >>> > > > > Viele Grüße, > Stefan > > -- > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2019-11-18 7:18 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CACwrecKgrDAoSOrmWmEawFUbj_c1o2j0T=CitkR9+Cn-qAMfTg@mail.gmail.com>
2019-11-08 20:08 ` [U-Boot] [BUG] U-Boot hangs on fatload, commit ee88eacbdd840199a3dec707234579fb15ddd46a Heinrich Schuchardt
2019-11-09 15:11 ` Gray Remlin
2019-11-09 17:08 ` Heinrich Schuchardt
2019-11-09 17:45 ` Gray Remlin
2019-11-09 18:40 ` Heinrich Schuchardt
2019-11-09 19:42 ` Gray Remlin
2019-11-09 20:50 ` Heinrich Schuchardt
2019-11-09 21:31 ` Gray Remlin
2019-11-10 8:41 ` Heinrich Schuchardt
2019-11-11 17:14 ` Gray Remlin
2019-11-12 19:50 ` Heinrich Schuchardt
2019-11-12 22:55 ` Gray Remlin
2019-11-13 7:07 ` Heinrich Schuchardt
2019-11-16 10:11 ` Heinrich Schuchardt
2019-11-17 7:50 ` Stefan Roese
2019-11-17 9:42 ` Heinrich Schuchardt
2019-11-18 7:18 ` Chris Packham
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.