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

[Jonathan Gray]

On Mon, Aug 07, 2017 at 12:16:54PM -0400, Rob Clark wrote:
> On Mon, Aug 7, 2017 at 11:47 AM, Jonathan Gray  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  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 8040 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: 0x8040
> >> Loading: *%08#
> >> 12.4 MiB/s
> >> done
> >> Bytes transferred = 64908 (fd8c hex)
> >> smc911x: MAC 52:54:00:12:34:56
> >> => crc32 8040 $filesize
> >> CRC32 for 8040 ... 8040fd8b ==> a9ac4fcf
> >> => bootefi 8040
> >> ## Starting EFI application at 8040 ...
> >> WARNING: Invalid device tree, expect boot to fail
> >> BS->LocateHandle() returns 0
> >> undefined instruction
> >> pc : [<9eec65c4>]   lr : [<9eeca390>]
> >> sp : 9fed7a18  ip : 003f fp : 9fed7a2c
> >> r10:   r9 : 9eed4658 r8 : 
> >> r7 : 9eed1ce4  r6 : 9eed3538 r5 : 9fed7a6c  r4 : 9eed4658
> >> r3 :   r2 : 9eed2f8e r1 : 9eed1ee0  r0 : 
> >> 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=19 

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

[Peter Jones]

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.

-- 
Peter
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

[Rob Clark]

On Mon, Aug 7, 2017 at 11:47 AM, Jonathan Gray  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  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 8040 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: 0x8040
>> Loading: *%08#
>> 12.4 MiB/s
>> done
>> Bytes transferred = 64908 (fd8c hex)
>> smc911x: MAC 52:54:00:12:34:56
>> => crc32 8040 $filesize
>> CRC32 for 8040 ... 8040fd8b ==> a9ac4fcf
>> => bootefi 8040
>> ## Starting EFI application at 8040 ...
>> WARNING: Invalid device tree, expect boot to fail
>> BS->LocateHandle() returns 0
>> undefined instruction
>> pc : [<9eec65c4>]   lr : [<9eeca390>]
>> sp : 9fed7a18  ip : 003f fp : 9fed7a2c
>> r10:   r9 : 9eed4658 r8 : 
>> r7 : 9eed1ce4  r6 : 9eed3538 r5 : 9fed7a6c  r4 : 9eed4658
>> r3 :   r2 : 9eed2f8e r1 : 9eed1ee0  r0 : 
>> 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 0x8100
> => 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 

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

[Jonathan Gray]

On Sun, Aug 06, 2017 at 11:34:15AM -0400, Rob Clark wrote:
> On Sun, Aug 6, 2017 at 10:45 AM, Rob Clark  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 8040 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: 0x8040
> Loading: *%08#
> 12.4 MiB/s
> done
> Bytes transferred = 64908 (fd8c hex)
> smc911x: MAC 52:54:00:12:34:56
> => crc32 8040 $filesize
> CRC32 for 8040 ... 8040fd8b ==> a9ac4fcf
> => bootefi 8040
> ## Starting EFI application at 8040 ...
> WARNING: Invalid device tree, expect boot to fail
> BS->LocateHandle() returns 0
> undefined instruction
> pc : [<9eec65c4>]   lr : [<9eeca390>]
> sp : 9fed7a18  ip : 003f fp : 9fed7a2c
> r10:   r9 : 9eed4658 r8 : 
> r7 : 9eed1ce4  r6 : 9eed3538 r5 : 9fed7a6c  r4 : 9eed4658
> r3 :   r2 : 9eed2f8e r1 : 9eed1ee0  r0 : 
> 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 0x8100
=> 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 0x8200
=> 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 8200 ...
Scanning disks on mmc...
MMC Device 1 not found
MMC Device 2 no

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

[Rob Clark]

On Sun, Aug 6, 2017 at 2:47 PM, Rob Clark  wrote:
> On Sun, Aug 6, 2017 at 2:37 PM, Mark Kettenis  wrote:
>>> Date: Sun, 6 Aug 2017 20:21:45 +0200 (CEST)
>>> From: Mark Kettenis 
>>>
>>> > 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 I fix the code to not insert the partition number there, I can boot
>> from SD card and SATA again.
>>
>> diff --git a/lib/efi_loader/efi_device_path.c 
>> b/lib/efi_loader/efi_device_path.c
>> index b5acf73f98..8ba0db2d7a 100644
>> --- a/lib/efi_loader/efi_device_path.c
>> +++ b/lib/efi_loader/efi_device_path.c
>> @@ -305,8 +305,8 @@ static void *dp_part_fill(void *buf, struct blk_desc 
>> *desc, int part)
>> struct efi_device_path_file_path *fp;
>> char devname[32] = { 0 }; /* fp->str is u16[32] long */
>>
>> -   snprintf(devname, sizeof(devname), "%d.%d.%d", desc->if_type,
>> -desc->devnum, part);
>> +   snprintf(devname, sizeof(devname), "%d.%d", desc->if_type,
>> +desc->devnum);
>>
>> memcpy(buf, &ROOT, sizeof(ROOT));
>> buf += sizeof(ROOT);
>
>
> Hmm, that is probably not a good idea, since now the disk object along
> w/ partition objects will have same devicepath.  (One change from
> before is now we have diskobjs for the disk (part=0) and child
> diskobjs for each partition.. fwiw in UEFI terminology a
> efi_device_path_hard_drive_path is actually a partition.. for maximum
> confusion)

Oh, scratch that.. you are right, part shouldn't be in the string..
since it results in the children having different parent device paths.

(It is still probably a bit funny to file-paths for parent of the
partition objects.. I'll try to come up with something a bit better.)

BR,
-R
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

[Rob Clark]

On Sun, Aug 6, 2017 at 2:37 PM, Mark Kettenis  wrote:
>> Date: Sun, 6 Aug 2017 20:21:45 +0200 (CEST)
>> From: Mark Kettenis 
>>
>> > 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 I fix the code to not insert the partition number there, I can boot
> from SD card and SATA again.
>
> diff --git a/lib/efi_loader/efi_device_path.c 
> b/lib/efi_loader/efi_device_path.c
> index b5acf73f98..8ba0db2d7a 100644
> --- a/lib/efi_loader/efi_device_path.c
> +++ b/lib/efi_loader/efi_device_path.c
> @@ -305,8 +305,8 @@ static void *dp_part_fill(void *buf, struct blk_desc 
> *desc, int part)
> struct efi_device_path_file_path *fp;
> char devname[32] = { 0 }; /* fp->str is u16[32] long */
>
> -   snprintf(devname, sizeof(devname), "%d.%d.%d", desc->if_type,
> -desc->devnum, part);
> +   snprintf(devname, sizeof(devname), "%d.%d", desc->if_type,
> +desc->devnum);
>
> memcpy(buf, &ROOT, sizeof(ROOT));
> buf += sizeof(ROOT);


Hmm, that is probably not a good idea, since now the disk object along
w/ partition objects will have same devicepath.  (One change from
before is now we have diskobjs for the disk (part=0) and child
diskobjs for each partition.. fwiw in UEFI terminology a
efi_device_path_hard_drive_path is actually a partition.. for maximum
confusion)

Probably that we have a file-path node w/ child hard-drive objects is
confusing your previous workaround.. let me see if I can come up with
something better.

BR,
-R
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

[Rob Clark]

On Sun, Aug 6, 2017 at 2:21 PM, Mark Kettenis  wrote:
>> From: Rob Clark 
>> Date: Sun, 6 Aug 2017 13:49:43 -0400
>>
>> On Sun, Aug 6, 2017 at 1:28 PM, Mark Kettenis  
>> wrote:
>> >> From: Rob Clark 
>> >> Date: Sun, 6 Aug 2017 11:34:15 -0400
>> >>
>> >> On Sun, Aug 6, 2017 at 10:45 AM, Rob Clark  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
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

[Mark Kettenis]

> Date: Sun, 6 Aug 2017 20:21:45 +0200 (CEST)
> From: Mark Kettenis 
> 
> > 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 I fix the code to not insert the partition number there, I can boot
from SD card and SATA again.

diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
index b5acf73f98..8ba0db2d7a 100644
--- a/lib/efi_loader/efi_device_path.c
+++ b/lib/efi_loader/efi_device_path.c
@@ -305,8 +305,8 @@ static void *dp_part_fill(void *buf, struct blk_desc *desc, 
int part)
struct efi_device_path_file_path *fp;
char devname[32] = { 0 }; /* fp->str is u16[32] long */
 
-   snprintf(devname, sizeof(devname), "%d.%d.%d", desc->if_type,
-desc->devnum, part);
+   snprintf(devname, sizeof(devname), "%d.%d", desc->if_type,
+desc->devnum);
 
memcpy(buf, &ROOT, sizeof(ROOT));
buf += sizeof(ROOT);
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

[Mark Kettenis]

> From: Rob Clark 
> Date: Sun, 6 Aug 2017 13:49:43 -0400
> 
> On Sun, Aug 6, 2017 at 1:28 PM, Mark Kettenis  wrote:
> >> From: Rob Clark 
> >> Date: Sun, 6 Aug 2017 11:34:15 -0400
> >>
> >> On Sun, Aug 6, 2017 at 10:45 AM, Rob Clark  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_TARGE

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

[Peter Robinson]

On Sun, Aug 6, 2017 at 6:49 PM, Rob Clark  wrote:
> On Sun, Aug 6, 2017 at 1:28 PM, Mark Kettenis  wrote:
>>> From: Rob Clark 
>>> Date: Sun, 6 Aug 2017 11:34:15 -0400
>>>
>>> On Sun, Aug 6, 2017 at 10:45 AM, Rob Clark  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
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

[Rob Clark]

On Sun, Aug 6, 2017 at 1:28 PM, Mark Kettenis  wrote:
>> From: Rob Clark 
>> Date: Sun, 6 Aug 2017 11:34:15 -0400
>>
>> On Sun, Aug 6, 2017 at 10:45 AM, Rob Clark  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
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

[Mark Kettenis]

> From: Rob Clark 
> Date: Sun, 6 Aug 2017 11:34:15 -0400
> 
> On Sun, Aug 6, 2017 at 10:45 AM, Rob Clark  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 8040 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: 0x8040
> Loading: *%08#
> 12.4 MiB/s
> done
> Bytes transferred = 64908 (fd8c hex)
> smc911x: MAC 52:54:00:12:34:56
> => crc32 8040 $filesize
> CRC32 for 8040 ... 8040fd8b ==> a9ac4fcf
> => bootefi 8040
> ## Starting EFI application at 8040 ...
> WARNING: Invalid device tree, expect boot to fail
> BS->LocateHandle() returns 0
> undefined instruction
> pc : [<9eec65c4>]   lr : [<9eeca390>]
> sp : 9fed7a18  ip : 003f fp : 9fed7a2c
> r10:   r9 : 9eed4658 r8 : 
> r7 : 9eed1ce4  r6 : 9eed3538 r5 : 9fed7a6c  r4 : 9eed4658
> r3 :   r2 : 9eed2f8e r1 : 9eed1ee0  r0 : 
> 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
> 
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

[Jonathan Gray]

On Sun, Aug 06, 2017 at 11:34:15AM -0400, Rob Clark wrote:
> On Sun, Aug 6, 2017 at 10:45 AM, Rob Clark  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
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

[Heinrich Schuchardt]

On 08/06/2017 05:34 PM, Rob Clark wrote:
> On Sun, Aug 6, 2017 at 10:45 AM, Rob Clark  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 8040 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: 0x8040
> Loading: *%08#
> 12.4 MiB/s
> done
> Bytes transferred = 64908 (fd8c hex)
> smc911x: MAC 52:54:00:12:34:56
> => crc32 8040 $filesize
> CRC32 for 8040 ... 8040fd8b ==> a9ac4fcf
> => bootefi 8040
> ## Starting EFI application at 8040 ...
> WARNING: Invalid device tree, expect boot to fail
> BS->LocateHandle() returns 0
> undefined instruction
> pc : [<9eec65c4>]   lr : [<9eeca390>]
> sp : 9fed7a18  ip : 003f fp : 9fed7a2c
> r10:   r9 : 9eed4658 r8 : 
> r7 : 9eed1ce4  r6 : 9eed3538 r5 : 9fed7a6c  r4 : 9eed4658
> r3 :   r2 : 9eed2f8e r1 : 9eed1ee0  r0 : 
> 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
> 

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

[Rob Clark]

On Sun, Aug 6, 2017 at 10:45 AM, Rob Clark  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 8040 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: 0x8040
Loading: *%08#
12.4 MiB/s
done
Bytes transferred = 64908 (fd8c hex)
smc911x: MAC 52:54:00:12:34:56
=> crc32 8040 $filesize
CRC32 for 8040 ... 8040fd8b ==> a9ac4fcf
=> bootefi 8040
## Starting EFI application at 8040 ...
WARNING: Invalid device tree, expect boot to fail
BS->LocateHandle() returns 0
undefined instruction
pc : [<9eec65c4>]   lr : [<9eeca390>]
sp : 9fed7a18  ip : 003f fp : 9fed7a2c
r10:   r9 : 9eed4658 r8 : 
r7 : 9eed1ce4  r6 : 9eed3538 r5 : 9fed7a6c  r4 : 9eed4658
r3 :   r2 : 9eed2f8e r1 : 9eed1ee0  r0 : 
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
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

[Rob Clark]

On Sun, Aug 6, 2017 at 10:28 AM, Mark Kettenis  wrote:
>> Date: Sun, 6 Aug 2017 15:16:09 +0200 (CEST)
>> From: Mark Kettenis 
>>
>> 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: 0005  r9 : 7af35ee0 r8 : 7efb4490
> r7 : 7af695e8  r6 : 7af69620 r5 : 005c  r4 : 7af2b828
> r3 : 7efae477  r2 : 005c r1 : 002f  r0 : 
> 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
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

[Mark Kettenis]

> Date: Sun, 6 Aug 2017 15:16:09 +0200 (CEST)
> From: Mark Kettenis 
> 
> 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: 0005  r9 : 7af35ee0 r8 : 7efb4490
r7 : 7af695e8  r6 : 7af69620 r5 : 005c  r4 : 7af2b828
r3 : 7efae477  r2 : 005c r1 : 002f  r0 : 
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.
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

[Rob Clark]

On Sun, Aug 6, 2017 at 10:17 AM, Rob Clark  wrote:
> On Sun, Aug 6, 2017 at 9:16 AM, Mark Kettenis  wrote:
>>> From: Rob Clark 
>>> Date: Sat, 5 Aug 2017 11:36:25 -0400
>>>
>>> On Sat, Aug 5, 2017 at 11:24 AM, Rob Clark  wrote:
>>> > On Sat, Aug 5, 2017 at 11:10 AM, Heinrich Schuchardt  
>>> > wrote:
>>> >> On 08/05/2017 04:35 PM, Rob Clark wrote:
>>> >>> On Sat, Aug 5, 2017 at 10:28 AM, Mark Kettenis 
>>> >>>  wrote:
>>> > Date: Sat, 5 Aug 2017 16:01:51 +0200 (CEST)
>>> > From: Mark Kettenis 
>>> >
>>> > 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 
>>>  Date:   Wed Jun 21 16:39:02 2017 -0400
>>> 
>>>  efi: add some more device path structures
>>> 
>>>  Signed-off-by: Peter Jones 
>>>  Signed-off-by: Rob Clark 
>>> >>>
>>> >>>
>>> >>> 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
>>> >>
>>> >> 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
>>> >>
>>> >> 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 :  fp : 7af29774
>> r10: 7efec230  r9 : 7af33ee0 r8 : 
>> r7 : 0009  r6 : 7ef9e8b8 r5 : 7af296a0  r4 : 7efa4495
>> r3 : 7af296a0  r2 : 008c r1 : 7af29658  r0 : 0004
>> Flags: nzCV  IRQs off  FIQs off

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

[Rob Clark]

On Sun, Aug 6, 2017 at 9:16 AM, Mark Kettenis  wrote:
>> From: Rob Clark 
>> Date: Sat, 5 Aug 2017 11:36:25 -0400
>>
>> On Sat, Aug 5, 2017 at 11:24 AM, Rob Clark  wrote:
>> > On Sat, Aug 5, 2017 at 11:10 AM, Heinrich Schuchardt  
>> > wrote:
>> >> On 08/05/2017 04:35 PM, Rob Clark wrote:
>> >>> On Sat, Aug 5, 2017 at 10:28 AM, Mark Kettenis  
>> >>> wrote:
>> > Date: Sat, 5 Aug 2017 16:01:51 +0200 (CEST)
>> > From: Mark Kettenis 
>> >
>> > 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 
>>  Date:   Wed Jun 21 16:39:02 2017 -0400
>> 
>>  efi: add some more device path structures
>> 
>>  Signed-off-by: Peter Jones 
>>  Signed-off-by: Rob Clark 
>> >>>
>> >>>
>> >>> 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
>> >>
>> >> 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
>> >>
>> >> 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 :  fp : 7af29774
> r10: 7efec230  r9 : 7af33ee0 r8 : 
> r7 : 0009  r6 : 7ef9e8b8 r5 : 7af296a0  r4 : 7efa4495
> r3 : 7af296a0  r2 : 008c r1 : 7af29658  r0 : 0004
> 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-bo

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

[Mark Kettenis]

> From: Rob Clark 
> Date: Sat, 5 Aug 2017 11:36:25 -0400
> 
> On Sat, Aug 5, 2017 at 11:24 AM, Rob Clark  wrote:
> > On Sat, Aug 5, 2017 at 11:10 AM, Heinrich Schuchardt  
> > wrote:
> >> On 08/05/2017 04:35 PM, Rob Clark wrote:
> >>> On Sat, Aug 5, 2017 at 10:28 AM, Mark Kettenis  
> >>> wrote:
> > Date: Sat, 5 Aug 2017 16:01:51 +0200 (CEST)
> > From: Mark Kettenis 
> >
> > 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 
>  Date:   Wed Jun 21 16:39:02 2017 -0400
> 
>  efi: add some more device path structures
> 
>  Signed-off-by: Peter Jones 
>  Signed-off-by: Rob Clark 
> >>>
> >>>
> >>> 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
> >>
> >> 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
> >>
> >> 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 :  fp : 7af29774
r10: 7efec230  r9 : 7af33ee0 r8 : 
r7 : 0009  r6 : 7ef9e8b8 r5 : 7af296a0  r4 : 7efa4495
r3 : 7af296a0  r2 : 008c r1 : 7af29658  r0 : 0004
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:   e1d430b4ldrhr3, [r4, #4]

and with r4 = 7efa4495 that will defenitely trap.

Looking at the defenition efi_guid_t in u-boot:

  typedef struct 

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

[Mark Kettenis]

> From: Rob Clark 
> Date: Sat, 5 Aug 2017 10:28:34 -0400
> 
> On Sat, Aug 5, 2017 at 10:01 AM, Mark Kettenis  
> 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 8100 ...
> Scanning disk sd...@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).
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

[Rob Clark]

On Sat, Aug 5, 2017 at 12:02 PM, Heinrich Schuchardt  wrote:
> On 08/05/2017 05:22 PM, Rob Clark wrote:
>> On Sat, Aug 5, 2017 at 11:08 AM, Mark Kettenis  
>> wrote:
 From: Rob Clark 
 Date: Sat, 5 Aug 2017 10:35:08 -0400

 On Sat, Aug 5, 2017 at 10:28 AM, Mark Kettenis  
 wrote:
>> Date: Sat, 5 Aug 2017 16:01:51 +0200 (CEST)
>> From: Mark Kettenis 
>>
>> 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 
> Date:   Wed Jun 21 16:39:02 2017 -0400
>
> efi: add some more device path structures
>
> Signed-off-by: Peter Jones 
> Signed-off-by: Rob Clark 


 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
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

[Heinrich Schuchardt]

On 08/05/2017 05:22 PM, Rob Clark wrote:
> On Sat, Aug 5, 2017 at 11:08 AM, Mark Kettenis  
> wrote:
>>> From: Rob Clark 
>>> Date: Sat, 5 Aug 2017 10:35:08 -0400
>>>
>>> On Sat, Aug 5, 2017 at 10:28 AM, Mark Kettenis  
>>> wrote:
> Date: Sat, 5 Aug 2017 16:01:51 +0200 (CEST)
> From: Mark Kettenis 
>
> 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 
 Date:   Wed Jun 21 16:39:02 2017 -0400

 efi: add some more device path structures

 Signed-off-by: Peter Jones 
 Signed-off-by: Rob Clark 
>>>
>>>
>>> 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
> 

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

[Rob Clark]

On Sat, Aug 5, 2017 at 11:24 AM, Rob Clark  wrote:
> On Sat, Aug 5, 2017 at 11:10 AM, Heinrich Schuchardt  
> wrote:
>> On 08/05/2017 04:35 PM, Rob Clark wrote:
>>> On Sat, Aug 5, 2017 at 10:28 AM, Mark Kettenis  
>>> wrote:
> Date: Sat, 5 Aug 2017 16:01:51 +0200 (CEST)
> From: Mark Kettenis 
>
> 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 
 Date:   Wed Jun 21 16:39:02 2017 -0400

 efi: add some more device path structures

 Signed-off-by: Peter Jones 
 Signed-off-by: Rob Clark 
>>>
>>>
>>> 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
>>
>> 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
>>
>> 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
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

[Rob Clark]

On Sat, Aug 5, 2017 at 11:10 AM, Heinrich Schuchardt  wrote:
> On 08/05/2017 04:35 PM, Rob Clark wrote:
>> On Sat, Aug 5, 2017 at 10:28 AM, Mark Kettenis  
>> wrote:
 Date: Sat, 5 Aug 2017 16:01:51 +0200 (CEST)
 From: Mark Kettenis 

 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 
>>> Date:   Wed Jun 21 16:39:02 2017 -0400
>>>
>>> efi: add some more device path structures
>>>
>>> Signed-off-by: Peter Jones 
>>> Signed-off-by: Rob Clark 
>>
>>
>> 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
>
> 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
>
> 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
>>
>
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

[Rob Clark]

On Sat, Aug 5, 2017 at 11:08 AM, Mark Kettenis  wrote:
>> From: Rob Clark 
>> Date: Sat, 5 Aug 2017 10:35:08 -0400
>>
>> On Sat, Aug 5, 2017 at 10:28 AM, Mark Kettenis  
>> wrote:
>> >> Date: Sat, 5 Aug 2017 16:01:51 +0200 (CEST)
>> >> From: Mark Kettenis 
>> >>
>> >> 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 
>> > Date:   Wed Jun 21 16:39:02 2017 -0400
>> >
>> > efi: add some more device path structures
>> >
>> > Signed-off-by: Peter Jones 
>> > Signed-off-by: Rob Clark 
>>
>>
>> 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
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

[Heinrich Schuchardt]

On 08/05/2017 04:35 PM, Rob Clark wrote:
> On Sat, Aug 5, 2017 at 10:28 AM, Mark Kettenis  
> wrote:
>>> Date: Sat, 5 Aug 2017 16:01:51 +0200 (CEST)
>>> From: Mark Kettenis 
>>>
>>> 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 
>> Date:   Wed Jun 21 16:39:02 2017 -0400
>>
>> efi: add some more device path structures
>>
>> Signed-off-by: Peter Jones 
>> Signed-off-by: Rob Clark 
> 
> 
> 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

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

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
> 

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

[Mark Kettenis]

> From: Rob Clark 
> Date: Sat, 5 Aug 2017 10:35:08 -0400
> 
> On Sat, Aug 5, 2017 at 10:28 AM, Mark Kettenis  
> wrote:
> >> Date: Sat, 5 Aug 2017 16:01:51 +0200 (CEST)
> >> From: Mark Kettenis 
> >>
> >> 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 
> > Date:   Wed Jun 21 16:39:02 2017 -0400
> >
> > efi: add some more device path structures
> >
> > Signed-off-by: Peter Jones 
> > Signed-off-by: Rob Clark 
> 
> 
> 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.
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

[Rob Clark]

On Sat, Aug 5, 2017 at 10:28 AM, Mark Kettenis  wrote:
>> Date: Sat, 5 Aug 2017 16:01:51 +0200 (CEST)
>> From: Mark Kettenis 
>>
>> 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 
> Date:   Wed Jun 21 16:39:02 2017 -0400
>
> efi: add some more device path structures
>
> Signed-off-by: Peter Jones 
> Signed-off-by: Rob Clark 


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
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

[Mark Kettenis]

> Date: Sat, 5 Aug 2017 16:01:51 +0200 (CEST)
> From: Mark Kettenis 
> 
> 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 
Date:   Wed Jun 21 16:39:02 2017 -0400

efi: add some more device path structures

Signed-off-by: Peter Jones 
Signed-off-by: Rob Clark 
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

[Rob Clark]

On Sat, Aug 5, 2017 at 10:01 AM, Mark Kettenis  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 8100 ...
Scanning disk sd...@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>
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

[Rob Clark]

On Sat, Aug 5, 2017 at 10:01 AM, Mark Kettenis  wrote:
>>
>> On Fri, Aug 4, 2017 at 4:41 PM, Mark Kettenis  
>> wrote:
>> >> From: Rob Clark 
>> >> 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(sd...@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,dd904a8c,1,1)/EndEntire
>> >>   
>> >> /ACPI(133741d0,0)/UnknownMessaging(1d)/HD(1,64800,20,dd904a8c,1,1)/EndEntire
>> >>   
>> >> /ACPI(133741d0,0)/UnknownMessaging(1d)/HD(2,264800,19a000,dd904a8c,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,6,38ca6802,1,1)/EndEntire
>> >>   
>> >> /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(1,61000,155000,38ca6802,1,1)/EndEntire
>> >>   
>> >> /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(2,20fa800,1bbf8800,38ca6802,1,1)/EndEntire
>> >>   
>> >> /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(3,1b6800,1f44000,38ca6802,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: 91200Hz, 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@01c5
> 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..

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

[Mark Kettenis]

> 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 
> Date: Fri, 4 Aug 2017 16:57:50 -0400
> Cc: U-Boot Mailing List ,
> Heinrich Schuchardt ,
> Peter Jones 
> 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=pGLkceIS:8 a=NEAV23lm: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.kette...@xs4all.nl
> 
> On Fri, Aug 4, 2017 at 4:41 PM, Mark Kettenis  wrote:
> >> From: Rob Clark 
> >> 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(sd...@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,dd904a8c,1,1)/EndEntire
> >>   
> >> /ACPI(133741d0,0)/UnknownMessaging(1d)/HD(1,64800,20,dd904a8c,1,1)/EndEntire
> >>   
> >> /ACPI(133741d0,0)/UnknownMessaging(1d)/HD(2,264800,19a000,dd904a8c,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,6,38ca6802,1,1)/EndEntire
> >>   
> >> /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(1,61000,155000,38ca6802,1,1)/EndEntire
> >>   
> >> /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(2,20fa800,1bbf8800,38ca6802,1,1)/EndEntire
> >>   
> >> /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(3,1b6800,1f44000,38ca6802,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 sens

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

[Rob Clark]

On Fri, Aug 4, 2017 at 4:41 PM, Mark Kettenis  wrote:
>> From: Rob Clark 
>> 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(sd...@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,dd904a8c,1,1)/EndEntire
>>   
>> /ACPI(133741d0,0)/UnknownMessaging(1d)/HD(1,64800,20,dd904a8c,1,1)/EndEntire
>>   
>> /ACPI(133741d0,0)/UnknownMessaging(1d)/HD(2,264800,19a000,dd904a8c,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,6,38ca6802,1,1)/EndEntire
>>   
>> /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(1,61000,155000,38ca6802,1,1)/EndEntire
>>   
>> /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(2,20fa800,1bbf8800,38ca6802,1,1)/EndEntire
>>   
>> /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(3,1b6800,1f44000,38ca6802,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
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

[Mark Kettenis]

> From: Rob Clark 
> 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(sd...@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,dd904a8c,1,1)/EndEntire
>   
> /ACPI(133741d0,0)/UnknownMessaging(1d)/HD(1,64800,20,dd904a8c,1,1)/EndEntire
>   
> /ACPI(133741d0,0)/UnknownMessaging(1d)/HD(2,264800,19a000,dd904a8c,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,6,38ca6802,1,1)/EndEntire
>   
> /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(1,61000,155000,38ca6802,1,1)/EndEntire
>   
> /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(2,20fa800,1bbf8800,38ca6802,1,1)/EndEntire
>   
> /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(3,1b6800,1f44000,38ca6802,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
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot