[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