[PATCH 0/5] qemu-arm64: Allow booting via Trusted Firmware

[PATCH 0/5] qemu-arm64: Allow booting via Trusted Firmware

[Andre Przywara]

U-Boot on QEMU-arm64 can be used in two configurations: Loaded directly
via QEMU's -bios option, or as a non-secure payload (BL33) via
ARM Trusted Firmware-A (TF-A).
In the latter case we need to define CONFIG_TFABOOT, to accommodate
the first flash bank being secure only, and manually set SYS_TEXT_BASE
to the address configured in TF-A (currently 0x60000000).

To avoid this poorly documented adventure, we enable a position
independent build, and also let the flash regions be always detected
through the DTB. This results in a single build to work under both
scenarios, and also allows to move the BL33 load address in TF-A to
something lower in the future.

For this to work, we have to first make PIE work when booted from ROM.
While writing to ROM should not hurt, it might trigger CFI flash
sequences, and indeed crashes for me in the middle of the fixup routine.
This is covered by patch 1/5, which skips the whole fixup routine if the
offset is actually 0 (as it is in our case).
Also we have to decouple the relative initial stack pointer from the
PIE option, as we always need to use the fixed version, pointing to
RAM (patch 2/5).
Patch 3/5 drops the hard-coded flash address, instead U-Boot can already
read all required information from QEMU's DTB.
Patch 4/5 is a cleanup, while the last patch enables the PIE build.

With this series the very same u-boot.bin file works when directly loaded
from the QEMU command line (-bios), but also when embedded into TF-A's
fip.bin, removing the need for case-specific build options.

Please have a look!

Cheers,
Andre

Andre Przywara (5):
  arm64: PIE: Skip fixups if distance is zero
  arm64: PIE: Allow fixed stack pointer
  qemu-arm: Remove need to specify flash banks
  qemu: Drop ARCH_SUPPORT_TFABOOT
  qemu/arm64: Enable POSITION_INDEPENDENT

 arch/arm/Kconfig             | 4 ++--
 arch/arm/cpu/armv8/start.S   | 3 ++-
 configs/qemu_arm64_defconfig | 1 +
 include/configs/qemu-arm.h   | 8 +-------
 4 files changed, 6 insertions(+), 10 deletions(-)

-- 
2.17.5

[PATCH 1/5] arm64: PIE: Skip fixups if distance is zero

[Andre Przywara]

When the actual offset between link and runtime address is zero, there
is no need for patching up U-Boot early when running with
CONFIG_POSITION_INDEPENDENT.

Skip the whole routine when the distance is 0.

This helps when U-Boot is loaded into ROM, or in otherwise sensitive
memory locations.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm/cpu/armv8/start.S | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/cpu/armv8/start.S b/arch/arm/cpu/armv8/start.S
index 002698b501c..02b952bb328 100644
--- a/arch/arm/cpu/armv8/start.S
+++ b/arch/arm/cpu/armv8/start.S
@@ -66,7 +66,8 @@ save_boot_params_ret:
 pie_fixup:
 	adr	x0, _start		/* x0 <- Runtime value of _start */
 	ldr	x1, _TEXT_BASE		/* x1 <- Linked value of _start */
-	sub	x9, x0, x1		/* x9 <- Run-vs-link offset */
+	subs	x9, x0, x1		/* x9 <- Run-vs-link offset */
+	beq	pie_fixup_done
 	adr	x2, __rel_dyn_start	/* x2 <- Runtime &__rel_dyn_start */
 	adr	x3, __rel_dyn_end	/* x3 <- Runtime &__rel_dyn_end */
 pie_fix_loop:
-- 
2.17.5

[PATCH 2/5] arm64: PIE: Allow fixed stack pointer

[Andre Przywara]

Currently selecting CONFIG_POSITION_INDEPENDENT also forces us to use an
initial stack pointer address relative to the load address. This makes
some sense, assuming there is some usable memory nearby.

However if U-Boot is started from some kind of ROM, a stack pointer
nearby will probably also end up in ROM, which is not very helpful.

Allow CONFIG_INIT_SP_RELATIVE to be turned off by a board's config, to
be able to select a fixed stack pointer, for instance in known good
DRAM.

This will help QEMU utilising PIE, when it's loaded to (Flash-)ROM.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
Verified all boards using CONFIG_POSITION_INDEPENDENT to generate the
same .config, before and after this patch.

Cheers,
Andre

 arch/arm/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 80702c23d34..d079e1930fc 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -12,7 +12,6 @@ config ARM64
 if ARM64
 config POSITION_INDEPENDENT
 	bool "Generate position-independent pre-relocation code"
-	select INIT_SP_RELATIVE
 	help
 	  U-Boot expects to be linked to a specific hard-coded address, and to
 	  be loaded to and run from that address. This option lifts that
@@ -23,6 +22,7 @@ config POSITION_INDEPENDENT
 
 config INIT_SP_RELATIVE
 	bool "Specify the early stack pointer relative to the .bss section"
+	default y if POSITION_INDEPENDENT
 	help
 	  U-Boot typically uses a hard-coded value for the stack pointer
 	  before relocation. Enable this option to instead calculate the
-- 
2.17.5

[PATCH 3/5] qemu-arm: Remove need to specify flash banks

[Andre Przywara]

Currently we hard-code the number and initial addresses of QEMU's flash
banks, even though our code is perfectly able to gather the same
information from the DTB provided by QEMU.
This is especially annoying, since we have two slightly different
U-Boot configurations ("bare-metal" vs. loaded via Arm Trusted
Firmware), which need to be selected at build time.

Drop the two hard coded alternatives, and use
CONFIG_SYS_MAX_FLASH_BANKS_DETECT instead, which relies on the DTB to
figure out the actual flash configuration at runtime.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 include/configs/qemu-arm.h | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/include/configs/qemu-arm.h b/include/configs/qemu-arm.h
index bc8b7c5c123..273fa1a7d7b 100644
--- a/include/configs/qemu-arm.h
+++ b/include/configs/qemu-arm.h
@@ -45,13 +45,7 @@
 #define CONFIG_SYS_CBSIZE 512
 
 #define CONFIG_SYS_MONITOR_BASE		CONFIG_SYS_TEXT_BASE
-#ifdef CONFIG_TFABOOT
-#define CONFIG_SYS_FLASH_BASE		0x4000000
-#define CONFIG_SYS_MAX_FLASH_BANKS	1
-#else
-#define CONFIG_SYS_FLASH_BASE		0x0
-#define CONFIG_SYS_MAX_FLASH_BANKS	2
-#endif
+#define CONFIG_SYS_MAX_FLASH_BANKS_DETECT	2
 #define CONFIG_SYS_MAX_FLASH_SECT	256 /* Sector: 256K, Bank: 64M */
 #define CONFIG_CFI_FLASH_USE_WEAK_ACCESSORS
 
-- 
2.17.5

[PATCH 4/5] qemu: Drop ARCH_SUPPORT_TFABOOT

[Andre Przywara]

CONFIG_ARCH_SUPPORT_TFABOOT seems to be a guard option to enable various
platform specific hacks, when U-Boot is run under TF-A.
Now that the QEMU port does not need to differentiate between secure
vs. non-secure anymore (this is taken care of by the DTB), there is
no need for a build-time option anymore, so remove it.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index d079e1930fc..dde8c4c3644 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -929,7 +929,6 @@ config ARCH_OWL
 
 config ARCH_QEMU
 	bool "QEMU Virtual Platform"
-	select ARCH_SUPPORT_TFABOOT
 	select DM
 	select DM_SERIAL
 	select OF_CONTROL
-- 
2.17.5

[PATCH 5/5] qemu-arm64: Enable POSITION_INDEPENDENT

[Andre Przywara]

Now that PIE works when U-Boot is started from ROM, let's enable
CONFIG_POSITION_INDEPENDENT, which allows to load U-Boot also via
ARM Trusted-Firmware's fip.bin to DRAM, without tweaking the
configuration.

To get a writable initial stack, we need to keep the fixed initial
stack pointer, which points to DRAM in our case.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm/Kconfig             | 1 +
 configs/qemu_arm64_defconfig | 1 +
 2 files changed, 2 insertions(+)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index dde8c4c3644..6e754936675 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -22,6 +22,7 @@ config POSITION_INDEPENDENT
 
 config INIT_SP_RELATIVE
 	bool "Specify the early stack pointer relative to the .bss section"
+	default n if ARCH_QEMU
 	default y if POSITION_INDEPENDENT
 	help
 	  U-Boot typically uses a hard-coded value for the stack pointer
diff --git a/configs/qemu_arm64_defconfig b/configs/qemu_arm64_defconfig
index 31ea2d342fc..4450e7ced42 100644
--- a/configs/qemu_arm64_defconfig
+++ b/configs/qemu_arm64_defconfig
@@ -1,6 +1,7 @@
 CONFIG_ARM=y
 CONFIG_ARCH_QEMU=y
 CONFIG_NR_DRAM_BANKS=1
+CONFIG_POSITION_INDEPENDENT=y
 CONFIG_ENV_SIZE=0x40000
 CONFIG_ENV_SECT_SIZE=0x40000
 CONFIG_AHCI=y
-- 
2.17.5

[PATCH 0/5] qemu-arm64: Allow booting via Trusted Firmware

[Amit Tomar]

Hi,

Andre Przywara (5):

>   arm64: PIE: Skip fixups if distance is zero
>   arm64: PIE: Allow fixed stack pointer
>   qemu-arm: Remove need to specify flash banks
>   qemu: Drop ARCH_SUPPORT_TFABOOT
>   qemu/arm64: Enable POSITION_INDEPENDENT
>
>  arch/arm/Kconfig             | 4 ++--
>  arch/arm/cpu/armv8/start.S   | 3 ++-
>  configs/qemu_arm64_defconfig | 1 +
>  include/configs/qemu-arm.h   | 8 +-------
>  4 files changed, 6 insertions(+), 10 deletions(-)
>
> --
> 2.17.5
>
>
I tried testing this series but don't see any output while loading U-Boot
from ROM:

# ./qemu-system-aarch64 -M virt -cpu cortex-a57 -nographic -smp 1 -bios
u-boot.bin

strangely enough SP is having a value of 0 after execution:

(qemu) info registers
 PC=0000000000001a00 X00=540000a0f100303f X01=000000000007c000
X02=0000000000000000 X03=00000000401fe000 X04=0000000000000000
X05=0000000000000000 X06=0000000000000030 X07=00000000401fe008
X08=0000000000000000 X09=14000008d51e115f X10=0000000000000000
X11=0000000000000000 X12=0000000000000000 X13=0000000000000000
X14=0000000000000000 X15=0000000000000000 X16=0000000000000000
X17=0000000000000000 X18=0000000000000000 X19=0000000000000000
X20=0000000000000000 X21=0000000000000000 X22=0000000000000000
X23=0000000000000000 X24=0000000000000000 X25=0000000000000000
X26=0000000000000000 X27=0000000000000000 X28=0000000000000000
X29=00000000000000c8 X30=14000008d51e39cb  SP=0000000000000000
PSTATE=000003c5 ---- EL1h     FPCR=00000000 FPSR=00000000

Wondering , if I have missed something ?

Thanks
-Amit

[PATCH 0/5] qemu-arm64: Allow booting via Trusted Firmware

[André Przywara]

On 24/09/2020 08:57, Amit Tomar wrote:
> Hi,
> 
> Andre Przywara (5):
> 
>     ? arm64: PIE: Skip fixups if distance is zero
>     ? arm64: PIE: Allow fixed stack pointer
>     ? qemu-arm: Remove need to specify flash banks
>     ? qemu: Drop ARCH_SUPPORT_TFABOOT
>     ? qemu/arm64: Enable POSITION_INDEPENDENT
> 
>     ?arch/arm/Kconfig? ? ? ? ? ? ?| 4 ++--
>     ?arch/arm/cpu/armv8/start.S? ?| 3 ++-
>     ?configs/qemu_arm64_defconfig | 1 +
>     ?include/configs/qemu-arm.h? ?| 8 +-------
>     ?4 files changed, 6 insertions(+), 10 deletions(-)
> 
>     -- 
>     2.17.5
> 
> 
> I tried testing this series but don't see any output while loading
> U-Boot from ROM:
> 
> # ./qemu-system-aarch64 -M virt -cpu cortex-a57 -nographic -smp 1 -bios
> u-boot.bin

I can't reproduce this (read: works for me).
I tried with cross-gcc 9.2 & QEMU 5.0.0, and with Ubuntu-arm64 (GCC 7.5
& QEMU 2.11.1).
Did you apply against master, used qemu_arm64_defconfig, cleaned the
build directory? Is your (self-built?) QEMU working? Are you using the
actual generated binary?

I pushed the branch to
https://github.com/Andre-ARM/u-boot/commits/qemu-tfa, please try this.

Cheers,
Andre

> 
> strangely enough SP is having a value of 0 after execution:
> 
> (qemu) info registers
> ?PC=0000000000001a00 X00=540000a0f100303f X01=000000000007c000
> X02=0000000000000000 X03=00000000401fe000 X04=0000000000000000
> X05=0000000000000000 X06=0000000000000030 X07=00000000401fe008
> X08=0000000000000000 X09=14000008d51e115f X10=0000000000000000
> X11=0000000000000000 X12=0000000000000000 X13=0000000000000000
> X14=0000000000000000 X15=0000000000000000 X16=0000000000000000
> X17=0000000000000000 X18=0000000000000000 X19=0000000000000000
> X20=0000000000000000 X21=0000000000000000 X22=0000000000000000
> X23=0000000000000000 X24=0000000000000000 X25=0000000000000000
> X26=0000000000000000 X27=0000000000000000 X28=0000000000000000
> X29=00000000000000c8 X30=14000008d51e39cb ?SP=0000000000000000
> PSTATE=000003c5 ---- EL1h ? ? FPCR=00000000 FPSR=00000000
> 
> Wondering , if I have missed something ?
> 
> Thanks
> -Amit

[PATCH 0/5] qemu-arm64: Allow booting via Trusted Firmware

[Heinrich Schuchardt]

On 24.09.20 09:57, Amit Tomar wrote:
> Hi,
>
> Andre Przywara (5):
>
>     ? arm64: PIE: Skip fixups if distance is zero
>     ? arm64: PIE: Allow fixed stack pointer
>     ? qemu-arm: Remove need to specify flash banks
>     ? qemu: Drop ARCH_SUPPORT_TFABOOT
>     ? qemu/arm64: Enable POSITION_INDEPENDENT
>
>     ?arch/arm/Kconfig? ? ? ? ? ? ?| 4 ++--
>     ?arch/arm/cpu/armv8/start.S? ?| 3 ++-
>     ?configs/qemu_arm64_defconfig | 1 +
>     ?include/configs/qemu-arm.h? ?| 8 +-------
>     ?4 files changed, 6 insertions(+), 10 deletions(-)
>
>     --
>     2.17.5
>
>
> I tried testing this series but don't see any output while loading
> U-Boot from ROM:
>
> # ./qemu-system-aarch64 -M virt -cpu cortex-a57 -nographic -smp 1 -bios
> u-boot.bin

TF-A runs at EL3 so you should test with
-machine virt,secure=true,virtualization=true

The bios parameter has to point to the TF-A bl1.bin binary. see TF-A's
docs/plat/qemu.rst: BL1 is used as the BootROM, supplied with the -bios
argument.

U-Boot is BL33.

Best regards

Heinrich

>
> strangely enough SP is having a value of 0 after execution:
>
> (qemu) info registers
> ?PC=0000000000001a00 X00=540000a0f100303f X01=000000000007c000
> X02=0000000000000000 X03=00000000401fe000 X04=0000000000000000
> X05=0000000000000000 X06=0000000000000030 X07=00000000401fe008
> X08=0000000000000000 X09=14000008d51e115f X10=0000000000000000
> X11=0000000000000000 X12=0000000000000000 X13=0000000000000000
> X14=0000000000000000 X15=0000000000000000 X16=0000000000000000
> X17=0000000000000000 X18=0000000000000000 X19=0000000000000000
> X20=0000000000000000 X21=0000000000000000 X22=0000000000000000
> X23=0000000000000000 X24=0000000000000000 X25=0000000000000000
> X26=0000000000000000 X27=0000000000000000 X28=0000000000000000
> X29=00000000000000c8 X30=14000008d51e39cb ?SP=0000000000000000
> PSTATE=000003c5 ---- EL1h ? ? FPCR=00000000 FPSR=00000000
>
> Wondering , if I have missed something ?
>
> Thanks
> -Amit

[PATCH 0/5] qemu-arm64: Allow booting via Trusted Firmware

[Ard Biesheuvel]

On Thu, 24 Sep 2020 at 09:58, Amit Tomar <atomar25opensource@gmail.com> wrote:
>
> Hi,
>
> Andre Przywara (5):
>>
>>   arm64: PIE: Skip fixups if distance is zero
>>   arm64: PIE: Allow fixed stack pointer
>>   qemu-arm: Remove need to specify flash banks
>>   qemu: Drop ARCH_SUPPORT_TFABOOT
>>   qemu/arm64: Enable POSITION_INDEPENDENT
>>
>>  arch/arm/Kconfig             | 4 ++--
>>  arch/arm/cpu/armv8/start.S   | 3 ++-
>>  configs/qemu_arm64_defconfig | 1 +
>>  include/configs/qemu-arm.h   | 8 +-------
>>  4 files changed, 6 insertions(+), 10 deletions(-)
>>
>> --
>> 2.17.5
>>
>
> I tried testing this series but don't see any output while loading U-Boot from ROM:
>
> # ./qemu-system-aarch64 -M virt -cpu cortex-a57 -nographic -smp 1 -bios u-boot.bin
>
> strangely enough SP is having a value of 0 after execution:
>
> (qemu) info registers
>  PC=0000000000001a00 X00=540000a0f100303f X01=000000000007c000
> X02=0000000000000000 X03=00000000401fe000 X04=0000000000000000
> X05=0000000000000000 X06=0000000000000030 X07=00000000401fe008
> X08=0000000000000000 X09=14000008d51e115f X10=0000000000000000
> X11=0000000000000000 X12=0000000000000000 X13=0000000000000000
> X14=0000000000000000 X15=0000000000000000 X16=0000000000000000
> X17=0000000000000000 X18=0000000000000000 X19=0000000000000000
> X20=0000000000000000 X21=0000000000000000 X22=0000000000000000
> X23=0000000000000000 X24=0000000000000000 X25=0000000000000000
> X26=0000000000000000 X27=0000000000000000 X28=0000000000000000
> X29=00000000000000c8 X30=14000008d51e39cb  SP=0000000000000000
> PSTATE=000003c5 ---- EL1h     FPCR=00000000 FPSR=00000000
>
> Wondering , if I have missed something ?
>

Did you regenerate the .config? Otherwise, CONFIG_INIT_SP_RELATIVE may
still be enabled.

[PATCH 0/5] qemu-arm64: Allow booting via Trusted Firmware

[Amit Tomar]

>
> Did you regenerate the .config? Otherwise, CONFIG_INIT_SP_RELATIVE may
> still be enabled.
>

I cloned a fresh U-boot source (with top commit 55004fa43364e) , and the
top of it applied these patches.
After generating the .config using "qemu_arm64_defconfig" where
CONFIG_INIT_SP_RELATIVE is disabled.

#
# ARM architecture
#
CONFIG_ARM64=y
CONFIG_POSITION_INDEPENDENT=y
# CONFIG_INIT_SP_RELATIVE is not set
# CONFIG_GIC_V3_ITS is not set

Using ./qemu-system-aarch64 --version
QEMU emulator version 5.0.50 (v5.0.0-2210-g45db94cc90c2-dirty)

U-boot is crossed compile using
"gcc-linaro-7.3.1-2018.05-x86_64_aarch64-linux-gnu"

Thanks
-Amit

[PATCH 0/5] qemu-arm64: Allow booting via Trusted Firmware

[Amit Tomar]

>
> >
> > I tried testing this series but don't see any output while loading
> > U-Boot from ROM:
> >
> > # ./qemu-system-aarch64 -M virt -cpu cortex-a57 -nographic -smp 1 -bios
> > u-boot.bin
>
> TF-A runs at EL3 so you should test with
> -machine virt,secure=true,virtualization=true
>
> The bios parameter has to point to the TF-A bl1.bin binary. see TF-A's
> docs/plat/qemu.rst: BL1 is used as the BootROM, supplied with the -bios
> argument.
>
> But, wanted to test it without loading U-boot from TF-A , for instance if
> I just disable the
>
   CONFIG_POSITION_INDEPENDENT, it works:

./qemu-system-aarch64 -M virt -cpu cortex-a57 -nographic -smp 1 -bios
u-boot.bin

U-Boot 2020.10-rc5-00020-gca11959a34f4 (Sep 24 2020 - 14:34:25 +0530)

DRAM:  128 MiB
Flash: 128 MiB
*** Warning - bad CRC, using default environment

PCI: Failed autoconfig bar 14
In:    pl011 at 9000000
Out:   pl011 at 9000000
Err:   pl011 at 9000000
Net:   No ethernet found.

Thanks
-Amit

[PATCH 0/5] qemu-arm64: Allow booting via Trusted Firmware

[André Przywara]

On 24/09/2020 09:44, Heinrich Schuchardt wrote:
> On 24.09.20 09:57, Amit Tomar wrote:
>> Hi,
>>
>> Andre Przywara (5):
>>
>>     ? arm64: PIE: Skip fixups if distance is zero
>>     ? arm64: PIE: Allow fixed stack pointer
>>     ? qemu-arm: Remove need to specify flash banks
>>     ? qemu: Drop ARCH_SUPPORT_TFABOOT
>>     ? qemu/arm64: Enable POSITION_INDEPENDENT
>>
>>     ?arch/arm/Kconfig? ? ? ? ? ? ?| 4 ++--
>>     ?arch/arm/cpu/armv8/start.S? ?| 3 ++-
>>     ?configs/qemu_arm64_defconfig | 1 +
>>     ?include/configs/qemu-arm.h? ?| 8 +-------
>>     ?4 files changed, 6 insertions(+), 10 deletions(-)
>>
>>     --
>>     2.17.5
>>
>>
>> I tried testing this series but don't see any output while loading
>> U-Boot from ROM:
>>
>> # ./qemu-system-aarch64 -M virt -cpu cortex-a57 -nographic -smp 1 -bios
>> u-boot.bin
> 
> TF-A runs at EL3 so you should test with
> -machine virt,secure=true,virtualization=true

This is true, but the point of this series is to work both within TF-A
and without it. So it should work (and does for me!) with Amit's line above.

> The bios parameter has to point to the TF-A bl1.bin binary. see TF-A's
> docs/plat/qemu.rst: BL1 is used as the BootROM, supplied with the -bios
> argument.

Yeah, and it seems to be even more complicated, since you have to glue
fip.bin within a certain offset to bl1.bin, into one file to give to
-bios. I use:
$ cp build/qemu/debug/bl1.bin flash.bin
$ dd if=build/qemu/debug/fip.bin of=flash.bin bs=4k seek=64

... and make sure to give QEMU more than 512MB of RAM, since the default
load address for U-Boot is there. Actually fixing this annoyance was the
main motivation for this series.

Once people agreed that this series is making some sense, I plan to move
the load address in TF-A and improve the documentation in both TF-A and
QEMU on this.

Thanks!
Andre

> 
> U-Boot is BL33.
> 
> Best regards
> 
> Heinrich
> 
>>
>> strangely enough SP is having a value of 0 after execution:
>>
>> (qemu) info registers
>> ?PC=0000000000001a00 X00=540000a0f100303f X01=000000000007c000
>> X02=0000000000000000 X03=00000000401fe000 X04=0000000000000000
>> X05=0000000000000000 X06=0000000000000030 X07=00000000401fe008
>> X08=0000000000000000 X09=14000008d51e115f X10=0000000000000000
>> X11=0000000000000000 X12=0000000000000000 X13=0000000000000000
>> X14=0000000000000000 X15=0000000000000000 X16=0000000000000000
>> X17=0000000000000000 X18=0000000000000000 X19=0000000000000000
>> X20=0000000000000000 X21=0000000000000000 X22=0000000000000000
>> X23=0000000000000000 X24=0000000000000000 X25=0000000000000000
>> X26=0000000000000000 X27=0000000000000000 X28=0000000000000000
>> X29=00000000000000c8 X30=14000008d51e39cb ?SP=0000000000000000
>> PSTATE=000003c5 ---- EL1h ? ? FPCR=00000000 FPSR=00000000
>>
>> Wondering , if I have missed something ?
>>
>> Thanks
>> -Amit
> 

[PATCH 1/5] arm64: PIE: Skip fixups if distance is zero

[André Przywara]

On 24/09/2020 01:17, Andre Przywara wrote:
> When the actual offset between link and runtime address is zero, there
> is no need for patching up U-Boot early when running with
> CONFIG_POSITION_INDEPENDENT.

That turns out to be not fully true.
Some toolchains (all Linaro cross compilers?) don't handle this well,
they keep the original locations in the binary uninitialised, and rely
on the reldyn fixup table to patch in the actual values.
Other compilers (GCC 9.2 vanilla, Ubuntu GCC 7.5.0, Arm website 9.2)
fill in the addresses both into the binary and the fixup, so this patch
works.

It seems to be fixed by enabling CONFIG_STATIC_RELA?
I see it's disabled for CONFIG_POSITION_INDEPENDENT, what was the reason
behind that?

Cheers,
Andre.

> 
> Skip the whole routine when the distance is 0.
> 
> This helps when U-Boot is loaded into ROM, or in otherwise sensitive
> memory locations.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  arch/arm/cpu/armv8/start.S | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/cpu/armv8/start.S b/arch/arm/cpu/armv8/start.S
> index 002698b501c..02b952bb328 100644
> --- a/arch/arm/cpu/armv8/start.S
> +++ b/arch/arm/cpu/armv8/start.S
> @@ -66,7 +66,8 @@ save_boot_params_ret:
>  pie_fixup:
>  	adr	x0, _start		/* x0 <- Runtime value of _start */
>  	ldr	x1, _TEXT_BASE		/* x1 <- Linked value of _start */
> -	sub	x9, x0, x1		/* x9 <- Run-vs-link offset */
> +	subs	x9, x0, x1		/* x9 <- Run-vs-link offset */
> +	beq	pie_fixup_done
>  	adr	x2, __rel_dyn_start	/* x2 <- Runtime &__rel_dyn_start */
>  	adr	x3, __rel_dyn_end	/* x3 <- Runtime &__rel_dyn_end */
>  pie_fix_loop:
> 

[PATCH 1/5] arm64: PIE: Skip fixups if distance is zero

[Ard Biesheuvel]

On Thu, 24 Sep 2020 at 16:45, Andr? Przywara <andre.przywara@arm.com> wrote:
>
> On 24/09/2020 01:17, Andre Przywara wrote:
> > When the actual offset between link and runtime address is zero, there
> > is no need for patching up U-Boot early when running with
> > CONFIG_POSITION_INDEPENDENT.
>
> That turns out to be not fully true.
> Some toolchains (all Linaro cross compilers?) don't handle this well,
> they keep the original locations in the binary uninitialised, and rely
> on the reldyn fixup table to patch in the actual values.
> Other compilers (GCC 9.2 vanilla, Ubuntu GCC 7.5.0, Arm website 9.2)
> fill in the addresses both into the binary and the fixup, so this patch
> works.
>

This was changed in binutils-2.27, to align with other architectures.
A new command line option for ld '--no-apply-dynamic-relocs' was also
added at the same time, to revert to the old behavior.

[PATCH 1/5] arm64: PIE: Skip fixups if distance is zero

[Stephen Warren]

On 9/24/20 8:45 AM, Andr? Przywara wrote:
> On 24/09/2020 01:17, Andre Przywara wrote:
>> When the actual offset between link and runtime address is zero, there
>> is no need for patching up U-Boot early when running with
>> CONFIG_POSITION_INDEPENDENT.
> 
> That turns out to be not fully true.
> Some toolchains (all Linaro cross compilers?) don't handle this well,
> they keep the original locations in the binary uninitialised, and rely
> on the reldyn fixup table to patch in the actual values.
> Other compilers (GCC 9.2 vanilla, Ubuntu GCC 7.5.0, Arm website 9.2)
> fill in the addresses both into the binary and the fixup, so this patch
> works.
> 
> It seems to be fixed by enabling CONFIG_STATIC_RELA?
> I see it's disabled for CONFIG_POSITION_INDEPENDENT, what was the reason
> behind that?

I can't remember for sure. I tried re-enabling STATIC_RELA at both the
upstream commit where POSITION_INDEPENDENT was first implemented and at
top-of-tree, and ran test/py on Tegra, and it works fine. Perhaps I just
figured there was no need to perform the static relocations since the
code would always do it? I figure it's safe to re-enable it, i.e.
decouple the two config options completely.

[PATCH 5/5] qemu-arm64: Enable POSITION_INDEPENDENT

[Stephen Warren]

On 9/23/20 6:17 PM, Andre Przywara wrote:
> Now that PIE works when U-Boot is started from ROM, let's enable
> CONFIG_POSITION_INDEPENDENT, which allows to load U-Boot also via
> ARM Trusted-Firmware's fip.bin to DRAM, without tweaking the
> configuration.
> 
> To get a writable initial stack, we need to keep the fixed initial
> stack pointer, which points to DRAM in our case.

Patches 1,2,5:
Reviewed-by: Stephen Warren <swarren@nvidia.com>

I guess we'll need to add/modify a patch to re-enable STATIC_RELA.

[PATCH 1/5] arm64: PIE: Skip fixups if distance is zero

[André Przywara]

On 24/09/2020 21:22, Stephen Warren wrote:

Hi Stephen,

> On 9/24/20 8:45 AM, Andr? Przywara wrote:
>> On 24/09/2020 01:17, Andre Przywara wrote:
>>> When the actual offset between link and runtime address is zero, there
>>> is no need for patching up U-Boot early when running with
>>> CONFIG_POSITION_INDEPENDENT.
>>
>> That turns out to be not fully true.
>> Some toolchains (all Linaro cross compilers?) don't handle this well,
>> they keep the original locations in the binary uninitialised, and rely
>> on the reldyn fixup table to patch in the actual values.
>> Other compilers (GCC 9.2 vanilla, Ubuntu GCC 7.5.0, Arm website 9.2)
>> fill in the addresses both into the binary and the fixup, so this patch
>> works.
>>
>> It seems to be fixed by enabling CONFIG_STATIC_RELA?
>> I see it's disabled for CONFIG_POSITION_INDEPENDENT, what was the reason
>> behind that?
> 
> I can't remember for sure. I tried re-enabling STATIC_RELA at both the
> upstream commit where POSITION_INDEPENDENT was first implemented and at
> top-of-tree, and ran test/py on Tegra, and it works fine. Perhaps I just
> figured there was no need to perform the static relocations since the
> code would always do it? I figure it's safe to re-enable it, i.e.
> decouple the two config options completely.

Many thanks for the reply and the test!
And yeah, I was hoping that it was just for "because we don't need to".

I will send a patch to always use STATIC_RELA for arm64. As Ard pointed
out, it should actually not be needed anymore (for newer binutils), but
the Linaro toolchains miss this change, and quite some people use them.

Cheers,
Andre

[PATCH 0/5] qemu-arm64: Allow booting via Trusted Firmware

[Tom Rini]

On Thu, Sep 24, 2020 at 01:17:10AM +0100, Andre Przywara wrote:

> U-Boot on QEMU-arm64 can be used in two configurations: Loaded directly
> via QEMU's -bios option, or as a non-secure payload (BL33) via
> ARM Trusted Firmware-A (TF-A).
> In the latter case we need to define CONFIG_TFABOOT, to accommodate
> the first flash bank being secure only, and manually set SYS_TEXT_BASE
> to the address configured in TF-A (currently 0x60000000).
> 
> To avoid this poorly documented adventure, we enable a position
> independent build, and also let the flash regions be always detected
> through the DTB. This results in a single build to work under both
> scenarios, and also allows to move the BL33 load address in TF-A to
> something lower in the future.
> 
> For this to work, we have to first make PIE work when booted from ROM.
> While writing to ROM should not hurt, it might trigger CFI flash
> sequences, and indeed crashes for me in the middle of the fixup routine.
> This is covered by patch 1/5, which skips the whole fixup routine if the
> offset is actually 0 (as it is in our case).
> Also we have to decouple the relative initial stack pointer from the
> PIE option, as we always need to use the fixed version, pointing to
> RAM (patch 2/5).
> Patch 3/5 drops the hard-coded flash address, instead U-Boot can already
> read all required information from QEMU's DTB.
> Patch 4/5 is a cleanup, while the last patch enables the PIE build.
> 
> With this series the very same u-boot.bin file works when directly loaded
> from the QEMU command line (-bios), but also when embedded into TF-A's
> fip.bin, removing the need for case-specific build options.
> 
> Please have a look!

This sounds very useful.  Would it be possible to extend CI to create
the files for the TF-A case and also run that through test.py?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200929/25066e7f/attachment.sig>

[PATCH 4/5] qemu: Drop ARCH_SUPPORT_TFABOOT

[Tom Rini]

On Thu, Sep 24, 2020 at 01:17:14AM +0100, Andre Przywara wrote:

> CONFIG_ARCH_SUPPORT_TFABOOT seems to be a guard option to enable various
> platform specific hacks, when U-Boot is run under TF-A.
> Now that the QEMU port does not need to differentiate between secure
> vs. non-secure anymore (this is taken care of by the DTB), there is
> no need for a build-time option anymore, so remove it.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

I don't quite like calling the changes under TFABOOT hacks in that
ARCH_SUPPORT_TFABOOT is used to guard TFABOOT and on other platforms
that's used to enable/disable various non-hacky build time things.
Maybe we need to tweak the help text on TFABOOT to be clear that only
may be required on a given platform?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200929/ae514266/attachment.sig>

[PATCH 4/5] qemu: Drop ARCH_SUPPORT_TFABOOT

[André Przywara]

On 29/09/2020 14:23, Tom Rini wrote:

Hi Tom,

> On Thu, Sep 24, 2020 at 01:17:14AM +0100, Andre Przywara wrote:
> 
>> CONFIG_ARCH_SUPPORT_TFABOOT seems to be a guard option to enable various
>> platform specific hacks, when U-Boot is run under TF-A.
>> Now that the QEMU port does not need to differentiate between secure
>> vs. non-secure anymore (this is taken care of by the DTB), there is
>> no need for a build-time option anymore, so remove it.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> 
> I don't quite like calling the changes under TFABOOT hacks in that

Yeah, hack sounds too harsh, apologies for that. I will reword the
commit message. On a first glance the code guarded by that symbol seemed
to be only very loosely connected to TF-A.

> ARCH_SUPPORT_TFABOOT is used to guard TFABOOT and on other platforms
> that's used to enable/disable various non-hacky build time things.
> Maybe we need to tweak the help text on TFABOOT to be clear that only
> may be required on a given platform?

Well, looking more closely now it looks like on STM32 and FSL this makes
the difference between: Does U-Boot handle the secure side (errata
handling, provide PSCI services) or is this done by other firmware (TF-A).
This seems like a legitimate option(*), but this may indeed be better
explained. I can make a patch for that if this seems useful.

Cheers.
Andre


(*) ... although one could argue that this could decided at runtime, by
looking at the EL U-Boot is entered in. But this is quite a separate
discussion and most probably not worth the effort.

[PATCH 4/5] qemu: Drop ARCH_SUPPORT_TFABOOT

[Tom Rini]

On Tue, Sep 29, 2020 at 06:13:09PM +0100, Andr? Przywara wrote:
> On 29/09/2020 14:23, Tom Rini wrote:
> 
> Hi Tom,
> 
> > On Thu, Sep 24, 2020 at 01:17:14AM +0100, Andre Przywara wrote:
> > 
> >> CONFIG_ARCH_SUPPORT_TFABOOT seems to be a guard option to enable various
> >> platform specific hacks, when U-Boot is run under TF-A.
> >> Now that the QEMU port does not need to differentiate between secure
> >> vs. non-secure anymore (this is taken care of by the DTB), there is
> >> no need for a build-time option anymore, so remove it.
> >>
> >> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > 
> > I don't quite like calling the changes under TFABOOT hacks in that
> 
> Yeah, hack sounds too harsh, apologies for that. I will reword the
> commit message. On a first glance the code guarded by that symbol seemed
> to be only very loosely connected to TF-A.
> 
> > ARCH_SUPPORT_TFABOOT is used to guard TFABOOT and on other platforms
> > that's used to enable/disable various non-hacky build time things.
> > Maybe we need to tweak the help text on TFABOOT to be clear that only
> > may be required on a given platform?
> 
> Well, looking more closely now it looks like on STM32 and FSL this makes
> the difference between: Does U-Boot handle the secure side (errata
> handling, provide PSCI services) or is this done by other firmware (TF-A).
> This seems like a legitimate option(*), but this may indeed be better
> explained. I can make a patch for that if this seems useful.

Yes please, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200929/e8df1a24/attachment.sig>