[U-Boot] [BUG] U-Boot hangs on fatload, commit ee88eacbdd840199a3dec707234579fb15ddd46a
Stefan Roese
sr at denx.de
Sun Nov 17 07:50:36 UTC 2019
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 at gmx.de
>>> <mailto:xypron.glpk at gmx.de>> wrote:
>>>
>>> On 11/11/19 6:14 PM, Gray Remlin wrote:
>>> >
>>> > This content is getting very convoluted, if appropriate feel
>>> free to
>>> > crop it.
>>> > New point raised at very bottom.
>>> >
>>> > On Sun, 10 Nov 2019 at 08:41, Heinrich Schuchardt
>>> <xypron.glpk at gmx.de <mailto:xypron.glpk at gmx.de>
>>> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk at gmx.de>>> wrote:
>>> >
>>> > On 11/9/19 10:31 PM, Gray Remlin wrote:
>>> > > On Sat, 9 Nov 2019 at 20:50, Heinrich Schuchardt
>>> > <xypron.glpk at gmx.de <mailto:xypron.glpk at gmx.de>
>>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk at gmx.de>>
>>> > > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk at gmx.de>
>>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk at gmx.de>>>> wrote:
>>> > >
>>> > > On 11/9/19 8:42 PM, Gray Remlin wrote:
>>> > > > On Sat, 9 Nov 2019 at 18:40, Heinrich Schuchardt
>>> > > <xypron.glpk at gmx.de <mailto:xypron.glpk at gmx.de>
>>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk at gmx.de>>
>>> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk at gmx.de>
>>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk at gmx.de>>>
>>> > > > <mailto:xypron.glpk at gmx.de
>>> <mailto:xypron.glpk at gmx.de> <mailto:xypron.glpk at gmx.de
>>> <mailto:xypron.glpk at gmx.de>>
>>> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk at gmx.de>
>>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk at gmx.de>>>>> wrote:
>>> > > >
>>> > > > On 11/9/19 6:08 PM, Heinrich Schuchardt wrote:
>>> > > > > On 11/9/19 4:11 PM, Gray Remlin wrote:
>>> > > > >> On Fri, 8 Nov 2019 at 20:08, Heinrich
>>> Schuchardt
>>> > > > <xypron.glpk at gmx.de <mailto:xypron.glpk at gmx.de>
>>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk at gmx.de>>
>>> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk at gmx.de>
>>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk at gmx.de>>>
>>> > > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk at gmx.de>
>>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk at gmx.de>>
>>> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk at gmx.de>
>>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk at gmx.de>>>>
>>> > > > >> <mailto:xypron.glpk at gmx.de
>>> <mailto:xypron.glpk at gmx.de>
>>> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk at gmx.de>>
>>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk at gmx.de>
>>> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk at gmx.de>>>
>>> > > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk at gmx.de>
>>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk at gmx.de>>
>>> > <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk at gmx.de>
>>> <mailto:xypron.glpk at gmx.de <mailto:xypron.glpk at gmx.de>>>>>> wrote:
>>> > > > >>
>>> > > > >> On 11/8/19 7:32 PM, Gray Remlin wrote:
>>> > > > >> > Please excuse the noise. I would
>>> like to file a
>>> > > bug report
>>> > > > >> against the
>>> > > > >> > above commit, a quick search of
>>> www.denx.de <http://www.denx.de>
>>> > <http://www.denx.de>
>>> > > <http://www.denx.de>
>>> > > > <http://www.denx.de> <http://www.denx.de>
>>> > > > >> <http://www.denx.de> did not
>>> > > > >> > reveal how I should proceed. Please
>>> point me in
>>> > > the right
>>> > > > >> direction.
>>> > > > >> >
>>> > > > >> >
>>> > > > >> > Issue:
>>> > > > >> > U-Boot hangs (i.e. during boot)
>>> whenever
>>> > the command
>>> > > > 'fatload' is
>>> > > > >> used.
>>> > > > >> >
>>> > > > >> > Details:
>>> > > > >> > U-Boot 2019.10 compiled with either
>>> > > dreamplug_defconfig or
>>> > > > >> > guruplug_defconfig.
>>> > > > >> >
>>> > > > >> > After the commit do_load() now
>>> additionally
>>> > calls
>>> > > > >> efi_set_bootdev()
>>> > > > >> > which was moved out of
>>> do_load_wrapper()
>>> > which is
>>> > > only called
>>> > > > >> by the
>>> > > > >> > 'load' command.
>>> > > > >> >
>>> > > > >> > Reverting the commit fixes this
>>> issue for me.
>>> > > > >> >
>>> > > > >>
>>> > > > >> Dear Gray,
>>> > > > >>
>>> > > > >> thanks for reporting the issue with
>>> commit
>>> > > > >> fs: do_load: pass device path for efi
>>> payload
>>> > > > >> ee88eacbdd840199a3dec707234579fb15ddd46a
>>> > > > >>
>>> > > > >> Is it only the fatload command that
>>> fails on your
>>> > > device or
>>> > > > also the
>>> > > > >> load command?
>>> > > > >>
>>> > > > >> There is no bug tracker for U-Boot. So
>>> sending
>>> > a mail
>>> > > to the
>>> > > > U-Boot
>>> > > > >> mailing list, the patch author, and the
>>> > maintainer is the
>>> > > > best way to
>>> > > > >> inform the developers about bugs.
>>> > > > >>
>>> > > > >> Best regards
>>> > > > >>
>>> > > > >> Heinrich
>>> > > > >>
>>> > > > >>
>>> >
>>> >
>>> > Distribution and version of GCC:
>>> >
>>> > > > >> Additional information:
>>> > > > >> cross-compiler
>>> > > gcc-linaro-7.4.1-2019.02-x86_64_arm-linux-gnueabi
>>> > > > >>
>>> > > > >> The U-Boot environment being used is the
>>> default
>>> > obtained by
>>> > > > >> compiling U-Boot
>>> 2020.01-rc1-00100-gee93ef0c4b as
>>> > > > dreamplug_defconfig
>>> > > > >>
>>> > > > >> => printenv
>>> > > > >> baudrate=115200
>>> > > > >> bootcmd=setenv ethact egiga0;
>>> ${x_bootcmd_ethernet};
>>> > > setenv ethact
>>> > > > >> egiga1; ${x_bootcmd_ethernet};
>>> ${x_bootcmd_usb};
>>> > > > ${x_bootcmd_kernel};
>>> > > > >> setenv bootargs ${x_bootargs}
>>> ${x_bootargs_root};
>>> > bootm
>>> > > 0x6400000;
>>> > > > >> bootdelay=3
>>> > > > >> ethact=egiga0
>>> > > > >> fdtcontroladdr=1fb8e7c8
>>> > > > >> stderr=serial
>>> > > > >> stdin=serial
>>> > > > >> stdout=serial
>>> > > > >> x_bootargs=console=ttyS0,115200
>>> > > > >> x_bootargs_root=root=/dev/sda2 rootdelay=10
>>> > > > >> x_bootcmd_ethernet=ping 192.168.2.1
>>> > > > >> x_bootcmd_kernel=fatload usb 0 0x6400000
>>> uImage
>>> > > > >> x_bootcmd_usb=usb start
>>> > > > >>
>>> > > > >> U-Boot hangs for other syntactically correct
>>> > invocations
>>> > > of either
>>> > > > >> 'fatload' or 'load'
>>> > > > >> Other commands such as 'fatls' function as
>>> expected.
>>> > > > >>
>>> > > > >> Program flow is as follows:
>>> > > > >>
>>> > > > >> command 'fatload' (or 'load')
>>> > > > >> efi_set_bootdev()
>>> > > > >> ...
>>> > > > >> efi_dp_split_file_path()
>>> > > > >> ...
>>> > > > >> efi_dp_dup()
>>> > > > >> ....
>>> > > > >>
>>> efi_dp_size()
>>> > > > >> *while
>>> exit condition
>>> > > never met*
>>> > > > >> *infinite
>>> > loop*
>>> > > > >>
>>> > > > >>
>>> > > > >> This is not an attempted EFI boot, why is
>>> EFI code
>>> > being
>>> > > invoked ?
>>> > > > >
>>> > > > > Thanks for debugging.
>>> > > > >
>>> > > > > When booting from EFI we need to know from
>>> which device
>>> > > the EFI
>>> > > > binary
>>> > > > > was loaded. We use this information to
>>> install the
>>> > loaded
>>> > > image
>>> > > > > protocol. At the time of the load command we
>>> do no
>>> > know if
>>> > > you will
>>> > > > > invoke bootz or bootefi.
>>> > > >
>>> > > >
>>> > > > Isn't that the purpose of the 'load' command ?
>>> > > >
>>> > > > >
>>> > > > > It might be that we have a problem with
>>> creating device
>>> > > paths for
>>> > > > USB. I
>>> > > > > will try to reproduce this.
>>> > > > >
>>> > > > > You could add
>>> > > > >
>>> > > > > printf("efi_dp_split_file_path(%pD)\n",
>>> full_path);
>>> > > > >
>>> > > > > at the beginning of
>>> efi_dp_split_file_path() to
>>> > identify
>>> > > what device
>>> > > > > path is passed to the function. This should
>>> produce an
>>> > > output like
>>> > > > >
>>> > > > > => load scsi 0:2 $kernel_addr_r
>>> description.txt
>>> > > > >
>>> > > >
>>> > >
>>> >
>>> efi_dp_split_file_path(/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Scsi(0,0)/HD(2,MBR,0x6fe3a999,0x400,0x400)/description.txt)
>>>
>>> > > > >
>>> > > > >
>>> > > > > Best regards
>>> > > > >
>>> > > > > Heinrich
>>> > > >
>>> > > > I just tested on an OrangePi PC with v2019.10
>>> and got
>>> > this:
>>> > > >
>>> > > > => fatload usb 0:1 $kernel_addr_r test.txt
>>> > > >
>>> > >
>>> >
>>> efi_dp_split_file_path(/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/UsbClass(0x0,0x0,0x9,0x0,0x1)/UsbClass(0x781,0x5571,0x0,0x0,0x0)/HD(1,MBR,0xfae8c6af,0x800,0x3b9f800)/test.txt)
>>>
>>> > > > device path =
>>> > > >
>>> > >
>>> >
>>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/UsbClass(0x0,0x0,0x9,0x0,0x1)/UsbClass(0x781,0x5571,0x0,0x0,0x0)/HD(1,MBR,0xfae8c6af,0x800,0x3b9f800)
>>>
>>> > > > file path = /test.txt
>>> > > > 12 bytes read in 26 ms (0 Bytes/s)
>>> > > > => md.b $kernel_addr_r 0c
>>> > > > 42000000: 4a 75 73 74 20 61 20 74 65 73 74 0a
>>> Just a
>>> > test.
>>> > > >
>>> > > > So debugging on your specific device is needed.
>>> > > >
>>> > > >
>>> > > > Why do you want to debug EFI code on a device that
>>> does not
>>> > > support EFI ?
>>> > > > I am not reporting a bug with EFI, the issue is
>>> 'fatload'
>>> > is now
>>> > > broken
>>> > > > by this commit.
>>> > > > Once 'fatload' is fixed I am willing to test
>>> U-Boot as
>>> > required for
>>> > > > other bugs whilst the
>>> > > > dreamplug platform is available to me for such.
>>> > >
>>> > > Your system is compiled with EFI_LOADER. So you
>>> could be
>>> > using fatload
>>> > > to load an EFI file. do_fatload() is the only place
>>> where we
>>> > can get the
>>> > > device from which you load the file.
>>> > >
>>> > >
>>> > > No, that is (was before this commit) the purpose of 'load'.
>>> > > I ask again, why do you want two commands that perform
>>> exactly
>>> > the same
>>> > > action ?
>>> > > Is it the intention to first unify them and then discard
>>> one ?
>>> >
>>> > You are right that the commands ext2ls, ext2load, ext4ls,
>>> ext4load,
>>> > ext4save, ext4size, fatls, fatload, fatsave, and fatsize are
>>> rather
>>> > superfluous in the light of ls, load, save, and size. They
>>> are just kept
>>> > for backward compatibility.
>>> >
>>> > >
>>> > >
>>> > >
>>> > > Could you, please, change the end of
>>> efi_dp_from_file() to
>>> > >
>>> > > printf("fpsize = %u\n", fpsize);
>>> > > printf("dpsize = %u\n", dpsize);
>>> > > size_t i;
>>> > > for (i = 0; i < dpsize + sizeof(END); ++i)
>>> > > printf("0x%02x ", ((char
>>> *)start)[i]):;
>>> > > printf("\n");
>>> > >
>>> > > return start;
>>> > >
>>> > > and provide the output.
>>> > >
>>> > > On my system the output is
>>> > >
>>> > > => fatload usb 0 $kernel_addr_r uImage
>>> > > fpsize = 18
>>> > > dpsize = 102
>>> > > 0x01 0x04 0x14 0x00 0xb9 0x73 0x1d 0xe6 0x84 0xa3 0xcc
>>> 0x4a
>>> > 0xae 0xab
>>> > > 0x82 0xe8 0x28 0xf3 0x62 0x8b 0x03 0x0f 0x0b 0x00 0x00
>>> 0x00
>>> > 0x00 0x00
>>> > > 0x09 0x00 0x01 0x03 0x0f 0x0b 0x00 0x81 0x07 0x71 0x55
>>> 0x00
>>> > 0x00 0x00
>>> > > 0x04 0x01 0x2a 0x00 0x01 0x00 0x00 0x00 0x00 0x08 0x00
>>> 0x00
>>> > 0x00 0x00
>>> > > 0x00 0x00 0x00 0x18 0x00 0x00 0x00 0x00 0x00 0x00 0xc3
>>> 0x43
>>> > 0x04 0xa5
>>> > > 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
>>> 0x00
>>> > 0x01 0x01
>>> > > 0x04 0x04 0x12 0x00 0x75 0x00 0x49 0x00 0x6d 0x00 0x61
>>> 0x00
>>> > 0x67 0x00
>>> > > 0x65 0x00 0x00 0x00 0x7f 0xff 0x04 0x00
>>> > >
>>> >
>>> efi_dp_split_file_path('/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/UsbClass(0x0,0x0,0x9,0x0,0x1)/UsbClass(0x781,0x5571,0x0,0x0,0x0)/HD(1,MBR,0xa50443c3,0x800,0x1800)/uImage')
>>>
>>> > > 20 bytes read in 2 ms (9.8 KiB/s)
>>> > > =>
>>> > >
>>> > > 0x7f 0xff 0x04 0x00
>>> > > is the end of device path that seems to be missing for
>>> you.
>>> > >
>>> > >
>>> > > This is the default (from the environment 'fatload'
>>> command)
>>> > > fpsize = 18
>>> > > dpsize = 113
>>> > > 0x01 0x04 0x14 0x00 0xb9 0x73 0x1d 0xe6 0x84 0xa3 0xcc
>>> 0x4a 0xae 0xab
>>> > > 0x82 0xe8 0x28 0xf3 0x62 0x8b 0x03 0x0f 0x0b 0x00 0x00
>>> 0x00 0x00 0x00
>>> > > 0x09 0x00 0x01 0x03 0x0f 0x0b 0x00 0x40 0x1a 0x01 0x01
>>> 0x09 0x00 0x01
>>> > > 0x03 0x0f 0x0b 0x00 0xe3 0x05 0x26 0x07 0x00 0x00 0x00
>>> 0x04 0x01 0x2a
>>> > > 0x00 0x01 0x00 0x00 0x00 0x00 0x08 0x00 0x00 0x00 0x00
>>> 0x00 0x00 0x00
>>> > > 0xf8 0x01 0x00 0x00 0x00 0x00 0x00 0x99 0x28 0x0f 0x00
>>> 0x00 0x00 0x00
>>> > > 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x01 0x01
>>> 0x04 0x04 0x12
>>> > > 0x75 0x00 0x49 0x00 0x6d 0x00 0x61 0x00 0x67 0x00 0x65
>>> 0x00 0x00 0x00
>>> > > 0x00 0x7f 0xff 0x04 0x00
>>> >
>>> > So here "uImage" is copied in one byte left of where we would
>>> expect it
>>> > according to the structure definition and overlaps the
>>> dp->length field.
>>> >
>>> > struct efi_device_path_file_path {
>>> > struct efi_device_path dp;
>>> > u16 str[];
>>> > } __packed;
>>> >
>>> > Could you, please, send me files lib/charset.o and
>>> > lib/efi_loader/efi_device_path.o.
>>> >
>>> > Which distribution and which version of GCC are you using?
>>> >
>>> > Adding the following printf() statements might give some more
>>> insight:
>>> >
>>> > fp->dp.length = fpsize;
>>> > printf("buf = %p\n", buf);
>>> > printf("fp->str = %p\n", fp->str);
>>> > path_to_uefi(fp->str, path);
>>> > buf += fpsize;
>>> >
>>> > Should the above printf() statements have buf + 4 != fp->str:
>>> > What happens when you change the structure to have »u16
>>> str[0];«? (This
>>> > is what was required before the C99 standard. Cf.
>>> > https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html)
>>> >
>>> > Best regards
>>> >
>>> > Heinrich
>>> >
>>> > >
>>> >
>>> efi_dp_split_file_path(/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/UsbClass(0x0,0x0,0x9,0x0,0x1)/UsbClass(0x1a40,0x101,0x9,0x0,0x1)/UsbClass(0x5e3,0x726,0x0,0x0,0x0)/HD(1,MBR,0x000f2899,0x800,0x1f800)/uImage/UNKNOWN(0000,0004)/UNKNOWN(0000,0004)/UNKNOWN(0000,0004)/UNKNOWN(0000,0004)/UNKNOWN(0000,0004)/UNKNOWN(0000,0004)/UNKNOWN(0000,0004)/UNKNOWN(0000,0004)/UNKNOWN(0000,0004)/UNKNOWN(0000,0004)/UNKNOWN(0000,0004)/UNKNOWN(0000,0004)/UNKNOWN(0000,0004)/UNKNOWN(0000,0004)/UNKNOWN(0000,0004)/UNKNOWN(0000,0004)/UNKNOWN(0000,0004)/UNKNOWN(0000,0004))
>>>
>>> > >
>>> > > This is from the U-Boot command prompt with partition
>>> specified
>>> > > => fatload usb 0:1 0x6400000 uImage
>>> > > fpsize = 18
>>> > > dpsize = 113
>>> > > 0x01 0x04 0x14 0x00 0xb9 0x73 0x1d 0xe6 0x84 0xa3 0xcc
>>> 0x4a 0xae 0xab
>>> > > 0x82 0xe8 0x28 0xf3 0x62 0x8b 0x03 0x0f 0x0b 0x00 0x00
>>> 0x00 0x00 0x00
>>> > > 0x09 0x00 0x01 0x03 0x0f 0x0b 0x00 0x40 0x1a 0x01 0x01
>>> 0x09 0x00 0x01
>>> > > 0x03 0x0f 0x0b 0x00 0xe3 0x05 0x26 0x07 0x00 0x00 0x00
>>> 0x04 0x01 0x2a
>>> > > 0x00 0x01 0x00 0x00 0x00 0x00 0x08 0x00 0x00 0x00 0x00
>>> 0x00 0x00 0x00
>>> > > 0xf8 0x01 0x00 0x00 0x00 0x00 0x00 0x99 0x28 0x0f 0x00
>>> 0x00 0x00 0x00
>>> > > 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x01 0x01
>>> 0x04 0x04 0x12
>>> > > 0x75 0x00 0x49 0x00 0x6d 0x00 0x61 0x00 0x67 0x00 0x65
>>> 0x00 0x00 0x00
>>> > > 0x00 0x7f 0xff 0x04 0x00
>>> > >
>>> >
>>> efi_dp_split_file_path(/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/UsbClass(0x0,0x0,0x9,0x0,0x1)/UsbClass(0x1a40,0x101,0x9,0x0,0x1)/UsbClass(0x5e3,0x726,0x0,0x0,0x0)/HD(1,MBR,0x000f2899,0x800,0x1f800)/uImage/UNKNOWN(0000,0000)/UNKNOWN(0000,0000)/UNKNOWN(0000,0000)/UNKNOWN(0000,0000)/UNKNOWN(0000,0000)/UNKNOWN(0000,0000)/UNKNOWN(0000,0000)/UNKNOWN(0000,0000)/UNKNOWN(0000,0000)/UNKNOWN(0000,0000)/UNKNOWN(0000,0000)/UNKNOWN(0000,0000)/UNKNOWN(0000,0000)/UNKNOWN(0000,0000)/UNKNOWN(0000,0000)/UNKNOWN(0000,0000)/UNKNOWN(0000,0000)/UNKNOWN(0000,0000))
>>>
>>> > >
>>> > >
>>> > > Best regards
>>> > >
>>> > > Heinrich
>>> > >
>>> > > >
>>> > > >
>>> > > > > x_bootcmd_kernel=fatload usb 0 0x6400000
>>> uImage
>>> > > > You do not specify a partition number. Do you
>>> have a
>>> > > partition table?
>>> > > > Than the partition defaults to 1. Or does the
>>> file
>>> > system sit
>>> > > directly
>>> > > > on the device?
>>> > > >
>>> > > >
>>> > > > I also tested other syntactically correct
>>> invocations of
>>> > > 'fatload' which
>>> > > > included the partition number.
>>> > > > Execution does not return from efi_set_bootdev().
>>> > > >
>>> > > >
>>> > > > Best regards
>>> > > >
>>> > > > Heinrich
>>> > > >
>>> > > >
>>> > > > The issue is this commit forces 'fatload' and
>>> 'load' to behave
>>> > > > identically, it does nothing else.
>>> > > > 'git show
>>> ee88eacbdd840199a3dec707234579fb15ddd46a'
>>> > > > Why would that duplication even be desired ?
>>> > > >
>>> > > > Further, the current approach of identical
>>> behaviour is
>>> > flawed, the
>>> > > > following scenario will fail for all platforms.
>>> > > >
>>> > > > load scsi 0:2 $kernel_addr_r kernimg
>>> > > > fatload scsi 0:1 $script_addr ubscript
>>> > > > source $script_addr
>>> > > >
>>> > > > With this commit reverted the above scenario would
>>> work as
>>> > 'fatload'
>>> > > > would not reset the EFI path.
>>> > > >
>>> > > >
>>> > > > Teaching granny to suck eggs....this is where my
>>> head is at to
>>> > > clarify
>>> > > > what I am raising.
>>> > > >
>>> > > > The 'fatload' command is used to load discrete
>>> files from
>>> > a FAT
>>> > > > filesystem into memory.
>>> > > > It is not exclusively to do with booting, it is
>>> often used
>>> > to load a
>>> > > > script for later sourcing
>>> > > > to set variables such as IP's, or kernel command
>>> line
>>> > arguments,
>>> > > or even
>>> > > > load the kernel
>>> > > > itself.
>>> > > >
>>> > > > 'fatload' has *nothing* to do with EFI, the fact
>>> that EFI is
>>> > > dependant
>>> > > > on a FAT filesystem is a different issue.
>>> > > > Invoking EFI code on non-EFI platforms is bad form
>>> and is
>>> > going
>>> > > to bite
>>> > > > back later again and again.
>>> > > >
>>> > > > I feel all this confusion has come about over the
>>> > misnaming of 'load'
>>> > > > (which for consistency should have been named
>>> 'loadefi').
>>> > > >
>>> > > >
>>> > > > >
>>> > > > >> Whilst the proposition 'EFI boot = FAT
>>> filesystem'
>>> > is True
>>> > > > >> the converse 'FAT filesystem = EFI boot' is
>>> Not True
>>> > > > >>
>>> > > > >> I am willing to help, but that may require
>>> some EFI
>>> > > hand-holding.
>>> > > > >> Gray
>>> > > > >>
>>> > > > >> PS. If anyone knows how to set '>' on reply
>>> content in
>>> > > GMail, please
>>> > > > >> email me off list.
>>> > > > >>
>>> > > > >
>>> > > > >
>>> > > >
>>> > >
>>> >
>>> >
>>> > Looking at the source:
>>> >
>>> > struct efi_device_path *efi_dp_from_file(struct blk_desc *desc,
>>> int
>>> > part, const char *path)
>>> > ...
>>> > if (desc)
>>> > buf = dp_part_fill(buf, desc, part);
>>> > // From this point on 'buf' can now be unaligned
>>> > fp = buf;
>>> > fp->dp.type = DEVICE_PATH_TYPE_MEDIA_DEVICE;
>>> > fp->dp.sub_type = DEVICE_PATH_SUB_TYPE_FILE_PATH;
>>> > ....
>>> >
>>> > Isn,t an unaligned structure an issue on the Kirkwood SoC ?
>>>
>>> U-Boot starts without support of unaligned data access. In
>>> path_to_uefi() we call allow_unaligned() to switch the unaligned
>>> access
>>> support on.
>>>
>>> At least my GCC 9.2.1 creates code for __packed structures that
>>> avoids
>>> unaligned access but that may be different with your compiler.
>>>
>>> This is why I asked you to tell me which compiler version you are
>>> using
>>> and to supply the efi_device_tree.o and charset.o files. I really
>>> need
>>> your support to be able to understand what is happening as I can not
>>> reproduce it on my systems. Cf.
>>> https://lists.denx.de/pipermail/u-boot/2019-November/389847.html
>>>
>>>
>>> I marked the position of the cross compiler details I originally
>>> posted above, I expect it was missed in the noise....
>>>
>>> gcc-linaro-7.4.1-2019.02-x86_64_arm-linux-gnueabi
>>>
>>> be surprised if it was a compiler issue, the rest of U-Boot and the
>>> Linux kernel compiles and runs.
>>> I will email you some broken object files (built from unmodified
>>> source) when I have my workstation back up and running properly (thank
>>> Fedora 31).
>>>
>>>
>>> I have prepared a patch
>>>
>>> https://github.com/xypron/u-boot-patches/blob/efi-next/0001-efi_loader-call-allow_unaligned-in-efi_set_bootdev.patch
>>>
>>> that moves the call to allow_unaligned() to efi_set_bootdev() but I
>>> doubt that this will fix your problem.
>>>
>>> I have not made myself clear. When I said:
>>> "Isn,t an unaligned structure an issue on the Kirkwood SoC ?"
>>>
>>> I mean "I believe unaligned access does not work well with the
>>> Kirkwood SoC!"
>>> This is from what I recall was happening back around 2012, obviously
>>> things have changed.
>>> If so, the code would need to be changed so the pointer to the struct
>>> is aligned.
>>
>> The UEFI spec prescribes that unaligned access must be enabled. You
>> cannot expect UEFI applications to use aligned access.
>>
>> If there is a design bug in the Kirkwood processor such that it does not
>> correctly implement the unaligned flag the only option will be to
>> disable CONFIG_EFI_LOADER for the boards. Afterwards efi_set_bootdev()
>> will not be called anymore.
>>
>> But couldn't you, please, provide the information requested in the prior
>> mail (pointer addresses, compiler version, object files) to elucidate if
>> there is a toolchain issue.
>>
>> Best regards
>>
>> Heinrich
>>
>>>
>>> Prafulla would be able to clear this up
>>>
>
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
More information about the U-Boot
mailing list