On Thu, 4 Nov 2021 16:56:15 +0000 Peter Hoyes peter.hoyes@arm.com wrote:

From: Peter Hoyes Peter.Hoyes@arm.com

Create a new documentation section for Arm Ltd boards with a sub-page for the VExpress64 boards (FVP-A and Juno).

Many thanks for providing this!

Signed-off-by: Peter Hoyes Peter.Hoyes@arm.com

doc/board/armltd/index.rst | 9 ++++++ doc/board/armltd/vexpress64.rst | 49 +++++++++++++++++++++++++++++++++ doc/board/index.rst | 1 + 3 files changed, 59 insertions(+) create mode 100644 doc/board/armltd/index.rst create mode 100644 doc/board/armltd/vexpress64.rst

diff --git a/doc/board/armltd/index.rst b/doc/board/armltd/index.rst new file mode 100644 index 0000000000..c20d8a0a26 --- /dev/null +++ b/doc/board/armltd/index.rst @@ -0,0 +1,9 @@ +.. SPDX-License-Identifier: GPL-2.0

+Arm Ltd +=============

+.. toctree::

• :maxdepth: 2
• vexpress64.rst

diff --git a/doc/board/armltd/vexpress64.rst b/doc/board/armltd/vexpress64.rst new file mode 100644 index 0000000000..9e97b0e9cd --- /dev/null +++ b/doc/board/armltd/vexpress64.rst @@ -0,0 +1,49 @@ +.. SPDX-License-Identifier: GPL-2.0

+Arm Versatile Express +=====================

+The vexpress_* board configuration supports the following platforms:

• • FVP_Base_RevC-2xAEMvA
• • Juno development board

+Fixed Virtual Platforms +-----------------------

+The Fixed Virtual Platforms (FVP) are complete simulations of an Arm system, +including processor, memory and peripherals. They are set out in a "programmer's +view", which gives a comprehensive model on which to build and test software.

+The supported FVPs are available free of charge and can be downloaded from the +Arm developer site [1]_ (user registration might be required).

+Supported features:

• • GICv3
• • Generic timer
• • PL011 UART

+The default configuration assumes that U-Boot is bootstrapped using a suitable +bootloader.

Can you give an example here, to give people some direction? For instance that it can be included as BL33 in a FIT image created by a TF-A build. Ideally with some example build command line?

Cheers, Andre

+The FVPs can be debugged using Arm Development Studio [2]_.

+Juno +----

+Juno is an Arm development board with the following features:

• • Arm Cortex-A72/A57 and Arm Cortex-A53 in a "big.LITTLE" configuration
• • A PCIe Gen2.0 bus with 4 lanes
• • 8GB of DRAM
• • GICv2

+More details can be found in the board documentation [3]_.

+References +----------

+.. [1] https://developer.arm.com/tools-and-software/simulation-models/fixed-virtual... +.. [2] https://developer.arm.com/tools-and-software/embedded/arm-development-studio +.. [3] https://developer.arm.com/tools-and-software/development-boards/juno-develop... \ No newline at end of file diff --git a/doc/board/index.rst b/doc/board/index.rst index 74ea33e081..78b486538b 100644 --- a/doc/board/index.rst +++ b/doc/board/index.rst @@ -11,6 +11,7 @@ Board-specific doc AndesTech/index amlogic/index apple/index

• armltd/index atmel/index congatec/index coreboot/index

On Thu, 4 Nov 2021 16:56:16 +0000 Peter Hoyes peter.hoyes@arm.com wrote:

From: Peter Hoyes Peter.Hoyes@arm.com

Rename from vexpress_aemv8a.h -> vepxress_aemv8.h as new FVPs may not be v8-A. No change in behavior.

That looks like a reasonable cleanup to me, mostly just changing "is it the FVP, or Juno?" into "is it the Juno, or something else?". And even though this only really makes sense with the (upcoming) v8-R support, I don't see any issues with having this patch already.

This is towards future work to enable support for the FVP_BaseR.

Signed-off-by: Peter Hoyes Peter.Hoyes@arm.com

Reviewed-by: Andre Przywara andre.przywara@arm.com

Cheers, Andre

---

board/armltd/vexpress64/Kconfig | 2 +- doc/README.semihosting | 2 +- .../{vexpress_aemv8a.h => vexpress_aemv8.h} | 48 ++++++++++--------- 3 files changed, 27 insertions(+), 25 deletions(-) rename include/configs/{vexpress_aemv8a.h => vexpress_aemv8.h} (88%)

diff --git a/board/armltd/vexpress64/Kconfig b/board/armltd/vexpress64/Kconfig index 1d13f542e6..4aab3f092e 100644 --- a/board/armltd/vexpress64/Kconfig +++ b/board/armltd/vexpress64/Kconfig @@ -7,7 +7,7 @@ config SYS_VENDOR default "armltd"

config SYS_CONFIG_NAME

• default "vexpress_aemv8a"

• default "vexpress_aemv8"

config JUNO_DTB_PART string "NOR flash partition holding DTB" diff --git a/doc/README.semihosting b/doc/README.semihosting index c019999bed..f382d0131e 100644 --- a/doc/README.semihosting +++ b/doc/README.semihosting @@ -25,7 +25,7 @@ or turning on CONFIG_BASE_FVP for the more full featured model. Rather than create a new armv8 board similar to armltd/vexpress64, add semihosting calls to the existing one, enabled with CONFIG_SEMIHOSTING and CONFIG_BASE_FVP both set. Also reuse the existing board config file -vexpress_aemv8a.h but differentiate the two models by the presence or +vexpress_aemv8.h but differentiate the two models by the presence or absence of CONFIG_BASE_FVP. This change is tested and works on both the Foundation and Base fastmodel simulators.

diff --git a/include/configs/vexpress_aemv8a.h b/include/configs/vexpress_aemv8.h similarity index 88% rename from include/configs/vexpress_aemv8a.h rename to include/configs/vexpress_aemv8.h index df22584d9a..49517a60b0 100644 --- a/include/configs/vexpress_aemv8a.h +++ b/include/configs/vexpress_aemv8.h @@ -4,36 +4,37 @@

• configurations.

*/

-#ifndef __VEXPRESS_AEMV8A_H -#define __VEXPRESS_AEMV8A_H +#ifndef __VEXPRESS_AEMV8_H +#define __VEXPRESS_AEMV8_H

#define CONFIG_REMAKE_ELF

/* Link Definitions */ -#ifdef CONFIG_TARGET_VEXPRESS64_BASE_FVP +#ifdef CONFIG_TARGET_VEXPRESS64_JUNO +#define CONFIG_SYS_INIT_SP_ADDR (CONFIG_SYS_SDRAM_BASE + 0x7fff0) +#else /* ATF loads u-boot here for BASE_FVP model */ #define CONFIG_SYS_INIT_SP_ADDR (CONFIG_SYS_SDRAM_BASE + 0x03f00000) -#elif CONFIG_TARGET_VEXPRESS64_JUNO -#define CONFIG_SYS_INIT_SP_ADDR (CONFIG_SYS_SDRAM_BASE + 0x7fff0) #endif

#define CONFIG_SYS_BOOTM_LEN (64 << 20) /* Increase max gunzip size */ /* CS register bases for the original memory map. */ -#define V2M_PA_CS0 0x00000000 -#define V2M_PA_CS1 0x14000000 -#define V2M_PA_CS2 0x18000000 -#define V2M_PA_CS3 0x1c000000 -#define V2M_PA_CS4 0x0c000000 -#define V2M_PA_CS5 0x10000000 +#define V2M_BASE 0x80000000 +#define V2M_PA_BASE 0x00000000

+#define V2M_PA_CS0 (V2M_PA_BASE + 0x00000000) +#define V2M_PA_CS1 (V2M_PA_BASE + 0x14000000) +#define V2M_PA_CS2 (V2M_PA_BASE + 0x18000000) +#define V2M_PA_CS3 (V2M_PA_BASE + 0x1c000000) +#define V2M_PA_CS4 (V2M_PA_BASE + 0x0c000000) +#define V2M_PA_CS5 (V2M_PA_BASE + 0x10000000)

#define V2M_PERIPH_OFFSET(x) (x << 16) #define V2M_SYSREGS (V2M_PA_CS3 + V2M_PERIPH_OFFSET(1)) #define V2M_SYSCTL (V2M_PA_CS3 + V2M_PERIPH_OFFSET(2)) #define V2M_SERIAL_BUS_PCI (V2M_PA_CS3 + V2M_PERIPH_OFFSET(3)) -#define V2M_BASE 0x80000000

/* Common peripherals relative to CS7. */ #define V2M_AACI (V2M_PA_CS3 + V2M_PERIPH_OFFSET(4)) #define V2M_MMCI (V2M_PA_CS3 + V2M_PERIPH_OFFSET(5)) @@ -72,23 +73,23 @@

/* Generic Interrupt Controller Definitions */ #ifdef CONFIG_GICV3 -#define GICD_BASE (0x2f000000) -#define GICR_BASE (0x2f100000) +#define GICD_BASE (V2M_PA_BASE + 0x2f000000) +#define GICR_BASE (V2M_PA_BASE + 0x2f100000) #else

-#ifdef CONFIG_TARGET_VEXPRESS64_BASE_FVP -#define GICD_BASE (0x2f000000) -#define GICC_BASE (0x2c000000) -#elif CONFIG_TARGET_VEXPRESS64_JUNO +#ifdef CONFIG_TARGET_VEXPRESS64_JUNO #define GICD_BASE (0x2C010000) #define GICC_BASE (0x2C02f000) +#else +#define GICD_BASE (V2M_PA_BASE + 0x2f000000) +#define GICC_BASE (V2M_PA_BASE + 0x2c000000) #endif #endif /* !CONFIG_GICV3 */

#ifndef CONFIG_TARGET_VEXPRESS64_JUNO /* The Vexpress64 simulators use SMSC91C111 */ #define CONFIG_SMC91111 1 -#define CONFIG_SMC91111_BASE (0x01A000000) +#define CONFIG_SMC91111_BASE (V2M_PA_BASE + 0x01A000000) #endif

/* PL011 Serial Configuration */ @@ -113,7 +114,7 @@ #ifdef CONFIG_TARGET_VEXPRESS64_JUNO #define PHYS_SDRAM_2 (0x880000000) #define PHYS_SDRAM_2_SIZE 0x180000000 -#elif CONFIG_TARGET_VEXPRESS64_BASE_FVP && CONFIG_NR_DRAM_BANKS == 2 +#elif CONFIG_NR_DRAM_BANKS == 2 #define PHYS_SDRAM_2 (0x880000000) #define PHYS_SDRAM_2_SIZE 0x80000000 #endif @@ -200,6 +201,7 @@ " booti $kernel_addr - $fdt_addr; " \ "fi" #endif

#endif

/* Monitor Command Prompt */ @@ -213,7 +215,7 @@ /* Store environment at top of flash in the same location as blank.img */ /* in the Juno firmware. */ #else -#define CONFIG_SYS_FLASH_BASE 0x0C000000 +#define CONFIG_SYS_FLASH_BASE (V2M_PA_BASE + 0x0C000000) /* 256 x 256KiB sectors */ #define CONFIG_SYS_MAX_FLASH_SECT 256 /* Store environment at top of flash */ @@ -230,4 +232,4 @@ #define CONFIG_SYS_FLASH_EMPTY_INFO /* flinfo indicates empty blocks */ #define FLASH_MAX_SECTOR_SIZE 0x00040000

-#endif /* __VEXPRESS_AEMV8A_H */ +#endif /* __VEXPRESS_AEMV8_H */

On Thu, 4 Nov 2021 16:56:17 +0000 Peter Hoyes peter.hoyes@arm.com wrote:

From: Peter Hoyes Peter.Hoyes@arm.com

Move env var address values to #defines so they can be reused elsewhere.

Rename env var names to those recommended in the README.

Many thanks for cleaning this up and fixing the wrong variable names. I think we should use opportunity to relax the load addresses, see below.

Fix issue where fdt is called with invalid arguments when booting without a ramdisk.

Signed-off-by: Peter Hoyes Peter.Hoyes@arm.com

include/configs/vexpress_aemv8.h | 50 ++++++++++++++++++++------------ 1 file changed, 31 insertions(+), 19 deletions(-)

diff --git a/include/configs/vexpress_aemv8.h b/include/configs/vexpress_aemv8.h index 49517a60b0..48c21082a6 100644 --- a/include/configs/vexpress_aemv8.h +++ b/include/configs/vexpress_aemv8.h @@ -7,6 +7,8 @@ #ifndef __VEXPRESS_AEMV8_H #define __VEXPRESS_AEMV8_H

+#include <linux/stringify.h>

#define CONFIG_REMAKE_ELF

/* Link Definitions */ @@ -172,33 +174,43 @@ BOOTENV

#elif CONFIG_TARGET_VEXPRESS64_BASE_FVP

+#define VEXPRESS_FDT_ADDR 0x83000000

So this layout is somewhat miserly. My debug kernel of the day is 42MB already, so the 47.5 MB we have reserved for the kernel now sound a bit tight. In arm64 we don't have real constraints, and the model has surely enough RAM, so can we become more generous? - Kernel at 512KB (for compatibility with older Linux versions) - ramdisk just below (or at?) 256MB - everything else (script, DTB) just below the ramdisk

Cheers, Andre

+#define VEXPRESS_RAMDISK_ADDR 0x88000000 +#define VEXPRESS_KERNEL_ADDR 0x80080000 +#define VEXPRESS_BOOT_ADDR 0x8007f800

#define CONFIG_EXTRA_ENV_SETTINGS \ "kernel_name=Image\0" \

• 		"kernel_addr=0x80080000\0"	\
  

• 		"initrd_name=ramdisk.img\0"	\
  

• 		"initrd_addr=0x88000000\0"	\
  

• 		"fdtfile=devtree.dtb\0"		\
  

• 		"fdt_addr=0x83000000\0"		\
  

• 		"boot_name=boot.img\0"		\
  

• 		"boot_addr=0x8007f800\0"
  

• 		"kernel_addr_r=" __stringify(VEXPRESS_KERNEL_ADDR) "\0"	\
  

• 		"ramdisk_name=ramdisk.img\0"	\
  

• 		"ramdisk_addr_r=" __stringify(VEXPRESS_RAMDISK_ADDR) "\0" \
  

• 		"fdtfile=devtree.dtb\0"	\
  

• 		"fdt_addr_r=" __stringify(VEXPRESS_FDT_ADDR) "\0"	\
  

• 		"boot_name=boot.img\0" \
  

• 		"boot_addr_r=" __stringify(VEXPRESS_BOOT_ADDR) "\0"
  
  

#ifndef CONFIG_BOOTCOMMAND -#define CONFIG_BOOTCOMMAND "if smhload ${boot_name} ${boot_addr}; then " \ +#define CONFIG_BOOTCOMMAND "if smhload ${boot_name} ${boot_addr_r}; then " \ " set bootargs; " \

• 		"  abootimg addr ${boot_addr}; " \
  

• 		"  abootimg get dtb --index=0 fdt_addr; " \
  

• 		"  bootm ${boot_addr} ${boot_addr} " \
  

• 		"  ${fdt_addr}; " \
  

• 		"  abootimg addr ${boot_addr_r}; " \
  

• 		"  abootimg get dtb --index=0 fdt_addr_r; " \
  

• 		"  bootm ${boot_addr_r} ${boot_addr_r} " \
  

• 		"  ${fdt_addr_r}; " \
  	"else; " \
  	"  set fdt_high 0xffffffffffffffff; " \
  	"  set initrd_high 0xffffffffffffffff; " \
  

• 		"  smhload ${kernel_name} ${kernel_addr}; " \
  

• 		"  smhload ${fdtfile} ${fdt_addr}; " \
  

• 		"  smhload ${initrd_name} ${initrd_addr} "\
  

• 		"  initrd_end; " \
  

• 		"  fdt addr ${fdt_addr}; fdt resize; " \
  

• 		"  fdt chosen ${initrd_addr} ${initrd_end}; " \
  

• 		"  booti $kernel_addr - $fdt_addr; " \
  

• 		"  smhload ${kernel_name} ${kernel_addr_r}; " \
  

• 		"  smhload ${fdtfile} ${fdt_addr_r}; " \
  

• 		"  smhload ${ramdisk_name} ${ramdisk_addr_r} "\
  

• 		"  ramdisk_end; " \
  

• 		"  fdt addr ${fdt_addr_r}; fdt resize; " \
  

• 		"  if test -n ${ramdisk_end}; then "\
  

• 		"    fdt chosen ${ramdisk_addr_r} ${ramdisk_end}; " \
  

• 		"  else; " \
  

• 		"    fdt chosen; " \
  

• 		"  fi; " \
  

• 		"  booti $kernel_addr_r - $fdt_addr_r; " \
  	"fi"
  

#endif

On Thu, 4 Nov 2021 16:56:18 +0000 Peter Hoyes peter.hoyes@arm.com wrote:

Hi,

From: Peter Hoyes Peter.Hoyes@arm.com

Capture x0 in lowlevel_init.S as potential fdt address. Modify board_fdt_blob_setup to use fdt address from either vexpress_aemv8.h or lowlevel_init.S.

Signed-off-by: Peter Hoyes Peter.Hoyes@arm.com

board/armltd/vexpress64/Makefile | 5 +++++ board/armltd/vexpress64/lowlevel_init.S | 12 ++++++++++++ board/armltd/vexpress64/vexpress64.c | 24 ++++++++++++++++++++++++ 3 files changed, 41 insertions(+) create mode 100644 board/armltd/vexpress64/lowlevel_init.S

diff --git a/board/armltd/vexpress64/Makefile b/board/armltd/vexpress64/Makefile index 868dc4f629..5703e75967 100644 --- a/board/armltd/vexpress64/Makefile +++ b/board/armltd/vexpress64/Makefile @@ -5,3 +5,8 @@

obj-y := vexpress64.o obj-$(CONFIG_TARGET_VEXPRESS64_JUNO) += pcie.o +ifdef CONFIG_OF_BOARD +ifndef CONFIG_TARGET_VEXPRESS64_JUNO +obj-y += lowlevel_init.o

I wonder if it hurts to avoid all these confusing conditionals and just always include this, even for Juno? Maybe we can use x0 even on the Juno at some day?

+endif +endif diff --git a/board/armltd/vexpress64/lowlevel_init.S b/board/armltd/vexpress64/lowlevel_init.S new file mode 100644 index 0000000000..3dcfb85d0e --- /dev/null +++ b/board/armltd/vexpress64/lowlevel_init.S @@ -0,0 +1,12 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/*

• • (C) Copyright 2021 Arm Limited
• */

+.global save_boot_params +save_boot_params:

• adr x8, prior_stage_fdt_address
• str x0, [x8]
• b save_boot_params_ret

diff --git a/board/armltd/vexpress64/vexpress64.c b/board/armltd/vexpress64/vexpress64.c index d2f307cca5..bde6ef1ba3 100644 --- a/board/armltd/vexpress64/vexpress64.c +++ b/board/armltd/vexpress64/vexpress64.c @@ -86,6 +86,8 @@ int dram_init_banksize(void) }

#ifdef CONFIG_OF_BOARD

Do we still need this? Every vexpress64 platform should now rely on OF_BOARD?

+#ifdef CONFIG_TARGET_VEXPRESS64_JUNO #define JUNO_FLASH_SEC_SIZE (256 * 1024) static phys_addr_t find_dtb_in_nor_flash(const char *partname) { @@ -130,9 +132,19 @@ static phys_addr_t find_dtb_in_nor_flash(const char *partname)

return ~0; } +#else

As suggested above, we probably should always include this variable below, so turn the #else into an #endif?

+/* Assigned in lowlevel_init.S

• • Push the variable into the .data section so that it
• • does not get cleared later.
• */

+unsigned long __section(".data") prior_stage_fdt_address;

+#endif

void *board_fdt_blob_setup(int *err) { +#ifdef CONFIG_TARGET_VEXPRESS64_JUNO phys_addr_t fdt_rom_addr = find_dtb_in_nor_flash(CONFIG_JUNO_DTB_PART);

*err = 0; @@ -142,6 +154,18 @@ void *board_fdt_blob_setup(int *err) }

return (void *)fdt_rom_addr; +#else

Can you turn this into an #endif?

• if (fdt_magic(VEXPRESS_FDT_ADDR) == FDT_MAGIC) {

• *err = 0;
  

• return (void *)VEXPRESS_FDT_ADDR;
  

"else" after an unconditional return is somewhat frowned upon, since it gives a wrong impression about the code flow.

What about: #ifdef VEXPRESS_FDT_ADDR if (fdt_magic(VEXPRESS_FDT_ADDR) ... { ... return ...; } #endif

• } else if (fdt_magic(prior_stage_fdt_address) == FDT_MAGIC) {

• *err = 0;
  

• return (void *)prior_stage_fdt_address;
  

• } else {

If we always include prior_stage_fdt_address (even though it may be unused), you can just always include this piece. And lose the else here, since we return inside the if branch.

Cheers, Andre

• *err = -ENXIO;
  

• return NULL;
  

• }

+#endif } #endif