[U-Boot] [BUG] U-Boot hangs on fatload, commit ee88eacbdd840199a3dec707234579fb15ddd46a

Heinrich Schuchardt xypron.glpk at gmx.de
Sun Nov 17 09:42:59 UTC 2019


On 11/17/19 8:50 AM, 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?
> 
>> 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
>>G>>      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
> 

According to "Marvell 88F6281 SoC with Sheeva Technology" 
(http://www.swissdutch.ch/Marvell-88F6281.pdf) the 88F6281 CPU of the 
Sheevaplug is "fully ARMv5TE compliant".

ARMv5 does not support unaligned access at all.

ARMv6 supports unaligned access when we clear the A flag and set the U flag.

With ARMv7 unaligned access is possible when clearing the aligned flag, 
which we do in function allow_unaligned() (arch/arm/cpu/armv7/sctlr.S).

For none of the other cpus in arch/arm/cpu/ we have implemented a 
similar function.

ARMv8 allows unaligned access.

UEFI requires unaligned access. So in Kconfig we currently have to 
restrict EFI_LOADER to only be available for armv7 and armv8 on ARM.

config EFI_LOADER
         bool "Support running UEFI applications"
         depends on OF_LIBFDT && ( \
                 ARM && (SYS_CPU = armv7 || \
                         SYS_CPU = armv8) || \
                 X86 || RISCV || SANDBOX)

Once allow_unaligned() is implemented for other armv6 and armv7 CPUs we 
can add these to Kconfig.

Thanks to Gray for reporting the issue.

Best regards

Heinrich


More information about the U-Boot mailing list