diff mbox

[U-Boot,v0,13/20] efi_loader: use proper device-paths for partitions

Message ID 20170804193205.24669-14-robdclark@gmail.com
State Superseded
Delegated to: Alexander Graf
Headers show

Commit Message

Rob Clark Aug. 4, 2017, 7:31 p.m. UTC
Also, create disk objects for the disk itself, in addition to the
partitions.  (UEFI terminology is a bit confusing, a "disk" object is
really a partition.)  This helps grub properly identify the boot device
since it is trying to match up partition "disk" object with it's parent
device.

Now instead of seeing devices like:

  /File(sdhci@07864000.blk)/EndEntire
  /File(usb_mass_storage.lun0)/EndEntire

You see:

  /ACPI(133741d0,0)/UnknownMessaging(1d)/EndEntire
  /ACPI(133741d0,0)/UnknownMessaging(1d)/HD(0,800,64000,dd904a8c00000000,1,1)/EndEntire
  /ACPI(133741d0,0)/UnknownMessaging(1d)/HD(1,64800,200000,dd904a8c00000000,1,1)/EndEntire
  /ACPI(133741d0,0)/UnknownMessaging(1d)/HD(2,264800,19a000,dd904a8c00000000,1,1)/EndEntire
  /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/EndEntire
  /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(0,800,60000,38ca680200000000,1,1)/EndEntire
  /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(1,61000,155000,38ca680200000000,1,1)/EndEntire
  /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(2,20fa800,1bbf8800,38ca680200000000,1,1)/EndEntire
  /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(3,1b6800,1f44000,38ca680200000000,1,1)/EndEntire

This is on a board with single USB disk and single sd-card.  The
UnknownMessaging(1d) node in the device-path is the MMC device,
but grub_efi_print_device_path() hasn't been updated yet for some
of the newer device-path sub-types.

This patch is inspired by a patch originally from Peter Jones, but
re-worked to use efi_device_path, so it doesn't much resemble the
original.

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 lib/efi_loader/efi_disk.c | 54 +++++++++++++++++++++++++++--------------------
 1 file changed, 31 insertions(+), 23 deletions(-)

Comments

Mark Kettenis Aug. 4, 2017, 8:41 p.m. UTC | #1
> From: Rob Clark <robdclark@gmail.com>
> Date: Fri,  4 Aug 2017 15:31:55 -0400

Hi Rob,

OpenBSD has been an early adopter of efi_loader and pretty much
completely relies on it for booting OpenBSD/armv7 and OpenBSD/arm64.
We use our own bootloader which is fairly lightweight.  Obviously we'd
like to keep it working if this patchset gets adopted.  We don't make
use of EFI variables and don't really plan to make use of them on our
ARM platforms.  But obviously we have to deal with device paths...

> Also, create disk objects for the disk itself, in addition to the
> partitions.  (UEFI terminology is a bit confusing, a "disk" object is
> really a partition.)  This helps grub properly identify the boot device
> since it is trying to match up partition "disk" object with it's parent
> device.
> 
> Now instead of seeing devices like:
> 
>   /File(sdhci@07864000.blk)/EndEntire
>   /File(usb_mass_storage.lun0)/EndEntire
> 
> You see:
> 
>   /ACPI(133741d0,0)/UnknownMessaging(1d)/EndEntire
>   /ACPI(133741d0,0)/UnknownMessaging(1d)/HD(0,800,64000,dd904a8c00000000,1,1)/EndEntire
>   /ACPI(133741d0,0)/UnknownMessaging(1d)/HD(1,64800,200000,dd904a8c00000000,1,1)/EndEntire
>   /ACPI(133741d0,0)/UnknownMessaging(1d)/HD(2,264800,19a000,dd904a8c00000000,1,1)/EndEntire
>   /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/EndEntire
>   /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(0,800,60000,38ca680200000000,1,1)/EndEntire
>   /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(1,61000,155000,38ca680200000000,1,1)/EndEntire
>   /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(2,20fa800,1bbf8800,38ca680200000000,1,1)/EndEntire
>   /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(3,1b6800,1f44000,38ca680200000000,1,1)/EndEntire
> 
> This is on a board with single USB disk and single sd-card.  The
> UnknownMessaging(1d) node in the device-path is the MMC device,
> but grub_efi_print_device_path() hasn't been updated yet for some
> of the newer device-path sub-types.

..and what you're sketching out here should work with recent enough
versions of our bootloader.

However, to me having an ACPI Device Path component in there doesn't
make much sense on a board without ACPI.  It certainly doesn't help
mapping a boot path back to an actual hardware device.  Wouldn't it be
more logical to a Hardware Device Path there instead?  In particular a
Memory Mapped Device Path would make a lot of sense as the start
address would make it fairly easy to do the mapping.

Cheers,

Mark
Rob Clark Aug. 4, 2017, 8:57 p.m. UTC | #2
On Fri, Aug 4, 2017 at 4:41 PM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>> From: Rob Clark <robdclark@gmail.com>
>> Date: Fri,  4 Aug 2017 15:31:55 -0400
>
> Hi Rob,
>
> OpenBSD has been an early adopter of efi_loader and pretty much
> completely relies on it for booting OpenBSD/armv7 and OpenBSD/arm64.
> We use our own bootloader which is fairly lightweight.  Obviously we'd
> like to keep it working if this patchset gets adopted.  We don't make
> use of EFI variables and don't really plan to make use of them on our
> ARM platforms.  But obviously we have to deal with device paths...
>
>> Also, create disk objects for the disk itself, in addition to the
>> partitions.  (UEFI terminology is a bit confusing, a "disk" object is
>> really a partition.)  This helps grub properly identify the boot device
>> since it is trying to match up partition "disk" object with it's parent
>> device.
>>
>> Now instead of seeing devices like:
>>
>>   /File(sdhci@07864000.blk)/EndEntire
>>   /File(usb_mass_storage.lun0)/EndEntire
>>
>> You see:
>>
>>   /ACPI(133741d0,0)/UnknownMessaging(1d)/EndEntire
>>   /ACPI(133741d0,0)/UnknownMessaging(1d)/HD(0,800,64000,dd904a8c00000000,1,1)/EndEntire
>>   /ACPI(133741d0,0)/UnknownMessaging(1d)/HD(1,64800,200000,dd904a8c00000000,1,1)/EndEntire
>>   /ACPI(133741d0,0)/UnknownMessaging(1d)/HD(2,264800,19a000,dd904a8c00000000,1,1)/EndEntire
>>   /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/EndEntire
>>   /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(0,800,60000,38ca680200000000,1,1)/EndEntire
>>   /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(1,61000,155000,38ca680200000000,1,1)/EndEntire
>>   /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(2,20fa800,1bbf8800,38ca680200000000,1,1)/EndEntire
>>   /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(3,1b6800,1f44000,38ca680200000000,1,1)/EndEntire
>>
>> This is on a board with single USB disk and single sd-card.  The
>> UnknownMessaging(1d) node in the device-path is the MMC device,
>> but grub_efi_print_device_path() hasn't been updated yet for some
>> of the newer device-path sub-types.
>
> ..and what you're sketching out here should work with recent enough
> versions of our bootloader.
>
> However, to me having an ACPI Device Path component in there doesn't
> make much sense on a board without ACPI.  It certainly doesn't help
> mapping a boot path back to an actual hardware device.  Wouldn't it be
> more logical to a Hardware Device Path there instead?  In particular a
> Memory Mapped Device Path would make a lot of sense as the start
> address would make it fairly easy to do the mapping.
>

It was pretty much an arbitrary choice, and it wouldn't be hard to
change.  From my reading of the grub code, all it really cares about
is that there is *some* parent of the "partition" HD device.  I'm not
really sure what maps best in a UEFI world to the "soc" node in a
device tree world.  I'm certainly open to changing this.

It would be cool if you have a chance to give this a try w/ OpenBSD
and let me know if you run into issues.  I want this to be something
that works across-distro so I'll try to help as much as I can.  (Not
sure if there is an OpenBSD port for db410c, but I guess there is
always qemu..).  Fwiw, if git pull/cherry-pick is easier than grabbing
patches from list, then [1].

[1] https://github.com/robclark/u-boot/commits/enough-uefi-for-shim

BR,
-R
Mark Kettenis Aug. 5, 2017, 2:01 p.m. UTC | #3
> Authentication-Results: xs4all.nl; spf=pass smtp.mailfrom=gmail.com;
> 	dkim=pass header.d=gmail.com; dmarc=pass header.from=gmail.com
> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
>         d=gmail.com; s=20161025;
>         h=mime-version:in-reply-to:references:from:date:message-id:subject:to
>          :cc;
>         bh=qkk49+XF6+Jn7RUjFU/ZFP7/ho5kdUEUNpSrCpqy9yw=;
>         b=knLDTd1vrl7R3BnRReKrxUD1UxYSIahqh5VTND3PEt1cLHtskGRiDc280ADRTm6ffV
>          izImBjr6hq94Qu/Jnbd6kn7+jSDuhDva9FU1/ZndFaNkTUhsatlgtvrK5RzJ38FpwvLa
>          0X7Y8NuVm99K+ijWdKI34YruaZfz47t863L40UYJhjdaj3/TLdIWrC/NzEBiQ/fXemHU
>          8mN1HM9JBD7STB64mMlDSttSTC2vJOPyX81CCbDnXLI/puqcyXewaJ+kW8Km+VDra2xs
>          PIWVzyA0PoV7nIihMbkrv95K7GQgpSOUUL7nlEmAreQoEbCCb8Iw2inSe6LqhN6IbJ4u
>          Js9A==
> X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
>         d=1e100.net; s=20161025;
>         h=x-gm-message-state:mime-version:in-reply-to:references:from:date
>          :message-id:subject:to:cc;
>         bh=qkk49+XF6+Jn7RUjFU/ZFP7/ho5kdUEUNpSrCpqy9yw=;
>         b=kiBq9lLF93IqOIhjRAbghuqi5rcOJ7mf7OvAvgo8T4qg72k9iEBX84R7yIY7KVyDpX
>          DxV+JJDPePbOtIN3qxpmIZF6vqT/HJ0w+z8RnsUiUGJxGCxTA9R97e+tcB1Ovh7zRIJe
>          JPSP/6/TqRmFgrlIsZdoPZAsypVgTx/AHfdMA/z3l4EOOdSOcd3AOd+sDdGnhCAm2G0+
>          QyAXxkxleKeB3t2AvXKiXpJknXzSb9FxM+0ug0gAPlTTT7d2JFEOZ43gzgLCTTsCVDdL
>          lWiJepjIeynmIuGKwcZJpFdV6C8nx2RMztWLpbhPMrbbR+eLkQcaJnJJrgv859lzbUrM
>          9XhQ==
> X-Gm-Message-State: AHYfb5hmb8q9IgSaZxMt3OsQOUjxd0l+fyjrLiox4JEB/7NaaHlf1K9h
> 	HiWJc8WMqU156stR2hj9qS4oZRxkCuHp0xg=
> X-Received: by 10.25.59.29 with SMTP id i29mr1058615lfa.224.1501880271850;
>  Fri, 04 Aug 2017 13:57:51 -0700 (PDT)
> From: Rob Clark <robdclark@gmail.com>
> Date: Fri, 4 Aug 2017 16:57:50 -0400
> Cc: U-Boot Mailing List <u-boot@lists.denx.de>,
>         Heinrich Schuchardt <xypron.glpk@gmx.de>,
>         Peter Jones <pjones@redhat.com>
> X-CNFS-Analysis: v=2.2 cv=eoad9chX c=1 sm=0 tr=0
> 	a=bdpqe3ZLSLbgpdprsY8WZA==:117 a=IkcTkHD0fZMA:10 a=x7bEGLp0ZPQA:10
> 	a=D2htVsi0J-IA:10 a=KeKAF7QvOSUA:10 a=pGLkceISAAAA:8 a=NEAV23lmAAAA:8
> 	a=YyEjxAJ5dRpCWmjka9UA:9 a=QEXdDO2ut3YA:10 a=6kGIvZw6iX1k4Y-7sg4_:22
> X-Virus-Scanned: by XS4ALL Virus Scanner
> X-XS4ALL-Spam-Score: -0.1 () DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU,
> 	FREEMAIL_FROM, SPF_PASS
> X-XS4ALL-Spam: NO
> Envelope-To: mark.kettenis@xs4all.nl
> 
> On Fri, Aug 4, 2017 at 4:41 PM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> >> From: Rob Clark <robdclark@gmail.com>
> >> Date: Fri,  4 Aug 2017 15:31:55 -0400
> >
> > Hi Rob,
> >
> > OpenBSD has been an early adopter of efi_loader and pretty much
> > completely relies on it for booting OpenBSD/armv7 and OpenBSD/arm64.
> > We use our own bootloader which is fairly lightweight.  Obviously we'd
> > like to keep it working if this patchset gets adopted.  We don't make
> > use of EFI variables and don't really plan to make use of them on our
> > ARM platforms.  But obviously we have to deal with device paths...
> >
> >> Also, create disk objects for the disk itself, in addition to the
> >> partitions.  (UEFI terminology is a bit confusing, a "disk" object is
> >> really a partition.)  This helps grub properly identify the boot device
> >> since it is trying to match up partition "disk" object with it's parent
> >> device.
> >>
> >> Now instead of seeing devices like:
> >>
> >>   /File(sdhci@07864000.blk)/EndEntire
> >>   /File(usb_mass_storage.lun0)/EndEntire
> >>
> >> You see:
> >>
> >>   /ACPI(133741d0,0)/UnknownMessaging(1d)/EndEntire
> >>   /ACPI(133741d0,0)/UnknownMessaging(1d)/HD(0,800,64000,dd904a8c00000000,1,1)/EndEntire
> >>   /ACPI(133741d0,0)/UnknownMessaging(1d)/HD(1,64800,200000,dd904a8c00000000,1,1)/EndEntire
> >>   /ACPI(133741d0,0)/UnknownMessaging(1d)/HD(2,264800,19a000,dd904a8c00000000,1,1)/EndEntire
> >>   /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/EndEntire
> >>   /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(0,800,60000,38ca680200000000,1,1)/EndEntire
> >>   /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(1,61000,155000,38ca680200000000,1,1)/EndEntire
> >>   /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(2,20fa800,1bbf8800,38ca680200000000,1,1)/EndEntire
> >>   /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(3,1b6800,1f44000,38ca680200000000,1,1)/EndEntire
> >>
> >> This is on a board with single USB disk and single sd-card.  The
> >> UnknownMessaging(1d) node in the device-path is the MMC device,
> >> but grub_efi_print_device_path() hasn't been updated yet for some
> >> of the newer device-path sub-types.
> >
> > ..and what you're sketching out here should work with recent enough
> > versions of our bootloader.
> >
> > However, to me having an ACPI Device Path component in there doesn't
> > make much sense on a board without ACPI.  It certainly doesn't help
> > mapping a boot path back to an actual hardware device.  Wouldn't it be
> > more logical to a Hardware Device Path there instead?  In particular a
> > Memory Mapped Device Path would make a lot of sense as the start
> > address would make it fairly easy to do the mapping.
> >
> 
> It was pretty much an arbitrary choice, and it wouldn't be hard to
> change.  From my reading of the grub code, all it really cares about
> is that there is *some* parent of the "partition" HD device.  I'm not
> really sure what maps best in a UEFI world to the "soc" node in a
> device tree world.  I'm certainly open to changing this.
> 
> It would be cool if you have a chance to give this a try w/ OpenBSD
> and let me know if you run into issues.  I want this to be something
> that works across-distro so I'll try to help as much as I can.  (Not
> sure if there is an OpenBSD port for db410c, but I guess there is
> always qemu..).  Fwiw, if git pull/cherry-pick is easier than grabbing
> patches from list, then [1].

OpenBSD doesn't run on the db410c.  However, our EFI bootloader should
just run.  You can download it from:

  http://ftp.openbsd.org/pub/OpenBSD/snapshots/arm64/BOOTAA64.EFI

for the 64-bit (AArch64) and

  http://ftp.openbsd.org/pub/OpenBSD/snapshots/armv7/BOOTARM.EFI

for the 32-bit version (AArch32).

Unfortunately something in this patch series breaks things for me on a
Banana Pi:

U-Boot SPL 2017.09-rc1-00020-g2ad6933c64-dirty (Aug 05 2017 - 15:26:15)
DRAM: 1024 MiB
CPU: 912000000Hz, AXI/AHB/APB: 3/2/2
Trying to boot from MMC1

                                
U-Boot 2017.09-rc1-00020-g2ad6933c64-dirty (Aug 05 2017 - 15:26:15 +0200) Allwinner Technology

CPU:   Allwinner A20 (SUN7I)
Model: LeMaker Banana Pi
I2C:   ready
DRAM:  1 GiB
MMC:   SUNXI SD/MMC: 0
*** Warning - bad CRC, using default environment

Setting up a 720x576i composite-pal console (overscan 32x20)
In:    serial
Out:   vga
Err:   vga
SCSI:  Target spinup took 0 ms.
AHCI 0001.0100 32 slots 1 ports 3 Gbps 0x1 impl SATA mode
flags: ncq stag pm led clo only pmp pio slum part ccc apst
Net:   eth0: ethernet@01c50000
starting USB...
USB0:   USB EHCI 1.00
USB1:   USB OHCI 1.0
USB2:   USB EHCI 1.00
USB3:   USB OHCI 1.0
scanning bus 0 for devices... 1 USB Device(s) found
scanning bus 2 for devices... 1 USB Device(s) found
       scanning usb for storage devices... 0 Storage Device(s) found
Hit any key to stop autoboot:  0
switch to partitions #0, OK
mmc0 is current device
Scanning mmc 0:1...
Found EFI removable media binary efi/boot/bootarm.efi
Scanning disks on scsi...
Scanning disks on usb...
Scanning disks on mmc...
MMC Device 1 not found
MMC Device 2 not found
MMC Device 3 not found
Found 6 disks
data abort
pc : [<7ef8d878>]          lr : [<7ef8d874>]
reloc pc : [<4a039878>]    lr : [<4a039874>]
sp : 7af29660  ip : 00000000     fp : 7af29774
r10: 7efec230  r9 : 7af33ee0     r8 : 00000000
r7 : 00000009  r6 : 7ef9e8b8     r5 : 7af296a0  r4 : 7efa4495
r3 : 7af296a0  r2 : 0000008c     r1 : 7af29658  r0 : 00000004
Flags: nzCV  IRQs off  FIQs off  Mode SVC_32
Resetting CPU ...                        

resetting ...

Normally it would print something like:

  >> OpenBSD/armv7 BOOTARM 0.8
  boot>

at that stage.
Rob Clark Aug. 5, 2017, 2:19 p.m. UTC | #4
On Sat, Aug 5, 2017 at 10:01 AM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>>
>> On Fri, Aug 4, 2017 at 4:41 PM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>> >> From: Rob Clark <robdclark@gmail.com>
>> >> Date: Fri,  4 Aug 2017 15:31:55 -0400
>> >
>> > Hi Rob,
>> >
>> > OpenBSD has been an early adopter of efi_loader and pretty much
>> > completely relies on it for booting OpenBSD/armv7 and OpenBSD/arm64.
>> > We use our own bootloader which is fairly lightweight.  Obviously we'd
>> > like to keep it working if this patchset gets adopted.  We don't make
>> > use of EFI variables and don't really plan to make use of them on our
>> > ARM platforms.  But obviously we have to deal with device paths...
>> >
>> >> Also, create disk objects for the disk itself, in addition to the
>> >> partitions.  (UEFI terminology is a bit confusing, a "disk" object is
>> >> really a partition.)  This helps grub properly identify the boot device
>> >> since it is trying to match up partition "disk" object with it's parent
>> >> device.
>> >>
>> >> Now instead of seeing devices like:
>> >>
>> >>   /File(sdhci@07864000.blk)/EndEntire
>> >>   /File(usb_mass_storage.lun0)/EndEntire
>> >>
>> >> You see:
>> >>
>> >>   /ACPI(133741d0,0)/UnknownMessaging(1d)/EndEntire
>> >>   /ACPI(133741d0,0)/UnknownMessaging(1d)/HD(0,800,64000,dd904a8c00000000,1,1)/EndEntire
>> >>   /ACPI(133741d0,0)/UnknownMessaging(1d)/HD(1,64800,200000,dd904a8c00000000,1,1)/EndEntire
>> >>   /ACPI(133741d0,0)/UnknownMessaging(1d)/HD(2,264800,19a000,dd904a8c00000000,1,1)/EndEntire
>> >>   /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/EndEntire
>> >>   /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(0,800,60000,38ca680200000000,1,1)/EndEntire
>> >>   /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(1,61000,155000,38ca680200000000,1,1)/EndEntire
>> >>   /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(2,20fa800,1bbf8800,38ca680200000000,1,1)/EndEntire
>> >>   /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(3,1b6800,1f44000,38ca680200000000,1,1)/EndEntire
>> >>
>> >> This is on a board with single USB disk and single sd-card.  The
>> >> UnknownMessaging(1d) node in the device-path is the MMC device,
>> >> but grub_efi_print_device_path() hasn't been updated yet for some
>> >> of the newer device-path sub-types.
>> >
>> > ..and what you're sketching out here should work with recent enough
>> > versions of our bootloader.
>> >
>> > However, to me having an ACPI Device Path component in there doesn't
>> > make much sense on a board without ACPI.  It certainly doesn't help
>> > mapping a boot path back to an actual hardware device.  Wouldn't it be
>> > more logical to a Hardware Device Path there instead?  In particular a
>> > Memory Mapped Device Path would make a lot of sense as the start
>> > address would make it fairly easy to do the mapping.
>> >
>>
>> It was pretty much an arbitrary choice, and it wouldn't be hard to
>> change.  From my reading of the grub code, all it really cares about
>> is that there is *some* parent of the "partition" HD device.  I'm not
>> really sure what maps best in a UEFI world to the "soc" node in a
>> device tree world.  I'm certainly open to changing this.
>>
>> It would be cool if you have a chance to give this a try w/ OpenBSD
>> and let me know if you run into issues.  I want this to be something
>> that works across-distro so I'll try to help as much as I can.  (Not
>> sure if there is an OpenBSD port for db410c, but I guess there is
>> always qemu..).  Fwiw, if git pull/cherry-pick is easier than grabbing
>> patches from list, then [1].
>
> OpenBSD doesn't run on the db410c.  However, our EFI bootloader should
> just run.  You can download it from:
>
>   http://ftp.openbsd.org/pub/OpenBSD/snapshots/arm64/BOOTAA64.EFI
>
> for the 64-bit (AArch64) and
>
>   http://ftp.openbsd.org/pub/OpenBSD/snapshots/armv7/BOOTARM.EFI
>
> for the 32-bit version (AArch32).

oh, good point..  I can try that

> Unfortunately something in this patch series breaks things for me on a
> Banana Pi:
>
> U-Boot SPL 2017.09-rc1-00020-g2ad6933c64-dirty (Aug 05 2017 - 15:26:15)
> DRAM: 1024 MiB
> CPU: 912000000Hz, AXI/AHB/APB: 3/2/2
> Trying to boot from MMC1
>
>
> U-Boot 2017.09-rc1-00020-g2ad6933c64-dirty (Aug 05 2017 - 15:26:15 +0200) Allwinner Technology
>
> CPU:   Allwinner A20 (SUN7I)
> Model: LeMaker Banana Pi
> I2C:   ready
> DRAM:  1 GiB
> MMC:   SUNXI SD/MMC: 0
> *** Warning - bad CRC, using default environment
>
> Setting up a 720x576i composite-pal console (overscan 32x20)
> In:    serial
> Out:   vga
> Err:   vga
> SCSI:  Target spinup took 0 ms.
> AHCI 0001.0100 32 slots 1 ports 3 Gbps 0x1 impl SATA mode
> flags: ncq stag pm led clo only pmp pio slum part ccc apst
> Net:   eth0: ethernet@01c50000
> starting USB...
> USB0:   USB EHCI 1.00
> USB1:   USB OHCI 1.0
> USB2:   USB EHCI 1.00
> USB3:   USB OHCI 1.0
> scanning bus 0 for devices... 1 USB Device(s) found
> scanning bus 2 for devices... 1 USB Device(s) found
>        scanning usb for storage devices... 0 Storage Device(s) found
> Hit any key to stop autoboot:  0
> switch to partitions #0, OK
> mmc0 is current device
> Scanning mmc 0:1...
> Found EFI removable media binary efi/boot/bootarm.efi
> Scanning disks on scsi...
> Scanning disks on usb...
> Scanning disks on mmc...
> MMC Device 1 not found
> MMC Device 2 not found
> MMC Device 3 not found
> Found 6 disks
> data abort
> pc : [<7ef8d878>]          lr : [<7ef8d874>]
> reloc pc : [<4a039878>]    lr : [<4a039874>]
> sp : 7af29660  ip : 00000000     fp : 7af29774
> r10: 7efec230  r9 : 7af33ee0     r8 : 00000000
> r7 : 00000009  r6 : 7ef9e8b8     r5 : 7af296a0  r4 : 7efa4495
> r3 : 7af296a0  r2 : 0000008c     r1 : 7af29658  r0 : 00000004
> Flags: nzCV  IRQs off  FIQs off  Mode SVC_32
> Resetting CPU ...
>
> resetting ...
>
> Normally it would print something like:
>
>   >> OpenBSD/armv7 BOOTARM 0.8
>   boot>
>
> at that stage.

hmm, well I'll give a quick try w/ your bootaa64.efi, but I guess this
is probably something board specific.

Could you:

  $(CROSS_COMPILE)-addr2line -e u-boot 7ef8d878

while you still have the build handy?

BR,
-R
Mark Kettenis Aug. 5, 2017, 2:28 p.m. UTC | #5
> Date: Sat, 5 Aug 2017 16:01:51 +0200 (CEST)
> From: Mark Kettenis <mark.kettenis@xs4all.nl>
> 
> Unfortunately something in this patch series breaks things for me on a
> Banana Pi:

And according to git bisect:

4e3e748a50fc3f43e20c7ff407184596d7c9a589 is the first bad commit
commit 4e3e748a50fc3f43e20c7ff407184596d7c9a589
Author: Peter Jones <pjones@redhat.com>
Date:   Wed Jun 21 16:39:02 2017 -0400

    efi: add some more device path structures
    
    Signed-off-by: Peter Jones <pjones@redhat.com>
    Signed-off-by: Rob Clark <robdclark@gmail.com>
Rob Clark Aug. 5, 2017, 2:28 p.m. UTC | #6
On Sat, Aug 5, 2017 at 10:01 AM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>
> OpenBSD doesn't run on the db410c.  However, our EFI bootloader should
> just run.  You can download it from:
>
>   http://ftp.openbsd.org/pub/OpenBSD/snapshots/arm64/BOOTAA64.EFI
>
> for the 64-bit (AArch64) and
>
>   http://ftp.openbsd.org/pub/OpenBSD/snapshots/armv7/BOOTARM.EFI
>
> for the 32-bit version (AArch32).
>

Yup, this appears to work, I think this is about what you'd expect
without having an openbsd fs:

dragonboard410c => load mmc 0:1 ${kernel_addr_r} efi/openbsd/bootaa64.efi
reading efi/openbsd/bootaa64.efi
75732 bytes read in 35 ms (2.1 MiB/s)
dragonboard410c => bootefi ${kernel_addr_r} ${fdt_addr_r}
## Starting EFI application at 81000000 ...
Scanning disk sdhci@07864000.blk...
Found 1 disks
WARNING: Invalid device tree, expect boot to fail
>> OpenBSD/arm64 BOOTAA64 0.6
sd0: getdisklabel: no disk label
open(sd0a:/etc/boot.conf): bad partition
boot>
sd0: getdisklabel: no disk label
cannot open sd0a:/etc/random.seed: bad partition
booting sd0a:/bsd: sd0: getdisklabel: no disk label
open sd0a:/bsd: bad partition
 failed(95). will try /bsd
boot>
sd0: getdisklabel: no disk label
cannot open sd0a:/etc/random.seed: bad partition
booting sd0a:/bsd: sd0: getdisklabel: no disk label
open sd0a:/bsd: bad partition
 failed(95). will try /bsd
Turning timeout off.
boot>
Rob Clark Aug. 5, 2017, 2:35 p.m. UTC | #7
On Sat, Aug 5, 2017 at 10:28 AM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>> Date: Sat, 5 Aug 2017 16:01:51 +0200 (CEST)
>> From: Mark Kettenis <mark.kettenis@xs4all.nl>
>>
>> Unfortunately something in this patch series breaks things for me on a
>> Banana Pi:
>
> And according to git bisect:
>
> 4e3e748a50fc3f43e20c7ff407184596d7c9a589 is the first bad commit
> commit 4e3e748a50fc3f43e20c7ff407184596d7c9a589
> Author: Peter Jones <pjones@redhat.com>
> Date:   Wed Jun 21 16:39:02 2017 -0400
>
>     efi: add some more device path structures
>
>     Signed-off-by: Peter Jones <pjones@redhat.com>
>     Signed-off-by: Rob Clark <robdclark@gmail.com>


hmm, odd.. it is only adding some #define's and structs that are not
used until a later commit..

although it does also make 'struct efi_device_path_mac_addr' __packed,
which it should have been before.  Is this an armv7?  I wonder if we
have some troubles with unaligned accesses on armv7 that we don't have
on aarch64 (or maybe compiler isn't turning access to device-path
nodes into byte accesses if it can't do unaligned accesses.  (The node
in the device-path structure are byte-packed.)

addr2line the faulting address I guess should confirm that.  If this
is the issue, it's going to be a bit sad since we'll have to do a lot
of copying back/forth of efi_device_path ptrs to aligned addresses :-/

BR,
-R
Mark Kettenis Aug. 5, 2017, 3:08 p.m. UTC | #8
> From: Rob Clark <robdclark@gmail.com>
> Date: Sat, 5 Aug 2017 10:35:08 -0400
> 
> On Sat, Aug 5, 2017 at 10:28 AM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> >> Date: Sat, 5 Aug 2017 16:01:51 +0200 (CEST)
> >> From: Mark Kettenis <mark.kettenis@xs4all.nl>
> >>
> >> Unfortunately something in this patch series breaks things for me on a
> >> Banana Pi:
> >
> > And according to git bisect:
> >
> > 4e3e748a50fc3f43e20c7ff407184596d7c9a589 is the first bad commit
> > commit 4e3e748a50fc3f43e20c7ff407184596d7c9a589
> > Author: Peter Jones <pjones@redhat.com>
> > Date:   Wed Jun 21 16:39:02 2017 -0400
> >
> >     efi: add some more device path structures
> >
> >     Signed-off-by: Peter Jones <pjones@redhat.com>
> >     Signed-off-by: Rob Clark <robdclark@gmail.com>
> 
> 
> hmm, odd.. it is only adding some #define's and structs that are not
> used until a later commit..
> 
> although it does also make 'struct efi_device_path_mac_addr' __packed,
> which it should have been before.  Is this an armv7?  I wonder if we
> have some troubles with unaligned accesses on armv7 that we don't have
> on aarch64 (or maybe compiler isn't turning access to device-path
> nodes into byte accesses if it can't do unaligned accesses.  (The node
> in the device-path structure are byte-packed.)

This is indeed armv7.

> addr2line the faulting address I guess should confirm that.  If this
> is the issue, it's going to be a bit sad since we'll have to do a lot
> of copying back/forth of efi_device_path ptrs to aligned addresses :-/

Sadly that's not going to help you:

  $ arm-none-eabi-addr2line -e u-boot 7ef8d878
  ??:0

I suppose it is faulting somewhere in BOOTARM.EFI, 

Anyway, removing __packed from struct efi_device_path_file_path makes
me boot again with a tree checked out out with that commit.

Our bootloader code doesn't explicitly enable alignment faults.  But
of course the UEFI standard says that for AArc32 platforms:

 * Unaligned access should be enabled if supported; Alignment faults
    are enabled otherwise.

So the efi_loader code has to align things properly I fear.
Heinrich Schuchardt Aug. 5, 2017, 3:10 p.m. UTC | #9
On 08/05/2017 04:35 PM, Rob Clark wrote:
> On Sat, Aug 5, 2017 at 10:28 AM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>>> Date: Sat, 5 Aug 2017 16:01:51 +0200 (CEST)
>>> From: Mark Kettenis <mark.kettenis@xs4all.nl>
>>>
>>> Unfortunately something in this patch series breaks things for me on a
>>> Banana Pi:
>>
>> And according to git bisect:
>>
>> 4e3e748a50fc3f43e20c7ff407184596d7c9a589 is the first bad commit
>> commit 4e3e748a50fc3f43e20c7ff407184596d7c9a589
>> Author: Peter Jones <pjones@redhat.com>
>> Date:   Wed Jun 21 16:39:02 2017 -0400
>>
>>     efi: add some more device path structures
>>
>>     Signed-off-by: Peter Jones <pjones@redhat.com>
>>     Signed-off-by: Rob Clark <robdclark@gmail.com>
> 
> 
> hmm, odd.. it is only adding some #define's and structs that are not
> used until a later commit..
> 
> although it does also make 'struct efi_device_path_mac_addr' __packed,
> which it should have been before.  Is this an armv7?  I wonder if we
> have some troubles with unaligned accesses on armv7 that we don't have
> on aarch64 (or maybe compiler isn't turning access to device-path
> nodes into byte accesses if it can't do unaligned accesses.  (The node
> in the device-path structure are byte-packed.)

http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.faqs/ka15414.html

<cite>On older processors, such as ARM9 family based processors, an
unaligned load had to be synthesised in software.  Typically by doing a
series of small accesses, and combining the results. ... Unaligned
access support must be enabled by setting the SCTLR.A bit in the system
control coprocessor</cite>

Generally packing structures is not a good idea performance-wise. The
sequence of fields should be carefully chosen to fill up to powers of
two (2, 4 , 8).

Regards

Heinrich

> 
> addr2line the faulting address I guess should confirm that.  If this
> is the issue, it's going to be a bit sad since we'll have to do a lot
> of copying back/forth of efi_device_path ptrs to aligned addresses :-/
> 
> BR,
> -R
>
Rob Clark Aug. 5, 2017, 3:22 p.m. UTC | #10
On Sat, Aug 5, 2017 at 11:08 AM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>> From: Rob Clark <robdclark@gmail.com>
>> Date: Sat, 5 Aug 2017 10:35:08 -0400
>>
>> On Sat, Aug 5, 2017 at 10:28 AM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>> >> Date: Sat, 5 Aug 2017 16:01:51 +0200 (CEST)
>> >> From: Mark Kettenis <mark.kettenis@xs4all.nl>
>> >>
>> >> Unfortunately something in this patch series breaks things for me on a
>> >> Banana Pi:
>> >
>> > And according to git bisect:
>> >
>> > 4e3e748a50fc3f43e20c7ff407184596d7c9a589 is the first bad commit
>> > commit 4e3e748a50fc3f43e20c7ff407184596d7c9a589
>> > Author: Peter Jones <pjones@redhat.com>
>> > Date:   Wed Jun 21 16:39:02 2017 -0400
>> >
>> >     efi: add some more device path structures
>> >
>> >     Signed-off-by: Peter Jones <pjones@redhat.com>
>> >     Signed-off-by: Rob Clark <robdclark@gmail.com>
>>
>>
>> hmm, odd.. it is only adding some #define's and structs that are not
>> used until a later commit..
>>
>> although it does also make 'struct efi_device_path_mac_addr' __packed,
>> which it should have been before.  Is this an armv7?  I wonder if we
>> have some troubles with unaligned accesses on armv7 that we don't have
>> on aarch64 (or maybe compiler isn't turning access to device-path
>> nodes into byte accesses if it can't do unaligned accesses.  (The node
>> in the device-path structure are byte-packed.)
>
> This is indeed armv7.
>
>> addr2line the faulting address I guess should confirm that.  If this
>> is the issue, it's going to be a bit sad since we'll have to do a lot
>> of copying back/forth of efi_device_path ptrs to aligned addresses :-/
>
> Sadly that's not going to help you:
>
>   $ arm-none-eabi-addr2line -e u-boot 7ef8d878
>   ??:0
>
> I suppose it is faulting somewhere in BOOTARM.EFI,
>
> Anyway, removing __packed from struct efi_device_path_file_path makes
> me boot again with a tree checked out out with that commit.
>
> Our bootloader code doesn't explicitly enable alignment faults.  But
> of course the UEFI standard says that for AArc32 platforms:
>
>  * Unaligned access should be enabled if supported; Alignment faults
>     are enabled otherwise.
>
> So the efi_loader code has to align things properly I fear.

Ok, so I have an idea for a reasonably (imho) sane way forward:

  struct efi_device_path_mac_addr {
      struct efi_device_path dp;
      struct efi_mac_addr mac;
      u8 if_type;
  #ifdef BROKEN_UNALIGNED
      u8 pad[3];
  #endif
  } __packed;

We'll just define BROKEN_UNALIGNED for armv7 and any other arch's that
can't handle unaligned accesses.  Technically it is a bit outside the
way things are *supposed* to work according to my understanding of the
UEFI spec.  But all the code I've seen that parses the device-paths
honors the length field in the efi_device_path header to find the
start of the next node in the path.  I can't guarantee that you'll be
able to boot windows from u-boot (but I guess that isn't what most
people care about ;-)), but it at least won't be more broken than it
was before on these archs.

It will take a bit of extra special handling for
efi_device_path_file_path (which is variable length) but with my
patchset that only gets constructed in one place, so it isn't so bad.

BR,
-R
Rob Clark Aug. 5, 2017, 3:24 p.m. UTC | #11
On Sat, Aug 5, 2017 at 11:10 AM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> On 08/05/2017 04:35 PM, Rob Clark wrote:
>> On Sat, Aug 5, 2017 at 10:28 AM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>>>> Date: Sat, 5 Aug 2017 16:01:51 +0200 (CEST)
>>>> From: Mark Kettenis <mark.kettenis@xs4all.nl>
>>>>
>>>> Unfortunately something in this patch series breaks things for me on a
>>>> Banana Pi:
>>>
>>> And according to git bisect:
>>>
>>> 4e3e748a50fc3f43e20c7ff407184596d7c9a589 is the first bad commit
>>> commit 4e3e748a50fc3f43e20c7ff407184596d7c9a589
>>> Author: Peter Jones <pjones@redhat.com>
>>> Date:   Wed Jun 21 16:39:02 2017 -0400
>>>
>>>     efi: add some more device path structures
>>>
>>>     Signed-off-by: Peter Jones <pjones@redhat.com>
>>>     Signed-off-by: Rob Clark <robdclark@gmail.com>
>>
>>
>> hmm, odd.. it is only adding some #define's and structs that are not
>> used until a later commit..
>>
>> although it does also make 'struct efi_device_path_mac_addr' __packed,
>> which it should have been before.  Is this an armv7?  I wonder if we
>> have some troubles with unaligned accesses on armv7 that we don't have
>> on aarch64 (or maybe compiler isn't turning access to device-path
>> nodes into byte accesses if it can't do unaligned accesses.  (The node
>> in the device-path structure are byte-packed.)
>
> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.faqs/ka15414.html
>
> <cite>On older processors, such as ARM9 family based processors, an
> unaligned load had to be synthesised in software.  Typically by doing a
> series of small accesses, and combining the results. ... Unaligned
> access support must be enabled by setting the SCTLR.A bit in the system
> control coprocessor</cite>
>
> Generally packing structures is not a good idea performance-wise. The
> sequence of fields should be carefully chosen to fill up to powers of
> two (2, 4 , 8).

Yeah, it was clearly a dumb idea for UEFI to not make device-path
nodes word aligned.  But when implementing a standard, we don't have a
choice but to implement it properly, warts and all :-/

BR,
-R


> Regards
>
> Heinrich
>
>>
>> addr2line the faulting address I guess should confirm that.  If this
>> is the issue, it's going to be a bit sad since we'll have to do a lot
>> of copying back/forth of efi_device_path ptrs to aligned addresses :-/
>>
>> BR,
>> -R
>>
>
Rob Clark Aug. 5, 2017, 3:36 p.m. UTC | #12
On Sat, Aug 5, 2017 at 11:24 AM, Rob Clark <robdclark@gmail.com> wrote:
> On Sat, Aug 5, 2017 at 11:10 AM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>> On 08/05/2017 04:35 PM, Rob Clark wrote:
>>> On Sat, Aug 5, 2017 at 10:28 AM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>>>>> Date: Sat, 5 Aug 2017 16:01:51 +0200 (CEST)
>>>>> From: Mark Kettenis <mark.kettenis@xs4all.nl>
>>>>>
>>>>> Unfortunately something in this patch series breaks things for me on a
>>>>> Banana Pi:
>>>>
>>>> And according to git bisect:
>>>>
>>>> 4e3e748a50fc3f43e20c7ff407184596d7c9a589 is the first bad commit
>>>> commit 4e3e748a50fc3f43e20c7ff407184596d7c9a589
>>>> Author: Peter Jones <pjones@redhat.com>
>>>> Date:   Wed Jun 21 16:39:02 2017 -0400
>>>>
>>>>     efi: add some more device path structures
>>>>
>>>>     Signed-off-by: Peter Jones <pjones@redhat.com>
>>>>     Signed-off-by: Rob Clark <robdclark@gmail.com>
>>>
>>>
>>> hmm, odd.. it is only adding some #define's and structs that are not
>>> used until a later commit..
>>>
>>> although it does also make 'struct efi_device_path_mac_addr' __packed,
>>> which it should have been before.  Is this an armv7?  I wonder if we
>>> have some troubles with unaligned accesses on armv7 that we don't have
>>> on aarch64 (or maybe compiler isn't turning access to device-path
>>> nodes into byte accesses if it can't do unaligned accesses.  (The node
>>> in the device-path structure are byte-packed.)
>>
>> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.faqs/ka15414.html
>>
>> <cite>On older processors, such as ARM9 family based processors, an
>> unaligned load had to be synthesised in software.  Typically by doing a
>> series of small accesses, and combining the results. ... Unaligned
>> access support must be enabled by setting the SCTLR.A bit in the system
>> control coprocessor</cite>
>>
>> Generally packing structures is not a good idea performance-wise. The
>> sequence of fields should be carefully chosen to fill up to powers of
>> two (2, 4 , 8).
>
> Yeah, it was clearly a dumb idea for UEFI to not make device-path
> nodes word aligned.  But when implementing a standard, we don't have a
> choice but to implement it properly, warts and all :-/
>

btw, just for reference (if anyone is curious), see sect 10.3.1 in
UEFI spec v2.7.  If you don't want to bother looking it up, here is
the exact wording:

   A Device Path is a series of generic Device Path nodes. The first
   Device Path node starts at byte offset zero of the Device Path.
   The next Device Path node starts at the end of the previous Device
   Path node. Therefore all nodes are byte-packed data structures that
   may appear on any byte boundary. All code references to device path
   notes must assume all fields are unaligned. Since every Device Path
   node contains a length field in a known place, it is possible to
   traverse Device Path nodes that are of an unknown type. There is
   no limit to the number, type, or sequence of nodes in a Device Path.

So clearly what we were doing before was incorrect.. but cheating w/
extra padding bytes on arch's that cannot handle unaligned accesses
will avoid having to go fix grub, bootaa64/shim/fallback (and anything
else that uses gnu-efi), and apparently openbsd's bootaa64.efi.  It is
kinda weird to be using efi on these arch's in the first place, so I
guess I don't feel as badly about the padding bytes hack on those
arch's.  (But we should not use the hack on aarch64.)

BR,
-R
Heinrich Schuchardt Aug. 5, 2017, 4:02 p.m. UTC | #13
On 08/05/2017 05:22 PM, Rob Clark wrote:
> On Sat, Aug 5, 2017 at 11:08 AM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>>> From: Rob Clark <robdclark@gmail.com>
>>> Date: Sat, 5 Aug 2017 10:35:08 -0400
>>>
>>> On Sat, Aug 5, 2017 at 10:28 AM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>>>>> Date: Sat, 5 Aug 2017 16:01:51 +0200 (CEST)
>>>>> From: Mark Kettenis <mark.kettenis@xs4all.nl>
>>>>>
>>>>> Unfortunately something in this patch series breaks things for me on a
>>>>> Banana Pi:
>>>>
>>>> And according to git bisect:
>>>>
>>>> 4e3e748a50fc3f43e20c7ff407184596d7c9a589 is the first bad commit
>>>> commit 4e3e748a50fc3f43e20c7ff407184596d7c9a589
>>>> Author: Peter Jones <pjones@redhat.com>
>>>> Date:   Wed Jun 21 16:39:02 2017 -0400
>>>>
>>>>     efi: add some more device path structures
>>>>
>>>>     Signed-off-by: Peter Jones <pjones@redhat.com>
>>>>     Signed-off-by: Rob Clark <robdclark@gmail.com>
>>>
>>>
>>> hmm, odd.. it is only adding some #define's and structs that are not
>>> used until a later commit..
>>>
>>> although it does also make 'struct efi_device_path_mac_addr' __packed,
>>> which it should have been before.  Is this an armv7?  I wonder if we
>>> have some troubles with unaligned accesses on armv7 that we don't have
>>> on aarch64 (or maybe compiler isn't turning access to device-path
>>> nodes into byte accesses if it can't do unaligned accesses.  (The node
>>> in the device-path structure are byte-packed.)
>>
>> This is indeed armv7.
>>
>>> addr2line the faulting address I guess should confirm that.  If this
>>> is the issue, it's going to be a bit sad since we'll have to do a lot
>>> of copying back/forth of efi_device_path ptrs to aligned addresses :-/
>>
>> Sadly that's not going to help you:
>>
>>   $ arm-none-eabi-addr2line -e u-boot 7ef8d878
>>   ??:0
>>
>> I suppose it is faulting somewhere in BOOTARM.EFI,
>>
>> Anyway, removing __packed from struct efi_device_path_file_path makes
>> me boot again with a tree checked out out with that commit.
>>
>> Our bootloader code doesn't explicitly enable alignment faults.  But
>> of course the UEFI standard says that for AArc32 platforms:
>>
>>  * Unaligned access should be enabled if supported; Alignment faults
>>     are enabled otherwise.
>>
>> So the efi_loader code has to align things properly I fear.
> 
> Ok, so I have an idea for a reasonably (imho) sane way forward:
> 
>   struct efi_device_path_mac_addr {
>       struct efi_device_path dp;
>       struct efi_mac_addr mac;
>       u8 if_type;
>   #ifdef BROKEN_UNALIGNED
>       u8 pad[3];
>   #endif
>   } __packed;

Why do you need _packed here?

These are the current definitions (before your patches):

struct efi_device_path {
        u8 type;
        u8 sub_type;
        u16 length;
};

struct efi_mac_addr {
        u8 addr[32];
};

struct efi_device_path_mac_addr {
        struct efi_device_path dp;
        struct efi_mac_addr mac;
        u8 if_type;
};

Everything is perfectly aligned to natural bounderies.
The only thing that does not conform to the UEFI standard is the length
of efi_mac_addr, which should be 6 for if_type in {0, 1}.

If you want to copy the data to or from unaligned addresses use memcpy.

Best regards

Heinrich

> 
> We'll just define BROKEN_UNALIGNED for armv7 and any other arch's that
> can't handle unaligned accesses.  Technically it is a bit outside the
> way things are *supposed* to work according to my understanding of the
> UEFI spec.  But all the code I've seen that parses the device-paths
> honors the length field in the efi_device_path header to find the
> start of the next node in the path.  I can't guarantee that you'll be
> able to boot windows from u-boot (but I guess that isn't what most
> people care about ;-)), but it at least won't be more broken than it
> was before on these archs.
> 
> It will take a bit of extra special handling for
> efi_device_path_file_path (which is variable length) but with my
> patchset that only gets constructed in one place, so it isn't so bad.
> 
> BR,
> -R
>
Rob Clark Aug. 5, 2017, 4:13 p.m. UTC | #14
On Sat, Aug 5, 2017 at 12:02 PM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> On 08/05/2017 05:22 PM, Rob Clark wrote:
>> On Sat, Aug 5, 2017 at 11:08 AM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>>>> From: Rob Clark <robdclark@gmail.com>
>>>> Date: Sat, 5 Aug 2017 10:35:08 -0400
>>>>
>>>> On Sat, Aug 5, 2017 at 10:28 AM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>>>>>> Date: Sat, 5 Aug 2017 16:01:51 +0200 (CEST)
>>>>>> From: Mark Kettenis <mark.kettenis@xs4all.nl>
>>>>>>
>>>>>> Unfortunately something in this patch series breaks things for me on a
>>>>>> Banana Pi:
>>>>>
>>>>> And according to git bisect:
>>>>>
>>>>> 4e3e748a50fc3f43e20c7ff407184596d7c9a589 is the first bad commit
>>>>> commit 4e3e748a50fc3f43e20c7ff407184596d7c9a589
>>>>> Author: Peter Jones <pjones@redhat.com>
>>>>> Date:   Wed Jun 21 16:39:02 2017 -0400
>>>>>
>>>>>     efi: add some more device path structures
>>>>>
>>>>>     Signed-off-by: Peter Jones <pjones@redhat.com>
>>>>>     Signed-off-by: Rob Clark <robdclark@gmail.com>
>>>>
>>>>
>>>> hmm, odd.. it is only adding some #define's and structs that are not
>>>> used until a later commit..
>>>>
>>>> although it does also make 'struct efi_device_path_mac_addr' __packed,
>>>> which it should have been before.  Is this an armv7?  I wonder if we
>>>> have some troubles with unaligned accesses on armv7 that we don't have
>>>> on aarch64 (or maybe compiler isn't turning access to device-path
>>>> nodes into byte accesses if it can't do unaligned accesses.  (The node
>>>> in the device-path structure are byte-packed.)
>>>
>>> This is indeed armv7.
>>>
>>>> addr2line the faulting address I guess should confirm that.  If this
>>>> is the issue, it's going to be a bit sad since we'll have to do a lot
>>>> of copying back/forth of efi_device_path ptrs to aligned addresses :-/
>>>
>>> Sadly that's not going to help you:
>>>
>>>   $ arm-none-eabi-addr2line -e u-boot 7ef8d878
>>>   ??:0
>>>
>>> I suppose it is faulting somewhere in BOOTARM.EFI,
>>>
>>> Anyway, removing __packed from struct efi_device_path_file_path makes
>>> me boot again with a tree checked out out with that commit.
>>>
>>> Our bootloader code doesn't explicitly enable alignment faults.  But
>>> of course the UEFI standard says that for AArc32 platforms:
>>>
>>>  * Unaligned access should be enabled if supported; Alignment faults
>>>     are enabled otherwise.
>>>
>>> So the efi_loader code has to align things properly I fear.
>>
>> Ok, so I have an idea for a reasonably (imho) sane way forward:
>>
>>   struct efi_device_path_mac_addr {
>>       struct efi_device_path dp;
>>       struct efi_mac_addr mac;
>>       u8 if_type;
>>   #ifdef BROKEN_UNALIGNED
>>       u8 pad[3];
>>   #endif
>>   } __packed;
>
> Why do you need _packed here?

We probably crossed threads, but see the other email I sent that
quoted the relevant part from the UEFI spec.

> These are the current definitions (before your patches):
>
> struct efi_device_path {
>         u8 type;
>         u8 sub_type;
>         u16 length;
> };
>
> struct efi_mac_addr {
>         u8 addr[32];
> };
>
> struct efi_device_path_mac_addr {
>         struct efi_device_path dp;
>         struct efi_mac_addr mac;
>         u8 if_type;
> };
>
> Everything is perfectly aligned to natural bounderies.
> The only thing that does not conform to the UEFI standard is the length
> of efi_mac_addr, which should be 6 for if_type in {0, 1}.

Actually, the mac is fixed size and zero padded, see 10.3.5.11.  The
only thing incorrect here was the missing __packed.

> If you want to copy the data to or from unaligned addresses use memcpy.

The problem isn't *just* u-boot.  We could do that, but it would be
annoying and make the code much more convoluted.  But that doesn't
solve the problem for grub/shim/fallback/etc.

BR,
-R
Mark Kettenis Aug. 6, 2017, 12:53 p.m. UTC | #15
> From: Rob Clark <robdclark@gmail.com>
> Date: Sat, 5 Aug 2017 10:28:34 -0400
> 
> On Sat, Aug 5, 2017 at 10:01 AM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> >
> > OpenBSD doesn't run on the db410c.  However, our EFI bootloader should
> > just run.  You can download it from:
> >
> >   http://ftp.openbsd.org/pub/OpenBSD/snapshots/arm64/BOOTAA64.EFI
> >
> > for the 64-bit (AArch64) and
> >
> >   http://ftp.openbsd.org/pub/OpenBSD/snapshots/armv7/BOOTARM.EFI
> >
> > for the 32-bit version (AArch32).
> >
> 
> Yup, this appears to work, I think this is about what you'd expect
> without having an openbsd fs:
> 
> dragonboard410c => load mmc 0:1 ${kernel_addr_r} efi/openbsd/bootaa64.efi
> reading efi/openbsd/bootaa64.efi
> 75732 bytes read in 35 ms (2.1 MiB/s)
> dragonboard410c => bootefi ${kernel_addr_r} ${fdt_addr_r}
> ## Starting EFI application at 81000000 ...
> Scanning disk sdhci@07864000.blk...
> Found 1 disks
> WARNING: Invalid device tree, expect boot to fail
> >> OpenBSD/arm64 BOOTAA64 0.6
> sd0: getdisklabel: no disk label
> open(sd0a:/etc/boot.conf): bad partition
> boot>
> sd0: getdisklabel: no disk label
> cannot open sd0a:/etc/random.seed: bad partition
> booting sd0a:/bsd: sd0: getdisklabel: no disk label
> open sd0a:/bsd: bad partition
>  failed(95). will try /bsd
> boot>
> sd0: getdisklabel: no disk label
> cannot open sd0a:/etc/random.seed: bad partition
> booting sd0a:/bsd: sd0: getdisklabel: no disk label
> open sd0a:/bsd: bad partition
>  failed(95). will try /bsd
> Turning timeout off.
> boot>

Right.  At that point it is trying to load the OpenBSD kernel from a
UFS/FFS filesystem, which fails because you don't have a BSD disklabel
on your SD/MMC device.  And even if it could, it would be game over
pretty quickly as the OpenBSD kernel doesn't support the UART on your
board (yet).
Mark Kettenis Aug. 6, 2017, 1:16 p.m. UTC | #16
> From: Rob Clark <robdclark@gmail.com>
> Date: Sat, 5 Aug 2017 11:36:25 -0400
> 
> On Sat, Aug 5, 2017 at 11:24 AM, Rob Clark <robdclark@gmail.com> wrote:
> > On Sat, Aug 5, 2017 at 11:10 AM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >> On 08/05/2017 04:35 PM, Rob Clark wrote:
> >>> On Sat, Aug 5, 2017 at 10:28 AM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> >>>>> Date: Sat, 5 Aug 2017 16:01:51 +0200 (CEST)
> >>>>> From: Mark Kettenis <mark.kettenis@xs4all.nl>
> >>>>>
> >>>>> Unfortunately something in this patch series breaks things for me on a
> >>>>> Banana Pi:
> >>>>
> >>>> And according to git bisect:
> >>>>
> >>>> 4e3e748a50fc3f43e20c7ff407184596d7c9a589 is the first bad commit
> >>>> commit 4e3e748a50fc3f43e20c7ff407184596d7c9a589
> >>>> Author: Peter Jones <pjones@redhat.com>
> >>>> Date:   Wed Jun 21 16:39:02 2017 -0400
> >>>>
> >>>>     efi: add some more device path structures
> >>>>
> >>>>     Signed-off-by: Peter Jones <pjones@redhat.com>
> >>>>     Signed-off-by: Rob Clark <robdclark@gmail.com>
> >>>
> >>>
> >>> hmm, odd.. it is only adding some #define's and structs that are not
> >>> used until a later commit..
> >>>
> >>> although it does also make 'struct efi_device_path_mac_addr' __packed,
> >>> which it should have been before.  Is this an armv7?  I wonder if we
> >>> have some troubles with unaligned accesses on armv7 that we don't have
> >>> on aarch64 (or maybe compiler isn't turning access to device-path
> >>> nodes into byte accesses if it can't do unaligned accesses.  (The node
> >>> in the device-path structure are byte-packed.)
> >>
> >> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.faqs/ka15414.html
> >>
> >> <cite>On older processors, such as ARM9 family based processors, an
> >> unaligned load had to be synthesised in software.  Typically by doing a
> >> series of small accesses, and combining the results. ... Unaligned
> >> access support must be enabled by setting the SCTLR.A bit in the system
> >> control coprocessor</cite>
> >>
> >> Generally packing structures is not a good idea performance-wise. The
> >> sequence of fields should be carefully chosen to fill up to powers of
> >> two (2, 4 , 8).
> >
> > Yeah, it was clearly a dumb idea for UEFI to not make device-path
> > nodes word aligned.  But when implementing a standard, we don't have a
> > choice but to implement it properly, warts and all :-/
> >
> 
> btw, just for reference (if anyone is curious), see sect 10.3.1 in
> UEFI spec v2.7.  If you don't want to bother looking it up, here is
> the exact wording:
> 
>    A Device Path is a series of generic Device Path nodes. The first
>    Device Path node starts at byte offset zero of the Device Path.
>    The next Device Path node starts at the end of the previous Device
>    Path node. Therefore all nodes are byte-packed data structures that
>    may appear on any byte boundary. All code references to device path
>    notes must assume all fields are unaligned. Since every Device Path
>    node contains a length field in a known place, it is possible to
>    traverse Device Path nodes that are of an unknown type. There is
>    no limit to the number, type, or sequence of nodes in a Device Path.
> 
> So clearly what we were doing before was incorrect.. but cheating w/
> extra padding bytes on arch's that cannot handle unaligned accesses
> will avoid having to go fix grub, bootaa64/shim/fallback (and anything
> else that uses gnu-efi), and apparently openbsd's bootaa64.efi.  It is
> kinda weird to be using efi on these arch's in the first place, so I
> guess I don't feel as badly about the padding bytes hack on those
> arch's.  (But we should not use the hack on aarch64.)

Looking a bit more closely at the OpenBSD efiboot code, I'm pretty
sure we handle the parsing of device path nodes correctly.  We use an
incarnation of the Intel EFI header files which have:

typedef struct _EFI_DEVICE_PATH {
        UINT8                           Type;
        UINT8                           SubType;
        UINT8                           Length[2];
} EFI_DEVICE_PATH;

#define DevicePathNodeLength(a)     ( ((a)->Length[0]) | ((a)->Length[1] << 8) )

so this is all done using byte access.

Going back to the original crash report:

data abort
pc : [<7ef8d878>]          lr : [<7ef8d874>]
reloc pc : [<4a039878>]    lr : [<4a039874>]
sp : 7af29660  ip : 00000000     fp : 7af29774
r10: 7efec230  r9 : 7af33ee0     r8 : 00000000
r7 : 00000009  r6 : 7ef9e8b8     r5 : 7af296a0  r4 : 7efa4495
r3 : 7af296a0  r2 : 0000008c     r1 : 7af29658  r0 : 00000004
Flags: nzCV  IRQs off  FIQs off  Mode SVC_32

I think it is actually "reloc pc" instead of "pc" that we need to look
at:

$ addr2line -e u-boot 0x4a039874 
/home/kettenis/tmp/rclark/u-boot/include/efi_loader.h:272

which points at the guidstr() function.  That code certainly looks
suspicious.  It will defenitely trigger alignment faults if the guid
isn't 32-bit aligned.

The relevant instruction is a 16-bit load:

  4a039878:       e1d430b4        ldrh    r3, [r4, #4]

and with r4 = 7efa4495 that will defenitely trap.

Looking at the defenition efi_guid_t in u-boot:

  typedef struct {
          u8 b[16];
  } efi_guid_t;

there is no guarantee that GUIDs are properly aligned, so you'll need
to fix the guidstr function introduced in commit
b6d913c6101ba891eb2bcb08a4ee9fc8fb57367.

Things are already broken before that commit though, so there is
another problem.  I'll see if I can figure out what it is...
Rob Clark Aug. 6, 2017, 2:17 p.m. UTC | #17
On Sun, Aug 6, 2017 at 9:16 AM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>> From: Rob Clark <robdclark@gmail.com>
>> Date: Sat, 5 Aug 2017 11:36:25 -0400
>>
>> On Sat, Aug 5, 2017 at 11:24 AM, Rob Clark <robdclark@gmail.com> wrote:
>> > On Sat, Aug 5, 2017 at 11:10 AM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>> >> On 08/05/2017 04:35 PM, Rob Clark wrote:
>> >>> On Sat, Aug 5, 2017 at 10:28 AM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>> >>>>> Date: Sat, 5 Aug 2017 16:01:51 +0200 (CEST)
>> >>>>> From: Mark Kettenis <mark.kettenis@xs4all.nl>
>> >>>>>
>> >>>>> Unfortunately something in this patch series breaks things for me on a
>> >>>>> Banana Pi:
>> >>>>
>> >>>> And according to git bisect:
>> >>>>
>> >>>> 4e3e748a50fc3f43e20c7ff407184596d7c9a589 is the first bad commit
>> >>>> commit 4e3e748a50fc3f43e20c7ff407184596d7c9a589
>> >>>> Author: Peter Jones <pjones@redhat.com>
>> >>>> Date:   Wed Jun 21 16:39:02 2017 -0400
>> >>>>
>> >>>>     efi: add some more device path structures
>> >>>>
>> >>>>     Signed-off-by: Peter Jones <pjones@redhat.com>
>> >>>>     Signed-off-by: Rob Clark <robdclark@gmail.com>
>> >>>
>> >>>
>> >>> hmm, odd.. it is only adding some #define's and structs that are not
>> >>> used until a later commit..
>> >>>
>> >>> although it does also make 'struct efi_device_path_mac_addr' __packed,
>> >>> which it should have been before.  Is this an armv7?  I wonder if we
>> >>> have some troubles with unaligned accesses on armv7 that we don't have
>> >>> on aarch64 (or maybe compiler isn't turning access to device-path
>> >>> nodes into byte accesses if it can't do unaligned accesses.  (The node
>> >>> in the device-path structure are byte-packed.)
>> >>
>> >> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.faqs/ka15414.html
>> >>
>> >> <cite>On older processors, such as ARM9 family based processors, an
>> >> unaligned load had to be synthesised in software.  Typically by doing a
>> >> series of small accesses, and combining the results. ... Unaligned
>> >> access support must be enabled by setting the SCTLR.A bit in the system
>> >> control coprocessor</cite>
>> >>
>> >> Generally packing structures is not a good idea performance-wise. The
>> >> sequence of fields should be carefully chosen to fill up to powers of
>> >> two (2, 4 , 8).
>> >
>> > Yeah, it was clearly a dumb idea for UEFI to not make device-path
>> > nodes word aligned.  But when implementing a standard, we don't have a
>> > choice but to implement it properly, warts and all :-/
>> >
>>
>> btw, just for reference (if anyone is curious), see sect 10.3.1 in
>> UEFI spec v2.7.  If you don't want to bother looking it up, here is
>> the exact wording:
>>
>>    A Device Path is a series of generic Device Path nodes. The first
>>    Device Path node starts at byte offset zero of the Device Path.
>>    The next Device Path node starts at the end of the previous Device
>>    Path node. Therefore all nodes are byte-packed data structures that
>>    may appear on any byte boundary. All code references to device path
>>    notes must assume all fields are unaligned. Since every Device Path
>>    node contains a length field in a known place, it is possible to
>>    traverse Device Path nodes that are of an unknown type. There is
>>    no limit to the number, type, or sequence of nodes in a Device Path.
>>
>> So clearly what we were doing before was incorrect.. but cheating w/
>> extra padding bytes on arch's that cannot handle unaligned accesses
>> will avoid having to go fix grub, bootaa64/shim/fallback (and anything
>> else that uses gnu-efi), and apparently openbsd's bootaa64.efi.  It is
>> kinda weird to be using efi on these arch's in the first place, so I
>> guess I don't feel as badly about the padding bytes hack on those
>> arch's.  (But we should not use the hack on aarch64.)
>
> Looking a bit more closely at the OpenBSD efiboot code, I'm pretty
> sure we handle the parsing of device path nodes correctly.  We use an
> incarnation of the Intel EFI header files which have:
>
> typedef struct _EFI_DEVICE_PATH {
>         UINT8                           Type;
>         UINT8                           SubType;
>         UINT8                           Length[2];
> } EFI_DEVICE_PATH;
>
> #define DevicePathNodeLength(a)     ( ((a)->Length[0]) | ((a)->Length[1] << 8) )
>
> so this is all done using byte access.

Hmm, I assume the OpenBSD efiboot code does look at the payload of
device-path nodes, like HARDDRIVE_DEVICE_PATH.. which does use u32 and
u64 fields (which would be unaligned).  Although that might not be the
problem here.

> Going back to the original crash report:
>
> data abort
> pc : [<7ef8d878>]          lr : [<7ef8d874>]
> reloc pc : [<4a039878>]    lr : [<4a039874>]
> sp : 7af29660  ip : 00000000     fp : 7af29774
> r10: 7efec230  r9 : 7af33ee0     r8 : 00000000
> r7 : 00000009  r6 : 7ef9e8b8     r5 : 7af296a0  r4 : 7efa4495
> r3 : 7af296a0  r2 : 0000008c     r1 : 7af29658  r0 : 00000004
> Flags: nzCV  IRQs off  FIQs off  Mode SVC_32
>
> I think it is actually "reloc pc" instead of "pc" that we need to look
> at:
>
> $ addr2line -e u-boot 0x4a039874
> /home/kettenis/tmp/rclark/u-boot/include/efi_loader.h:272
>
> which points at the guidstr() function.  That code certainly looks
> suspicious.  It will defenitely trigger alignment faults if the guid
> isn't 32-bit aligned.

hmm, interesting.  At least gnu-efi's EFI_GUID uses the same
u32+u16+u16+u8[8] layout.  And iirc so did linux kernel and grub, so
it seemed like u-boot was the odd one out for using u8[16].  Although
maybe we are printing one of our own guid's or openbsd efiboot is also
using u8[16].

Either way I've dropped this patch and instead added %pG support to
vsprintf, using uuid_bin_to_str() which only does byte accesses.

The latest can be found here:

  https://github.com/robclark/u-boot/commits/enough-uefi-for-shim

  https://github.com/robclark/u-boot.git enough-uefi-for-shim


> The relevant instruction is a 16-bit load:
>
>   4a039878:       e1d430b4        ldrh    r3, [r4, #4]
>
> and with r4 = 7efa4495 that will defenitely trap.
>
> Looking at the defenition efi_guid_t in u-boot:
>
>   typedef struct {
>           u8 b[16];
>   } efi_guid_t;
>
> there is no guarantee that GUIDs are properly aligned, so you'll need
> to fix the guidstr function introduced in commit
> b6d913c6101ba891eb2bcb08a4ee9fc8fb57367.
>
> Things are already broken before that commit though, so there is
> another problem.  I'll see if I can figure out what it is...

Thanks

BR,
-R
Rob Clark Aug. 6, 2017, 2:26 p.m. UTC | #18
On Sun, Aug 6, 2017 at 10:17 AM, Rob Clark <robdclark@gmail.com> wrote:
> On Sun, Aug 6, 2017 at 9:16 AM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>>> From: Rob Clark <robdclark@gmail.com>
>>> Date: Sat, 5 Aug 2017 11:36:25 -0400
>>>
>>> On Sat, Aug 5, 2017 at 11:24 AM, Rob Clark <robdclark@gmail.com> wrote:
>>> > On Sat, Aug 5, 2017 at 11:10 AM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>> >> On 08/05/2017 04:35 PM, Rob Clark wrote:
>>> >>> On Sat, Aug 5, 2017 at 10:28 AM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>>> >>>>> Date: Sat, 5 Aug 2017 16:01:51 +0200 (CEST)
>>> >>>>> From: Mark Kettenis <mark.kettenis@xs4all.nl>
>>> >>>>>
>>> >>>>> Unfortunately something in this patch series breaks things for me on a
>>> >>>>> Banana Pi:
>>> >>>>
>>> >>>> And according to git bisect:
>>> >>>>
>>> >>>> 4e3e748a50fc3f43e20c7ff407184596d7c9a589 is the first bad commit
>>> >>>> commit 4e3e748a50fc3f43e20c7ff407184596d7c9a589
>>> >>>> Author: Peter Jones <pjones@redhat.com>
>>> >>>> Date:   Wed Jun 21 16:39:02 2017 -0400
>>> >>>>
>>> >>>>     efi: add some more device path structures
>>> >>>>
>>> >>>>     Signed-off-by: Peter Jones <pjones@redhat.com>
>>> >>>>     Signed-off-by: Rob Clark <robdclark@gmail.com>
>>> >>>
>>> >>>
>>> >>> hmm, odd.. it is only adding some #define's and structs that are not
>>> >>> used until a later commit..
>>> >>>
>>> >>> although it does also make 'struct efi_device_path_mac_addr' __packed,
>>> >>> which it should have been before.  Is this an armv7?  I wonder if we
>>> >>> have some troubles with unaligned accesses on armv7 that we don't have
>>> >>> on aarch64 (or maybe compiler isn't turning access to device-path
>>> >>> nodes into byte accesses if it can't do unaligned accesses.  (The node
>>> >>> in the device-path structure are byte-packed.)
>>> >>
>>> >> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.faqs/ka15414.html
>>> >>
>>> >> <cite>On older processors, such as ARM9 family based processors, an
>>> >> unaligned load had to be synthesised in software.  Typically by doing a
>>> >> series of small accesses, and combining the results. ... Unaligned
>>> >> access support must be enabled by setting the SCTLR.A bit in the system
>>> >> control coprocessor</cite>
>>> >>
>>> >> Generally packing structures is not a good idea performance-wise. The
>>> >> sequence of fields should be carefully chosen to fill up to powers of
>>> >> two (2, 4 , 8).
>>> >
>>> > Yeah, it was clearly a dumb idea for UEFI to not make device-path
>>> > nodes word aligned.  But when implementing a standard, we don't have a
>>> > choice but to implement it properly, warts and all :-/
>>> >
>>>
>>> btw, just for reference (if anyone is curious), see sect 10.3.1 in
>>> UEFI spec v2.7.  If you don't want to bother looking it up, here is
>>> the exact wording:
>>>
>>>    A Device Path is a series of generic Device Path nodes. The first
>>>    Device Path node starts at byte offset zero of the Device Path.
>>>    The next Device Path node starts at the end of the previous Device
>>>    Path node. Therefore all nodes are byte-packed data structures that
>>>    may appear on any byte boundary. All code references to device path
>>>    notes must assume all fields are unaligned. Since every Device Path
>>>    node contains a length field in a known place, it is possible to
>>>    traverse Device Path nodes that are of an unknown type. There is
>>>    no limit to the number, type, or sequence of nodes in a Device Path.
>>>
>>> So clearly what we were doing before was incorrect.. but cheating w/
>>> extra padding bytes on arch's that cannot handle unaligned accesses
>>> will avoid having to go fix grub, bootaa64/shim/fallback (and anything
>>> else that uses gnu-efi), and apparently openbsd's bootaa64.efi.  It is
>>> kinda weird to be using efi on these arch's in the first place, so I
>>> guess I don't feel as badly about the padding bytes hack on those
>>> arch's.  (But we should not use the hack on aarch64.)
>>
>> Looking a bit more closely at the OpenBSD efiboot code, I'm pretty
>> sure we handle the parsing of device path nodes correctly.  We use an
>> incarnation of the Intel EFI header files which have:
>>
>> typedef struct _EFI_DEVICE_PATH {
>>         UINT8                           Type;
>>         UINT8                           SubType;
>>         UINT8                           Length[2];
>> } EFI_DEVICE_PATH;
>>
>> #define DevicePathNodeLength(a)     ( ((a)->Length[0]) | ((a)->Length[1] << 8) )
>>
>> so this is all done using byte access.
>
> Hmm, I assume the OpenBSD efiboot code does look at the payload of
> device-path nodes, like HARDDRIVE_DEVICE_PATH.. which does use u32 and
> u64 fields (which would be unaligned).  Although that might not be the
> problem here.
>
>> Going back to the original crash report:
>>
>> data abort
>> pc : [<7ef8d878>]          lr : [<7ef8d874>]
>> reloc pc : [<4a039878>]    lr : [<4a039874>]
>> sp : 7af29660  ip : 00000000     fp : 7af29774
>> r10: 7efec230  r9 : 7af33ee0     r8 : 00000000
>> r7 : 00000009  r6 : 7ef9e8b8     r5 : 7af296a0  r4 : 7efa4495
>> r3 : 7af296a0  r2 : 0000008c     r1 : 7af29658  r0 : 00000004
>> Flags: nzCV  IRQs off  FIQs off  Mode SVC_32
>>
>> I think it is actually "reloc pc" instead of "pc" that we need to look
>> at:
>>
>> $ addr2line -e u-boot 0x4a039874
>> /home/kettenis/tmp/rclark/u-boot/include/efi_loader.h:272
>>
>> which points at the guidstr() function.  That code certainly looks
>> suspicious.  It will defenitely trigger alignment faults if the guid
>> isn't 32-bit aligned.
>
> hmm, interesting.  At least gnu-efi's EFI_GUID uses the same
> u32+u16+u16+u8[8] layout.  And iirc so did linux kernel and grub, so
> it seemed like u-boot was the odd one out for using u8[16].  Although
> maybe we are printing one of our own guid's or openbsd efiboot is also
> using u8[16].
>
> Either way I've dropped this patch and instead added %pG support to
> vsprintf, using uuid_bin_to_str() which only does byte accesses.
>
> The latest can be found here:
>
>   https://github.com/robclark/u-boot/commits/enough-uefi-for-shim
>
>   https://github.com/robclark/u-boot.git enough-uefi-for-shim
>
>
>> The relevant instruction is a 16-bit load:
>>
>>   4a039878:       e1d430b4        ldrh    r3, [r4, #4]
>>
>> and with r4 = 7efa4495 that will defenitely trap.
>>
>> Looking at the defenition efi_guid_t in u-boot:
>>
>>   typedef struct {
>>           u8 b[16];
>>   } efi_guid_t;
>>
>> there is no guarantee that GUIDs are properly aligned, so you'll need
>> to fix the guidstr function introduced in commit
>> b6d913c6101ba891eb2bcb08a4ee9fc8fb57367.
>>
>> Things are already broken before that commit though, so there is
>> another problem.  I'll see if I can figure out what it is...
>
> Thanks
>

btw, we do have some travis tests that run grub.efi (in qemu) on armv7
and others.. maybe adding OpenBSD's efiboot to the test suit would be
a good idea?   (And eventually shim+fallback.efi after this patchset
is merged..)

BR,
-R
Mark Kettenis Aug. 6, 2017, 2:28 p.m. UTC | #19
> Date: Sun, 6 Aug 2017 15:16:09 +0200 (CEST)
> From: Mark Kettenis <mark.kettenis@xs4all.nl>
> 
> Things are already broken before that commit though, so there is
> another problem.  I'll see if I can figure out what it is...

data abort
pc : [<7ef59160>]          lr : [<7ef59118>]
reloc pc : [<4a003160>]    lr : [<4a003118>]
sp : 7af2b820  ip : 7af69635     fp : 7ef5aee4
r10: 00000005  r9 : 7af35ee0     r8 : 7efb4490
r7 : 7af695e8  r6 : 7af69620     r5 : 0000005c  r4 : 7af2b828
r3 : 7efae477  r2 : 0000005c     r1 : 0000002f  r0 : 00000000
Flags: nzCv  IRQs off  FIQs off  Mode SVC_32

addr2line -e u-boot.bin 0x4a003160
/home/kettenis/tmp/rclark/u-boot/include/efi_loader.h:204

which is the ascii2unicode() function which is used in
efi_disk_add_dev() and indeed does 16-bit stores to potentiaslly
unaligned memory.  And yes, adding __packed to struct
efi_device_file_path will trigger the unaligned access in this case.
Rob Clark Aug. 6, 2017, 2:45 p.m. UTC | #20
On Sun, Aug 6, 2017 at 10:28 AM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>> Date: Sun, 6 Aug 2017 15:16:09 +0200 (CEST)
>> From: Mark Kettenis <mark.kettenis@xs4all.nl>
>>
>> Things are already broken before that commit though, so there is
>> another problem.  I'll see if I can figure out what it is...
>
> data abort
> pc : [<7ef59160>]          lr : [<7ef59118>]
> reloc pc : [<4a003160>]    lr : [<4a003118>]
> sp : 7af2b820  ip : 7af69635     fp : 7ef5aee4
> r10: 00000005  r9 : 7af35ee0     r8 : 7efb4490
> r7 : 7af695e8  r6 : 7af69620     r5 : 0000005c  r4 : 7af2b828
> r3 : 7efae477  r2 : 0000005c     r1 : 0000002f  r0 : 00000000
> Flags: nzCv  IRQs off  FIQs off  Mode SVC_32
>
> addr2line -e u-boot.bin 0x4a003160
> /home/kettenis/tmp/rclark/u-boot/include/efi_loader.h:204
>
> which is the ascii2unicode() function which is used in
> efi_disk_add_dev() and indeed does 16-bit stores to potentiaslly
> unaligned memory.  And yes, adding __packed to struct
> efi_device_file_path will trigger the unaligned access in this case.

Hmm, I could see that.  Have you had a chance to try with "efi_loader:
hack for archs that cannot do unaligned accesses"?  (That patch should
probably be squashed back in to various earlier patches, but I figured
keeping it separate for now would be easier to review.)
ascii2unicode() is probably only the first place that would hit
unaligned accesses..

But that all said, [1] seems to imply armv7 *can* do unaligned
accesses.  So maybe this is a banana-pi specific issue.  Maybe some
cp15 bit not set correctly?  (Otherwise I think I should have it this
issue in travis with tests that load grub.efi on various qemu
platforms.)


[1] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.faqs/ka15414.html

I've started trying to hack up test_efi_loader.py to add a test that
loads OpenBSD's bootloader..  kinda muddling through it at this point,
since not a py expert or too familiar w/ u-boot's test framework.  But
I'll see if I can get to the point where I can run the same thing on
various arm7 and aarch64 devices in qemu.

BR,
-R
Rob Clark Aug. 6, 2017, 3:34 p.m. UTC | #21
On Sun, Aug 6, 2017 at 10:45 AM, Rob Clark <robdclark@gmail.com> wrote:
>
> I've started trying to hack up test_efi_loader.py to add a test that
> loads OpenBSD's bootloader..  kinda muddling through it at this point,
> since not a py expert or too familiar w/ u-boot's test framework.  But
> I'll see if I can get to the point where I can run the same thing on
> various arm7 and aarch64 devices in qemu.
>

Making a bit of progress on this (running it on a vexpress_ca15_tc2
board in qemu).. any hint where I can find BOOTARM.EFI src code?

=> tftpboot 80400000 obsdboot.efi
smc911x: MAC 52:54:00:12:34:56
smc911x: detected LAN9118 controller
smc911x: phy initialized
smc911x: MAC 52:54:00:12:34:56
Using smc911x-0 device
TFTP from server 10.0.2.2; our IP address is 10.0.2.15
Filename 'obsdboot.efi'.
Load address: 0x80400000
Loading: *%08#####
12.4 MiB/s
done
Bytes transferred = 64908 (fd8c hex)
smc911x: MAC 52:54:00:12:34:56
=> crc32 80400000 $filesize
CRC32 for 80400000 ... 8040fd8b ==> a9ac4fcf
=> bootefi 80400000
## Starting EFI application at 80400000 ...
WARNING: Invalid device tree, expect boot to fail
BS->LocateHandle() returns 0
undefined instruction
pc : [<9eec65c4>]   lr : [<9eeca390>]
sp : 9fed7a18  ip : 0000003f fp : 9fed7a2c
r10: 00000000  r9 : 9eed4658 r8 : 00000000
r7 : 9eed1ce4  r6 : 9eed3538 r5 : 9fed7a6c  r4 : 9eed4658
r3 : 00000000  r2 : 9eed2f8e r1 : 9eed1ee0  r0 : 00000000
Flags: nZCv  IRQs off  FIQs off  Mode SVC_32
Resetting CPU ...

resetting ...


U-Boot 2017.09-rc1-00025-g534695d189 (Aug 06 2017 - 06:58:16 -0400)

DRAM:  1 GiB
WARNING: Caches not enabled
Flash: 128 MiB
MMC:   MMC: 0
*** Warning - bad CRC, using default environment

In:    serial
Out:   serial
Err:   serial
Net:   smc911x-0
Hit any key to stop autoboot:  2
Heinrich Schuchardt Aug. 6, 2017, 4 p.m. UTC | #22
On 08/06/2017 05:34 PM, Rob Clark wrote:
> On Sun, Aug 6, 2017 at 10:45 AM, Rob Clark <robdclark@gmail.com> wrote:
>>
>> I've started trying to hack up test_efi_loader.py to add a test that
>> loads OpenBSD's bootloader..  kinda muddling through it at this point,
>> since not a py expert or too familiar w/ u-boot's test framework.  But
>> I'll see if I can get to the point where I can run the same thing on
>> various arm7 and aarch64 devices in qemu.
>>
> 
> Making a bit of progress on this (running it on a vexpress_ca15_tc2
> board in qemu).. any hint where I can find BOOTARM.EFI src code?

On Debian arm64 the following commands create bootaa64.efi.

sudo apt-get install grub-efi-arm64
sudo update-grub
sudo grub-install --target=arm64-efi --boot-directory=/boot
--efi-directory=/EFI  {/EFI is my mounted FAT partition}

I guess you can do the same on armhf to create bootarm.efi

Regards

Heinrich


> 
> => tftpboot 80400000 obsdboot.efi
> smc911x: MAC 52:54:00:12:34:56
> smc911x: detected LAN9118 controller
> smc911x: phy initialized
> smc911x: MAC 52:54:00:12:34:56
> Using smc911x-0 device
> TFTP from server 10.0.2.2; our IP address is 10.0.2.15
> Filename 'obsdboot.efi'.
> Load address: 0x80400000
> Loading: *%08#####
> 12.4 MiB/s
> done
> Bytes transferred = 64908 (fd8c hex)
> smc911x: MAC 52:54:00:12:34:56
> => crc32 80400000 $filesize
> CRC32 for 80400000 ... 8040fd8b ==> a9ac4fcf
> => bootefi 80400000
> ## Starting EFI application at 80400000 ...
> WARNING: Invalid device tree, expect boot to fail
> BS->LocateHandle() returns 0
> undefined instruction
> pc : [<9eec65c4>]   lr : [<9eeca390>]
> sp : 9fed7a18  ip : 0000003f fp : 9fed7a2c
> r10: 00000000  r9 : 9eed4658 r8 : 00000000
> r7 : 9eed1ce4  r6 : 9eed3538 r5 : 9fed7a6c  r4 : 9eed4658
> r3 : 00000000  r2 : 9eed2f8e r1 : 9eed1ee0  r0 : 00000000
> Flags: nZCv  IRQs off  FIQs off  Mode SVC_32
> Resetting CPU ...
> 
> resetting ...
> 
> 
> U-Boot 2017.09-rc1-00025-g534695d189 (Aug 06 2017 - 06:58:16 -0400)
> 
> DRAM:  1 GiB
> WARNING: Caches not enabled
> Flash: 128 MiB
> MMC:   MMC: 0
> *** Warning - bad CRC, using default environment
> 
> In:    serial
> Out:   serial
> Err:   serial
> Net:   smc911x-0
> Hit any key to stop autoboot:  2
>
Jonathan Gray Aug. 6, 2017, 4:14 p.m. UTC | #23
On Sun, Aug 06, 2017 at 11:34:15AM -0400, Rob Clark wrote:
> On Sun, Aug 6, 2017 at 10:45 AM, Rob Clark <robdclark@gmail.com> wrote:
> >
> > I've started trying to hack up test_efi_loader.py to add a test that
> > loads OpenBSD's bootloader..  kinda muddling through it at this point,
> > since not a py expert or too familiar w/ u-boot's test framework.  But
> > I'll see if I can get to the point where I can run the same thing on
> > various arm7 and aarch64 devices in qemu.
> >
> 
> Making a bit of progress on this (running it on a vexpress_ca15_tc2
> board in qemu).. any hint where I can find BOOTARM.EFI src code?

https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/arch/armv7/stand/efiboot/
https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/stand/efi/include/

https://www.openbsd.org/anoncvs.html
Mark Kettenis Aug. 6, 2017, 5:28 p.m. UTC | #24
> From: Rob Clark <robdclark@gmail.com>
> Date: Sun, 6 Aug 2017 11:34:15 -0400
> 
> On Sun, Aug 6, 2017 at 10:45 AM, Rob Clark <robdclark@gmail.com> wrote:
> >
> > I've started trying to hack up test_efi_loader.py to add a test that
> > loads OpenBSD's bootloader..  kinda muddling through it at this point,
> > since not a py expert or too familiar w/ u-boot's test framework.  But
> > I'll see if I can get to the point where I can run the same thing on
> > various arm7 and aarch64 devices in qemu.
> >
> 
> Making a bit of progress on this (running it on a vexpress_ca15_tc2
> board in qemu).. any hint where I can find BOOTARM.EFI src code?

https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/arch/armv7/stand/efiboot/efiboot.c?rev=1.17&content-type=text/x-cvsweb-markup

Your failure below looks a bit different from what I'm getting on the
Banana Pi now.  There I get stuck because the 2nd BS->HandleProtocol()
call in efi_main() fails.  Somehow the device path of the registered
disk devices isn't matched correctly to the boot device path...

BTW, the OpenBSD code runs fine without the alignment hack.  Our code
is pretty minimal and doesn't actualy look into the device path
components.  It just matches the components based on type and by soing
memcmp.

> => tftpboot 80400000 obsdboot.efi
> smc911x: MAC 52:54:00:12:34:56
> smc911x: detected LAN9118 controller
> smc911x: phy initialized
> smc911x: MAC 52:54:00:12:34:56
> Using smc911x-0 device
> TFTP from server 10.0.2.2; our IP address is 10.0.2.15
> Filename 'obsdboot.efi'.
> Load address: 0x80400000
> Loading: *%08#####
> 12.4 MiB/s
> done
> Bytes transferred = 64908 (fd8c hex)
> smc911x: MAC 52:54:00:12:34:56
> => crc32 80400000 $filesize
> CRC32 for 80400000 ... 8040fd8b ==> a9ac4fcf
> => bootefi 80400000
> ## Starting EFI application at 80400000 ...
> WARNING: Invalid device tree, expect boot to fail
> BS->LocateHandle() returns 0
> undefined instruction
> pc : [<9eec65c4>]   lr : [<9eeca390>]
> sp : 9fed7a18  ip : 0000003f fp : 9fed7a2c
> r10: 00000000  r9 : 9eed4658 r8 : 00000000
> r7 : 9eed1ce4  r6 : 9eed3538 r5 : 9fed7a6c  r4 : 9eed4658
> r3 : 00000000  r2 : 9eed2f8e r1 : 9eed1ee0  r0 : 00000000
> Flags: nZCv  IRQs off  FIQs off  Mode SVC_32
> Resetting CPU ...
> 
> resetting ...
> 
> 
> U-Boot 2017.09-rc1-00025-g534695d189 (Aug 06 2017 - 06:58:16 -0400)
> 
> DRAM:  1 GiB
> WARNING: Caches not enabled
> Flash: 128 MiB
> MMC:   MMC: 0
> *** Warning - bad CRC, using default environment
> 
> In:    serial
> Out:   serial
> Err:   serial
> Net:   smc911x-0
> Hit any key to stop autoboot:  2
>
Rob Clark Aug. 6, 2017, 5:49 p.m. UTC | #25
On Sun, Aug 6, 2017 at 1:28 PM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>> From: Rob Clark <robdclark@gmail.com>
>> Date: Sun, 6 Aug 2017 11:34:15 -0400
>>
>> On Sun, Aug 6, 2017 at 10:45 AM, Rob Clark <robdclark@gmail.com> wrote:
>> >
>> > I've started trying to hack up test_efi_loader.py to add a test that
>> > loads OpenBSD's bootloader..  kinda muddling through it at this point,
>> > since not a py expert or too familiar w/ u-boot's test framework.  But
>> > I'll see if I can get to the point where I can run the same thing on
>> > various arm7 and aarch64 devices in qemu.
>> >
>>
>> Making a bit of progress on this (running it on a vexpress_ca15_tc2
>> board in qemu).. any hint where I can find BOOTARM.EFI src code?
>
> https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/arch/armv7/stand/efiboot/efiboot.c?rev=1.17&content-type=text/x-cvsweb-markup
>
> Your failure below looks a bit different from what I'm getting on the
> Banana Pi now.  There I get stuck because the 2nd BS->HandleProtocol()
> call in efi_main() fails.  Somehow the device path of the registered
> disk devices isn't matched correctly to the boot device path...

that is.. odd.. mind adding in lib/efi_loader/Makefile:

  ccflags-y += -DDEBUG=1

?

(you can make the console output easier to read again w/ #undef DEBUG
at top of efi_console.c)

With my complete patchset (ie. assuming this isn't in the middle of a
bisect between 13/20 and 15/20) the device paths for the diskobj and
EFI_LOADED_IMAGE::DeviceHandle should be constructed identically.
(Ie. the patchset consolidates the two different places it was
constructed before... and also fixes the thing I notice you work
around in efi_diskprobe())

> BTW, the OpenBSD code runs fine without the alignment hack.  Our code
> is pretty minimal and doesn't actualy look into the device path
> components.  It just matches the components based on type and by soing
> memcmp.

Hmm, well I do suspect there are still cases where u-boot could crash
because of unaligned access without the hack.  Although I'm less
convinced that we should need the hack on armv7 and more thinking this
is something specific about banana-pi (or allwinner?).  The
vexpress_ca15_tc2 "board" in qemu seems to be working properly..

Mind sending me or pastebin'ing your u-boot .config?  There are some
different device-path construction depending on legacy vs
CONFIG_DM+CONFIG_BLK (the legacy case *looks* right to me, and is used
by vexpress_ca15_tc2.. so I think it should work..)

If OpenBSD supports the vexpress boards, I guess with a suitable qemu
-sd disk.img I should be able to try booting all the way to OS..

BR,
-R
Peter Robinson Aug. 6, 2017, 6:13 p.m. UTC | #26
On Sun, Aug 6, 2017 at 6:49 PM, Rob Clark <robdclark@gmail.com> wrote:
> On Sun, Aug 6, 2017 at 1:28 PM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>>> From: Rob Clark <robdclark@gmail.com>
>>> Date: Sun, 6 Aug 2017 11:34:15 -0400
>>>
>>> On Sun, Aug 6, 2017 at 10:45 AM, Rob Clark <robdclark@gmail.com> wrote:
>>> >
>>> > I've started trying to hack up test_efi_loader.py to add a test that
>>> > loads OpenBSD's bootloader..  kinda muddling through it at this point,
>>> > since not a py expert or too familiar w/ u-boot's test framework.  But
>>> > I'll see if I can get to the point where I can run the same thing on
>>> > various arm7 and aarch64 devices in qemu.
>>> >
>>>
>>> Making a bit of progress on this (running it on a vexpress_ca15_tc2
>>> board in qemu).. any hint where I can find BOOTARM.EFI src code?
>>
>> https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/arch/armv7/stand/efiboot/efiboot.c?rev=1.17&content-type=text/x-cvsweb-markup
>>
>> Your failure below looks a bit different from what I'm getting on the
>> Banana Pi now.  There I get stuck because the 2nd BS->HandleProtocol()
>> call in efi_main() fails.  Somehow the device path of the registered
>> disk devices isn't matched correctly to the boot device path...
>
> that is.. odd.. mind adding in lib/efi_loader/Makefile:
>
>   ccflags-y += -DDEBUG=1
>
> ?
>
> (you can make the console output easier to read again w/ #undef DEBUG
> at top of efi_console.c)
>
> With my complete patchset (ie. assuming this isn't in the middle of a
> bisect between 13/20 and 15/20) the device paths for the diskobj and
> EFI_LOADED_IMAGE::DeviceHandle should be constructed identically.
> (Ie. the patchset consolidates the two different places it was
> constructed before... and also fixes the thing I notice you work
> around in efi_diskprobe())
>
>> BTW, the OpenBSD code runs fine without the alignment hack.  Our code
>> is pretty minimal and doesn't actualy look into the device path
>> components.  It just matches the components based on type and by soing
>> memcmp.
>
> Hmm, well I do suspect there are still cases where u-boot could crash
> because of unaligned access without the hack.  Although I'm less
> convinced that we should need the hack on armv7 and more thinking this
> is something specific about banana-pi (or allwinner?).  The
> vexpress_ca15_tc2 "board" in qemu seems to be working properly..

All AllWinner SoCs are Cortex-A series so ARMv7, in the case of the
banana pi series are AW A20s so are Cortex-A7 based so should be fine
too
Mark Kettenis Aug. 6, 2017, 6:21 p.m. UTC | #27
> From: Rob Clark <robdclark@gmail.com>
> Date: Sun, 6 Aug 2017 13:49:43 -0400
> 
> On Sun, Aug 6, 2017 at 1:28 PM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> >> From: Rob Clark <robdclark@gmail.com>
> >> Date: Sun, 6 Aug 2017 11:34:15 -0400
> >>
> >> On Sun, Aug 6, 2017 at 10:45 AM, Rob Clark <robdclark@gmail.com> wrote:
> >> >
> >> > I've started trying to hack up test_efi_loader.py to add a test that
> >> > loads OpenBSD's bootloader..  kinda muddling through it at this point,
> >> > since not a py expert or too familiar w/ u-boot's test framework.  But
> >> > I'll see if I can get to the point where I can run the same thing on
> >> > various arm7 and aarch64 devices in qemu.
> >> >
> >>
> >> Making a bit of progress on this (running it on a vexpress_ca15_tc2
> >> board in qemu).. any hint where I can find BOOTARM.EFI src code?
> >
> > https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/arch/armv7/stand/efiboot/efiboot.c?rev=1.17&content-type=text/x-cvsweb-markup
> >
> > Your failure below looks a bit different from what I'm getting on the
> > Banana Pi now.  There I get stuck because the 2nd BS->HandleProtocol()
> > call in efi_main() fails.  Somehow the device path of the registered
> > disk devices isn't matched correctly to the boot device path...
> 
> that is.. odd.. mind adding in lib/efi_loader/Makefile:
> 
>   ccflags-y += -DDEBUG=1
> 
> ?
> 
> (you can make the console output easier to read again w/ #undef DEBUG
> at top of efi_console.c)
> 
> With my complete patchset (ie. assuming this isn't in the middle of a
> bisect between 13/20 and 15/20) the device paths for the diskobj and
> EFI_LOADED_IMAGE::DeviceHandle should be constructed identically.
> (Ie. the patchset consolidates the two different places it was
> constructed before... and also fixes the thing I notice you work
> around in efi_diskprobe())
> 
> > BTW, the OpenBSD code runs fine without the alignment hack.  Our code
> > is pretty minimal and doesn't actualy look into the device path
> > components.  It just matches the components based on type and by soing
> > memcmp.
> 
> Hmm, well I do suspect there are still cases where u-boot could crash
> because of unaligned access without the hack.  Although I'm less
> convinced that we should need the hack on armv7 and more thinking this
> is something specific about banana-pi (or allwinner?).  The
> vexpress_ca15_tc2 "board" in qemu seems to be working properly..

I suspect qemu simply doesn't emulate the alignment trap.  The u-boot
startup code explicitly enables alignment fauls on armv7.  See
arch/arm/cpu/armv7/start.S:152.  This helps catching bugs!

> Mind sending me or pastebin'ing your u-boot .config?  There are some
> different device-path construction depending on legacy vs
> CONFIG_DM+CONFIG_BLK (the legacy case *looks* right to me, and is used
> by vexpress_ca15_tc2.. so I think it should work..)

See below.  The Banana Pi (and all other sunxi boards) indeed uses the
legacy code path.  And I think there is a bug in the legacy codepath
where it encodes the partition in the "file" path component.

> If OpenBSD supports the vexpress boards, I guess with a suitable qemu
> -sd disk.img I should be able to try booting all the way to OS..

I think it does.  We don't have a "miniroot" image for it though.  It
might be possible to take 

  http://ftp.openbsd.org/pub/OpenBSD/snapshots/armv7/miniroot-cubie-61.fs

and dd the vexpress u-boot into the right location and copy the device
tree onto the msdos filesystem in that image.

#
# Automatically generated file; DO NOT EDIT.
# U-Boot 2017.09-rc1 Configuration
#
CONFIG_CREATE_ARCH_SYMLINK=y
# CONFIG_ARC is not set
CONFIG_ARM=y
# CONFIG_M68K is not set
# CONFIG_MICROBLAZE is not set
# CONFIG_MIPS is not set
# CONFIG_NDS32 is not set
# CONFIG_NIOS2 is not set
# CONFIG_PPC is not set
# CONFIG_SANDBOX is not set
# CONFIG_SH is not set
# CONFIG_X86 is not set
# CONFIG_XTENSA is not set
CONFIG_SYS_ARCH="arm"
CONFIG_SYS_CPU="armv7"
CONFIG_SYS_SOC="sunxi"
CONFIG_SYS_BOARD="sunxi"
CONFIG_SYS_CONFIG_NAME="sun7i"

#
# ARM architecture
#
CONFIG_HAS_VBAR=y
CONFIG_HAS_THUMB2=y
CONFIG_ARM_ASM_UNIFIED=y
CONFIG_CPU_V7=y
CONFIG_SYS_ARM_ARCH=7
CONFIG_SYS_CACHE_SHIFT_6=y
CONFIG_SYS_CACHELINE_SIZE=64
# CONFIG_ARM_SMCCC is not set
# CONFIG_SEMIHOSTING is not set
# CONFIG_SYS_THUMB_BUILD is not set
CONFIG_SPL_SYS_THUMB_BUILD=y
# CONFIG_SYS_L2CACHE_OFF is not set
# CONFIG_ENABLE_ARM_SOC_BOOT0_HOOK is not set
# CONFIG_ARM_CORTEX_CPU_IS_UP is not set
CONFIG_USE_ARCH_MEMCPY=y
CONFIG_SPL_USE_ARCH_MEMCPY=y
CONFIG_USE_ARCH_MEMSET=y
CONFIG_SPL_USE_ARCH_MEMSET=y
# CONFIG_ARM64_SUPPORT_AARCH32 is not set
# CONFIG_ARCH_AT91 is not set
# CONFIG_TARGET_EDB93XX is not set
# CONFIG_TARGET_ASPENITE is not set
# CONFIG_TARGET_GPLUGD is not set
# CONFIG_ARCH_DAVINCI is not set
# CONFIG_KIRKWOOD is not set
# CONFIG_ARCH_MVEBU is not set
# CONFIG_TARGET_DEVKIT3250 is not set
# CONFIG_TARGET_WORK_92105 is not set
# CONFIG_TARGET_MX25PDK is not set
# CONFIG_TARGET_ZMX25 is not set
# CONFIG_TARGET_APF27 is not set
# CONFIG_TARGET_APX4DEVKIT is not set
# CONFIG_TARGET_XFI3 is not set
# CONFIG_TARGET_M28EVK is not set
# CONFIG_TARGET_MX23EVK is not set
# CONFIG_TARGET_MX28EVK is not set
# CONFIG_TARGET_MX23_OLINUXINO is not set
# CONFIG_TARGET_BG0900 is not set
# CONFIG_TARGET_SANSA_FUZE_PLUS is not set
# CONFIG_TARGET_SC_SPS_1 is not set
# CONFIG_ORION5X is not set
# CONFIG_TARGET_SPEAR300 is not set
# CONFIG_TARGET_SPEAR310 is not set
# CONFIG_TARGET_SPEAR320 is not set
# CONFIG_TARGET_SPEAR600 is not set
# CONFIG_TARGET_STV0991 is not set
# CONFIG_TARGET_X600 is not set
# CONFIG_TARGET_IMX31_PHYCORE is not set
# CONFIG_TARGET_IMX31_PHYCORE_EET is not set
# CONFIG_TARGET_MX31ADS is not set
# CONFIG_TARGET_MX31PDK is not set
# CONFIG_TARGET_WOODBURN is not set
# CONFIG_TARGET_WOODBURN_SD is not set
# CONFIG_TARGET_FLEA3 is not set
# CONFIG_TARGET_MX35PDK is not set
# CONFIG_ARCH_BCM283X is not set
# CONFIG_TARGET_VEXPRESS_CA15_TC2 is not set
# CONFIG_TARGET_VEXPRESS_CA5X2 is not set
# CONFIG_TARGET_VEXPRESS_CA9X4 is not set
# CONFIG_TARGET_BCM23550_W1D is not set
# CONFIG_TARGET_BCM28155_AP is not set
# CONFIG_TARGET_BCMCYGNUS is not set
# CONFIG_TARGET_BCMNSP is not set
# CONFIG_TARGET_BCMNS2 is not set
# CONFIG_ARCH_EXYNOS is not set
# CONFIG_ARCH_S5PC1XX is not set
# CONFIG_ARCH_HIGHBANK is not set
# CONFIG_ARCH_INTEGRATOR is not set
# CONFIG_ARCH_KEYSTONE is not set
# CONFIG_ARCH_OMAP2PLUS is not set
# CONFIG_ARCH_MESON is not set
# CONFIG_ARCH_MX7ULP is not set
# CONFIG_ARCH_MX7 is not set
# CONFIG_ARCH_MX6 is not set
# CONFIG_ARCH_MX5 is not set
# CONFIG_ARCH_RMOBILE is not set
# CONFIG_TARGET_S32V234EVB is not set
# CONFIG_ARCH_SNAPDRAGON is not set
# CONFIG_ARCH_SOCFPGA is not set
CONFIG_ARCH_SUNXI=y
# CONFIG_TARGET_TS4600 is not set
# CONFIG_ARCH_VF610 is not set
# CONFIG_ARCH_ZYNQ is not set
# CONFIG_ARCH_ZYNQMP is not set
# CONFIG_TEGRA is not set
# CONFIG_TARGET_VEXPRESS64_AEMV8A is not set
# CONFIG_TARGET_VEXPRESS64_BASE_FVP is not set
# CONFIG_TARGET_VEXPRESS64_BASE_FVP_DRAM is not set
# CONFIG_TARGET_VEXPRESS64_JUNO is not set
# CONFIG_TARGET_LS2080A_EMU is not set
# CONFIG_TARGET_LS2080A_SIMU is not set
# CONFIG_TARGET_LS2080AQDS is not set
# CONFIG_TARGET_LS2080ARDB is not set
# CONFIG_TARGET_LS2081ARDB is not set
# CONFIG_TARGET_HIKEY is not set
# CONFIG_TARGET_POPLAR is not set
# CONFIG_TARGET_LS1012AQDS is not set
# CONFIG_TARGET_LS1012ARDB is not set
# CONFIG_TARGET_LS1012AFRDM is not set
# CONFIG_TARGET_LS1021AQDS is not set
# CONFIG_TARGET_LS1021ATWR is not set
# CONFIG_TARGET_LS1021AIOT is not set
# CONFIG_TARGET_LS1043AQDS is not set
# CONFIG_TARGET_LS1043ARDB is not set
# CONFIG_TARGET_LS1046AQDS is not set
# CONFIG_TARGET_LS1046ARDB is not set
# CONFIG_TARGET_H2200 is not set
# CONFIG_TARGET_ZIPITZ2 is not set
# CONFIG_TARGET_COLIBRI_PXA270 is not set
# CONFIG_ARCH_UNIPHIER is not set
# CONFIG_STM32 is not set
# CONFIG_ARCH_STI is not set
# CONFIG_ARCH_ROCKCHIP is not set
# CONFIG_TARGET_THUNDERX_88XX is not set
# CONFIG_ARCH_ASPEED is not set
CONFIG_SPL_GPIO_SUPPORT=y
CONFIG_SPL_LIBCOMMON_SUPPORT=y
CONFIG_SPL_LIBGENERIC_SUPPORT=y
CONFIG_SYS_MALLOC_F_LEN=0x400
CONFIG_CONS_INDEX=1
CONFIG_SPL_MMC_SUPPORT=y
CONFIG_SPL_SERIAL_SUPPORT=y
# CONFIG_SPL_DRIVERS_MISC_SUPPORT is not set
CONFIG_SPL_LIBDISK_SUPPORT=y
# CONFIG_SPL_NAND_SUPPORT is not set
# CONFIG_SPL_SPI_FLASH_SUPPORT is not set
# CONFIG_SPL_SPI_SUPPORT is not set
# CONFIG_SPL_WATCHDOG_SUPPORT is not set
CONFIG_IDENT_STRING=" Allwinner Technology"
# CONFIG_SUNXI_HIGH_SRAM is not set
CONFIG_SUNXI_GEN_SUN4I=y
# CONFIG_MACH_SUN4I is not set
# CONFIG_MACH_SUN5I is not set
# CONFIG_MACH_SUN6I is not set
CONFIG_MACH_SUN7I=y
# CONFIG_MACH_SUN8I_A23 is not set
# CONFIG_MACH_SUN8I_A33 is not set
# CONFIG_MACH_SUN8I_A83T is not set
# CONFIG_MACH_SUN8I_H3 is not set
# CONFIG_MACH_SUN8I_R40 is not set
# CONFIG_MACH_SUN8I_V3S is not set
# CONFIG_MACH_SUN9I is not set
# CONFIG_MACH_SUN50I is not set
# CONFIG_MACH_SUN50I_H5 is not set
# CONFIG_RESERVE_ALLWINNER_BOOT0_HEADER is not set
CONFIG_DRAM_CLK=432
CONFIG_DRAM_MBUS_CLK=300
CONFIG_DRAM_ZQ=127
# CONFIG_DRAM_ODT_EN is not set
CONFIG_DRAM_EMR1=4
CONFIG_DRAM_TPR3=0
CONFIG_DRAM_DQS_GATING_DELAY=0
CONFIG_DRAM_TIMINGS_VENDOR_MAGIC=y
# CONFIG_DRAM_TIMINGS_DDR3_1066F_1333H is not set
# CONFIG_DRAM_TIMINGS_DDR3_800E_1066G_1333J is not set
CONFIG_SYS_CLK_FREQ=912000000
# CONFIG_UART0_PORT_F is not set
# CONFIG_OLD_SUNXI_KERNEL_COMPAT is not set
CONFIG_MACPWR="PH23"
CONFIG_MMC0_CD_PIN=""
CONFIG_MMC1_CD_PIN=""
CONFIG_MMC2_CD_PIN=""
CONFIG_MMC3_CD_PIN=""
CONFIG_MMC1_PINS=""
CONFIG_MMC2_PINS=""
CONFIG_MMC3_PINS=""
CONFIG_MMC_SUNXI_SLOT_EXTRA=-1
CONFIG_INITIAL_USB_SCAN_DELAY=0
CONFIG_USB0_VBUS_PIN=""
CONFIG_USB0_VBUS_DET=""
CONFIG_USB0_ID_DET=""
CONFIG_USB1_VBUS_PIN="PH6"
CONFIG_USB2_VBUS_PIN="PH3"
CONFIG_USB3_VBUS_PIN=""
CONFIG_I2C0_ENABLE=y
# CONFIG_I2C1_ENABLE is not set
# CONFIG_I2C2_ENABLE is not set
# CONFIG_I2C3_ENABLE is not set
# CONFIG_I2C4_ENABLE is not set
# CONFIG_AXP_GPIO is not set
CONFIG_VIDEO=y
CONFIG_VIDEO_HDMI=y
# CONFIG_VIDEO_VGA is not set
CONFIG_VIDEO_COMPOSITE=y
CONFIG_VIDEO_LCD_MODE=""
CONFIG_VIDEO_LCD_DCLK_PHASE=1
CONFIG_VIDEO_LCD_POWER=""
CONFIG_VIDEO_LCD_RESET=""
CONFIG_VIDEO_LCD_BL_EN=""
CONFIG_VIDEO_LCD_BL_PWM=""
CONFIG_VIDEO_LCD_BL_PWM_ACTIVE_LOW=y
# CONFIG_VIDEO_LCD_PANEL_I2C is not set
CONFIG_VIDEO_LCD_IF_PARALLEL=y
# CONFIG_SUNXI_DE2 is not set
CONFIG_VIDEO_LCD_PANEL_PARALLEL=y
# CONFIG_VIDEO_LCD_PANEL_LVDS is not set
# CONFIG_VIDEO_LCD_PANEL_MIPI_4_LANE_513_MBPS_VIA_SSD2828 is not set
# CONFIG_VIDEO_LCD_PANEL_EDP_4_LANE_1620M_VIA_ANX9804 is not set
# CONFIG_VIDEO_LCD_PANEL_HITACHI_TX18D42VM is not set
# CONFIG_VIDEO_LCD_TL059WV5C0 is not set
CONFIG_SATAPWR=""
CONFIG_GMAC_TX_DELAY=3
CONFIG_SPL_STACK_R_ADDR=0x4fe00000
# CONFIG_SPL_FAT_SUPPORT is not set
CONFIG_CPU_V7_HAS_NONSEC=y
CONFIG_CPU_V7_HAS_VIRT=y
CONFIG_ARCH_SUPPORT_PSCI=y
CONFIG_ARMV7_NONSEC=y
# CONFIG_ARMV7_BOOT_SEC_DEFAULT is not set
CONFIG_ARMV7_VIRT=y
CONFIG_ARMV7_PSCI=y
CONFIG_ARMV7_PSCI_NR_CPUS=4
# CONFIG_ARMV7_LPAE is not set
# CONFIG_CMD_DEKBLOB is not set
# CONFIG_CMD_HDMIDETECT is not set

#
# ARM debug
#
# CONFIG_DEBUG_LL is not set
CONFIG_DEFAULT_DEVICE_TREE="sun7i-a20-bananapi"
CONFIG_SMBIOS_PRODUCT_NAME="sunxi"
# CONFIG_DEBUG_UART is not set
CONFIG_AHCI=y

#
# General setup
#
CONFIG_LOCALVERSION=""
CONFIG_LOCALVERSION_AUTO=y
CONFIG_CC_OPTIMIZE_FOR_SIZE=y
CONFIG_DISTRO_DEFAULTS=y
CONFIG_SYS_MALLOC_F=y
CONFIG_SPL_SYS_MALLOC_F_LEN=0x400
CONFIG_EXPERT=y
# CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
# CONFIG_TOOLS_DEBUG is not set
# CONFIG_PHYS_64BIT is not set

#
# Boot images
#
# CONFIG_FIT is not set
CONFIG_OF_BOARD_SETUP=y
# CONFIG_OF_SYSTEM_SETUP is not set
# CONFIG_OF_STDOUT_VIA_ALIAS is not set
CONFIG_SYS_EXTRA_OPTIONS=""
CONFIG_ARCH_FIXUP_FDT_MEMORY=y

#
# API
#
# CONFIG_API is not set

#
# Boot timing
#
# CONFIG_BOOTSTAGE is not set
CONFIG_BOOTSTAGE_USER_COUNT=20
CONFIG_BOOTSTAGE_RECORD_COUNT=30
CONFIG_BOOTSTAGE_STASH_ADDR=0
CONFIG_BOOTSTAGE_STASH_SIZE=0x1000

#
# Boot media
#
# CONFIG_NAND_BOOT is not set
# CONFIG_ONENAND_BOOT is not set
# CONFIG_QSPI_BOOT is not set
# CONFIG_SATA_BOOT is not set
# CONFIG_SD_BOOT is not set
# CONFIG_SPI_BOOT is not set

#
# Environment
#
# CONFIG_ENV_IS_IN_DATAFLASH is not set
# CONFIG_ENV_IS_IN_EEPROM is not set
# CONFIG_ENV_IS_IN_FAT is not set
# CONFIG_ENV_IS_IN_FLASH is not set
CONFIG_ENV_IS_IN_MMC=y
# CONFIG_ENV_IS_IN_NAND is not set
# CONFIG_ENV_IS_IN_NVRAM is not set
# CONFIG_ENV_IS_IN_ONENAND is not set
# CONFIG_ENV_IS_IN_REMOTE is not set
# CONFIG_ENV_IS_IN_SPI_FLASH is not set
# CONFIG_ENV_IS_IN_UBI is not set
# CONFIG_ENV_IS_NOWHERE is not set
CONFIG_ENV_OFFSET=0x88000
CONFIG_ENV_SIZE=0x20000
CONFIG_BOOTDELAY=2

#
# Console
#
CONFIG_MENU=y
# CONFIG_CONSOLE_RECORD is not set
# CONFIG_SILENT_CONSOLE is not set
CONFIG_PRE_CONSOLE_BUFFER=y
CONFIG_PRE_CON_BUF_SZ=4096
CONFIG_PRE_CON_BUF_ADDR=0x4f000000
CONFIG_CONSOLE_MUX=y
CONFIG_SYS_CONSOLE_IS_IN_ENV=y
# CONFIG_SYS_CONSOLE_OVERWRITE_ROUTINE is not set
# CONFIG_SYS_CONSOLE_ENV_OVERWRITE is not set
# CONFIG_SYS_CONSOLE_INFO_QUIET is not set
CONFIG_SYS_STDIO_DEREGISTER=y
# CONFIG_FIT_EMBED is not set
CONFIG_DEFAULT_FDT_FILE=""
# CONFIG_VERSION_VARIABLE is not set
CONFIG_DISPLAY_CPUINFO=y
CONFIG_DISPLAY_BOARDINFO=y

#
# Start-up hooks
#
# CONFIG_ARCH_EARLY_INIT_R is not set
# CONFIG_ARCH_MISC_INIT is not set
# CONFIG_BOARD_EARLY_INIT_F is not set

#
# Security support
#
CONFIG_HASH=y

#
# SPL / TPL
#
CONFIG_SUPPORT_SPL=y
CONFIG_SPL=y
# CONFIG_SPL_BOARD_INIT is not set
CONFIG_SPL_RAW_IMAGE_SUPPORT=y
CONFIG_SPL_LEGACY_IMAGE_SUPPORT=y
CONFIG_SPL_SYS_MALLOC_SIMPLE=y
CONFIG_SPL_STACK_R=y
CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x100000
# CONFIG_SPL_SEPARATE_BSS is not set
# CONFIG_SPL_DISPLAY_PRINT is not set
CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR=y
CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x50
# CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION is not set
# CONFIG_SPL_CPU_SUPPORT is not set
# CONFIG_SPL_CRYPTO_SUPPORT is not set
# CONFIG_SPL_HASH_SUPPORT is not set
# CONFIG_SPL_DMA_SUPPORT is not set
# CONFIG_SPL_ENV_SUPPORT is not set
# CONFIG_SPL_EXT_SUPPORT is not set
# CONFIG_SPL_FPGA_SUPPORT is not set
CONFIG_SPL_I2C_SUPPORT=y
# CONFIG_SPL_MPC8XXX_INIT_DDR_SUPPORT is not set
# CONFIG_SPL_MTD_SUPPORT is not set
# CONFIG_SPL_MUSB_NEW_SUPPORT is not set
# CONFIG_SPL_NET_SUPPORT is not set
# CONFIG_SPL_NO_CPU_SUPPORT is not set
# CONFIG_SPL_NOR_SUPPORT is not set
# CONFIG_SPL_XIP_SUPPORT is not set
# CONFIG_SPL_ONENAND_SUPPORT is not set
# CONFIG_SPL_OS_BOOT is not set
# CONFIG_SPL_PCI_SUPPORT is not set
# CONFIG_SPL_PCH_SUPPORT is not set
# CONFIG_SPL_POST_MEM_SUPPORT is not set
CONFIG_SPL_POWER_SUPPORT=y
# CONFIG_SPL_RAM_SUPPORT is not set
# CONFIG_SPL_RTC_SUPPORT is not set
# CONFIG_SPL_SATA_SUPPORT is not set
# CONFIG_SPL_TIMER_SUPPORT is not set
# CONFIG_SPL_USB_HOST_SUPPORT is not set
# CONFIG_SPL_USB_GADGET_SUPPORT is not set
# CONFIG_SPL_YMODEM_SUPPORT is not set

#
# Command line interface
#
CONFIG_CMDLINE=y
CONFIG_HUSH_PARSER=y
CONFIG_SYS_PROMPT="=> "

#
# Autoboot options
#
CONFIG_AUTOBOOT=y
# CONFIG_AUTOBOOT_KEYED is not set

#
# FASTBOOT
#
# CONFIG_FASTBOOT is not set

#
# Commands
#

#
# Info commands
#
CONFIG_CMD_BDI=y
# CONFIG_CMD_CONFIG is not set
CONFIG_CMD_CONSOLE=y
# CONFIG_CMD_CPU is not set
# CONFIG_CMD_LICENSE is not set

#
# Boot commands
#
CONFIG_CMD_BOOTD=y
CONFIG_CMD_BOOTM=y
CONFIG_CMD_BOOTZ=y
CONFIG_CMD_BOOTEFI=y
CONFIG_CMD_BOOTEFI_HELLO_COMPILE=y
# CONFIG_CMD_BOOTEFI_HELLO is not set
# CONFIG_CMD_BOOTMENU is not set
CONFIG_CMD_ELF=y
CONFIG_CMD_FDT=y
CONFIG_CMD_GO=y
CONFIG_CMD_RUN=y
CONFIG_CMD_IMI=y
# CONFIG_CMD_IMLS is not set
CONFIG_CMD_XIMG=y
CONFIG_CMD_POWEROFF=y

#
# Environment commands
#
# CONFIG_CMD_ASKENV is not set
CONFIG_CMD_EXPORTENV=y
CONFIG_CMD_IMPORTENV=y
CONFIG_CMD_EDITENV=y
# CONFIG_CMD_GREPENV is not set
CONFIG_CMD_SAVEENV=y
CONFIG_CMD_ENV_EXISTS=y
# CONFIG_CMD_ENV_CALLBACK is not set
# CONFIG_CMD_ENV_FLAGS is not set

#
# Memory commands
#
CONFIG_CMD_MEMORY=y
CONFIG_CMD_CRC32=y
# CONFIG_CRC32_VERIFY is not set
# CONFIG_CMD_EEPROM is not set
# CONFIG_CMD_MD5SUM is not set
# CONFIG_CMD_SHA1SUM is not set
# CONFIG_LOOPW is not set
# CONFIG_CMD_MEMTEST is not set
# CONFIG_CMD_MX_CYCLIC is not set
# CONFIG_CMD_MEMINFO is not set

#
# Compression commands
#
# CONFIG_CMD_LZMADEC is not set
# CONFIG_CMD_UNZIP is not set
# CONFIG_CMD_ZIP is not set

#
# Device access commands
#
# CONFIG_CMD_CLK is not set
CONFIG_CMD_DM=y
# CONFIG_CMD_DEMO is not set
# CONFIG_CMD_IDE is not set
# CONFIG_CMD_IO is not set
# CONFIG_CMD_IOTRACE is not set
CONFIG_CMD_LOADB=y
CONFIG_CMD_LOADS=y
# CONFIG_CMD_FLASH is not set
# CONFIG_CMD_GPT is not set
# CONFIG_CMD_ARMFLASH is not set
CONFIG_CMD_MMC=y
# CONFIG_CMD_NAND is not set
CONFIG_CMD_PART=y
# CONFIG_CMD_SF is not set
# CONFIG_CMD_SPI is not set
CONFIG_CMD_I2C=y
CONFIG_CMD_USB=y
# CONFIG_CMD_DFU is not set
# CONFIG_CMD_USB_MASS_STORAGE is not set
# CONFIG_CMD_FPGA is not set
# CONFIG_CMD_FPGAD is not set
# CONFIG_CMD_FUSE is not set
CONFIG_CMD_GPIO=y
# CONFIG_CMD_FDC is not set
# CONFIG_CMD_SATA is not set

#
# Shell scripting commands
#
CONFIG_CMD_ECHO=y
CONFIG_CMD_ITEST=y
CONFIG_CMD_SOURCE=y
CONFIG_CMD_SETEXPR=y

#
# Network commands
#
CONFIG_CMD_NET=y
# CONFIG_CMD_TFTPPUT is not set
# CONFIG_CMD_TFTPSRV is not set
# CONFIG_CMD_RARP is not set
CONFIG_CMD_DHCP=y
CONFIG_CMD_PXE=y
CONFIG_CMD_NFS=y
CONFIG_CMD_MII=y
CONFIG_CMD_PING=y
# CONFIG_CMD_CDP is not set
# CONFIG_CMD_SNTP is not set
# CONFIG_CMD_DNS is not set
# CONFIG_CMD_LINK_LOCAL is not set
# CONFIG_CMD_ETHSW is not set

#
# Misc commands
#
# CONFIG_CMD_BMP is not set
# CONFIG_CMD_BSP is not set
# CONFIG_CMD_BKOPS_ENABLE is not set
# CONFIG_CMD_CACHE is not set
# CONFIG_CMD_DISPLAY is not set
# CONFIG_CMD_LED is not set
# CONFIG_CMD_DATE is not set
# CONFIG_CMD_TIME is not set
# CONFIG_CMD_GETTIME is not set
CONFIG_CMD_MISC=y
# CONFIG_CMD_TIMER is not set
# CONFIG_CMD_QFW is not set

#
# Power commands
#

#
# Security commands
#
# CONFIG_CMD_AES is not set
# CONFIG_CMD_BLOB is not set
# CONFIG_CMD_HASH is not set

#
# Firmware commands
#

#
# Filesystem commands
#
CONFIG_CMD_EXT2=y
CONFIG_CMD_EXT4=y
# CONFIG_CMD_EXT4_WRITE is not set
CONFIG_CMD_FAT=y
CONFIG_CMD_FS_GENERIC=y
# CONFIG_CMD_FS_UUID is not set
# CONFIG_CMD_JFFS2 is not set
# CONFIG_CMD_MTDPARTS is not set

#
# Debug commands
#
# CONFIG_CMD_BEDBUG is not set
# CONFIG_CMD_DIAG is not set
# CONFIG_CMD_KGDB is not set
# CONFIG_CMD_UBI is not set

#
# Partition Types
#
CONFIG_PARTITIONS=y
# CONFIG_MAC_PARTITION is not set
# CONFIG_SPL_MAC_PARTITION is not set
CONFIG_DOS_PARTITION=y
# CONFIG_SPL_DOS_PARTITION is not set
CONFIG_ISO_PARTITION=y
# CONFIG_SPL_ISO_PARTITION is not set
# CONFIG_AMIGA_PARTITION is not set
# CONFIG_SPL_AMIGA_PARTITION is not set
CONFIG_EFI_PARTITION=y
CONFIG_EFI_PARTITION_ENTRIES_OFF=0
# CONFIG_SPL_EFI_PARTITION is not set
CONFIG_PARTITION_UUIDS=y
# CONFIG_SPL_PARTITION_UUIDS is not set
# CONFIG_PARTITION_TYPE_GUID is not set
CONFIG_SUPPORT_OF_CONTROL=y

#
# Device Tree Control
#
CONFIG_OF_CONTROL=y
# CONFIG_OF_BOARD_FIXUP is not set
# CONFIG_SPL_OF_CONTROL is not set
# CONFIG_OF_LIVE is not set
CONFIG_OF_SEPARATE=y
# CONFIG_OF_EMBED is not set
# CONFIG_OF_BOARD is not set
CONFIG_NET=y
# CONFIG_NET_RANDOM_ETHADDR is not set
CONFIG_NETCONSOLE=y
CONFIG_NET_TFTP_VARS=y
CONFIG_BOOTP_PXE_CLIENTARCH=0x15
CONFIG_BOOTP_VCI_STRING="U-Boot.armv7"

#
# Device Drivers
#

#
# Generic Driver Options
#
CONFIG_DM=y
# CONFIG_SPL_DM is not set
CONFIG_DM_WARN=y
CONFIG_DM_DEVICE_REMOVE=y
CONFIG_DM_STDIO=y
CONFIG_DM_SEQ_ALIAS=y
# CONFIG_SPL_DM_SEQ_ALIAS is not set
# CONFIG_REGMAP is not set
# CONFIG_SPL_REGMAP is not set
# CONFIG_DEVRES is not set
CONFIG_SIMPLE_BUS=y
CONFIG_OF_TRANSLATE=y
CONFIG_DM_DEV_READ_INLINE=y
# CONFIG_ADC is not set
# CONFIG_ADC_EXYNOS is not set
# CONFIG_ADC_SANDBOX is not set
# CONFIG_SATA is not set
CONFIG_SCSI=y

#
# SATA/SCSI device support
#
# CONFIG_BLK is not set
# CONFIG_BLOCK_CACHE is not set
# CONFIG_IDE is not set

#
# Clock
#
# CONFIG_CLK is not set
# CONFIG_CPU is not set

#
# Hardware crypto devices
#
# CONFIG_FSL_CAAM is not set
# CONFIG_SYS_FSL_SEC_BE is not set
# CONFIG_SYS_FSL_SEC_LE is not set

#
# Demo for driver model
#
# CONFIG_DM_DEMO is not set

#
# DFU support
#

#
# DMA Support
#
# CONFIG_DMA is not set
# CONFIG_TI_EDMA3 is not set

#
# FPGA support
#
# CONFIG_FPGA_ALTERA is not set
# CONFIG_FPGA_SOCFPGA is not set
# CONFIG_FPGA_XILINX is not set

#
# GPIO Support
#
CONFIG_DM_GPIO=y
# CONFIG_ALTERA_PIO is not set
# CONFIG_DWAPB_GPIO is not set
# CONFIG_AT91_GPIO is not set
# CONFIG_ATMEL_PIO4 is not set
# CONFIG_INTEL_BROADWELL_GPIO is not set
# CONFIG_IMX_RGPIO2P is not set
# CONFIG_LPC32XX_GPIO is not set
# CONFIG_MSM_GPIO is not set
# CONFIG_ROCKCHIP_GPIO is not set
# CONFIG_TEGRA_GPIO is not set
# CONFIG_TEGRA186_GPIO is not set
# CONFIG_VYBRID_GPIO is not set
# CONFIG_DM_74X164 is not set
# CONFIG_DM_PCA953X is not set
# CONFIG_MPC85XX_GPIO is not set

#
# I2C support
#
# CONFIG_DM_I2C is not set
# CONFIG_DM_I2C_COMPAT is not set
# CONFIG_SYS_I2C_DW is not set
# CONFIG_SYS_I2C_IMX_LPI2C is not set
CONFIG_DM_KEYBOARD=y
# CONFIG_CROS_EC_KEYB is not set
# CONFIG_I8042_KEYB is not set

#
# LED Support
#
# CONFIG_LED is not set
# CONFIG_LED_STATUS is not set

#
# Mailbox Controller Support
#
# CONFIG_DM_MAILBOX is not set

#
# Memory Controller drivers
#

#
# Multifunction device drivers
#
# CONFIG_MISC is not set
# CONFIG_CROS_EC is not set
# CONFIG_DS4510 is not set
# CONFIG_FSL_SEC_MON is not set
# CONFIG_MXC_OCOTP is not set
# CONFIG_NUVOTON_NCT6102D is not set
# CONFIG_PWRSEQ is not set
# CONFIG_PCA9551_LED is not set
# CONFIG_WINBOND_W83627 is not set

#
# MMC Host controller Support
#
CONFIG_MMC=y
# CONFIG_DM_MMC is not set
# CONFIG_SPL_MMC_TINY is not set
# CONFIG_MMC_DW is not set
# CONFIG_MMC_MXC is not set
# CONFIG_MMC_MXS is not set
# CONFIG_MMC_PCI is not set
# CONFIG_MMC_OMAP_HS is not set
# CONFIG_MMC_SDHCI is not set
CONFIG_MMC_SUNXI=y

#
# MTD Support
#
# CONFIG_MTD is not set
# CONFIG_MTD_NOR_FLASH is not set

#
# NAND Device Support
#
# CONFIG_NAND_DENALI is not set
# CONFIG_NAND_VF610_NFC is not set
# CONFIG_NAND_PXA3XX is not set
# CONFIG_NAND_SUNXI is not set
# CONFIG_NAND_ARASAN is not set
# CONFIG_NAND_ZYNQ is not set

#
# Generic NAND options
#
# CONFIG_SYS_NAND_U_BOOT_LOCATIONS is not set
# CONFIG_SPL_NAND_DENALI is not set

#
# SPI Flash Support
#
# CONFIG_SPI_FLASH is not set
# CONFIG_SPL_SPI_SUNXI is not set

#
# UBI support
#
# CONFIG_MTD_UBI is not set
# CONFIG_BITBANGMII is not set
# CONFIG_MV88E6352_SWITCH is not set
CONFIG_PHYLIB=y
# CONFIG_MV88E61XX_SWITCH is not set
# CONFIG_PHYLIB_10G is not set
# CONFIG_PHY_AQUANTIA is not set
# CONFIG_PHY_ATHEROS is not set
# CONFIG_PHY_BROADCOM is not set
# CONFIG_PHY_CORTINA is not set
# CONFIG_PHY_DAVICOM is not set
# CONFIG_PHY_ET1011C is not set
# CONFIG_PHY_LXT is not set
# CONFIG_PHY_MARVELL is not set
# CONFIG_PHY_MICREL is not set
# CONFIG_PHY_MSCC is not set
# CONFIG_PHY_NATSEMI is not set
# CONFIG_PHY_REALTEK is not set
# CONFIG_PHY_SMSC is not set
# CONFIG_PHY_TERANETICS is not set
# CONFIG_PHY_TI is not set
# CONFIG_PHY_VITESSE is not set
# CONFIG_PHY_XILINX is not set
# CONFIG_PHY_FIXED is not set
CONFIG_DM_ETH=y
CONFIG_NETDEVICES=y
# CONFIG_PHY_GIGE is not set
# CONFIG_ALTERA_TSE is not set
# CONFIG_DWC_ETH_QOS is not set
# CONFIG_E1000 is not set
CONFIG_ETH_DESIGNWARE=y
# CONFIG_ETHOC is not set
# CONFIG_FTMAC100 is not set
# CONFIG_MACB is not set
CONFIG_RGMII=y
# CONFIG_RTL8139 is not set
# CONFIG_RTL8169 is not set
CONFIG_SUN7I_GMAC=y
# CONFIG_SUN4I_EMAC is not set
# CONFIG_SUN8I_EMAC is not set
# CONFIG_GMAC_ROCKCHIP is not set
# CONFIG_PCI is not set

#
# PHY Subsystem
#
# CONFIG_PHY is not set
# CONFIG_SPL_PHY is not set
# CONFIG_MVEBU_COMPHY_SUPPORT is not set

#
# Pin controllers
#
# CONFIG_PINCTRL is not set

#
# Power
#

#
# Power Domain Support
#
# CONFIG_POWER_DOMAIN is not set
# CONFIG_DM_PMIC is not set
# CONFIG_PMIC_AS3722 is not set
# CONFIG_POWER_MC34VR500 is not set
# CONFIG_DM_REGULATOR is not set
# CONFIG_SUNXI_NO_PMIC is not set
CONFIG_AXP209_POWER=y
CONFIG_AXP_DCDC2_VOLT=1400
CONFIG_AXP_DCDC3_VOLT=1250
CONFIG_AXP_ALDO2_VOLT=3000
CONFIG_AXP_ALDO3_VOLT=0
CONFIG_AXP_ALDO4_VOLT=0
# CONFIG_DM_PWM is not set
# CONFIG_PWM_SANDBOX is not set
# CONFIG_RAM is not set

#
# Remote Processor drivers
#

#
# Reset Controller Support
#
# CONFIG_DM_RESET is not set

#
# Real Time Clock
#
# CONFIG_DM_RTC is not set

#
# Serial drivers
#
CONFIG_BAUDRATE=115200
CONFIG_REQUIRE_SERIAL_CONSOLE=y
CONFIG_SERIAL_PRESENT=y
CONFIG_SPL_SERIAL_PRESENT=y
CONFIG_DM_SERIAL=y
# CONFIG_SERIAL_IRQ_BUFFER is not set
CONFIG_SPL_DM_SERIAL=y
# CONFIG_TPL_DM_SERIAL is not set
# CONFIG_DEBUG_UART_SKIP_INIT is not set
# CONFIG_ALTERA_JTAG_UART is not set
# CONFIG_ALTERA_UART is not set
# CONFIG_ATMEL_USART is not set
# CONFIG_FSL_LPUART is not set
# CONFIG_MVEBU_A3700_UART is not set
CONFIG_SYS_NS16550=y
# CONFIG_MSM_SERIAL is not set
# CONFIG_PXA_SERIAL is not set

#
# Sound support
#
# CONFIG_SOUND is not set

#
# SPI Support
#
# CONFIG_DM_SPI is not set
# CONFIG_SOFT_SPI is not set
# CONFIG_FSL_ESPI is not set
# CONFIG_FSL_QSPI is not set
# CONFIG_TI_QSPI is not set

#
# SPMI support
#
# CONFIG_SPMI is not set

#
# System reset device drivers
#
# CONFIG_SYSRESET is not set
# CONFIG_SYSRESET_SYSCON is not set
# CONFIG_SYSRESET_WATCHDOG is not set
# CONFIG_DM_THERMAL is not set

#
# Timer Support
#
# CONFIG_TIMER is not set

#
# TPM support
#
CONFIG_USB=y
CONFIG_DM_USB=y

#
# USB Host Controller Drivers
#
CONFIG_USB_HOST=y
# CONFIG_USB_XHCI_HCD is not set
CONFIG_USB_EHCI_HCD=y
# CONFIG_USB_EHCI_MSM is not set
# CONFIG_USB_EHCI_GENERIC is not set
# CONFIG_USB_OHCI_HCD is not set
# CONFIG_USB_UHCI_HCD is not set
# CONFIG_USB_DWC2 is not set

#
# MUSB Controller Driver
#
# CONFIG_USB_MUSB_HOST is not set
# CONFIG_USB_MUSB_GADGET is not set
# CONFIG_USB_MUSB_TI is not set

#
# ULPI drivers
#

#
# USB peripherals
#
CONFIG_USB_STORAGE=y
CONFIG_USB_KEYBOARD=y
# CONFIG_USB_GADGET is not set

#
# Graphics support
#
# CONFIG_DM_VIDEO is not set
# CONFIG_SYS_WHITE_ON_BLACK is not set

#
# TrueType Fonts
#
# CONFIG_VIDEO_VESA is not set
# CONFIG_VIDEO_LCD_ANX9804 is not set
# CONFIG_VIDEO_LCD_SSD2828 is not set
# CONFIG_VIDEO_LCD_HITACHI_TX18D42VM is not set
# CONFIG_VIDEO_MVEBU is not set
# CONFIG_DISPLAY is not set
# CONFIG_VIDEO_FSL_DCU_FB is not set
# CONFIG_VIDEO_TEGRA20 is not set
# CONFIG_VIDEO_BRIDGE is not set
CONFIG_CFB_CONSOLE=y
# CONFIG_CFB_CONSOLE_ANSI is not set
CONFIG_VGA_AS_SINGLE_DEVICE=y
CONFIG_VIDEO_SW_CURSOR=y
# CONFIG_CONSOLE_EXTRA_INFO is not set
CONFIG_CONSOLE_SCROLL_LINES=1
# CONFIG_VIDEO_CT69000 is not set
CONFIG_SYS_CONSOLE_BG_COL=0x00
CONFIG_SYS_CONSOLE_FG_COL=0xa0
# CONFIG_LCD is not set

#
# Watchdog Timer Support
#
# CONFIG_BCM2835_WDT is not set
# CONFIG_ULP_WATCHDOG is not set
# CONFIG_WDT is not set
# CONFIG_PHYS_TO_BUS is not set

#
# File systems
#
# CONFIG_FS_CBFS is not set
CONFIG_FS_FAT=y
CONFIG_FAT_WRITE=y
CONFIG_FS_FAT_MAX_CLUSTSIZE=65536
# CONFIG_FS_JFFS2 is not set
# CONFIG_FS_CRAMFS is not set

#
# Library routines
#
# CONFIG_CC_OPTIMIZE_LIBS_FOR_SPEED is not set
CONFIG_HAVE_PRIVATE_LIBGCC=y
CONFIG_USE_PRIVATE_LIBGCC=y
CONFIG_SYS_HZ=1000
CONFIG_USE_TINY_PRINTF=y
CONFIG_REGEX=y
# CONFIG_LIB_RAND is not set
# CONFIG_SPL_TINY_MEMSET is not set
# CONFIG_CMD_DHRYSTONE is not set

#
# Security support
#
# CONFIG_AES is not set
# CONFIG_RSA is not set
# CONFIG_TPM is not set

#
# Hashing Support
#
# CONFIG_SHA1 is not set
# CONFIG_SHA256 is not set
# CONFIG_SHA_HW_ACCEL is not set

#
# Compression Support
#
# CONFIG_LZ4 is not set
# CONFIG_LZMA is not set
# CONFIG_LZO is not set
# CONFIG_ERRNO_STR is not set
CONFIG_OF_LIBFDT=y
# CONFIG_OF_LIBFDT_OVERLAY is not set
# CONFIG_SPL_OF_LIBFDT is not set
# CONFIG_FDT_FIXUP_PARTITIONS is not set

#
# System tables
#
CONFIG_GENERATE_SMBIOS_TABLE=y
CONFIG_SMBIOS_MANUFACTURER=""
CONFIG_EFI_LOADER=y
# CONFIG_UNIT_TEST is not set
Rob Clark Aug. 6, 2017, 6:41 p.m. UTC | #28
On Sun, Aug 6, 2017 at 2:21 PM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>> From: Rob Clark <robdclark@gmail.com>
>> Date: Sun, 6 Aug 2017 13:49:43 -0400
>>
>> On Sun, Aug 6, 2017 at 1:28 PM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>> >> From: Rob Clark <robdclark@gmail.com>
>> >> Date: Sun, 6 Aug 2017 11:34:15 -0400
>> >>
>> >> On Sun, Aug 6, 2017 at 10:45 AM, Rob Clark <robdclark@gmail.com> wrote:
>> >> >
>> >> > I've started trying to hack up test_efi_loader.py to add a test that
>> >> > loads OpenBSD's bootloader..  kinda muddling through it at this point,
>> >> > since not a py expert or too familiar w/ u-boot's test framework.  But
>> >> > I'll see if I can get to the point where I can run the same thing on
>> >> > various arm7 and aarch64 devices in qemu.
>> >> >
>> >>
>> >> Making a bit of progress on this (running it on a vexpress_ca15_tc2
>> >> board in qemu).. any hint where I can find BOOTARM.EFI src code?
>> >
>> > https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/arch/armv7/stand/efiboot/efiboot.c?rev=1.17&content-type=text/x-cvsweb-markup
>> >
>> > Your failure below looks a bit different from what I'm getting on the
>> > Banana Pi now.  There I get stuck because the 2nd BS->HandleProtocol()
>> > call in efi_main() fails.  Somehow the device path of the registered
>> > disk devices isn't matched correctly to the boot device path...
>>
>> that is.. odd.. mind adding in lib/efi_loader/Makefile:
>>
>>   ccflags-y += -DDEBUG=1
>>
>> ?
>>
>> (you can make the console output easier to read again w/ #undef DEBUG
>> at top of efi_console.c)
>>
>> With my complete patchset (ie. assuming this isn't in the middle of a
>> bisect between 13/20 and 15/20) the device paths for the diskobj and
>> EFI_LOADED_IMAGE::DeviceHandle should be constructed identically.
>> (Ie. the patchset consolidates the two different places it was
>> constructed before... and also fixes the thing I notice you work
>> around in efi_diskprobe())
>>
>> > BTW, the OpenBSD code runs fine without the alignment hack.  Our code
>> > is pretty minimal and doesn't actualy look into the device path
>> > components.  It just matches the components based on type and by soing
>> > memcmp.
>>
>> Hmm, well I do suspect there are still cases where u-boot could crash
>> because of unaligned access without the hack.  Although I'm less
>> convinced that we should need the hack on armv7 and more thinking this
>> is something specific about banana-pi (or allwinner?).  The
>> vexpress_ca15_tc2 "board" in qemu seems to be working properly..
>
> I suspect qemu simply doesn't emulate the alignment trap.  The u-boot
> startup code explicitly enables alignment fauls on armv7.  See
> arch/arm/cpu/armv7/start.S:152.  This helps catching bugs!

Hmm, that is a really bad idea with EFI_LOADER.. since the efi payload
is inheriting this setting.


BR,
-R
Jonathan Gray Aug. 7, 2017, 3:47 p.m. UTC | #29
On Sun, Aug 06, 2017 at 11:34:15AM -0400, Rob Clark wrote:
> On Sun, Aug 6, 2017 at 10:45 AM, Rob Clark <robdclark@gmail.com> wrote:
> >
> > I've started trying to hack up test_efi_loader.py to add a test that
> > loads OpenBSD's bootloader..  kinda muddling through it at this point,
> > since not a py expert or too familiar w/ u-boot's test framework.  But
> > I'll see if I can get to the point where I can run the same thing on
> > various arm7 and aarch64 devices in qemu.
> >
> 
> Making a bit of progress on this (running it on a vexpress_ca15_tc2
> board in qemu).. any hint where I can find BOOTARM.EFI src code?
> 
> => tftpboot 80400000 obsdboot.efi
> smc911x: MAC 52:54:00:12:34:56
> smc911x: detected LAN9118 controller
> smc911x: phy initialized
> smc911x: MAC 52:54:00:12:34:56
> Using smc911x-0 device
> TFTP from server 10.0.2.2; our IP address is 10.0.2.15
> Filename 'obsdboot.efi'.
> Load address: 0x80400000
> Loading: *%08#####
> 12.4 MiB/s
> done
> Bytes transferred = 64908 (fd8c hex)
> smc911x: MAC 52:54:00:12:34:56
> => crc32 80400000 $filesize
> CRC32 for 80400000 ... 8040fd8b ==> a9ac4fcf
> => bootefi 80400000
> ## Starting EFI application at 80400000 ...
> WARNING: Invalid device tree, expect boot to fail
> BS->LocateHandle() returns 0
> undefined instruction
> pc : [<9eec65c4>]   lr : [<9eeca390>]
> sp : 9fed7a18  ip : 0000003f fp : 9fed7a2c
> r10: 00000000  r9 : 9eed4658 r8 : 00000000
> r7 : 9eed1ce4  r6 : 9eed3538 r5 : 9fed7a6c  r4 : 9eed4658
> r3 : 00000000  r2 : 9eed2f8e r1 : 9eed1ee0  r0 : 00000000
> Flags: nZCv  IRQs off  FIQs off  Mode SVC_32
> Resetting CPU ...
> 
> resetting ...
> 
> 
> U-Boot 2017.09-rc1-00025-g534695d189 (Aug 06 2017 - 06:58:16 -0400)
> 
> DRAM:  1 GiB
> WARNING: Caches not enabled
> Flash: 128 MiB
> MMC:   MMC: 0
> *** Warning - bad CRC, using default environment
> 
> In:    serial
> Out:   serial
> Err:   serial
> Net:   smc911x-0
> Hit any key to stop autoboot:  2

Why does U-Boot not set fdt_addr_r or fdtfile for vexpress?  Worse yet
trying to load to the default kernel_addr_r fails.  So it requires a
script or manual commands to boot instead of the usual distro boot
arrangement?

There is some kind of hard hang on OpenBSD with vexpress at the moment
and there is no driver for the sd/mmc but getting to that point seemed
quite a bit more painful than using U-Boot on real hardware.

After adding vexpress-v2p-ca15-tc1.dtb to the FAT16 on miniroot-panda-61.fs:

$ qemu-system-arm -M vexpress-a15 -kernel vexpress_ca15_tc2/u-boot -nographic -sd miniroot-panda-61.fs

U-Boot 2017.09-rc1 (Aug 02 2017 - 10:55:19 +1000)

DRAM:  128 MiB
WARNING: Caches not enabled
Flash: 128 MiB
MMC:   MMC: 0
*** Warning - bad CRC, using default environment

In:    serial
Out:   serial
Err:   serial
Net:   smc911x-0
Hit any key to stop autoboot:  0 
MMC Device 1 not found
no mmc device at slot 1
switch to partitions #0, OK
mmc0 is current device
env - environment handling commands

Usage:
env default [-f] -a - [forcibly] reset default environment
env default [-f] var [...] - [forcibly] reset variable(s) to their default values
env delete [-f] var [...] - [forcibly] delete variable(s)
env export [-t | -b | -c] [-s size] addr [var ...] - export environment
env import [-d] [-t [-r] | -b | -c] addr [size] - import environment
env print [-a | name ...] - print environment
env run var [...] - run commands in an environment variable
env save - save environment
env set [-f] name [arg ...]

Scanning mmc 0:1...
Found EFI removable media binary efi/boot/bootarm.efi
reading efi/boot/bootarm.efi
64908 bytes read in 52 ms (1.2 MiB/s)
## Starting EFI application at a0008000 ...
WARNING: Invalid device tree, expect boot to fail
efi_load_pe: Invalid DOS Signature
## Application terminated, r = 2147483646
EFI LOAD FAILED: continuing...
smc911x: MAC 52:54:00:12:34:56
smc911x: detected LAN9118 controller
smc911x: phy initialized
smc911x: MAC 52:54:00:12:34:56
BOOTP broadcast 1
DHCP client bound to address 10.0.2.15 (3 ms)
*** Warning: no boot file name; using '0A00020F.img'
Using smc911x-0 device
TFTP from server 10.0.2.2; our IP address is 10.0.2.15
Filename '0A00020F.img'.
Load address: 0xa0008000
Loading: *
TFTP error: 'Access violation' (2)

...

=> setenv fdt_addr_r 0x81000000
=> setenv fdtfile vexpress-v2p-ca15-tc1.dtb
reading vexpress-v2p-ca15-tc1.dtb
13384 bytes read in 22 ms (593.8 KiB/s)
=> load mmc 0:1 ${kernel_addr_r} efi/boot/bootarm.efi
reading efi/boot/bootarm.efi
64908 bytes read in 51 ms (1.2 MiB/s)
=> bootefi ${kernel_addr_r} ${fdt_addr_r}
## Starting EFI application at a0008000 ...
efi_load_pe: Invalid DOS Signature
## Application terminated, r = 2147483646
=> printenv kernel_addr_r
kernel_addr_r=0xa0008000

=> setenv kernel_addr_r 0x82000000
=> load mmc 0:1 ${kernel_addr_r} efi/boot/bootarm.efi
reading efi/boot/bootarm.efi
64908 bytes read in 49 ms (1.3 MiB/s)
=> bootefi ${kernel_addr_r} ${fdt_addr_r}
## Starting EFI application at 82000000 ...
Scanning disks on mmc...
MMC Device 1 not found
MMC Device 2 not found
MMC Device 3 not found
Found 1 disks
>> OpenBSD/armv7 BOOTARM 0.8
boot> 
cannot open sd0a:/etc/random.seed: No such file or directory
booting sd0a:/bsd: 2265112+7989364+447000 [80+314496+149702]=0xaad1c0

OpenBSD/armv7 booting ...
arg0 0xc0dad1c0 arg1 0x8e0 arg2 0x86ed0000
Allocating page tables
freestart = 0x80dae000, free_pages = 29266 (0x00007252)
IRQ stack: p0x80ddc000 v0xc0ddc000
ABT stack: p0x80ddd000 v0xc0ddd000
UND stack: p0x80dde000 v0xc0dde000
SVC stack: p0x80ddf000 v0xc0ddf000
Creating L1 page table at 0x80db0000
Mapping kernel
Constructing L2 page tables
undefined page pmap board type: 2272
Copyright (c) 1982, 1986, 1989, 1991, 1993
        The Regents of the University of California.  All rights reserved.
Copyright (c) 1995-2017 OpenBSD. All rights reserved.  https://www.OpenBSD.org

OpenBSD 6.1-current (RAMDISK) #30: Sat Aug  5 22:01:16 MDT 2017
    deraadt@armv7.openbsd.org:/usr/src/sys/arch/armv7/compile/RAMDISK
real mem  = 134217728 (128MB)
avail mem = 117125120 (111MB)
mainbus0 at root: V2P-CA15
cpu0 at mainbus0: ARM Cortex-A15 r2p1 (ARMv7)
cpu0: DC enabled IC enabled WB disabled EABT branch prediction enabled
cpu0: 32KB(64b/l,2way) I-cache, 32KB(64b/l,2way) wr-back D-cache
cortex0 at mainbus0
ampintc0 at mainbus0 nirq 160, ncpu 1
agtimer0 at mainbus0: tick rate 62500 KHz
simplebus0 at mainbus0: "smb"
simplebus1 at simplebus0: "motherboard"
simplebus2 at simplebus1: "iofpga"
sysreg0 at simplebus2: ID 0x1190f500 PROCID0 0x14000237
pluart0 at simplebus2: console
pluart1 at simplebus2
pluart2 at simplebus2
pluart3 at simplebus2
plrtc0 at simplebus2
simplebus3 at mainbus0: "hsb"
boot device: lookup 'sd0a:/bsd' failed.
root on rd0a swap on rd0b dump on rd0b
e
Rob Clark Aug. 7, 2017, 4:16 p.m. UTC | #30
On Mon, Aug 7, 2017 at 11:47 AM, Jonathan Gray <jsg@jsg.id.au> wrote:
> On Sun, Aug 06, 2017 at 11:34:15AM -0400, Rob Clark wrote:
>> On Sun, Aug 6, 2017 at 10:45 AM, Rob Clark <robdclark@gmail.com> wrote:
>> >
>> > I've started trying to hack up test_efi_loader.py to add a test that
>> > loads OpenBSD's bootloader..  kinda muddling through it at this point,
>> > since not a py expert or too familiar w/ u-boot's test framework.  But
>> > I'll see if I can get to the point where I can run the same thing on
>> > various arm7 and aarch64 devices in qemu.
>> >
>>
>> Making a bit of progress on this (running it on a vexpress_ca15_tc2
>> board in qemu).. any hint where I can find BOOTARM.EFI src code?
>>
>> => tftpboot 80400000 obsdboot.efi
>> smc911x: MAC 52:54:00:12:34:56
>> smc911x: detected LAN9118 controller
>> smc911x: phy initialized
>> smc911x: MAC 52:54:00:12:34:56
>> Using smc911x-0 device
>> TFTP from server 10.0.2.2; our IP address is 10.0.2.15
>> Filename 'obsdboot.efi'.
>> Load address: 0x80400000
>> Loading: *%08#####
>> 12.4 MiB/s
>> done
>> Bytes transferred = 64908 (fd8c hex)
>> smc911x: MAC 52:54:00:12:34:56
>> => crc32 80400000 $filesize
>> CRC32 for 80400000 ... 8040fd8b ==> a9ac4fcf
>> => bootefi 80400000
>> ## Starting EFI application at 80400000 ...
>> WARNING: Invalid device tree, expect boot to fail
>> BS->LocateHandle() returns 0
>> undefined instruction
>> pc : [<9eec65c4>]   lr : [<9eeca390>]
>> sp : 9fed7a18  ip : 0000003f fp : 9fed7a2c
>> r10: 00000000  r9 : 9eed4658 r8 : 00000000
>> r7 : 9eed1ce4  r6 : 9eed3538 r5 : 9fed7a6c  r4 : 9eed4658
>> r3 : 00000000  r2 : 9eed2f8e r1 : 9eed1ee0  r0 : 00000000
>> Flags: nZCv  IRQs off  FIQs off  Mode SVC_32
>> Resetting CPU ...
>>
>> resetting ...
>>
>>
>> U-Boot 2017.09-rc1-00025-g534695d189 (Aug 06 2017 - 06:58:16 -0400)
>>
>> DRAM:  1 GiB
>> WARNING: Caches not enabled
>> Flash: 128 MiB
>> MMC:   MMC: 0
>> *** Warning - bad CRC, using default environment
>>
>> In:    serial
>> Out:   serial
>> Err:   serial
>> Net:   smc911x-0
>> Hit any key to stop autoboot:  2
>
> Why does U-Boot not set fdt_addr_r or fdtfile for vexpress?  Worse yet
> trying to load to the default kernel_addr_r fails.  So it requires a
> script or manual commands to boot instead of the usual distro boot
> arrangement?

I suspect this is specific to the test framework (probably not
enabling distro-boot-cmd so that the test framework can run the cmds
it wants??)

BR,
-R

> There is some kind of hard hang on OpenBSD with vexpress at the moment
> and there is no driver for the sd/mmc but getting to that point seemed
> quite a bit more painful than using U-Boot on real hardware.
>
> After adding vexpress-v2p-ca15-tc1.dtb to the FAT16 on miniroot-panda-61.fs:
>
> $ qemu-system-arm -M vexpress-a15 -kernel vexpress_ca15_tc2/u-boot -nographic -sd miniroot-panda-61.fs
>
> U-Boot 2017.09-rc1 (Aug 02 2017 - 10:55:19 +1000)
>
> DRAM:  128 MiB
> WARNING: Caches not enabled
> Flash: 128 MiB
> MMC:   MMC: 0
> *** Warning - bad CRC, using default environment
>
> In:    serial
> Out:   serial
> Err:   serial
> Net:   smc911x-0
> Hit any key to stop autoboot:  0
> MMC Device 1 not found
> no mmc device at slot 1
> switch to partitions #0, OK
> mmc0 is current device
> env - environment handling commands
>
> Usage:
> env default [-f] -a - [forcibly] reset default environment
> env default [-f] var [...] - [forcibly] reset variable(s) to their default values
> env delete [-f] var [...] - [forcibly] delete variable(s)
> env export [-t | -b | -c] [-s size] addr [var ...] - export environment
> env import [-d] [-t [-r] | -b | -c] addr [size] - import environment
> env print [-a | name ...] - print environment
> env run var [...] - run commands in an environment variable
> env save - save environment
> env set [-f] name [arg ...]
>
> Scanning mmc 0:1...
> Found EFI removable media binary efi/boot/bootarm.efi
> reading efi/boot/bootarm.efi
> 64908 bytes read in 52 ms (1.2 MiB/s)
> ## Starting EFI application at a0008000 ...
> WARNING: Invalid device tree, expect boot to fail
> efi_load_pe: Invalid DOS Signature
> ## Application terminated, r = 2147483646
> EFI LOAD FAILED: continuing...
> smc911x: MAC 52:54:00:12:34:56
> smc911x: detected LAN9118 controller
> smc911x: phy initialized
> smc911x: MAC 52:54:00:12:34:56
> BOOTP broadcast 1
> DHCP client bound to address 10.0.2.15 (3 ms)
> *** Warning: no boot file name; using '0A00020F.img'
> Using smc911x-0 device
> TFTP from server 10.0.2.2; our IP address is 10.0.2.15
> Filename '0A00020F.img'.
> Load address: 0xa0008000
> Loading: *
> TFTP error: 'Access violation' (2)
>
> ...
>
> => setenv fdt_addr_r 0x81000000
> => setenv fdtfile vexpress-v2p-ca15-tc1.dtb
> reading vexpress-v2p-ca15-tc1.dtb
> 13384 bytes read in 22 ms (593.8 KiB/s)
> => load mmc 0:1 ${kernel_addr_r} efi/boot/bootarm.efi
> reading efi/boot/bootarm.efi
> 64908 bytes read in 51 ms (1.2 MiB/s)
> => bootefi ${kernel_addr_r} ${fdt_addr_r}
> ## Starting EFI application at a0008000 ...
> efi_load_pe: Invalid DOS Signature
> ## Application terminated, r = 2147483646
> => printenv kernel_addr_r
> kernel_addr_r=0xa0008000
>
> => setenv kernel_addr_r 0x82000000
> => load mmc 0:1 ${kernel_addr_r} efi/boot/bootarm.efi
> reading efi/boot/bootarm.efi
> 64908 bytes read in 49 ms (1.3 MiB/s)
> => bootefi ${kernel_addr_r} ${fdt_addr_r}
> ## Starting EFI application at 82000000 ...
> Scanning disks on mmc...
> MMC Device 1 not found
> MMC Device 2 not found
> MMC Device 3 not found
> Found 1 disks
>>> OpenBSD/armv7 BOOTARM 0.8
> boot>
> cannot open sd0a:/etc/random.seed: No such file or directory
> booting sd0a:/bsd: 2265112+7989364+447000 [80+314496+149702]=0xaad1c0
>
> OpenBSD/armv7 booting ...
> arg0 0xc0dad1c0 arg1 0x8e0 arg2 0x86ed0000
> Allocating page tables
> freestart = 0x80dae000, free_pages = 29266 (0x00007252)
> IRQ stack: p0x80ddc000 v0xc0ddc000
> ABT stack: p0x80ddd000 v0xc0ddd000
> UND stack: p0x80dde000 v0xc0dde000
> SVC stack: p0x80ddf000 v0xc0ddf000
> Creating L1 page table at 0x80db0000
> Mapping kernel
> Constructing L2 page tables
> undefined page pmap board type: 2272
> Copyright (c) 1982, 1986, 1989, 1991, 1993
>         The Regents of the University of California.  All rights reserved.
> Copyright (c) 1995-2017 OpenBSD. All rights reserved.  https://www.OpenBSD.org
>
> OpenBSD 6.1-current (RAMDISK) #30: Sat Aug  5 22:01:16 MDT 2017
>     deraadt@armv7.openbsd.org:/usr/src/sys/arch/armv7/compile/RAMDISK
> real mem  = 134217728 (128MB)
> avail mem = 117125120 (111MB)
> mainbus0 at root: V2P-CA15
> cpu0 at mainbus0: ARM Cortex-A15 r2p1 (ARMv7)
> cpu0: DC enabled IC enabled WB disabled EABT branch prediction enabled
> cpu0: 32KB(64b/l,2way) I-cache, 32KB(64b/l,2way) wr-back D-cache
> cortex0 at mainbus0
> ampintc0 at mainbus0 nirq 160, ncpu 1
> agtimer0 at mainbus0: tick rate 62500 KHz
> simplebus0 at mainbus0: "smb"
> simplebus1 at simplebus0: "motherboard"
> simplebus2 at simplebus1: "iofpga"
> sysreg0 at simplebus2: ID 0x1190f500 PROCID0 0x14000237
> pluart0 at simplebus2: console
> pluart1 at simplebus2
> pluart2 at simplebus2
> pluart3 at simplebus2
> plrtc0 at simplebus2
> simplebus3 at mainbus0: "hsb"
> boot device: lookup 'sd0a:/bsd' failed.
> root on rd0a swap on rd0b dump on rd0b
> e
Peter Jones Aug. 7, 2017, 5:32 p.m. UTC | #31
On Fri, Aug 04, 2017 at 10:41:32PM +0200, Mark Kettenis wrote:
[...]
> ..and what you're sketching out here should work with recent enough
> versions of our bootloader.
> 
> However, to me having an ACPI Device Path component in there doesn't
> make much sense on a board without ACPI.  It certainly doesn't help
> mapping a boot path back to an actual hardware device.  Wouldn't it be
> more logical to a Hardware Device Path there instead?  In particular a
> Memory Mapped Device Path would make a lot of sense as the start
> address would make it fairly easy to do the mapping.

It was really an arbitrary choice, as Rob said.  I don't think there's
any big problem with changing it, but I'm not sure Memory Mapped is
correct.  As I read it, StartingAddress and EndingAddress in that class
should be pointing at some window into a memory map entry that is
holding the thing described /by/ the node, but there's really nothing
here.  It's just an arbitrary root node.

If we want something other than the ACPI path I made up, we should
probably just go with a Vendor Specific Device Path with a constant
well-known GUID of our choosing.  e61d73b9-a384-4acc-aeab-82e828f3628b,
say.
Jonathan Gray Aug. 8, 2017, 1:36 a.m. UTC | #32
On Mon, Aug 07, 2017 at 12:16:54PM -0400, Rob Clark wrote:
> On Mon, Aug 7, 2017 at 11:47 AM, Jonathan Gray <jsg@jsg.id.au> wrote:
> > On Sun, Aug 06, 2017 at 11:34:15AM -0400, Rob Clark wrote:
> >> On Sun, Aug 6, 2017 at 10:45 AM, Rob Clark <robdclark@gmail.com> wrote:
> >> >
> >> > I've started trying to hack up test_efi_loader.py to add a test that
> >> > loads OpenBSD's bootloader..  kinda muddling through it at this point,
> >> > since not a py expert or too familiar w/ u-boot's test framework.  But
> >> > I'll see if I can get to the point where I can run the same thing on
> >> > various arm7 and aarch64 devices in qemu.
> >> >
> >>
> >> Making a bit of progress on this (running it on a vexpress_ca15_tc2
> >> board in qemu).. any hint where I can find BOOTARM.EFI src code?
> >>
> >> => tftpboot 80400000 obsdboot.efi
> >> smc911x: MAC 52:54:00:12:34:56
> >> smc911x: detected LAN9118 controller
> >> smc911x: phy initialized
> >> smc911x: MAC 52:54:00:12:34:56
> >> Using smc911x-0 device
> >> TFTP from server 10.0.2.2; our IP address is 10.0.2.15
> >> Filename 'obsdboot.efi'.
> >> Load address: 0x80400000
> >> Loading: *%08#####
> >> 12.4 MiB/s
> >> done
> >> Bytes transferred = 64908 (fd8c hex)
> >> smc911x: MAC 52:54:00:12:34:56
> >> => crc32 80400000 $filesize
> >> CRC32 for 80400000 ... 8040fd8b ==> a9ac4fcf
> >> => bootefi 80400000
> >> ## Starting EFI application at 80400000 ...
> >> WARNING: Invalid device tree, expect boot to fail
> >> BS->LocateHandle() returns 0
> >> undefined instruction
> >> pc : [<9eec65c4>]   lr : [<9eeca390>]
> >> sp : 9fed7a18  ip : 0000003f fp : 9fed7a2c
> >> r10: 00000000  r9 : 9eed4658 r8 : 00000000
> >> r7 : 9eed1ce4  r6 : 9eed3538 r5 : 9fed7a6c  r4 : 9eed4658
> >> r3 : 00000000  r2 : 9eed2f8e r1 : 9eed1ee0  r0 : 00000000
> >> Flags: nZCv  IRQs off  FIQs off  Mode SVC_32
> >> Resetting CPU ...
> >>
> >> resetting ...
> >>
> >>
> >> U-Boot 2017.09-rc1-00025-g534695d189 (Aug 06 2017 - 06:58:16 -0400)
> >>
> >> DRAM:  1 GiB
> >> WARNING: Caches not enabled
> >> Flash: 128 MiB
> >> MMC:   MMC: 0
> >> *** Warning - bad CRC, using default environment
> >>
> >> In:    serial
> >> Out:   serial
> >> Err:   serial
> >> Net:   smc911x-0
> >> Hit any key to stop autoboot:  2
> >
> > Why does U-Boot not set fdt_addr_r or fdtfile for vexpress?  Worse yet
> > trying to load to the default kernel_addr_r fails.  So it requires a
> > script or manual commands to boot instead of the usual distro boot
> > arrangement?
> 
> I suspect this is specific to the test framework (probably not
> enabling distro-boot-cmd so that the test framework can run the cmds
> it wants??)

I didn't attempt to use the test framework, just loaded the result
of building vexpress_ca15_tc2_defconfig into qemu.

It looks like the load addresses are only good for >= 1G physmem.

U-Boot 2017.09-rc1 (Aug 02 2017 - 10:55:19 +1000)

DRAM:  128 MiB
WARNING: Caches not enabled
Flash: 128 MiB
MMC:   MMC: 0
*** Warning - bad CRC, using default environment

In:    serial
Out:   serial
Err:   serial
Net:   smc911x-0
Hit any key to stop autoboot:  0
=> printenv
arch=arm
baudrate=38400
board=vexpress
board_name=vexpress
boot_a_script=load ${devtype} ${devnum}:${distro_bootpart} ${scriptaddr} ${prefix}${script}; source ${scriptaddr}
boot_efi_binary=load ${devtype} ${devnum}:${distro_bootpart} ${kernel_addr_r} efi/boot/bootarm.efi; if fdt addr ${fdt_addr_r}; then bootefi ${kernel_addr_r} ${fdt_addr_r};else bootefi ${kernel_addr_r} ${fdtcontroladdr};fi
boot_extlinux=sysboot ${devtype} ${devnum}:${distro_bootpart} any ${scriptaddr} ${prefix}extlinux/extlinux.conf
boot_prefixes=/ /boot/
boot_script_dhcp=boot.scr.uimg
boot_scripts=boot.scr.uimg boot.scr
boot_targets=mmc1 mmc0 pxe dhcp
bootcmd=run distro_bootcmd; run bootflash;
bootcmd_dhcp=if dhcp ${scriptaddr} ${boot_script_dhcp}; then source ${scriptaddr}; fi;setenv efi_fdtfile ${fdtfile}; if test -z "${fdtfile}" -a -n "${soc}"; then setenv efi_fdtfile ${soc}-${board}${boardver}.dtb; fi; setenv efi_old_vci ${bootp_vci};setenv efi_old_arch ${bootp_arch};setenv bootp_vci PXEClient:Arch:00010:UNDI:003000;setenv bootp_arch 0xa;if dhcp ${kernel_addr_r}; then tftpboot ${fdt_addr_r} dtb/${efi_fdtfile};if fdt addr ${fdt_addr_r}; then bootefi ${kernel_addr_r} ${fdt_addr_r}; else bootefi ${kernel_addr_r} ${fdtcontroladdr};fi;fi;setenv bootp_vci ${efi_old_vci};setenv bootp_arch ${efi_old_arch};setenv efi_fdtfile;setenv efi_old_arch;setenv efi_old_vci;
bootcmd_mmc0=setenv devnum 0; run mmc_boot
bootcmd_mmc1=setenv devnum 1; run mmc_boot
bootcmd_pxe=dhcp; if pxe get; then pxe boot; fi
bootdelay=2
bootflash=run flashargs; cp ${ramdisk_addr} ${ramdisk_addr_r} ${maxramdisk}; bootm ${kernel_addr} ${ramdisk_addr_r}
console=ttyAMA0,38400n8
cpu=armv7
distro_bootcmd=for target in ${boot_targets}; do run bootcmd_${target}; done
dram=1024M
efi_dtb_prefixes=/ /dtb/ /dtb/current/
ethact=smc911x-0
ethaddr=52:54:00:12:34:56
flashargs=setenv bootargs root=${root} console=${console} mem=${dram} mtdparts=${mtd} mmci.fmax=190000 devtmpfs.mount=0	 vmalloc=256M
kernel_addr=0x0c100000
kernel_addr_r=0xa0008000
load_efi_dtb=load ${devtype} ${devnum}:${distro_bootpart} ${fdt_addr_r} ${prefix}${efi_fdtfile}
loadaddr=0xa0008000
maxramdisk=0x1800000
mmc_boot=if mmc dev ${devnum}; then setenv devtype mmc; run scan_dev_for_boot_part; fi
mtd=armflash:1M@0x800000(uboot),7M@0x1000000(kernel),24M@0x2000000(initrd)
pxefile_addr_r=0xa8000000
ramdisk_addr=0x0c800000
ramdisk_addr_r=0x81000000
root=/dev/sda1 rw
scan_dev_for_boot=echo Scanning ${devtype} ${devnum}:${distro_bootpart}...; for prefix in ${boot_prefixes}; do run scan_dev_for_extlinux; run scan_dev_for_scripts; done;run scan_dev_for_efi;
scan_dev_for_boot_part=part list ${devtype} ${devnum} -bootable devplist; env exists devplist || setenv devplist 1; for distro_bootpart in ${devplist}; do if fstype ${devtype} ${devnum}:${distro_bootpart} bootfstype; then run scan_dev_for_boot; fi; done
scan_dev_for_efi=setenv efi_fdtfile ${fdtfile}; if test -z "${fdtfile}" -a -n "${soc}"; then setenv efi_fdtfile ${soc}-${board}${boardver}.dtb; fi; for prefix in ${efi_dtb_prefixes}; do if test -e ${devtype} ${devnum}:${distro_bootpart} ${prefix}${efi_fdtfile}; then run load_efi_dtb; fi;done;if test -e ${devtype} ${devnum}:${distro_bootpart} efi/boot/bootarm.efi; then echo Found EFI removable media binary efi/boot/bootarm.efi; run boot_efi_binary; echo EFI LOAD FAILED: continuing...; fi; setenv efi_fdtfile
scan_dev_for_extlinux=if test -e ${devtype} ${devnum}:${distro_bootpart} ${prefix}extlinux/extlinux.conf; then echo Found ${prefix}extlinux/extlinux.conf; run boot_extlinux; echo SCRIPT FAILED: continuing...; fi
scan_dev_for_scripts=for script in ${boot_scripts}; do if test -e ${devtype} ${devnum}:${distro_bootpart} ${prefix}${script}; then echo Found U-Boot script ${prefix}${script}; run boot_a_script; echo SCRIPT FAILED: continuing...; fi; done
scriptaddr=0xa8000000
stderr=serial
stdin=serial
stdout=serial
vendor=armltd

Environment size: 3974/262140 bytes
=> bdinfo
arch_number = 0x000008E0
boot_params = 0x80002000
DRAM bank   = 0x00000000
-> start    = 0x80000000
-> size     = 0x08000000
DRAM bank   = 0x00000001
-> start    = 0xA0000000
-> size     = 0x00000004
eth0name    = smc911x-0
ethaddr     = 52:54:00:12:34:56
current eth = smc911x-0
ip_addr     = <NULL>
baudrate    = 38400 bps
TLB addr    = 0x87FF0000
relocaddr   = 0x87F7B000
reloc off   = 0x0777B000
irq_sp      = 0x87EDAEF0
sp start    = 0x87EDAEE0

With -m 512

U-Boot 2017.09-rc1 (Aug 02 2017 - 10:55:19 +1000)

DRAM:  512 MiB
WARNING: Caches not enabled
Flash: 128 MiB
MMC:   MMC: 0
*** Warning - bad CRC, using default environment

In:    serial
Out:   serial
Err:   serial
Net:   smc911x-0
Hit any key to stop autoboot:  0 
=> bdinfo
arch_number = 0x000008E0
boot_params = 0x80002000
DRAM bank   = 0x00000000
-> start    = 0x80000000
-> size     = 0x20000000
DRAM bank   = 0x00000001
-> start    = 0xA0000000
-> size     = 0x00000004
eth0name    = smc911x-0
ethaddr     = 52:54:00:12:34:56
current eth = smc911x-0
ip_addr     = <NULL>
baudrate    = 38400 bps
TLB addr    = 0x9FFF0000
relocaddr   = 0x9FF7B000
reloc off   = 0x1F77B000
irq_sp      = 0x9FEDAEF0
sp start    = 0x9FEDAEE0

With -m 1024

U-Boot 2017.09-rc1 (Aug 02 2017 - 10:55:19 +1000)

DRAM:  1 GiB
WARNING: Caches not enabled
Flash: 128 MiB
MMC:   MMC: 0
*** Warning - bad CRC, using default environment

In:    serial
Out:   serial
Err:   serial
Net:   smc911x-0
Hit any key to stop autoboot:  0 
=> bdinfo
arch_number = 0x000008E0
boot_params = 0x80002000
DRAM bank   = 0x00000000
-> start    = 0x80000000
-> size     = 0x20000000
DRAM bank   = 0x00000001
-> start    = 0xA0000000
-> size     = 0x20000000
eth0name    = smc911x-0
ethaddr     = 52:54:00:12:34:56
current eth = smc911x-0
ip_addr     = <NULL>
baudrate    = 38400 bps
TLB addr    = 0x9FFF0000
relocaddr   = 0x9FF7B000
reloc off   = 0x1F77B000
irq_sp      = 0x9FEDAEF0
sp start    = 0x9FEDAEE0

-m 1024 works with ${kernel_addr_r} instead of giving
"efi_load_pe: Invalid DOS Signature"

=> setenv fdt_addr_r 0x81000000
=> setenv fdtfile vexpress-v2p-ca15-tc1.dtb
=> load mmc 0:1 ${fdt_addr_r} ${fdtfile}
reading vexpress-v2p-ca15-tc1.dtb
13384 bytes read in 22 ms (593.8 KiB/s)
=> load mmc 0:1 ${kernel_addr_r} efi/boot/bootarm.efi
reading efi/boot/bootarm.efi
64908 bytes read in 49 ms (1.3 MiB/s)
=> bootefi ${kernel_addr_r} ${fdt_addr_r}
## Starting EFI application at a0008000 ...
Scanning disks on mmc...
MMC Device 1 not found
MMC Device 2 not found
MMC Device 3 not found
Found 1 disks
>> OpenBSD/armv7 BOOTARM 0.8
boot> 
cannot open sd0a:/etc/random.seed: No such file or directory
booting sd0a:/bsd: 2265112+7989364+447000 [80+314496+149702]=0xaad1c0

OpenBSD/armv7 booting ...
arg0 0xc0dad1c0 arg1 0x8e0 arg2 0x88000000
Allocating page tables
freestart = 0x80dae000, free_pages = 127570 (0x0001f252)
IRQ stack: p0x80ddc000 v0xc0ddc000
ABT stack: p0x80ddd000 v0xc0ddd000
UND stack: p0x80dde000 v0xc0dde000
SVC stack: p0x80ddf000 v0xc0ddf000
Creating L1 page table at 0x80db0000
Mapping kernel
Constructing L2 page tables
undefined page pmap board type: 2272
Copyright (c) 1982, 1986, 1989, 1991, 1993
        The Regents of the University of California.  All rights reserved.
Copyright (c) 1995-2017 OpenBSD. All rights reserved.  https://www.OpenBSD.org

OpenBSD 6.1-current (RAMDISK) #30: Sat Aug  5 22:01:16 MDT 2017
    deraadt@armv7.openbsd.org:/usr/src/sys/arch/armv7/compile/RAMDISK
real mem  = 1073741824 (1024MB)
avail mem = 1038520320 (990MB)
mainbus0 at root: V2P-CA15
cpu0 at mainbus0: ARM Cortex-A15 r2p1 (ARMv7)
cpu0: DC enabled IC enabled WB disabled EABT branch prediction enabled
cpu0: 32KB(64b/l,2way) I-cache, 32KB(64b/l,2way) wr-back D-cache
cortex0 at mainbus0
ampintc0 at mainbus0 nirq 160, ncpu 1
agtimer0 at mainbus0: tick rate 62500 KHz
simplebus0 at mainbus0: "smb"
simplebus1 at simplebus0: "motherboard"
simplebus2 at simplebus1: "iofpga"
sysreg0 at simplebus2: ID 0x1190f500 PROCID0 0x14000237
pluart0 at simplebus2: console
pluart1 at simplebus2
pluart2 at simplebus2
pluart3 at simplebus2
plrtc0 at simplebus2
simplebus3 at mainbus0: "hsb"
boot device: lookup 'sd0a:/bsd' failed.
root on rd0a swap on rd0b dump on rd0b
e
diff mbox

Patch

diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index ed06485e33..eea65a402a 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -28,11 +28,13 @@  struct efi_disk_obj {
 	/* EFI Interface Media descriptor struct, referenced by ops */
 	struct efi_block_io_media media;
 	/* EFI device path to this block device */
-	struct efi_device_path_file_path *dp;
+	struct efi_device_path *dp;
+	/* partition # */
+	unsigned part;
 	/* Offset into disk for simple partitions */
 	lbaint_t offset;
 	/* Internal block device */
-	const struct blk_desc *desc;
+	struct blk_desc *desc;
 };
 
 static efi_status_t EFIAPI efi_disk_reset(struct efi_block_io *this,
@@ -172,26 +174,26 @@  static const struct efi_block_io block_io_disk_template = {
 
 static void efi_disk_add_dev(const char *name,
 			     const char *if_typename,
-			     const struct blk_desc *desc,
+			     struct blk_desc *desc,
 			     int dev_index,
-			     lbaint_t offset)
+			     lbaint_t offset,
+			     unsigned part)
 {
 	struct efi_disk_obj *diskobj;
-	struct efi_device_path_file_path *dp;
-	int objlen = sizeof(*diskobj) + (sizeof(*dp) * 2);
 
 	/* Don't add empty devices */
 	if (!desc->lba)
 		return;
 
-	diskobj = calloc(1, objlen);
+	diskobj = calloc(1, sizeof(*diskobj));
 
 	/* Fill in object data */
-	dp = (void *)&diskobj[1];
+	diskobj->dp = efi_dp_from_part(desc, part);
+	diskobj->part = part;
 	diskobj->parent.protocols[0].guid = &efi_block_io_guid;
 	diskobj->parent.protocols[0].protocol_interface = &diskobj->ops;
 	diskobj->parent.protocols[1].guid = &efi_guid_device_path;
-	diskobj->parent.protocols[1].protocol_interface = dp;
+	diskobj->parent.protocols[1].protocol_interface = diskobj->dp;
 	diskobj->parent.handle = diskobj;
 	diskobj->ops = block_io_disk_template;
 	diskobj->ifname = if_typename;
@@ -207,17 +209,6 @@  static void efi_disk_add_dev(const char *name,
 	diskobj->media.last_block = desc->lba - offset;
 	diskobj->ops.media = &diskobj->media;
 
-	/* Fill in device path */
-	diskobj->dp = dp;
-	dp[0].dp.type = DEVICE_PATH_TYPE_MEDIA_DEVICE;
-	dp[0].dp.sub_type = DEVICE_PATH_SUB_TYPE_FILE_PATH;
-	dp[0].dp.length = sizeof(*dp);
-	ascii2unicode(dp[0].str, name);
-
-	dp[1].dp.type = DEVICE_PATH_TYPE_END;
-	dp[1].dp.sub_type = DEVICE_PATH_SUB_TYPE_END;
-	dp[1].dp.length = sizeof(*dp);
-
 	/* Hook up to the device list */
 	list_add_tail(&diskobj->parent.link, &efi_obj_list);
 }
@@ -236,14 +227,18 @@  static int efi_disk_create_eltorito(struct blk_desc *desc,
 	if (desc->part_type != PART_TYPE_ISO)
 		return 0;
 
+	/* and devices for each partition: */
 	while (!part_get_info(desc, part, &info)) {
 		snprintf(devname, sizeof(devname), "%s:%d", pdevname,
 			 part);
 		efi_disk_add_dev(devname, if_typename, desc, diskid,
-				 info.start);
+				 info.start, part);
 		part++;
 		disks++;
 	}
+
+	/* ... and add block device: */
+	efi_disk_add_dev(devname, if_typename, desc, diskid, 0, 0);
 #endif
 
 	return disks;
@@ -271,9 +266,22 @@  int efi_disk_register(void)
 	     uclass_next_device_check(&dev)) {
 		struct blk_desc *desc = dev_get_uclass_platdata(dev);
 		const char *if_typename = dev->driver->name;
+		disk_partition_t info;
+		int part = 1;
 
 		printf("Scanning disk %s...\n", dev->name);
-		efi_disk_add_dev(dev->name, if_typename, desc, desc->devnum, 0);
+
+		/* add devices for each partition: */
+		while (!part_get_info(desc, part, &info)) {
+			efi_disk_add_dev(dev->name, if_typename, desc,
+					desc->devnum, 0, part);
+			part++;
+		}
+
+		/* ... and add block device: */
+		efi_disk_add_dev(dev->name, if_typename, desc,
+				desc->devnum, 0, 0);
+
 		disks++;
 
 		/*
@@ -309,7 +317,7 @@  int efi_disk_register(void)
 
 			snprintf(devname, sizeof(devname), "%s%d",
 				 if_typename, i);
-			efi_disk_add_dev(devname, if_typename, desc, i, 0);
+			efi_disk_add_dev(devname, if_typename, desc, i, 0, 0);
 			disks++;
 
 			/*