[U-Boot] [BUG] U-Boot hangs on fatload, commit ee88eacbdd840199a3dec707234579fb15ddd46a
Chris Packham
judge.packham at gmail.com
Mon Nov 18 07:18:30 UTC 2019
On Sun, Nov 17, 2019 at 8:50 PM Stefan Roese <sr at 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 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