From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chris Packham Date: Mon, 18 Nov 2019 20:18:30 +1300 Subject: [U-Boot] [BUG] U-Boot hangs on fatload, commit ee88eacbdd840199a3dec707234579fb15ddd46a In-Reply-To: <591cf26d-9bf5-5c4f-6d2e-9278da325bcf@denx.de> 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> <591cf26d-9bf5-5c4f-6d2e-9278da325bcf@denx.de> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit To: u-boot@lists.denx.de On Sun, Nov 17, 2019 at 8:50 PM 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? > 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 >>> > 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