From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefan Roese Date: Sun, 17 Nov 2019 08:50:36 +0100 Subject: [U-Boot] [BUG] U-Boot hangs on fatload, commit ee88eacbdd840199a3dec707234579fb15ddd46a In-Reply-To: References: <469255ff-e461-fafb-8c7a-5e485906f579@gmx.de> <6ec3214e-313f-75e7-6c73-063c2e45ac7e@gmx.de> <4f55b6e9-610c-8e1c-7c4b-f5338a0e956e@gmx.de> <7b21952e-7597-0447-dd94-385ae60d2331@gmx.de> Message-ID: <591cf26d-9bf5-5c4f-6d2e-9278da325bcf@denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit To: u-boot@lists.denx.de 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 >> > 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 >>>     >>>      > >> wrote: >>>      > >>>      >     On 11/9/19 10:31 PM, Gray Remlin wrote: >>>      >      > On Sat, 9 Nov 2019 at 20:50, Heinrich Schuchardt >>>      >      >>>     > >>>      >      > >>>     >>> wrote: >>>      >      > >>>      >      >     On 11/9/19 8:42 PM, Gray Remlin wrote: >>>      >      >      > On Sat, 9 Nov 2019 at 18:40, Heinrich Schuchardt >>>      >      >      >>>     > >>>      >      >>>     >> >>>      >      >      > >>     >>     > >>>      >      >>>     >>>> 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 >>>      >      >      >      >>>     > >>>      >      >>>     >> >>>      >      >      >>>     > >>>      >      >>>     >>> >>>      >      >      >      >> >>     >>>      >     > >>>     >>>      >     >> >>>      >      >      >>>     > >>>      >      >>>     >>>>> 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 >>>      >      >>>      >      >      >>>      >      >      >      >>>      >      >      >      >>     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